Skip to content

Commit 9936302

Browse files
authored
refactor(dotcom): rename group admin role to member (tldraw#9029)
In order to make the group role label match what it actually represents, this PR renames the non-owner group role from `admin` to `member`. The role confers no admin-level capabilities — it's a standard group member — so the old "Admin" label was misleading. Closes tldraw#9014. This builds on the capabilities abstraction from tldraw#9067: authorization asks `can(role, capability)` and never branches on the role name, so the rename is purely a relabel plus a data migration — no authorization logic changes. What changes: - **Role definition** (`roles.ts`) — the `admin` key in the `roles` capability table becomes `member`. `Role` is derived from these keys, so the `group_user.role` enum in `tlaSchema.ts` and every `Role`-typed call site update for free. - **Sync worker** — accepting a group invite now assigns `role: 'member'`. - **Client UI** — the members list and role dropdown show "Member" (the dropdown is data-driven from the role → label map, so only the label and the map key change). - **Database** — migration `033` drops the `group_user` role check constraint, rewrites existing `admin` rows to `member`, then re-adds the constraint allowing `('member', 'owner')`. Because the new check constraint rejects `'admin'`, the migration should run as part of the deploy that ships this change. In the `deploy-dotcom` flow migrations run before the sync worker is deployed, so existing rows are rewritten and the constraint swapped before any new code writes `'member'`. Groups is behind the `groups_backend` / `groups_frontend` flags and was added recently, so little data has accumulated. ### Change type - [x] `improvement` ### Test plan 1. Open a group's settings dialog as the owner. 2. Confirm the members list shows "Member" (not "Admin") next to non-owner members. 3. Confirm the role dropdown offers "Owner" and "Member". 4. Change a member's role to Owner and back; confirm it persists. 5. Accept a group invite as a new user; confirm they join as a member. - [x] Unit tests - [ ] End to end tests `roles.test.ts` covers the role → capability matrix and `isRole` under the new name; `mutators.test.ts` group tests run against `member` fixtures. ### Code changes | Section | LOC change | | --------------- | ---------- | | Core code | +2 / -8 | | Tests | +35 / -35 | | Automated files | +9 / -9 | | Apps | +17 / -4 |
1 parent ba8af21 commit 9936302

8 files changed

Lines changed: 63 additions & 56 deletions

File tree

apps/dotcom/client/public/tla/locales-compiled/en.json

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -699,6 +699,12 @@
699699
"value": "."
700700
}
701701
],
702+
"858ba4765e": [
703+
{
704+
"type": 0,
705+
"value": "Member"
706+
}
707+
],
702708
"85a082de1b": [
703709
{
704710
"type": 0,
@@ -1277,12 +1283,6 @@
12771283
"value": "Legal summary"
12781284
}
12791285
],
1280-
"e3afed0047": [
1281-
{
1282-
"type": 0,
1283-
"value": "Admin"
1284-
}
1285-
],
12861286
"e404559826": [
12871287
{
12881288
"type": 0,

apps/dotcom/client/public/tla/locales/en.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,9 @@
314314
"815e116a2e": {
315315
"translation": "Have a bug, issue, or idea for tldraw? Let us know! Fill out this form and we will follow up over email if needed. You can also <discord>chat with us on Discord</discord> or <github>submit an issue on GitHub</github>."
316316
},
317+
"858ba4765e": {
318+
"translation": "Member"
319+
},
317320
"85a082de1b": {
318321
"translation": "Please refresh the page to get the latest version of tldraw."
319322
},
@@ -549,9 +552,6 @@
549552
"e2ae80a510": {
550553
"translation": "Legal summary"
551554
},
552-
"e3afed0047": {
553-
"translation": "Admin"
554-
},
555555
"e404559826": {
556556
"translation": "Terms of service"
557557
},

apps/dotcom/client/src/tla/components/dialogs/GroupSettingsDialog.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ const messages = defineMessages({
3333
copyInviteLink: { defaultMessage: 'Copy invite link' },
3434
members: { defaultMessage: 'Members' },
3535
owner: { defaultMessage: 'Owner' },
36-
admin: { defaultMessage: 'Admin' },
36+
member: { defaultMessage: 'Member' },
3737
you: { defaultMessage: 'you' },
3838
dangerZone: { defaultMessage: 'Danger zone' },
3939
leaveGroup: { defaultMessage: 'Leave group…' },
@@ -62,7 +62,7 @@ export function GroupSettingsDialog({ groupId, onClose }: GroupSettingsDialogPro
6262

6363
const namePlaceholderMsg = useMsg(messages.namePlaceholder)
6464
const ownerMsg = useMsg(messages.owner)
65-
const adminMsg = useMsg(messages.admin)
65+
const memberMsg = useMsg(messages.member)
6666
const youMsg = useMsg(messages.you)
6767

6868
// Get group data
@@ -83,7 +83,7 @@ export function GroupSettingsDialog({ groupId, onClose }: GroupSettingsDialogPro
8383
// Leaving is allowed for everyone except the last owner — a group invariant
8484
// (it must always keep at least one owner), not a capability.
8585
const canLeave = role !== 'owner' || ownersCount > 1
86-
const roleLabels: Record<Role, string> = { owner: ownerMsg, admin: adminMsg }
86+
const roleLabels: Record<Role, string> = { owner: ownerMsg, member: memberMsg }
8787

8888
const handleCopyInviteLink = async () => {
8989
if (copiedInviteLink) return

apps/dotcom/sync-worker/src/routes/tla/acceptInvite.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ export async function acceptInvite(request: IRequest, env: Environment): Promise
119119
userId: auth.userId,
120120
userColor: user.color || '#000000',
121121
userName: user.name,
122-
role: 'admin',
122+
role: 'member',
123123
index,
124124
createdAt: Date.now(),
125125
updatedAt: Date.now(),
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
-- Rename the non-owner group role from 'admin' to 'member'.
2+
-- The 'admin' label was misleading: the role confers no admin-level
3+
-- capabilities, it's a standard group member.
4+
5+
-- Drop the existing CHECK constraint. It was created as an inline column
6+
-- constraint in 023_groups.sql, so Postgres auto-named it "group_user_role_check".
7+
ALTER TABLE public."group_user" DROP CONSTRAINT IF EXISTS "group_user_role_check";
8+
9+
-- Rewrite existing rows to the new role value.
10+
UPDATE public."group_user" SET "role" = 'member' WHERE "role" = 'admin';
11+
12+
-- Re-add the CHECK constraint with the new allowed values.
13+
ALTER TABLE public."group_user" ADD CONSTRAINT "group_user_role_check" CHECK ("role" IN ('member', 'owner'));

packages/dotcom-shared/src/mutators.test.ts

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ function makeGroupUser(
7373
return {
7474
createdAt: 1,
7575
updatedAt: 1,
76-
role: 'admin',
76+
role: 'member',
7777
userName: 'Test',
7878
userColor: '#000',
7979
index: 'a1' as IndexKey,
@@ -386,7 +386,7 @@ describe('file mutations', () => {
386386
file: [] as TlaFile[],
387387
file_state: [] as TlaFileState[],
388388
group: [makeGroup({ id: groupId })],
389-
group_user: [makeGroupUser({ userId, groupId, role: 'admin' })],
389+
group_user: [makeGroupUser({ userId, groupId, role: 'member' })],
390390
group_file: [] as TlaGroupFile[],
391391
}
392392
}
@@ -651,14 +651,14 @@ describe('group mutations', () => {
651651
await expectValid(() => m.updateGroup(tx, { id: groupId, name: 'Renamed' }))
652652
})
653653

654-
it('admin cannot update group name', async () => {
654+
it('member cannot update group name', async () => {
655655
const groupId = 'group_aaa11112222bbb'
656656
const s = {
657657
user: [makeUser({ id: userId, flags: 'groups_backend' })],
658658
file: [],
659659
file_state: [],
660660
group: [makeGroup({ id: groupId })],
661-
group_user: [makeGroupUser({ userId, groupId, role: 'admin' })],
661+
group_user: [makeGroupUser({ userId, groupId, role: 'member' })],
662662
group_file: [],
663663
}
664664
const { tx } = createMockTx(s)
@@ -692,7 +692,7 @@ describe('group mutations', () => {
692692
file: [],
693693
file_state: [],
694694
group: [makeGroup({ id: groupId })],
695-
group_user: [makeGroupUser({ userId, groupId, role: 'admin' })],
695+
group_user: [makeGroupUser({ userId, groupId, role: 'member' })],
696696
group_file: [],
697697
}
698698
const { tx } = createMockTx(s)
@@ -714,7 +714,7 @@ describe('membership', () => {
714714
group: [makeGroup({ id: groupId })],
715715
group_user: [
716716
makeGroupUser({ userId, groupId, role: 'owner' }),
717-
makeGroupUser({ userId: memberId, groupId, role: 'admin' }),
717+
makeGroupUser({ userId: memberId, groupId, role: 'member' }),
718718
],
719719
group_file: [],
720720
}
@@ -724,15 +724,15 @@ describe('membership', () => {
724724
expect(s.group_user.find((gu) => gu.userId === memberId)?.role).toBe('owner')
725725
})
726726

727-
it('admin cannot set member roles', async () => {
727+
it('member cannot set member roles', async () => {
728728
const s = {
729729
user: [makeUser({ id: userId, flags: 'groups_backend' })],
730730
file: [],
731731
file_state: [],
732732
group: [makeGroup({ id: groupId })],
733733
group_user: [
734-
makeGroupUser({ userId, groupId, role: 'admin' }),
735-
makeGroupUser({ userId: memberId, groupId, role: 'admin' }),
734+
makeGroupUser({ userId, groupId, role: 'member' }),
735+
makeGroupUser({ userId: memberId, groupId, role: 'member' }),
736736
],
737737
group_file: [],
738738
}
@@ -743,22 +743,22 @@ describe('membership', () => {
743743
)
744744
})
745745

746-
it('cannot demote last owner to admin', async () => {
746+
it('cannot demote last owner to member', async () => {
747747
const s = {
748748
user: [makeUser({ id: userId, flags: 'groups_backend' })],
749749
file: [],
750750
file_state: [],
751751
group: [makeGroup({ id: groupId })],
752752
group_user: [
753753
makeGroupUser({ userId, groupId, role: 'owner' }),
754-
makeGroupUser({ userId: memberId, groupId, role: 'admin' }),
754+
makeGroupUser({ userId: memberId, groupId, role: 'member' }),
755755
],
756756
group_file: [],
757757
}
758758
const { tx } = createMockTx(s)
759759
const m = createMutators(userId)
760760
await expectForbidden(() =>
761-
m.setGroupMemberRole(tx, { groupId, targetUserId: userId, role: 'admin' })
761+
m.setGroupMemberRole(tx, { groupId, targetUserId: userId, role: 'member' })
762762
)
763763
})
764764

@@ -784,7 +784,7 @@ describe('membership', () => {
784784
group: [makeGroup({ id: groupId })],
785785
group_user: [
786786
makeGroupUser({ userId: 'user_owner12345678ab', groupId, role: 'owner' }),
787-
makeGroupUser({ userId, groupId, role: 'admin' }),
787+
makeGroupUser({ userId, groupId, role: 'member' }),
788788
],
789789
group_file: [],
790790
}
@@ -847,13 +847,13 @@ describe('file operations across groups', () => {
847847
await expectForbidden(() => m.moveFileToGroup(tx, { fileId, groupId: groupB }))
848848
})
849849

850-
it('admin can remove file from group', async () => {
850+
it('member can remove file from group', async () => {
851851
const s = {
852852
user: [makeUser({ id: userId, flags: 'groups_backend' })],
853853
file: [makeFile({ id: fileId, owningGroupId: groupA })],
854854
file_state: [makeFileState({ userId, fileId })],
855855
group: [makeGroup({ id: groupA })],
856-
group_user: [makeGroupUser({ userId, groupId: groupA, role: 'admin' })],
856+
group_user: [makeGroupUser({ userId, groupId: groupA, role: 'member' })],
857857
group_file: [makeGroupFile({ fileId, groupId: groupA })],
858858
}
859859
const { tx } = createMockTx(s)
@@ -959,13 +959,13 @@ describe('regenerateGroupInviteSecret', () => {
959959
const userId = 'user_aaaa11112222bbbb'
960960
const groupId = 'group_aaa11112222bbb'
961961

962-
it('admin can regenerate invite secret', async () => {
962+
it('member can regenerate invite secret', async () => {
963963
const s = {
964964
user: [makeUser({ id: userId, flags: 'groups_backend' })],
965965
file: [],
966966
file_state: [],
967967
group: [makeGroup({ id: groupId, inviteSecret: 'old_secret_1234567' })],
968-
group_user: [makeGroupUser({ userId, groupId, role: 'admin' })],
968+
group_user: [makeGroupUser({ userId, groupId, role: 'member' })],
969969
group_file: [],
970970
}
971971
const { tx } = createMockTx(s, { location: 'server' })
@@ -976,18 +976,18 @@ describe('regenerateGroupInviteSecret', () => {
976976
})
977977

978978
it('non-member cannot regenerate invite secret', async () => {
979-
const nonAdminId = 'user_nonadmin1234567'
979+
const nonMemberId = 'user_nonmember123456'
980980
const s = {
981-
user: [makeUser({ id: nonAdminId, flags: 'groups_backend' })],
981+
user: [makeUser({ id: nonMemberId, flags: 'groups_backend' })],
982982
file: [],
983983
file_state: [],
984984
group: [makeGroup({ id: groupId })],
985-
// No group_user for nonAdminId — not a member at all
985+
// No group_user for nonMemberId — not a member at all
986986
group_user: [],
987987
group_file: [],
988988
}
989989
const { tx } = createMockTx(s, { location: 'server' })
990-
const m = createMutators(nonAdminId)
990+
const m = createMutators(nonMemberId)
991991
await expectForbidden(() => m.regenerateGroupInviteSecret(tx, { id: groupId }))
992992
})
993993
})

packages/dotcom-shared/src/roles.test.ts

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -24,29 +24,29 @@ describe('can', () => {
2424
}
2525
})
2626

27-
it('grants admins the non-administrative capabilities only', () => {
28-
const adminCapabilities = capabilities.filter((capability) => can('admin', capability))
29-
expect(adminCapabilities).toEqual(['accessFiles', 'addFiles', 'removeFiles', 'manageInvites'])
27+
it('grants members the non-administrative capabilities only', () => {
28+
const memberCapabilities = capabilities.filter((capability) => can('member', capability))
29+
expect(memberCapabilities).toEqual(['accessFiles', 'addFiles', 'removeFiles', 'manageInvites'])
3030
})
3131

32-
it('denies admins the administrative capabilities', () => {
33-
expect(can('admin', 'editGroup')).toBe(false)
34-
expect(can('admin', 'editMembers')).toBe(false)
35-
expect(can('admin', 'deleteGroup')).toBe(false)
32+
it('denies members the administrative capabilities', () => {
33+
expect(can('member', 'editGroup')).toBe(false)
34+
expect(can('member', 'editMembers')).toBe(false)
35+
expect(can('member', 'deleteGroup')).toBe(false)
3636
})
3737

38-
it("admins' capabilities are a subset of owners'", () => {
38+
it("members' capabilities are a subset of owners'", () => {
3939
for (const capability of capabilities) {
40-
if (can('admin', capability)) {
40+
if (can('member', capability)) {
4141
expect(can('owner', capability)).toBe(true)
4242
}
4343
}
4444
})
4545

4646
it('denies unknown, empty, or missing roles every capability', () => {
47-
// `member` is included deliberately: the rename from `admin` is still
48-
// pending, so it is not yet a valid role.
49-
const notRoles = [null, undefined, '', 'member', 'nope', 'OWNER', 'toString', '__proto__']
47+
// `admin` is included deliberately: it was renamed to `member`, so it is no
48+
// longer a valid role.
49+
const notRoles = [null, undefined, '', 'admin', 'nope', 'OWNER', 'toString', '__proto__']
5050
for (const role of notRoles) {
5151
for (const capability of capabilities) {
5252
expect(can(role, capability)).toBe(false)
@@ -58,11 +58,11 @@ describe('can', () => {
5858
describe('isRole', () => {
5959
it('accepts known role strings', () => {
6060
expect(isRole('owner')).toBe(true)
61-
expect(isRole('admin')).toBe(true)
61+
expect(isRole('member')).toBe(true)
6262
})
6363

6464
it('rejects unknown, empty, or missing values', () => {
65-
for (const value of [null, undefined, '', 'member', 'nope', 'toString', '__proto__']) {
65+
for (const value of [null, undefined, '', 'admin', 'nope', 'toString', '__proto__']) {
6666
expect(isRole(value)).toBe(false)
6767
}
6868
})

packages/dotcom-shared/src/roles.ts

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,22 +9,16 @@ import { Capability } from './capabilities'
99
*
1010
* The role is stored in the DB as a plain string (`group_user.role`);
1111
* capabilities are never persisted — they're derived from that string here.
12-
*
13-
* NOTE: `'admin'` is in the process of being renamed to `'member'`. No
14-
* authorization logic branches on that name — the only role literals left in
15-
* logic are the last-owner invariant, which checks `'owner'` (not renamed). So
16-
* the rename is relabeling this key, the `acceptInvite` default, the display
17-
* labels, and a data migration of stored values.
1812
*/
1913

2014
/**
2115
* What each role can do — the single source of truth. The role name is the key,
2216
* and {@link Role} is derived from these keys. Today the only difference between
23-
* `admin` and `owner` is the three administrative capabilities at the end of the
17+
* `member` and `owner` is the three administrative capabilities at the end of the
2418
* owner list.
2519
*/
2620
const roles = {
27-
admin: ['accessFiles', 'addFiles', 'removeFiles', 'manageInvites'],
21+
member: ['accessFiles', 'addFiles', 'removeFiles', 'manageInvites'],
2822
owner: [
2923
'accessFiles',
3024
'addFiles',

0 commit comments

Comments
 (0)