native] extend joinThread to handle auto join functionality
ClosedPublic
Actions

Authored by ginsu on Jun 17 2024, 11:39 PM.

Details

Reviewers

ashoat
kamil

Commits

rCOMM3e408b3e428d: [keysever/lib/native] extend joinThread to handle auto join functionality

Summary

This diff introduces two new changes to joinThread to support auto join functionality

We introduce an optional defaultSubscription option where we can define what we want the default subscription of the thread to be when we initially join.
We introduce a new check to see if the thread the user is trying to join is a root thread that has a farcaster channel tag which means it is "discoverable" and we can bypass the standard threadPermission check

Depends on D12444

Test Plan

EDIT: Confirmed that I was still getting the same result as the video below. Also confirmed that a user can still join a thread through the standard join thread button

Please see the demo video below

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

ginsu created this revision.Jun 17 2024, 11:39 PM

Herald added subscribers: tomek, ashoat. · View Herald TranscriptJun 17 2024, 11:39 PM

ginsu edited the summary of this revision. (Show Details)Jun 17 2024, 11:41 PM

ginsu edited the test plan for this revision. (Show Details)

ginsu added reviewers: ashoat, kamil.

ginsu added a parent revision: D12444: [keyserver] introduce setSubcriptionToDefault option in changeRole.

ginsu attached a referenced file: F2029868: Screen Recording 2024-06-14 at 1.33.12 PM.mov. (Show Details)

Harbormaster completed remote builds in B29736: Diff 41431.Jun 17 2024, 11:55 PM

ginsu requested review of this revision.Jun 17 2024, 11:55 PM

This diff allows anybody to join any thread they want, regardless of permissions!!

keyserver/src/responders/thread-responders.js
182 ↗	(On Diff #41431)	You are allowing any caller to just ignore all permissions! What's the point of the permission check if it can be overriden like this?
keyserver/src/updaters/thread-updaters.js
830 ↗	(On Diff #41431)	Why are we doing the permission check if we don't need it?
858–862 ↗	(On Diff #41431)	This isn't very readable. Can you do this instead? If necessary, update `defaultSubscription` type to `defaultSubscription?: ?SomeType` const defaultSubscription = request.autoJoin ? { home: false, pushNotifs: false } : undefined; const changeset = await changeRole(request.threadID, [viewer.userID], null, { defaultSubscription });

This revision now requires changes to proceed.Jun 18 2024, 9:57 AM

update

ginsu retitled this revision from [keysever/lib/native] introduce autoJoin to joinThread request to [keysever/lib/native] extend joinThread to handle auto join functionality.Jun 19 2024, 11:59 PM

ginsu edited the summary of this revision. (Show Details)

ginsu edited the test plan for this revision. (Show Details)

ginsu added inline comments.

lib/types/subscription-types.js
35–38 ↗	(On Diff #41540)	This might be helpful for D12494

ginsu mentioned this in D12494: [lib] Factor out defaultSubscription in createPendingThread.Jun 20 2024, 12:01 AM

Harbormaster completed remote builds in B29808: Diff 41540.Jun 20 2024, 12:09 AM

kamil resigned from this revision.Jun 21 2024, 3:36 AM

Perfect!

lib/types/subscription-types.js
35–38 ↗	(On Diff #41540)	I ended up landing that one before this one... would you mind deduping the version I introduced in `lib/shared/thread-utils.js`, so that it just imports this one instead? Sorry for the inconvenience