Page MenuHomePhabricator

[lib] add localID field to reaction message type
ClosedPublic

Authored by ginsu on Dec 30 2022, 3:38 PM.
Tags
None
Referenced Files
F2164750: D6117.id20524.diff
Tue, Jul 2, 12:28 AM
Unknown Object (File)
Mon, Jul 1, 9:21 AM
Unknown Object (File)
Mon, Jul 1, 9:21 AM
Unknown Object (File)
Mon, Jul 1, 9:21 AM
Unknown Object (File)
Mon, Jul 1, 9:21 AM
Unknown Object (File)
Mon, Jul 1, 9:13 AM
Unknown Object (File)
Sat, Jun 29, 5:59 PM
Unknown Object (File)
Mon, Jun 24, 12:22 PM
Subscribers

Details

Summary

add localID field to reaction message type. We are going to need this to create local reaction messages which we will display optimistically


Linear Task: ENG-2519

Test Plan

flow and will be testing more in subsequent diffs

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

ginsu edited the summary of this revision. (Show Details)
ginsu requested review of this revision.Dec 30 2022, 3:51 PM

I wonder if this should be LocallyComposedMessageInfo. Looks like it's only use for CREATE_LOCAL_MESSAGE. Should we use that for reactions at all?

tomek added a reviewer: ashoat.

It sounds a little strange to say that reaction message is composable, but it shares a lot of properties with other composable message types, so I think it makes sense to include it in this set.

ashoat requested changes to this revision.Jan 3 2023, 9:14 AM

Requesting changes for an answer to my question above

This revision now requires changes to proceed.Jan 3 2023, 9:14 AM
ginsu edited the summary of this revision. (Show Details)

add Reaction Message to LocallyComposedMessageInfo type

Are we gonna use CREATE_LOCAL_MESSAGE for reactions? Guessing no, but I guess it didn't hurt to update LocallyComposedMessageInfo anyways

This revision is now accepted and ready to land.Jan 3 2023, 10:48 AM

Are we gonna use CREATE_LOCAL_MESSAGE for reactions? Guessing no, but I guess it didn't hurt to update LocallyComposedMessageInfo anyways

That is correct, we won't but since they share many similar properties, I figured adding it made sense