Revert "Set track changes state permissions for reviewer role (#22345)" (#22431)

This reverts commit 6a03d2355b3fb7d7b755ed1d3ab1a080126cd2dc.

GitOrigin-RevId: 4e962bd9a547a9d6205460c9a8e9a0e835442be4

services/web/app/src/Features/Collaborators/CollaboratorsHandler.js

@@ -284,39 +284,20 @@ async function setCollaboratorPrivilegeLevel(
   // collaborator
   const query = {
     _id: projectId,
-    $or: [
-      { collaberator_refs: userId },
-      { readOnly_refs: userId },
-      { reviewer_refs: userId },
-    ],
+    $or: [{ collaberator_refs: userId }, { readOnly_refs: userId }],
   }
   let update
   switch (privilegeLevel) {
     case PrivilegeLevels.READ_AND_WRITE: {
       update = {
-        $pull: {
-          readOnly_refs: userId,
-          pendingEditor_refs: userId,
-          reviewer_refs: userId,
-        },
+        $pull: { readOnly_refs: userId, pendingEditor_refs: userId },
         $addToSet: { collaberator_refs: userId },
       }
       break
     }
-    case PrivilegeLevels.REVIEW: {
-      update = {
-        $pull: {
-          readOnly_refs: userId,
-          pendingEditor_refs: userId,
-          collaberator_refs: userId,
-        },
-        $addToSet: { reviewer_refs: userId },
-      }
-      break
-    }
     case PrivilegeLevels.READ_ONLY: {
       update = {
-        $pull: { collaberator_refs: userId, reviewer_refs: userId },
+        $pull: { collaberator_refs: userId },
         $addToSet: { readOnly_refs: userId },
       }
       if (pendingEditor) {

services/web/app/src/Features/Collaborators/CollaboratorsRouter.mjs

@@ -50,11 +50,7 @@ export default {
         }),
         body: Joi.object({
           privilegeLevel: Joi.string()
-            .valid(
-              PrivilegeLevels.READ_ONLY,
-              PrivilegeLevels.READ_AND_WRITE,
-              PrivilegeLevels.REVIEW
-            )
+            .valid(PrivilegeLevels.READ_ONLY, PrivilegeLevels.READ_AND_WRITE)
             .required(),
         }),
       }),

services/web/app/src/Features/Helpers/AuthorizationHelper.js

@@ -1,14 +1,7 @@
 const { UserSchema } = require('../../models/User')
-const SplitTestHandler = require('../SplitTests/SplitTestHandler')
-const ProjectGetter = require('../Project/ProjectGetter')
-const { callbackify } = require('@overleaf/promise-utils')
 
 module.exports = {
   hasAnyStaffAccess,
-  isReviewerRoleEnabled: callbackify(isReviewerRoleEnabled),
-  promises: {
-    isReviewerRoleEnabled,
-  },
 }
 
 function hasAnyStaffAccess(user) {
@@ -21,28 +14,3 @@ function hasAnyStaffAccess(user) {
   }
   return false
 }
-
-async function isReviewerRoleEnabled(userId, projectId) {
-  const project = await ProjectGetter.promises.getProject(projectId, {
-    reviewer_refs: 1,
-    owner_ref: 1,
-  })
-
-  // if there are reviewers, it means the role is enabled
-  if (Object.keys(project.reviewer_refs || {}).length > 0) {
-    return true
-  }
-
-  // if there are no reviewers, check split test from project owner
-  if (project.owner_ref === userId) {
-    const reviewerRoleAssigment =
-      await SplitTestHandler.promises.getAssignmentForUser(
-        userId,
-        'reviewer-role'
-      )
-
-    return reviewerRoleAssigment.variant === 'enabled'
-  }
-
-  return false
-}

services/web/frontend/js/features/share-project-modal/components/restricted-link-sharing/edit-member.tsx

@@ -92,7 +92,6 @@ export default function EditMember({
       )
     } else if (
       newPrivileges === 'readAndWrite' ||
-      newPrivileges === 'review' ||
       newPrivileges === 'readOnly'
     ) {
       monitorRequest(() =>

services/web/locales/en.json

@@ -1824,7 +1824,6 @@
   "revert_pending_plan_change": "Revert scheduled plan change",
   "review": "Review",
   "review_your_peers_work": "Review your peers’ work",
-  "reviewer": "Reviewer",
   "revoke": "Revoke",
   "revoke_invite": "Revoke Invite",
   "right": "Right",

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/CollaboratorsHandlerTests.js

@@ -590,14 +590,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 +617,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 +636,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 +644,6 @@ describe('CollaboratorsHandler', function () {
             $or: [
               { collaberator_refs: this.userId },
               { readOnly_refs: this.userId },
-              { reviewer_refs: this.userId },
             ],
           },
           {
@@ -687,7 +653,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
-    })
-  })
 })