Page MenuHomePhabricator

[lib] introduce useJoinCommunity
ClosedPublic

Authored by ginsu on Tue, Jul 9, 9:49 AM.
Tags
None
Referenced Files
F2322388: D12706.id42246.diff
Tue, Jul 23, 8:28 AM
F2322025: D12706.id42244.diff
Tue, Jul 23, 7:23 AM
F2319295: D12706.id42246.diff
Mon, Jul 22, 7:21 PM
Unknown Object (File)
Sun, Jul 21, 6:16 PM
Unknown Object (File)
Sat, Jul 20, 12:21 PM
Unknown Object (File)
Fri, Jul 19, 5:03 PM
Unknown Object (File)
Fri, Jul 19, 11:20 AM
Unknown Object (File)
Wed, Jul 17, 10:35 PM
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
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

lib/shared/community-utils.js
281 ↗(On Diff #42147)

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

334 ↗(On Diff #42147)

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.Tue, Jul 9, 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 ↗(On Diff #42147)

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.Tue, Jul 9, 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.Thu, Jul 11, 9:01 PM
This revision was automatically updated to reflect the committed changes.