Skip to content

Commit 41b0ca8

Browse files
jmgasperjmgasper
committed
Add check before manually closing an Appeals Response phase to make sure no appeals are pending
1 parent c003e03 commit 41b0ca8

File tree

2 files changed

+188
-1
lines changed

2 files changed

+188
-1
lines changed

‎src/services/ChallengePhaseService.js‎

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,57 @@ async function hasSubmittedAppealsForChallenge(challengeId) {
180180
return Number(count) > 0;
181181
}
182182

183+
async function hasPendingAppealResponsesForChallenge(challengeId) {
184+
if (!config.REVIEW_DB_URL) {
185+
logger.debug(
186+
`Skipping pending appeal response check for challenge ${challengeId} because REVIEW_DB_URL is not configured`
187+
);
188+
return false;
189+
}
190+
191+
const reviewPrisma = getReviewClient();
192+
const reviewSchema = config.REVIEW_DB_SCHEMA;
193+
const appealTable = Prisma.raw(`"${reviewSchema}"."appeal"`);
194+
const appealResponseTable = Prisma.raw(`"${reviewSchema}"."appealResponse"`);
195+
const reviewItemCommentTable = Prisma.raw(`"${reviewSchema}"."reviewItemComment"`);
196+
const reviewItemTable = Prisma.raw(`"${reviewSchema}"."reviewItem"`);
197+
const reviewTable = Prisma.raw(`"${reviewSchema}"."review"`);
198+
const submissionTable = Prisma.raw(`"${reviewSchema}"."submission"`);
199+
200+
let rows;
201+
try {
202+
rows = await reviewPrisma.$queryRaw(
203+
Prisma.sql`
204+
SELECT COUNT(DISTINCT a."id")::int AS count
205+
FROM ${appealTable} a
206+
LEFT JOIN ${appealResponseTable} ar ON ar."appealId" = a."id"
207+
INNER JOIN ${reviewItemCommentTable} ric ON ric."id" = a."reviewItemCommentId"
208+
INNER JOIN ${reviewItemTable} ri ON ri."id" = ric."reviewItemId"
209+
INNER JOIN ${reviewTable} r ON r."id" = ri."reviewId"
210+
WHERE ar."id" IS NULL
211+
AND EXISTS (
212+
SELECT 1
213+
FROM ${submissionTable} s
214+
WHERE s."challengeId" = ${challengeId}
215+
AND (
216+
(r."submissionId" IS NOT NULL AND s."id" = r."submissionId")
217+
OR (r."legacySubmissionId" IS NOT NULL AND s."legacySubmissionId" = r."legacySubmissionId")
218+
)
219+
)
220+
`
221+
);
222+
} catch (err) {
223+
logger.error(
224+
`Failed to check pending appeal responses for challenge ${challengeId}: ${err.message}`,
225+
err
226+
);
227+
throw err;
228+
}
229+
230+
const [{ count = 0 } = {}] = rows || [];
231+
return Number(count) > 0;
232+
}
233+
183234
async function checkChallengeExists(challengeId) {
184235
const challenge = await prisma.challenge.findUnique({ where: { id: challengeId } });
185236
if (!challenge) {
@@ -419,13 +470,22 @@ async function partiallyUpdateChallengePhase(currentUser, challengeId, id, data)
419470
const isReopeningPhase =
420471
"isOpen" in data && data["isOpen"] === true && !challengePhase.isOpen;
421472
if (isClosingPhase) {
473+
const closingPhaseName = data.name || challengePhase.name;
422474
const pendingScorecards = await hasPendingScorecardsForPhase(challengePhase.id);
423475
if (pendingScorecards) {
424-
const phaseName = challengePhase.name || "phase";
476+
const phaseName = closingPhaseName || "phase";
425477
throw new errors.ForbiddenError(
426478
`Cannot close ${phaseName} because there are still pending scorecards`
427479
);
428480
}
481+
if (
482+
String(closingPhaseName || "").toLowerCase() === "appeals response" &&
483+
(await hasPendingAppealResponsesForChallenge(challengePhase.challengeId))
484+
) {
485+
throw new errors.BadRequestError(
486+
"Appeals Response phase can't be closed because there are still appeals that haven't been responded to"
487+
);
488+
}
429489
if (!("actualEndDate" in data) || _.isNil(data.actualEndDate)) {
430490
data.actualEndDate = new Date();
431491
}

‎test/unit/ChallengePhaseService.test.js‎

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,10 +81,17 @@ describe('challenge phase service unit tests', () => {
8181
"reviewItemCommentId" varchar(14) NOT NULL
8282
)
8383
`)
84+
await reviewClient.$executeRawUnsafe(`
85+
CREATE TABLE IF NOT EXISTS "${reviewSchema}"."appealResponse" (
86+
"id" varchar(14) PRIMARY KEY,
87+
"appealId" varchar(14) UNIQUE NOT NULL
88+
)
89+
`)
8490
})
8591

8692
after(async () => {
8793
if (reviewClient) {
94+
await reviewClient.$executeRawUnsafe(`TRUNCATE TABLE "${reviewSchema}"."appealResponse"`)
8895
await reviewClient.$executeRawUnsafe(`TRUNCATE TABLE "${reviewSchema}"."appeal"`)
8996
await reviewClient.$executeRawUnsafe(`TRUNCATE TABLE "${reviewSchema}"."reviewItemComment"`)
9097
await reviewClient.$executeRawUnsafe(`TRUNCATE TABLE "${reviewSchema}"."reviewItem"`)
@@ -696,6 +703,126 @@ describe('challenge phase service unit tests', () => {
696703
}
697704
})
698705

706+
it('partially update challenge phase - cannot close Appeals Response when appeals lack responses', async function () {
707+
this.timeout(50000)
708+
const appealsPhaseId = uuid()
709+
const appealsChallengePhaseId = uuid()
710+
const submissionId = shortId()
711+
const reviewId = uuid()
712+
const reviewItemId = shortId()
713+
const reviewItemCommentId = shortId()
714+
const appealId = shortId()
715+
716+
await prisma.phase.create({
717+
data: {
718+
id: appealsPhaseId,
719+
name: 'Appeals Response',
720+
description: 'Appeals Response phase',
721+
isOpen: true,
722+
duration: 123,
723+
createdBy: 'testuser',
724+
updatedBy: 'testuser'
725+
}
726+
})
727+
728+
await prisma.challengePhase.create({
729+
data: {
730+
id: appealsChallengePhaseId,
731+
challengeId: data.challenge.id,
732+
phaseId: appealsPhaseId,
733+
name: 'Appeals Response',
734+
isOpen: true,
735+
duration: 1000,
736+
actualStartDate: new Date(),
737+
createdBy: 'testuser',
738+
updatedBy: 'testuser'
739+
}
740+
})
741+
742+
try {
743+
await reviewClient.$executeRaw(
744+
Prisma.sql`
745+
INSERT INTO ${submissionTable} ("id", "challengeId")
746+
VALUES (${submissionId}, ${data.challenge.id})
747+
ON CONFLICT ("id") DO NOTHING
748+
`
749+
)
750+
751+
await reviewClient.$executeRaw(
752+
Prisma.sql`
753+
INSERT INTO ${reviewTable} ("id", "phaseId", "submissionId", "status")
754+
VALUES (${reviewId}, ${appealsChallengePhaseId}, ${submissionId}, ${'COMPLETED'})
755+
ON CONFLICT ("id") DO NOTHING
756+
`
757+
)
758+
759+
await reviewClient.$executeRaw(
760+
Prisma.sql`
761+
INSERT INTO ${reviewItemTable} ("id", "reviewId")
762+
VALUES (${reviewItemId}, ${reviewId})
763+
ON CONFLICT ("id") DO NOTHING
764+
`
765+
)
766+
767+
await reviewClient.$executeRaw(
768+
Prisma.sql`
769+
INSERT INTO ${reviewItemCommentTable} ("id", "reviewItemId")
770+
VALUES (${reviewItemCommentId}, ${reviewItemId})
771+
ON CONFLICT ("id") DO NOTHING
772+
`
773+
)
774+
775+
await reviewClient.$executeRaw(
776+
Prisma.sql`
777+
INSERT INTO ${appealTable} ("id", "reviewItemCommentId")
778+
VALUES (${appealId}, ${reviewItemCommentId})
779+
ON CONFLICT ("id") DO NOTHING
780+
`
781+
)
782+
783+
let caughtError
784+
try {
785+
await service.partiallyUpdateChallengePhase(
786+
authUser,
787+
data.challenge.id,
788+
appealsChallengePhaseId,
789+
{ isOpen: false }
790+
)
791+
} catch (e) {
792+
caughtError = e
793+
}
794+
795+
should.exist(caughtError)
796+
should.equal(caughtError.httpStatus || caughtError.statusCode, 400)
797+
should.equal(
798+
caughtError.message,
799+
"Appeals Response phase can't be closed because there are still appeals that haven't been responded to"
800+
)
801+
} finally {
802+
await reviewClient.$executeRaw(
803+
Prisma.sql`DELETE FROM ${appealTable} WHERE "id" = ${appealId}`
804+
)
805+
await reviewClient.$executeRaw(
806+
Prisma.sql`DELETE FROM ${reviewItemCommentTable} WHERE "id" = ${reviewItemCommentId}`
807+
)
808+
await reviewClient.$executeRaw(
809+
Prisma.sql`DELETE FROM ${reviewItemTable} WHERE "id" = ${reviewItemId}`
810+
)
811+
await reviewClient.$executeRaw(
812+
Prisma.sql`DELETE FROM ${reviewTable} WHERE "id" = ${reviewId}`
813+
)
814+
await reviewClient.$executeRaw(
815+
Prisma.sql`DELETE FROM ${submissionTable} WHERE "id" = ${submissionId}`
816+
)
817+
await prisma.challengePhase.delete({
818+
where: { id: appealsChallengePhaseId }
819+
})
820+
await prisma.phase.delete({
821+
where: { id: appealsPhaseId }
822+
})
823+
}
824+
})
825+
699826
it('partially update challenge phase - unexpected field', async () => {
700827
try {
701828
await service.partiallyUpdateChallengePhase(authUser, data.challenge.id, data.challengePhase1Id, { name: 'xx', other: 'xx' })

0 commit comments

Comments
 (0)