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
F3373846: D12869.diff
Tue, Nov 26, 12:11 PM
Unknown Object (File)
Wed, Nov 13, 1:48 AM
Unknown Object (File)
Fri, Nov 1, 12:02 PM
Unknown Object (File)
Oct 17 2024, 11:17 PM
Unknown Object (File)
Oct 14 2024, 9:57 PM
Unknown Object (File)
Oct 14 2024, 4:43 PM
Unknown Object (File)
Oct 14 2024, 4:42 PM
Unknown Object (File)
Oct 14 2024, 4:42 PM
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
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, 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

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

Created D12966 to address