From dd2ec74d964f8cf84bf4ab7b2d8fb59604c32e27 Mon Sep 17 00:00:00 2001 From: bircni Date: Sun, 18 Oct 2026 20:40:25 +0200 Subject: [PATCH] fix(team,repo,org,wiki,auth): detect a kept admin permission, validate topics up front, page public members, name disabled wikis, treat auth switch's account as --user Signed-off-by: bircni --- internal/app/auth.go | 25 +++++--- internal/app/auth_test.go | 13 ++++ internal/app/repo_mutations.go | 58 ++++++++++++++--- internal/app/repository_edit_test.go | 55 ++++++++++++++-- internal/app/teams.go | 4 ++ internal/app/teams_test.go | 6 +- internal/app/wiki_test.go | 2 + internal/services/organizations.go | 85 +++++++++++++++++++------ internal/services/organizations_test.go | 29 +++++++++ internal/services/repositories.go | 14 ++-- internal/services/wiki.go | 43 +++++++++---- internal/services/wiki_test.go | 19 ++++++ 12 files changed, 289 insertions(+), 64 deletions(-) diff --git a/internal/app/auth.go b/internal/app/auth.go index 5d150a4f..7241d71a 100644 --- a/internal/app/auth.go +++ b/internal/app/auth.go @@ -655,6 +655,9 @@ selects another. With two accounts, tea auth switch toggles between them; with m asks which one on a terminal.`, Example: "$ tea auth switch\n$ tea auth switch bob\n$ tea auth switch bob --host gitea.com", RunE: func(cmd *cobra.Command, args []string) error { + if err := a.positionalAccount(args); err != nil { + return err + } t, err := a.instanceTarget(cmd.Context()) if err != nil { return err @@ -665,9 +668,6 @@ asks which one on a terminal.`, } names := slices.Sorted(maps.Keys(users)) requested := a.options.user - if len(args) == 1 { - requested = args[0] - } var user string switch { case requested != "": @@ -710,6 +710,18 @@ asks which one on a terminal.`, } } +// positionalAccount treats the account argument of auth switch and auth logout like --user. +func (a *application) positionalAccount(args []string) error { + if len(args) == 0 { + return nil + } + if a.options.user != "" && !strings.EqualFold(a.options.user, args[0]) { + return clierr.Wrap(clierr.Usage, fmt.Errorf("--user %s and the account %s differ", a.options.user, args[0])) + } + a.options.user = args[0] + return nil +} + // chooseAccount asks which of the accounts of instance to use, or explains how to name one. func (a *application) chooseAccount(cmd *cobra.Command, instance string, names []string, active string) (string, error) { chooser, ok := a.chooser() @@ -735,11 +747,8 @@ other credential helpers are asked to forget what they stored for the account on instance, such as a copy of its token that a keychain kept after a push.`, Example: "$ tea auth logout\n$ tea auth logout bob --host gitea.com\n# Without confirmation\n$ tea auth logout alice --yes", RunE: func(cmd *cobra.Command, args []string) error { - if len(args) == 1 { - if a.options.user != "" && !strings.EqualFold(a.options.user, args[0]) { - return clierr.Wrap(clierr.Usage, fmt.Errorf("--user %s and the account %s differ", a.options.user, args[0])) - } - a.options.user = args[0] + if err := a.positionalAccount(args); err != nil { + return err } t, err := a.instanceTarget(cmd.Context()) if err != nil { diff --git a/internal/app/auth_test.go b/internal/app/auth_test.go index fbdbbca8..135ba542 100644 --- a/internal/app/auth_test.go +++ b/internal/app/auth_test.go @@ -860,3 +860,16 @@ func TestAuthLoginSSHKeyNeverAsksTheVersionOfAPrivateInstance(t *testing.T) { "auth", "login", "--host", server.URL, "--ssh-key", keyPath, "--no-input", "--yes", })) } + +func TestAuthSwitchTreatsThePositionalAccountLikeUser(t *testing.T) { + const instance = "https://example.org" + deps, _, _ := terminalDeps(t) + configureAccounts(t, deps, instance, "alice", "alice", "bob") + err := Execute(context.Background(), deps, []string{"auth", "switch", "bob", "--user", "alice"}) + require.Equal(t, clierr.Usage, clierr.ExitCode(err), "%v", err) + require.EqualError(t, err, "--user alice and the account bob differ") + s, err := deps.Settings.Load() + require.NoError(t, err) + require.Equal(t, "alice", s.Hosts.Hosts[instance].User) + require.NoError(t, Execute(context.Background(), deps, []string{"auth", "switch", "BOB", "--user", "bob"})) +} diff --git a/internal/app/repo_mutations.go b/internal/app/repo_mutations.go index 4d44b5c6..cc4b76e9 100644 --- a/internal/app/repo_mutations.go +++ b/internal/app/repo_mutations.go @@ -10,6 +10,7 @@ import ( "net/url" "path/filepath" "regexp" + "slices" "strconv" "strings" @@ -99,6 +100,25 @@ var repositorySwitches = []struct { // validTopic is the form Gitea accepts for a lowercased topic name. var validTopic = regexp.MustCompile(`^[a-z0-9][-.a-z0-9]{0,34}$`) +// maxTopics is the number of topics Gitea allows per repository. +const maxTopics = 25 + +// finalTopics returns current without the removed topics and with the added ones, lowercased like Gitea does. +func finalTopics(current, add, remove []string) []string { + removed := make(map[string]bool, len(remove)) + for _, topic := range remove { + removed[strings.ToLower(topic)] = true + } + var topics []string + for _, topic := range slices.Concat(current, add) { + topic = strings.ToLower(topic) + if !removed[topic] && !slices.Contains(topics, topic) { + topics = append(topics, topic) + } + } + return topics +} + func (a *application) repoEditCommand() *cobra.Command { var name, website, defaultBranch, description, visibility, mergeStyle string var addTopics, removeTopics []string @@ -162,6 +182,9 @@ $ tea repo edit --default-merge-style squash --add-topic cli --add-topic go`, return clierr.Wrap(clierr.Usage, fmt.Errorf("invalid topic %q: use up to 35 letters, digits, - or ., starting with a letter or digit", topic)) } } + if len(addTopics) > maxTopics { + return clierr.Wrap(clierr.Usage, fmt.Errorf("a repository has at most %d topics, got %d to add", maxTopics, len(addTopics))) + } if options == (services.RepositoryEdit{}) && len(addTopics)+len(removeTopics) == 0 { return clierr.Wrap(clierr.Usage, errors.New("supply a setting to change; see tea repo edit --help")) } @@ -181,6 +204,20 @@ $ tea repo edit --default-merge-style squash --add-topic cli --add-topic go`, } } } + // Topics are replaced in one request; compute and check them before anything changes. + var topics []string + setTopics := false + if len(addTopics)+len(removeTopics) > 0 { + current, err := service.Topics(cmd.Context(), t.Repo) + if err != nil { + return fmt.Errorf("list topics: %w", err) + } + topics = finalTopics(current, addTopics, removeTopics) + if len(topics) > maxTopics { + return clierr.Wrap(clierr.Usage, fmt.Errorf("a repository has at most %d topics, the edit would leave %d", maxTopics, len(topics))) + } + setTopics = !slices.Equal(slices.Sorted(slices.Values(topics)), slices.Sorted(slices.Values(finalTopics(current, nil, nil)))) + } // Gitea refuses topic changes on an archived repository, so archiving comes last. repo, archive := t.Repo, options.Archived != nil && *options.Archived if archive { @@ -193,26 +230,27 @@ $ tea repo edit --default-merge-style squash --add-topic cli --add-topic go`, } options.Archived = nil } - if options != (services.RepositoryEdit{}) { + settings := options != (services.RepositoryEdit{}) + if settings { item, err := service.Edit(cmd.Context(), t.Repo, options) if err != nil { return err } repo = item.FullName } - for _, topic := range addTopics { - if err := service.AddTopic(cmd.Context(), repo, topic); err != nil { - return fmt.Errorf("add topic %q: %w", topic, err) - } - } - for _, topic := range removeTopics { - // A topic the repository does not have is already removed. - if err := service.RemoveTopic(cmd.Context(), repo, topic); err != nil && !services.IsNotFound(err) { - return fmt.Errorf("remove topic %q: %w", topic, err) + if setTopics { + if err := service.SetTopics(cmd.Context(), repo, topics); err != nil { + if settings { + return fmt.Errorf("repository settings were changed, but setting the topics failed: %w", err) + } + return fmt.Errorf("set topics: %w", err) } } if archive { if _, err := service.Edit(cmd.Context(), repo, services.RepositoryEdit{Archived: &archive}); err != nil { + if settings || setTopics { + return fmt.Errorf("the other changes were applied, but archiving failed: %w", err) + } return err } } diff --git a/internal/app/repository_edit_test.go b/internal/app/repository_edit_test.go index 6e9c7964..3588cfdc 100644 --- a/internal/app/repository_edit_test.go +++ b/internal/app/repository_edit_test.go @@ -5,6 +5,8 @@ package app import ( "context" + "encoding/json" + "fmt" "io" "net/http" "strings" @@ -54,15 +56,23 @@ func TestRepositoryEditReportsTheChangedSettings(t *testing.T) { var requests []string server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { requests = append(requests, r.Method+" "+r.URL.Path) - if r.Method == http.MethodPatch { + switch r.Method + " " + r.URL.Path { + case "PATCH /api/v1/repos/alice/project": requirePayload(t, `{"description":"CLI","private":true}`, r.Body) + case "GET /api/v1/repos/alice/project/topics": + _, _ = w.Write([]byte(`{"topics":["old","keep"]}`)) + return + case "PUT /api/v1/repos/alice/project/topics": + requirePayload(t, `{"topics":["keep","cli","go"]}`, r.Body) + w.WriteHeader(http.StatusNoContent) + return } _, _ = w.Write([]byte(`{"full_name":"alice/project"}`)) })) defer server.Close() deps, out, diagnostics := repoTTYDeps(t, server.URL) require.NoError(t, Execute(context.Background(), deps, []string{"repo", "edit", "alice/project", "-d", "CLI", "--visibility", "private", "--add-topic", "cli,go", "--remove-topic", "old"})) - require.Equal(t, []string{"PATCH /api/v1/repos/alice/project", "PUT /api/v1/repos/alice/project/topics/cli", "PUT /api/v1/repos/alice/project/topics/go", "DELETE /api/v1/repos/alice/project/topics/old"}, requests) + require.Equal(t, []string{"GET /api/v1/repos/alice/project/topics", "PATCH /api/v1/repos/alice/project", "PUT /api/v1/repos/alice/project/topics"}, requests) require.Empty(t, out.String()) require.Equal(t, "✓ Edited repository alice/project (add-topic, description, remove-topic, visibility)\n", diagnostics.String()) } @@ -82,8 +92,9 @@ func TestRepositoryEditArchivesAfterTheOtherChanges(t *testing.T) { deps, _, _ := serverDeps(t, server.URL) require.NoError(t, Execute(context.Background(), deps, []string{"repo", "edit", "alice/project", "-d", "CLI", "--archived", "--add-topic", "cli", "--yes"})) require.Equal(t, []string{ + "GET /api/v1/repos/alice/project/topics", "PATCH /api/v1/repos/alice/project", - "PUT /api/v1/repos/alice/project/topics/cli", + "PUT /api/v1/repos/alice/project/topics", "PATCH /api/v1/repos/alice/project archived", }, requests) } @@ -128,14 +139,44 @@ func TestRepositoryEditIgnoresRemovingAMissingTopic(t *testing.T) { var requests []string server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { requests = append(requests, r.Method+" "+r.URL.Path) - if r.Method == http.MethodDelete { - w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"full_name":"alice/project","topics":["other"]}`)) + })) + defer server.Close() + deps, _, _ := serverDeps(t, server.URL) + require.NoError(t, Execute(context.Background(), deps, []string{"repo", "edit", "alice/project", "-d", "new", "--remove-topic", "legacy"})) + require.Equal(t, []string{"GET /api/v1/repos/alice/project/topics", "PATCH /api/v1/repos/alice/project"}, requests) +} + +func TestRepositoryEditRejectsTooManyTopicsBeforeChangingAnything(t *testing.T) { + var requests []string + server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests = append(requests, r.Method+" "+r.URL.Path) + var topics []string + for i := range 25 { + topics = append(topics, fmt.Sprintf("t%d", i)) + } + data, _ := json.Marshal(map[string]any{"topics": topics}) + _, _ = w.Write(data) + })) + defer server.Close() + deps, _, _ := serverDeps(t, server.URL) + err := Execute(context.Background(), deps, []string{"repo", "edit", "alice/project", "-d", "new", "--add-topic", "extra"}) + require.Equal(t, clierr.Usage, clierr.ExitCode(err), "%v", err) + require.ErrorContains(t, err, "at most 25 topics") + require.Equal(t, []string{"GET /api/v1/repos/alice/project/topics"}, requests) +} + +func TestRepositoryEditReportsSettingsAppliedWhenTheTopicsFail(t *testing.T) { + server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPut { + w.WriteHeader(http.StatusUnprocessableEntity) + _, _ = w.Write([]byte(`{"message":"Topic names are invalid"}`)) return } _, _ = w.Write([]byte(`{"full_name":"alice/project"}`)) })) defer server.Close() deps, _, _ := serverDeps(t, server.URL) - require.NoError(t, Execute(context.Background(), deps, []string{"repo", "edit", "alice/project", "-d", "new", "--remove-topic", "legacy"})) - require.Equal(t, []string{"PATCH /api/v1/repos/alice/project", "DELETE /api/v1/repos/alice/project/topics/legacy"}, requests) + err := Execute(context.Background(), deps, []string{"repo", "edit", "alice/project", "-d", "new", "--add-topic", "cli"}) + require.ErrorContains(t, err, "repository settings were changed, but setting the topics failed") } diff --git a/internal/app/teams.go b/internal/app/teams.go index ffa79284..2c6d1eec 100644 --- a/internal/app/teams.go +++ b/internal/app/teams.go @@ -434,6 +434,10 @@ $ tea team edit acme/developers --name devs --description "Core developers"`, return err } edited.Organization = team.Organization + // Some Gitea versions keep an admin team admin when permission and units_map are sent together. + if opts.Permission != nil && edited.Permission != "" && edited.Permission != *opts.Permission { + return fmt.Errorf("team %s was edited, but its permission is %s instead of the requested %s; other changes were applied", edited.Slug(), edited.Permission, *opts.Permission) + } a.done(cmd, "Edited team %s (ID %d)", edited.Slug(), edited.ID) return a.jsonResult(cmd, edited) }, diff --git a/internal/app/teams_test.go b/internal/app/teams_test.go index 117e86cd..8c326f87 100644 --- a/internal/app/teams_test.go +++ b/internal/app/teams_test.go @@ -199,7 +199,11 @@ func TestTeamPermissionSetsEveryUnit(t *testing.T) { require.NoError(t, json.NewDecoder(r.Body).Decode(&body)) sent = append(sent, body) } - _, _ = w.Write([]byte(`{"id":5,"name":"devs","permission":"none","organization":{"username":"acme"}}`)) + permission := "none" + if len(sent) > 0 { + permission, _ = sent[len(sent)-1]["permission"].(string) + } + _, _ = w.Write([]byte(`{"id":5,"name":"devs","permission":"` + permission + `","organization":{"username":"acme"}}`)) })) defer server.Close() deps, _, _ := serverDeps(t, server.URL) diff --git a/internal/app/wiki_test.go b/internal/app/wiki_test.go index 23e8d65b..e9a51569 100644 --- a/internal/app/wiki_test.go +++ b/internal/app/wiki_test.go @@ -80,6 +80,8 @@ func TestWikiEditRefusesToOverwriteOrCreate(t *testing.T) { case r.Method == http.MethodPatch: patched++ _, _ = w.Write([]byte(`{"title":"Getting Started","content_base64":"eA=="}`)) + case r.URL.Path == "/api/v1/repos/alice/x": + _, _ = w.Write([]byte(`{"full_name":"alice/x","has_wiki":true}`)) case r.URL.Path == "/api/v1/repos/alice/x/wiki/page/Draft": _, _ = w.Write([]byte(`{"title":"Draft","content_base64":"ZHJhZnQ="}`)) case r.URL.Path == "/api/v1/repos/alice/x/wiki/page/Page B": diff --git a/internal/services/organizations.go b/internal/services/organizations.go index c1526973..d87d20f1 100644 --- a/internal/services/organizations.go +++ b/internal/services/organizations.go @@ -9,6 +9,7 @@ import ( "fmt" "net/http" "net/url" + "slices" "strconv" "sync" @@ -210,40 +211,84 @@ type MemberListOptions struct { Limit, Page int } +// publicScanPages bounds the server pages Members reads to find public memberships. +const publicScanPages = 20 + // Members lists the members of an organization. func (s Organizations) Members(ctx context.Context, name string, opts MemberListOptions) (Page[OrganizationMember], error) { - users, err := callPage(ctx, s.Client, opts.Limit, opts.Page, func(ctx context.Context, c *gitea.Client, list gitea.ListOptions) ([]*gitea.User, *gitea.Response, error) { + if opts.Public { + return s.publicMembers(ctx, name, opts) + } + members, err := s.memberPage(ctx, name, opts.Limit, opts.Page, false) + if err != nil || !opts.Visibility { + return members, err + } + members.Items, _, err = s.withVisibility(ctx, name, members.Items) + return members, err +} + +// memberPage lists one server page of members, or of the memberships Gitea reports as public. +func (s Organizations) memberPage(ctx context.Context, name string, limit, page int, public bool) (Page[OrganizationMember], error) { + users, err := callPage(ctx, s.Client, limit, page, func(ctx context.Context, c *gitea.Client, list gitea.ListOptions) ([]*gitea.User, *gitea.Response, error) { membership := gitea.ListOrgMembershipOption{ListOptions: list} - if opts.Public { + if public { return c.Organizations.ListPublicOrgMembership(ctx, name, membership) } return c.Organizations.ListOrgMembership(ctx, name, membership) }) - members := convertPage(users, memberFrom) - if err != nil || !(opts.Public || opts.Visibility) { - return members, err - } - public := make([]bool, len(members.Items)) - err = concurrently(len(members.Items), func(i int) (err error) { - public[i], err = s.IsPublicMember(ctx, name, members.Items[i].Username) + return convertPage(users, memberFrom), err +} + +// withVisibility checks the membership of each member and reports which ones are public. +func (s Organizations) withVisibility(ctx context.Context, name string, members []OrganizationMember) ([]OrganizationMember, []bool, error) { + public := make([]bool, len(members)) + err := concurrently(len(members), func(i int) (err error) { + public[i], err = s.IsPublicMember(ctx, name, members[i].Username) return err }) if err != nil { - return Page[OrganizationMember]{}, err + return nil, nil, err } - shown := members.Items[:0] - for i, member := range members.Items { - member.Visibility = MembershipConcealed + members = slices.Clone(members) + for i := range members { + members[i].Visibility = MembershipConcealed if public[i] { - member.Visibility = MembershipPublic - } - if !opts.Public || public[i] { - shown = append(shown, member) + members[i].Visibility = MembershipPublic } } - members.Items = shown - members.Counted = members.Counted && !opts.Public - return members, nil + return members, public, nil +} + +// publicMembers lists the public memberships. Gitea lists concealed members as public to +// administrators, so it reads server pages until the requested page of confirmed ones is full. +func (s Organizations) publicMembers(ctx context.Context, name string, opts MemberListOptions) (Page[OrganizationMember], error) { + const size = 50 + skip := (opts.Page - 1) * opts.Limit + var matches []OrganizationMember + more := false + for page := 1; page <= publicScanPages && len(matches) <= skip+opts.Limit; page++ { + batch, err := s.memberPage(ctx, name, size, page, true) + if err != nil { + return Page[OrganizationMember]{}, err + } + checked, public, err := s.withVisibility(ctx, name, batch.Items) + if err != nil { + return Page[OrganizationMember]{}, err + } + for i, member := range checked { + if public[i] { + matches = append(matches, member) + } + } + more = batch.More + if !more { + break + } + } + // Running out of the scan bound leaves later pages unread, which counts as more results. + more = more || len(matches) > skip+opts.Limit + matches = matches[min(len(matches), skip):] + return Page[OrganizationMember]{Items: matches[:min(len(matches), opts.Limit)], More: more}, nil } // concurrentRequests bounds the requests that concurrently keeps in flight. diff --git a/internal/services/organizations_test.go b/internal/services/organizations_test.go index e86ddef0..1189a642 100644 --- a/internal/services/organizations_test.go +++ b/internal/services/organizations_test.go @@ -6,7 +6,9 @@ package services import ( "context" "errors" + "fmt" "net/http" + "strings" "testing" "gitea.dev/tea/internal/transport" @@ -194,3 +196,30 @@ func TestOrganizationMembersCheckEachMembership(t *testing.T) { }) } } + +func TestOrganizationPublicMembersReadLaterPages(t *testing.T) { + // The first server page holds only concealed members; a public one follows on page 2. + client := newTestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.URL.Path == "/api/v1/orgs/acme/public_members" && r.URL.Query().Get("page") == "1": + var users []string + for i := range 50 { + users = append(users, fmt.Sprintf(`{"id":%d,"login":"hidden%d"}`, i+10, i)) + } + w.Header().Set("Link", `; rel="next"`) + _, _ = w.Write([]byte("[" + strings.Join(users, ",") + "]")) + case r.URL.Path == "/api/v1/orgs/acme/public_members": + _, _ = w.Write([]byte(`[{"id":1,"login":"alice"}]`)) + case r.URL.Path == "/api/v1/orgs/acme/public_members/alice": + w.WriteHeader(http.StatusNoContent) + case strings.HasPrefix(r.URL.Path, "/api/v1/orgs/acme/public_members/hidden"): + w.WriteHeader(http.StatusNotFound) + default: + t.Errorf("unexpected request %s %s", r.Method, r.URL.Path) + } + })) + members, err := (Organizations{Client: client}).Members(context.Background(), "acme", MemberListOptions{Public: true, Visibility: true, Limit: 1, Page: 1}) + require.NoError(t, err) + require.Equal(t, []OrganizationMember{{ID: 1, Username: "alice", Visibility: MembershipPublic}}, members.Items) + require.False(t, members.More) +} diff --git a/internal/services/repositories.go b/internal/services/repositories.go index c764cef8..b3073c05 100644 --- a/internal/services/repositories.go +++ b/internal/services/repositories.go @@ -273,17 +273,17 @@ func (s Repositories) Edit(ctx context.Context, repo string, opts RepositoryEdit return item, Classify(err, "repository", repo, "") } -// AddTopic adds a topic to the repository. -func (s Repositories) AddTopic(ctx context.Context, repo, topic string) error { - return repoDo(ctx, s.Client, repo, func(ctx context.Context, c *gitea.Client, owner, name string) (*gitea.Response, error) { - return c.Repositories.AddRepoTopic(ctx, owner, name, topic) +// Topics lists the topics of the repository. Gitea allows at most 25, so one page holds them all. +func (s Repositories) Topics(ctx context.Context, repo string) ([]string, error) { + return repoCall(ctx, s.Client, repo, func(ctx context.Context, c *gitea.Client, owner, name string) ([]string, *gitea.Response, error) { + return c.Repositories.ListRepoTopics(ctx, owner, name, gitea.ListRepoTopicsOptions{ListOptions: gitea.ListOptions{Page: 1, PageSize: 50}}) }) } -// RemoveTopic removes a topic from the repository. -func (s Repositories) RemoveTopic(ctx context.Context, repo, topic string) error { +// SetTopics replaces the topics of the repository. +func (s Repositories) SetTopics(ctx context.Context, repo string, topics []string) error { return repoDo(ctx, s.Client, repo, func(ctx context.Context, c *gitea.Client, owner, name string) (*gitea.Response, error) { - return c.Repositories.DeleteRepoTopic(ctx, owner, name, topic) + return c.Repositories.SetRepoTopics(ctx, owner, name, topics) }) } diff --git a/internal/services/wiki.go b/internal/services/wiki.go index adcdd098..7934cd2c 100644 --- a/internal/services/wiki.go +++ b/internal/services/wiki.go @@ -133,8 +133,34 @@ func (s Wiki) ListPage(ctx context.Context, limit, page int) (Page[WikiPage], er return Page[WikiPage]{Items: items, More: page*limit < len(all), Total: len(all), Counted: true}, err } +// disabled explains a 404 from the wiki API. Gitea answers a bare 404 when the wiki is disabled or +// the repository is missing, which says nothing about the page itself. It returns nil for any other +// error, and the repository lookup's error when the repository cannot be read. +func (s Wiki) disabled(ctx context.Context, err error) error { + if !IsNotFound(err) { + return nil + } + repository, checkErr := (Repositories{Client: s.Client}).get(ctx, s.Repository) + if checkErr != nil { + return checkErr + } + if !repository.HasWiki { + return fmt.Errorf("the wiki of %s is disabled; turn it on with: tea repo edit --enable-wiki -R %s", s.Repository, s.Repository) + } + return nil +} + // View retrieves and decodes a wiki page. func (s Wiki) View(ctx context.Context, name string) (WikiPage, error) { + item, err := s.lookup(ctx, name) + if err := s.disabled(ctx, err); err != nil { + return WikiPage{}, err + } + return item, err +} + +// lookup is View without the check for a disabled wiki. +func (s Wiki) lookup(ctx context.Context, name string) (WikiPage, error) { item, err := s.page(ctx, func(ctx context.Context, c *gitea.Client, owner, repo string) (*gitea.WikiPage, *gitea.Response, error) { return c.Wiki.GetPage(ctx, owner, repo, name) }) @@ -150,16 +176,8 @@ func (s Wiki) Create(ctx context.Context, opts WikiOptions) (WikiPage, error) { item, err := s.page(ctx, func(ctx context.Context, c *gitea.Client, owner, repo string) (*gitea.WikiPage, *gitea.Response, error) { return c.Wiki.CreatePage(ctx, owner, repo, wikiOptions(opts)) }) - if IsNotFound(err) { - // Gitea answers a bare 404 when the wiki is disabled or the repository is missing, which - // says nothing about the page being created. - repository, checkErr := (Repositories{Client: s.Client}).get(ctx, s.Repository) - if checkErr != nil { - return WikiPage{}, checkErr - } - if !repository.HasWiki { - return WikiPage{}, fmt.Errorf("the wiki of %s is disabled; turn it on with: tea repo edit --enable-wiki -R %s", s.Repository, s.Repository) - } + if err := s.disabled(ctx, err); err != nil { + return WikiPage{}, err } return item, Classify(err, "wiki page", strconv.Quote(opts.Title), s.Repository) } @@ -177,7 +195,7 @@ func (s Wiki) Edit(ctx context.Context, name string, opts WikiOptions) (WikiPage opts.Title = current.Title } if renamed { - existing, err := s.View(ctx, opts.Title) + existing, err := s.lookup(ctx, opts.Title) if err == nil && existing.Title != current.Title { return WikiPage{}, false, &ConflictError{Reason: ReasonExists, Kind: "wiki page", Key: strconv.Quote(opts.Title), Scope: s.Repository} } @@ -196,6 +214,9 @@ func (s Wiki) Delete(ctx context.Context, name string) error { err := repoDo(ctx, s.Client, s.Repository, func(ctx context.Context, c *gitea.Client, owner, repo string) (*gitea.Response, error) { return c.Wiki.DeletePage(ctx, owner, repo, name) }) + if err := s.disabled(ctx, err); err != nil { + return err + } return Classify(err, "wiki page", strconv.Quote(name), s.Repository) } diff --git a/internal/services/wiki_test.go b/internal/services/wiki_test.go index 05ecd903..fba18061 100644 --- a/internal/services/wiki_test.go +++ b/internal/services/wiki_test.go @@ -134,3 +134,22 @@ func TestWikiCreateExplainsADisabledWiki(t *testing.T) { require.ErrorContains(t, err, "the wiki of alice/project is disabled") require.NotErrorAs(t, err, new(*NotFoundError)) } + +func TestWikiViewEditAndDeleteExplainADisabledWiki(t *testing.T) { + client := newTestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/api/v1/repos/alice/project" { + _, _ = w.Write([]byte(`{"full_name":"alice/project","has_wiki":false}`)) + return + } + http.NotFound(w, r) + })) + service := Wiki{Client: client, Repository: "alice/project"} + _, viewErr := service.View(context.Background(), "Home") + _, _, editErr := service.Edit(context.Background(), "Home", WikiOptions{Body: "body"}) + deleteErr := service.Delete(context.Background(), "Home") + for _, err := range []error{viewErr, editErr, deleteErr} { + require.ErrorContains(t, err, "the wiki of alice/project is disabled") + require.ErrorContains(t, err, "tea repo edit --enable-wiki") + require.NotErrorAs(t, err, new(*NotFoundError)) + } +}