tenseleyflow/shithub / 81ee7f9

Browse files

Use org member picker for teams

Authored by espadonne
SHA
81ee7f95decc174890086579a74a1f580725c938
Parents
f926e68
Tree
ece5615

3 changed files

StatusFile+-
M internal/web/handlers/orgs/teams.go 81 23
M internal/web/handlers/orgs/teams_test.go 38 1
M internal/web/templates/orgs/team_view.html 11 2
internal/web/handlers/orgs/teams.gomodified
@@ -62,6 +62,12 @@ type teamRepoCandidate struct {
6262
 	Visibility string
6363
 }
6464
65
+type teamMemberCandidate struct {
66
+	ID          int64
67
+	Username    string
68
+	DisplayName string
69
+}
70
+
6571
 // teamsList renders /{org}/teams. GitHub keeps org teams member-only:
6672
 // visible teams are visible to org members, while secret teams are
6773
 // further limited to team members and org owners.
@@ -168,6 +174,7 @@ func (h *Handlers) teamView(w http.ResponseWriter, r *http.Request) {
168174
 	children, _ := q.ListChildTeams(r.Context(), h.d.Pool, pgtype.Int8{Int64: team.ID, Valid: true})
169175
 	childItems := h.teamListItems(org, h.filterSecretTeams(r, children, org.ID, viewer),
170176
 		h.teamAggregateCounts(r.Context(), org.ID), teamParentSlugs(children))
177
+	memberCandidates := h.teamMemberCandidates(r.Context(), org.ID, team.ID)
171178
 	repoCandidates := h.teamRepoCandidates(r.Context(), org.ID, team.ID)
172179
 	isOwner := false
173180
 	if !viewer.IsAnonymous() {
@@ -186,6 +193,7 @@ func (h *Handlers) teamView(w http.ResponseWriter, r *http.Request) {
186193
 		"TeamIsSecret":     team.Privacy == orgsdb.TeamPrivacySecret,
187194
 		"ChildTeams":       childItems,
188195
 		"Members":          members,
196
+		"MemberCandidates": memberCandidates,
189197
 		"Repos":            repos,
190198
 		"RepoCandidates":   repoCandidates,
191199
 		"ActiveOrgTab":     "teams",
@@ -215,15 +223,14 @@ func (h *Handlers) teamMemberAddRemove(w http.ResponseWriter, r *http.Request) {
215223
 		h.d.Render.HTTPError(w, r, http.StatusBadRequest, "")
216224
 		return
217225
 	}
218
-	username := strings.ToLower(strings.TrimSpace(r.PostFormValue("username")))
219226
 	action := r.PostFormValue("action")
220
-	if username == "" {
227
+	uid, ok := h.userIDFromTeamMemberForm(r)
228
+	if !ok {
221229
 		http.Redirect(w, r, h.teamPath(org, team), http.StatusSeeOther)
222230
 		return
223231
 	}
224
-	uid, ok := h.userIDByUsername(r, username)
225
-	if !ok {
226
-		http.Redirect(w, r, h.teamPath(org, team), http.StatusSeeOther)
232
+	if action != "remove" && !h.userIsOrgMember(r.Context(), org.ID, uid) {
233
+		h.d.Render.HTTPError(w, r, http.StatusBadRequest, "")
227234
 		return
228235
 	}
229236
 	switch action {
@@ -329,6 +336,30 @@ func (h *Handlers) userIDByUsername(r *http.Request, username string) (int64, bo
329336
 	return id, true
330337
 }
331338
339
+func (h *Handlers) userIDFromTeamMemberForm(r *http.Request) (int64, bool) {
340
+	if raw := strings.TrimSpace(r.PostFormValue("user_id")); raw != "" {
341
+		id, err := strconv.ParseInt(raw, 10, 64)
342
+		if err == nil && id != 0 {
343
+			return id, true
344
+		}
345
+		return 0, false
346
+	}
347
+	username := strings.ToLower(strings.TrimSpace(r.PostFormValue("username")))
348
+	if username == "" {
349
+		return 0, false
350
+	}
351
+	return h.userIDByUsername(r, username)
352
+}
353
+
354
+func (h *Handlers) userIsOrgMember(ctx context.Context, orgID, userID int64) bool {
355
+	var exists bool
356
+	err := h.d.Pool.QueryRow(ctx,
357
+		`SELECT EXISTS(SELECT 1 FROM org_members WHERE org_id = $1 AND user_id = $2)`,
358
+		orgID, userID,
359
+	).Scan(&exists)
360
+	return err == nil && exists
361
+}
362
+
332363
 // canSeeTeam decides whether the viewer is allowed to see a team's members
333364
 // and repositories. canSeeOrgTeams has already enforced org membership;
334365
 // visible teams are readable to those members, while secret teams require
@@ -538,6 +569,33 @@ func (h *Handlers) teamRepoCandidates(ctx context.Context, orgID, teamID int64)
538569
 	return out
539570
 }
540571
572
+func (h *Handlers) teamMemberCandidates(ctx context.Context, orgID, teamID int64) []teamMemberCandidate {
573
+	rows, err := h.d.Pool.Query(ctx, `
574
+		SELECT u.id, u.username, u.display_name
575
+		  FROM org_members om
576
+		  JOIN users u ON u.id = om.user_id
577
+		  LEFT JOIN team_members tm
578
+		    ON tm.team_id = $2 AND tm.user_id = u.id
579
+		 WHERE om.org_id = $1
580
+		   AND u.deleted_at IS NULL
581
+		   AND tm.user_id IS NULL
582
+		 ORDER BY lower(u.username)
583
+		 LIMIT 100`, orgID, teamID)
584
+	if err != nil {
585
+		h.d.Logger.WarnContext(ctx, "teams: member candidates", "org_id", orgID, "team_id", teamID, "error", err)
586
+		return nil
587
+	}
588
+	defer rows.Close()
589
+	out := []teamMemberCandidate{}
590
+	for rows.Next() {
591
+		var item teamMemberCandidate
592
+		if err := rows.Scan(&item.ID, &item.Username, &item.DisplayName); err == nil {
593
+			out = append(out, item)
594
+		}
595
+	}
596
+	return out
597
+}
598
+
541599
 func (h *Handlers) repoIDFromTeamForm(r *http.Request, orgID int64) (int64, error) {
542600
 	if raw := strings.TrimSpace(r.PostFormValue("repo_id")); raw != "" {
543601
 		return strconv.ParseInt(raw, 10, 64)
internal/web/handlers/orgs/teams_test.gomodified
@@ -8,6 +8,8 @@ import (
88
 	"log/slog"
99
 	"net/http"
1010
 	"net/http/httptest"
11
+	"net/url"
12
+	"strconv"
1113
 	"strings"
1214
 	"testing"
1315
 	"testing/fstest"
@@ -64,7 +66,34 @@ func TestTeamsListRequiresOrgMemberAndFiltersSecretTeams(t *testing.T) {
6466
 	}
6567
 }
6668
69
+func TestTeamMemberAddRejectsNonOrgUsers(t *testing.T) {
70
+	t.Parallel()
71
+	ctx := context.Background()
72
+	pool := dbtest.NewTestDB(t)
73
+	ownerID := insertOrgAvatarUser(t, pool, "owner")
74
+	outsiderID := insertOrgAvatarUser(t, pool, "outsider")
75
+	orgID := insertOrgAvatarOrg(t, pool, ownerID, "acme")
76
+	teamID := insertTeamForTest(t, pool, orgID, "engineering", "Engineering", "visible")
77
+
78
+	form := url.Values{"user_id": {strconv.FormatInt(outsiderID, 10)}, "role": {"member"}}
79
+	body, status, _ := performTeamsRequest(t, pool, middleware.CurrentUser{ID: ownerID, Username: "owner"}, http.MethodPost, "/acme/teams/engineering/members", form)
80
+	if status != http.StatusBadRequest {
81
+		t.Fatalf("status=%d body=%s", status, body)
82
+	}
83
+	var count int
84
+	if err := pool.QueryRow(ctx, `SELECT count(*) FROM team_members WHERE team_id = $1`, teamID).Scan(&count); err != nil {
85
+		t.Fatalf("count team members: %v", err)
86
+	}
87
+	if count != 0 {
88
+		t.Fatalf("expected no team member insert, got %d", count)
89
+	}
90
+}
91
+
6792
 func performTeamsListRequest(t *testing.T, pool *pgxpool.Pool, viewer middleware.CurrentUser, target string) (string, int, string) {
93
+	return performTeamsRequest(t, pool, viewer, http.MethodGet, target, nil)
94
+}
95
+
96
+func performTeamsRequest(t *testing.T, pool *pgxpool.Pool, viewer middleware.CurrentUser, method, target string, form url.Values) (string, int, string) {
6897
 	t.Helper()
6998
 	rr, err := render.New(fstest.MapFS{
7099
 		"_layout.html":         {Data: []byte(`{{ define "layout" }}<html><body>{{ template "page" . }}</body></html>{{ end }}`)},
@@ -72,6 +101,7 @@ func performTeamsListRequest(t *testing.T, pool *pgxpool.Pool, viewer middleware
72101
 		"orgs/teams_list.html": {Data: []byte(`{{ define "page" }}{{ template "org-nav" . }} TOTAL={{ .TeamTotalCount }}{{ range .Teams }} TEAM={{ .Slug }}:{{ .DisplayName }}:{{ .MemberCount }}:{{ .RepoCount }}{{ end }}{{ end }}`)},
73102
 		"orgs/team_view.html":  {Data: []byte(`{{ define "page" }}TEAM{{ end }}`)},
74103
 		"orgs/people.html":     {Data: []byte(`{{ define "page" }}PEOPLE{{ end }}`)},
104
+		"errors/400.html":      {Data: []byte(`{{ define "page" }}400{{ end }}`)},
75105
 		"errors/403.html":      {Data: []byte(`{{ define "page" }}403{{ end }}`)},
76106
 		"errors/404.html":      {Data: []byte(`{{ define "page" }}404{{ end }}`)},
77107
 		"errors/500.html":      {Data: []byte(`{{ define "page" }}500{{ end }}`)},
@@ -95,7 +125,14 @@ func performTeamsListRequest(t *testing.T, pool *pgxpool.Pool, viewer middleware
95125
 	})
96126
 	h.MountOrgRoutes(r)
97127
98
-	req := httptest.NewRequest(http.MethodGet, target, nil)
128
+	var body io.Reader
129
+	if form != nil {
130
+		body = strings.NewReader(form.Encode())
131
+	}
132
+	req := httptest.NewRequest(method, target, body)
133
+	if form != nil {
134
+		req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
135
+	}
99136
 	rec := httptest.NewRecorder()
100137
 	r.ServeHTTP(rec, req)
101138
 	return rec.Body.String(), rec.Code, rec.Header().Get("Location")
internal/web/templates/orgs/team_view.htmlmodified
@@ -46,7 +46,7 @@
4646
             {{ if $.IsOwner }}
4747
             <form method="POST" action="/{{ $.Org.Slug }}/teams/{{ $.Team.Slug }}/members">
4848
               <input type="hidden" name="csrf_token" value="{{ $.CSRFToken }}">
49
-              <input type="hidden" name="username" value="{{ .Username }}">
49
+              <input type="hidden" name="user_id" value="{{ .UserID }}">
5050
               <input type="hidden" name="action" value="remove">
5151
               <button type="submit" class="shithub-button shithub-button-danger">Remove</button>
5252
             </form>
@@ -129,9 +129,15 @@
129129
       {{ if .IsOwner }}
130130
       <section class="shithub-org-team-manage-box">
131131
         <h2>Add member</h2>
132
+        {{ if .MemberCandidates }}
132133
         <form method="POST" action="/{{ .Org.Slug }}/teams/{{ .Team.Slug }}/members">
133134
           <input type="hidden" name="csrf_token" value="{{ .CSRFToken }}">
134
-          <label><span>Username</span><input type="text" name="username" required placeholder="@username"></label>
135
+          <label><span>Organization member</span>
136
+            <select name="user_id" required>
137
+              <option value="">Select member</option>
138
+              {{ range .MemberCandidates }}<option value="{{ .ID }}">{{ if .DisplayName }}{{ .DisplayName }} · {{ end }}@{{ .Username }}</option>{{ end }}
139
+            </select>
140
+          </label>
135141
           <label><span>Role</span>
136142
             <select name="role">
137143
               <option value="member" selected>Member</option>
@@ -140,6 +146,9 @@
140146
           </label>
141147
           <button type="submit" class="shithub-button shithub-button-primary">Add member</button>
142148
         </form>
149
+        {{ else }}
150
+        <p class="shithub-muted">All organization members are already on this team.</p>
151
+        {{ end }}
143152
       </section>
144153
145154
       <section class="shithub-org-team-manage-box">