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
Unknown Object (File)
Mon, Dec 23, 11:28 PM
Unknown Object (File)
Mon, Dec 23, 11:28 PM
Unknown Object (File)
Mon, Dec 23, 11:27 PM
Unknown Object (File)
Mon, Dec 23, 7:51 AM
Unknown Object (File)
Fri, Dec 20, 6:52 AM
Unknown Object (File)
Tue, Dec 10, 7:48 PM
Unknown Object (File)
Nov 25 2024, 3:37 AM
Unknown Object (File)
Nov 25 2024, 1:35 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