Page MenuHomePhabricator
Differential D11151

[sqlite] add methods to operate on communities table
ClosedPublic
Actions

Authored by ginsu on Feb 22 2024, 11:55 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Apr 25, 1:04 AM
Unknown Object (File)
Sat, Apr 19, 8:34 AM
Unknown Object (File)
Thu, Apr 17, 10:00 PM
Unknown Object (File)
Thu, Apr 17, 10:54 AM
Unknown Object (File)
Tue, Apr 15, 10:17 PM
Unknown Object (File)
Feb 21 2025, 8:27 PM
Unknown Object (File)
Feb 21 2025, 8:27 PM
Unknown Object (File)
Feb 21 2025, 8:27 PM
View All 55 Files
Subscribers
ashoat
marcin
tomek

Details

Reviewers
atul
inka
kamil
marcin
Commits
rCOMM68a6672a3569: [sqlite] add methods to operate on communities table
Summary

This diff introduces the logic + boilerplate for how we will represent a community in the table data row, the methods to add, modify and remove those data rows and the emscripten binding

Linear task: https://linear.app/comm/issue/ENG-6531/add-methods-to-operate-on-operate-on-communities-table

Depends on D11087

Test Plan

Locally built some buttons + actions for each method and confirmed that when I triggered the actions the client db communities table was correctly being modified.

For reference, here is the local code with the callbacks that dispatch the actions to my community reducer (The community reducer will be introduced in subsequent diff)

const dispatch = useDispatch();

const fakeCommunityID = '256|84261';

const onPressAddCommunity = React.useCallback(() => {
  const communityInfo: CommunityInfo = {
    enabledApps: {
      calendar: true,
      wiki: false,
      tasks: true,
      files: true,
    },
  };

  dispatch({
    type: addCommunityActionType,
    payload: {
      id: fakeCommunityID,
      newCommunityInfo: communityInfo,
    },
  });
}, [dispatch]);

const onPressRemoveCommunity = React.useCallback(() => {
  dispatch({
    type: removeCommunityActionType,
    payload: {
      id: fakeCommunityID,
    },
  });
}, [dispatch]);

const onPressRemoveAllCommunities = React.useCallback(() => {
  dispatch({
    type: removeAllCommunitesActionType,
  });
}, [dispatch]);

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

ginsu created this revision.Feb 22 2024, 11:55 PM
Herald added subscribers: tomek, ashoat. · View Herald TranscriptFeb 22 2024, 11:55 PM
ginsu updated this revision to Diff 37476.Feb 22 2024, 11:57 PM

update

ginsu edited the summary of this revision. (Show Details)Feb 23 2024, 12:00 AM
ginsu added reviewers: atul, inka, kamil.
ginsu added a parent revision: D11087: [sqlite] add communites table for community store data.
ginsu added a reviewer: marcin.Feb 23 2024, 12:05 AM
ginsu added a subscriber: marcin.

Also adding @marcin as a reviewer since he did some work recently to deprecate our use of ORM

ginsu edited the test plan for this revision. (Show Details)Feb 23 2024, 12:09 AM
ginsu added a child revision: D11152: [sqlite] implement C++ community store ops.
ginsu edited the test plan for this revision. (Show Details)Feb 23 2024, 12:11 AM
Harbormaster completed remote builds in B27084: Diff 37476.Feb 23 2024, 12:19 AM
ginsu requested review of this revision.Feb 23 2024, 12:19 AM
Harbormaster completed remote builds in B27083: Diff 37475.Feb 23 2024, 12:19 AM
kamil accepted this revision.Feb 23 2024, 6:05 AM

LGTM!

Could you add tests to this?
Here are some examples:

This should be quick and it's better to make sure queries work and make it safe to introduce some changes in the future.

This revision is now accepted and ready to land.Feb 23 2024, 6:05 AM
ginsu updated this revision to Diff 37552.Feb 23 2024, 5:16 PM

address feedback

Harbormaster failed remote builds in B27135: Diff 37552!Feb 23 2024, 5:26 PM
Harbormaster completed remote builds in B27135: Diff 37552.Feb 26 2024, 2:32 PM
ginsu updated this revision to Diff 37667.Feb 27 2024, 7:19 PM

rebase before landing

Harbormaster completed remote builds in B27214: Diff 37667.Feb 27 2024, 7:38 PM
Closed by commit rCOMM68a6672a3569: [sqlite] add methods to operate on communities table (authored by ginsu). · Explain WhyFeb 27 2024, 8:02 PM
This revision was automatically updated to reflect the committed changes.
ginsu added a commit: rCOMM68a6672a3569: [sqlite] add methods to operate on communities table.

Revision Contents
Changeset List

PathSize
lib/
ops/
23 lines
native/
cpp/
CommonCpp/
DatabaseManagers/
5 lines
5 lines
43 lines
entities/
24 lines
web/
cpp/
8 lines
database/
_generated/
queries/
132 lines
types/
6 lines

Diff 37552

View Options

lib/ops/community-store-ops.js

// @flow // @flow
import { DATABASE_WORKER_PATH, SQLJS_FILE_PATH } from './utils/constants.js'; import { DATABASE_WORKER_PATH, SQLJS_FILE_PATH } from './utils/constants.js';
import { isSQLiteSupported } from './utils/db-utils.js'; import { isDesktopSafari, isSQLiteSupported } from './utils/db-utils.js';
import WorkerConnectionProxy from './utils/WorkerConnectionProxy.js'; import WorkerConnectionProxy from './utils/WorkerConnectionProxy.js';
import type { AppState } from '../redux/redux-setup.js'; import type { AppState } from '../redux/redux-setup.js';
import { import {
workerRequestMessageTypes, workerRequestMessageTypes,
type WorkerRequestMessage, type WorkerRequestMessage,
type WorkerResponseMessage, type WorkerResponseMessage,
} from '../types/worker-types.js'; } from '../types/worker-types.js';
Show All 16 Linesclass DatabaseModule {
status: DatabaseStatus; status: DatabaseStatus;
constructor() { constructor() {
const currentLoggedInUserID = preloadedState.currentUserInfo?.anonymous const currentLoggedInUserID = preloadedState.currentUserInfo?.anonymous
? undefined ? undefined
: preloadedState.currentUserInfo?.id; : preloadedState.currentUserInfo?.id;
const isSupported = isSQLiteSupported(currentLoggedInUserID); const isSupported = isSQLiteSupported(currentLoggedInUserID);
if (!isSupported) { if (!isSupported || isDesktopSafari()) {
tomekUnsubmitted
Not Done
Inline Actions

Can we have a const instead of a function that tells if it is Safari? We can avoid having to detect the browser multiple times, as this fact doesn't change through the lifetime of the app.

tomek: Can we have a const instead of a function that tells if it is Safari? We can avoid having to…
kamilAuthorUnsubmitted
Done
Inline Actions

good point, thanks

kamil: good point, thanks
this.status = databaseStatuses.notSupported; this.status = databaseStatuses.notSupported;
kamilAuthorUnsubmitted
Done
Inline Actions

Naming might be confusing, but at this point database is not supported (we don't have a key to it), it will be supported after main thread will provide encryption key

kamil: Naming might be confusing, but at this point database is not supported (we don't have a key to…
tomekUnsubmitted
Not Done
Inline Actions

Do we change the status later?

tomek: Do we change the status later?
kamilAuthorUnsubmitted
Done
Inline Actions

yes, in initDBForLoggedInUser()

kamil: yes, in `initDBForLoggedInUser()`
} else { } else {
this.init(); this.init();
} }
} }
init(encryptionKey?: ?SubtleCrypto$JsonWebKey) { init(encryptionKey?: ?SubtleCrypto$JsonWebKey) {
this.status = databaseStatuses.initInProgress; this.status = databaseStatuses.initInProgress;
this.worker = new SharedWorker(DATABASE_WORKER_PATH); this.worker = new SharedWorker(DATABASE_WORKER_PATH);
▲ Show 20 Lines • Show All 77 Lines • Show Last 20 Lines
View Options

native/cpp/CommonCpp/DatabaseManagers/DatabaseQueryExecutor.h

// @flow // @flow
import { DATABASE_WORKER_PATH, SQLJS_FILE_PATH } from './utils/constants.js'; import { DATABASE_WORKER_PATH, SQLJS_FILE_PATH } from './utils/constants.js';
import { isSQLiteSupported } from './utils/db-utils.js'; import { isDesktopSafari, isSQLiteSupported } from './utils/db-utils.js';
import WorkerConnectionProxy from './utils/WorkerConnectionProxy.js'; import WorkerConnectionProxy from './utils/WorkerConnectionProxy.js';
import type { AppState } from '../redux/redux-setup.js'; import type { AppState } from '../redux/redux-setup.js';
import { import {
workerRequestMessageTypes, workerRequestMessageTypes,
type WorkerRequestMessage, type WorkerRequestMessage,
type WorkerResponseMessage, type WorkerResponseMessage,
} from '../types/worker-types.js'; } from '../types/worker-types.js';
Show All 16 Linesclass DatabaseModule {
status: DatabaseStatus; status: DatabaseStatus;
constructor() { constructor() {
const currentLoggedInUserID = preloadedState.currentUserInfo?.anonymous const currentLoggedInUserID = preloadedState.currentUserInfo?.anonymous
? undefined ? undefined
: preloadedState.currentUserInfo?.id; : preloadedState.currentUserInfo?.id;
const isSupported = isSQLiteSupported(currentLoggedInUserID); const isSupported = isSQLiteSupported(currentLoggedInUserID);
if (!isSupported) { if (!isSupported || isDesktopSafari()) {
tomekUnsubmitted
Not Done
Inline Actions

Can we have a const instead of a function that tells if it is Safari? We can avoid having to detect the browser multiple times, as this fact doesn't change through the lifetime of the app.

tomek: Can we have a const instead of a function that tells if it is Safari? We can avoid having to…
kamilAuthorUnsubmitted
Done
Inline Actions

good point, thanks

kamil: good point, thanks
this.status = databaseStatuses.notSupported; this.status = databaseStatuses.notSupported;
kamilAuthorUnsubmitted
Done
Inline Actions

Naming might be confusing, but at this point database is not supported (we don't have a key to it), it will be supported after main thread will provide encryption key

kamil: Naming might be confusing, but at this point database is not supported (we don't have a key to…
tomekUnsubmitted
Not Done
Inline Actions

Do we change the status later?

tomek: Do we change the status later?
kamilAuthorUnsubmitted
Done
Inline Actions

yes, in initDBForLoggedInUser()

kamil: yes, in `initDBForLoggedInUser()`
} else { } else {
this.init(); this.init();
} }
} }
init(encryptionKey?: ?SubtleCrypto$JsonWebKey) { init(encryptionKey?: ?SubtleCrypto$JsonWebKey) {
this.status = databaseStatuses.initInProgress; this.status = databaseStatuses.initInProgress;
this.worker = new SharedWorker(DATABASE_WORKER_PATH); this.worker = new SharedWorker(DATABASE_WORKER_PATH);
▲ Show 20 Lines • Show All 77 Lines • Show Last 20 Lines
View Options

native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.h

// @flow // @flow
import { DATABASE_WORKER_PATH, SQLJS_FILE_PATH } from './utils/constants.js'; import { DATABASE_WORKER_PATH, SQLJS_FILE_PATH } from './utils/constants.js';
import { isSQLiteSupported } from './utils/db-utils.js'; import { isDesktopSafari, isSQLiteSupported } from './utils/db-utils.js';
import WorkerConnectionProxy from './utils/WorkerConnectionProxy.js'; import WorkerConnectionProxy from './utils/WorkerConnectionProxy.js';
import type { AppState } from '../redux/redux-setup.js'; import type { AppState } from '../redux/redux-setup.js';
import { import {
workerRequestMessageTypes, workerRequestMessageTypes,
type WorkerRequestMessage, type WorkerRequestMessage,
type WorkerResponseMessage, type WorkerResponseMessage,
} from '../types/worker-types.js'; } from '../types/worker-types.js';
Show All 16 Linesclass DatabaseModule {
status: DatabaseStatus; status: DatabaseStatus;
constructor() { constructor() {
const currentLoggedInUserID = preloadedState.currentUserInfo?.anonymous const currentLoggedInUserID = preloadedState.currentUserInfo?.anonymous
? undefined ? undefined
: preloadedState.currentUserInfo?.id; : preloadedState.currentUserInfo?.id;
const isSupported = isSQLiteSupported(currentLoggedInUserID); const isSupported = isSQLiteSupported(currentLoggedInUserID);
if (!isSupported) { if (!isSupported || isDesktopSafari()) {
tomekUnsubmitted
Not Done
Inline Actions

Can we have a const instead of a function that tells if it is Safari? We can avoid having to detect the browser multiple times, as this fact doesn't change through the lifetime of the app.

tomek: Can we have a const instead of a function that tells if it is Safari? We can avoid having to…
kamilAuthorUnsubmitted
Done
Inline Actions

good point, thanks

kamil: good point, thanks
this.status = databaseStatuses.notSupported; this.status = databaseStatuses.notSupported;
kamilAuthorUnsubmitted
Done
Inline Actions

Naming might be confusing, but at this point database is not supported (we don't have a key to it), it will be supported after main thread will provide encryption key

kamil: Naming might be confusing, but at this point database is not supported (we don't have a key to…
tomekUnsubmitted
Not Done
Inline Actions

Do we change the status later?

tomek: Do we change the status later?
kamilAuthorUnsubmitted
Done
Inline Actions

yes, in initDBForLoggedInUser()

kamil: yes, in `initDBForLoggedInUser()`
} else { } else {
this.init(); this.init();
} }
} }
init(encryptionKey?: ?SubtleCrypto$JsonWebKey) { init(encryptionKey?: ?SubtleCrypto$JsonWebKey) {
this.status = databaseStatuses.initInProgress; this.status = databaseStatuses.initInProgress;
this.worker = new SharedWorker(DATABASE_WORKER_PATH); this.worker = new SharedWorker(DATABASE_WORKER_PATH);
▲ Show 20 Lines • Show All 77 Lines • Show Last 20 Lines
View Options

native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp

// @flow // @flow
import { DATABASE_WORKER_PATH, SQLJS_FILE_PATH } from './utils/constants.js'; import { DATABASE_WORKER_PATH, SQLJS_FILE_PATH } from './utils/constants.js';
import { isSQLiteSupported } from './utils/db-utils.js'; import { isDesktopSafari, isSQLiteSupported } from './utils/db-utils.js';
import WorkerConnectionProxy from './utils/WorkerConnectionProxy.js'; import WorkerConnectionProxy from './utils/WorkerConnectionProxy.js';
import type { AppState } from '../redux/redux-setup.js'; import type { AppState } from '../redux/redux-setup.js';
import { import {
workerRequestMessageTypes, workerRequestMessageTypes,
type WorkerRequestMessage, type WorkerRequestMessage,
type WorkerResponseMessage, type WorkerResponseMessage,
} from '../types/worker-types.js'; } from '../types/worker-types.js';
Show All 16 Linesclass DatabaseModule {
status: DatabaseStatus; status: DatabaseStatus;
constructor() { constructor() {
const currentLoggedInUserID = preloadedState.currentUserInfo?.anonymous const currentLoggedInUserID = preloadedState.currentUserInfo?.anonymous
? undefined ? undefined
: preloadedState.currentUserInfo?.id; : preloadedState.currentUserInfo?.id;
const isSupported = isSQLiteSupported(currentLoggedInUserID); const isSupported = isSQLiteSupported(currentLoggedInUserID);
if (!isSupported) { if (!isSupported || isDesktopSafari()) {
tomekUnsubmitted
Not Done
Inline Actions

Can we have a const instead of a function that tells if it is Safari? We can avoid having to detect the browser multiple times, as this fact doesn't change through the lifetime of the app.

tomek: Can we have a const instead of a function that tells if it is Safari? We can avoid having to…
kamilAuthorUnsubmitted
Done
Inline Actions

good point, thanks

kamil: good point, thanks
this.status = databaseStatuses.notSupported; this.status = databaseStatuses.notSupported;
kamilAuthorUnsubmitted
Done
Inline Actions

Naming might be confusing, but at this point database is not supported (we don't have a key to it), it will be supported after main thread will provide encryption key

kamil: Naming might be confusing, but at this point database is not supported (we don't have a key to…
tomekUnsubmitted
Not Done
Inline Actions

Do we change the status later?

tomek: Do we change the status later?
kamilAuthorUnsubmitted
Done
Inline Actions

yes, in initDBForLoggedInUser()

kamil: yes, in `initDBForLoggedInUser()`
} else { } else {
this.init(); this.init();
} }
} }
init(encryptionKey?: ?SubtleCrypto$JsonWebKey) { init(encryptionKey?: ?SubtleCrypto$JsonWebKey) {
this.status = databaseStatuses.initInProgress; this.status = databaseStatuses.initInProgress;
this.worker = new SharedWorker(DATABASE_WORKER_PATH); this.worker = new SharedWorker(DATABASE_WORKER_PATH);
▲ Show 20 Lines • Show All 77 Lines • Show Last 20 Lines
View Options

native/cpp/CommonCpp/DatabaseManagers/entities/CommunityInfo.h

Loading...
View Options

web/cpp/SQLiteQueryExecutorBindings.cpp

Loading...
View Options

web/database/_generated/comm_query_executor.wasm

Loading...
View Options

web/database/queries/communities-queries.test.js

Loading...
View Options

web/database/types/sqlite-query-executor.js

Loading...