fix(team): merge team edit --unit into the current unit policy

Gitea replaces a team's whole unit policy on edit, so
`tea team edit 5 --unit repo.wiki=write` revoked code, issue and pull
request access. team edit now merges --unit into the current policy;
--unit-replace keeps the old replacing behavior.

Signed-off-by: bircni <bircni@icloud.com>

internal/app/teams.go

@@ -35,7 +35,7 @@ func (a *application) teamCommand() *cobra.Command {
 	group.AddCommand(list)
 	for _, verb := range []string{"view", "create", "edit", "delete"} {
 		var name, description, permission string
-		var createRepo, allRepos, force bool
+		var createRepo, allRepos, force, replaceUnits bool
 		var units map[string]string
 		command := &cobra.Command{Use: verb + " <id>", Short: titleCase(verb) + " an organization team", Args: argsExact(1)}
 		switch verb {
@@ -49,7 +49,12 @@ func (a *application) teamCommand() *cobra.Command {
 			command.Flags().StringVar(&name, "name", "", "Team name")
 			command.Flags().StringVar(&description, "description", "", "Team description")
 			command.Flags().StringVar(&permission, "permission", "read", "Default access: read, write, admin")
-			command.Flags().StringToStringVar(&units, "unit", nil, "Unit permissions, for example repo.code=write,repo.issues=read (replaces unit policy)")
+			unitUsage := "Unit permissions, for example repo.code=write,repo.issues=read"
+			if verb == "edit" {
+				unitUsage = "Unit permissions to change, for example repo.wiki=write; merged into the current policy"
+				command.Flags().BoolVar(&replaceUnits, "unit-replace", false, "Replace the whole unit policy with --unit; omitted units lose access")
+			}
+			command.Flags().StringToStringVar(&units, "unit", nil, unitUsage)
 			command.Flags().BoolVar(&createRepo, "can-create-repo", false, "Allow creating organization repositories")
 			command.Flags().BoolVar(&allRepos, "all-repositories", false, "Access every organization repository")
 		}
@@ -68,6 +73,9 @@ func (a *application) teamCommand() *cobra.Command {
 				if err := oneOf("permission", permission, "read", "write", "admin"); err != nil {
 					return err
 				}
+				if replaceUnits && !cmd.Flags().Changed("unit") {
+					return clierr.Wrap(clierr.Usage, errors.New("--unit-replace requires --unit"))
+				}
 				if cmd.Flags().Changed("unit") && (len(units) == 0 || permission == "admin") {
 					return clierr.Wrap(clierr.Usage, errors.New("--unit needs a nonempty policy and cannot restrict administrator access"))
 				}
@@ -106,6 +114,7 @@ func (a *application) teamCommand() *cobra.Command {
 				}
 				if cmd.Flags().Changed("unit") {
 					opts.Units = units
+					opts.ReplaceUnits = replaceUnits
 				}
 				if cmd.Flags().Changed("can-create-repo") {
 					opts.CanCreateRepo = &createRepo

internal/app/teams_test.go

@@ -5,6 +5,7 @@ package app
 
 import (
 	"context"
+	"encoding/json"
 	"net/http"
 	"testing"
 
@@ -52,3 +53,28 @@ func TestTeamDeleteNamesTheTeamAndProtectsTheOwnerTeam(t *testing.T) {
 	require.Equal(t, []string{`Delete team "devs" (ID 5) from acme?`}, prompt.confirmations)
 	require.Equal(t, []string{"/api/v1/teams/3", "/api/v1/teams/5"}, deleted)
 }
+
+func TestTeamEditMergesUnitsIntoTheCurrentPolicy(t *testing.T) {
+	var sent []map[string]any
+	server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		require.Equal(t, "/api/v1/teams/5", r.URL.Path)
+		if r.Method == http.MethodPatch {
+			var body map[string]any
+			require.NoError(t, json.NewDecoder(r.Body).Decode(&body))
+			sent = append(sent, body)
+		}
+		_, _ = w.Write([]byte(`{"id":5,"name":"devs","permission":"write","units_map":{"repo.code":"write","repo.issues":"write","repo.pulls":"write"}}`))
+	}))
+	defer server.Close()
+	deps, _, _ := serverDeps(t, server.URL)
+	require.NoError(t, Execute(context.Background(), deps, []string{"team", "edit", "5", "--unit", "repo.wiki=write", "--json"}))
+	require.NoError(t, Execute(context.Background(), deps, []string{"team", "edit", "5", "--unit", "repo.wiki=write", "--unit-replace", "--json"}))
+	require.Equal(t, []map[string]any{
+		{"units_map": map[string]any{"repo.code": "write", "repo.issues": "write", "repo.pulls": "write", "repo.wiki": "write"}},
+		{"units_map": map[string]any{"repo.wiki": "write"}},
+	}, sent)
+	err := Execute(context.Background(), deps, []string{"team", "edit", "5", "--unit-replace", "--name", "devs"})
+	require.Equal(t, clierr.Usage, clierr.ExitCode(err), "%v", err)
+	require.ErrorContains(t, err, "--unit-replace requires --unit")
+	require.Len(t, sent, 2)
+}

internal/services/teams.go

@@ -6,6 +6,7 @@ package services
 import (
 	"context"
 	"fmt"
+	"maps"
 	"net/http"
 	"strings"
 
@@ -47,6 +48,8 @@ type TeamOptions struct {
 	CanCreateRepo   *bool             `json:"can_create_org_repo,omitempty"`
 	AllRepositories *bool             `json:"includes_all_repositories,omitempty"`
 	Units           map[string]string `json:"units_map,omitempty"`
+	// ReplaceUnits makes Edit replace the unit policy with Units instead of merging Units into it.
+	ReplaceUnits bool `json:"-"`
 }
 
 // Teams implements team, member and repository access operations.
@@ -101,19 +104,28 @@ func (s Teams) Create(ctx context.Context, org string, opts TeamOptions) (Team,
 	return teamFrom(team), nil
 }
 
-// Edit preserves unit permissions when changing access to all repositories.
+// Edit merges Units into the current unit policy unless ReplaceUnits is set, and preserves unit
+// permissions when changing access to all repositories. Gitea replaces the whole policy on edit.
 func (s Teams) Edit(ctx context.Context, id int64, opts TeamOptions) (Team, error) {
-	if opts.AllRepositories != nil && opts.Permission == nil {
+	repositoryPolicy := opts.AllRepositories != nil && opts.Permission == nil
+	if repositoryPolicy || (opts.Units != nil && !opts.ReplaceUnits) {
 		current, err := s.View(ctx, id)
 		if err != nil {
 			return Team{}, err
 		}
+		if repositoryPolicy {
 			if current.Permission == "owner" {
 				return Team{}, &CapabilityError{Feature: "owner-team repository policy editing", Reason: "owner teams already manage all organization repositories"}
 			}
 			opts.Permission = &current.Permission
-		if opts.Units == nil {
+		}
+		switch {
+		case opts.Units == nil:
 			opts.Units = current.Units
+		case !opts.ReplaceUnits:
+			units := maps.Clone(current.Units)
+			maps.Copy(units, opts.Units)
+			opts.Units = units
 		}
 	}
 	var team gitea.Team

internal/services/teams_test.go

@@ -37,6 +37,34 @@ func TestTeamRepositoryPolicyPreservesUnitPermissions(t *testing.T) {
 	require.Equal(t, 1, mutations)
 }
 
+func TestTeamEditMergesUnitsUnlessReplaced(t *testing.T) {
+	for _, test := range []struct {
+		replace bool
+		reads   int
+		want    map[string]any
+	}{
+		{false, 1, map[string]any{"repo.code": "write", "repo.issues": "read", "repo.wiki": "write"}},
+		{true, 0, map[string]any{"repo.wiki": "write"}},
+	} {
+		reads := 0
+		client := newTestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			require.Equal(t, "/api/v1/teams/5", r.URL.Path)
+			if r.Method == http.MethodGet {
+				reads++
+				_, _ = w.Write([]byte(`{"id":5,"permission":"write","units_map":{"repo.code":"write","repo.issues":"read","repo.wiki":"none"}}`))
+				return
+			}
+			body := requestJSON(t, r)
+			require.Equal(t, test.want, body["units_map"])
+			require.NotContains(t, body, "permission")
+			_, _ = w.Write([]byte(`{"id":5}`))
+		}))
+		_, err := (Teams{Client: client}).Edit(context.Background(), 5, TeamOptions{Units: map[string]string{"repo.wiki": "write"}, ReplaceUnits: test.replace})
+		require.NoError(t, err)
+		require.Equal(t, test.reads, reads)
+	}
+}
+
 func TestTeamCreateSendsPermissionAndUnits(t *testing.T) {
 	client := newTestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		require.Equal(t, http.MethodPost, r.Method)