Skip to content

Commit

Permalink
[lib] Handle a case where viewer is added to a thread while being its…
Browse files Browse the repository at this point in the history
… member

Summary:
It is possible that we receive an operation about viewer being added to a thread while the thread is already present in the store and viewer is its member. We need to make sure that we don't override the whole thread and instead update it as little as possible.

https://linear.app/comm/issue/ENG-8720/add-protection-against-thick-thread-creation-overwriting-existing

Depends on D13483

Test Plan: Create a sidebar, add, remove, and add a user and check on that user's device if the thread is displayed and updated correctly.

Reviewers: kamil, will, ashoat

Reviewed By: ashoat

Subscribers: ashoat

Differential Revision: https://phab.comm.dev/D13490
  • Loading branch information
palys-swm committed Sep 27, 2024
1 parent 23ee267 commit a432a96
Show file tree
Hide file tree
Showing 2 changed files with 103 additions and 28 deletions.
73 changes: 47 additions & 26 deletions lib/shared/dm-ops/add-members-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ import { messageTypes } from '../../types/message-types-enum.js';
import type { RawMessageInfo } from '../../types/message-types.js';
import type { AddMembersMessageData } from '../../types/messages/add-members.js';
import { minimallyEncodeMemberInfo } from '../../types/minimally-encoded-thread-permissions-types.js';
import type { ThickRawThreadInfo } from '../../types/minimally-encoded-thread-permissions-types.js';
import { joinThreadSubscription } from '../../types/subscription-types.js';
import type { ThreadPermissionsInfo } from '../../types/thread-permission-types.js';
import type { ThickMemberInfo } from '../../types/thread-types.js';
import { updateTypes } from '../../types/update-types-enum.js';
import { values } from '../../utils/objects.js';
Expand All @@ -41,6 +43,46 @@ function createAddNewMembersMessageDataWithInfoFromDMOperation(
return { messageData, rawMessageInfo };
}

function createPermissionsForNewMembers(
threadInfo: ThickRawThreadInfo,
utilities: ProcessDMOperationUtilities,
): {
+membershipPermissions: ThreadPermissionsInfo,
+roleID: string,
} {
const defaultRoleID = values(threadInfo.roles).find(role =>
roleIsDefaultRole(role),
)?.id;
invariant(defaultRoleID, 'Default role ID must exist');

const { parentThreadID } = threadInfo;
const parentThreadInfo = parentThreadID
? utilities.threadInfos[parentThreadID]
: null;
if (parentThreadID && !parentThreadInfo) {
console.log(
`Parent thread with ID ${parentThreadID} was expected while adding ` +
'thread members but is missing from the store',
);
}
invariant(
!parentThreadInfo || parentThreadInfo.thick,
'Parent thread should be thick',
);

const { membershipPermissions } = createRoleAndPermissionForThickThreads(
threadInfo.type,
threadInfo.id,
defaultRoleID,
parentThreadInfo,
);

return {
membershipPermissions,
roleID: defaultRoleID,
};
}

const addMembersSpec: DMOperationSpec<DMAddMembersOperation> = Object.freeze({
notificationsCreationData: async (dmOperation: DMAddMembersOperation) => {
return {
Expand Down Expand Up @@ -69,31 +111,9 @@ const addMembersSpec: DMOperationSpec<DMAddMembersOperation> = Object.freeze({
};
}

const defaultRoleID = values(currentThreadInfo.roles).find(role =>
roleIsDefaultRole(role),
)?.id;
invariant(defaultRoleID, 'Default role ID must exist');

const parentThreadID = currentThreadInfo.parentThreadID;
const parentThreadInfo = parentThreadID
? utilities.threadInfos[parentThreadID]
: null;
if (parentThreadID && !parentThreadInfo) {
console.log(
`Parent thread with ID ${parentThreadID} was expected while adding ` +
'thread members but is missing from the store',
);
}
invariant(
!parentThreadInfo || parentThreadInfo.thick,
'Parent thread should be thick',
);

const { membershipPermissions } = createRoleAndPermissionForThickThreads(
currentThreadInfo.type,
currentThreadInfo.id,
defaultRoleID,
parentThreadInfo,
const { membershipPermissions, roleID } = createPermissionsForNewMembers(
currentThreadInfo,
utilities,
);

const memberTimestamps = { ...currentThreadInfo.timestamps.members };
Expand Down Expand Up @@ -122,7 +142,7 @@ const addMembersSpec: DMOperationSpec<DMAddMembersOperation> = Object.freeze({
newMembers.push(
minimallyEncodeMemberInfo<ThickMemberInfo>({
id: userID,
role: defaultRoleID,
role: roleID,
permissions: membershipPermissions,
isSender: editorID === viewerID,
subscription: joinThreadSubscription,
Expand Down Expand Up @@ -175,4 +195,5 @@ const addMembersSpec: DMOperationSpec<DMAddMembersOperation> = Object.freeze({
export {
addMembersSpec,
createAddNewMembersMessageDataWithInfoFromDMOperation,
createPermissionsForNewMembers,
};
58 changes: 56 additions & 2 deletions lib/shared/dm-ops/add-viewer-to-thread-members-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import uuid from 'uuid';

import { createPermissionsForNewMembers } from './add-members-spec.js';
import { createThickRawThreadInfo } from './create-thread-spec.js';
import type {
DMOperationSpec,
Expand All @@ -15,7 +16,12 @@ import { messageTypes } from '../../types/message-types-enum.js';
import type { RawMessageInfo } from '../../types/message-types.js';
import { messageTruncationStatus } from '../../types/message-types.js';
import type { AddMembersMessageData } from '../../types/messages/add-members.js';
import {
minimallyEncodeMemberInfo,
minimallyEncodeThreadCurrentUserInfo,
} from '../../types/minimally-encoded-thread-permissions-types.js';
import { joinThreadSubscription } from '../../types/subscription-types.js';
import type { ThickMemberInfo } from '../../types/thread-types.js';
import { updateTypes } from '../../types/update-types-enum.js';
import { rawMessageInfoFromMessageData } from '../message-utils.js';
import { userIsMember } from '../thread-utils.js';
Expand Down Expand Up @@ -56,7 +62,7 @@ const addViewerToThreadMembersSpec: DMOperationSpec<DMAddViewerToThreadMembersOp
dmOperation: DMAddViewerToThreadMembersOperation,
utilities: ProcessDMOperationUtilities,
) => {
const { time, messageID, addedUserIDs, existingThreadDetails } =
const { time, messageID, addedUserIDs, existingThreadDetails, editorID } =
dmOperation;
const { threadInfos } = utilities;

Expand Down Expand Up @@ -100,6 +106,54 @@ const addViewerToThreadMembersSpec: DMOperationSpec<DMAddViewerToThreadMembersOp
}
}

if (currentThreadInfo) {
const { membershipPermissions, roleID } =
createPermissionsForNewMembers(currentThreadInfo, utilities);

const newMemberInfos = newMembers.map(userID =>
minimallyEncodeMemberInfo<ThickMemberInfo>({
id: userID,
role: roleID,
permissions: membershipPermissions,
isSender: editorID === utilities.viewerID,
subscription: joinThreadSubscription,
}),
);

const resultThreadInfo = {
...currentThreadInfo,
members: [...currentThreadInfo.members, ...newMemberInfos],
currentUser: minimallyEncodeThreadCurrentUserInfo({
role: roleID,
permissions: membershipPermissions,
subscription: joinThreadSubscription,
unread: true,
}),
timestamps: {
...currentThreadInfo.timestamps,
members: {
...currentThreadInfo.timestamps.members,
...memberTimestamps,
},
},
};

const updateInfos = [
{
type: updateTypes.UPDATE_THREAD,
id: uuid.v4(),
time,
threadInfo: resultThreadInfo,
},
];

return {
rawMessageInfos,
updateInfos,
blobOps: [],
};
}

const resultThreadInfo = createThickRawThreadInfo(
{
...existingThreadDetails,
Expand Down Expand Up @@ -132,7 +186,7 @@ const addViewerToThreadMembersSpec: DMOperationSpec<DMAddViewerToThreadMembersOp
},
];
return {
rawMessageInfos,
rawMessageInfos: [],
updateInfos,
blobOps: [],
};
Expand Down

0 comments on commit a432a96

Please sign in to comment.