fix(org): check membership before member changes and polish org output

org member remove and public/private exited 0 without a word for users
who are not members, and member list --public showed concealed members,
because Gitea lists them as public to administrators. Membership changes
now check the membership first: removing a non-member warns, publicizing
one fails, and a change to the current visibility warns. Member list
checks each membership and gains a VISIBILITY column.

org list and member list get a header and context line through
writeTable, org view shows the description, website, location and member,
team and repository counts, mutations print ✓ lines, and org delete asks
to type the organization's name after refusing organizations that still
own repositories. public and private become publicize and conceal, the
old names stay as aliases, and org edit --visibility no longer shows the
create default. Every command has examples, and organizations and members
complete.

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

internal/services/organizations.go

@@ -9,6 +9,7 @@ import (
 	"fmt"
 	"net/http"
 	"net/url"
+	"sync"
 
 	gitea "gitea.dev/sdk"
 	"gitea.dev/tea/internal/transport"
@@ -32,7 +33,7 @@ func organization(ctx context.Context, client *transport.Client, owner string) (
 	org, _, err := call(ctx, client, func(ctx context.Context, c *gitea.Client) (*gitea.Organization, *gitea.Response, error) {
 		return c.Organizations.GetOrg(ctx, owner)
 	})
-	if failure, ok := errors.AsType[*transport.APIError](err); ok && failure.Status == http.StatusNotFound {
+	if statusNotFound(err) {
 		return 0, false, nil
 	}
 	if err != nil {
@@ -44,6 +45,12 @@ func organization(ctx context.Context, client *transport.Client, owner string) (
 	return org.ID, true, nil
 }
 
+// statusNotFound reports whether err is a 404 response.
+func statusNotFound(err error) bool {
+	failure, ok := errors.AsType[*transport.APIError](err)
+	return ok && failure.Status == http.StatusNotFound
+}
+
 // OrganizationOptions permits explicit empty and false preference updates.
 type OrganizationOptions struct {
 	Name                      *string `json:"username,omitempty"`
@@ -59,9 +66,8 @@ type OrganizationOptions struct {
 type Organizations struct{ Client *transport.Client }
 
 // List selects public organizations, your organizations, or a named user's organizations.
-// It reports whether more organizations follow the selected page.
-func (s Organizations) List(ctx context.Context, mine bool, username string, limit, page int) ([]Organization, bool, error) {
-	orgs, more, err := listWindow(limit, page, func(limit, page int) ([]*gitea.Organization, http.Header, error) {
+func (s Organizations) List(ctx context.Context, mine bool, username string, limit, page int) (Page[Organization], error) {
+	orgs, err := listPage(limit, page, func(limit, page int) ([]*gitea.Organization, http.Header, error) {
 		orgs, resp, err := call(ctx, s.Client, func(ctx context.Context, c *gitea.Client) ([]*gitea.Organization, *gitea.Response, error) {
 			opts := gitea.ListOrgsOptions{ListOptions: listOptions(limit, page)}
 			switch {
@@ -74,10 +80,12 @@ func (s Organizations) List(ctx context.Context, mine bool, username string, lim
 		})
 		return orgs, headerOf(resp), err
 	})
-	if err != nil {
-		return nil, false, err
-	}
-	return convertAll(orgs, organizationFrom), more, nil
+	return convertPage(orgs, organizationFrom), err
+}
+
+// convertPage maps a page of SDK items onto tea results.
+func convertPage[S, T any](page Page[*S], convert func(*S) T) Page[T] {
+	return Page[T]{Items: convertAll(page.Items, convert), More: page.More, Total: page.Total, Counted: page.Counted}
 }
 
 // View retrieves a named organization.
@@ -91,6 +99,56 @@ func (s Organizations) View(ctx context.Context, name string) (Organization, err
 	return organizationFrom(org), nil
 }
 
+// OrganizationSize counts what an organization holds; a count the account cannot read is nil.
+type OrganizationSize struct {
+	Members, Teams, Repositories *int
+}
+
+// Size counts the members, teams and repositories of an organization.
+func (s Organizations) Size(ctx context.Context, name string) (OrganizationSize, error) {
+	first := listOptions(1, 1)
+	counts := []func(context.Context, *gitea.Client) (*gitea.Response, error){
+		func(ctx context.Context, c *gitea.Client) (*gitea.Response, error) {
+			_, resp, err := c.Organizations.ListOrgMembership(ctx, name, gitea.ListOrgMembershipOption{ListOptions: first})
+			return resp, err
+		},
+		func(ctx context.Context, c *gitea.Client) (*gitea.Response, error) {
+			_, resp, err := c.Organizations.ListOrgTeams(ctx, name, gitea.ListTeamsOptions{ListOptions: first})
+			return resp, err
+		},
+		func(ctx context.Context, c *gitea.Client) (*gitea.Response, error) {
+			_, resp, err := c.Repositories.ListOrgRepos(ctx, name, gitea.ListOrgReposOptions{ListOptions: first})
+			return resp, err
+		},
+	}
+	totals := make([]*int, len(counts))
+	for i, count := range counts {
+		total, err := listTotal(ctx, s.Client, count)
+		if ctx.Err() != nil {
+			return OrganizationSize{}, ctx.Err()
+		}
+		if err == nil {
+			totals[i] = total
+		}
+	}
+	return OrganizationSize{Members: totals[0], Teams: totals[1], Repositories: totals[2]}, nil
+}
+
+// listTotal returns the collection size a list request reports, or nil when the server does not count it.
+func listTotal(ctx context.Context, client *transport.Client, list func(context.Context, *gitea.Client) (*gitea.Response, error)) (*int, error) {
+	_, resp, err := call(ctx, client, func(ctx context.Context, c *gitea.Client) (struct{}, *gitea.Response, error) {
+		resp, err := list(ctx, c)
+		return struct{}{}, resp, err
+	})
+	if err != nil {
+		return nil, err
+	}
+	if _, total, counted := pageHints(headerOf(resp)); counted {
+		return &total, nil
+	}
+	return nil, nil
+}
+
 // Create creates an organization owned by the selected account.
 func (s Organizations) Create(ctx context.Context, opts OrganizationOptions) (Organization, error) {
 	org, _, err := call(ctx, s.Client, func(ctx context.Context, c *gitea.Client) (*gitea.Organization, *gitea.Response, error) {
@@ -128,21 +186,107 @@ type OrganizationMember struct {
 	ID       int64  `json:"id"`
 	Username string `json:"username"`
 	FullName string `json:"full_name"`
+	// Visibility of an organization membership is public or concealed; team members have none.
+	Visibility string `json:"visibility,omitempty"`
 }
 
-// Members lists visible members in an organization.
-func (s Organizations) Members(ctx context.Context, name string, public bool, limit, page int) ([]OrganizationMember, error) {
-	users, _, err := call(ctx, s.Client, func(ctx context.Context, c *gitea.Client) ([]*gitea.User, *gitea.Response, error) {
-		opts := gitea.ListOrgMembershipOption{ListOptions: listOptions(limit, page)}
-		if public {
-			return c.Organizations.ListPublicOrgMembership(ctx, name, opts)
-		}
-		return c.Organizations.ListOrgMembership(ctx, name, opts)
+// Membership visibilities.
+const (
+	MembershipPublic    = "public"
+	MembershipConcealed = "concealed"
+)
+
+// MemberListOptions selects a page of organization members.
+type MemberListOptions struct {
+	// Public lists only public memberships, and Visibility reports the visibility of each one.
+	// Gitea lists concealed members as public to administrators, so both check each membership on
+	// its own, and Public drops the server's total, which counts concealed members too.
+	Public, Visibility bool
+	Limit, Page        int
+}
+
+// Members lists the members of an organization.
+func (s Organizations) Members(ctx context.Context, name string, opts MemberListOptions) (Page[OrganizationMember], error) {
+	users, err := listPage(opts.Limit, opts.Page, func(limit, page int) ([]*gitea.User, http.Header, error) {
+		users, resp, err := call(ctx, s.Client, func(ctx context.Context, c *gitea.Client) ([]*gitea.User, *gitea.Response, error) {
+			list := gitea.ListOrgMembershipOption{ListOptions: listOptions(limit, page)}
+			if opts.Public {
+				return c.Organizations.ListPublicOrgMembership(ctx, name, list)
+			}
+			return c.Organizations.ListOrgMembership(ctx, name, list)
+		})
+		return users, headerOf(resp), err
+	})
+	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 err
 	})
 	if err != nil {
-		return nil, err
+		return Page[OrganizationMember]{}, err
 	}
-	return convertAll(users, memberFrom), nil
+	shown := members.Items[:0]
+	for i, member := range members.Items {
+		member.Visibility = MembershipConcealed
+		if public[i] {
+			member.Visibility = MembershipPublic
+		}
+		if !opts.Public || public[i] {
+			shown = append(shown, member)
+		}
+	}
+	members.Items = shown
+	members.Counted = members.Counted && !opts.Public
+	return members, nil
+}
+
+// concurrentRequests bounds the requests that concurrently keeps in flight.
+const concurrentRequests = 8
+
+// concurrently runs request for each index 0..n-1 with a few requests in flight, and returns their
+// errors joined.
+func concurrently(n int, request func(i int) error) error {
+	errs := make([]error, n)
+	slots := make(chan struct{}, concurrentRequests)
+	var wg sync.WaitGroup
+	for i := range n {
+		slots <- struct{}{}
+		wg.Go(func() {
+			defer func() { <-slots }()
+			errs[i] = request(i)
+		})
+	}
+	wg.Wait()
+	return errors.Join(errs...)
+}
+
+// IsMember reports whether username belongs to the organization.
+func (s Organizations) IsMember(ctx context.Context, name, username string) (bool, error) {
+	return s.check(ctx, func(ctx context.Context, c *gitea.Client) (*gitea.Response, error) {
+		_, resp, err := c.Organizations.CheckOrgMembership(ctx, name, username)
+		return resp, err
+	})
+}
+
+// IsPublicMember reports whether username's membership of the organization is public.
+func (s Organizations) IsPublicMember(ctx context.Context, name, username string) (bool, error) {
+	return s.check(ctx, func(ctx context.Context, c *gitea.Client) (*gitea.Response, error) {
+		_, resp, err := c.Organizations.CheckPublicOrgMembership(ctx, name, username)
+		return resp, err
+	})
+}
+
+// check runs a membership check that answers no with a 404.
+func (s Organizations) check(ctx context.Context, op func(context.Context, *gitea.Client) (*gitea.Response, error)) (bool, error) {
+	err := do(ctx, s.Client, op)
+	if statusNotFound(err) {
+		return false, nil
+	}
+	return err == nil, err
 }
 
 // RemoveMember removes an organization member and their organization access.

internal/services/organizations_test.go

@@ -82,10 +82,11 @@ func TestOrganizationListSelectsScopeAndPage(t *testing.T) {
 				w.Header().Set("X-Total-Count", "7")
 				_, _ = w.Write([]byte(`[{"id":1,"username":"legacy"},{"id":2,"name":"acme"}]`))
 			}))
-			items, more, err := (Organizations{Client: client}).List(context.Background(), tc.mine, tc.username, 2, 3)
+			items, err := (Organizations{Client: client}).List(context.Background(), tc.mine, tc.username, 2, 3)
 			require.NoError(t, err)
-			require.True(t, more)
-			require.Equal(t, []string{"legacy", "acme"}, []string{items[0].Name, items[1].Name})
+			require.True(t, items.More)
+			require.Equal(t, 7, items.Total)
+			require.Equal(t, []string{"legacy", "acme"}, []string{items.Items[0].Name, items.Items[1].Name})
 		})
 	}
 }
@@ -111,20 +112,53 @@ func TestOrganizationMembershipRequests(t *testing.T) {
 	var requests []string
 	client := newTestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		requests = append(requests, r.Method+" "+r.URL.Path)
-		if r.Method == http.MethodGet {
-			_, _ = w.Write([]byte(`[{"id":2,"login":"bob","full_name":"Bob"}]`))
-			return
-		}
 		w.WriteHeader(http.StatusNoContent)
 	}))
 	service := Organizations{Client: client}
-	members, err := service.Members(context.Background(), "acme", true, 10, 1)
+	member, err := service.IsMember(context.Background(), "acme", "bob")
 	require.NoError(t, err)
-	require.Equal(t, []OrganizationMember{{ID: 2, Username: "bob", FullName: "Bob"}}, members)
+	require.True(t, member)
 	require.NoError(t, service.SetMemberPublic(context.Background(), "acme", "bob", true))
 	require.NoError(t, service.RemoveMember(context.Background(), "acme", "bob"))
 	require.Equal(t, []string{
-		"GET /api/v1/orgs/acme/public_members", "PUT /api/v1/orgs/acme/public_members/bob",
-		"DELETE /api/v1/orgs/acme/members/bob",
+		"GET /api/v1/orgs/acme/members/bob", "PUT /api/v1/orgs/acme/public_members/bob", "DELETE /api/v1/orgs/acme/members/bob",
 	}, requests)
 }
+
+func TestOrganizationMembersCheckEachMembership(t *testing.T) {
+	// Gitea lists concealed members among the public ones to administrators; only the check of
+	// each membership tells them apart.
+	client := newTestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		switch r.URL.Path {
+		case "/api/v1/orgs/acme/members", "/api/v1/orgs/acme/public_members":
+			w.Header().Set("X-Total-Count", "2")
+			_, _ = w.Write([]byte(`[{"id":1,"login":"alice"},{"id":2,"login":"bob","full_name":"Bob"}]`))
+		case "/api/v1/orgs/acme/public_members/alice":
+			w.WriteHeader(http.StatusNoContent)
+		case "/api/v1/orgs/acme/public_members/bob":
+			w.WriteHeader(http.StatusNotFound)
+		default:
+			t.Errorf("unexpected request %s %s", r.Method, r.URL.Path)
+		}
+	}))
+	alice, bob := OrganizationMember{ID: 1, Username: "alice"}, OrganizationMember{ID: 2, Username: "bob", FullName: "Bob"}
+	public, concealed := alice, bob
+	public.Visibility, concealed.Visibility = MembershipPublic, MembershipConcealed
+	for name, tc := range map[string]struct {
+		opts    MemberListOptions
+		want    []OrganizationMember
+		counted bool
+	}{
+		"names":      {MemberListOptions{}, []OrganizationMember{alice, bob}, true},
+		"visibility": {MemberListOptions{Visibility: true}, []OrganizationMember{public, concealed}, true},
+		"public":     {MemberListOptions{Public: true}, []OrganizationMember{public}, false},
+	} {
+		t.Run(name, func(t *testing.T) {
+			tc.opts.Limit, tc.opts.Page = 10, 1
+			members, err := (Organizations{Client: client}).Members(context.Background(), "acme", tc.opts)
+			require.NoError(t, err)
+			require.Equal(t, tc.want, members.Items)
+			require.Equal(t, tc.counted, members.Counted)
+		})
+	}
+}

internal/services/pagination_test.go

@@ -71,14 +71,14 @@ func TestListsCollectPagesBeyondServerPageSize(t *testing.T) {
 				want = append(want, int64(id))
 			}
 			client := transport.New(server.URL, "", false)
-			orgs, more, err := (Organizations{Client: client}).List(context.Background(), true, "", tc.limit, tc.page)
+			orgs, err := (Organizations{Client: client}).List(context.Background(), true, "", tc.limit, tc.page)
 			require.NoError(t, err)
 			got := []int64{}
-			for _, item := range orgs {
+			for _, item := range orgs.Items {
 				got = append(got, item.ID)
 			}
 			require.Equal(t, want, got)
-			require.Equal(t, tc.total > tc.page*tc.limit, more)
+			require.Equal(t, tc.total > tc.page*tc.limit, orgs.More)
 			require.LessOrEqual(t, *requests, tc.limit/min(tc.limit, tc.maxItems)+2)
 			repos, more, err := (Repositories{Client: client}).List(context.Background(), RepositoryListOptions{Limit: tc.limit, Page: tc.page})
 			require.NoError(t, err)