Page MenuHomePhabricator

[lib] Use configurableCommunityPermissions in getRolePermissionBlobsForCommunityRoot
AbandonedPublic

Authored by rohan on Nov 17 2023, 9:02 AM.
Tags
None
Referenced Files
F2184938: D9924.id33389.diff
Wed, Jul 3, 10:27 PM
Unknown Object (File)
Wed, Jul 3, 3:31 AM
Unknown Object (File)
Tue, Jul 2, 4:51 PM
Unknown Object (File)
Tue, Jul 2, 11:08 AM
Unknown Object (File)
Sat, Jun 29, 2:47 AM
Unknown Object (File)
Fri, Jun 28, 3:19 PM
Unknown Object (File)
Thu, Jun 27, 9:45 AM
Unknown Object (File)
May 25 2024, 2:56 PM
Subscribers

Details

Reviewers
atul
ginsu
ashoat
Summary

The next step in refactoring getRolePermissionBlobsForCommunityRoot is to use the user-surfaced permissions in configurableCommunityPermissions when constructing the initial Members blob for communities.

This will allow the system to be a little more unified, and for changes to any of the user-surfaced permissions to not diverge from the initial Members role blob.

Test Plan

Tested this in three different ways.

Client
I created a COMMUNITY_ROOT and COMMUNITY_ANNOUNCEMENT_ROOT both before and after the changes in this diff. For each community type, I compared the pre-selected user-surfaced options when going in to edit the Members role. I confirmed that they are the same both before and after.

MariaDB Permissions
Took the permissions blobs from MariaDB for Members created both before and after the changes for COMMUNITY_ROOT and put them into https://www.jsondiff.com/. Verified they were identical. I did the same thing for the Members permission blobs created for COMMUNITY_ANNOUNCEMENT_ROOT before and after these changes as well.

lodash isEqual
For voiced permission blob (top-level):

const old_voicedPermissions = {
  [threadPermissions.VOICED]: true,
  [threadPermissions.EDIT_ENTRIES]: true,
  [threadPermissions.EDIT_THREAD_NAME]: true,
  [threadPermissions.EDIT_THREAD_COLOR]: true,
  [threadPermissions.EDIT_THREAD_DESCRIPTION]: true,
  [threadPermissions.EDIT_THREAD_AVATAR]: true,
  [threadPermissions.CREATE_SUBCHANNELS]: true,
  [threadPermissions.ADD_MEMBERS]: true,
};

const voicedPermissions = {
  ...editCalendar,
  ...createAndEditChannels,
  [threadPermissions.ADD_MEMBERS]: true,
  [threadPermissions.VOICED]: true,
};

console.log(
  'VoicedPermissions ',
  _isEqual(old_voicedPermissions, voicedPermissions),
);

For genesisMemberPermissions

const old_genesisMemberPermissions = {
  [threadPermissions.KNOW_OF]: true,
  [threadPermissions.VISIBLE]: true,
  [openDescendantKnowOf]: true,
  [openDescendantVisible]: true,
  [openTopLevelDescendantJoinThread]: true,
};

const genesisUniversalPermissions = getUniversalCommunityRootPermissionsBlob(
  threadTypes.GENESIS,
);


console.log(
  'Genesis ',
  _isEqual(old_genesisMemberPermissions, genesisUniversalPermissions),
);

For baseMemberPermissions

const old_baseMemberPermissions = {
    ...old_genesisMemberPermissions,
    [threadPermissions.REACT_TO_MESSAGE]: true,
    [threadPermissions.EDIT_MESSAGE]: true,
    [threadPermissions.LEAVE_THREAD]: true,
    [threadPermissions.CREATE_SIDEBARS]: true,
    [threadPermissions.ADD_MEMBERS]: true,
    [openChildJoinThread]: true,
    [openChildAddMembers]: true,
  };

const memberUniversalPermissions =
    getUniversalCommunityRootPermissionsBlob(threadType);

  const baseMemberPermissions = {
    ...reactToMessages,
    ...editMessages,
    ...addMembers,
    ...memberUniversalPermissions,
    ...genesisUniversalPermissions,
  };

  console.log(
    'Members ',
    _isEqual(old_baseMemberPermissions, baseMemberPermissions),
  );

Test output:

VoicedPermissions  true

Calling for COMMUNITY_ANNOUNCEMENT_ROOT
Genesis  true
Members  true

Calling for COMMUNITY_ROOT
Genesis  true
Members  false
{ voiced: true }

Seeing the voiced permission blob being equal before and after and the genesis permission blob before and after is good news (means it's a no-op change ultimately).

For COMMUNITY_ANNOUNCEMENT_ROOT, seeing the base members permission blob being the same before and after is also good news.

For COMMUNITY_ROOT, seeing the base members permission blob have just one difference is as expected since we include voiced as a universal community permission for COMMUNITY_ROOT. For this thread type, the top-level voiced permissions blob gets merged into the base members permission blob, so this different ultimately goes away anyways.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

rohan held this revision as a draft.

Adding some annotations here to help reviewers compare the before/after of the permissions changes

lib/permissions/thread-permissions.js
182–187

These are no longer used

189–195

Covered by getting the universal community permissions for threadTypes.GENESIS (genesisUniversalPermissions variable)

197–198

Left these as-is since voicedPermissions is also used outside of getRolePermissionBlobsForCommunityRoot and I don't want to change permissions for individual channels.

In getRolePermissionBlobsForCommunityRoot I end up using the user-surfaced permission ADD_MEMBERS which corresponds to both 'add_members' and 'child_open_add_members', so leaving [threadPermissions.ADD_MEMBERS] doesn't cause any issues

198

corresponds to userSurfacedPermissions.REACT_TO_MESSAGES (reactToMessages variable)

199

corresponds to userSurfacedPermissions.EDIT_MESSAGES (editMessages variable)

200

Covered in the universal community permissions (memberUniversalPermissions variable)

201

Covered in the universal community permissions (memberUniversalPermissions variable)

202

corresponds to userSurfacedPermissions.ADD_MEMBERS (addMembers variable)

203

Covered in the universal community permissions (memberUniversalPermissions variable)

204

corresponds to userSurfacedPermissions.ADD_MEMBERS(addMembers variable)

397–398

Swapped the order here to appease flow

lib/permissions/thread-permissions.js
170

corresponds to userSurfacedPermissions.EDIT_CALENDAR (editCalendar variable)

171–175

corresponds to userSurfacedPermissions.CREATE_AND_EDIT_CHANNELS (createAndEditChannels variable)

rohan published this revision for review.Nov 17 2023, 10:15 AM
ashoat requested changes to this revision.Nov 19 2023, 11:05 AM
ashoat added inline comments.
lib/permissions/thread-permissions.js
210

Talked to @rohan about this in our 1:1, but to summarize – would love if we could move the userSurfacedPermissions -> threadPermissions step a bit later

So basically – collect set of userSurfacedPermissions first, then map them to threadPermissions and flatten

Ideally all of baseMemberPermissions / genesisUniversalPermissions / voicedPermissions are defined in terms of userSurfacedPermissions

This revision now requires changes to proceed.Nov 19 2023, 11:05 AM