depends on D13295
updated the joinThread logic to:
- fetch user's farcaster ID from identity service
- fetch farcaster channels led by fid
- if channel tag for community is in list of channels led by fid, make user admin of thread
Details
Details
- created two new communities and tagged them with /varun (my new channel, of which i'm the lead) and /sovereignty (i just follow this channel)
- registered a new user and linked my farcaster account
- confirmed that i was admin of the community tagged with /varun and a member of the community tagged with /sovereignty
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| keyserver/src/updaters/thread-updaters.js | ||
|---|---|---|
| 895 ↗ | (On Diff #44073) | not combining this with the Promise.all on line 863 because these promises are only relevant if the conditions in lines 867-893 are not met |
Comment Actions
Basically ready but I want to review it one last time before you land
| keyserver/src/updaters/thread-updaters.js | ||
|---|---|---|
| 860 ↗ | (On Diff #44073) | I think it'd be a bit less messy to return this here: return { hasPermission: threadPermission || !!farcasterChannelTag, farcasterChannelTag, }; |
| 895–929 ↗ | (On Diff #44073) | I wonder if it would be cleaner to factor this out into a separate function:
I haven't thought too deeply about the set of inputs and outputs though – would it be messy? |
| 902 ↗ | (On Diff #44073) | What's the difference between identityInfo.userId and viewer.userID? Why do we use one in one place and another in the other? |
| 908–911 ↗ | (On Diff #44073) | What's the difference between this and: const { farcasterID } = response.identities[viewer.userID]; |
| keyserver/src/updaters/thread-updaters.js | ||
|---|---|---|
| 902 ↗ | (On Diff #44073) | this is the user ID of the keyserver owner, which we need in order to call the auth RPC. viewer.userID would be the requester's user ID |
| keyserver/src/updaters/thread-updaters.js | ||
|---|---|---|
| 860 ↗ | (On Diff #44073) | thanks. realized there's actually a bug in my current code. if the condition on line 837 is true, we never fetch the communityFarcasterChannelTag and thus cannot check if the user owns the corresponding farcaster channel (if one exists). i assume we want to make channel owners admins if they join via invite link too, i.e. not just via the auto join community handler. will fix this |
| 860 ↗ | (On Diff #44073) | forgot to publish this comment earlier, but the latest revision fixes the bug i mentioned here |
| 908–911 ↗ | (On Diff #44073) | forgot to use this shorthand, thanks |
| keyserver/src/updaters/thread-updaters.js | ||
|---|---|---|
| 834–866 ↗ | (On Diff #44098) | Good catch on the bug earlier. I think we can probably simplify this code just a little bit by keeping communityFarcasterChannelTagPromise outside of the closure: const communityFarcasterChannelTagPromise = fetchCommunityFarcasterChannelTag(viewer, request.threadID); const permissionPromise = (async () => { if (request.inviteLinkSecret) { return await checkIfInviteLinkIsValid( request.inviteLinkSecret, request.threadID, ); } const threadPermissionPromise = checkThreadPermission( viewer, request.threadID, threadPermissions.JOIN_THREAD, ); const [threadPermission, communityFarcasterChannelTag] = await Promise.all([ threadPermissionPromise, communityFarcasterChannelTagPromise, ]); return threadPermission || !!communityFarcasterChannelTag; })(); const [isMember, hasPermission, communityFarcasterChannelTag] = await Promise.all([ fetchViewerIsMember(viewer, request.threadID), permissionPromise, communityFarcasterChannelTagPromise, ]); |