Page MenuHomePhabricator

[keyserver] update joinThread to automatically make farcaster channel lead a community admin
ClosedPublic

Authored by varun on Sep 11 2024, 4:23 PM.
Tags
None
Referenced Files
F3203984: D13300.diff
Sat, Nov 9, 8:38 PM
Unknown Object (File)
Fri, Oct 18, 7:41 AM
Unknown Object (File)
Mon, Oct 14, 2:56 PM
Unknown Object (File)
Mon, Oct 14, 2:56 PM
Unknown Object (File)
Mon, Oct 14, 2:56 PM
Unknown Object (File)
Mon, Oct 14, 2:56 PM
Unknown Object (File)
Mon, Oct 14, 2:56 PM
Unknown Object (File)
Mon, Oct 14, 2:28 PM
Subscribers

Details

Summary

depends on D13295

updated the joinThread logic to:

  1. fetch user's farcaster ID from identity service
  2. fetch farcaster channels led by fid
  3. if channel tag for community is in list of channels led by fid, make user admin of thread
Test Plan
  1. 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)
  2. registered a new user and linked my farcaster account
  3. confirmed that i was admin of the community tagged with /varun and a member of the community tagged with /sovereignty

Diff Detail

Repository
rCOMM Comm
Branch
farcasteradmin (branched from master)
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

varun held this revision as a draft.
varun edited the summary of this revision. (Show Details)
varun edited the test plan for this revision. (Show Details)

publish

varun published this revision for review.Sep 12 2024, 4:21 AM
varun added inline comments.
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

ashoat requested changes to this revision.Sep 12 2024, 8:21 AM

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:

  • You could avoid some of the nested awaits with some early exits
  • It would "self-document" to have a name for this batch of logic

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];
This revision now requires changes to proceed.Sep 12 2024, 8:21 AM
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

ashoat added inline comments.
keyserver/src/updaters/thread-updaters.js
834–866

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,
  ]);
This revision is now accepted and ready to land.Sep 12 2024, 12:30 PM