Page MenuHomePhabricator

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

Authored by rohan on Mar 2 2023, 10:15 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Mar 15, 9:52 AM
Unknown Object (File)
Fri, Mar 15, 9:52 AM
Unknown Object (File)
Thu, Mar 7, 1:31 PM
Unknown Object (File)
Thu, Mar 7, 1:04 PM
Unknown Object (File)
Thu, Mar 7, 12:55 PM
Unknown Object (File)
Thu, Mar 7, 11:26 AM
Unknown Object (File)
Thu, Mar 7, 11:25 AM
Unknown Object (File)
Thu, Mar 7, 10:56 AM
Subscribers

Details

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:

  1. Update the baseAdminPriviledges so new chats with admins will have the correct, updated permissions
  2. 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 D7199

Test Plan
  1. Confirm that the migration is successful

migration.png (358×1 px, 297 KB)

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

roles.png (254×2 px, 131 KB)

  1. 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)

memberships.png (306×2 px, 162 KB)

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 requested review of this revision.Mar 2 2023, 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.Mar 2 2023, 11:17 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

atul requested changes to this revision.Mar 10 2023, 3:40 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.Mar 10 2023, 3:40 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)
rohan edited the summary of this revision. (Show Details)
This revision is now accepted and ready to land.Mar 20 2023, 2:50 PM