Page MenuHomePhabricator
Differential D5698

[lib] introduced messageContentForServerDB function in reaction message spec
ClosedPublic
Actions

Authored by ginsu on Nov 21 2022, 6:50 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Nov 28, 2:46 AM
Unknown Object (File)
Thu, Nov 28, 12:46 AM
Unknown Object (File)
Wed, Nov 27, 10:51 PM
Unknown Object (File)
Mon, Nov 25, 3:53 AM
Unknown Object (File)
Mon, Nov 25, 3:53 AM
Unknown Object (File)
Wed, Nov 13, 7:36 PM
Unknown Object (File)
Mon, Nov 4, 8:20 AM
Unknown Object (File)
Mon, Nov 4, 8:20 AM
View All Files
Subscribers
ashoat
atul
tomek

Details

Reviewers
atul
rohan
tomek
Commits
rCOMM32145d947e07: [lib] introduced messageContentForServerDB function in reaction message spec
Summary

introduced messageContentForServerDB function in reaction message spec. messageContentForServerDB returns the targetMessageID and the reaction as a stringified JSON object. The contents of this function are actually the same as messageContentForClientDB, so messageContentForClientDB can actually just call and return messageContentForServerDB for it's method now.

UPDATE

Based on some feedback I got on some subsequent diffs, I have removed targetMessageID from the messageContent in messageContentForServerDB. The big change with this update is that we should now be pulling targetMessageID directly from the server row instead of content. We are now separating the two messageContent functions since rawMessageInfoFromClientDB still needs targetMessageID from the client and ClientDBMessageInfo doesn't have a target_message_id field

Depends on D5690
Linear Task: ENG-2245

Test Plan

Ran web and mobile app locally and nothing crashes/will be doing more tests on this message spec in subsequent diffs

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

ginsu created this revision.Nov 21 2022, 6:50 AM
Herald added subscribers: atul, tomek, ashoat. · View Herald TranscriptNov 21 2022, 6:50 AM
ginsu edited the summary of this revision. (Show Details)Nov 21 2022, 6:55 AM
ginsu added reviewers: atul, rohan.
ginsu added a parent revision: D5690: [lib] introduced reaction message spec boilerplate.
Harbormaster completed remote builds in B13633: Diff 18680.Nov 21 2022, 7:05 AM
ginsu requested review of this revision.Nov 21 2022, 7:05 AM
rohan accepted this revision.Nov 21 2022, 3:43 PM

The explanation for calling messageContentForServerDB makes sense to me

atul accepted this revision.Nov 21 2022, 3:54 PM
This revision is now accepted and ready to land.Nov 21 2022, 3:54 PM
ginsu updated this revision to Diff 19213.Dec 7 2022, 6:49 AM

rebase

ginsu mentioned this in D5803: [lib] introduced rawMessageInfoFromServerDBRow function to reaction message spec.Dec 7 2022, 6:52 AM
ginsu added inline comments.Dec 7 2022, 6:58 AM
lib/shared/messages/reaction-message-spec.js
26–28 ↗(On Diff #19213)

@tomek this is where I insert targetMessageID into the message content. Again, let me know if we think this is unnecessary, and I can make the respective changes

Harbormaster completed remote builds in B14038: Diff 19213.Dec 7 2022, 7:02 AM
ginsu updated this revision to Diff 19239.Dec 7 2022, 1:35 PM

removed targetMessageID from message content based on some feedback from tomek

ginsu requested review of this revision.Dec 7 2022, 1:37 PM

rerequesting review, because made some bigger changes with having separate messageContent functions for the client and server

ginsu edited the summary of this revision. (Show Details)Dec 7 2022, 1:41 PM
ginsu added a reviewer: tomek.
Harbormaster completed remote builds in B14052: Diff 19239.Dec 7 2022, 1:47 PM
tomek accepted this revision.Dec 8 2022, 4:49 AM
This revision is now accepted and ready to land.Dec 8 2022, 4:49 AM
ginsu updated this revision to Diff 19405.Dec 15 2022, 11:28 AM

rebase before landing

Harbormaster completed remote builds in B14184: Diff 19405.Dec 15 2022, 11:43 AM
Closed by commit rCOMM32145d947e07: [lib] introduced messageContentForServerDB function in reaction message spec (authored by ginsu). · Explain WhyDec 15 2022, 11:51 AM
This revision was automatically updated to reflect the committed changes.
ginsu added a commit: rCOMM32145d947e07: [lib] introduced messageContentForServerDB function in reaction message spec.

Revision Contents
Changeset List

PathSize
lib/
shared/
messages/
9 lines

Diff 19239

View Options

lib/shared/messages/reaction-message-spec.js

Loading...