Support for adding reviewer role (#22137)

* Support for adding reviewer role

* show reviewer in track changes user list

* added "review" in assertClientCanViewProject

* test if reviewer can read project

* added collaboratorsGetter tests

* eit toggle-track-changes when track changes changes

* Support for changing privilege to reviewers for invited users (#22159)

* Add reviewer in change privilege level handler

* added reviewer translation

* added acceptance tests

* fix tests

* Set track changes state permissions for reviewer role (#22167)

* Add reviewer in change privilege level handler

* added reviewer translation

* added acceptance tests

* fix tests

* Set track changes state permissions for reviewer role

* added authorization helper tests

* added ensureUserCanReviewProjectContent middleware

* allow changing track changes only with write permissions

* removed canUserReviewProjectContent

* List projects where user is added as a reviewer (#22249)

* List projects where user is added as reviewer

* list projects in /user/projects

* fix tests

GitOrigin-RevId: 54064a7f961fe06f188ab449cd469cdaaf01b20a

services/real-time/app/js/AuthorizationManager.js

@@ -5,7 +5,7 @@ module.exports = AuthorizationManager = {
   assertClientCanViewProject(client, callback) {
     AuthorizationManager._assertClientHasPrivilegeLevel(
       client,
-      ['readOnly', 'readAndWrite', 'owner'],
+      ['readOnly', 'readAndWrite', 'review', 'owner'],
       callback
     )
   },

services/real-time/test/unit/js/AuthorizationManagerTests.js

@@ -47,6 +47,17 @@ describe('AuthorizationManager', function () {
       )
     })
 
+    it('should allow the review privilegeLevel', function (done) {
+      this.client.ol_context.privilege_level = 'review'
+      return this.AuthorizationManager.assertClientCanViewProject(
+        this.client,
+        error => {
+          expect(error).to.be.null
+          return done()
+        }
+      )
+    })
+
     it('should allow the owner privilegeLevel', function (done) {
       this.client.ol_context.privilege_level = 'owner'
       return this.AuthorizationManager.assertClientCanViewProject(

services/web/app/src/Features/Authorization/AuthorizationManager.js

@@ -186,6 +186,7 @@ async function canUserReadProject(userId, projectId, token) {
     PrivilegeLevels.OWNER,
     PrivilegeLevels.READ_AND_WRITE,
     PrivilegeLevels.READ_ONLY,
+    PrivilegeLevels.REVIEW,
   ].includes(privilegeLevel)
 }
 

services/web/app/src/Features/Authorization/PrivilegeLevels.js

@@ -2,6 +2,7 @@ const PrivilegeLevels = {
   NONE: false,
   READ_ONLY: 'readOnly',
   READ_AND_WRITE: 'readAndWrite',
+  REVIEW: 'review',
   OWNER: 'owner',
 }
 

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

@@ -61,6 +61,7 @@ async function getMemberIdsWithPrivilegeLevels(projectId) {
     tokenAccessReadAndWrite_refs: 1,
     publicAccesLevel: 1,
     pendingEditor_refs: 1,
+    reviewer_refs: 1,
   })
   if (!project) {
     throw new Errors.NotFoundError(`no project found with id ${projectId}`)
@@ -72,7 +73,8 @@ async function getMemberIdsWithPrivilegeLevels(projectId) {
     project.tokenAccessReadAndWrite_refs,
     project.tokenAccessReadOnly_refs,
     project.publicAccesLevel,
-    project.pendingEditor_refs
+    project.pendingEditor_refs,
+    project.reviewer_refs
   )
   return memberIds
 }
@@ -222,9 +224,10 @@ async function getPublicShareTokens(userId, projectId) {
 // token access has been disabled.
 async function getProjectsUserIsMemberOf(userId, fields) {
   const limit = pLimit(2)
-  const [readAndWrite, readOnly, tokenReadAndWrite, tokenReadOnly] =
+  const [readAndWrite, review, readOnly, tokenReadAndWrite, tokenReadOnly] =
     await Promise.all([
       limit(() => Project.find({ collaberator_refs: userId }, fields).exec()),
+      limit(() => Project.find({ reviewer_refs: userId }, fields).exec()),
       limit(() => Project.find({ readOnly_refs: userId }, fields).exec()),
       limit(() =>
         Project.find(
@@ -245,7 +248,7 @@ async function getProjectsUserIsMemberOf(userId, fields) {
         ).exec()
       ),
     ])
-  return { readAndWrite, readOnly, tokenReadAndWrite, tokenReadOnly }
+  return { readAndWrite, review, readOnly, tokenReadAndWrite, tokenReadOnly }
 }
 
 // This function returns all the projects that a user is a member of, regardless of
@@ -324,7 +327,8 @@ function _getMemberIdsWithPrivilegeLevelsFromFields(
   tokenAccessIds,
   tokenAccessReadOnlyIds,
   publicAccessLevel,
-  pendingEditorIds
+  pendingEditorIds,
+  reviewerIds
 ) {
   const members = []
   members.push({
@@ -365,6 +369,13 @@ function _getMemberIdsWithPrivilegeLevelsFromFields(
       })
     }
   }
+  for (const memberId of reviewerIds || []) {
+    members.push({
+      id: memberId.toString(),
+      privilegeLevel: PrivilegeLevels.REVIEW,
+      source: Sources.INVITE,
+    })
+  }
   return members
 }
 

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

@@ -10,6 +10,7 @@ const TpdsProjectFlusher = require('../ThirdPartyDataStore/TpdsProjectFlusher')
 const CollaboratorsGetter = require('./CollaboratorsGetter')
 const Errors = require('../Errors/Errors')
 const TpdsUpdateSender = require('../ThirdPartyDataStore/TpdsUpdateSender')
+const EditorRealTimeController = require('../Editor/EditorRealTimeController')
 
 module.exports = {
   userIsTokenMember: callbackify(userIsTokenMember),
@@ -48,6 +49,7 @@ async function removeUserFromProject(projectId, userId) {
           $set: { archived },
           $pull: {
             collaberator_refs: userId,
+            reviewer_refs: userId,
             readOnly_refs: userId,
             pendingEditor_refs: userId,
             tokenAccessReadOnly_refs: userId,
@@ -63,6 +65,7 @@ async function removeUserFromProject(projectId, userId) {
           $pull: {
             collaberator_refs: userId,
             readOnly_refs: userId,
+            reviewer_refs: userId,
             pendingEditor_refs: userId,
             tokenAccessReadOnly_refs: userId,
             tokenAccessReadAndWrite_refs: userId,
@@ -109,6 +112,8 @@ async function addUserIdToProject(
     name: 1,
     collaberator_refs: 1,
     readOnly_refs: 1,
+    reviewer_refs: 1,
+    track_changes: 1,
   })
   let level
   let existingUsers = project.collaberator_refs || []
@@ -132,6 +137,9 @@ async function addUserIdToProject(
       { privileges: 'readOnly', userId, projectId, pendingEditor },
       'adding user'
     )
+  } else if (privilegeLevel === PrivilegeLevels.REVIEW) {
+    level = { reviewer_refs: userId }
+    logger.debug({ privileges: 'reviewer', userId, projectId }, 'adding user')
   } else {
     throw new Error(`unknown privilegeLevel: ${privilegeLevel}`)
   }
@@ -140,7 +148,46 @@ async function addUserIdToProject(
     ContactManager.addContact(addingUserId, userId, () => {})
   }
 
-  await Project.updateOne({ _id: projectId }, { $addToSet: level }).exec()
+  if (privilegeLevel === PrivilegeLevels.REVIEW) {
+    const trackChanges =
+      typeof project.track_changes === 'object' ? project.track_changes : {}
+
+    trackChanges[userId] = true
+
+    if (project.track_changes === true) {
+      // track changes are enabled for all
+      // we need to convert it to explicit format
+      const members =
+        await CollaboratorsGetter.promises.getMemberIdsWithPrivilegeLevels(
+          project
+        )
+
+      for (const { id, privilegeLevel } of members) {
+        if (
+          [
+            PrivilegeLevels.OWNER,
+            PrivilegeLevels.READ_AND_WRITE,
+            PrivilegeLevels.REVIEW,
+          ].includes(privilegeLevel)
+        ) {
+          trackChanges[id] = true
+        }
+      }
+    }
+
+    await Project.updateOne(
+      { _id: projectId },
+      { track_changes: trackChanges, $addToSet: level }
+    ).exec()
+
+    EditorRealTimeController.emitToRoom(
+      projectId,
+      'toggle-track-changes',
+      trackChanges
+    )
+  } else {
+    await Project.updateOne({ _id: projectId }, { $addToSet: level }).exec()
+  }
 
   // Ensure there is a dedicated folder for this "new" project.
   await TpdsUpdateSender.promises.createProject({
@@ -237,20 +284,39 @@ async function setCollaboratorPrivilegeLevel(
   // collaborator
   const query = {
     _id: projectId,
-    $or: [{ collaberator_refs: userId }, { readOnly_refs: userId }],
+    $or: [
+      { collaberator_refs: userId },
+      { readOnly_refs: userId },
+      { reviewer_refs: userId },
+    ],
   }
   let update
   switch (privilegeLevel) {
     case PrivilegeLevels.READ_AND_WRITE: {
       update = {
-        $pull: { readOnly_refs: userId, pendingEditor_refs: userId },
+        $pull: {
+          readOnly_refs: userId,
+          pendingEditor_refs: userId,
+          reviewer_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 },
+        $pull: { collaberator_refs: userId, reviewer_refs: userId },
         $addToSet: { readOnly_refs: userId },
       }
       if (pendingEditor) {

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

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

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

@@ -1,7 +1,14 @@
 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) {
@@ -14,3 +21,28 @@ 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/app/src/Features/Project/ProjectController.js

@@ -353,6 +353,7 @@ const _ProjectController = {
       'default-visual-for-beginners',
       'hotjar',
       'ai-add-on',
+      'reviewer-role',
     ].filter(Boolean)
 
     const getUserValues = async userId =>
@@ -894,8 +895,14 @@ const _ProjectController = {
   },
   _buildProjectList(allProjects, userId) {
     let project
-    const { owned, readAndWrite, readOnly, tokenReadAndWrite, tokenReadOnly } =
-      allProjects
+    const {
+      owned,
+      review,
+      readAndWrite,
+      readOnly,
+      tokenReadAndWrite,
+      tokenReadOnly,
+    } = allProjects
     const projects = []
     for (project of owned) {
       projects.push(
@@ -918,6 +925,16 @@ const _ProjectController = {
         )
       )
     }
+    for (project of review) {
+      projects.push(
+        ProjectController._buildProjectViewModel(
+          project,
+          'review',
+          Sources.INVITE,
+          userId
+        )
+      )
+    }
     for (project of readOnly) {
       projects.push(
         ProjectController._buildProjectViewModel(

services/web/app/src/Features/Project/ProjectGetter.js

@@ -102,6 +102,7 @@ const ProjectGetter = {
       readOnly: projects.readOnly || [],
       tokenReadAndWrite: projects.tokenReadAndWrite || [],
       tokenReadOnly: projects.tokenReadOnly || [],
+      review: projects.review || [],
     }
 
     // Remove duplicate projects. The order of result values is determined by the order they occur.

services/web/app/src/Features/Project/ProjectListController.mjs

@@ -549,8 +549,14 @@ async function _getProjects(
  * @private
  */
 function _formatProjects(projects, userId) {
-  const { owned, readAndWrite, readOnly, tokenReadAndWrite, tokenReadOnly } =
-    projects
+  const {
+    owned,
+    review,
+    readAndWrite,
+    readOnly,
+    tokenReadAndWrite,
+    tokenReadOnly,
+  } = projects
 
   const formattedProjects = /** @type {Project[]} **/ []
   for (const project of owned) {
@@ -564,6 +570,11 @@ function _formatProjects(projects, userId) {
       _formatProjectInfo(project, 'readWrite', Sources.INVITE, userId)
     )
   }
+  for (const project of review) {
+    formattedProjects.push(
+      _formatProjectInfo(project, 'review', Sources.INVITE, userId)
+    )
+  }
   for (const project of readOnly) {
     formattedProjects.push(
       _formatProjectInfo(project, 'readOnly', Sources.INVITE, userId)

services/web/app/src/Features/TokenAccess/TokenAccessController.mjs

@@ -25,6 +25,7 @@ import LimitationsManager from '../Subscription/LimitationsManager.js'
 const orderedPrivilegeLevels = [
   PrivilegeLevels.NONE,
   PrivilegeLevels.READ_ONLY,
+  PrivilegeLevels.REVIEW,
   PrivilegeLevels.READ_AND_WRITE,
   PrivilegeLevels.OWNER,
 ]

services/web/app/src/models/Project.js

@@ -38,6 +38,7 @@ const ProjectSchema = new Schema(
     active: { type: Boolean, default: true },
     owner_ref: { type: ObjectId, ref: 'User' },
     collaberator_refs: [{ type: ObjectId, ref: 'User' }],
+    reviewer_refs: [{ type: ObjectId, ref: 'User' }],
     readOnly_refs: [{ type: ObjectId, ref: 'User' }],
     pendingEditor_refs: [{ type: ObjectId, ref: 'User' }],
     rootDoc_id: { type: ObjectId },

services/web/frontend/extracted-translations.json

@@ -171,6 +171,7 @@
   "can_link_institution_email_acct_to_institution_acct": "",
   "can_link_your_institution_acct_2": "",
   "can_now_relink_dropbox": "",
+  "can_review": "",
   "can_view": "",
   "cancel": "",
   "cancel_add_on": "",
@@ -1280,6 +1281,7 @@
   "revert_pending_plan_change": "",
   "review": "",
   "review_your_peers_work": "",
+  "reviewer": "",
   "revoke": "",
   "revoke_invite": "",
   "right": "",

services/web/frontend/js/features/ide-react/context/permissions-context.tsx

@@ -21,6 +21,12 @@ const permissionsMap: DeepReadonly<Record<PermissionsLevel, Permissions>> = {
     admin: false,
     comment: true,
   },
+  review: {
+    read: true,
+    write: true,
+    admin: false,
+    comment: true,
+  },
   readAndWrite: {
     read: true,
     write: true,
@@ -38,12 +44,14 @@ const permissionsMap: DeepReadonly<Record<PermissionsLevel, Permissions>> = {
 const anonymousPermissionsMap: typeof permissionsMap = {
   readOnly: { ...permissionsMap.readOnly, comment: false },
   readAndWrite: { ...permissionsMap.readAndWrite, comment: false },
+  review: { ...permissionsMap.review, comment: false },
   owner: { ...permissionsMap.owner, comment: false },
 }
 
 const linkSharingWarningPermissionsMap: typeof permissionsMap = {
   readOnly: { ...permissionsMap.readOnly, comment: false },
   readAndWrite: permissionsMap.readAndWrite,
+  review: permissionsMap.review,
   owner: permissionsMap.owner,
 }
 

services/web/frontend/js/features/ide-react/context/review-panel/hooks/use-review-panel-state.ts

@@ -814,7 +814,10 @@ function useReviewPanelState(): ReviewPanel.ReviewPanelState {
       }
       const members = project.members ?? []
       for (const member of members) {
-        if (member.privileges === 'readAndWrite') {
+        if (
+          member.privileges === 'readAndWrite' ||
+          member.privileges === 'review'
+        ) {
           if (!trackChangesState[member._id]) {
             // An added member will have track changes enabled if track changes is on for everyone
             setUserTCState(

services/web/frontend/js/features/ide-react/types/permissions.ts

@@ -5,4 +5,4 @@ export type Permissions = {
   comment: boolean
 }
 
-export type PermissionsLevel = 'owner' | 'readAndWrite' | 'readOnly'
+export type PermissionsLevel = 'owner' | 'readAndWrite' | 'review' | 'readOnly'

services/web/frontend/js/features/share-project-modal/components/member-privileges.jsx

@@ -11,6 +11,9 @@ export default function MemberPrivileges({ privileges }) {
     case 'readOnly':
       return t('read_only')
 
+    case 'review':
+      return t('can_review')
+
     default:
       return null
   }

services/web/frontend/js/features/share-project-modal/components/restricted-link-sharing/add-collaborators.jsx

@@ -14,6 +14,7 @@ import OLForm from '@/features/ui/components/ol/ol-form'
 import OLFormGroup from '@/features/ui/components/ol/ol-form-group'
 import OLFormSelect from '@/features/ui/components/ol/ol-form-select'
 import OLButton from '@/features/ui/components/ol/ol-button'
+import { isSplitTestEnabled } from '@/utils/splitTestUtils'
 
 export default function AddCollaborators({ readOnly }) {
   const [privileges, setPrivileges] = useState('readAndWrite')
@@ -173,6 +174,9 @@ export default function AddCollaborators({ readOnly }) {
               {t('can_edit')}
             </option>
             <option value="readOnly">{t('can_view')}</option>
+            {isSplitTestEnabled('reviewer-role') && (
+              <option value="review">{t('can_review')}</option>
+            )}
           </OLFormSelect>
           <span>&nbsp;&nbsp;</span>
           <ClickableElementEnhancer

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

@@ -92,6 +92,7 @@ export default function EditMember({
       )
     } else if (
       newPrivileges === 'readAndWrite' ||
+      newPrivileges === 'review' ||
       newPrivileges === 'readOnly'
     ) {
       monitorRequest(() =>
@@ -237,6 +238,7 @@ function SelectPrivilege({
     (): Privilege[] => [
       { key: 'owner', label: t('make_owner') },
       { key: 'readAndWrite', label: t('editor') },
+      { key: 'review', label: t('reviewer') },
       { key: 'readOnly', label: t('viewer') },
       { key: 'removeAccess', label: t('remove_access') },
     ],

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

@@ -11,6 +11,9 @@ export default function MemberPrivileges({ privileges }) {
     case 'readOnly':
       return t('read_only')
 
+    case 'review':
+      return t('can_review')
+
     default:
       return null
   }

services/web/frontend/js/shared/context/types/project-context.tsx

@@ -3,7 +3,7 @@ import { PublicAccessLevel } from '../../../../../types/public-access-level'
 
 export type ProjectContextMember = {
   _id: UserId
-  privileges: 'readOnly' | 'readAndWrite'
+  privileges: 'readOnly' | 'readAndWrite' | 'review'
   email: string
   first_name: string
   last_name: string

services/web/locales/en.json

@@ -233,6 +233,7 @@
   "can_link_institution_email_to_login": "You can link your <b>__email__</b> <b>__appName__</b> account to your <b>__institutionName__</b> account, which will allow you to log in to <b>__appName__</b> through your institution and will reconfirm your institutional email address.",
   "can_link_your_institution_acct_2": "You can now <0>link</0> your <0>__appName__</0> account to your <0>__institutionName__</0> institutional account.",
   "can_now_relink_dropbox": "You can now <0>relink your Dropbox account</0>.",
+  "can_review": "Can review",
   "can_view": "Can view",
   "cancel": "Cancel",
   "cancel_add_on": "Cancel add-on",
@@ -1804,6 +1805,7 @@
   "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,12 +8,15 @@ 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')
   })
 
@@ -35,6 +38,19 @@ 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 () {
@@ -45,6 +61,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])
     })
 
@@ -96,7 +113,40 @@ 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,6 +968,10 @@ 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,6 +20,8 @@ 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()
@@ -33,6 +35,7 @@ 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',
@@ -121,6 +124,16 @@ describe('CollaboratorsGetter', function () {
             privilegeLevel: 'readAndWrite',
             source: 'token',
           },
+          {
+            id: this.reviewer1Ref.toString(),
+            privilegeLevel: 'review',
+            source: 'invite',
+          },
+          {
+            id: this.reviewer2Ref.toString(),
+            privilegeLevel: 'review',
+            source: 'invite',
+          },
         ])
       })
     })
@@ -154,6 +167,8 @@ describe('CollaboratorsGetter', function () {
         this.pendingEditorRef.toString(),
         this.readWriteTokenRef.toString(),
         this.readOnlyTokenRef.toString(),
+        this.reviewer1Ref.toString(),
+        this.reviewer2Ref.toString(),
       ])
     })
   })
@@ -171,6 +186,8 @@ describe('CollaboratorsGetter', function () {
         this.readWriteRef1.toString(),
         this.readWriteRef2.toString(),
         this.pendingEditorRef.toString(),
+        this.reviewer1Ref.toString(),
+        this.reviewer2Ref.toString(),
       ])
     })
   })
@@ -189,6 +206,9 @@ 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 () {
@@ -199,6 +219,7 @@ 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' },
       ])
     })
   })
@@ -213,6 +234,15 @@ 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(
@@ -289,6 +319,11 @@ 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(
           {
@@ -337,6 +372,7 @@ describe('CollaboratorsGetter', function () {
           'mock-token-read-only-project-1',
           'mock-token-read-only-project-2',
         ],
+        review: ['mock-review-project-1', 'mock-review-project-2'],
       })
     })
   })
@@ -352,15 +388,21 @@ 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
@@ -369,6 +411,9 @@ 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,8 +65,10 @@ 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,
@@ -76,6 +78,7 @@ describe('CollaboratorsHandler', function () {
         '../ThirdPartyDataStore/TpdsUpdateSender': this.TpdsUpdateSender,
         '../Project/ProjectGetter': this.ProjectGetter,
         '../Project/ProjectHelper': this.ProjectHelper,
+        '../Editor/EditorRealTimeController': this.EditorRealTimeController,
         './CollaboratorsGetter': this.CollaboratorsGetter,
       },
     })
@@ -105,6 +108,7 @@ describe('CollaboratorsHandler', function () {
             {
               $pull: {
                 collaberator_refs: this.userId,
+                reviewer_refs: this.userId,
                 readOnly_refs: this.userId,
                 pendingEditor_refs: this.userId,
                 tokenAccessReadOnly_refs: this.userId,
@@ -148,6 +152,7 @@ describe('CollaboratorsHandler', function () {
               },
               $pull: {
                 collaberator_refs: this.userId,
+                reviewer_refs: this.userId,
                 readOnly_refs: this.userId,
                 pendingEditor_refs: this.userId,
                 tokenAccessReadOnly_refs: this.userId,
@@ -183,6 +188,7 @@ describe('CollaboratorsHandler', function () {
             {
               $pull: {
                 collaberator_refs: this.userId,
+                reviewer_refs: this.userId,
                 readOnly_refs: this.userId,
                 pendingEditor_refs: this.userId,
                 tokenAccessReadOnly_refs: this.userId,
@@ -302,6 +308,43 @@ 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(
@@ -405,6 +448,7 @@ describe('CollaboratorsHandler', function () {
             {
               $pull: {
                 collaberator_refs: this.userId,
+                reviewer_refs: this.userId,
                 readOnly_refs: this.userId,
                 pendingEditor_refs: this.userId,
                 tokenAccessReadOnly_refs: this.userId,
@@ -546,12 +590,14 @@ 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 },
           }
@@ -573,12 +619,14 @@ 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,
             },
           }
@@ -592,6 +640,35 @@ 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(
@@ -600,6 +677,7 @@ describe('CollaboratorsHandler', function () {
             $or: [
               { collaberator_refs: this.userId },
               { readOnly_refs: this.userId },
+              { reviewer_refs: this.userId },
             ],
           },
           {
@@ -609,6 +687,7 @@ describe('CollaboratorsHandler', function () {
             },
             $pull: {
               collaberator_refs: this.userId,
+              reviewer_refs: this.userId,
             },
           }
         )

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

@@ -25,6 +25,10 @@ describe('AuthorizationHelper', function () {
             },
           },
         },
+        '../Project/ProjectGetter': (this.ProjectGetter = { promises: {} }),
+        '../SplitTests/SplitTestHandler': (this.SplitTestHandler = {
+          promises: {},
+        }),
       },
     })
   })
@@ -59,4 +63,76 @@ 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,6 +275,7 @@ 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' }
@@ -289,6 +290,7 @@ describe('ProjectGetter', function () {
         readOnly: [this.projectRO],
         tokenReadAndWrite: [this.projectTokenRW],
         tokenReadOnly: [this.projectTokenRO],
+        review: [this.projectReview],
       })
       const projects = await this.ProjectGetter.promises.findAllUsersProjects(
         this.userId,
@@ -301,6 +303,7 @@ describe('ProjectGetter', function () {
         readOnly: [this.projectRO],
         tokenReadAndWrite: [this.projectTokenRW],
         tokenReadOnly: [this.projectTokenRO],
+        review: [this.projectReview],
       })
     })
 
@@ -314,6 +317,7 @@ describe('ProjectGetter', function () {
           this.projectTokenRO,
           this.projectRO,
         ],
+        review: [this.projectReview],
       })
       const projects = await this.ProjectGetter.promises.findAllUsersProjects(
         this.userId,
@@ -326,6 +330,7 @@ describe('ProjectGetter', function () {
         readOnly: [this.projectRO],
         tokenReadAndWrite: [this.projectTokenRW],
         tokenReadOnly: [this.projectTokenRO],
+        review: [this.projectReview],
       })
     })
   })

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

@@ -219,12 +219,14 @@ 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(
@@ -279,7 +281,8 @@ describe('ProjectListController', function () {
             this.readAndWrite.length +
             this.readOnly.length +
             this.tokenReadAndWrite.length +
-            this.tokenReadOnly.length
+            this.tokenReadOnly.length +
+            this.review.length
         )
         done()
       }
@@ -719,12 +722,14 @@ 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(
@@ -747,7 +752,8 @@ describe('ProjectListController', function () {
             this.readAndWrite.length +
             this.readOnly.length +
             this.tokenReadAndWrite.length +
-            this.tokenReadOnly.length -
+            this.tokenReadOnly.length +
+            this.review.length -
             1
         )
         done()