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)
Wed, Sep 4, 1:58 AM
Unknown Object (File)
Wed, Sep 4, 1:58 AM
Unknown Object (File)
Wed, Aug 28, 5:46 AM
Unknown Object (File)
Sun, Aug 25, 12:42 AM
Unknown Object (File)
Sat, Aug 24, 9:46 PM
Unknown Object (File)
Tue, Aug 20, 1:08 AM
Unknown Object (File)
Tue, Aug 20, 1:07 AM
Unknown Object (File)
Tue, Aug 20, 12:38 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
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