Page MenuHomePhabricator

[lib] dispatch actions and populate aux user store with fids
AcceptedPublic

Authored by will on Sun, May 12, 8:10 PM.
Tags
None
Referenced Files
F1880355: D12011.id40104.diff
Mon, May 27, 9:40 PM
F1879271: D12011.id40351.diff
Mon, May 27, 7:00 PM
F1877186: D12011.id40351.diff
Mon, May 27, 2:05 PM
Unknown Object (File)
Thu, May 23, 11:08 AM
Unknown Object (File)
Wed, May 22, 9:25 AM
Unknown Object (File)
Tue, May 21, 7:41 PM
Unknown Object (File)
Mon, May 20, 1:04 PM
Unknown Object (File)
Mon, May 20, 1:56 AM
Subscribers

Details

Reviewers
ginsu
ashoat
Summary

Depends on D12105

Test Plan

Hardcoded userIDs from staging as the user store and confirmed aux user store was populated with fids on connecting farcaster account

Diffed up first without a full end to end test. Will not land until I verify that the aux user store is properly populated with fids for users in the User Store

Diff Detail

Repository
rCOMM Comm
Branch
farcaster/set_aux_user_fids
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

will requested review of this revision.Sun, May 12, 8:27 PM
ashoat requested changes to this revision.Mon, May 13, 3:31 PM
ashoat added inline comments.
lib/components/farcaster-data-handler.react.js
110–133 ↗(On Diff #40104)

It doesn't seem like this logic is connected at all to the block you put it in. This makes the code more confusing, and it forces your logic to wait on the other logic to complete before being able to proceed.

I think we should factor it out. What if we had a single effect that called two React.useCallbacks, each of which was responsible for one of these blocks of functionality? We should also probably move some of the checks on lines from 70 to 80 to the callbacks

116–122 ↗(On Diff #40104)

Shorthand

124–133 ↗(On Diff #40104)

Doing two dispatches in a row like this is an anti-pattern... do you think we could introduce a replaceAuxUserFIDsActionType?

This revision now requires changes to proceed.Mon, May 13, 3:31 PM
lib/components/farcaster-data-handler.react.js
124–133 ↗(On Diff #40104)

I agree with these suggestions. Will include a new action type in a separate diff and separate logic into two React.useCallbacks on next rebase

will edited the summary of this revision. (Show Details)
will marked an inline comment as done.
lib/components/farcaster-data-handler.react.js
129–134

I introduced a previous diff where setAuxUserFIDs now clears fids prior to updating with new fids from the payload. The previous functionality of setAuxUserFIDs of just adding without clearing is now a new action type addAuxUserFIDs

There are now two callbacks which are called by a single useEffect. The two callbacks are called handleFarcasterMutuals and handleUserStoreFIDs

ashoat added inline comments.
lib/components/farcaster-data-handler.react.js
142–145
  1. Line 145 isn't doing anything
  2. Since we aren't waiting on these anyways, we can simplify this a little bit
This revision is now accepted and ready to land.Mon, May 20, 6:10 AM