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 <bircni@icloud.com>

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 {

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"}))
+}

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
 				}
 			}

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")
 }

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)
 		},

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)

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":