diff --git a/services/real-time/app/js/AuthorizationManager.js b/services/real-time/app/js/AuthorizationManager.js index 261e2e91ee..01e99810bb 100644 --- a/services/real-time/app/js/AuthorizationManager.js +++ b/services/real-time/app/js/AuthorizationManager.js @@ -5,7 +5,7 @@ module.exports = AuthorizationManager = { assertClientCanViewProject(client, callback) { AuthorizationManager._assertClientHasPrivilegeLevel( client, - ['readOnly', 'readAndWrite', 'review', 'owner'], + ['readOnly', 'readAndWrite', 'owner'], callback ) }, diff --git a/services/real-time/test/unit/js/AuthorizationManagerTests.js b/services/real-time/test/unit/js/AuthorizationManagerTests.js index 57882eaacc..e2d296624a 100644 --- a/services/real-time/test/unit/js/AuthorizationManagerTests.js +++ b/services/real-time/test/unit/js/AuthorizationManagerTests.js @@ -47,17 +47,6 @@ 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( diff --git a/services/web/app/src/Features/Authorization/AuthorizationManager.js b/services/web/app/src/Features/Authorization/AuthorizationManager.js index 565788c3ba..28de66047f 100644 --- a/services/web/app/src/Features/Authorization/AuthorizationManager.js +++ b/services/web/app/src/Features/Authorization/AuthorizationManager.js @@ -186,7 +186,6 @@ async function canUserReadProject(userId, projectId, token) { PrivilegeLevels.OWNER, PrivilegeLevels.READ_AND_WRITE, PrivilegeLevels.READ_ONLY, - PrivilegeLevels.REVIEW, ].includes(privilegeLevel) } diff --git a/services/web/app/src/Features/Authorization/PrivilegeLevels.js b/services/web/app/src/Features/Authorization/PrivilegeLevels.js index ff03cf8762..a598f3d04d 100644 --- a/services/web/app/src/Features/Authorization/PrivilegeLevels.js +++ b/services/web/app/src/Features/Authorization/PrivilegeLevels.js @@ -2,7 +2,6 @@ const PrivilegeLevels = { NONE: false, READ_ONLY: 'readOnly', READ_AND_WRITE: 'readAndWrite', - REVIEW: 'review', OWNER: 'owner', } diff --git a/services/web/app/src/Features/Collaborators/CollaboratorsGetter.js b/services/web/app/src/Features/Collaborators/CollaboratorsGetter.js index c2e1163b65..e37e18f443 100644 --- a/services/web/app/src/Features/Collaborators/CollaboratorsGetter.js +++ b/services/web/app/src/Features/Collaborators/CollaboratorsGetter.js @@ -61,7 +61,6 @@ 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}`) @@ -73,8 +72,7 @@ async function getMemberIdsWithPrivilegeLevels(projectId) { project.tokenAccessReadAndWrite_refs, project.tokenAccessReadOnly_refs, project.publicAccesLevel, - project.pendingEditor_refs, - project.reviewer_refs + project.pendingEditor_refs ) return memberIds } @@ -224,10 +222,9 @@ async function getPublicShareTokens(userId, projectId) { // token access has been disabled. async function getProjectsUserIsMemberOf(userId, fields) { const limit = pLimit(2) - const [readAndWrite, review, readOnly, tokenReadAndWrite, tokenReadOnly] = + const [readAndWrite, 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( @@ -248,7 +245,7 @@ async function getProjectsUserIsMemberOf(userId, fields) { ).exec() ), ]) - return { readAndWrite, review, readOnly, tokenReadAndWrite, tokenReadOnly } + return { readAndWrite, readOnly, tokenReadAndWrite, tokenReadOnly } } // This function returns all the projects that a user is a member of, regardless of @@ -327,8 +324,7 @@ function _getMemberIdsWithPrivilegeLevelsFromFields( tokenAccessIds, tokenAccessReadOnlyIds, publicAccessLevel, - pendingEditorIds, - reviewerIds + pendingEditorIds ) { const members = [] members.push({ @@ -369,13 +365,6 @@ function _getMemberIdsWithPrivilegeLevelsFromFields( }) } } - for (const memberId of reviewerIds || []) { - members.push({ - id: memberId.toString(), - privilegeLevel: PrivilegeLevels.REVIEW, - source: Sources.INVITE, - }) - } return members } diff --git a/services/web/app/src/Features/Collaborators/CollaboratorsHandler.js b/services/web/app/src/Features/Collaborators/CollaboratorsHandler.js index 7247c9170d..10dd76dd7a 100644 --- a/services/web/app/src/Features/Collaborators/CollaboratorsHandler.js +++ b/services/web/app/src/Features/Collaborators/CollaboratorsHandler.js @@ -10,7 +10,6 @@ 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), @@ -49,7 +48,6 @@ async function removeUserFromProject(projectId, userId) { $set: { archived }, $pull: { collaberator_refs: userId, - reviewer_refs: userId, readOnly_refs: userId, pendingEditor_refs: userId, tokenAccessReadOnly_refs: userId, @@ -65,7 +63,6 @@ async function removeUserFromProject(projectId, userId) { $pull: { collaberator_refs: userId, readOnly_refs: userId, - reviewer_refs: userId, pendingEditor_refs: userId, tokenAccessReadOnly_refs: userId, tokenAccessReadAndWrite_refs: userId, @@ -112,8 +109,6 @@ async function addUserIdToProject( name: 1, collaberator_refs: 1, readOnly_refs: 1, - reviewer_refs: 1, - track_changes: 1, }) let level let existingUsers = project.collaberator_refs || [] @@ -137,9 +132,6 @@ 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}`) } @@ -148,46 +140,7 @@ async function addUserIdToProject( ContactManager.addContact(addingUserId, userId, () => {}) } - 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() - } + await Project.updateOne({ _id: projectId }, { $addToSet: level }).exec() // Ensure there is a dedicated folder for this "new" project. await TpdsUpdateSender.promises.createProject({ @@ -284,39 +237,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) { diff --git a/services/web/app/src/Features/Collaborators/CollaboratorsRouter.mjs b/services/web/app/src/Features/Collaborators/CollaboratorsRouter.mjs index a057e1d8fe..4ff5097124 100644 --- a/services/web/app/src/Features/Collaborators/CollaboratorsRouter.mjs +++ b/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(), }), }), diff --git a/services/web/app/src/Features/Helpers/AuthorizationHelper.js b/services/web/app/src/Features/Helpers/AuthorizationHelper.js index d8e6bd932d..8369f2d321 100644 --- a/services/web/app/src/Features/Helpers/AuthorizationHelper.js +++ b/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 -} diff --git a/services/web/app/src/Features/Project/ProjectController.js b/services/web/app/src/Features/Project/ProjectController.js index c6ab071a75..1f44051c32 100644 --- a/services/web/app/src/Features/Project/ProjectController.js +++ b/services/web/app/src/Features/Project/ProjectController.js @@ -353,7 +353,6 @@ const _ProjectController = { 'default-visual-for-beginners', 'hotjar', 'ai-add-on', - 'reviewer-role', ].filter(Boolean) const getUserValues = async userId => @@ -895,14 +894,8 @@ const _ProjectController = { }, _buildProjectList(allProjects, userId) { let project - const { - owned, - review, - readAndWrite, - readOnly, - tokenReadAndWrite, - tokenReadOnly, - } = allProjects + const { owned, readAndWrite, readOnly, tokenReadAndWrite, tokenReadOnly } = + allProjects const projects = [] for (project of owned) { projects.push( @@ -925,16 +918,6 @@ const _ProjectController = { ) ) } - for (project of review) { - projects.push( - ProjectController._buildProjectViewModel( - project, - 'review', - Sources.INVITE, - userId - ) - ) - } for (project of readOnly) { projects.push( ProjectController._buildProjectViewModel( diff --git a/services/web/app/src/Features/Project/ProjectGetter.js b/services/web/app/src/Features/Project/ProjectGetter.js index 7edf08d7a2..55e400e0f5 100644 --- a/services/web/app/src/Features/Project/ProjectGetter.js +++ b/services/web/app/src/Features/Project/ProjectGetter.js @@ -102,7 +102,6 @@ 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. diff --git a/services/web/app/src/Features/Project/ProjectListController.mjs b/services/web/app/src/Features/Project/ProjectListController.mjs index 95c34e049b..8c173e0f6d 100644 --- a/services/web/app/src/Features/Project/ProjectListController.mjs +++ b/services/web/app/src/Features/Project/ProjectListController.mjs @@ -549,14 +549,8 @@ async function _getProjects( * @private */ function _formatProjects(projects, userId) { - const { - owned, - review, - readAndWrite, - readOnly, - tokenReadAndWrite, - tokenReadOnly, - } = projects + const { owned, readAndWrite, readOnly, tokenReadAndWrite, tokenReadOnly } = + projects const formattedProjects = /** @type {Project[]} **/ [] for (const project of owned) { @@ -570,11 +564,6 @@ 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) diff --git a/services/web/app/src/Features/TokenAccess/TokenAccessController.mjs b/services/web/app/src/Features/TokenAccess/TokenAccessController.mjs index fa94b0673e..e95ff4c963 100644 --- a/services/web/app/src/Features/TokenAccess/TokenAccessController.mjs +++ b/services/web/app/src/Features/TokenAccess/TokenAccessController.mjs @@ -25,7 +25,6 @@ import LimitationsManager from '../Subscription/LimitationsManager.js' const orderedPrivilegeLevels = [ PrivilegeLevels.NONE, PrivilegeLevels.READ_ONLY, - PrivilegeLevels.REVIEW, PrivilegeLevels.READ_AND_WRITE, PrivilegeLevels.OWNER, ] diff --git a/services/web/app/src/models/Project.js b/services/web/app/src/models/Project.js index 1555b471a4..249ee9979c 100644 --- a/services/web/app/src/models/Project.js +++ b/services/web/app/src/models/Project.js @@ -38,7 +38,6 @@ 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 }, diff --git a/services/web/frontend/extracted-translations.json b/services/web/frontend/extracted-translations.json index aa77aa94df..c57f04bcc6 100644 --- a/services/web/frontend/extracted-translations.json +++ b/services/web/frontend/extracted-translations.json @@ -171,7 +171,6 @@ "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": "", @@ -1281,7 +1280,6 @@ "revert_pending_plan_change": "", "review": "", "review_your_peers_work": "", - "reviewer": "", "revoke": "", "revoke_invite": "", "right": "", diff --git a/services/web/frontend/js/features/ide-react/context/permissions-context.tsx b/services/web/frontend/js/features/ide-react/context/permissions-context.tsx index b58d859791..f21c2bb21c 100644 --- a/services/web/frontend/js/features/ide-react/context/permissions-context.tsx +++ b/services/web/frontend/js/features/ide-react/context/permissions-context.tsx @@ -21,12 +21,6 @@ const permissionsMap: DeepReadonly> = { admin: false, comment: true, }, - review: { - read: true, - write: true, - admin: false, - comment: true, - }, readAndWrite: { read: true, write: true, @@ -44,14 +38,12 @@ const permissionsMap: DeepReadonly> = { 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, } diff --git a/services/web/frontend/js/features/ide-react/context/review-panel/hooks/use-review-panel-state.ts b/services/web/frontend/js/features/ide-react/context/review-panel/hooks/use-review-panel-state.ts index be12f5c3f9..082f466824 100644 --- a/services/web/frontend/js/features/ide-react/context/review-panel/hooks/use-review-panel-state.ts +++ b/services/web/frontend/js/features/ide-react/context/review-panel/hooks/use-review-panel-state.ts @@ -814,10 +814,7 @@ function useReviewPanelState(): ReviewPanel.ReviewPanelState { } const members = project.members ?? [] for (const member of members) { - if ( - member.privileges === 'readAndWrite' || - member.privileges === 'review' - ) { + if (member.privileges === 'readAndWrite') { if (!trackChangesState[member._id]) { // An added member will have track changes enabled if track changes is on for everyone setUserTCState( diff --git a/services/web/frontend/js/features/ide-react/types/permissions.ts b/services/web/frontend/js/features/ide-react/types/permissions.ts index 0234c21e65..531f4d3e79 100644 --- a/services/web/frontend/js/features/ide-react/types/permissions.ts +++ b/services/web/frontend/js/features/ide-react/types/permissions.ts @@ -5,4 +5,4 @@ export type Permissions = { comment: boolean } -export type PermissionsLevel = 'owner' | 'readAndWrite' | 'review' | 'readOnly' +export type PermissionsLevel = 'owner' | 'readAndWrite' | 'readOnly' diff --git a/services/web/frontend/js/features/share-project-modal/components/member-privileges.jsx b/services/web/frontend/js/features/share-project-modal/components/member-privileges.jsx index a968bfc5fd..d49a9ea479 100644 --- a/services/web/frontend/js/features/share-project-modal/components/member-privileges.jsx +++ b/services/web/frontend/js/features/share-project-modal/components/member-privileges.jsx @@ -11,9 +11,6 @@ export default function MemberPrivileges({ privileges }) { case 'readOnly': return t('read_only') - case 'review': - return t('can_review') - default: return null } diff --git a/services/web/frontend/js/features/share-project-modal/components/restricted-link-sharing/add-collaborators.jsx b/services/web/frontend/js/features/share-project-modal/components/restricted-link-sharing/add-collaborators.jsx index 0b64e9143b..9e62f19404 100644 --- a/services/web/frontend/js/features/share-project-modal/components/restricted-link-sharing/add-collaborators.jsx +++ b/services/web/frontend/js/features/share-project-modal/components/restricted-link-sharing/add-collaborators.jsx @@ -14,7 +14,6 @@ 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') @@ -174,9 +173,6 @@ export default function AddCollaborators({ readOnly }) { {t('can_edit')} - {isSplitTestEnabled('reviewer-role') && ( - - )}    @@ -238,7 +237,6 @@ 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') }, ], diff --git a/services/web/frontend/js/features/share-project-modal/components/restricted-link-sharing/member-privileges.jsx b/services/web/frontend/js/features/share-project-modal/components/restricted-link-sharing/member-privileges.jsx index a968bfc5fd..d49a9ea479 100644 --- a/services/web/frontend/js/features/share-project-modal/components/restricted-link-sharing/member-privileges.jsx +++ b/services/web/frontend/js/features/share-project-modal/components/restricted-link-sharing/member-privileges.jsx @@ -11,9 +11,6 @@ export default function MemberPrivileges({ privileges }) { case 'readOnly': return t('read_only') - case 'review': - return t('can_review') - default: return null } 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 556ba8d296..7eedd18654 100644 --- a/services/web/frontend/js/shared/context/types/project-context.tsx +++ b/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' | 'review' + privileges: 'readOnly' | 'readAndWrite' email: string first_name: string last_name: string diff --git a/services/web/locales/en.json b/services/web/locales/en.json index 72ca8bf3bf..c0ef3860d5 100644 --- a/services/web/locales/en.json +++ b/services/web/locales/en.json @@ -233,7 +233,6 @@ "can_link_institution_email_to_login": "You can link your __email__ __appName__ account to your __institutionName__ account, which will allow you to log in to __appName__ through your institution and will reconfirm your institutional email address.", "can_link_your_institution_acct_2": "You can now <0>link your <0>__appName__ account to your <0>__institutionName__ institutional account.", "can_now_relink_dropbox": "You can now <0>relink your Dropbox account.", - "can_review": "Can review", "can_view": "Can view", "cancel": "Cancel", "cancel_add_on": "Cancel add-on", @@ -1805,7 +1804,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", diff --git a/services/web/test/acceptance/src/SharingTests.mjs b/services/web/test/acceptance/src/SharingTests.mjs index f644c39180..aa537d76b6 100644 --- a/services/web/test/acceptance/src/SharingTests.mjs +++ b/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]) - }) - }) }) diff --git a/services/web/test/acceptance/src/helpers/User.js b/services/web/test/acceptance/src/helpers/User.js index 75a8d287ac..fa4ab677ee 100644 --- a/services/web/test/acceptance/src/helpers/User.js +++ b/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) } diff --git a/services/web/test/unit/src/Collaborators/CollaboratorsGetterTests.js b/services/web/test/unit/src/Collaborators/CollaboratorsGetterTests.js index 0547ef4062..2bb3e5907c 100644 --- a/services/web/test/unit/src/Collaborators/CollaboratorsGetterTests.js +++ b/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( diff --git a/services/web/test/unit/src/Collaborators/CollaboratorsHandlerTests.js b/services/web/test/unit/src/Collaborators/CollaboratorsHandlerTests.js index 8e14590b0f..a36b438578 100644 --- a/services/web/test/unit/src/Collaborators/CollaboratorsHandlerTests.js +++ b/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, }, } ) diff --git a/services/web/test/unit/src/HelperFiles/AuthorizationHelperTests.js b/services/web/test/unit/src/HelperFiles/AuthorizationHelperTests.js index 4aa21ef7a9..7a887e2beb 100644 --- a/services/web/test/unit/src/HelperFiles/AuthorizationHelperTests.js +++ b/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 - }) - }) }) diff --git a/services/web/test/unit/src/Project/ProjectGetterTests.js b/services/web/test/unit/src/Project/ProjectGetterTests.js index 8f58b8a10b..1aecd4ab11 100644 --- a/services/web/test/unit/src/Project/ProjectGetterTests.js +++ b/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], }) }) }) diff --git a/services/web/test/unit/src/Project/ProjectListControllerTests.mjs b/services/web/test/unit/src/Project/ProjectListControllerTests.mjs index 74065db753..478845f846 100644 --- a/services/web/test/unit/src/Project/ProjectListControllerTests.mjs +++ b/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()