Skip to content

Commit 0d0546f

Browse files
committed
Team admin admin
Admins now can designate other members as admins, and can also remove admin permissions. Admins can also leave the team now, except for the last admin.
1 parent fa81f28 commit 0d0546f

5 files changed

Lines changed: 164 additions & 12 deletions

File tree

app/assets/stylesheets/components/settings.css

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@
8282
.settings-page .button-primary,
8383
.settings-page .button-secondary,
8484
.settings-page .button-danger,
85+
.settings-page .button-danger-secondary,
8586
.settings-page input[type="submit"] {
8687
display: inline-flex;
8788
align-items: center;
@@ -304,7 +305,8 @@
304305
margin-left: var(--spacing-2);
305306
}
306307

307-
.settings-page .member-list .button-secondary {
308+
.settings-page .member-list .button-secondary,
309+
.settings-page .member-list .button-danger-secondary {
308310
font-size: var(--font-size-xs);
309311
padding: var(--spacing-2) var(--spacing-3);
310312
}
@@ -382,3 +384,42 @@
382384
font-size: var(--font-size-sm);
383385
margin-top: var(--spacing-1);
384386
}
387+
388+
/* Member management */
389+
.settings-page .member-list .member-item {
390+
flex-wrap: nowrap;
391+
}
392+
393+
.settings-page .member-list .member-info {
394+
display: flex;
395+
align-items: center;
396+
gap: var(--spacing-2);
397+
flex: 1;
398+
min-width: 0;
399+
}
400+
401+
.settings-page .member-list .member-actions {
402+
display: flex;
403+
align-items: center;
404+
gap: var(--spacing-2);
405+
flex-shrink: 0;
406+
}
407+
408+
.settings-page .member-list .member-actions .hint {
409+
font-size: var(--font-size-xs);
410+
color: var(--color-text-secondary);
411+
font-style: italic;
412+
}
413+
414+
.settings-page .button-danger-secondary {
415+
background: var(--color-bg-card);
416+
color: var(--color-danger-text);
417+
border-color: var(--color-border);
418+
}
419+
420+
.settings-page .button-danger-secondary:hover {
421+
border-color: var(--color-danger-text);
422+
background: var(--color-danger-bg-hover);
423+
box-shadow: var(--shadow-sm);
424+
transform: translateY(-1px);
425+
}

app/controllers/settings/team_members_controller.rb

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,39 @@ def create
1717
redirect_to settings_team_path(@team), alert: e.record.errors.full_messages.to_sentence
1818
end
1919

20+
def update
21+
authorize_admin!
22+
return if performed?
23+
24+
membership = @team.team_members.find(params[:id])
25+
new_role = params[:role]
26+
27+
# Prevent admin from removing their own admin status
28+
if membership.user_id == current_user.id && new_role == "member"
29+
return redirect_to settings_team_path(@team), alert: "You cannot remove your own admin status"
30+
end
31+
32+
# Prevent removing the last admin
33+
if membership.admin? && new_role == "member" && @team.last_admin?(membership)
34+
return redirect_to settings_team_path(@team), alert: "Cannot demote the last admin"
35+
end
36+
37+
membership.update!(role: new_role)
38+
redirect_to settings_team_path(@team), notice: "Member role updated"
39+
rescue ActiveRecord::RecordInvalid => e
40+
redirect_to settings_team_path(@team), alert: e.record.errors.full_messages.to_sentence
41+
end
42+
2043
def destroy
2144
membership = @team.team_members.find(params[:id])
2245

2346
if membership.user_id == current_user.id
24-
membership.destroy
25-
redirect_to settings_teams_path, notice: "You left the team"
47+
if @team.last_admin?(membership)
48+
redirect_to settings_team_path(@team), alert: "You cannot leave as the last admin"
49+
else
50+
membership.destroy
51+
redirect_to settings_teams_path, notice: "You left the team"
52+
end
2653
else
2754
authorize_admin!
2855
return if performed?

app/views/settings/teams/show.html.slim

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -47,14 +47,22 @@
4747
- if @team_members.any?
4848
ul.member-list
4949
- @team_members.each do |member|
50-
li
51-
= member.user.aliases.first&.name || "User ##{member.user.id}"
52-
- if member.role_admin?
53-
span.role-badge Admin
54-
- if @can_manage && member.user_id != current_user&.id
55-
= button_to "Remove", settings_team_team_member_path(@team, member), method: :delete, class: "button-secondary", data: { turbo_confirm: "Remove this member?" }
56-
- if member.user_id == current_user&.id && !member.role_admin?
57-
= button_to "Leave team", settings_team_team_member_path(@team, member), method: :delete, class: "button-secondary", data: { turbo_confirm: "Leave this team?" }
50+
li.member-item
51+
.member-info
52+
= member.user.aliases.first&.name || "User ##{member.user.id}"
53+
- if member.role_admin?
54+
span.role-badge Admin
55+
.member-actions
56+
- if @can_manage && member.user_id != current_user&.id
57+
- if member.role_admin?
58+
= button_to "Make member", settings_team_team_member_path(@team, member), method: :patch, params: { role: "member" }, class: "button-secondary", data: { turbo_confirm: "Demote this admin to regular member?" }
59+
- else
60+
= button_to "Make admin", settings_team_team_member_path(@team, member), method: :patch, params: { role: "admin" }, class: "button-secondary"
61+
= button_to "Remove", settings_team_team_member_path(@team, member), method: :delete, class: "button-danger-secondary", data: { turbo_confirm: "Remove this member from the team?" }
62+
- elsif member.user_id == current_user&.id
63+
- if member.role_admin?
64+
span.hint You cannot change your own admin status
65+
= button_to "Leave team", settings_team_team_member_path(@team, member), method: :delete, class: "button-secondary", data: { turbo_confirm: "Leave this team?" }
5866
- else
5967
p No members yet.
6068

config/routes.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
resource :deletion, only: [:show, :create]
3030

3131
resources :teams, only: [:index, :show, :create, :update, :destroy] do
32-
resources :team_members, only: [:create, :destroy]
32+
resources :team_members, only: [:create, :update, :destroy]
3333
end
3434

3535
resource :username, only: [:update]

spec/requests/team_members_spec.rb

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,4 +46,80 @@ def attach_verified_alias(user, email:, primary: true)
4646
expect(response).to redirect_to(settings_team_path(team))
4747
end
4848
end
49+
50+
describe "PATCH /teams/:team_id/team_members/:id" do
51+
it "blocks non-admins from changing roles" do
52+
attach_verified_alias(member, email: "member@example.com")
53+
sign_in(email: "member@example.com")
54+
55+
member_record = team.team_members.find_by(user: member)
56+
patch settings_team_team_member_path(team, member_record), params: { role: "admin" }
57+
58+
expect(response).to redirect_to(settings_team_path(team))
59+
expect(member_record.reload.role).to eq("member")
60+
end
61+
62+
it "allows admins to promote members to admin" do
63+
attach_verified_alias(admin, email: "admin@example.com")
64+
sign_in(email: "admin@example.com")
65+
66+
member_record = team.team_members.find_by(user: member)
67+
patch settings_team_team_member_path(team, member_record), params: { role: "admin" }
68+
69+
expect(response).to redirect_to(settings_team_path(team))
70+
expect(member_record.reload.role).to eq("admin")
71+
end
72+
73+
it "allows admins to demote other admins to members" do
74+
attach_verified_alias(admin, email: "admin@example.com")
75+
sign_in(email: "admin@example.com")
76+
77+
other_admin = create(:user, password: "secret", password_confirmation: "secret")
78+
other_admin_record = create(:team_member, team: team, user: other_admin, role: "admin")
79+
80+
patch settings_team_team_member_path(team, other_admin_record), params: { role: "member" }
81+
82+
expect(response).to redirect_to(settings_team_path(team))
83+
expect(other_admin_record.reload.role).to eq("member")
84+
end
85+
86+
it "prevents admin from removing their own admin status" do
87+
attach_verified_alias(admin, email: "admin@example.com")
88+
sign_in(email: "admin@example.com")
89+
90+
admin_record = team.team_members.find_by(user: admin)
91+
patch settings_team_team_member_path(team, admin_record), params: { role: "member" }
92+
93+
expect(response).to redirect_to(settings_team_path(team))
94+
expect(flash[:alert]).to include("cannot remove your own admin status")
95+
expect(admin_record.reload.role).to eq("admin")
96+
end
97+
98+
it "prevents demoting the last admin when there are two admins and one tries to demote the other after becoming the only admin" do
99+
# Create a second admin who will try to demote the first
100+
other_admin = create(:user, password: "secret", password_confirmation: "secret")
101+
attach_verified_alias(other_admin, email: "other_admin@example.com")
102+
create(:team_member, team: team, user: other_admin, role: "admin")
103+
104+
sign_in(email: "other_admin@example.com")
105+
106+
# Demote the first admin - this should work since there are 2 admins
107+
admin_record = team.team_members.find_by(user: admin)
108+
patch settings_team_team_member_path(team, admin_record), params: { role: "member" }
109+
expect(admin_record.reload.role).to eq("member")
110+
111+
# Now other_admin is the only admin
112+
# Try to demote them using the first admin (now just a member, so should fail authorization)
113+
attach_verified_alias(admin, email: "admin@example.com")
114+
sign_in(email: "admin@example.com")
115+
116+
other_admin_record = team.team_members.find_by(user: other_admin)
117+
patch settings_team_team_member_path(team, other_admin_record), params: { role: "member" }
118+
119+
expect(response).to redirect_to(settings_team_path(team))
120+
expect(flash[:alert]).to eq("Admins only")
121+
expect(other_admin_record.reload.role).to eq("admin")
122+
end
123+
124+
end
49125
end

0 commit comments

Comments
 (0)