Give every team a page of its own
The Admin tab's team list was growing controls the way the user list did beforeac9af8e: a Rename button behind window.prompt, a Delete beside it, and -- on the Users page, of all places -- an invite form with a team picker in front of it. The picker was the admission that an invite is a fact about a team rather than about the server, and a prompt() is the wrong place to read a 409 about a name already taken. So a team is now a subject with a page, at /admin/teams/{id}, the mirror of /admin/users/{id}: when it was created, how many are in it and how much is open, a field to rename it, the members with their roles, the invites into it, and deletion. The list goes back to being a list, and the name in it is the way in. The member list is the one thing there that needed a new endpoint. GET /api/teams/{id}/members is requireTeamMember and answers 404 to an administrator who is not in the team, and that stays exactly as it is: member means membership and nothing else. Reading a team's shape is a different question from reading its work, so it gets an endpoint of its own under AdminOnly -- GET /api/admin/teams/{id}, returning {"team", "members"} -- rather than an exception carved into that rule. It is a wrapper and not a team with the members hung off it, because "members" already means a count on the list endpoint and one name must not be a number in one answer and an array in the next. The query and its ordering are copied from handleListTeamMembers so the two answers to "who is in this team" cannot disagree. An administrator still sees none of that team's incidents, alerts or rota. Nothing about what the flag may do changed; it could already rename and delete any team, and staff one it is not in. Rename now trims what it is given, as creation has always trimmed. Before this, " " was a legal name to rename a team to but not to create one with, which is one rule stated twice and applied once. Nobody has looked at this in a browser, the caveatac9af8eand07914d5both carried. What is checked is the wiring: admin_test.go covers the new endpoint for an administrator outside the team, the 404 the member-only endpoint still gives that same administrator, the 403 for a member who is not one, a 404 for a team that does not exist, a 400 for an id that is not a number, and the trim; the module graph evaluates at /admin/teams/{id}, and the server serves index.html there, so a reload survives. Claude-Session: https://claude.ai/code/session_01RHPj4ggeFdEjKKfm4SHbD7
This commit is contained in:
@@ -310,6 +310,127 @@ func TestAdmin_ConfiguresATeamTheyAreNotIn(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// The team page at /admin/teams/{id} needs the one question the test above
|
||||
// leaves shut: who is in a team the administrator is not in.
|
||||
//
|
||||
// It is answered by a separate endpoint under AdminOnly rather than by letting
|
||||
// the admin flag through requireTeamMember, and the second half of this test is
|
||||
// the reason — /api/teams/{id}/members must keep answering 404, so that "member
|
||||
// means membership and nothing else" stays true of the endpoint it was said
|
||||
// about. Reading a team's shape and reading a team's work are different things.
|
||||
func TestAdminGetTeam_ReadsAnyTeamWithoutJoiningIt(t *testing.T) {
|
||||
s := newTS(t)
|
||||
|
||||
founderID, call := member(t, s, "founder")
|
||||
var team struct {
|
||||
ID int64 `json:"id"`
|
||||
}
|
||||
decode(t, call(http.MethodPost, "/api/teams", map[string]string{"name": "theirs"}), &team)
|
||||
if team.ID == 0 {
|
||||
t.Fatal("no team was created")
|
||||
}
|
||||
|
||||
// The admin reads it whole, without being in it.
|
||||
var got struct {
|
||||
Team struct {
|
||||
ID int64 `json:"id"`
|
||||
Name string `json:"name"`
|
||||
Members int64 `json:"members"`
|
||||
OpenIncidents int64 `json:"open_incidents"`
|
||||
} `json:"team"`
|
||||
Members []struct {
|
||||
UserID int64 `json:"user_id"`
|
||||
Username string `json:"username"`
|
||||
Role string `json:"role"`
|
||||
} `json:"members"`
|
||||
}
|
||||
decode(t, s.req(t, http.MethodGet, "/api/admin/teams/"+id64(team.ID), nil), &got)
|
||||
|
||||
if got.Team.ID != team.ID || got.Team.Name != "theirs" {
|
||||
t.Errorf("expected team %d named theirs, got %d named %q", team.ID, got.Team.ID, got.Team.Name)
|
||||
}
|
||||
if got.Team.Members != 1 {
|
||||
t.Errorf("expected a member count of 1, got %d", got.Team.Members)
|
||||
}
|
||||
if len(got.Members) != 1 {
|
||||
t.Fatalf("expected one member, got %d", len(got.Members))
|
||||
}
|
||||
if got.Members[0].UserID != founderID || got.Members[0].Username != "founder" {
|
||||
t.Errorf("expected founder (%d), got %q (%d)",
|
||||
founderID, got.Members[0].Username, got.Members[0].UserID)
|
||||
}
|
||||
// Whoever creates a team owns it, and the page's role toggle depends on
|
||||
// that being reported rather than assumed.
|
||||
if got.Members[0].Role != "owner" {
|
||||
t.Errorf("expected the creator to be owner, got %q", got.Members[0].Role)
|
||||
}
|
||||
|
||||
// The rule this endpoint exists in order not to break. Same admin, same
|
||||
// team, the member-only endpoint: still not found.
|
||||
resp := s.req(t, http.MethodGet, "/api/teams/"+id64(team.ID)+"/members", nil)
|
||||
resp.Body.Close()
|
||||
if resp.StatusCode != http.StatusNotFound {
|
||||
t.Errorf("an admin outside the team must still get 404 from the member-only list, got %d",
|
||||
resp.StatusCode)
|
||||
}
|
||||
|
||||
// And the new one is administration, not membership: being in the team is
|
||||
// not enough.
|
||||
resp = call(http.MethodGet, "/api/admin/teams/"+id64(team.ID), nil)
|
||||
resp.Body.Close()
|
||||
if resp.StatusCode != http.StatusForbidden {
|
||||
t.Errorf("a non-admin member must get 403, got %d", resp.StatusCode)
|
||||
}
|
||||
|
||||
for _, c := range []struct {
|
||||
name string
|
||||
path string
|
||||
want int
|
||||
}{
|
||||
{"a team that does not exist", "/api/admin/teams/999999", http.StatusNotFound},
|
||||
{"a team id that is not a number", "/api/admin/teams/nonsense", http.StatusBadRequest},
|
||||
} {
|
||||
resp := s.req(t, http.MethodGet, c.path, nil)
|
||||
resp.Body.Close()
|
||||
if resp.StatusCode != c.want {
|
||||
t.Errorf("%s: expected %d, got %d", c.name, c.want, resp.StatusCode)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// A team name is trimmed when it is created, and renaming had not been, so " "
|
||||
// was a legal name to rename to and an illegal one to start with.
|
||||
func TestRenameTeam_TrimsTheName(t *testing.T) {
|
||||
s := newTS(t)
|
||||
var team struct {
|
||||
ID int64 `json:"id"`
|
||||
}
|
||||
decode(t, s.req(t, http.MethodPost, "/api/teams", map[string]string{"name": "trimmed"}), &team)
|
||||
|
||||
path := "/api/teams/" + id64(team.ID)
|
||||
resp := s.req(t, http.MethodPut, path, map[string]string{"name": " "})
|
||||
resp.Body.Close()
|
||||
if resp.StatusCode != http.StatusBadRequest {
|
||||
t.Errorf("a blank name must be refused, got %d", resp.StatusCode)
|
||||
}
|
||||
|
||||
resp = s.req(t, http.MethodPut, path, map[string]string{"name": " padded "})
|
||||
resp.Body.Close()
|
||||
if resp.StatusCode != http.StatusNoContent {
|
||||
t.Fatalf("expected 204, got %d", resp.StatusCode)
|
||||
}
|
||||
|
||||
var got struct {
|
||||
Team struct {
|
||||
Name string `json:"name"`
|
||||
} `json:"team"`
|
||||
}
|
||||
decode(t, s.req(t, http.MethodGet, "/api/admin/teams/"+id64(team.ID), nil), &got)
|
||||
if got.Team.Name != "padded" {
|
||||
t.Errorf("expected the name to be trimmed to %q, got %q", "padded", got.Team.Name)
|
||||
}
|
||||
}
|
||||
|
||||
// The admin page's per-user view asks what somebody is in. Self or admin, like
|
||||
// the rest of the per-user endpoints.
|
||||
func TestUserTeams_SelfOrAdmin(t *testing.T) {
|
||||
|
||||
@@ -94,6 +94,11 @@ func NewRouter(db *sql.DB, notify NotifyConfig, cfg config.Config) http.Handler
|
||||
// What exists on this server, and how it behaves. /api/teams
|
||||
// answers "what am I in"; this one answers "what is there".
|
||||
r.Get("/api/admin/teams", handleAdminListTeams(db))
|
||||
// One team and who is in it. The member list under
|
||||
// /api/teams/{id}/members stays member-only and still 404s
|
||||
// an administrator from outside; this is a different
|
||||
// question, so it is a different endpoint.
|
||||
r.Get("/api/admin/teams/{teamID}", handleAdminGetTeam(db))
|
||||
r.Get("/api/admin/settings", handleGetSettings(db, cfg))
|
||||
r.Put("/api/admin/settings", handleSetSettings(db))
|
||||
})
|
||||
|
||||
@@ -6,9 +6,11 @@ import (
|
||||
"errors"
|
||||
"net/http"
|
||||
"strconv"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"git.ryuvia.com/niklas/terdut-server/internal/config"
|
||||
"git.ryuvia.com/niklas/terdut-server/internal/models"
|
||||
"github.com/go-chi/chi/v5"
|
||||
)
|
||||
|
||||
@@ -226,6 +228,17 @@ func handleSetSettings(db *sql.DB) http.HandlerFunc {
|
||||
}
|
||||
}
|
||||
|
||||
// adminTeam is a team as an administrator sees it: what it is, plus how big it
|
||||
// is and how much is on fire in it. One definition, so a team in the list and a
|
||||
// team on its own page cannot describe themselves differently.
|
||||
type adminTeam struct {
|
||||
ID int64 `json:"id"`
|
||||
Name string `json:"name"`
|
||||
CreatedAt time.Time `json:"created_at"`
|
||||
Members int64 `json:"members"`
|
||||
OpenIncidents int64 `json:"open_incidents"`
|
||||
}
|
||||
|
||||
// handleAdminListTeams lists every team on the server, with its size. The
|
||||
// ordinary /api/teams answers "what am I in"; this one answers "what exists",
|
||||
// which only an administrator may ask.
|
||||
@@ -244,13 +257,6 @@ func handleAdminListTeams(db *sql.DB) http.HandlerFunc {
|
||||
}
|
||||
defer rows.Close()
|
||||
|
||||
type adminTeam struct {
|
||||
ID int64 `json:"id"`
|
||||
Name string `json:"name"`
|
||||
CreatedAt time.Time `json:"created_at"`
|
||||
Members int64 `json:"members"`
|
||||
OpenIncidents int64 `json:"open_incidents"`
|
||||
}
|
||||
teams := []adminTeam{}
|
||||
for rows.Next() {
|
||||
var t adminTeam
|
||||
@@ -270,6 +276,79 @@ func handleAdminListTeams(db *sql.DB) http.HandlerFunc {
|
||||
}
|
||||
}
|
||||
|
||||
// handleAdminGetTeam answers "what is this team, and who is in it" for any team
|
||||
// on the server, which is the one question an administrator could not ask.
|
||||
//
|
||||
// GET /api/teams/{id}/members is requireTeamMember and answers 404 to somebody
|
||||
// outside the team, administrator or not, and that stays exactly as it is:
|
||||
// member means membership and nothing else. Reading a team's shape is a
|
||||
// different thing from reading its work, so it gets an endpoint of its own
|
||||
// under AdminOnly rather than an exception carved into that rule. An
|
||||
// administrator still sees none of the team's incidents, alerts or rota.
|
||||
func handleAdminGetTeam(db *sql.DB) http.HandlerFunc {
|
||||
return func(w http.ResponseWriter, r *http.Request) {
|
||||
teamID, ok := teamParam(w, r)
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
|
||||
var t adminTeam
|
||||
var created int64
|
||||
err := db.QueryRowContext(r.Context(), `
|
||||
SELECT t.id, t.name, t.created_at,
|
||||
(SELECT COUNT(*) FROM team_members m WHERE m.team_id = t.id),
|
||||
(SELECT COUNT(*) FROM incidents i
|
||||
WHERE i.team_id = t.id AND i.resolved_at IS NULL)
|
||||
FROM teams t
|
||||
WHERE t.id = $1`, teamID).
|
||||
Scan(&t.ID, &t.Name, &created, &t.Members, &t.OpenIncidents)
|
||||
if errors.Is(err, sql.ErrNoRows) {
|
||||
respond(w, http.StatusNotFound, errResp("not found"))
|
||||
return
|
||||
}
|
||||
if err != nil {
|
||||
respond(w, http.StatusInternalServerError, errResp("internal error"))
|
||||
return
|
||||
}
|
||||
t.CreatedAt = time.Unix(created, 0).UTC()
|
||||
|
||||
// Same query and same ordering as handleListTeamMembers, so the two
|
||||
// answers to "who is in this team" cannot disagree about the answer.
|
||||
rows, err := db.QueryContext(r.Context(), `
|
||||
SELECT m.team_id, m.user_id, u.username, m.role, m.joined_at
|
||||
FROM team_members m
|
||||
JOIN users u ON u.id = m.user_id
|
||||
WHERE m.team_id = $1
|
||||
ORDER BY u.username`, teamID)
|
||||
if err != nil {
|
||||
respond(w, http.StatusInternalServerError, errResp("internal error"))
|
||||
return
|
||||
}
|
||||
defer rows.Close()
|
||||
|
||||
members := []models.TeamMember{}
|
||||
for rows.Next() {
|
||||
var m models.TeamMember
|
||||
var joined int64
|
||||
if err := rows.Scan(&m.TeamID, &m.UserID, &m.Username, &m.Role, &joined); err != nil {
|
||||
respond(w, http.StatusInternalServerError, errResp("internal error"))
|
||||
return
|
||||
}
|
||||
m.JoinedAt = time.Unix(joined, 0).UTC()
|
||||
members = append(members, m)
|
||||
}
|
||||
if err := rows.Err(); err != nil {
|
||||
respond(w, http.StatusInternalServerError, errResp("internal error"))
|
||||
return
|
||||
}
|
||||
|
||||
// A wrapper rather than a team with the members hung off it: "members"
|
||||
// already means a count on the list endpoint, and one name must not be
|
||||
// a number in one answer and an array in the next.
|
||||
respond(w, http.StatusOK, map[string]any{"team": t, "members": members})
|
||||
}
|
||||
}
|
||||
|
||||
// handleRenameTeam renames a team. An owner's job, and an administrator's when
|
||||
// a team has nobody left to do it.
|
||||
func handleRenameTeam(db *sql.DB) http.HandlerFunc {
|
||||
@@ -285,7 +364,15 @@ func handleRenameTeam(db *sql.DB) http.HandlerFunc {
|
||||
var req struct {
|
||||
Name string `json:"name"`
|
||||
}
|
||||
if err := decodeJSON(r, &req); err != nil || req.Name == "" {
|
||||
// Trimmed, as handleCreateTeam trims: without it " " is a team name
|
||||
// here but not at creation, which is one rule stated twice and only
|
||||
// half applied.
|
||||
if err := decodeJSON(r, &req); err != nil {
|
||||
respond(w, http.StatusBadRequest, errResp("name is required"))
|
||||
return
|
||||
}
|
||||
req.Name = strings.TrimSpace(req.Name)
|
||||
if req.Name == "" {
|
||||
respond(w, http.StatusBadRequest, errResp("name is required"))
|
||||
return
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user