Page MenuHomePhabricator

[lib][native][web] Ignore showInMessagePreview if no messages found
ClosedPublic

Authored by ashoat on Oct 30 2024, 11:48 AM.
Tags
None
Referenced Files
F3405196: D13830.id.diff
Tue, Dec 3, 7:05 PM
Unknown Object (File)
Mon, Dec 2, 10:54 PM
Unknown Object (File)
Thu, Nov 28, 4:55 AM
Unknown Object (File)
Thu, Nov 28, 4:12 AM
Unknown Object (File)
Wed, Nov 27, 9:46 AM
Unknown Object (File)
Tue, Nov 26, 3:49 AM
Unknown Object (File)
Sun, Nov 24, 4:20 AM
Unknown Object (File)
Sat, Nov 23, 6:32 AM
Subscribers
None

Details

Summary

This diff does two things:

  1. Updates useGetMessageInfoForPreview so that it falls back to the old behavior if it fails to find a message to preview that passes showInMessagePreview. This allows us to avoid showing "No messages". This is "Potential solution 2" described on Linear here.
  2. Updates useGetMessageInfoForPreview to also return shouldFetchOlderMessages, a flag that indicates to the caller if the fallback described above occurred. This will allow us to implement "Potential solution 3" from the Linear comment linked above.
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
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

tomek added inline comments.
lib/hooks/message-hooks.js
79 ↗(On Diff #45480)

We could avoid iterating again by saving the first present message to a variable in the previous loop.

This revision is now accepted and ready to land.Oct 31 2024, 6:12 AM
lib/hooks/message-hooks.js
79 ↗(On Diff #45480)

Good suggestion – this actually significantly simplies the return here

Avoid unnecessary second pass

This revision was landed with ongoing or failed builds.Oct 31 2024, 11:38 AM
This revision was automatically updated to reflect the committed changes.