Merge pull request #24433 from overleaf/em-pending-reviewers

Support reviewers in the collaborator limit enforcement logic

GitOrigin-RevId: f11a8e37ca6ef36f9894233803c6ee8363bf0ff8

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 },

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)
     })
   })
 })

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'
         )

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,