Page MenuHomePhabricator

[lib] Fetch message to preview from MessagePreview
ClosedPublic

Authored by ashoat on Wed, Oct 30, 11:46 AM.
Tags
None
Referenced Files
F3335969: D13826.diff
Thu, Nov 21, 1:06 PM
Unknown Object (File)
Mon, Nov 18, 4:17 PM
Unknown Object (File)
Fri, Nov 15, 11:01 PM
Unknown Object (File)
Sun, Nov 10, 4:07 PM
Unknown Object (File)
Sun, Nov 10, 7:43 AM
Unknown Object (File)
Thu, Nov 7, 6:22 PM
Unknown Object (File)
Thu, Nov 7, 6:05 AM
Unknown Object (File)
Wed, Nov 6, 5:08 PM
Subscribers
None

Details

Summary

In a later diff, we'll need to potentially call an async function to fetch a message from the database in order to determine which message should be the MessagePreview.

The code in chat-selectors.js isn't async, and it would be rather complicated to convert it to be.

Instead, in this diff I move the logic for determining which message to preview to the MessagePreview components that actually render the message.

Depends on D13825

Test Plan

Tested in combination with the rest of the stack:

  1. Make sure membership operations (user joining / leaving) don't appear in MessagePreview
  2. Make sure reactions to the viewer's messages still appear in MessagePreview
  3. Make sure reactions to other user's messages don't appear in MessagePreview
  4. Test fresh login to thread with only one message in the last 14 days, which is a reaction to a non-viewer message. Make sure the reaction initially appears in MessagePreview, but then is replaced after more messages are fetched by the client

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

lib/hooks/message-hooks.js
17

This function is substantially similar to getMostRecentMessageInfo in chat-selectors.js, but has one difference which introduces a slight regression which is addressed in the next diff.

The difference is that we don't check isEmptyMediaMessage here. Since the check is specific to multimedia messages, I add it to multimedia-message-spec.js in the next diff.

lib/selectors/chat-selectors.js
106

Check mentioned above

tomek added inline comments.
lib/hooks/message-hooks.js
23

At this point this function doesn't need to be async, but I guess it changes later in the stack

This revision is now accepted and ready to land.Thu, Oct 31, 5:51 AM
lib/hooks/message-hooks.js
23

Yes, it changes in the next diff (D13827)