From 8c3fe3bd31f46d57f927e0cdd37786f73cd302ba Mon Sep 17 00:00:00 2001 From: Domagoj Kriskovic Date: Mon, 25 Aug 2025 14:04:55 +0200 Subject: [PATCH] [web] change the order when creating a memebers list in permissions checks (#28063) GitOrigin-RevId: 73fd9218841d189dc95edec86f09d451005e6189 --- .../Collaborators/CollaboratorsGetter.js | 15 ++-- .../Collaborators/CollaboratorsGetterTests.js | 86 +++++++++++++++++++ 2 files changed, 94 insertions(+), 7 deletions(-) diff --git a/services/web/app/src/Features/Collaborators/CollaboratorsGetter.js b/services/web/app/src/Features/Collaborators/CollaboratorsGetter.js index a3543ae614..14f2eab4ca 100644 --- a/services/web/app/src/Features/Collaborators/CollaboratorsGetter.js +++ b/services/web/app/src/Features/Collaborators/CollaboratorsGetter.js @@ -498,6 +498,14 @@ function _getMemberIdsWithPrivilegeLevelsFromFields( }) } + for (const memberId of reviewerIds || []) { + members.push({ + id: memberId.toString(), + privilegeLevel: PrivilegeLevels.REVIEW, + source: Sources.INVITE, + }) + } + for (const memberId of readOnlyIds || []) { const record = { id: memberId.toString(), @@ -530,13 +538,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/test/unit/src/Collaborators/CollaboratorsGetterTests.js b/services/web/test/unit/src/Collaborators/CollaboratorsGetterTests.js index 10542c4564..f5ac06fb58 100644 --- a/services/web/test/unit/src/Collaborators/CollaboratorsGetterTests.js +++ b/services/web/test/unit/src/Collaborators/CollaboratorsGetterTests.js @@ -231,6 +231,26 @@ describe('CollaboratorsGetter', function () { ) expect(level).to.equal(false) }) + + it('should return review privilege level when user is both reviewer and token member', async function () { + const userWhoIsBothReviewerAndToken = new ObjectId() + + const projectWithDuplicateUser = { + ...this.project, + reviewer_refs: [userWhoIsBothReviewerAndToken], + tokenAccessReadAndWrite_refs: [userWhoIsBothReviewerAndToken], + } + + this.ProjectGetter.promises.getProject.resolves(projectWithDuplicateUser) + + const level = + await this.CollaboratorsGetter.promises.getMemberIdPrivilegeLevel( + userWhoIsBothReviewerAndToken, + this.project._id + ) + + expect(level).to.equal('review') + }) }) describe('isUserInvitedMemberOfProject', function () { @@ -533,4 +553,70 @@ describe('CollaboratorsGetter', function () { expect(count).to.equal(2) }) }) + + describe('ProjectAccess', function () { + describe('privilegeLevelForUser', function () { + it('should return reviewer privilege when user is both reviewer and token member', function () { + const userWhoIsBothReviewerAndToken = new ObjectId() + + const projectWithDuplicateUser = { + owner_ref: this.ownerRef, + collaberator_refs: [], + readOnly_refs: [], + tokenAccessReadAndWrite_refs: [userWhoIsBothReviewerAndToken], + tokenAccessReadOnly_refs: [], + publicAccesLevel: 'tokenBased', + pendingEditor_refs: [], + reviewer_refs: [userWhoIsBothReviewerAndToken], + pendingReviewer_refs: [], + } + + const projectAccess = new this.CollaboratorsGetter.ProjectAccess( + projectWithDuplicateUser + ) + const privilegeLevel = projectAccess.privilegeLevelForUser( + userWhoIsBothReviewerAndToken + ) + + expect(privilegeLevel).to.equal('review') + }) + + it('should return readOnly privilege when user is both readOnly and token readAndWrite member', function () { + const userWhoIsBothReadOnlyAndTokenRW = new ObjectId() + + const projectWithDuplicateUser = { + owner_ref: this.ownerRef, + collaberator_refs: [], + readOnly_refs: [userWhoIsBothReadOnlyAndTokenRW], + tokenAccessReadAndWrite_refs: [userWhoIsBothReadOnlyAndTokenRW], + tokenAccessReadOnly_refs: [], + publicAccesLevel: 'tokenBased', + pendingEditor_refs: [], + reviewer_refs: [], + pendingReviewer_refs: [], + } + + const projectAccess = new this.CollaboratorsGetter.ProjectAccess( + projectWithDuplicateUser + ) + const privilegeLevel = projectAccess.privilegeLevelForUser( + userWhoIsBothReadOnlyAndTokenRW + ) + + // Should return 'readOnly' from invite, not 'readAndWrite' from token access + expect(privilegeLevel).to.equal('readOnly') + }) + + it('should return none for non-members', function () { + const projectAccess = new this.CollaboratorsGetter.ProjectAccess( + this.project + ) + const privilegeLevel = projectAccess.privilegeLevelForUser( + this.nonMemberRef + ) + + expect(privilegeLevel).to.equal(false) + }) + }) + }) })