HomePhabricator
Diffusion Comm 11d0868e0328

[lib] Prevent editing role permissions from adding unexpected permissions

Description

[lib] Prevent editing role permissions from adding unexpected permissions

Summary:
Full context is in the comment in comment in ENG-5622, but basically there was not a clear 1:1 match of permissions before and after editing a role sometimes. This is because the user-surfaced permissions provided some descendant_* permissions that Members, upon creation, did not originally have.

This diff fixes that so there's a clear equality between the two when permissions are edited.

Some user-surfaced permissions like KNOW_OF_SECRET_CHANNELS should still have their descendant_* permissions since that's how we can allow non-Admins to see secret channels

Addresses ENG-5622

After this, I'll need to update the MariaDB migration in D9599 and the redux persist migration in D9608 to remove all of these permissions. That's why I've added them to an array in lib called permissionsToRemoveInMigration so I can share one common object, rather than copy and pasting the huge collection of strings in both migrations.

Adding @ashoat as a reviewer here since I think he may have the most context on my proposed solution

Depends on D9493

Test Plan:
Created some communities, edited some permissions for the Members role and then removed them, and created some custom roles with some random permissions.

Ran yarn script dist/scripts/validate-role-permissions.js to verify that there are no discrepencies. Before this diff, the permissions blob would have unexpected permissions like

deepDiff(existingRolePermissions, baseExpectedPermissionBlob) = {
  "join_thread": true,
  "descendant_react_to_message": true,
  "descendant_edit_message": true,
  "descendant_add_members": true,
  "descendant_edit_entries": true,
  "descendant_edit_thread": true,
  "descendant_edit_thread_description": true,
  "descendant_edit_thread_color": true,
  "descendant_toplevel_create_subthreads": true,
  "descendant_edit_thread_avatar": true,
  "descendant_toplevel_create_sidebars": true
}

Reviewers: ashoat, atul, ginsu

Reviewed By: ashoat

Subscribers: tomek, wyilio, ashoat

Differential Revision: https://phab.comm.dev/D9686