Page MenuHomePhabricator

set farcaster avatar and channel description in db during createOrUpdateFarcasterChannelTag
ClosedPublic

Authored by varun on Wed, Oct 23, 10:47 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Nov 10, 12:56 PM
Unknown Object (File)
Fri, Nov 8, 11:07 PM
Unknown Object (File)
Thu, Nov 7, 8:54 AM
Unknown Object (File)
Thu, Nov 7, 3:04 AM
Unknown Object (File)
Wed, Nov 6, 1:41 PM
Unknown Object (File)
Tue, Nov 5, 10:57 PM
Unknown Object (File)
Mon, Nov 4, 10:56 PM
Unknown Object (File)
Mon, Nov 4, 9:53 AM
Subscribers

Details

Summary

when a community is tagged with a farcaster channel, we want to display the farcaster channel avatar and description (without overwriting a pre-existing description or avatar)

Test Plan

Tested in conjunction with next diff

idnameavatardescription

Created community

84426Oct23NULLNULL

Tagged community with spam channel. NULL description was replaced with description from neynar. (also confirmed that avatar and description were updated on client)

84426Oct23{"type":"farcaster"}Dedicated channel for everything $SPAM

Removed tag. avatar was reset to NULL. description remained

84426Oct23NULLDedicated channel for everything $SPAM

Removed description from native app. It was set to an empty string

84426Oct23NULL

Added a new description from app.

84426Oct23NULLAsdf

Tagged community with a different channel. Avatar changed but description remained the same.

84426Oct23{"type":"farcaster"}Asdf

Removed tag

84426Oct23NULLAsdf

Removed description

84426Oct23NULL

Tagged community with a different channel. empty string description was replaced with description from neynar

84426Oct23{"type":"farcaster"}your musings, thoughts & dreams; welcome here

Removed tag and set a custom emoji avatar

84426Oct23{"type":"emoji","emoji":"?","color":"57697f"}your musings, thoughts & dreams; welcome here

Added and removed a new tag, avatar was not overwritten

84426Oct23{"type":"emoji","emoji":"?","color":"57697f"}your musings, thoughts & dreams; welcome here

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

varun published this revision for review.Wed, Oct 23, 11:01 AM

planning changes to rethink this a little

varun added inline comments.
keyserver/src/creators/farcaster-channel-tag-creator.js
114–125 ↗(On Diff #45351)

instead of doing this in the transaction, we should just do it in the background and use the updateThread function

use updateThread and modify the thread in the background. doing this lets us display robotext indicating what has changed

ashoat requested changes to this revision.Wed, Oct 23, 7:10 PM
ashoat added inline comments.
keyserver/src/creators/farcaster-channel-tag-creator.js
149–152 ↗(On Diff #45359)

I would do this

160–161 ↗(On Diff #45359)

Won't this crash if we fail to find request.commCommunityID? How about this

174 ↗(On Diff #45359)

We can't do it in the background because the result of updateThread should be returned to the client and handled in reducers

This revision now requires changes to proceed.Wed, Oct 23, 7:10 PM
keyserver/src/creators/farcaster-channel-tag-creator.js
174 ↗(On Diff #45359)

ah ok, yeah makes sense that we wouldn't rely on the state check

keyserver/src/creators/farcaster-channel-tag-creator.js
157–161 ↗(On Diff #45379)

i wonder if it would be confusing to the reader to just return request here, instead of all this dot notation

160–161 ↗(On Diff #45359)

did this a little differently to exit early

ashoat added inline comments.
keyserver/src/creators/farcaster-channel-tag-creator.js
157–161 ↗(On Diff #45379)

I don't feel strongly

lib/types/community-types.js
63–66 ↗(On Diff #45379)

Looks like this type is used on both client and server. On the client it'd be better to use a type where it's always set (since we currently update keyserver before client, it'll always be set)

Can you split into two separate types? When I've had to do this before, I usually prefix one type name with Client and the other with Server (open to other naming though)

lib/types/validators/farcaster-channel-tag-validators.js
25–26 ↗(On Diff #45379)

Isn't this only used on the client? Given we currently update keyserver before client, seems like it'll always be set

This revision is now accepted and ready to land.Thu, Oct 24, 2:47 PM
lib/types/community-types.js
63–66 ↗(On Diff #45379)

i can split the type, but it won't always be set on the client. on lines 157 and 164 of keyserver/src/creators/farcaster-channel-tag-creator.js above, we return early without updatesResult or newMessageInfos, so even new clients can get a response without this data

lib/types/validators/farcaster-channel-tag-validators.js
25–26 ↗(On Diff #45379)

as explained above, it's not always set

lib/types/community-types.js
63–66 ↗(On Diff #45379)

You're right... I missed the conditions on lines 157 and 164. Sorry about that!