Page MenuHomePhabricator

[lib/web] introduce handleCurrentUserFID to FarcasterDataHandler
ClosedPublic

Authored by ginsu on Jun 24 2024, 11:41 PM.
Tags
None
Referenced Files
F2833642: D12569.id42132.diff
Sat, Sep 28, 8:14 AM
F2833192: D12569.id42165.diff
Sat, Sep 28, 7:09 AM
F2832806: D12569.id42096.diff
Sat, Sep 28, 6:13 AM
F2831593: D12569.id42131.diff
Sat, Sep 28, 12:41 AM
Unknown Object (File)
Fri, Sep 27, 9:24 AM
Unknown Object (File)
Wed, Sep 25, 3:31 PM
Unknown Object (File)
Tue, Sep 24, 11:59 PM
Unknown Object (File)
Sat, Sep 21, 4:15 PM
Subscribers

Details

Summary

At the moment, the data in the synced metadata table store is not properly synced + shared across different clients. This has two consequences:

  1. the web client will not have an fid.
  2. If the user logs out on native after connecting to farcaster their fid will be lost because the sqlite client db will get cleared

We have an identity RPC called findUserIdentities where we can pass in the current user id and check if they have a farcaster id in the identity service and store. If the user has an fid persisted in the identity service we can grab it from there and also store it in the synced metadata table store through handleCurrentUserFID. A subsequent diff will handle checking if the user fid is still valid and if it isn't handle unlinking/removing the fid

Depends on D12683

Test Plan

Logged out the value of currentUserFID from useCurrentUserFID and confirmed that the web client was getting and returning the correct fid

Also logged out and then back into the same account that was previously connected to farcaster and confirmed that I was getting the expected/correct fid

Also confirmed that this effect only runs once per app start / foreground event

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

When do we want handleCurrentUserFID to run?

lib/components/farcaster-data-handler.react.js
153–161 ↗(On Diff #41677)

This is run regardless of if there was any change or not. The reducer seems to laso not check if there was any change and return a new reference every time this action. is dispatched. This will cause re-renders. Do we want that to happen every time?

ashoat requested changes to this revision.Jul 1 2024, 12:10 PM

Passing back to @ginsu for @inka

This revision now requires changes to proceed.Jul 1 2024, 12:10 PM
ginsu edited the test plan for this revision. (Show Details)

When do we want handleCurrentUserFID to run?

This effect should run whenever the user does not have their fid in the synced metadata table store BUT the user has an fid in the identity service. This is caused by two scenarios

  1. The user is using a different client (ie. web client)
  2. The user logs out and triggers a sqlite client db deletion

All of this is happening because the synced metadata table store is supposed to sync + share this data to other client using the backup service and tunnelbroker but those are not ready atm.

lib/components/farcaster-data-handler.react.js
153–161 ↗(On Diff #41677)

I am a little confused by this inline comment, but, I updated the summary to be more clear + responded to your comment above which I am hoping will implicitly answer this question too. However, lmk if that is not the case, and I will try and do a better job explaining/answering

inka added inline comments.
lib/components/farcaster-data-handler.react.js
153–161 ↗(On Diff #41677)

Sorry, I must have missed that the if checks if fid is present. Your comment above explained everything

If the network issue gets triggered by the keyserver connection status cycling, then we'll want to add some condition to eg. make sure it only runs once per app start / foreground event. If that's the case, please re-request review. Otherwise it looks good to land!

lib/components/farcaster-data-handler.react.js
150–161

Does this get run again when the client disconnects with the keyserver socket? Can you test this by killing the keyserver while the app is open/connected and seeing if it triggers a network call?

This revision is now accepted and ready to land.Jul 8 2024, 12:32 PM

fix network call getting triggered when client disconnect with the keyserver socket

ginsu requested review of this revision.Jul 8 2024, 2:41 PM

If the network issue gets triggered by the keyserver connection status cycling, then we'll want to add some condition to eg. make sure it only runs once per app start / foreground event. If that's the case, please re-request review. Otherwise it looks good to land!

This was happening, so re-requesting review with new condition if we should query or not

Both of my pieces of feedback below are also mentioned in D12685, so feel free to just address them there if it's easier

lib/components/farcaster-data-handler.react.js
142 ↗(On Diff #42132)

Can this be moved outside of the useCallback?

150–154 ↗(On Diff #42132)

It seems like three of these conditions could be replaced with a canQuery check

This revision is now accepted and ready to land.Jul 9 2024, 10:30 AM