Page MenuHomePhabricator

Enable SQLite database to hold two olm accounts: content and notifs
ClosedPublic

Authored by marcin on Feb 20 2024, 10:19 AM.
Tags
None
Referenced Files
F3363303: D11127.diff
Mon, Nov 25, 1:35 AM
Unknown Object (File)
Wed, Nov 6, 3:24 PM
Unknown Object (File)
Oct 15 2024, 2:18 PM
Unknown Object (File)
Oct 15 2024, 2:18 PM
Unknown Object (File)
Oct 15 2024, 2:18 PM
Unknown Object (File)
Sep 4 2024, 9:00 AM
Unknown Object (File)
Sep 4 2024, 9:00 AM
Unknown Object (File)
Sep 4 2024, 9:00 AM
Subscribers

Details

Summary

This differential enables SQLite database to hold to olm accounts: content and notifs.

Test Plan

Run olm tables tests from database/queries directory.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

michal added inline comments.
native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp
18–19

Could we define them with some const variable and not C #define?

native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp
335

I find it a bit unclear that we are using true/false to device between content and notifications accounts when reading the code. I wonder if it wouldn't be better to just allow the programmer to pass the ID which is defined as a constant, similar to what we use for secure store:

CommSecureStore::set(CommSecureStore::userID, "");

But up to you, I don't think it matters a lot.

web/database/types/sqlite-query-executor.js
106–111
This revision is now accepted and ready to land.Mar 4 2024, 3:32 AM
kamil added inline comments.
native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp
335

I also think using ID instead of flag makes more sense

Use accountID integer instead of boolean