[native][web] Fetch related messages for search
ClosedPublic
Actions

Authored by inka on Jul 24 2024, 3:26 AM.

Details

Reviewers

tomek
kamil

Commits

rCOMM86b47afb4189: [native][web] Fetch related messages for search

Summary

issue: ENG-8783

Test Plan

Tested by commenting out isThreadThin check, and commenting out extractKeyserverIDFromIDOptional(messageInfo.threadID) check in getMessageSearchStoreOps, and setting threadMessageInfos in createChatMessageItems to empty array to make sure messages from redux are not being used.. This allowed me to treat a thin thread as if it were thick, and execute local search on it.

Tested that edits and reactions show up in search results

Diff Detail

Repository

rCOMM Comm

Branch

inka/search

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

inka created this revision.Jul 24 2024, 3:26 AM

Herald added a subscriber: ashoat. · View Herald TranscriptJul 24 2024, 3:26 AM

inka edited the test plan for this revision. (Show Details)Jul 24 2024, 3:27 AM

inka added a parent revision: D12866: [native][web] Fix timestamp type in cpp.

Harbormaster returned this revision to the author for changes because remote builds failed.Jul 24 2024, 3:28 AM

Harbormaster failed remote builds in B30629: Diff 42725!

tomek added inline comments.Jul 24 2024, 5:54 AM

native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp
2598–2604	Should we perform these in a single transaction?
2606–2609	Should we sort the messages? Results from both `processMessagesResults` and `getRelatedMessagesForSearch` are sorted, but their concatenation isn't.

inka added a child revision: D12869: [native][web] Fill target_message column for sidebar source.Jul 24 2024, 7:51 AM

Update tests

Harbormaster returned this revision to the author for changes because remote builds failed.Jul 24 2024, 8:03 AM

Harbormaster failed remote builds in B30642: Diff 42738!

Harbormaster completed remote builds in B30642: Diff 42738.Jul 24 2024, 9:38 AM

inka requested review of this revision.Jul 24 2024, 9:38 AM

inka added inline comments.Jul 25 2024, 1:29 AM

native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp
2598–2604	We talked about this with Tomek and Kamil and it doesn't have to because we access the db from one thread and synchronously, on both web and native. So there is no threat of some insert being executed between those two selects. And two selects don't need to be run in one transaction.
2606–2609	This data is sorted on the js side in `createChatMessageItems` by `sortMessageInfoList`

Rebase

Harbormaster completed remote builds in B30666: Diff 42770.Jul 25 2024, 5:17 AM

tomek accepted this revision.Jul 25 2024, 6:08 AM

tomek added inline comments.

native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp
2631 ↗	(On Diff #42770)	Why do we have `i + 1` here?

This revision is now accepted and ready to land.Jul 25 2024, 6:08 AM

inka added inline comments.Jul 26 2024, 2:23 AM

native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp
2631 ↗	(On Diff #42770)	This was copied from your code in `getAllEntitiesByPrimaryKeys`. This is correct because i starts from `0` and `bindStringToSQL` indexes start from 1. We don't start the loop from `1` because then we would have to do `messageIDs[i-1]`.