Page MenuHomePhabricator

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

Authored by inka on Jul 24 2024, 3:26 AM.
Tags
None
Referenced Files
F3390064: D12867.diff
Fri, Nov 29, 9:56 PM
Unknown Object (File)
Sun, Nov 10, 9:23 AM
Unknown Object (File)
Fri, Nov 1, 11:08 AM
Unknown Object (File)
Oct 24 2024, 4:56 AM
Unknown Object (File)
Oct 24 2024, 4:47 AM
Unknown Object (File)
Oct 19 2024, 12:32 PM
Unknown Object (File)
Oct 19 2024, 12:32 PM
Unknown Object (File)
Oct 19 2024, 12:32 PM
Subscribers

Details

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

image.png (2×2 px, 325 KB)

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

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!
native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp
2598–2604 ↗(On Diff #42725)

Should we perform these in a single transaction?

2606–2609 ↗(On Diff #42725)

Should we sort the messages? Results from both processMessagesResults and getRelatedMessagesForSearch are sorted, but their concatenation isn't.

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!
inka requested review of this revision.Jul 24 2024, 9:38 AM
native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp
2598–2604 ↗(On Diff #42725)

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 ↗(On Diff #42725)

This data is sorted on the js side in createChatMessageItems by sortMessageInfoList

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
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].