Revert "Support for adding reviewer role (#22137)" (#22293)

This reverts commit 54064a7f961fe06f188ab449cd469cdaaf01b20a.

GitOrigin-RevId: 1dabc635756aff465d2fe9aa0d18121b081df3b0

services/web/test/acceptance/src/SharingTests.mjs

@@ -8,15 +8,12 @@ describe('Sharing', function () {
     this.ownerSession = new User()
     this.collaboratorSession = new User()
     this.strangerSession = new User()
-    this.reviewerSession = new User()
     await this.ownerSession.login()
     await this.collaboratorSession.login()
     await this.strangerSession.login()
-    await this.reviewerSession.login()
     this.owner = await this.ownerSession.get()
     this.collaborator = await this.collaboratorSession.get()
     this.stranger = await this.strangerSession.get()
-    this.reviewer = await this.reviewerSession.get()
     this.projectId = await this.ownerSession.createProject('Test project')
   })
 
@@ -38,19 +35,6 @@ describe('Sharing', function () {
       const project = await this.ownerSession.getProject(this.projectId)
       expect(project.collaberator_refs).to.deep.equal([this.collaborator._id])
       expect(project.readOnly_refs).to.deep.equal([])
-      expect(project.reviewer_refs).to.deep.equal([])
-    })
-
-    it('sets the privilege level to review', async function () {
-      await this.ownerSession.setCollaboratorInfo(
-        this.projectId,
-        this.collaborator._id,
-        { privilegeLevel: 'review' }
-      )
-      const project = await this.ownerSession.getProject(this.projectId)
-      expect(project.reviewer_refs).to.deep.equal([this.collaborator._id])
-      expect(project.collaberator_refs).to.deep.equal([])
-      expect(project.readOnly_refs).to.deep.equal([])
     })
 
     it('treats setting the privilege to read-only as a noop', async function () {
@@ -61,7 +45,6 @@ describe('Sharing', function () {
       )
       const project = await this.ownerSession.getProject(this.projectId)
       expect(project.collaberator_refs).to.deep.equal([])
-      expect(project.reviewer_refs).to.deep.equal([])
       expect(project.readOnly_refs).to.deep.equal([this.collaborator._id])
     })
 
@@ -113,40 +96,7 @@ describe('Sharing', function () {
       )
       const project = await this.ownerSession.getProject(this.projectId)
       expect(project.collaberator_refs).to.deep.equal([])
-      expect(project.reviewer_refs).to.deep.equal([])
       expect(project.readOnly_refs).to.deep.equal([this.collaborator._id])
     })
   })
-
-  describe('with reviewer collaborator', function () {
-    beforeEach(async function () {
-      await this.ownerSession.addUserToProject(
-        this.projectId,
-        this.reviewer,
-        'review'
-      )
-    })
-
-    it('prevents non-owners to set the privilege level', async function () {
-      await expect(
-        this.collaboratorSession.setCollaboratorInfo(
-          this.projectId,
-          this.reviewer._id,
-          { privilegeLevel: 'review' }
-        )
-      ).to.be.rejectedWith(/failed: status=403 /)
-    })
-
-    it('sets the privilege level to read-only', async function () {
-      await this.ownerSession.setCollaboratorInfo(
-        this.projectId,
-        this.reviewer._id,
-        { privilegeLevel: 'readOnly' }
-      )
-      const project = await this.ownerSession.getProject(this.projectId)
-      expect(project.collaberator_refs).to.deep.equal([])
-      expect(project.reviewer_refs).to.deep.equal([])
-      expect(project.readOnly_refs).to.deep.equal([this.reviewer._id])
-    })
-  })
 })

services/web/test/acceptance/src/helpers/User.js

@@ -968,10 +968,6 @@ class User {
       updateOp = {
         $addToSet: { readOnly_refs: user._id, pendingEditor_refs: user._id },
       }
-    } else if (privileges === 'review') {
-      updateOp = {
-        $addToSet: { reviewer_refs: user._id },
-      }
     }
     db.projects.updateOne({ _id: new ObjectId(projectId) }, updateOp, callback)
   }

services/web/test/unit/src/Collaborators/CollaboratorsGetterTests.js

@@ -20,8 +20,6 @@ describe('CollaboratorsGetter', function () {
     this.pendingEditorRef = new ObjectId()
     this.readWriteRef1 = new ObjectId()
     this.readWriteRef2 = new ObjectId()
-    this.reviewer1Ref = new ObjectId()
-    this.reviewer2Ref = new ObjectId()
     this.readOnlyTokenRef = new ObjectId()
     this.readWriteTokenRef = new ObjectId()
     this.nonMemberRef = new ObjectId()
@@ -35,7 +33,6 @@ describe('CollaboratorsGetter', function () {
       ],
       pendingEditor_refs: [this.pendingEditorRef],
       collaberator_refs: [this.readWriteRef1, this.readWriteRef2],
-      reviewer_refs: [this.reviewer1Ref, this.reviewer2Ref],
       tokenAccessReadAndWrite_refs: [this.readWriteTokenRef],
       tokenAccessReadOnly_refs: [this.readOnlyTokenRef],
       publicAccesLevel: 'tokenBased',
@@ -124,16 +121,6 @@ describe('CollaboratorsGetter', function () {
             privilegeLevel: 'readAndWrite',
             source: 'token',
           },
-          {
-            id: this.reviewer1Ref.toString(),
-            privilegeLevel: 'review',
-            source: 'invite',
-          },
-          {
-            id: this.reviewer2Ref.toString(),
-            privilegeLevel: 'review',
-            source: 'invite',
-          },
         ])
       })
     })
@@ -167,8 +154,6 @@ describe('CollaboratorsGetter', function () {
         this.pendingEditorRef.toString(),
         this.readWriteTokenRef.toString(),
         this.readOnlyTokenRef.toString(),
-        this.reviewer1Ref.toString(),
-        this.reviewer2Ref.toString(),
       ])
     })
   })
@@ -186,8 +171,6 @@ describe('CollaboratorsGetter', function () {
         this.readWriteRef1.toString(),
         this.readWriteRef2.toString(),
         this.pendingEditorRef.toString(),
-        this.reviewer1Ref.toString(),
-        this.reviewer2Ref.toString(),
       ])
     })
   })
@@ -206,9 +189,6 @@ describe('CollaboratorsGetter', function () {
       this.UserGetter.promises.getUser
         .withArgs(this.readWriteTokenRef.toString())
         .resolves({ _id: this.readWriteTokenRef })
-      this.UserGetter.promises.getUser
-        .withArgs(this.reviewer1Ref.toString())
-        .resolves({ _id: this.reviewer1Ref })
     })
 
     it('should return an array of invited members with their privilege levels', async function () {
@@ -219,7 +199,6 @@ describe('CollaboratorsGetter', function () {
       expect(result).to.have.deep.members([
         { user: { _id: this.readOnlyRef1 }, privilegeLevel: 'readOnly' },
         { user: { _id: this.readWriteRef2 }, privilegeLevel: 'readAndWrite' },
-        { user: { _id: this.reviewer1Ref }, privilegeLevel: 'review' },
       ])
     })
   })
@@ -234,15 +213,6 @@ describe('CollaboratorsGetter', function () {
       expect(level).to.equal('readOnly')
     })
 
-    it('should return review privilege level', async function () {
-      const level =
-        await this.CollaboratorsGetter.promises.getMemberIdPrivilegeLevel(
-          this.reviewer1Ref,
-          this.project._id
-        )
-      expect(level).to.equal('review')
-    })
-
     it('should return false if the member has no privilege level', async function () {
       const level =
         await this.CollaboratorsGetter.promises.getMemberIdPrivilegeLevel(
@@ -319,11 +289,6 @@ describe('CollaboratorsGetter', function () {
         .withArgs({ readOnly_refs: this.userId }, this.fields)
         .chain('exec')
         .resolves(['mock-read-only-project-1', 'mock-read-only-project-2'])
-
-      this.ProjectMock.expects('find')
-        .withArgs({ reviewer_refs: this.userId }, this.fields)
-        .chain('exec')
-        .resolves(['mock-review-project-1', 'mock-review-project-2'])
       this.ProjectMock.expects('find')
         .withArgs(
           {
@@ -372,7 +337,6 @@ describe('CollaboratorsGetter', function () {
           'mock-token-read-only-project-1',
           'mock-token-read-only-project-2',
         ],
-        review: ['mock-review-project-1', 'mock-review-project-2'],
       })
     })
   })
@@ -388,21 +352,15 @@ describe('CollaboratorsGetter', function () {
         _id: this.readWriteRef1,
         email: 'readwrite@example.com',
       }
-      this.reviewUser = {
-        _id: this.reviewer1Ref,
-        email: 'review@example.com',
-      }
       this.members = [
         { user: this.owningUser, privilegeLevel: 'owner' },
         { user: this.readWriteUser, privilegeLevel: 'readAndWrite' },
-        { user: this.reviewUser, privilegeLevel: 'review' },
       ]
       this.views = {
         owner: this.owningUser,
         ownerFeatures: this.owningUser.features,
         members: [
           { _id: this.readWriteUser._id, email: this.readWriteUser.email },
-          { _id: this.reviewUser._id, email: this.reviewUser.email },
         ],
       }
       this.UserGetter.promises.getUser
@@ -411,9 +369,6 @@ describe('CollaboratorsGetter', function () {
       this.UserGetter.promises.getUser
         .withArgs(this.readWriteUser._id.toString())
         .resolves(this.readWriteUser)
-      this.UserGetter.promises.getUser
-        .withArgs(this.reviewUser._id.toString())
-        .resolves(this.reviewUser)
       this.ProjectEditorHandler.buildOwnerAndMembersViews.returns(this.views)
       this.result =
         await this.CollaboratorsGetter.promises.getAllInvitedMembers(

services/web/test/unit/src/Collaborators/CollaboratorsHandlerTests.js

@@ -65,10 +65,8 @@ describe('CollaboratorsHandler', function () {
     this.CollaboratorsGetter = {
       promises: {
         dangerouslyGetAllProjectsUserIsMemberOf: sinon.stub(),
-        getMemberIdsWithPrivilegeLevels: sinon.stub().resolves([]),
       },
     }
-    this.EditorRealTimeController = { emitToRoom: sinon.stub() }
     this.CollaboratorsHandler = SandboxedModule.require(MODULE_PATH, {
       requires: {
         '../User/UserGetter': this.UserGetter,
@@ -78,7 +76,6 @@ describe('CollaboratorsHandler', function () {
         '../ThirdPartyDataStore/TpdsUpdateSender': this.TpdsUpdateSender,
         '../Project/ProjectGetter': this.ProjectGetter,
         '../Project/ProjectHelper': this.ProjectHelper,
-        '../Editor/EditorRealTimeController': this.EditorRealTimeController,
         './CollaboratorsGetter': this.CollaboratorsGetter,
       },
     })
@@ -108,7 +105,6 @@ describe('CollaboratorsHandler', function () {
             {
               $pull: {
                 collaberator_refs: this.userId,
-                reviewer_refs: this.userId,
                 readOnly_refs: this.userId,
                 pendingEditor_refs: this.userId,
                 tokenAccessReadOnly_refs: this.userId,
@@ -152,7 +148,6 @@ describe('CollaboratorsHandler', function () {
               },
               $pull: {
                 collaberator_refs: this.userId,
-                reviewer_refs: this.userId,
                 readOnly_refs: this.userId,
                 pendingEditor_refs: this.userId,
                 tokenAccessReadOnly_refs: this.userId,
@@ -188,7 +183,6 @@ describe('CollaboratorsHandler', function () {
             {
               $pull: {
                 collaberator_refs: this.userId,
-                reviewer_refs: this.userId,
                 readOnly_refs: this.userId,
                 pendingEditor_refs: this.userId,
                 tokenAccessReadOnly_refs: this.userId,
@@ -308,43 +302,6 @@ describe('CollaboratorsHandler', function () {
       })
     })
 
-    describe('as reviewer', function () {
-      beforeEach(async function () {
-        this.ProjectMock.expects('updateOne')
-          .withArgs(
-            {
-              _id: this.project._id,
-            },
-            {
-              track_changes: { [this.userId]: true },
-              $addToSet: { reviewer_refs: this.userId },
-            }
-          )
-          .chain('exec')
-          .resolves()
-        await this.CollaboratorsHandler.promises.addUserIdToProject(
-          this.project._id,
-          this.addingUserId,
-          this.userId,
-          'review'
-        )
-      })
-
-      it('should update the client with new track changes settings', function () {
-        return this.EditorRealTimeController.emitToRoom
-          .calledWith(this.project._id, 'toggle-track-changes', {
-            [this.userId]: true,
-          })
-          .should.equal(true)
-      })
-
-      it('should flush the project to the TPDS', function () {
-        expect(
-          this.TpdsProjectFlusher.promises.flushProjectToTpds
-        ).to.have.been.calledWith(this.project._id)
-      })
-    })
-
     describe('with invalid privilegeLevel', function () {
       it('should call the callback with an error', async function () {
         await expect(
@@ -448,7 +405,6 @@ describe('CollaboratorsHandler', function () {
             {
               $pull: {
                 collaberator_refs: this.userId,
-                reviewer_refs: this.userId,
                 readOnly_refs: this.userId,
                 pendingEditor_refs: this.userId,
                 tokenAccessReadOnly_refs: this.userId,
@@ -590,14 +546,12 @@ describe('CollaboratorsHandler', function () {
             $or: [
               { collaberator_refs: this.userId },
               { readOnly_refs: this.userId },
-              { reviewer_refs: this.userId },
             ],
           },
           {
             $pull: {
               collaberator_refs: this.userId,
               pendingEditor_refs: this.userId,
-              reviewer_refs: this.userId,
             },
             $addToSet: { readOnly_refs: this.userId },
           }
@@ -619,14 +573,12 @@ describe('CollaboratorsHandler', function () {
             $or: [
               { collaberator_refs: this.userId },
               { readOnly_refs: this.userId },
-              { reviewer_refs: this.userId },
             ],
           },
           {
             $addToSet: { collaberator_refs: this.userId },
             $pull: {
               readOnly_refs: this.userId,
-              reviewer_refs: this.userId,
               pendingEditor_refs: this.userId,
             },
           }
@@ -640,35 +592,6 @@ describe('CollaboratorsHandler', function () {
       )
     })
 
-    it('sets a collaborator to reviewer', async function () {
-      this.ProjectMock.expects('updateOne')
-        .withArgs(
-          {
-            _id: this.projectId,
-            $or: [
-              { collaberator_refs: this.userId },
-              { readOnly_refs: this.userId },
-              { reviewer_refs: this.userId },
-            ],
-          },
-          {
-            $addToSet: { reviewer_refs: this.userId },
-            $pull: {
-              readOnly_refs: this.userId,
-              collaberator_refs: this.userId,
-              pendingEditor_refs: this.userId,
-            },
-          }
-        )
-        .chain('exec')
-        .resolves({ matchedCount: 1 })
-      await this.CollaboratorsHandler.promises.setCollaboratorPrivilegeLevel(
-        this.projectId,
-        this.userId,
-        'review'
-      )
-    })
-
     it('sets a collaborator to read-only as a pendingEditor', async function () {
       this.ProjectMock.expects('updateOne')
         .withArgs(
@@ -677,7 +600,6 @@ describe('CollaboratorsHandler', function () {
             $or: [
               { collaberator_refs: this.userId },
               { readOnly_refs: this.userId },
-              { reviewer_refs: this.userId },
             ],
           },
           {
@@ -687,7 +609,6 @@ describe('CollaboratorsHandler', function () {
             },
             $pull: {
               collaberator_refs: this.userId,
-              reviewer_refs: this.userId,
             },
           }
         )

services/web/test/unit/src/HelperFiles/AuthorizationHelperTests.js

@@ -25,10 +25,6 @@ describe('AuthorizationHelper', function () {
             },
           },
         },
-        '../Project/ProjectGetter': (this.ProjectGetter = { promises: {} }),
-        '../SplitTests/SplitTestHandler': (this.SplitTestHandler = {
-          promises: {},
-        }),
       },
     })
   })
@@ -63,76 +59,4 @@ describe('AuthorizationHelper', function () {
       expect(this.AuthorizationHelper.hasAnyStaffAccess(user)).to.be.false
     })
   })
-
-  describe('isReviewerRoleEnabled', function () {
-    it('with no reviewers and no split test', async function () {
-      this.ProjectGetter.promises.getProject = sinon.stub().resolves({
-        reviewer_refs: {},
-        owner_ref: 'ownerId',
-      })
-      this.SplitTestHandler.promises.getAssignmentForUser = sinon
-        .stub()
-        .resolves({
-          variant: 'disabled',
-        })
-      expect(
-        await this.AuthorizationHelper.promises.isReviewerRoleEnabled(
-          'userId',
-          'projectId'
-        )
-      ).to.be.false
-    })
-
-    it('with no reviewers and enabled split test', async function () {
-      this.ProjectGetter.promises.getProject = sinon.stub().resolves({
-        reviewer_refs: {},
-        owner_ref: 'userId',
-      })
-      this.SplitTestHandler.promises.getAssignmentForUser = sinon
-        .stub()
-        .resolves({
-          variant: 'enabled',
-        })
-      expect(
-        await this.AuthorizationHelper.promises.isReviewerRoleEnabled(
-          'userId',
-          'projectId'
-        )
-      ).to.be.true
-    })
-
-    it('with reviewers and disabled split test', async function () {
-      this.ProjectGetter.promises.getProject = sinon.stub().resolves({
-        reviewer_refs: [{ $oid: 'userId' }],
-      })
-      this.SplitTestHandler.promises.getAssignmentForUser = sinon
-        .stub()
-        .resolves({
-          variant: 'default',
-        })
-      expect(
-        await this.AuthorizationHelper.promises.isReviewerRoleEnabled(
-          'userId',
-          'projectId'
-        )
-      ).to.be.true
-    })
-
-    it('with reviewers and enabled split test', async function () {
-      this.ProjectGetter.promises.getProject = sinon.stub().resolves({
-        reviewer_refs: [{ $oid: 'userId' }],
-      })
-      this.SplitTestHandler.promises.getAssignmentForUser = sinon
-        .stub()
-        .resolves({
-          variant: 'enabled',
-        })
-      expect(
-        await this.AuthorizationHelper.promises.isReviewerRoleEnabled(
-          'userId',
-          'projectId'
-        )
-      ).to.be.true
-    })
-  })
 })

services/web/test/unit/src/Project/ProjectGetterTests.js

@@ -275,7 +275,6 @@ describe('ProjectGetter', function () {
       this.fields = { mock: 'fields' }
       this.projectOwned = { _id: 'mock-owned-projects' }
       this.projectRW = { _id: 'mock-rw-projects' }
-      this.projectReview = { _id: 'mock-review-projects' }
       this.projectRO = { _id: 'mock-ro-projects' }
       this.projectTokenRW = { _id: 'mock-token-rw-projects' }
       this.projectTokenRO = { _id: 'mock-token-ro-projects' }
@@ -290,7 +289,6 @@ describe('ProjectGetter', function () {
         readOnly: [this.projectRO],
         tokenReadAndWrite: [this.projectTokenRW],
         tokenReadOnly: [this.projectTokenRO],
-        review: [this.projectReview],
       })
       const projects = await this.ProjectGetter.promises.findAllUsersProjects(
         this.userId,
@@ -303,7 +301,6 @@ describe('ProjectGetter', function () {
         readOnly: [this.projectRO],
         tokenReadAndWrite: [this.projectTokenRW],
         tokenReadOnly: [this.projectTokenRO],
-        review: [this.projectReview],
       })
     })
 
@@ -317,7 +314,6 @@ describe('ProjectGetter', function () {
           this.projectTokenRO,
           this.projectRO,
         ],
-        review: [this.projectReview],
       })
       const projects = await this.ProjectGetter.promises.findAllUsersProjects(
         this.userId,
@@ -330,7 +326,6 @@ describe('ProjectGetter', function () {
         readOnly: [this.projectRO],
         tokenReadAndWrite: [this.projectTokenRW],
         tokenReadOnly: [this.projectTokenRO],
-        review: [this.projectReview],
       })
     })
   })

services/web/test/unit/src/Project/ProjectListControllerTests.mjs

@@ -219,14 +219,12 @@ describe('ProjectListController', function () {
       this.readOnly = [{ _id: 3, lastUpdated: 3, owner_ref: 'user-1' }]
       this.tokenReadAndWrite = [{ _id: 6, lastUpdated: 5, owner_ref: 'user-4' }]
       this.tokenReadOnly = [{ _id: 7, lastUpdated: 4, owner_ref: 'user-5' }]
-      this.review = [{ _id: 8, lastUpdated: 4, owner_ref: 'user-6' }]
       this.allProjects = {
         owned: this.projects,
         readAndWrite: this.readAndWrite,
         readOnly: this.readOnly,
         tokenReadAndWrite: this.tokenReadAndWrite,
         tokenReadOnly: this.tokenReadOnly,
-        review: this.review,
       }
 
       this.ProjectGetter.promises.findAllUsersProjects.resolves(
@@ -281,8 +279,7 @@ describe('ProjectListController', function () {
             this.readAndWrite.length +
             this.readOnly.length +
             this.tokenReadAndWrite.length +
-            this.tokenReadOnly.length +
-            this.review.length
+            this.tokenReadOnly.length
         )
         done()
       }
@@ -722,14 +719,12 @@ describe('ProjectListController', function () {
         { _id: 6, lastUpdated: 5, owner_ref: 'user-4' }, // Also in tokenReadAndWrite
         { _id: 7, lastUpdated: 4, owner_ref: 'user-5' },
       ]
-      this.review = [{ _id: 8, lastUpdated: 5, owner_ref: 'user-6' }]
       this.allProjects = {
         owned: this.projects,
         readAndWrite: this.readAndWrite,
         readOnly: this.readOnly,
         tokenReadAndWrite: this.tokenReadAndWrite,
         tokenReadOnly: this.tokenReadOnly,
-        review: this.review,
       }
 
       this.ProjectGetter.promises.findAllUsersProjects.resolves(
@@ -752,8 +747,7 @@ describe('ProjectListController', function () {
             this.readAndWrite.length +
             this.readOnly.length +
             this.tokenReadAndWrite.length +
-            this.tokenReadOnly.length +
-            this.review.length -
+            this.tokenReadOnly.length -
             1
         )
         done()