From f152925e49eb8cd061bb1ce44084b3650253a865 Mon Sep 17 00:00:00 2001 From: Domagoj Kriskovic Date: Thu, 9 Jan 2025 10:48:52 +0100 Subject: [PATCH] Use owner's reviewer role assigment (#22735) GitOrigin-RevId: f632ea8c23b5a1f9ac6985922c53f1b6d49e9494 --- .../Features/Helpers/AuthorizationHelper.js | 18 +++++++----------- .../src/Features/Project/ProjectController.js | 13 ++++++++++--- .../HelperFiles/AuthorizationHelperTests.js | 4 ---- 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/services/web/app/src/Features/Helpers/AuthorizationHelper.js b/services/web/app/src/Features/Helpers/AuthorizationHelper.js index d8e6bd932d..f193398b87 100644 --- a/services/web/app/src/Features/Helpers/AuthorizationHelper.js +++ b/services/web/app/src/Features/Helpers/AuthorizationHelper.js @@ -22,7 +22,7 @@ function hasAnyStaffAccess(user) { return false } -async function isReviewerRoleEnabled(userId, projectId) { +async function isReviewerRoleEnabled(projectId) { const project = await ProjectGetter.promises.getProject(projectId, { reviewer_refs: 1, owner_ref: 1, @@ -34,15 +34,11 @@ async function isReviewerRoleEnabled(userId, projectId) { } // 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' - ) + const reviewerRoleAssigment = + await SplitTestHandler.promises.getAssignmentForUser( + project.owner_ref, + 'reviewer-role' + ) - return reviewerRoleAssigment.variant === 'enabled' - } - - return false + 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 7f5648f507..be4847e971 100644 --- a/services/web/app/src/Features/Project/ProjectController.js +++ b/services/web/app/src/Features/Project/ProjectController.js @@ -484,7 +484,11 @@ const _ProjectController = { anonRequestToken ) - const [linkSharingChanges, linkSharingEnforcement] = await Promise.all([ + const [ + linkSharingChanges, + linkSharingEnforcement, + reviewerRoleAssigment, + ] = await Promise.all([ SplitTestHandler.promises.getAssignmentForUser( project.owner_ref, 'link-sharing-warning' @@ -493,6 +497,10 @@ const _ProjectController = { project.owner_ref, 'link-sharing-enforcement' ), + SplitTestHandler.promises.getAssignmentForUser( + project.owner_ref, + 'reviewer-role' + ), ]) if (linkSharingChanges?.variant === 'active') { @@ -829,8 +837,7 @@ const _ProjectController = { isSaas: Features.hasFeature('saas'), shouldLoadHotjar: splitTestAssignments.hotjar?.variant === 'enabled', isReviewerRoleEnabled: - (privilegeLevel === PrivilegeLevels.OWNER && - splitTestAssignments['reviewer-role']?.variant === 'enabled') || + reviewerRoleAssigment || Object.keys(project.reviewer_refs || {}).length > 0, }) timer.done() diff --git a/services/web/test/unit/src/HelperFiles/AuthorizationHelperTests.js b/services/web/test/unit/src/HelperFiles/AuthorizationHelperTests.js index 4aa21ef7a9..ef8b5fcc6a 100644 --- a/services/web/test/unit/src/HelperFiles/AuthorizationHelperTests.js +++ b/services/web/test/unit/src/HelperFiles/AuthorizationHelperTests.js @@ -77,7 +77,6 @@ describe('AuthorizationHelper', function () { }) expect( await this.AuthorizationHelper.promises.isReviewerRoleEnabled( - 'userId', 'projectId' ) ).to.be.false @@ -95,7 +94,6 @@ describe('AuthorizationHelper', function () { }) expect( await this.AuthorizationHelper.promises.isReviewerRoleEnabled( - 'userId', 'projectId' ) ).to.be.true @@ -112,7 +110,6 @@ describe('AuthorizationHelper', function () { }) expect( await this.AuthorizationHelper.promises.isReviewerRoleEnabled( - 'userId', 'projectId' ) ).to.be.true @@ -129,7 +126,6 @@ describe('AuthorizationHelper', function () { }) expect( await this.AuthorizationHelper.promises.isReviewerRoleEnabled( - 'userId', 'projectId' ) ).to.be.true