Page MenuHomePhabricator

[keyserver] Add unique key on thread_name in the roles table
ClosedPublic

Authored by rohan on Jul 6 2023, 9:46 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Dec 31, 3:56 PM
Unknown Object (File)
Tue, Dec 31, 7:20 AM
Unknown Object (File)
Tue, Dec 31, 7:20 AM
Unknown Object (File)
Tue, Dec 31, 7:20 AM
Unknown Object (File)
Tue, Dec 31, 7:20 AM
Unknown Object (File)
Tue, Dec 31, 7:20 AM
Unknown Object (File)
Tue, Dec 31, 7:20 AM
Unknown Object (File)
Tue, Dec 31, 7:11 AM
Subscribers

Details

Summary

We want to ensure that for a given community, role names are unique (so we don't have several instances of one role with a different set of permissions). The key is a composite key between thread and name because we don't want to prevent the same role name from occurring across several different communities (i.e. 'Moderator' should be allowed in two different communities, but cannot exist twice in the same community).

This is the keyserver validation of preventing this error, the next step will involve adding some kind of client-side notice when they click 'Create' that it is a duplicate role name, before the call to create the role is made

Note: Given that the ability to create custom roles has begun in this stack, there shouldn't be an instance where there is a malformed DB state (except mine) with role names besides Admins and Members, so I don't see any issues coming out of this migration.

Part of ENG-4178.

Depends on D8431

Test Plan

Confirmed that the migration was successful and tried to create a role with a duplicate name ('Members') and verified that an error throws

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

rohan edited the test plan for this revision. (Show Details)
rohan edited the summary of this revision. (Show Details)
rohan edited the summary of this revision. (Show Details)
rohan requested review of this revision.Jul 6 2023, 10:02 AM
keyserver/src/database/setup-db.js
343 ↗(On Diff #28443)

Nit: I know we're inconsistent about this throughout this file, but I think having a space here (like you do in the migration) is better

This revision is now accepted and ready to land.Jul 7 2023, 6:30 AM