Skip to content

Commit 91e3c5a

Browse files
jmgasperjmgasper
committed
Failed approval phase handling
1 parent 07bc173 commit 91e3c5a

File tree

3 files changed

+264
-47
lines changed

3 files changed

+264
-47
lines changed

‎src/autopilot/services/autopilot.service.spec.ts‎

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,8 @@ describe('AutopilotService - handleSubmissionNotificationAggregate', () => {
106106
getChallengeById: jest.fn(),
107107
advancePhase: jest.fn(),
108108
getPhaseTypeName: jest.fn(),
109+
createIterativeReviewPhase: jest.fn(),
110+
createApprovalPhase: jest.fn(),
109111
} as unknown as jest.Mocked<ChallengeApiService>;
110112

111113
reviewService = {
@@ -403,6 +405,171 @@ describe('AutopilotService - handleSubmissionNotificationAggregate', () => {
403405
});
404406
});
405407

408+
describe('handleReviewCompleted (approval phase)', () => {
409+
const buildApprovalPhase = (): IPhase => ({
410+
id: 'phase-approval',
411+
phaseId: 'template-approval',
412+
name: 'Approval',
413+
description: 'Approval Phase',
414+
isOpen: true,
415+
duration: 7200,
416+
scheduledStartDate: new Date().toISOString(),
417+
scheduledEndDate: new Date(Date.now() + 2 * 3600 * 1000).toISOString(),
418+
actualStartDate: new Date().toISOString(),
419+
actualEndDate: null,
420+
predecessor: null,
421+
constraints: [],
422+
});
423+
424+
const buildChallenge = (phase: IPhase): IChallenge => ({
425+
id: 'challenge-approval',
426+
name: 'Approval Challenge',
427+
description: null,
428+
descriptionFormat: 'markdown',
429+
projectId: 999,
430+
typeId: 'type-approval',
431+
trackId: 'track-approval',
432+
timelineTemplateId: 'timeline-approval',
433+
currentPhaseNames: [phase.name],
434+
tags: [],
435+
groups: [],
436+
submissionStartDate: new Date().toISOString(),
437+
submissionEndDate: new Date().toISOString(),
438+
registrationStartDate: new Date().toISOString(),
439+
registrationEndDate: new Date().toISOString(),
440+
startDate: new Date().toISOString(),
441+
endDate: null,
442+
legacyId: null,
443+
status: 'ACTIVE',
444+
createdBy: 'tester',
445+
updatedBy: 'tester',
446+
metadata: {},
447+
phases: [phase],
448+
reviewers: [],
449+
winners: [],
450+
discussions: [],
451+
events: [],
452+
prizeSets: [],
453+
terms: [],
454+
skills: [],
455+
attachments: [],
456+
track: 'DEVELOP',
457+
type: 'Standard',
458+
legacy: {},
459+
task: {},
460+
created: new Date().toISOString(),
461+
updated: new Date().toISOString(),
462+
overview: {},
463+
numOfSubmissions: 1,
464+
numOfCheckpointSubmissions: 0,
465+
numOfRegistrants: 0,
466+
});
467+
468+
const buildPayload = (
469+
overrides: Partial<ReviewCompletedPayload> = {},
470+
): ReviewCompletedPayload => ({
471+
challengeId: 'challenge-approval',
472+
reviewId: 'review-approval',
473+
submissionId: 'submission-approval',
474+
phaseId: 'phase-approval',
475+
scorecardId: 'scorecard-approval',
476+
reviewerResourceId: 'resource-approval',
477+
reviewerHandle: 'approver',
478+
reviewerMemberId: '123',
479+
submitterHandle: 'submitter',
480+
submitterMemberId: '456',
481+
completedAt: new Date().toISOString(),
482+
initialScore: 92,
483+
...overrides,
484+
});
485+
486+
beforeEach(() => {
487+
const approvalPhase = buildApprovalPhase();
488+
challengeApiService.getChallengeById.mockResolvedValue(
489+
buildChallenge(approvalPhase),
490+
);
491+
492+
reviewService.getReviewById.mockResolvedValue({
493+
id: 'review-approval',
494+
phaseId: approvalPhase.id,
495+
resourceId: 'resource-approval',
496+
submissionId: 'submission-approval',
497+
scorecardId: 'scorecard-approval',
498+
score: 0,
499+
status: 'COMPLETED',
500+
});
501+
});
502+
503+
it('closes the approval phase without creating a follow-up when the score meets the minimum', async () => {
504+
reviewService.getReviewById.mockResolvedValueOnce({
505+
id: 'review-approval',
506+
phaseId: 'phase-approval',
507+
resourceId: 'resource-approval',
508+
submissionId: 'submission-approval',
509+
scorecardId: 'scorecard-approval',
510+
score: 95,
511+
status: 'COMPLETED',
512+
});
513+
reviewService.getScorecardPassingScore.mockResolvedValueOnce(75);
514+
515+
await autopilotService.handleReviewCompleted(buildPayload());
516+
517+
expect(schedulerService.advancePhase).toHaveBeenCalledWith(
518+
expect.objectContaining({
519+
challengeId: 'challenge-approval',
520+
phaseId: 'phase-approval',
521+
state: 'END',
522+
}),
523+
);
524+
expect(challengeApiService.createApprovalPhase).not.toHaveBeenCalled();
525+
expect(phaseReviewService.handlePhaseOpened).not.toHaveBeenCalled();
526+
});
527+
528+
it('creates a follow-up approval phase when the score is below the minimum', async () => {
529+
reviewService.getScorecardPassingScore.mockResolvedValueOnce(90);
530+
reviewService.getReviewById.mockResolvedValueOnce({
531+
id: 'review-approval',
532+
phaseId: 'phase-approval',
533+
resourceId: 'resource-approval',
534+
submissionId: 'submission-approval',
535+
scorecardId: 'scorecard-approval',
536+
score: 72,
537+
status: 'COMPLETED',
538+
});
539+
540+
const followUpPhase: IPhase = {
541+
...buildApprovalPhase(),
542+
id: 'phase-approval-next',
543+
isOpen: true,
544+
};
545+
challengeApiService.createApprovalPhase.mockResolvedValueOnce(
546+
followUpPhase,
547+
);
548+
549+
await autopilotService.handleReviewCompleted(buildPayload());
550+
551+
expect(schedulerService.advancePhase).toHaveBeenCalledWith(
552+
expect.objectContaining({
553+
challengeId: 'challenge-approval',
554+
phaseId: 'phase-approval',
555+
state: 'END',
556+
}),
557+
);
558+
expect(challengeApiService.createApprovalPhase).toHaveBeenCalledWith(
559+
'challenge-approval',
560+
'phase-approval',
561+
'template-approval',
562+
'Approval',
563+
'Approval Phase',
564+
expect.any(Number),
565+
);
566+
expect(phaseReviewService.handlePhaseOpened).toHaveBeenCalledWith(
567+
'challenge-approval',
568+
'phase-approval-next',
569+
);
570+
});
571+
});
572+
406573
describe('handleReviewCompleted (post-mortem)', () => {
407574
const buildPhase = (name = POST_MORTEM_PHASE_NAME): IPhase => ({
408575
id: 'phase-1',

‎src/autopilot/services/autopilot.service.ts‎

Lines changed: 51 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -294,20 +294,24 @@ export class AutopilotService {
294294
return;
295295
}
296296

297-
// Special handling: Approval phase pass/fail adds additional Approval if failed
297+
// Approval: compare against minimum passing score and create a follow-up Approval phase if it fails
298298
if (APPROVAL_PHASE_NAMES.has(phase.name)) {
299-
const passingScore = await this.reviewService.getScorecardPassingScore(
300-
review.scorecardId,
301-
);
302-
const rawScore =
303-
typeof review.score === 'number'
304-
? review.score
305-
: Number(review.score ?? payload.initialScore ?? 0);
306-
const finalScore = Number.isFinite(rawScore)
307-
? Number(rawScore)
308-
: Number(payload.initialScore ?? 0);
309-
310-
// Close current approval phase
299+
const scorecardId = review.scorecardId ?? payload.scorecardId ?? null;
300+
const passingScore =
301+
await this.reviewService.getScorecardPassingScore(scorecardId);
302+
303+
const normalizedScore = (() => {
304+
if (typeof review.score === 'number') {
305+
return review.score;
306+
}
307+
const numeric = Number(review.score ?? payload.initialScore ?? 0);
308+
if (Number.isFinite(numeric)) {
309+
return numeric;
310+
}
311+
const fallback = Number(payload.initialScore ?? 0);
312+
return Number.isFinite(fallback) ? fallback : 0;
313+
})();
314+
311315
await this.schedulerService.advancePhase({
312316
projectId: challenge.projectId,
313317
challengeId: challenge.id,
@@ -318,37 +322,44 @@ export class AutopilotService {
318322
projectStatus: challenge.status,
319323
});
320324

321-
if (finalScore >= passingScore) {
325+
if (normalizedScore >= passingScore) {
322326
this.logger.log(
323-
`Approval review passed for challenge ${challenge.id} (score ${finalScore} / passing ${passingScore}).`,
327+
`Approval review passed for challenge ${challenge.id} (score ${normalizedScore} / passing ${passingScore}).`,
324328
);
325-
} else {
326-
this.logger.log(
327-
`Approval review failed for challenge ${challenge.id} (score ${finalScore} / passing ${passingScore}). Creating another Approval phase.`,
329+
return;
330+
}
331+
332+
this.logger.log(
333+
`Approval review failed for challenge ${challenge.id} (score ${normalizedScore} / passing ${passingScore}). Creating another Approval phase.`,
334+
);
335+
336+
if (!phase.phaseId) {
337+
this.logger.error(
338+
`Cannot create follow-up Approval phase for challenge ${challenge.id}; missing phase template ID on phase ${phase.id}.`,
328339
);
329-
try {
330-
const nextApproval =
331-
await this.challengeApiService.createIterativeReviewPhase(
332-
challenge.id,
333-
phase.id,
334-
phase.phaseId!,
335-
phase.name,
336-
phase.description ?? null,
337-
Math.max(phase.duration || 0, 1),
338-
);
340+
return;
341+
}
339342

340-
// Create pending reviews for the newly opened Approval
341-
await this.phaseReviewService.handlePhaseOpened(
342-
challenge.id,
343-
nextApproval.id,
344-
);
345-
} catch (error) {
346-
const err = error as Error;
347-
this.logger.error(
348-
`Failed to create next Approval phase for challenge ${challenge.id}: ${err.message}`,
349-
err.stack,
350-
);
351-
}
343+
try {
344+
const nextApproval = await this.challengeApiService.createApprovalPhase(
345+
challenge.id,
346+
phase.id,
347+
phase.phaseId,
348+
phase.name,
349+
phase.description ?? null,
350+
Math.max(phase.duration || 0, 1),
351+
);
352+
353+
await this.phaseReviewService.handlePhaseOpened(
354+
challenge.id,
355+
nextApproval.id,
356+
);
357+
} catch (error) {
358+
const err = error as Error;
359+
this.logger.error(
360+
`Failed to create follow-up Approval phase for challenge ${challenge.id}: ${err.message}`,
361+
err.stack,
362+
);
352363
}
353364
return;
354365
}

‎src/challenge/challenge-api.service.ts‎

Lines changed: 46 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1161,7 +1161,8 @@ export class ChallengeApiService {
11611161
}
11621162
}
11631163

1164-
async createIterativeReviewPhase(
1164+
private async createContinuationPhase(
1165+
logAction: string,
11651166
challengeId: string,
11661167
predecessorPhaseId: string,
11671168
phaseTypeId: string,
@@ -1181,7 +1182,7 @@ export class ChallengeApiService {
11811182

11821183
if (!challenge) {
11831184
throw new NotFoundException(
1184-
`Challenge with ID ${challengeId} not found when creating iterative review phase.`,
1185+
`Challenge with ID ${challengeId} not found when creating follow-up phase ${phaseName}.`,
11851186
);
11861187
}
11871188

@@ -1191,7 +1192,7 @@ export class ChallengeApiService {
11911192

11921193
if (!predecessorPhase) {
11931194
throw new NotFoundException(
1194-
`Predecessor phase ${predecessorPhaseId} not found for challenge ${challengeId}.`,
1195+
`Predecessor phase ${predecessorPhaseId} not found for challenge ${challengeId} when creating follow-up phase ${phaseName}.`,
11951196
);
11961197
}
11971198

@@ -1237,13 +1238,13 @@ export class ChallengeApiService {
12371238

12381239
if (!phaseRecord) {
12391240
throw new Error(
1240-
`Created iterative review phase ${newPhaseId} not found after insertion for challenge ${challengeId}.`,
1241+
`Created follow-up phase ${newPhaseId} not found after insertion for challenge ${challengeId}.`,
12411242
);
12421243
}
12431244

12441245
const mapped = this.mapPhase(phaseRecord);
12451246

1246-
void this.dbLogger.logAction('challenge.createIterativeReviewPhase', {
1247+
void this.dbLogger.logAction(logAction, {
12471248
challengeId,
12481249
status: 'SUCCESS',
12491250
source: ChallengeApiService.name,
@@ -1258,7 +1259,7 @@ export class ChallengeApiService {
12581259
return mapped;
12591260
} catch (error) {
12601261
const err = error as Error;
1261-
void this.dbLogger.logAction('challenge.createIterativeReviewPhase', {
1262+
void this.dbLogger.logAction(logAction, {
12621263
challengeId,
12631264
status: 'ERROR',
12641265
source: ChallengeApiService.name,
@@ -1268,10 +1269,48 @@ export class ChallengeApiService {
12681269
error: err.message,
12691270
},
12701271
});
1271-
throw error;
1272+
throw err;
12721273
}
12731274
}
12741275

1276+
async createIterativeReviewPhase(
1277+
challengeId: string,
1278+
predecessorPhaseId: string,
1279+
phaseTypeId: string,
1280+
phaseName: string,
1281+
phaseDescription: string | null,
1282+
durationSeconds: number,
1283+
): Promise<IPhase> {
1284+
return this.createContinuationPhase(
1285+
'challenge.createIterativeReviewPhase',
1286+
challengeId,
1287+
predecessorPhaseId,
1288+
phaseTypeId,
1289+
phaseName,
1290+
phaseDescription,
1291+
durationSeconds,
1292+
);
1293+
}
1294+
1295+
async createApprovalPhase(
1296+
challengeId: string,
1297+
predecessorPhaseId: string,
1298+
phaseTypeId: string,
1299+
phaseName: string,
1300+
phaseDescription: string | null,
1301+
durationSeconds: number,
1302+
): Promise<IPhase> {
1303+
return this.createContinuationPhase(
1304+
'challenge.createApprovalPhase',
1305+
challengeId,
1306+
predecessorPhaseId,
1307+
phaseTypeId,
1308+
phaseName,
1309+
phaseDescription,
1310+
durationSeconds,
1311+
);
1312+
}
1313+
12751314
async completeChallenge(
12761315
challengeId: string,
12771316
winners: IChallengeWinner[],

0 commit comments

Comments
 (0)