Page MenuHomePhabricator

[keyserver] create message and sidebar based on cast
Needs RevisionPublic

Authored by will on Wed, Oct 23, 10:38 PM.
Tags
None
Referenced Files
F3061303: D13782.id45364.diff
Thu, Oct 24, 12:29 PM
F3061248: D13782.diff
Thu, Oct 24, 12:21 PM
F3059047: D13782.diff
Thu, Oct 24, 3:18 AM
F3058947: D13782.diff
Thu, Oct 24, 2:32 AM
F3058045: D13782.id45364.diff
Wed, Oct 23, 10:57 PM
F3058042: D13782.id.diff
Wed, Oct 23, 10:57 PM
F3058039: D13782.diff
Wed, Oct 23, 10:57 PM
Subscribers

Details

Reviewers
ashoat
varun
Summary

This creates a message based on a cast and converts it to a sidebar. If there is a parent cast included in the webhook event, we use the parent cast, if not, we use the tagger's cast.

Depends D13775

Test Plan

Triggered the creation of a new community with a webhook POST request and confirmed sidebar was correctly created in the community

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

will requested review of this revision.Wed, Oct 23, 10:55 PM
keyserver/src/responders/farcaster-webhook-responders.js
246

Placeholder for now

ashoat requested changes to this revision.Thu, Oct 24, 6:20 AM

Can you a share a screenshot of an example message/sidebar that is created this way?

keyserver/src/responders/farcaster-webhook-responders.js
44–47
  1. Can we move this logic to the caller, and avoid passing both params in here?
  2. Can you add a code comment explaining the logic here?
  3. Seems like it would cleaner like this (see code change). Note that this is slightly different in the case where parentHash is an empty string (I think ?? only considers null / undefined)... not sure if this matters here, but if it does you could do parentHash ? parentHash : castHash instead
57

Not sure the newline makes sense here

58

Can we put quotes around the cast text? Might want to consider "escaping" quotes inside of the text, although not sure how important that is

76
  1. Is the condition necessary here?
  2. Why'd you choose to diverge from the approach we use for sidebar titles in the rest of the codebase?
This revision now requires changes to proceed.Thu, Oct 24, 6:20 AM