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)
Details
Details
Tested in conjunction with next diff
| id | name | avatar | description |
Created community
| 84426 | Oct23 | NULL | NULL |
Tagged community with spam channel. NULL description was replaced with description from neynar. (also confirmed that avatar and description were updated on client)
| 84426 | Oct23 | {"type":"farcaster"} | Dedicated channel for everything $SPAM |
Removed tag. avatar was reset to NULL. description remained
| 84426 | Oct23 | NULL | Dedicated channel for everything $SPAM |
Removed description from native app. It was set to an empty string
| 84426 | Oct23 | NULL | |
Added a new description from app.
| 84426 | Oct23 | NULL | Asdf |
Tagged community with a different channel. Avatar changed but description remained the same.
| 84426 | Oct23 | {"type":"farcaster"} | Asdf |
Removed tag
| 84426 | Oct23 | NULL | Asdf |
Removed description
| 84426 | Oct23 | NULL | |
Tagged community with a different channel. empty string description was replaced with description from neynar
| 84426 | Oct23 | {"type":"farcaster"} | your musings, thoughts & dreams; welcome here |
Removed tag and set a custom emoji avatar
| 84426 | Oct23 | {"type":"emoji","emoji":"?","color":"57697f"} | your musings, thoughts & dreams; welcome here |
Added and removed a new tag, avatar was not overwritten
| 84426 | Oct23 | {"type":"emoji","emoji":"?","color":"57697f"} | your musings, thoughts & dreams; welcome here |
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| 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 |
Comment Actions
use updateThread and modify the thread in the background. doing this lets us display robotext indicating what has changed
| 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 |
| 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 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 |
| 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! |