Page MenuHomePhabricator

[lib] introduce useJoinCommunity
ClosedPublic

Authored by ginsu on Jul 9 2024, 9:49 AM.
Tags
None
Referenced Files
F3374243: D12706.id42147.diff
Tue, Nov 26, 2:37 PM
F3374209: D12706.id42218.diff
Tue, Nov 26, 2:26 PM
F3374187: D12706.id42244.diff
Tue, Nov 26, 2:18 PM
F3374178: D12706.id42246.diff
Tue, Nov 26, 2:17 PM
F3374165: D12706.id42195.diff
Tue, Nov 26, 2:13 PM
F3373867: D12706.diff
Tue, Nov 26, 12:21 PM
Unknown Object (File)
Sun, Nov 17, 5:17 PM
Unknown Object (File)
Mon, Nov 11, 1:15 AM
Subscribers

Details

Summary

Move diff. This diff introduces useJoinCommunity. useJoinCommunity factors out the shared logic between joining a community through accepting an invite link and joining a community through the auto join community handler. Subsequent diffs will handle making useJoinCommunity more agnostic so that the hook can be used at both callsites

Depends on D12702

Test Plan

flow + confirmed that there were no regressions with the joining a community through accepitng an invite link user flow

Diff Detail

Repository
rCOMM Comm
Branch
eng-7462 (branched from master)
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

lib/shared/community-utils.js
281

The change here is that we set threadID to the threadID hook param rather than verificationResponse.thread?.id

334

The changes here are because we already declared threadID in the upper scope as a hook param, so we needed a new variable name here

ginsu requested review of this revision.Jul 9 2024, 10:05 AM

Didn't review the code closely since it's marked as a move diff

I think it would be good to lift the state back into the component

lib/hooks/invite-links.js
187

Rather than having useJoinCommunity create the state and pass it back out, I think it would be more readable for this component to create the state and then pass it in

In general, it's best to have state "lifted" to the shared common ancestor

This revision is now accepted and ready to land.Jul 9 2024, 11:06 AM
lib/shared/community-utils.js
366–415 ↗(On Diff #42218)

Realized that I also should have this effect in useJoinCommunity. Updated the diff to address this

This revision was landed with ongoing or failed builds.Jul 11 2024, 9:01 PM
This revision was automatically updated to reflect the committed changes.