Page MenuHomePhabricator
Differential D12154

[lib] Add `skipMemberAdminRoleCheck` argument to `threadIsWithBlockedUserOnly`
ClosedPublic
Actions

Authored by atul on May 21 2024, 8:27 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Mar 6, 7:04 PM
Unknown Object (File)
Tue, Mar 4, 6:33 PM
Unknown Object (File)
Tue, Mar 4, 6:33 PM
Unknown Object (File)
Tue, Mar 4, 6:33 PM
Unknown Object (File)
Tue, Mar 4, 6:33 PM
Unknown Object (File)
Tue, Mar 4, 6:33 PM
Unknown Object (File)
Tue, Mar 4, 6:33 PM
Unknown Object (File)
Tue, Mar 4, 6:33 PM
View All Files
Subscribers
None

Details

Reviewers
ashoat
ginsu
tomek
varun
Commits
rCOMMa962807ac47f: [lib] Add `skipMemberAdminRoleCheck` argument to `threadIsWithBlockedUserOnly`
Summary

We want this specifically for useThreadsWithPermission where we're doing member admin role check the "updated way." We handle the early return condition previously handled by threadOrParentThreadHasAdminRole using community root role check.

Considered a few other options which included

  • turning threadIsWithBlockedUserOnly into hook (would be tedious because we'd need to "hook-ify" a couple other functions and then have variants which accept collection of items vs. single item because hooks can't be called in loop, etc.)
  • creating duplicate/higher order functions similar to threadFrozenDueToBlock/threadFrozenDueToViewerBlock but the call stack would get more complex than needed imo

Depends on D12150

Test Plan

Will add log statements to make sure behavior is as expected. Specifically with native client connected to prod store since after MariaDB update my local DB is sparse.

Diff Detail

Repository
rCOMM Comm
Branch
master
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

atul created this revision.May 21 2024, 8:27 AM
atul edited the summary of this revision. (Show Details)May 21 2024, 8:28 AM
atul added inline comments.
lib/shared/thread-utils.js
1188–1189

We could create some type of options argument that's an object, but I think that this is fine if there are just two of these booleans and the callsites are fairly limited?

At least with VSCode, WebStorm, etc there are inlay hints that make things more clear eg:

00b489.png (348×818 px, 53 KB)

But can go ahead and add options obj if preferred.

atul attached a referenced file: F1808448: 00b489.png. (Show Details)May 21 2024, 8:30 AM
atul published this revision for review.May 21 2024, 9:03 AM
atul added a child revision: D12158: [lib] Remove `getCurrentUser`.May 21 2024, 9:06 AM
Harbormaster completed remote builds in B29117: Diff 40484.May 21 2024, 9:16 AM
varun resigned from this revision.May 22 2024, 7:41 AM

i'm probably not a good reviewer of this code

ashoat requested changes to this revision.May 24 2024, 3:03 AM
ashoat added inline comments.
lib/shared/thread-utils.js
1167

Can we avoid exposing this outside of this file?

  1. Rename this to innerThreadFrozenDueToBlock
  2. Introduce new threadFrozenDueToBlock that just calls innerThreadFrozenDueToBlock with false
1188–1189

Yeah, can you please do this? Personally I think we should "bag of params" when we have so many params and several of them are booleans

This revision now requires changes to proceed.May 24 2024, 3:03 AM
atul updated this revision to Diff 40786.May 30 2024, 11:21 AM

rebase before addressing feedback

atul planned changes to this revision.May 30 2024, 11:25 AM
Harbormaster completed remote builds in B29313: Diff 40786.May 30 2024, 11:38 AM
atul updated this revision to Diff 40790.EditedMay 30 2024, 11:41 AM

partially address feedback

  • new threadFrozenDueToBlock that calls innerThreadFrozenDueToBlock with skipMemberAdminRoleCheck set to false for callsites outside of file
  • create some sort of options object for threadIsWithBlockedUserOnly
Harbormaster completed remote builds in B29317: Diff 40790.May 30 2024, 12:01 PM
atul planned changes to this revision.May 30 2024, 12:09 PM
atul updated this revision to Diff 40797.EditedMay 30 2024, 7:07 PM

"create some sort of options object for threadIsWithBlockedUserOnly"

Harbormaster completed remote builds in B29322: Diff 40797.May 30 2024, 7:24 PM
ashoat accepted this revision.May 31 2024, 4:05 AM
ashoat added inline comments.
lib/shared/thread-utils.js
881–882 ↗(On Diff #40797)

Read-only

This revision is now accepted and ready to land.May 31 2024, 4:05 AM
atul added inline comments.May 31 2024, 11:29 AM
lib/shared/thread-utils.js
881–882 ↗(On Diff #40797)

Oops, cut/paste from function argument below and missed adding +. Will update diff.

atul edited the test plan for this revision. (Show Details)May 31 2024, 11:37 AM
atul updated this revision to Diff 40808.May 31 2024, 11:38 AM

make entries of ThreadIsWithBlockedUserOnlyOptions read-only

This revision was landed with ongoing or failed builds.May 31 2024, 11:44 AM
Closed by commit rCOMMa962807ac47f: [lib] Add `skipMemberAdminRoleCheck` argument to `threadIsWithBlockedUserOnly` (authored by atul). · Explain Why
This revision was automatically updated to reflect the committed changes.
atul added a commit: rCOMMa962807ac47f: [lib] Add `skipMemberAdminRoleCheck` argument to `threadIsWithBlockedUserOnly`.
Harbormaster completed remote builds in B29330: Diff 40808.May 31 2024, 11:55 AM

Revision Contents
Changeset List

PathSize
keyserver/
src/
fetchers/
1 line
lib/
shared/
23 lines

Diff 40484

View Options

keyserver/src/fetchers/thread-permission-fetchers.js

// @flow // @flow
import * as React from 'react'; import * as React from 'react';
import { setAuxUserFIDsActionType } from '../actions/aux-user-actions.js';
import { import {
updateRelationships as serverUpdateRelationships, updateRelationships as serverUpdateRelationships,
updateRelationshipsActionTypes, updateRelationshipsActionTypes,
} from '../actions/relationship-actions.js'; } from '../actions/relationship-actions.js';
import { NeynarClientContext } from '../components/neynar-client-provider.react.js'; import { NeynarClientContext } from '../components/neynar-client-provider.react.js';
import { useLegacyAshoatKeyserverCall } from '../keyserver-conn/legacy-keyserver-call.js'; import { useLegacyAshoatKeyserverCall } from '../keyserver-conn/legacy-keyserver-call.js';
import { cookieSelector } from '../selectors/keyserver-selectors.js'; import { cookieSelector } from '../selectors/keyserver-selectors.js';
import { IdentityClientContext } from '../shared/identity-client-context.js'; import { IdentityClientContext } from '../shared/identity-client-context.js';
import { relationshipActions } from '../types/relationship-types.js'; import { relationshipActions } from '../types/relationship-types.js';
import { authoritativeKeyserverID } from '../utils/authoritative-keyserver.js'; import { authoritativeKeyserverID } from '../utils/authoritative-keyserver.js';
import { useCurrentUserFID } from '../utils/farcaster-utils.js'; import { useCurrentUserFID } from '../utils/farcaster-utils.js';
import { useDispatchActionPromise } from '../utils/redux-promise-utils.js'; import { useDispatchActionPromise } from '../utils/redux-promise-utils.js';
import { useSelector } from '../utils/redux-utils.js'; import { useSelector, useDispatch } from '../utils/redux-utils.js';
import { usingCommServicesAccessToken } from '../utils/services-utils.js'; import { usingCommServicesAccessToken } from '../utils/services-utils.js';
function FarcasterDataHandler(): React.Node { function FarcasterDataHandler(): React.Node {
const isActive = useSelector(state => state.lifecycleState !== 'background'); const isActive = useSelector(state => state.lifecycleState !== 'background');
const cookie = useSelector(cookieSelector(authoritativeKeyserverID())); const cookie = useSelector(cookieSelector(authoritativeKeyserverID()));
const hasUserCookie = !!(cookie && cookie.startsWith('user=')); const hasUserCookie = !!(cookie && cookie.startsWith('user='));
const currentUserID = useSelector(state => state.currentUserInfo?.id); const currentUserID = useSelector(state => state.currentUserInfo?.id);
const loggedIn = !!currentUserID && hasUserCookie; const loggedIn = !!currentUserID && hasUserCookie;
const neynarClient = React.useContext(NeynarClientContext)?.client; const neynarClient = React.useContext(NeynarClientContext)?.client;
const identityServiceClient = React.useContext(IdentityClientContext); const identityServiceClient = React.useContext(IdentityClientContext);
const getFarcasterUsers = const getFarcasterUsers =
identityServiceClient?.identityClient.getFarcasterUsers; identityServiceClient?.identityClient.getFarcasterUsers;
const findUserIdentities =
identityServiceClient?.identityClient.findUserIdentities;
const dispatch = useDispatch();
const dispatchActionPromise = useDispatchActionPromise(); const dispatchActionPromise = useDispatchActionPromise();
const updateRelationships = useLegacyAshoatKeyserverCall( const updateRelationships = useLegacyAshoatKeyserverCall(
serverUpdateRelationships, serverUpdateRelationships,
); );
const createThreadsAndRobotextForFarcasterMutuals = React.useCallback( const createThreadsAndRobotextForFarcasterMutuals = React.useCallback(
(userIDsToFID: { +[userID: string]: string }) => (userIDsToFID: { +[userID: string]: string }) =>
updateRelationships({ updateRelationships({
action: relationshipActions.FARCASTER_MUTUAL, action: relationshipActions.FARCASTER_MUTUAL,
userIDsToFID, userIDsToFID,
}), }),
[updateRelationships], [updateRelationships],
); );
const userInfos = useSelector(state => state.userStore.userInfos); const userInfos = useSelector(state => state.userStore.userInfos);
const fid = useCurrentUserFID(); const fid = useCurrentUserFID();
const prevCanQueryRef = React.useRef<?boolean>(); const prevCanQueryRef = React.useRef<?boolean>();
React.useEffect(() => {
if (!usingCommServicesAccessToken) {
return;
}
const handleFarcasterMutuals = React.useCallback(async () => {
const canQuery = isActive && !!fid && loggedIn; const canQuery = isActive && !!fid && loggedIn;
if (canQuery === prevCanQueryRef.current) { if (canQuery === prevCanQueryRef.current) {
return; return;
} }
prevCanQueryRef.current = canQuery; prevCanQueryRef.current = canQuery;
if ( if (
!loggedIn || !loggedIn ||
!isActive || !isActive ||
!fid || !fid ||
!neynarClient || !neynarClient ||
!getFarcasterUsers || !getFarcasterUsers ||
!currentUserID !currentUserID
) { ) {
return; return;
} }
void (async () => {
const followerFIDs = await neynarClient.fetchFriendFIDs(fid); const followerFIDs = await neynarClient.fetchFriendFIDs(fid);
const commFCUsers = await getFarcasterUsers(followerFIDs); const commFCUsers = await getFarcasterUsers(followerFIDs);
const newCommUsers = commFCUsers.filter( const newCommUsers = commFCUsers.filter(({ userID }) => !userInfos[userID]);
({ userID }) => !userInfos[userID],
);
if (newCommUsers.length === 0) { if (newCommUsers.length === 0) {
return; return;
} }
const userIDsToFID: { +[userID: string]: string } = Object.fromEntries( const userIDsToFID: { +[userID: string]: string } = Object.fromEntries(
newCommUsers.map(({ userID, farcasterID }) => [userID, farcasterID]), newCommUsers.map(({ userID, farcasterID }) => [userID, farcasterID]),
); );
const userIDsToFIDIncludingCurrentUser: { +[userID: string]: string } = { const userIDsToFIDIncludingCurrentUser: { +[userID: string]: string } = {
...userIDsToFID, ...userIDsToFID,
[(currentUserID: string)]: fid, [(currentUserID: string)]: fid,
}; };
void dispatchActionPromise( void dispatchActionPromise(
updateRelationshipsActionTypes, updateRelationshipsActionTypes,
createThreadsAndRobotextForFarcasterMutuals( createThreadsAndRobotextForFarcasterMutuals(
userIDsToFIDIncludingCurrentUser, userIDsToFIDIncludingCurrentUser,
), ),
); );
})();
}, [ }, [
isActive, isActive,
fid, fid,
loggedIn, loggedIn,
neynarClient, neynarClient,
getFarcasterUsers, getFarcasterUsers,
userInfos, userInfos,
dispatchActionPromise, dispatchActionPromise,
createThreadsAndRobotextForFarcasterMutuals, createThreadsAndRobotextForFarcasterMutuals,
currentUserID, currentUserID,
]); ]);
const handleUserStoreFIDs = React.useCallback(async () => {
if (!loggedIn || !isActive || !findUserIdentities) {
return;
}
const userStoreIDs = Object.keys(userInfos);
const userIdentities = await findUserIdentities(userStoreIDs);
const userStoreFarcasterUsers = Object.entries(userIdentities)
.filter(([, identity]) => identity.farcasterID !== null)
.map(([userID, identity]) => ({
userID,
username: identity.username,
farcasterID: identity.farcasterID,
}));
dispatch({
type: setAuxUserFIDsActionType,
payload: {
farcasterUsers: userStoreFarcasterUsers,
},
});
willAuthorUnsubmitted
Done
Inline Actions

I introduced a previous diff where setAuxUserFIDs now clears fids prior to updating with new fids from the payload. The previous functionality of setAuxUserFIDs of just adding without clearing is now a new action type addAuxUserFIDs

There are now two callbacks which are called by a single useEffect. The two callbacks are called handleFarcasterMutuals and handleUserStoreFIDs

will: I introduced a previous diff where `setAuxUserFIDs` now clears fids prior to updating with new…
}, [loggedIn, isActive, findUserIdentities, userInfos, dispatch]);
React.useEffect(() => {
if (!usingCommServicesAccessToken) {
return;
}
void (async () => {
await Promise.all([handleFarcasterMutuals(), handleUserStoreFIDs()]);
},
[]);
ashoatUnsubmitted
Not Done
Inline Actions
return;
}
- void (async () => {
- await Promise.all([handleFarcasterMutuals(), handleUserStoreFIDs()]);
- },
- []);
+ void handleFarcasterMutuals();
+ void handleUserStoreFIDs();
}, [handleFarcasterMutuals, handleUserStoreFIDs]);
  1. Line 145 isn't doing anything
  2. Since we aren't waiting on these anyways, we can simplify this a little bit
ashoat: 1. Line 145 isn't doing anything 2. Since we aren't waiting on these anyways, we can simplify…
}, [handleFarcasterMutuals, handleUserStoreFIDs]);
return null; return null;
} }
export { FarcasterDataHandler }; export { FarcasterDataHandler };
View Options

lib/shared/thread-utils.js

// @flow // @flow
import * as React from 'react'; import * as React from 'react';
import { setAuxUserFIDsActionType } from '../actions/aux-user-actions.js';
import { import {
updateRelationships as serverUpdateRelationships, updateRelationships as serverUpdateRelationships,
updateRelationshipsActionTypes, updateRelationshipsActionTypes,
} from '../actions/relationship-actions.js'; } from '../actions/relationship-actions.js';
import { NeynarClientContext } from '../components/neynar-client-provider.react.js'; import { NeynarClientContext } from '../components/neynar-client-provider.react.js';
import { useLegacyAshoatKeyserverCall } from '../keyserver-conn/legacy-keyserver-call.js'; import { useLegacyAshoatKeyserverCall } from '../keyserver-conn/legacy-keyserver-call.js';
import { cookieSelector } from '../selectors/keyserver-selectors.js'; import { cookieSelector } from '../selectors/keyserver-selectors.js';
import { IdentityClientContext } from '../shared/identity-client-context.js'; import { IdentityClientContext } from '../shared/identity-client-context.js';
import { relationshipActions } from '../types/relationship-types.js'; import { relationshipActions } from '../types/relationship-types.js';
import { authoritativeKeyserverID } from '../utils/authoritative-keyserver.js'; import { authoritativeKeyserverID } from '../utils/authoritative-keyserver.js';
import { useCurrentUserFID } from '../utils/farcaster-utils.js'; import { useCurrentUserFID } from '../utils/farcaster-utils.js';
import { useDispatchActionPromise } from '../utils/redux-promise-utils.js'; import { useDispatchActionPromise } from '../utils/redux-promise-utils.js';
import { useSelector } from '../utils/redux-utils.js'; import { useSelector, useDispatch } from '../utils/redux-utils.js';
import { usingCommServicesAccessToken } from '../utils/services-utils.js'; import { usingCommServicesAccessToken } from '../utils/services-utils.js';
function FarcasterDataHandler(): React.Node { function FarcasterDataHandler(): React.Node {
const isActive = useSelector(state => state.lifecycleState !== 'background'); const isActive = useSelector(state => state.lifecycleState !== 'background');
const cookie = useSelector(cookieSelector(authoritativeKeyserverID())); const cookie = useSelector(cookieSelector(authoritativeKeyserverID()));
const hasUserCookie = !!(cookie && cookie.startsWith('user=')); const hasUserCookie = !!(cookie && cookie.startsWith('user='));
const currentUserID = useSelector(state => state.currentUserInfo?.id); const currentUserID = useSelector(state => state.currentUserInfo?.id);
const loggedIn = !!currentUserID && hasUserCookie; const loggedIn = !!currentUserID && hasUserCookie;
const neynarClient = React.useContext(NeynarClientContext)?.client; const neynarClient = React.useContext(NeynarClientContext)?.client;
const identityServiceClient = React.useContext(IdentityClientContext); const identityServiceClient = React.useContext(IdentityClientContext);
const getFarcasterUsers = const getFarcasterUsers =
identityServiceClient?.identityClient.getFarcasterUsers; identityServiceClient?.identityClient.getFarcasterUsers;
const findUserIdentities =
identityServiceClient?.identityClient.findUserIdentities;
const dispatch = useDispatch();
const dispatchActionPromise = useDispatchActionPromise(); const dispatchActionPromise = useDispatchActionPromise();
const updateRelationships = useLegacyAshoatKeyserverCall( const updateRelationships = useLegacyAshoatKeyserverCall(
serverUpdateRelationships, serverUpdateRelationships,
); );
const createThreadsAndRobotextForFarcasterMutuals = React.useCallback( const createThreadsAndRobotextForFarcasterMutuals = React.useCallback(
(userIDsToFID: { +[userID: string]: string }) => (userIDsToFID: { +[userID: string]: string }) =>
updateRelationships({ updateRelationships({
action: relationshipActions.FARCASTER_MUTUAL, action: relationshipActions.FARCASTER_MUTUAL,
userIDsToFID, userIDsToFID,
}), }),
[updateRelationships], [updateRelationships],
); );
const userInfos = useSelector(state => state.userStore.userInfos); const userInfos = useSelector(state => state.userStore.userInfos);
const fid = useCurrentUserFID(); const fid = useCurrentUserFID();
const prevCanQueryRef = React.useRef<?boolean>(); const prevCanQueryRef = React.useRef<?boolean>();
React.useEffect(() => {
if (!usingCommServicesAccessToken) {
return;
}
const handleFarcasterMutuals = React.useCallback(async () => {
const canQuery = isActive && !!fid && loggedIn; const canQuery = isActive && !!fid && loggedIn;
if (canQuery === prevCanQueryRef.current) { if (canQuery === prevCanQueryRef.current) {
return; return;
} }
prevCanQueryRef.current = canQuery; prevCanQueryRef.current = canQuery;
if ( if (
!loggedIn || !loggedIn ||
!isActive || !isActive ||
!fid || !fid ||
!neynarClient || !neynarClient ||
!getFarcasterUsers || !getFarcasterUsers ||
!currentUserID !currentUserID
) { ) {
return; return;
} }
void (async () => {
const followerFIDs = await neynarClient.fetchFriendFIDs(fid); const followerFIDs = await neynarClient.fetchFriendFIDs(fid);
const commFCUsers = await getFarcasterUsers(followerFIDs); const commFCUsers = await getFarcasterUsers(followerFIDs);
const newCommUsers = commFCUsers.filter( const newCommUsers = commFCUsers.filter(({ userID }) => !userInfos[userID]);
({ userID }) => !userInfos[userID],
);
if (newCommUsers.length === 0) { if (newCommUsers.length === 0) {
return; return;
} }
const userIDsToFID: { +[userID: string]: string } = Object.fromEntries( const userIDsToFID: { +[userID: string]: string } = Object.fromEntries(
newCommUsers.map(({ userID, farcasterID }) => [userID, farcasterID]), newCommUsers.map(({ userID, farcasterID }) => [userID, farcasterID]),
); );
const userIDsToFIDIncludingCurrentUser: { +[userID: string]: string } = { const userIDsToFIDIncludingCurrentUser: { +[userID: string]: string } = {
...userIDsToFID, ...userIDsToFID,
[(currentUserID: string)]: fid, [(currentUserID: string)]: fid,
}; };
void dispatchActionPromise( void dispatchActionPromise(
updateRelationshipsActionTypes, updateRelationshipsActionTypes,
createThreadsAndRobotextForFarcasterMutuals( createThreadsAndRobotextForFarcasterMutuals(
userIDsToFIDIncludingCurrentUser, userIDsToFIDIncludingCurrentUser,
), ),
); );
})();
}, [ }, [
isActive, isActive,
fid, fid,
loggedIn, loggedIn,
neynarClient, neynarClient,
getFarcasterUsers, getFarcasterUsers,
userInfos, userInfos,
dispatchActionPromise, dispatchActionPromise,
createThreadsAndRobotextForFarcasterMutuals, createThreadsAndRobotextForFarcasterMutuals,
currentUserID, currentUserID,
]); ]);
const handleUserStoreFIDs = React.useCallback(async () => {
if (!loggedIn || !isActive || !findUserIdentities) {
return;
}
const userStoreIDs = Object.keys(userInfos);
const userIdentities = await findUserIdentities(userStoreIDs);
const userStoreFarcasterUsers = Object.entries(userIdentities)
.filter(([, identity]) => identity.farcasterID !== null)
.map(([userID, identity]) => ({
userID,
username: identity.username,
farcasterID: identity.farcasterID,
}));
dispatch({
type: setAuxUserFIDsActionType,
payload: {
farcasterUsers: userStoreFarcasterUsers,
},
});
willAuthorUnsubmitted
Done
Inline Actions

I introduced a previous diff where setAuxUserFIDs now clears fids prior to updating with new fids from the payload. The previous functionality of setAuxUserFIDs of just adding without clearing is now a new action type addAuxUserFIDs

There are now two callbacks which are called by a single useEffect. The two callbacks are called handleFarcasterMutuals and handleUserStoreFIDs

will: I introduced a previous diff where `setAuxUserFIDs` now clears fids prior to updating with new…
}, [loggedIn, isActive, findUserIdentities, userInfos, dispatch]);
React.useEffect(() => {
if (!usingCommServicesAccessToken) {
return;
}
void (async () => {
await Promise.all([handleFarcasterMutuals(), handleUserStoreFIDs()]);
},
[]);
ashoatUnsubmitted
Not Done
Inline Actions
return;
}
- void (async () => {
- await Promise.all([handleFarcasterMutuals(), handleUserStoreFIDs()]);
- },
- []);
+ void handleFarcasterMutuals();
+ void handleUserStoreFIDs();
}, [handleFarcasterMutuals, handleUserStoreFIDs]);
  1. Line 145 isn't doing anything
  2. Since we aren't waiting on these anyways, we can simplify this a little bit
ashoat: 1. Line 145 isn't doing anything 2. Since we aren't waiting on these anyways, we can simplify…
}, [handleFarcasterMutuals, handleUserStoreFIDs]);
return null; return null;
} }
export { FarcasterDataHandler }; export { FarcasterDataHandler };