Page MenuHomePhabricator

[lib] introduced reaction message type
ClosedPublic

Authored by ginsu on Nov 16 2022, 11:37 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Dec 25, 9:32 AM
Unknown Object (File)
Nov 15 2024, 2:30 PM
Unknown Object (File)
Nov 9 2024, 11:50 AM
Unknown Object (File)
Nov 9 2024, 11:50 AM
Unknown Object (File)
Nov 9 2024, 11:49 AM
Unknown Object (File)
Nov 8 2024, 9:05 PM
Unknown Object (File)
Nov 5 2024, 2:08 AM
Unknown Object (File)
Oct 29 2024, 1:44 PM

Details

Summary

Introduced reaction message type. Most of the fields for ReactionMessageData like type threadID, creatorID, and timeare taken from other message types and should be pretty clear what each do. type is the type of message defined in messageTypes object. threadID is the id of the thread the reaction message lives in, creatorID is the id of whoever created the reaction message, and time is the time the reaction message was created.

Two new fields have been introduced: targetMessageID and reaction. targetMessageID is the id of the message that is being reacted to, and reaction is the string of the emoji that is the content of the reaction. reaction can also be null and I was thinking that this could mean that the user has unliked/unreacted to a message.

One thing to note is that in the notion implementation details, there was a mention of having a localID; however, after further discussion, we have decided that for now, we do not want any retry behavior if the user is disconnected so localID is not necessary


Linear Task: ENG-2244

Test Plan

Flow, also will be further tested in subsequent diffs

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Nov 16 2022, 11:40 AM
Harbormaster failed remote builds in B13491: Diff 18498!
ginsu added reviewers: atul, rohan.

fixed keyserver flow issues

ginsu requested review of this revision.Nov 16 2022, 1:06 PM

Looks reasonable, adding @ashoat as blocking reviewer to sign off on design

lib/types/message-types.js
229 ↗(On Diff #18499)

We can also do ?string here, but maybe enumerating null explicitly makes it more clear that setting reaction to null is for unsetting reaction?

This revision is now accepted and ready to land.Nov 17 2022, 9:02 AM
This revision now requires review to proceed.Nov 17 2022, 9:02 AM

I don't have a great memory around what needs to be updated in relation to this, but I think this is good

lib/types/message-types.js
229 ↗(On Diff #18499)

The main difference is that ?string allows undefined while string | null does not

This revision is now accepted and ready to land.Nov 17 2022, 9:54 AM
lib/types/message-types.js
229 ↗(On Diff #18499)

Yea I want to make this very clear that it can only be string sets the reaction and null unsets it

This revision was automatically updated to reflect the committed changes.