Skip to content

Commit

Permalink
[lib] Introduce RawThickThreadInfo type
Browse files Browse the repository at this point in the history
Summary:
This diff splits `RawThreadInfo` into `RawThinThreadInfo` (basically the old type) and `RawThickThreadInfo`, which will be used for E2E-encrypted DMs.

The latter type includes `ThreadSubscription` for each member.

I tried to break this diff down but it was tough due to Flow issues.

This diff doesn't update `ThreadInfo`. That type is used generally in frontend code, and I'm not sure we'll need the new `ThreadSubscription`.

This diff also doesn't do anything to make sure we handle `RawThickThreadInfo` correctly when we encounter it. That will be handled in later tasks. It's okay to commit this type change now as we aren't actually introducing `RawThickThreadInfo` to any stores.

On the Flow side here, I had to add a `thick: true` attribute. I initially tried to differentiate the two types through the `type` field, which should let us represent `RawThreadInfo` was a union of two disjoint types (`RawThinThreadInfo` and `RawThickThreadInfo`). But I wasn't able to get Flow to narrow the types when I checked the `type` field. Here's what I tried:
- Enumerating all of the possible values of `type` at every location, for both the thick and thin conditions. This worked but was way too verbose
- Considered using [type guards](https://flow.org/en/docs/types/type-guards/), but our setup [doesn't support](https://flow.org/en/docs/types/type-guards/#toc-adoption) them yet. More details in [this blog](https://medium.com/flow-type/announcing-user-defined-type-guards-in-flow-b979bb2e78cf)
- Considered using an older version of type guards: assert functions that use `%checks`. However this wasn't able to narrow a `threadInfo` by checking `threadInfo.type` for some reason
- Considered using assert functions that just have `invariant`s and `any`s instead of `%checks`. This resulted in four functions: `assertThinRawThreadInfo`, `assertThickRawThreadInfo`, `assertLegacyThinRawThreadInfo`, `assertLegacyThickRawThreadInfo`. A pair of these would be combined with a `threadTypeIsThick` check. Ultimately decided it was too verbose
- Referenced [this GitHub issue](facebook/flow#6516 (comment)) where something similar is discussed

Depends on D12494

Test Plan: Flow

Reviewers: kamil, marcin

Subscribers: tomek

Differential Revision: https://phab.comm.dev/D12495
  • Loading branch information
Ashoat authored and marcinwasowicz committed Jun 25, 2024
1 parent ac84f13 commit c44ffc0
Show file tree
Hide file tree
Showing 9 changed files with 419 additions and 144 deletions.
12 changes: 9 additions & 3 deletions lib/selectors/user-selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,16 +71,22 @@ function getRelativeMemberInfos(
const username = userInfos[memberInfo.id]
? userInfos[memberInfo.id].username
: null;
const { permissions, ...memberInfoSansPermissions } = memberInfo;
const { id, role, isSender, minimallyEncoded } = memberInfo;
if (memberInfo.id === currentUserID) {
relativeMemberInfos.unshift({
...memberInfoSansPermissions,
id,
role,
isSender,
minimallyEncoded,
username,
isViewer: true,
});
} else {
relativeMemberInfos.push({
...memberInfoSansPermissions,
id,
role,
isSender,
minimallyEncoded,
username,
isViewer: false,
});
Expand Down
68 changes: 58 additions & 10 deletions lib/shared/redux/legacy-update-roles-and-permissions.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import type {
ThreadStoreThreadInfos,
LegacyMemberInfo,
MixedRawThreadInfos,
ThickMemberInfo,
} from '../../types/thread-types.js';
import { values } from '../../utils/objects.js';

Expand Down Expand Up @@ -51,6 +52,12 @@ type MemberToThreadPermissionsFromParent = {
+[member: string]: ?ThreadPermissionsBlob,
};

type BaseMemberInfo = {
+id: string,
+role: ?string,
...
};

// This migration utility can only be used with LegacyRawThreadInfos
function legacyUpdateRolesAndPermissions(
threadStoreInfos: MixedRawThreadInfos,
Expand Down Expand Up @@ -81,18 +88,19 @@ function legacyUpdateRolesAndPermissions(
node.children?.map(recursivelyUpdateRoles);
};

const recursivelyUpdatePermissions = (
node: $ReadOnly<ThreadTraversalNode>,
const updateMembers = <T: BaseMemberInfo>(
threadInfo: LegacyRawThreadInfo,
members: $ReadOnlyArray<$ReadOnly<T>>,
memberToThreadPermissionsFromParent: ?MemberToThreadPermissionsFromParent,
): void => {
const threadInfo: LegacyRawThreadInfo =
updatedThreadStoreInfos[node.threadID];

): {
members: $ReadOnlyArray<$ReadOnly<T>>,
memberToThreadPermissionsForChildren: { [string]: ?ThreadPermissionsBlob },
} => {
const updatedMembers = [];
const memberToThreadPermissionsForChildren: {
[string]: ?ThreadPermissionsBlob,
} = {};
for (const member: LegacyMemberInfo of threadInfo.members) {
for (const member of members) {
const { id, role } = member;

const rolePermissions = role ? threadInfo.roles[role].permissions : null;
Expand All @@ -116,11 +124,51 @@ function legacyUpdateRolesAndPermissions(
memberToThreadPermissionsForChildren[member.id] =
makePermissionsForChildrenBlob(computedPermissions);
}

updatedThreadStoreInfos[node.threadID] = {
...threadInfo,
return {
members: updatedMembers,
memberToThreadPermissionsForChildren,
};
};

const recursivelyUpdatePermissions = (
node: $ReadOnly<ThreadTraversalNode>,
memberToThreadPermissionsFromParent: ?MemberToThreadPermissionsFromParent,
): void => {
const threadInfo: LegacyRawThreadInfo =
updatedThreadStoreInfos[node.threadID];

let memberToThreadPermissionsForChildren: {
[string]: ?ThreadPermissionsBlob,
};
if (threadInfo.thick) {
const {
members: updatedMembers,
memberToThreadPermissionsForChildren: test,
} = updateMembers<ThickMemberInfo>(
threadInfo,
threadInfo.members,
memberToThreadPermissionsFromParent,
);
updatedThreadStoreInfos[node.threadID] = {
...threadInfo,
members: updatedMembers,
};
memberToThreadPermissionsForChildren = test;
} else {
const {
members: updatedMembers,
memberToThreadPermissionsForChildren: test,
} = updateMembers<LegacyMemberInfo>(
threadInfo,
threadInfo.members,
memberToThreadPermissionsFromParent,
);
updatedThreadStoreInfos[node.threadID] = {
...threadInfo,
members: updatedMembers,
};
memberToThreadPermissionsForChildren = test;
}

node.children?.map(child =>
recursivelyUpdatePermissions(child, memberToThreadPermissionsForChildren),
Expand Down
113 changes: 82 additions & 31 deletions lib/shared/thread-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,15 @@ import {
threadTypes,
threadTypeIsCommunityRoot,
assertThreadType,
threadTypeIsThick,
assertThinThreadType,
assertThickThreadType,
} from '../types/thread-types-enum.js';
import type {
LegacyRawThreadInfo,
ClientLegacyRoleInfo,
ServerThreadInfo,
ServerMemberInfo,
ThickMemberInfo,
UserProfileThreadInfo,
MixedRawThreadInfos,
LegacyMemberInfo,
Expand Down Expand Up @@ -470,38 +473,84 @@ function createPendingThread({
specialRole: specialRoles.DEFAULT_ROLE,
};

const rawThreadInfo: RawThreadInfo = {
minimallyEncoded: true,
id: threadID,
type: threadType,
name: name ?? null,
description: null,
color: threadColor ?? generatePendingThreadColor(memberIDs),
creationTime: now,
parentThreadID: parentThreadInfo?.id ?? null,
containingThreadID: getContainingThreadID(parentThreadInfo, threadType),
community: getCommunity(parentThreadInfo),
members: members.map(member =>
minimallyEncodeMemberInfo({
id: member.id,
let rawThreadInfo: RawThreadInfo;
if (threadTypeIsThick(threadType)) {
const thickThreadType = assertThickThreadType(threadType);
rawThreadInfo = {
minimallyEncoded: true,
thick: true,
id: threadID,
type: thickThreadType,
name: name ?? null,
description: null,
color: threadColor ?? generatePendingThreadColor(memberIDs),
creationTime: now,
parentThreadID: parentThreadInfo?.id ?? null,
containingThreadID: getContainingThreadID(
parentThreadInfo,
thickThreadType,
),
community: getCommunity(parentThreadInfo),
members: members.map(member =>
minimallyEncodeMemberInfo<ThickMemberInfo>({
id: member.id,
role: role.id,
permissions: membershipPermissions,
isSender: false,
subscription: defaultSubscription,
}),
),
roles: {
[role.id]: role,
},
currentUser: minimallyEncodeThreadCurrentUserInfo({
role: role.id,
permissions: membershipPermissions,
isSender: false,
subscription: defaultSubscription,
unread: false,
}),
),
roles: {
[role.id]: role,
},
currentUser: minimallyEncodeThreadCurrentUserInfo({
role: role.id,
permissions: membershipPermissions,
subscription: defaultSubscription,
unread: false,
}),
repliesCount: 0,
sourceMessageID,
pinnedCount: 0,
};
repliesCount: 0,
sourceMessageID,
pinnedCount: 0,
};
} else {
const thinThreadType = assertThinThreadType(threadType);
rawThreadInfo = {
minimallyEncoded: true,
id: threadID,
type: thinThreadType,
name: name ?? null,
description: null,
color: threadColor ?? generatePendingThreadColor(memberIDs),
creationTime: now,
parentThreadID: parentThreadInfo?.id ?? null,
containingThreadID: getContainingThreadID(
parentThreadInfo,
thinThreadType,
),
community: getCommunity(parentThreadInfo),
members: members.map(member =>
minimallyEncodeMemberInfo<LegacyMemberInfo>({
id: member.id,
role: role.id,
permissions: membershipPermissions,
isSender: false,
}),
),
roles: {
[role.id]: role,
},
currentUser: minimallyEncodeThreadCurrentUserInfo({
role: role.id,
permissions: membershipPermissions,
subscription: defaultSubscription,
unread: false,
}),
repliesCount: 0,
sourceMessageID,
pinnedCount: 0,
};
}

const userInfos: { [string]: UserInfo } = {};
for (const member of members) {
Expand Down Expand Up @@ -1024,7 +1073,9 @@ const threadTypeDescriptions: { [ThreadType]: string } = {
// Since we don't have access to all of the ancestor ThreadInfos, we approximate
// "parent admin" as anybody with CHANGE_ROLE permissions.
function memberHasAdminPowers(
memberInfo: LegacyMemberInfo | MemberInfoWithPermissions | ServerMemberInfo,
memberInfo:
| { +minimallyEncoded: true, +permissions: string, ... }
| { +minimallyEncoded?: void, +permissions: ThreadPermissionsInfo, ... },
): boolean {
if (memberInfo.minimallyEncoded) {
return hasPermission(memberInfo.permissions, threadPermissions.CHANGE_ROLE);
Expand Down
36 changes: 26 additions & 10 deletions lib/shared/updates/delete-account-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,35 @@ export const deleteAccountSpec: UpdateSpec<
const operations = [];
for (const threadID in storeThreadInfos) {
const threadInfo = storeThreadInfos[threadID];
const newMembers = threadInfo.members.filter(
member => member.id !== update.deletedUserID,
);
if (newMembers.length < threadInfo.members.length) {
const updatedThread = {
...threadInfo,
members: newMembers,
};

let replacedThreadInfo;
if (threadInfo.thick) {
const newMembers = threadInfo.members.filter(
member => member.id !== update.deletedUserID,
);
if (newMembers.length < threadInfo.members.length) {
replacedThreadInfo = {
...threadInfo,
members: newMembers,
};
}
} else {
const newMembers = threadInfo.members.filter(
member => member.id !== update.deletedUserID,
);
if (newMembers.length < threadInfo.members.length) {
replacedThreadInfo = {
...threadInfo,
members: newMembers,
};
}
}
if (replacedThreadInfo) {
operations.push({
type: 'replace',
payload: {
id: threadID,
threadInfo: updatedThread,
id: threadInfo.id,
threadInfo: replacedThreadInfo,
},
});
}
Expand Down
Loading

0 comments on commit c44ffc0

Please sign in to comment.