Page MenuHomePhabricator

[lib] introduced messageContentForServerDB function in reaction message spec
ClosedPublic

Authored by ginsu on Nov 21 2022, 6:50 AM.
Tags
None
Referenced Files
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
Unknown Object (File)
Mon, Nov 4, 8:20 AM
Unknown Object (File)
Mon, Nov 4, 8:20 AM
Unknown Object (File)
Oct 28 2024, 1:00 AM
Subscribers

Details

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
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

ginsu requested review of this revision.Nov 21 2022, 7:05 AM

The explanation for calling messageContentForServerDB makes sense to me

This revision is now accepted and ready to land.Nov 21 2022, 3:54 PM
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

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 added a reviewer: tomek.
This revision is now accepted and ready to land.Dec 8 2022, 4:49 AM