feat(projects): change a member's role from the member list
An invite sent with the wrong role could not be corrected. The only way back was to remove the person and invite them again — and removing a member unassigns every task they were responsible for, so fixing a dropdown cost somebody their work. This is the other side of "an external user cannot create tasks": the rule was right, the role was wrong, and there was no way to say so. PATCH against a membership that already exists, not the upsert AddMember does. Somebody who is not a member is not silently made one: that is an invite, it belongs to the invite flow, and doing it here would let a typo in a user id add a stranger to a project without anybody being asked. The owner is refused, with the real reason rather than the true but useless one. Ownership lives on the project rather than in project_members, so the plain answer would have been "not a member" — which is correct and tells nobody anything. Ownership is transferred, not granted, and two owners is a state with no way to settle a disagreement about who may remove whom. In the list, the owner's standing is text and everybody else's is a dropdown. A disabled control would be a thing that exists to say no. And the roles are finally in Danish. The API's words are `owner`, `editor` and `viewer`, and they were being printed straight into a Danish page; the list now uses the same three phrases the invite form beside it already did. The smoke test drives both browsers, because the interesting part is that the other one's permissions really change: a viewer is told the project is read-only and has no quick-add box, the owner promotes them from the list, and after a reload the box is there and the notice is gone. Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5
parent
fc32fea607
commit
b6ea87ac19
@@ -194,3 +194,92 @@ func TestListMembersWorksForTheInbox(t *testing.T) {
|
||||
t.Fatalf("status %d, body %v", resp.StatusCode, body)
|
||||
}
|
||||
}
|
||||
|
||||
// A role sent by mistake used to be uncorrectable: the only way back was to remove
|
||||
// the person and invite them again, which unassigns every task they were
|
||||
// responsible for. Fixing a dropdown should not cost somebody their work.
|
||||
func TestAMembersRoleCanBeChangedWithoutRemovingThem(t *testing.T) {
|
||||
ts := newTestServer(t)
|
||||
ts.bootstrap(t)
|
||||
member := ts.newUser(t, "[email protected]", "Anden")
|
||||
|
||||
memberUser, err := ts.db.UserByEmail(t.Context(), "[email protected]")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
_, project := ts.do(t, "POST", "/api/v1/projects", map[string]any{"name": "Delt"})
|
||||
projectID := project["id"].(string)
|
||||
ts.do(t, "POST", "/api/v1/projects/"+projectID+"/invites", map[string]any{
|
||||
"email": "[email protected]", "role": "viewer",
|
||||
})
|
||||
|
||||
// A viewer cannot write. That is the rule, not the bug.
|
||||
if resp, _ := member.do(t, "POST", "/api/v1/tasks",
|
||||
map[string]any{"content": "prøv", "project_id": projectID}); resp.StatusCode != http.StatusNotFound {
|
||||
t.Fatalf("a viewer created a task: status %d", resp.StatusCode)
|
||||
}
|
||||
|
||||
resp, _ := ts.do(t, "PATCH", "/api/v1/projects/"+projectID+"/members/"+memberUser.ID,
|
||||
map[string]any{"role": "editor"})
|
||||
if resp.StatusCode != http.StatusNoContent {
|
||||
t.Fatalf("promote to editor: status %d", resp.StatusCode)
|
||||
}
|
||||
|
||||
if resp, _ := member.do(t, "POST", "/api/v1/tasks",
|
||||
map[string]any{"content": "nu kan jeg", "project_id": projectID}); resp.StatusCode != http.StatusCreated {
|
||||
t.Errorf("an editor could not create a task: status %d", resp.StatusCode)
|
||||
}
|
||||
_, members := ts.do(t, "GET", "/api/v1/projects/"+projectID+"/members", nil)
|
||||
for _, raw := range members["members"].([]any) {
|
||||
if m := raw.(map[string]any); m["user_id"] == memberUser.ID && m["role"] != "editor" {
|
||||
t.Errorf("role = %v after the change, want editor", m["role"])
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// The owner is not a member row, and ownership is not a role you can hand out.
|
||||
func TestTheOwnersStandingCannotBeChanged(t *testing.T) {
|
||||
ts := newTestServer(t)
|
||||
ts.bootstrap(t)
|
||||
member := ts.newUser(t, "[email protected]", "Anden")
|
||||
|
||||
owner, err := ts.db.UserByEmail(t.Context(), "[email protected]")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
memberUser, err := ts.db.UserByEmail(t.Context(), "[email protected]")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
_, project := ts.do(t, "POST", "/api/v1/projects", map[string]any{"name": "Delt"})
|
||||
projectID := project["id"].(string)
|
||||
ts.do(t, "POST", "/api/v1/projects/"+projectID+"/invites", map[string]any{
|
||||
"email": "[email protected]", "role": "editor",
|
||||
})
|
||||
|
||||
// Demoting the owner would leave a project nobody can administer.
|
||||
if resp, _ := ts.do(t, "PATCH", "/api/v1/projects/"+projectID+"/members/"+owner.ID,
|
||||
map[string]any{"role": "viewer"}); resp.StatusCode != http.StatusConflict {
|
||||
t.Errorf("changing the owner's role: status %d, want 409", resp.StatusCode)
|
||||
}
|
||||
// And no member may promote themselves to owner.
|
||||
if resp, _ := ts.do(t, "PATCH", "/api/v1/projects/"+projectID+"/members/"+memberUser.ID,
|
||||
map[string]any{"role": "owner"}); resp.StatusCode != http.StatusUnprocessableEntity {
|
||||
t.Errorf("granting ownership: status %d, want 422", resp.StatusCode)
|
||||
}
|
||||
// Nor may an editor change anybody's role: this is the owner's to do.
|
||||
if resp, _ := member.do(t, "PATCH", "/api/v1/projects/"+projectID+"/members/"+memberUser.ID,
|
||||
map[string]any{"role": "viewer"}); resp.StatusCode != http.StatusNotFound {
|
||||
t.Errorf("an editor changed a role: status %d, want 404", resp.StatusCode)
|
||||
}
|
||||
// Somebody with no membership at all is not quietly given one.
|
||||
stranger := ts.newUser(t, "[email protected]", "Tredje")
|
||||
_ = stranger
|
||||
strangerUser, _ := ts.db.UserByEmail(t.Context(), "[email protected]")
|
||||
if resp, _ := ts.do(t, "PATCH", "/api/v1/projects/"+projectID+"/members/"+strangerUser.ID,
|
||||
map[string]any{"role": "editor"}); resp.StatusCode != http.StatusNotFound {
|
||||
t.Errorf("a non-member was given a role: status %d, want 404", resp.StatusCode)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user