Paths

Table of Contentst

-
lib/shared/
-
shared/
1/1
thread-utils.js

[lib] Pull `filterOutDisabledPermissions(permissionsBitmask)` out of `getCurrentUser`
ClosedPublic
Actions

Authored by atul on Apr 23 2024, 10:55 AM.

Details

Reviewers

ginsu
tomek
varun
will

Commits

rCOMMec961262c8c1: [lib] Pull `filterOutDisabledPermissions(permissionsBitmask)` out of…

Summary

We will need to use this logic at permission check sites (eg threadMemberHasPermission/threadHasPermission/etc). Eventually we'll consolidate so we use one hook for everything, but for now we're going to use this in a couple places so we can first remove usage from getCurrentUser.

Depends on D11741

Test Plan

This is a very simple extract/move diff. flow + careful reading + will be tested implicitly by later diffs in the stack.

Diff Detail

Repository

rCOMM Comm

Branch

arcpatch-D11742 (branched from master)

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

atul created this revision.Apr 23 2024, 10:55 AM

Herald added a subscriber: ashoat. · View Herald TranscriptApr 23 2024, 10:55 AM

Harbormaster completed remote builds in B28413: Diff 39418.Apr 23 2024, 11:11 AM

atul requested review of this revision.Apr 23 2024, 11:11 AM

atul added a child revision: D11746: [lib] Introduce `useThreadHasPermission` hook.Apr 23 2024, 12:29 PM

tomek accepted this revision.Apr 24 2024, 4:29 AM

tomek added inline comments.

lib/shared/thread-utils.js
993 ↗	(On Diff #39418)	This function name sounds confusing. We receive a permissions bitmask, decode it, add more permissions to it, and encode. So we're adding some permissions instead of filtering some out.

This revision is now accepted and ready to land.Apr 24 2024, 4:29 AM

atul added inline comments.Apr 24 2024, 12:22 PM

lib/shared/thread-utils.js
993 ↗	(On Diff #39418)	We're not adding more permissions, but we could potentially be adding more permission entries. We're overriding value of permissions in `decodedPermission` (if they exist) with `{ value: false, source: null }` if the key/permission is also in `disabledPermissions`: So we're effectively filtering out permissions, but could also name this something like `patchDisabledPermissions` or `patchPermissionsBitmaskForBlock` or something like that? If that would be preferred I can create a followup diff doing that rename. I think current name makes sense for now.

atul attached a referenced file: F1654016: 851623.png. (Show Details)Apr 24 2024, 12:22 PM

land trivial

This revision was landed with ongoing or failed builds.Apr 24 2024, 12:38 PM

Closed by commit rCOMMec961262c8c1: [lib] Pull `filterOutDisabledPermissions(permissionsBitmask)` out of… (authored by atul). · Explain Why

This revision was automatically updated to reflect the committed changes.

atul added a commit: rCOMMec961262c8c1: [lib] Pull `filterOutDisabledPermissions(permissionsBitmask)` out of….

Harbormaster completed remote builds in B28436: Diff 39445.Apr 24 2024, 12:43 PM

Revision Contents
Changeset List

Path

Size

lib/

shared/

thread-utils.js

22 lines

Diff 39445

View Options

lib/shared/thread-utils.js

Show First 20 Lines • Show All 147 Lines • ▼ Show 20 Lines `${permission} can be disabled by a block, but threadHasPermission can't ` +

'check for a block on RawThreadInfo. Please pass in ThreadInfo instead!', 'check for a block on RawThreadInfo. Please pass in ThreadInfo instead!',

); );

if (threadInfo.minimallyEncoded) { if (threadInfo.minimallyEncoded) {

return hasPermission(threadInfo.currentUser.permissions, permission); return hasPermission(threadInfo.currentUser.permissions, permission);

} }

return permissionLookup(threadInfo.currentUser.permissions, permission); return permissionLookup(threadInfo.currentUser.permissions, permission);

} }

function useThreadHasPermission( function useThreadsWithPermission(

threadInfo: ?ThreadInfo, threadInfos: $ReadOnlyArray<ThreadInfo>,

permission: ThreadPermission, permission: ThreadPermission,

): boolean { ): $ReadOnlyArray<ThreadInfo> {

const loggedInUserInfo = useLoggedInUserInfo(); const loggedInUserInfo = useLoggedInUserInfo();

const userInfos = useSelector(state => state.userStore.userInfos); const userInfos = useSelector(state => state.userStore.userInfos);

return React.useMemo(() => { return React.useMemo(() => {

if (!threadInfo) { return threadInfos.filter((threadInfo: ThreadInfo) => {

return false;

}

const permissions = threadFrozenDueToBlock( const permissions = threadFrozenDueToBlock(

threadInfo, threadInfo,

loggedInUserInfo?.id, loggedInUserInfo?.id,

userInfos, userInfos,

) )

? filterOutDisabledPermissions(threadInfo.currentUser.permissions) ? filterOutDisabledPermissions(threadInfo.currentUser.permissions)

: threadInfo.currentUser.permissions; : threadInfo.currentUser.permissions;

return hasPermission(permissions, permission); return hasPermission(permissions, permission);

}, [loggedInUserInfo?.id, permission, threadInfo, userInfos]); });

}, [loggedInUserInfo?.id, permission, threadInfos, userInfos]);

}

function useThreadHasPermission(

threadInfo: ?ThreadInfo,

permission: ThreadPermission,

): boolean {

return (

useThreadsWithPermission(threadInfo ? [threadInfo] : [], permission)

.length === 1

);

ashoatUnsubmitted

Done

): boolean {

- return (

- useThreadsWithPermission(threadInfo ? [threadInfo] : [], permission)

- .length === 1

- );

+ const threads = useThreadsWithPermission(threadInfo ? [threadInfo] : [], permission);

+ return threads.length === 1;

}

function viewerIsMember(

Can we extract this to its own line? Similar to how we want await to be visible, I think it's good for hooks to be as well

ashoat: Can we extract this to its own line? Similar to how we want `await` to be visible, I think it's…

} }

function viewerIsMember( function viewerIsMember(

threadInfo: ?(ThreadInfo | LegacyRawThreadInfo | RawThreadInfo), threadInfo: ?(ThreadInfo | LegacyRawThreadInfo | RawThreadInfo),

): boolean { ): boolean {

return !!( return !!(

threadInfo && threadInfo &&

threadInfo.currentUser.role !== null && threadInfo.currentUser.role !== null &&

▲ Show 20 Lines • Show All 1,812 Lines • Show Last 20 Lines

[lib] Pull `filterOutDisabledPermissions(permissionsBitmask)` out of `getCurrentUser`ClosedPublicActions