[keyserver] Introduce a new thread permission for pinning / unpinning messages
AcceptedPublic
Actions

Authored by rohan on Thu, Mar 2, 10:15 AM.

Details

Reviewers

ginsu
tomek
ashoat
atul

Summary

We need to introduces new thread permission that limits who is able to manage the pins for a thread. At the moment, this is only granted to Admins, so there's a couple of things that need to be done here:

Update the baseAdminPriviledges so new chats with admins will have the correct, updated permissions
Run a migration to cover existing threads

We call updateRolesAndPermissionsForAllThreads() which should handle updating the permissions for each of the threads given the new roles.

The migration was discussed in the Linear task.

Depends on D6924

Test Plan

Confirm that the migration is successful

Confirm that the roles table is updated for Admins (expected result is that the query only finds Admins to have the new permissions)

Confirm that the memberships table for each of the threads is updated (expected result is that the query finds the permissions to be NOT NULL in each thread for user 256, the admin user locally)

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

rohan created this revision.Thu, Mar 2, 10:15 AM

Herald added a subscriber: ashoat. · View Herald TranscriptThu, Mar 2, 10:15 AM

rohan edited the summary of this revision. (Show Details)Thu, Mar 2, 10:18 AM

rohan edited the test plan for this revision. (Show Details)

rohan attached a referenced file: F410690: Memberships table updated.png. (Show Details)

rohan attached a referenced file: F410689: Roles table updated.png. (Show Details)

rohan attached a referenced file: F410688: Confirm migration success.png. (Show Details)

rohan edited the summary of this revision. (Show Details)Thu, Mar 2, 10:22 AM

rohan edited the summary of this revision. (Show Details)Thu, Mar 2, 10:29 AM

Harbormaster completed remote builds in B17056: Diff 23356.Thu, Mar 2, 10:31 AM

rohan requested review of this revision.Thu, Mar 2, 10:31 AM

I'm going to wait until the architecture of pinned_messages is decided in D6924 before requesting review, I just put this up so I could continue working locally.

rohan requested review of this revision.Thu, Mar 2, 11:17 AM

rohan added inline comments.Thu, Mar 2, 11:24 AM

keyserver/src/database/migration-config.js
211 ↗	(On Diff #23356)	This is > 80 characters, I prefer to have the name and value of the variable on the same line but I can also push it to a new line

rohan added a child revision: D6929: [keyserver] Add a function to toggle the pin status of a message.Thu, Mar 2, 12:49 PM

I'm going to wait until the architecture of pinned_messages is decided in D6924 before requesting review, I just put this up so I could continue working locally.

Is this still the case? I've been holding off on reviewing this since D6924 is still on your queue, but please let me know if I should go ahead and review.

Sending this to your queue for now.

This revision now requires changes to proceed.Fri, Mar 10, 3:40 PM

Simplify the migration

rohan edited the summary of this revision. (Show Details)Mon, Mar 20, 12:00 PM

rohan edited the test plan for this revision. (Show Details)

rohan attached a referenced file: F435798: migration.png. (Show Details)

rohan attached a referenced file: F435799: roles.png. (Show Details)

rohan attached a referenced file: F435800: memberships.png. (Show Details)

rohan edited the summary of this revision. (Show Details)

Harbormaster completed remote builds in B17423: Diff 23876.Mon, Mar 20, 12:15 PM

ashoat accepted this revision.Mon, Mar 20, 2:49 PM

atul resigned from this revision.Mon, Mar 20, 2:50 PM