Skip to content

Commit

Permalink
[native] Update updateRolesAndPermissions to only handle `LegacyRaw…
Browse files Browse the repository at this point in the history
…ThreadInfos`

Summary:
Similar to what we did in D10283, we're "lying" to `flow` and saying that `updatedRolesAndPermissions` (which we also rename `legacyUpdateRolesAndPermissions`) accepts `RawThreadInfos` instead of just `LegacyRawThreadInfos`. However, we call `assertAllThreadInfosAreLegacy` right at the start of `legacyUpdateRolesAndPermissions` to ensure that only `LegacyRawThreadInfos` are passed to this function.

It could be argued that this is worse than D10283, because it's unlikely that we'd want to run the `convertThreadStoreThreadInfosToNewIDSchema` migration again, whereas it's VERY likely that we'll want to run `updateRolesAndPermissions` in the future. I will introduce a new version of `updateRolesAndPermissions` that'll handle `MinimallyEncodedRawThreadInfos` (by going through and converting all to `LegacyRawThreadInfos` which is what the logic in this migration requires since that's what eg `getRolePermissionBlobs` and `makePermissionsBlob` deal with. I'll put that up as a followup diff in this stack (probably before landing anything).

However, as of this diff we're now at 0 `flow` issues in `native`/`web`/`keyserver`/`lib`.

---

Depends on D10283

Test Plan: CI, `flow`, etc. I think in the future it would be nice to have unit tests for migrations.

Reviewers: ashoat, ginsu, tomek, rohan

Reviewed By: ashoat

Differential Revision: https://phab.comm.dev/D10284
  • Loading branch information
atulsmadhugiri committed Dec 14, 2023
1 parent 24bd20b commit b506bf3
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@ import {
makePermissionsBlob,
makePermissionsForChildrenBlob,
} from 'lib/permissions/thread-permissions.js';
import { assertAllThreadInfosAreLegacy } from 'lib/shared/thread-utils.js';
import type { ThreadPermissionsBlob } from 'lib/types/thread-permission-types.js';
import type {
LegacyRawThreadInfo,
ThreadStoreThreadInfos,
LegacyMemberInfo,
RawThreadInfos,
} from 'lib/types/thread-types.js';
import { values } from 'lib/utils/objects.js';

Expand Down Expand Up @@ -48,10 +50,13 @@ type MemberToThreadPermissionsFromParent = {
+[member: string]: ?ThreadPermissionsBlob,
};

function updateRolesAndPermissions(
threadStoreInfos: ThreadStoreThreadInfos,
): ThreadStoreThreadInfos {
const updatedThreadStoreInfos = { ...threadStoreInfos };
// This migration utility can only be used with LegacyRawThreadInfos
function legacyUpdateRolesAndPermissions(
threadStoreInfos: RawThreadInfos,
): RawThreadInfos {
const updatedThreadStoreInfos = assertAllThreadInfosAreLegacy({
...threadStoreInfos,
});

const recursivelyUpdateRoles = (
node: $ReadOnly<ThreadTraversalNode>,
Expand Down Expand Up @@ -166,4 +171,4 @@ function updateRolesAndPermissions(
return updatedThreadStoreInfos;
}

export { updateRolesAndPermissions };
export { legacyUpdateRolesAndPermissions };
17 changes: 13 additions & 4 deletions native/redux/persist.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,11 @@ import {
} from './client-db-utils.js';
import { defaultState } from './default-state.js';
import { migrateThreadStoreForEditThreadPermissions } from './edit-thread-permission-migration.js';
import { legacyUpdateRolesAndPermissions } from './legacy-update-roles-and-permissions.js';
import { persistMigrationForManagePinsThreadPermission } from './manage-pins-permission-migration.js';
import { persistMigrationToRemoveSelectRolePermissions } from './remove-select-role-permissions.js';
import type { AppState } from './state-types.js';
import { unshimClientDB } from './unshim-utils.js';
import { updateRolesAndPermissions } from './update-roles-and-permissions.js';
import { commCoreModule } from '../native-modules.js';
import { defaultDeviceCameraInfo } from '../types/camera.js';
import { isTaskCancelledError } from '../utils/error-handling.js';
Expand Down Expand Up @@ -600,10 +600,16 @@ const migrations = {
return state;
},
[38]: (state: AppState) =>
updateClientDBThreadStoreThreadInfos(state, updateRolesAndPermissions),
updateClientDBThreadStoreThreadInfos(
state,
legacyUpdateRolesAndPermissions,
),
[39]: (state: AppState) => unshimClientDB(state, [messageTypes.EDIT_MESSAGE]),
[40]: (state: AppState) =>
updateClientDBThreadStoreThreadInfos(state, updateRolesAndPermissions),
updateClientDBThreadStoreThreadInfos(
state,
legacyUpdateRolesAndPermissions,
),
[41]: (state: AppState) => {
const queuedReports = state.reportStore.queuedReports.map(report => ({
...report,
Expand Down Expand Up @@ -979,7 +985,10 @@ const migrations = {
return state;
},
[60]: (state: AppState) =>
updateClientDBThreadStoreThreadInfos(state, updateRolesAndPermissions),
updateClientDBThreadStoreThreadInfos(
state,
legacyUpdateRolesAndPermissions,
),
};

// After migration 31, we'll no longer want to persist `messageStore.messages`
Expand Down
12 changes: 7 additions & 5 deletions native/redux/update-roles-and-permissions.test.js
Original file line number Diff line number Diff line change
@@ -1,37 +1,39 @@
// @flow

import { legacyUpdateRolesAndPermissions } from './legacy-update-roles-and-permissions.js';
import {
threadStoreThreads,
threadStoreThreadsWithEmptyRolePermissions,
threadStoreThreadsWithEmptyRolePermissionsAndMemberPermissions,
threadStoreThreadsWithEmptyRoleAndMemberAndCurrentUserPermissions,
} from './update-roles-and-permissions-test-data.js';
import { updateRolesAndPermissions } from './update-roles-and-permissions.js';

describe.skip('updateRolesAndPermissions()', () => {
it('should leave threadStoreThreads from server unchanged', () => {
expect(updateRolesAndPermissions(threadStoreThreads)).toStrictEqual(
expect(legacyUpdateRolesAndPermissions(threadStoreThreads)).toStrictEqual(
threadStoreThreads,
);
});

it('should construct role permissions when missing from existing store', () => {
expect(
updateRolesAndPermissions(threadStoreThreadsWithEmptyRolePermissions),
legacyUpdateRolesAndPermissions(
threadStoreThreadsWithEmptyRolePermissions,
),
).toStrictEqual(threadStoreThreads);
});

it('should construct role permissions AND member permissions when missing from existing store', () => {
expect(
updateRolesAndPermissions(
legacyUpdateRolesAndPermissions(
threadStoreThreadsWithEmptyRolePermissionsAndMemberPermissions,
),
).toStrictEqual(threadStoreThreads);
});

it('should construct role permissions AND member permissions AND current user permissions when missing from existing store', () => {
expect(
updateRolesAndPermissions(
legacyUpdateRolesAndPermissions(
threadStoreThreadsWithEmptyRoleAndMemberAndCurrentUserPermissions,
),
).toStrictEqual(threadStoreThreads);
Expand Down

0 comments on commit b506bf3

Please sign in to comment.