Page MenuHomePhabricator
Differential D11651

[lib] Include `[creator/target]FID` in `messageContentForServerDB` when update relationship op is `farcaster_mutual`
ClosedPublic
Actions

Authored by atul on Fri, Apr 12, 12:21 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Apr 27, 12:07 PM
Unknown Object (File)
Fri, Apr 26, 5:59 PM
Unknown Object (File)
Sun, Apr 21, 5:51 AM
Unknown Object (File)
Thu, Apr 18, 8:33 PM
Unknown Object (File)
Thu, Apr 18, 3:11 PM
Unknown Object (File)
Wed, Apr 17, 8:18 AM
Unknown Object (File)
Wed, Apr 17, 8:17 AM
Unknown Object (File)
Wed, Apr 17, 8:17 AM
View All 9 Files
Subscribers
ashoat

Details

Reviewers
ginsu
tomek
varun
will
Commits
rCOMM06f7b1904d97: [lib] Include `[creator/target]FID` in `messageContentForServerDB` when update…
Summary

Pretty self explanatory, we want to include these in content column so we can reconstruct rawMessageInfo from both client and serverDB.

Depends on D11650

Test Plan

Will create UPDATE_RELATIONSHIP message with operation = 'farcaster_mutual' and observe that row in messages table is correctly constructed before landing.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

atul created this revision.Fri, Apr 12, 12:21 PM
Herald added a subscriber: ashoat. · View Herald TranscriptFri, Apr 12, 12:21 PM
atul published this revision for review.Fri, Apr 12, 12:26 PM
atul edited the test plan for this revision. (Show Details)
Harbormaster completed remote builds in B28197: Diff 39087.Fri, Apr 12, 12:37 PM
atul added a child revision: D11652: [lib] Update `UPDATE_RELATIONSHIP.rawMessageInfoFromServerDB` to handle `farcaster_mutual`.Fri, Apr 12, 12:52 PM
tomek accepted this revision.Mon, Apr 15, 1:35 AM
This revision is now accepted and ready to land.Mon, Apr 15, 1:35 AM
tomek added inline comments.Mon, Apr 15, 1:37 AM
lib/shared/messages/update-relationship-message-spec.js
66 ↗(On Diff #39087)

Do we have to check this condition?

atul edited the summary of this revision. (Show Details)Tue, Apr 16, 12:22 PM
atul added a parent revision: D11650: [lib] Split `UpdateRelationshipMessageInfo` into `Update[Traditional/Farcaster]RelationshipMessageInfo`.
atul removed a parent revision: D11642: [lib] Split `rawUpdateRelationshipMessageInfoValidator` into `rawUpdate[Traditional/Farcaster]RelationshipMessageInfoValidator`.
atul added inline comments.Thu, Apr 18, 11:50 AM
lib/shared/messages/update-relationship-message-spec.js
66 ↗(On Diff #39087)

Yeah, the reason we need to check this is because updateRelationshipMessageSpec is used for both LEGACY_UPDATE_RELATIONSHIP and UPDATE_RELATIONSHIP messages.

atul added inline comments.Thu, Apr 18, 12:37 PM
lib/shared/messages/update-relationship-message-spec.js
66 ↗(On Diff #39087)

JK, I think what you're saying is that data.operation === 'farcaster_mutual' would be a sufficient check since that implicitly tells us that the message type is UPDATE_RELATIONSHIP so having both is a bit redundant.

Personally prefer including the first clause since it makes things more clear IMO, but can remove if others feel otherwise

Closed by commit rCOMM06f7b1904d97: [lib] Include `[creator/target]FID` in `messageContentForServerDB` when update… (authored by atul). · Explain WhyThu, Apr 18, 12:38 PM
This revision was automatically updated to reflect the committed changes.
atul added a commit: rCOMM06f7b1904d97: [lib] Include `[creator/target]FID` in `messageContentForServerDB` when update….
tomek added inline comments.Fri, Apr 19, 3:04 AM
lib/shared/messages/update-relationship-message-spec.js
66 ↗(On Diff #39087)

Ok, we can keep both conditions.

Revision Contents
Changeset List

PathSize
lib/
shared/
messages/
11 lines

Diff 39238

View Options

lib/shared/messages/update-relationship-message-spec.js

Loading...