Page MenuHomePhabricator

[native][web] Fill target_message column for sidebar source
ClosedPublic

Authored by inka on Jul 24 2024, 7:51 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 1, 12:02 PM
Unknown Object (File)
Thu, Oct 17, 11:17 PM
Unknown Object (File)
Mon, Oct 14, 9:57 PM
Unknown Object (File)
Mon, Oct 14, 4:43 PM
Unknown Object (File)
Mon, Oct 14, 4:42 PM
Unknown Object (File)
Mon, Oct 14, 4:42 PM
Unknown Object (File)
Thu, Oct 10, 6:50 AM
Unknown Object (File)
Thu, Oct 10, 6:50 AM
Subscribers

Details

Summary

issue: ENG-8891

Test Plan

Tested that in a newly created db the target_message is filled in for sidebar source, by searching in a sidebar and getting a result for the first message of that siedbar
Tested that the migration succeeds and after it is run it is also possible to search for the first message in a sidebar

Diff Detail

Repository
rCOMM Comm
Branch
inka/search
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Jul 24 2024, 7:54 AM
Harbormaster failed remote builds in B30641: Diff 42737!
inka requested review of this revision.Jul 25 2024, 1:18 AM
tomek added inline comments.
native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp
227 ↗(On Diff #42771)

Can we use MessageType::SIDEBAR_SOURCE? By the way, MessageType should use explicit value expressions.

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

Use MessageType::SIDEBAR_SOURCE, update MessageType. Had to move MessageType to avoid importing folly on web

inka requested review of this revision.Jul 26 2024, 3:10 AM

Requesting review because I am not familiar with cpp and a lot of decisions had to be made

native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp
817 ↗(On Diff #42814)

My ide formats is like this

This revision is now accepted and ready to land.Jul 26 2024, 3:39 AM

Thanks for doing the MessageType extraction! I put up a diff for this (D12761) but ended up abandoning it... probably should have just left it there.

native/cpp/CommonCpp/NativeModules/PersistentStorageUtilities/MessageOperationsUtilities/MessageTypeEnum.h
1–28 ↗(On Diff #42830)

This file is rather messy.

  1. Can you add #pragma once to the top?
  2. Can you add line breaks before and after the namespace?
  3. Remove trailing line break
native/cpp/CommonCpp/NativeModules/PersistentStorageUtilities/MessageOperationsUtilities/MessageTypeEnum.h
1–28 ↗(On Diff #42830)

Created D12966 to address