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)
Apr 16 2024, 1:48 PM
Unknown Object (File)
Apr 9 2024, 1:23 AM
Unknown Object (File)
Mar 28 2024, 6:10 AM
Unknown Object (File)
Mar 16 2024, 6:50 PM
Unknown Object (File)
Mar 11 2024, 4:38 AM
Unknown Object (File)
Mar 5 2024, 3:44 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
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

michal added inline comments.
native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp
18–19 ↗(On Diff #37375)

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

native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp
335 ↗(On Diff #37375)

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 ↗(On Diff #37375)
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 ↗(On Diff #37375)

I also think using ID instead of flag makes more sense

Use accountID integer instead of boolean