Skip to content

Commit 1b8bbfd

Browse files
committed
Tweaks for showing specific details to mixed roles
1 parent ef42e7e commit 1b8bbfd

File tree

2 files changed

+145
-2
lines changed

2 files changed

+145
-2
lines changed

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

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -806,6 +806,35 @@ describe('ReviewService.getReview authorization checks', () => {
806806
});
807807
});
808808

809+
it('allows copilots who also have reviewer resources to access other reviews before completion', async () => {
810+
resourceApiServiceMock.getMemberResourcesRoles.mockResolvedValueOnce([
811+
baseReviewerResource,
812+
{
813+
...baseReviewerResource,
814+
id: 'resource-copilot',
815+
roleName: 'Copilot',
816+
},
817+
]);
818+
819+
prismaMock.review.findUniqueOrThrow.mockResolvedValueOnce({
820+
...defaultReviewData(),
821+
resourceId: 'resource-other',
822+
phaseId: 'phase-review',
823+
});
824+
825+
const copilotReviewerUser: JwtUser = {
826+
...baseAuthUser,
827+
roles: [UserRole.Reviewer, UserRole.Copilot],
828+
};
829+
830+
await expect(
831+
service.getReview(copilotReviewerUser, 'review-1'),
832+
).resolves.toMatchObject({
833+
id: 'review-1',
834+
resourceId: 'resource-other',
835+
});
836+
});
837+
809838
it('allows reviewers to access screening reviews that are not their own before completion', async () => {
810839
prismaMock.review.findUniqueOrThrow.mockResolvedValue({
811840
...defaultReviewData(),
@@ -1544,6 +1573,117 @@ describe('ReviewService.getReviews reviewer visibility', () => {
15441573
expect(callArgs.where.resourceId).toBeUndefined();
15451574
});
15461575

1576+
it('uses the most permissive access when the requester is both reviewer and copilot', async () => {
1577+
const now = new Date();
1578+
const reviewerResource = {
1579+
...buildResource('Reviewer'),
1580+
id: 'resource-reviewer',
1581+
};
1582+
const copilotResource = {
1583+
...buildResource('Copilot'),
1584+
id: 'resource-copilot',
1585+
roleId: 'role-copilot',
1586+
};
1587+
1588+
resourceApiServiceMock.getMemberResourcesRoles.mockResolvedValue([
1589+
reviewerResource,
1590+
copilotResource,
1591+
]);
1592+
1593+
const reviewRecords = [
1594+
{
1595+
id: 'review-1',
1596+
resourceId: 'resource-other',
1597+
submissionId: 'submission-1',
1598+
phaseId: 'phase-review',
1599+
scorecardId: 'scorecard-1',
1600+
typeId: 'type-1',
1601+
status: ReviewStatus.COMPLETED,
1602+
reviewDate: now,
1603+
committed: true,
1604+
metadata: null,
1605+
initialScore: 80,
1606+
finalScore: 85,
1607+
createdAt: now,
1608+
createdBy: 'system',
1609+
updatedAt: now,
1610+
updatedBy: 'system',
1611+
reviewItems: [
1612+
{
1613+
id: 'item-1',
1614+
reviewId: 'review-1',
1615+
scorecardQuestionId: 'question-1',
1616+
initialAnswer: 'Yes',
1617+
finalAnswer: 'No',
1618+
managerComment: null,
1619+
reviewItemComments: [],
1620+
},
1621+
],
1622+
submission: {
1623+
id: 'submission-1',
1624+
memberId: 'submitter-1',
1625+
challengeId: 'challenge-1',
1626+
},
1627+
},
1628+
{
1629+
id: 'review-2',
1630+
resourceId: 'resource-reviewer',
1631+
submissionId: 'submission-2',
1632+
phaseId: 'phase-review',
1633+
scorecardId: 'scorecard-1',
1634+
typeId: 'type-1',
1635+
status: ReviewStatus.COMPLETED,
1636+
reviewDate: now,
1637+
committed: true,
1638+
metadata: null,
1639+
initialScore: 90,
1640+
finalScore: 95,
1641+
createdAt: now,
1642+
createdBy: 'system',
1643+
updatedAt: now,
1644+
updatedBy: 'system',
1645+
reviewItems: [
1646+
{
1647+
id: 'item-2',
1648+
reviewId: 'review-2',
1649+
scorecardQuestionId: 'question-2',
1650+
initialAnswer: 'Yes',
1651+
finalAnswer: 'Yes',
1652+
managerComment: 'Great work',
1653+
reviewItemComments: [],
1654+
},
1655+
],
1656+
submission: {
1657+
id: 'submission-2',
1658+
memberId: 'submitter-2',
1659+
challengeId: 'challenge-1',
1660+
},
1661+
},
1662+
];
1663+
1664+
prismaMock.review.findMany.mockResolvedValue(reviewRecords);
1665+
prismaMock.review.count.mockResolvedValue(reviewRecords.length);
1666+
1667+
const response = await service.getReviews(
1668+
{
1669+
...baseAuthUser,
1670+
roles: [UserRole.Reviewer, UserRole.Copilot],
1671+
},
1672+
undefined,
1673+
'challenge-1',
1674+
);
1675+
1676+
const callArgs = prismaMock.review.findMany.mock.calls[0][0];
1677+
expect(callArgs.where.resourceId).toBeUndefined();
1678+
1679+
const otherReview = response.data.find(
1680+
(review) => review.id === 'review-1',
1681+
);
1682+
expect(otherReview).toBeDefined();
1683+
expect(otherReview?.finalScore).toBe(85);
1684+
expect(otherReview?.reviewItems?.length).toBe(1);
1685+
});
1686+
15471687
it('hides scores and review items for other reviewers on active challenges while keeping reviewer metadata', async () => {
15481688
const now = new Date();
15491689
const reviewerUser: JwtUser = {

src/api/review/review.service.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3432,6 +3432,7 @@ export class ReviewService {
34323432
challengeForReview?.status ?? challengeDetail?.status ?? null;
34333433
const shouldMaskOtherReviewerDetails =
34343434
!isPrivilegedRequester &&
3435+
!hasCopilotRoleForChallenge &&
34353436
reviewerResourceIdSet.size > 0 &&
34363437
!isReviewerForReview &&
34373438
!this.isCompletedOrCancelledStatus(challengeStatusForReview) &&
@@ -3690,7 +3691,9 @@ export class ReviewService {
36903691
);
36913692
isReviewerForReview = reviewerResourceIds.has(reviewResourceId);
36923693

3693-
if (reviewerResources.length > 0) {
3694+
if (hasCopilotRole) {
3695+
// Copilots on the challenge can view any review regardless of reviewer ownership.
3696+
} else if (reviewerResources.length > 0) {
36943697
const challengeInFinalState = [
36953698
ChallengeStatus.COMPLETED,
36963699
ChallengeStatus.CANCELLED_FAILED_REVIEW,
@@ -3720,7 +3723,7 @@ export class ReviewService {
37203723
details: { challengeId, reviewId, requester: uid },
37213724
});
37223725
}
3723-
} else if (!hasCopilotRole) {
3726+
} else {
37243727
// Confirm the user has actually submitted to this challenge (has a submission record)
37253728
const mySubs = await this.prisma.submission.findMany({
37263729
where: { challengeId, memberId: uid },

0 commit comments

Comments
 (0)