Page MenuHomePhabricator

[lib] modify rawThreadInfoFromServerThreadInfo to return default avatar to old clients
ClosedPublic

Authored by varun on Oct 17 2024, 10:13 PM.
Tags
None
Referenced Files
F3337885: D13749.diff
Thu, Nov 21, 5:22 PM
Unknown Object (File)
Wed, Nov 20, 8:17 AM
Unknown Object (File)
Wed, Nov 20, 8:16 AM
Unknown Object (File)
Wed, Nov 20, 8:16 AM
Unknown Object (File)
Sun, Nov 10, 11:41 AM
Unknown Object (File)
Fri, Nov 8, 5:39 PM
Unknown Object (File)
Fri, Nov 8, 5:01 AM
Unknown Object (File)
Sat, Nov 2, 2:04 PM
Subscribers

Details

Summary

this change means that when fetchThreadInfos is called with an old client as the viewer, the raw thread info will contain a null avatar

the min native and web versions were set to the same versions as fetchKnownUserInfos

Depends on D13746

Test Plan

fetched threads from old native client and got back default avatar for farcaster avatar threads

Diff Detail

Repository
rCOMM Comm
Branch
farcaster-thread-avatars (branched from master)
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

varun held this revision as a draft.
varun published this revision for review.Oct 18 2024, 12:59 PM
ashoat requested changes to this revision.Oct 20 2024, 11:43 AM
ashoat added inline comments.
keyserver/src/fetchers/thread-fetchers.js
315–316

the min native and web versions were set to the same versions as fetchKnownUserInfos

Wait, is this right? I'm a bit confused... won't clients need the changes in the preceding diffs in the stack in order to be able to render thread avatars set to { type: 'farcaster' }?

If not, can you:

  1. Explain what makes the referenced fetchKnownUserInfos versions important (code link would be helpful)
  2. Explain what clients with version 405 and above (but below the next code version) will see

(My instinct is that you want NEXT_CODE_VERSION here)

lib/shared/thread-utils.js
907–911

I think this condition would be more clear if inverted

This revision now requires changes to proceed.Oct 20 2024, 11:43 AM
keyserver/src/fetchers/thread-fetchers.js
315–316
  1. In fetchKnownUserInfos, we check the client code version to determine whether the client is aware of the farcaster avatar type.

https://github.com/CommE2E/comm/blob/4fd312324232d629655075a693683e9a03b38be6/keyserver/src/fetchers/user-fetchers.js#L182

  1. Versions above 405 but below the next code version will have either the before or after of this landed commit, or they will have the changes from this diff stack which include support for farcaster thread avatars

in the case of the former, the defaultAnonymousUserEmojiAvatar will be returned by the hook. but i'm realizing it would be better to use NEXT_CODE_VERSION here to display a default thread avatar instead... will update this diff

Versions above 405 but below the next code version will have either the before or after of this landed commit, or they will have the changes from this diff stack which include support for farcaster thread avatars

Hard to follow what you're trying to say here, but since you're using NEXT_CODE_VERSION now I'll accept

This revision is now accepted and ready to land.Oct 22 2024, 1:32 PM