From ef82e9e5b4e2d628046167c1cc71cc25a7b8fb17 Mon Sep 17 00:00:00 2001 From: David <33458145+davidmcpowell@users.noreply.github.com> Date: Thu, 22 May 2025 13:27:22 +0100 Subject: [PATCH] Merge pull request #25784 from overleaf/dp-backend-reviewer-role-cleanup Remove references to `reviewer-role` feature flag in the backend GitOrigin-RevId: 4d2088e4c2815d3221817a182a0a66b5a60b3532 --- .../Features/Helpers/AuthorizationHelper.js | 28 -------- .../src/Features/Project/ProjectController.js | 10 --- .../web/app/views/project/editor/_meta.pug | 1 - .../HelperFiles/AuthorizationHelperTests.js | 68 ------------------- 4 files changed, 107 deletions(-) diff --git a/services/web/app/src/Features/Helpers/AuthorizationHelper.js b/services/web/app/src/Features/Helpers/AuthorizationHelper.js index f193398b87..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,24 +14,3 @@ function hasAnyStaffAccess(user) { } return false } - -async function isReviewerRoleEnabled(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 - const reviewerRoleAssigment = - await SplitTestHandler.promises.getAssignmentForUser( - project.owner_ref, - 'reviewer-role' - ) - - return reviewerRoleAssigment.variant === 'enabled' -} diff --git a/services/web/app/src/Features/Project/ProjectController.js b/services/web/app/src/Features/Project/ProjectController.js index 160914db81..ec128ffd54 100644 --- a/services/web/app/src/Features/Project/ProjectController.js +++ b/services/web/app/src/Features/Project/ProjectController.js @@ -347,7 +347,6 @@ const _ProjectController = { 'track-pdf-download', !anonymous && 'writefull-oauth-promotion', 'hotjar', - 'reviewer-role', 'editor-redesign', 'paywall-change-compile-timeout', 'overleaf-assist-bundle', @@ -482,12 +481,6 @@ const _ProjectController = { anonRequestToken ) - const reviewerRoleAssignment = - await SplitTestHandler.promises.getAssignmentForUser( - project.owner_ref, - 'reviewer-role' - ) - await Modules.promises.hooks.fire('enforceCollaboratorLimit', projectId) if (isTokenMember) { // Check explicitly that the user is in read write token refs, while this could be inferred @@ -883,9 +876,6 @@ const _ProjectController = { : null, isSaas: Features.hasFeature('saas'), shouldLoadHotjar: splitTestAssignments.hotjar?.variant === 'enabled', - isReviewerRoleEnabled: - reviewerRoleAssignment?.variant === 'enabled' || - Object.keys(project.reviewer_refs || {}).length > 0, isPaywallChangeCompileTimeoutEnabled, isOverleafAssistBundleEnabled, paywallPlans, diff --git a/services/web/app/views/project/editor/_meta.pug b/services/web/app/views/project/editor/_meta.pug index ab31cef89e..e8539e1d76 100644 --- a/services/web/app/views/project/editor/_meta.pug +++ b/services/web/app/views/project/editor/_meta.pug @@ -40,7 +40,6 @@ meta(name="ol-projectTags" data-type="json" content=projectTags) meta(name="ol-ro-mirror-on-client-no-local-storage" data-type="boolean" content=roMirrorOnClientNoLocalStorage) meta(name="ol-isSaas" data-type="boolean" content=isSaas) meta(name="ol-shouldLoadHotjar" data-type="boolean" content=shouldLoadHotjar) -meta(name="ol-isReviewerRoleEnabled" data-type="boolean" content=isReviewerRoleEnabled) meta(name="ol-odcRole" data-type="string" content=odcRole) meta(name="ol-isPaywallChangeCompileTimeoutEnabled" data-type="boolean" content=isPaywallChangeCompileTimeoutEnabled) meta(name='ol-customerIoEnabled' data-type="boolean" content=customerIoEnabled) diff --git a/services/web/test/unit/src/HelperFiles/AuthorizationHelperTests.js b/services/web/test/unit/src/HelperFiles/AuthorizationHelperTests.js index ef8b5fcc6a..a82143dce6 100644 --- a/services/web/test/unit/src/HelperFiles/AuthorizationHelperTests.js +++ b/services/web/test/unit/src/HelperFiles/AuthorizationHelperTests.js @@ -63,72 +63,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( - '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( - '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( - '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( - 'projectId' - ) - ).to.be.true - }) - }) })