Skip to content

Commit f6d153f

Browse files
committed
PM-4011 - check reviewer role
1 parent b3ede6f commit f6d153f

2 files changed

Lines changed: 77 additions & 15 deletions

File tree

src/api/review/review.service.spec.ts

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3667,13 +3667,51 @@ describe('ReviewService.updateReviewItem validations', () => {
36673667
expect(prismaMock.reviewItem.update).not.toHaveBeenCalled();
36683668
});
36693669

3670+
it('allows admins assigned as reviewers on the challenge to update review item scores without a manager comment', async () => {
3671+
const adminReviewerUser: JwtUser = {
3672+
userId: 'admin-reviewer-1',
3673+
roles: [UserRole.Admin],
3674+
isMachine: false,
3675+
};
3676+
3677+
resourceApiServiceMock.getMemberResourcesRoles.mockResolvedValueOnce([
3678+
{
3679+
id: 'resource-1',
3680+
memberId: 'admin-reviewer-1',
3681+
roleName: 'Reviewer',
3682+
challengeId: 'challenge-1',
3683+
},
3684+
]);
3685+
3686+
prismaMock.reviewItem.update.mockResolvedValueOnce({
3687+
...baseExistingItem,
3688+
finalAnswer: 'No',
3689+
managerComment: null,
3690+
reviewItemComments: [],
3691+
});
3692+
3693+
await expect(
3694+
service.updateReviewItem(adminReviewerUser, 'item-1', {
3695+
...baseRequest,
3696+
finalAnswer: 'No',
3697+
}),
3698+
).resolves.toMatchObject({ id: 'item-1' });
3699+
3700+
expect(resourceApiServiceMock.getMemberResourcesRoles).toHaveBeenCalledWith(
3701+
'challenge-1',
3702+
'admin-reviewer-1',
3703+
);
3704+
expect(prismaMock.reviewItem.update).toHaveBeenCalled();
3705+
});
3706+
36703707
it('requires manager comment when an admin updates the score on a review item', async () => {
36713708
const adminUser: JwtUser = {
36723709
userId: 'admin-1',
36733710
roles: [UserRole.Admin],
36743711
isMachine: false,
36753712
};
36763713

3714+
resourceApiServiceMock.getMemberResourcesRoles.mockResolvedValueOnce([]);
36773715
prismaMock.reviewItem.update.mockClear();
36783716
prismaMock.reviewAudit.create.mockClear();
36793717

@@ -3689,9 +3727,10 @@ describe('ReviewService.updateReviewItem validations', () => {
36893727
status: 400,
36903728
});
36913729

3692-
expect(
3693-
resourceApiServiceMock.getMemberResourcesRoles,
3694-
).not.toHaveBeenCalled();
3730+
expect(resourceApiServiceMock.getMemberResourcesRoles).toHaveBeenCalledWith(
3731+
'challenge-1',
3732+
'admin-1',
3733+
);
36953734
expect(prismaMock.reviewItem.update).not.toHaveBeenCalled();
36963735
expect(prismaMock.reviewAudit.create).not.toHaveBeenCalled();
36973736
});

src/api/review/review.service.ts

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -799,6 +799,8 @@ export class ReviewService {
799799

800800
await this.ensureChallengeWhitelistAccess(requester, challengeId);
801801

802+
const isRequesterAdmin = isAdmin(requester);
803+
802804
if (requester.isMachine) {
803805
return {
804806
mode: 'machine',
@@ -810,17 +812,6 @@ export class ReviewService {
810812
};
811813
}
812814

813-
if (isAdmin(requester)) {
814-
return {
815-
mode: 'admin',
816-
hasReviewerRole: false,
817-
hasCopilotRole: false,
818-
ownsReview: false,
819-
requiresManagerComment: true,
820-
requesterResources: [],
821-
};
822-
}
823-
824815
const normalizedRoles = Array.isArray(requester.roles)
825816
? requester.roles.map((role) => String(role).trim().toLowerCase())
826817
: [];
@@ -840,7 +831,7 @@ export class ReviewService {
840831
? 'delete'
841832
: 'update';
842833

843-
if (!hasReviewerRole && !hasCopilotRole) {
834+
if (!hasReviewerRole && !hasCopilotRole && !isRequesterAdmin) {
844835
throw new ForbiddenException({
845836
message: `You do not have permission to ${actionVerb} this review item.`,
846837
code: `REVIEW_ITEM_${context.action.toUpperCase()}_FORBIDDEN_ROLE`,
@@ -891,6 +882,38 @@ export class ReviewService {
891882
});
892883
}
893884

885+
if (isRequesterAdmin) {
886+
const isAssignedReviewerRole = requesterResources.some((resource) => {
887+
const normalizedRoleName = (resource.roleName || '').toLowerCase();
888+
const matchesReviewer = normalizedRoleName.includes('reviewer');
889+
const matchesChallenge = challengeId
890+
? resource.challengeId === challengeId
891+
: false;
892+
return matchesReviewer && matchesChallenge;
893+
});
894+
895+
return {
896+
mode: 'admin',
897+
hasReviewerRole,
898+
hasCopilotRole,
899+
ownsReview: false,
900+
requiresManagerComment: !isAssignedReviewerRole,
901+
requesterResources,
902+
};
903+
}
904+
905+
if (!hasReviewerRole && !hasCopilotRole) {
906+
throw new ForbiddenException({
907+
message: `You do not have permission to ${actionVerb} this review item.`,
908+
code: `REVIEW_ITEM_${context.action.toUpperCase()}_FORBIDDEN_ROLE`,
909+
details: {
910+
reviewId: review.id,
911+
itemId: context.itemId,
912+
requesterRoles: requester.roles,
913+
},
914+
});
915+
}
916+
894917
let ownsReview = false;
895918
let mode: ReviewItemAccessMode | null = null;
896919
let hasCopilotAccess = false;

0 commit comments

Comments
 (0)