Page MenuHomePhabricator
Differential D6117

[lib] add localID field to reaction message type
ClosedPublic
Actions

Authored by ginsu on Dec 30 2022, 3:38 PM.
Tags
None
Referenced Files
F3349991: D6117.diff
Fri, Nov 22, 8:08 PM
Unknown Object (File)
Sun, Nov 10, 8:02 AM
Unknown Object (File)
Wed, Nov 6, 6:32 AM
Unknown Object (File)
Tue, Nov 5, 10:01 PM
Unknown Object (File)
Mon, Nov 4, 4:06 PM
Unknown Object (File)
Mon, Oct 28, 8:21 AM
Unknown Object (File)
Oct 4 2024, 8:33 AM
Unknown Object (File)
Sep 28 2024, 11:00 AM
View All Files
Subscribers
ashoat
atul
tomek

Details

Reviewers
atul
tomek
ashoat
Commits
rCOMM3de73d0da05a: [lib] add localID field to reaction message type
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
No Lint Coverage
Unit
No Test Coverage

Event Timeline

ginsu created this revision.Dec 30 2022, 3:38 PM
Herald added subscribers: atul, tomek, ashoat. · View Herald TranscriptDec 30 2022, 3:38 PM
ginsu added reviewers: atul, tomek.Dec 30 2022, 3:39 PM
ginsu edited the summary of this revision. (Show Details)
ginsu edited the summary of this revision. (Show Details)Dec 30 2022, 3:42 PM
ginsu added a child revision: D6118: [keyserver/lib] introuce localID field to sendReactionMessage action and reactionMessageCreationResponder.
Harbormaster completed remote builds in B14986: Diff 20455.Dec 30 2022, 3:51 PM
ginsu requested review of this revision.Dec 30 2022, 3:51 PM
ashoat added a comment.Dec 30 2022, 9:01 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 accepted this revision.Jan 2 2023, 5:02 AM
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 updated this revision to Diff 20524.Jan 3 2023, 9:15 AM
ginsu edited the summary of this revision. (Show Details)

add Reaction Message to LocallyComposedMessageInfo type

Harbormaster completed remote builds in B15044: Diff 20524.Jan 3 2023, 9:29 AM
ashoat accepted this revision.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

This revision is now accepted and ready to land.Jan 3 2023, 10:48 AM
ginsu added a comment.Jan 3 2023, 12:15 PM

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

ginsu updated this revision to Diff 20712.Jan 9 2023, 9:38 AM

rebase before landing

Harbormaster completed remote builds in B15197: Diff 20712.Jan 9 2023, 9:53 AM
Closed by commit rCOMM3de73d0da05a: [lib] add localID field to reaction message type (authored by ginsu). · Explain WhyJan 9 2023, 10:04 AM
This revision was automatically updated to reflect the committed changes.
ginsu added a commit: rCOMM3de73d0da05a: [lib] add localID field to reaction message type.

Revision Contents
Changeset List

PathSize
lib/
shared/
9 lines
types/
10 lines
messages/
6 lines

Diff 20712

View Options

lib/shared/message-utils.js

Loading...
View Options

lib/types/message-types.js

Loading...
View Options

lib/types/messages/reaction.js

Loading...