Page MenuHomePhabricator

[web][native] Fix message search showing toggle pin robotext
ClosedPublic

Authored by inka on Sep 11 2023, 6:25 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Jul 19, 7:41 PM
Unknown Object (File)
Fri, Jul 19, 11:00 AM
Unknown Object (File)
Tue, Jul 16, 6:57 AM
Unknown Object (File)
Thu, Jul 11, 2:26 AM
Unknown Object (File)
Tue, Jul 9, 6:47 AM
Unknown Object (File)
Mon, Jul 8, 12:31 AM
Unknown Object (File)
Sun, Jul 7, 11:28 PM
Unknown Object (File)
Sat, Jul 6, 12:22 PM
Subscribers

Details

Summary

issue: https://linear.app/comm/issue/ENG-4910/message-search-show-unpin-messages
This diff fixes toggle pin robotexts showing up in search results

Test Plan

checked that the robotext does not show up in search results, but the messages do.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Is there a way we can implement this fix in a way that doesn't require copy-paste between the search and pinned messages views? Besides DRY, I'm worried that a future author of a new, similar interface won't expect that they need to filter these message types out. I'd love if there was some shared utility function that could be used

inka requested review of this revision.Sep 11 2023, 6:43 AM

Changes look good, thanks for fixing this!

Regarding @ashoat's comment, we could probably just make a utility function in lib called visibleMessagesInResultsModal or something and do the whole filtering logic, but I think we filter off different conditions between search and pinned modals. Even so, it all being in one function would make it easier to make future changes even if we need to use conditionals and filter differently based on the modal. I can try to handle it in a separate diff if it's time consuming

native/search/message-search.react.js
111 ↗(On Diff #30923)

Same comment as below

web/modals/search/message-search-utils.react.js
46 ↗(On Diff #30923)

Does isComposableMessageType work here (just to keep it consistent with pinned messages)?

This revision is now accepted and ready to land.Sep 11 2023, 7:01 AM

Extract common code to lib

inka requested review of this revision.Sep 11 2023, 7:27 AM

Requesting review, because I ended up extracting a lot of common code to lib. This is code that takes messages returned from messageListData and filters them to just include the messages we initaily fetched from the search endpoint

web/modals/search/message-search-utils.react.js
46 ↗(On Diff #30923)

Yes! will update!

Use isComposableMessageType

I removed copy paste between search on web and native. I will create a followup task to use this function in pinned messages, because it seems to me that this logic wan be used there as well.

This revision is now accepted and ready to land.Sep 13 2023, 8:48 AM