From f46fd6f2d574098289cd33fc374f988fb0cf432d Mon Sep 17 00:00:00 2001 From: Eric Mc Sween <5454374+emcsween@users.noreply.github.com> Date: Thu, 27 Mar 2025 08:33:08 -0400 Subject: [PATCH] Merge pull request #24433 from overleaf/em-pending-reviewers Support reviewers in the collaborator limit enforcement logic GitOrigin-RevId: f11a8e37ca6ef36f9894233803c6ee8363bf0ff8 --- .../Collaborators/CollaboratorsGetter.js | 42 +++++-- .../Collaborators/CollaboratorsHandler.js | 39 ++++++- .../CollaboratorsInviteHandler.mjs | 57 ++++++--- .../Features/Project/ProjectEditorHandler.js | 1 + .../Subscription/LimitationsManager.js | 5 +- services/web/app/src/models/Project.js | 1 + .../web/frontend/extracted-translations.json | 1 + .../components/edit-member.tsx | 18 ++- .../components/share-modal-body.tsx | 21 +++- .../components/share-project-modal.tsx | 9 +- .../js/shared/context/editor-context.tsx | 6 +- .../shared/context/types/project-context.tsx | 1 + services/web/locales/en.json | 1 + .../web/test/acceptance/src/helpers/User.mjs | 4 + .../Collaborators/CollaboratorsGetterTests.js | 15 ++- .../CollaboratorsHandlerTests.js | 108 ++++++++++++++++-- .../CollaboratorsInviteHandlerTests.mjs | 2 +- 17 files changed, 268 insertions(+), 63 deletions(-) diff --git a/services/web/app/src/Features/Collaborators/CollaboratorsGetter.js b/services/web/app/src/Features/Collaborators/CollaboratorsGetter.js index 663b1bc654..77fb7ab2d3 100644 --- a/services/web/app/src/Features/Collaborators/CollaboratorsGetter.js +++ b/services/web/app/src/Features/Collaborators/CollaboratorsGetter.js @@ -60,6 +60,7 @@ async function getMemberIdsWithPrivilegeLevels(projectId) { publicAccesLevel: 1, pendingEditor_refs: 1, reviewer_refs: 1, + pendingReviewer_refs: 1, }) if (!project) { throw new Errors.NotFoundError(`no project found with id ${projectId}`) @@ -72,7 +73,8 @@ async function getMemberIdsWithPrivilegeLevels(projectId) { project.tokenAccessReadOnly_refs, project.publicAccesLevel, project.pendingEditor_refs, - project.reviewer_refs + project.reviewer_refs, + project.pendingReviewer_refs ) return memberIds } @@ -107,7 +109,8 @@ async function getInvitedMembersWithPrivilegeLevelsFromFields( [], null, [], - reviewerIds + reviewerIds, + [] ) return _loadMembers(members) } @@ -139,13 +142,14 @@ async function getInvitedEditCollaboratorCount(projectId) { } async function getInvitedPendingEditorCount(projectId) { - // Only counts invited members that are readonly pending editors + // Only counts invited members that are readonly pending editors or pending + // reviewers const members = await getMemberIdsWithPrivilegeLevels(projectId) return members.filter( m => m.source === Sources.INVITE && m.privilegeLevel === PrivilegeLevels.READ_ONLY && - m.pendingEditor === true + (m.pendingEditor || m.pendingReviewer) ).length } @@ -320,7 +324,8 @@ function _getMemberIdsWithPrivilegeLevelsFromFields( tokenAccessReadOnlyIds, publicAccessLevel, pendingEditorIds, - reviewerIds + reviewerIds, + pendingReviewerIds ) { const members = [] members.push({ @@ -328,6 +333,7 @@ function _getMemberIdsWithPrivilegeLevelsFromFields( privilegeLevel: PrivilegeLevels.OWNER, source: Sources.OWNER, }) + for (const memberId of collaboratorIds || []) { members.push({ id: memberId.toString(), @@ -335,16 +341,22 @@ function _getMemberIdsWithPrivilegeLevelsFromFields( source: Sources.INVITE, }) } + for (const memberId of readOnlyIds || []) { - members.push({ + const record = { id: memberId.toString(), privilegeLevel: PrivilegeLevels.READ_ONLY, source: Sources.INVITE, - ...(pendingEditorIds?.some(pe => memberId.equals(pe)) && { - pendingEditor: true, - }), - }) + } + + if (pendingEditorIds?.some(pe => memberId.equals(pe))) { + record.pendingEditor = true + } else if (pendingReviewerIds?.some(pr => memberId.equals(pr))) { + record.pendingReviewer = true + } + members.push(record) } + if (publicAccessLevel === PublicAccessLevels.TOKEN_BASED) { for (const memberId of tokenAccessIds || []) { members.push({ @@ -361,6 +373,7 @@ function _getMemberIdsWithPrivilegeLevelsFromFields( }) } } + for (const memberId of reviewerIds || []) { members.push({ id: memberId.toString(), @@ -385,11 +398,16 @@ async function _loadMembers(members) { signUpDate: 1, }) if (user != null) { - return { + const record = { user, privilegeLevel: member.privilegeLevel, - ...(member.pendingEditor && { pendingEditor: true }), } + if (member.pendingEditor) { + record.pendingEditor = true + } else if (member.pendingReviewer) { + record.pendingReviewer = true + } + return record } else { return null } diff --git a/services/web/app/src/Features/Collaborators/CollaboratorsHandler.js b/services/web/app/src/Features/Collaborators/CollaboratorsHandler.js index 00ec34022c..752a87a580 100644 --- a/services/web/app/src/Features/Collaborators/CollaboratorsHandler.js +++ b/services/web/app/src/Features/Collaborators/CollaboratorsHandler.js @@ -53,6 +53,7 @@ async function removeUserFromProject(projectId, userId) { reviewer_refs: userId, readOnly_refs: userId, pendingEditor_refs: userId, + pendingReviewer_refs: userId, tokenAccessReadOnly_refs: userId, tokenAccessReadAndWrite_refs: userId, trashed: userId, @@ -68,6 +69,7 @@ async function removeUserFromProject(projectId, userId) { readOnly_refs: userId, reviewer_refs: userId, pendingEditor_refs: userId, + pendingReviewer_refs: userId, tokenAccessReadOnly_refs: userId, tokenAccessReadAndWrite_refs: userId, archived: userId, @@ -106,7 +108,7 @@ async function addUserIdToProject( addingUserId, userId, privilegeLevel, - { pendingEditor } = {} + { pendingEditor, pendingReviewer } = {} ) { const project = await ProjectGetter.promises.getProject(projectId, { owner_ref: 1, @@ -133,9 +135,17 @@ async function addUserIdToProject( level = { readOnly_refs: userId } if (pendingEditor) { level.pendingEditor_refs = userId + } else if (pendingReviewer) { + level.pendingReviewer_refs = userId } logger.debug( - { privileges: 'readOnly', userId, projectId, pendingEditor }, + { + privileges: 'readOnly', + userId, + projectId, + pendingEditor, + pendingReviewer, + }, 'adding user' ) } else if (privilegeLevel === PrivilegeLevels.REVIEW) { @@ -246,6 +256,19 @@ async function transferProjects(fromUserId, toUserId) { } ).exec() + await Project.updateMany( + { pendingReviewer_refs: fromUserId }, + { + $addToSet: { pendingReviewer_refs: toUserId }, + } + ).exec() + await Project.updateMany( + { pendingReviewer_refs: fromUserId }, + { + $pull: { pendingReviewer_refs: fromUserId }, + } + ).exec() + // Flush in background, no need to block on this _flushProjects(projectIds).catch(err => { logger.err( @@ -259,7 +282,7 @@ async function setCollaboratorPrivilegeLevel( projectId, userId, privilegeLevel, - { pendingEditor } = {} + { pendingEditor, pendingReviewer } = {} ) { // Make sure we're only updating the project if the user is already a // collaborator @@ -279,6 +302,7 @@ async function setCollaboratorPrivilegeLevel( readOnly_refs: userId, pendingEditor_refs: userId, reviewer_refs: userId, + pendingReviewer_refs: userId, }, $addToSet: { collaberator_refs: userId }, } @@ -290,6 +314,7 @@ async function setCollaboratorPrivilegeLevel( readOnly_refs: userId, pendingEditor_refs: userId, collaberator_refs: userId, + pendingReviewer_refs: userId, }, $addToSet: { reviewer_refs: userId }, } @@ -316,11 +341,19 @@ async function setCollaboratorPrivilegeLevel( $pull: { collaberator_refs: userId, reviewer_refs: userId }, $addToSet: { readOnly_refs: userId }, } + if (pendingEditor) { update.$addToSet.pendingEditor_refs = userId } else { update.$pull.pendingEditor_refs = userId } + + if (pendingReviewer) { + update.$addToSet.pendingReviewer_refs = userId + } else { + update.$pull.pendingReviewer_refs = userId + } + break } default: { diff --git a/services/web/app/src/Features/Collaborators/CollaboratorsInviteHandler.mjs b/services/web/app/src/Features/Collaborators/CollaboratorsInviteHandler.mjs index 1409b6e43b..02db4dee99 100644 --- a/services/web/app/src/Features/Collaborators/CollaboratorsInviteHandler.mjs +++ b/services/web/app/src/Features/Collaborators/CollaboratorsInviteHandler.mjs @@ -152,33 +152,52 @@ const CollaboratorsInviteHandler = { const project = await ProjectGetter.promises.getProject(projectId, { owner_ref: 1, }) - const pendingEditor = - invite.privileges === PrivilegeLevels.READ_AND_WRITE && - !(await LimitationsManager.promises.canAcceptEditCollaboratorInvite( - project._id - )) - if (pendingEditor) { - logger.debug( - { projectId, userId: user._id }, - 'no collaborator slots available, user added as read only (pending editor)' + + let privilegeLevel = invite.privileges + const opts = {} + if ( + [PrivilegeLevels.READ_AND_WRITE, PrivilegeLevels.REVIEW].includes( + invite.privileges ) - await ProjectAuditLogHandler.promises.addEntry( - projectId, - 'editor-moved-to-pending', // controller already logged accept-invite - null, - null, - { - userId: user._id.toString(), + ) { + const allowed = + await LimitationsManager.promises.canAcceptEditCollaboratorInvite( + project._id + ) + if (!allowed) { + privilegeLevel = PrivilegeLevels.READ_ONLY + if (invite.privileges === PrivilegeLevels.READ_AND_WRITE) { + opts.pendingEditor = true + } else if (invite.privileges === PrivilegeLevels.REVIEW) { + opts.pendingReviewer = true } - ) + + logger.debug( + { projectId, userId: user._id, privileges: invite.privileges }, + 'no collaborator slots available, user added as read only (pending editor)' + ) + await ProjectAuditLogHandler.promises.addEntry( + projectId, + 'editor-moved-to-pending', // controller already logged accept-invite + null, + null, + { + userId: user._id.toString(), + role: + invite.privileges === PrivilegeLevels.REVIEW + ? 'reviewer' + : 'editor', + } + ) + } } await CollaboratorsHandler.promises.addUserIdToProject( projectId, invite.sendingUserId, user._id, - pendingEditor ? PrivilegeLevels.READ_ONLY : invite.privileges, - { pendingEditor } + privilegeLevel, + opts ) // Remove invite diff --git a/services/web/app/src/Features/Project/ProjectEditorHandler.js b/services/web/app/src/Features/Project/ProjectEditorHandler.js index 84c0a5831a..a85e8b5764 100644 --- a/services/web/app/src/Features/Project/ProjectEditorHandler.js +++ b/services/web/app/src/Features/Project/ProjectEditorHandler.js @@ -107,6 +107,7 @@ module.exports = ProjectEditorHandler = { privileges: member.privilegeLevel, signUpDate: user.signUpDate, pendingEditor: member.pendingEditor, + pendingReviewer: member.pendingReviewer, } }, diff --git a/services/web/app/src/Features/Subscription/LimitationsManager.js b/services/web/app/src/Features/Subscription/LimitationsManager.js index 1fc85434a8..d0c3d29b7b 100644 --- a/services/web/app/src/Features/Subscription/LimitationsManager.js +++ b/services/web/app/src/Features/Subscription/LimitationsManager.js @@ -84,9 +84,8 @@ async function canChangeCollaboratorPrivilegeLevel( projectId ) if ( - [PrivilegeLevels.READ_AND_WRITE, PrivilegeLevels.REVIEW].includes( - currentPrivilegeLevel - ) + currentPrivilegeLevel === PrivilegeLevels.READ_AND_WRITE || + currentPrivilegeLevel === PrivilegeLevels.REVIEW ) { // Current collaborator already takes a slot, so changing the privilege // level won't increase the collaborator count diff --git a/services/web/app/src/models/Project.js b/services/web/app/src/models/Project.js index 1555b471a4..8da4b888d3 100644 --- a/services/web/app/src/models/Project.js +++ b/services/web/app/src/models/Project.js @@ -41,6 +41,7 @@ const ProjectSchema = new Schema( reviewer_refs: [{ type: ObjectId, ref: 'User' }], readOnly_refs: [{ type: ObjectId, ref: 'User' }], pendingEditor_refs: [{ type: ObjectId, ref: 'User' }], + pendingReviewer_refs: [{ type: ObjectId, ref: 'User' }], rootDoc_id: { type: ObjectId }, rootFolder: [FolderSchema], mainBibliographyDoc_id: { type: ObjectId }, diff --git a/services/web/frontend/extracted-translations.json b/services/web/frontend/extracted-translations.json index d3b2b41c8b..3fa667ac8e 100644 --- a/services/web/frontend/extracted-translations.json +++ b/services/web/frontend/extracted-translations.json @@ -1975,6 +1975,7 @@ "view_more": "", "view_only_access": "", "view_only_downgraded": "", + "view_only_reviewer_downgraded": "", "view_options": "", "view_pdf": "", "view_your_invoices": "", diff --git a/services/web/frontend/js/features/share-project-modal/components/edit-member.tsx b/services/web/frontend/js/features/share-project-modal/components/edit-member.tsx index f4f309ae75..433d16d9c7 100644 --- a/services/web/frontend/js/features/share-project-modal/components/edit-member.tsx +++ b/services/web/frontend/js/features/share-project-modal/components/edit-member.tsx @@ -73,7 +73,10 @@ export default function EditMember({ } function shouldWarnMember() { - return hasExceededCollaboratorLimit && privileges === 'readAndWrite' + return ( + hasExceededCollaboratorLimit && + ['readAndWrite', 'review'].includes(privileges) + ) } function commitPrivilegeChange(newPrivileges: PermissionsOption) { @@ -145,12 +148,16 @@ export default function EditMember({
{t('view_only_downgraded')}
)} + {member.pendingReviewer && ( +
+ {t('view_only_reviewer_downgraded')} +
+ )} {shouldWarnMember() && (
{t('will_lose_edit_access_on_date', { diff --git a/services/web/frontend/js/features/share-project-modal/components/share-modal-body.tsx b/services/web/frontend/js/features/share-project-modal/components/share-modal-body.tsx index 1dc6689514..500834d30d 100644 --- a/services/web/frontend/js/features/share-project-modal/components/share-modal-body.tsx +++ b/services/web/frontend/js/features/share-project-modal/components/share-modal-body.tsx @@ -41,8 +41,10 @@ export default function ShareModalBody() { // for moving between warning and info notification states etc. const somePendingEditorsResolved = useMemo(() => { return ( - members.some(member => member.privileges === 'readAndWrite') && - members.some(member => member.pendingEditor) + members.some(member => + ['readAndWrite', 'review'].includes(member.privileges) + ) && + members.some(member => member.pendingEditor || member.pendingReviewer) ) }, [members]) @@ -54,7 +56,9 @@ export default function ShareModalBody() { if (features.collaborators === -1) { return false } - return members.some(member => member.pendingEditor) + return members.some( + member => member.pendingEditor || member.pendingReviewer + ) }, [features, isProjectOwner, members]) const hasExceededCollaboratorLimit = useMemo(() => { @@ -76,8 +80,13 @@ export default function ShareModalBody() { return [ ...members.filter(member => member.privileges === 'readAndWrite'), ...members.filter(member => member.pendingEditor), + ...members.filter(member => member.privileges === 'review'), + ...members.filter(member => member.pendingReviewer), ...members.filter( - member => !member.pendingEditor && member.privileges !== 'readAndWrite' + member => + !member.pendingEditor && + !member.pendingReviewer && + !['readAndWrite', 'review'].includes(member.privileges) ), ] }, [members]) @@ -104,7 +113,9 @@ export default function ShareModalBody() { key={member._id} member={member} hasExceededCollaboratorLimit={hasExceededCollaboratorLimit} - hasBeenDowngraded={member.pendingEditor ?? false} + hasBeenDowngraded={Boolean( + member.pendingEditor || member.pendingReviewer + )} canAddCollaborators={canAddCollaborators} /> ) : ( diff --git a/services/web/frontend/js/features/share-project-modal/components/share-project-modal.tsx b/services/web/frontend/js/features/share-project-modal/components/share-project-modal.tsx index ca2756fa14..7a4139e2de 100644 --- a/services/web/frontend/js/features/share-project-modal/components/share-project-modal.tsx +++ b/services/web/frontend/js/features/share-project-modal/components/share-project-modal.tsx @@ -78,9 +78,12 @@ const ShareProjectModal = React.memo(function ShareProjectModal({ return false } return ( - project.members.filter(member => member.privileges === 'readAndWrite') - .length > (project.features.collaborators ?? 1) || - project.members.some(member => member.pendingEditor) + project.members.filter(member => + ['readAndWrite', 'review'].includes(member.privileges) + ).length > (project.features.collaborators ?? 1) || + project.members.some( + member => member.pendingEditor || member.pendingReviewer + ) ) } diff --git a/services/web/frontend/js/shared/context/editor-context.tsx b/services/web/frontend/js/shared/context/editor-context.tsx index d514db29d3..f059f14319 100644 --- a/services/web/frontend/js/shared/context/editor-context.tsx +++ b/services/web/frontend/js/shared/context/editor-context.tsx @@ -116,7 +116,11 @@ export const EditorProvider: FC = ({ children }) => { const isPendingEditor = useMemo( () => - members?.some(member => member._id === userId && member.pendingEditor), + members?.some( + member => + member._id === userId && + (member.pendingEditor || member.pendingReviewer) + ), [members, userId] ) diff --git a/services/web/frontend/js/shared/context/types/project-context.tsx b/services/web/frontend/js/shared/context/types/project-context.tsx index 53911afb26..91419ed06f 100644 --- a/services/web/frontend/js/shared/context/types/project-context.tsx +++ b/services/web/frontend/js/shared/context/types/project-context.tsx @@ -9,6 +9,7 @@ export type ProjectContextMember = { first_name: string last_name: string pendingEditor?: boolean + pendingReviewer?: boolean } export type ProjectContextValue = { diff --git a/services/web/locales/en.json b/services/web/locales/en.json index 5b869d6b6b..7f2a66a285 100644 --- a/services/web/locales/en.json +++ b/services/web/locales/en.json @@ -2522,6 +2522,7 @@ "view_more": "View more", "view_only_access": "View-only access", "view_only_downgraded": "View only. Upgrade to restore edit access.", + "view_only_reviewer_downgraded": "View only. Upgrade to restore review access.", "view_options": "View options", "view_pdf": "View PDF", "view_source": "View Source", diff --git a/services/web/test/acceptance/src/helpers/User.mjs b/services/web/test/acceptance/src/helpers/User.mjs index 51a6fc5721..5f8ac2903f 100644 --- a/services/web/test/acceptance/src/helpers/User.mjs +++ b/services/web/test/acceptance/src/helpers/User.mjs @@ -992,6 +992,10 @@ class User { updateOp = { $addToSet: { readOnly_refs: user._id, pendingEditor_refs: user._id }, } + } else if (privileges === 'pendingReviewer') { + updateOp = { + $addToSet: { readOnly_refs: user._id, pendingReviewer_refs: user._id }, + } } else if (privileges === 'review') { updateOp = { $addToSet: { reviewer_refs: user._id }, diff --git a/services/web/test/unit/src/Collaborators/CollaboratorsGetterTests.js b/services/web/test/unit/src/Collaborators/CollaboratorsGetterTests.js index 5135dea412..7bfbc1c423 100644 --- a/services/web/test/unit/src/Collaborators/CollaboratorsGetterTests.js +++ b/services/web/test/unit/src/Collaborators/CollaboratorsGetterTests.js @@ -18,6 +18,7 @@ describe('CollaboratorsGetter', function () { this.readOnlyRef1 = new ObjectId() this.readOnlyRef2 = new ObjectId() this.pendingEditorRef = new ObjectId() + this.pendingReviewerRef = new ObjectId() this.readWriteRef1 = new ObjectId() this.readWriteRef2 = new ObjectId() this.reviewer1Ref = new ObjectId() @@ -32,8 +33,10 @@ describe('CollaboratorsGetter', function () { this.readOnlyRef1, this.readOnlyRef2, this.pendingEditorRef, + this.pendingReviewerRef, ], pendingEditor_refs: [this.pendingEditorRef], + pendingReviewer_refs: [this.pendingReviewerRef], collaberator_refs: [this.readWriteRef1, this.readWriteRef2], reviewer_refs: [this.reviewer1Ref, this.reviewer2Ref], tokenAccessReadAndWrite_refs: [this.readWriteTokenRef], @@ -114,6 +117,12 @@ describe('CollaboratorsGetter', function () { source: 'invite', pendingEditor: true, }, + { + id: this.pendingReviewerRef.toString(), + privilegeLevel: 'readOnly', + source: 'invite', + pendingReviewer: true, + }, { id: this.readOnlyTokenRef.toString(), privilegeLevel: 'readOnly', @@ -165,6 +174,7 @@ describe('CollaboratorsGetter', function () { this.readWriteRef1.toString(), this.readWriteRef2.toString(), this.pendingEditorRef.toString(), + this.pendingReviewerRef.toString(), this.readWriteTokenRef.toString(), this.readOnlyTokenRef.toString(), this.reviewer1Ref.toString(), @@ -186,6 +196,7 @@ describe('CollaboratorsGetter', function () { this.readWriteRef1.toString(), this.readWriteRef2.toString(), this.pendingEditorRef.toString(), + this.pendingReviewerRef.toString(), this.reviewer1Ref.toString(), this.reviewer2Ref.toString(), ]) @@ -545,12 +556,12 @@ describe('CollaboratorsGetter', function () { }) describe('getInvitedPendingEditorCount', function () { - it('should return the count of pending editors', async function () { + it('should return the count of pending editors and reviewers', async function () { const count = await this.CollaboratorsGetter.promises.getInvitedPendingEditorCount( this.project._id ) - expect(count).to.equal(1) + expect(count).to.equal(2) }) }) }) diff --git a/services/web/test/unit/src/Collaborators/CollaboratorsHandlerTests.js b/services/web/test/unit/src/Collaborators/CollaboratorsHandlerTests.js index caca5b0145..734c474f86 100644 --- a/services/web/test/unit/src/Collaborators/CollaboratorsHandlerTests.js +++ b/services/web/test/unit/src/Collaborators/CollaboratorsHandlerTests.js @@ -111,6 +111,7 @@ describe('CollaboratorsHandler', function () { reviewer_refs: this.userId, readOnly_refs: this.userId, pendingEditor_refs: this.userId, + pendingReviewer_refs: this.userId, tokenAccessReadOnly_refs: this.userId, tokenAccessReadAndWrite_refs: this.userId, archived: this.userId, @@ -155,6 +156,7 @@ describe('CollaboratorsHandler', function () { reviewer_refs: this.userId, readOnly_refs: this.userId, pendingEditor_refs: this.userId, + pendingReviewer_refs: this.userId, tokenAccessReadOnly_refs: this.userId, tokenAccessReadAndWrite_refs: this.userId, trashed: this.userId, @@ -191,6 +193,7 @@ describe('CollaboratorsHandler', function () { reviewer_refs: this.userId, readOnly_refs: this.userId, pendingEditor_refs: this.userId, + pendingReviewer_refs: this.userId, tokenAccessReadOnly_refs: this.userId, tokenAccessReadAndWrite_refs: this.userId, archived: this.userId, @@ -278,6 +281,32 @@ describe('CollaboratorsHandler', function () { ) }) }) + + describe('with pendingReviewer flag', function () { + it('should add them to the pending reviewer refs', async function () { + this.ProjectMock.expects('updateOne') + .withArgs( + { + _id: this.project._id, + }, + { + $addToSet: { + readOnly_refs: this.userId, + pendingReviewer_refs: this.userId, + }, + } + ) + .chain('exec') + .resolves() + await this.CollaboratorsHandler.promises.addUserIdToProject( + this.project._id, + this.addingUserId, + this.userId, + 'readOnly', + { pendingReviewer: true } + ) + }) + }) }) describe('as readAndWrite', function () { @@ -451,6 +480,7 @@ describe('CollaboratorsHandler', function () { reviewer_refs: this.userId, readOnly_refs: this.userId, pendingEditor_refs: this.userId, + pendingReviewer_refs: this.userId, tokenAccessReadOnly_refs: this.userId, tokenAccessReadAndWrite_refs: this.userId, archived: this.userId, @@ -549,6 +579,24 @@ describe('CollaboratorsHandler', function () { ) .chain('exec') .resolves() + this.ProjectMock.expects('updateMany') + .withArgs( + { pendingReviewer_refs: this.fromUserId }, + { + $addToSet: { pendingReviewer_refs: this.toUserId }, + } + ) + .chain('exec') + .resolves() + this.ProjectMock.expects('updateMany') + .withArgs( + { pendingReviewer_refs: this.fromUserId }, + { + $pull: { pendingReviewer_refs: this.fromUserId }, + } + ) + .chain('exec') + .resolves() }) describe('successfully', function () { @@ -586,7 +634,7 @@ describe('CollaboratorsHandler', function () { this.ProjectMock.expects('updateOne') .withArgs( { - _id: this.projectId, + _id: this.project._id, $or: [ { collaberator_refs: this.userId }, { readOnly_refs: this.userId }, @@ -597,6 +645,7 @@ describe('CollaboratorsHandler', function () { $pull: { collaberator_refs: this.userId, pendingEditor_refs: this.userId, + pendingReviewer_refs: this.userId, reviewer_refs: this.userId, }, $addToSet: { readOnly_refs: this.userId }, @@ -605,7 +654,7 @@ describe('CollaboratorsHandler', function () { .chain('exec') .resolves({ matchedCount: 1 }) await this.CollaboratorsHandler.promises.setCollaboratorPrivilegeLevel( - this.projectId, + this.project._id, this.userId, 'readOnly' ) @@ -615,7 +664,7 @@ describe('CollaboratorsHandler', function () { this.ProjectMock.expects('updateOne') .withArgs( { - _id: this.projectId, + _id: this.project._id, $or: [ { collaberator_refs: this.userId }, { readOnly_refs: this.userId }, @@ -628,13 +677,14 @@ describe('CollaboratorsHandler', function () { readOnly_refs: this.userId, reviewer_refs: this.userId, pendingEditor_refs: this.userId, + pendingReviewer_refs: this.userId, }, } ) .chain('exec') .resolves({ matchedCount: 1 }) await this.CollaboratorsHandler.promises.setCollaboratorPrivilegeLevel( - this.projectId, + this.project._id, this.userId, 'readAndWrite' ) @@ -653,7 +703,7 @@ describe('CollaboratorsHandler', function () { this.ProjectMock.expects('updateOne') .withArgs( { - _id: this.projectId, + _id: this.project._id, $or: [ { collaberator_refs: this.userId }, { readOnly_refs: this.userId }, @@ -667,13 +717,14 @@ describe('CollaboratorsHandler', function () { readOnly_refs: this.userId, collaberator_refs: this.userId, pendingEditor_refs: this.userId, + pendingReviewer_refs: this.userId, }, } ) .chain('exec') .resolves({ matchedCount: 1 }) await this.CollaboratorsHandler.promises.setCollaboratorPrivilegeLevel( - this.projectId, + this.project._id, this.userId, 'review' ) @@ -695,7 +746,7 @@ describe('CollaboratorsHandler', function () { this.ProjectMock.expects('updateOne') .withArgs( { - _id: this.projectId, + _id: this.project._id, $or: [ { collaberator_refs: this.userId }, { readOnly_refs: this.userId }, @@ -709,13 +760,14 @@ describe('CollaboratorsHandler', function () { readOnly_refs: this.userId, collaberator_refs: this.userId, pendingEditor_refs: this.userId, + pendingReviewer_refs: this.userId, }, } ) .chain('exec') .resolves({ matchedCount: 1 }) await this.CollaboratorsHandler.promises.setCollaboratorPrivilegeLevel( - this.projectId, + this.project._id, this.userId, 'review' ) @@ -726,7 +778,7 @@ describe('CollaboratorsHandler', function () { this.ProjectMock.expects('updateOne') .withArgs( { - _id: this.projectId, + _id: this.project._id, $or: [ { collaberator_refs: this.userId }, { readOnly_refs: this.userId }, @@ -741,26 +793,60 @@ describe('CollaboratorsHandler', function () { $pull: { collaberator_refs: this.userId, reviewer_refs: this.userId, + pendingReviewer_refs: this.userId, }, } ) .chain('exec') .resolves({ matchedCount: 1 }) await this.CollaboratorsHandler.promises.setCollaboratorPrivilegeLevel( - this.projectId, + this.project._id, this.userId, 'readOnly', { pendingEditor: true } ) }) + it('sets a collaborator to read-only as a pendingReviewer', async function () { + this.ProjectMock.expects('updateOne') + .withArgs( + { + _id: this.project._id, + $or: [ + { collaberator_refs: this.userId }, + { readOnly_refs: this.userId }, + { reviewer_refs: this.userId }, + ], + }, + { + $addToSet: { + readOnly_refs: this.userId, + pendingReviewer_refs: this.userId, + }, + $pull: { + collaberator_refs: this.userId, + reviewer_refs: this.userId, + pendingEditor_refs: this.userId, + }, + } + ) + .chain('exec') + .resolves({ matchedCount: 1 }) + await this.CollaboratorsHandler.promises.setCollaboratorPrivilegeLevel( + this.project._id, + this.userId, + 'readOnly', + { pendingReviewer: true } + ) + }) + it('throws a NotFoundError if the project or collaborator does not exist', async function () { this.ProjectMock.expects('updateOne') .chain('exec') .resolves({ matchedCount: 0 }) await expect( this.CollaboratorsHandler.promises.setCollaboratorPrivilegeLevel( - this.projectId, + this.project._id, this.userId, 'readAndWrite' ) diff --git a/services/web/test/unit/src/Collaborators/CollaboratorsInviteHandlerTests.mjs b/services/web/test/unit/src/Collaborators/CollaboratorsInviteHandlerTests.mjs index 1b2caa7fc3..f386648552 100644 --- a/services/web/test/unit/src/Collaborators/CollaboratorsInviteHandlerTests.mjs +++ b/services/web/test/unit/src/Collaborators/CollaboratorsInviteHandlerTests.mjs @@ -561,7 +561,7 @@ describe('CollaboratorsInviteHandler', function () { 'editor-moved-to-pending', null, null, - { userId: this.userId.toString() } + { userId: this.userId.toString(), role: 'editor' } ) this.CollaboratorsHandler.promises.addUserIdToProject.should.have.been.calledWith( this.projectId,