From 230a85700d6cd6e89e2eca6602fc7d2271390460 Mon Sep 17 00:00:00 2001 From: Ranjeet Prabhat Date: Wed, 14 Oct 2026 07:30:40 +0545 Subject: [PATCH] feat(grading): validate tutor review scores and feedback (BA-043) --- .../src/grading/dto/tutor-review.dto.ts | 14 ++- .../src/grading/grading.controller.ts | 36 +++++- .../src/grading/grading.service.spec.ts | 116 ++++++++++++++++++ 3 files changed, 160 insertions(+), 6 deletions(-) diff --git a/BackendAcademy/src/grading/dto/tutor-review.dto.ts b/BackendAcademy/src/grading/dto/tutor-review.dto.ts index cdb649175e..05f7763a64 100644 --- a/BackendAcademy/src/grading/dto/tutor-review.dto.ts +++ b/BackendAcademy/src/grading/dto/tutor-review.dto.ts @@ -1,4 +1,4 @@ -import { IsIn, IsInt, IsNotEmpty, IsString, Max, Min, ValidateIf } from 'class-validator'; +import { IsIn, IsInt, IsNotEmpty, IsOptional, IsString, Max, Min, ValidateIf } from 'class-validator'; import { TutorDecision } from '../grading.types'; export class TutorReviewDto { @@ -14,16 +14,22 @@ export class TutorReviewDto { @IsIn(['confirm', 'override']) decision: TutorDecision; - /** Required for an override: the tutor's replacement score. */ - @ValidateIf((dto: TutorReviewDto) => dto.decision === 'override') + /** Required for an override: the tutor's replacement score (0 to 100). */ + @ValidateIf((dto: TutorReviewDto) => dto.decision === 'override' || dto.score !== undefined) @IsInt() @Min(0) @Max(100) score?: number; /** Required for an override: why the AI pre-score was changed. */ - @ValidateIf((dto: TutorReviewDto) => dto.decision === 'override') + @ValidateIf((dto: TutorReviewDto) => dto.decision === 'override' && !dto.feedback) @IsString() @IsNotEmpty() reason?: string; + + /** Tutor review feedback for the learner or review audit. */ + @ValidateIf((dto: TutorReviewDto) => (dto.decision === 'override' && !dto.reason) || dto.feedback !== undefined) + @IsString() + @IsNotEmpty() + feedback?: string; } diff --git a/BackendAcademy/src/grading/grading.controller.ts b/BackendAcademy/src/grading/grading.controller.ts index 381bf41747..481a224db9 100644 --- a/BackendAcademy/src/grading/grading.controller.ts +++ b/BackendAcademy/src/grading/grading.controller.ts @@ -1,4 +1,4 @@ -import { Body, Controller, Get, Param, Post } from '@nestjs/common'; +import { BadRequestException, Body, Controller, Get, Param, Post } from '@nestjs/common'; import { ApiOperation, ApiTags } from '@nestjs/swagger'; import { CreateSubmissionDto } from './dto/create-submission.dto'; import { TutorReviewDto } from './dto/tutor-review.dto'; @@ -25,7 +25,39 @@ export class GradingController { @Post('submissions/:submissionId/review') @ApiOperation({ summary: 'Stage 2 — tutor confirms or overrides the AI pre-score' }) tutorReview(@Param('submissionId') submissionId: string, @Body() dto: TutorReviewDto) { - return this.gradingService.tutorReview(submissionId, dto); + // Validate score bounds whenever a score is provided + if (dto.score !== undefined && dto.score !== null) { + if (typeof dto.score !== 'number' || !Number.isInteger(dto.score) || dto.score < 0 || dto.score > 100) { + throw new BadRequestException({ + error: 'Score must be an integer between 0 and 100 inclusive.', + code: 'INVALID_SCORE_BOUNDS', + }); + } + } + + if (dto.decision === 'override') { + if (dto.score === undefined || dto.score === null) { + throw new BadRequestException({ + error: 'An override must include both the replacement score and a reason/feedback.', + code: 'OVERRIDE_REQUIRES_SCORE_AND_REASON', + }); + } + + const feedbackText = dto.feedback ?? dto.reason; + if (!feedbackText || typeof feedbackText !== 'string' || feedbackText.trim() === '') { + throw new BadRequestException({ + error: 'An override must include non-empty feedback or a reason.', + code: 'OVERRIDE_REQUIRES_FEEDBACK', + }); + } + } + + const input = { + ...dto, + reason: dto.reason ?? dto.feedback, + feedback: dto.feedback ?? dto.reason, + }; + return this.gradingService.tutorReview(submissionId, input); } @Get('submissions/:submissionId') diff --git a/BackendAcademy/src/grading/grading.service.spec.ts b/BackendAcademy/src/grading/grading.service.spec.ts index 308f8b4721..2d17b5fbca 100644 --- a/BackendAcademy/src/grading/grading.service.spec.ts +++ b/BackendAcademy/src/grading/grading.service.spec.ts @@ -5,6 +5,8 @@ import { DomainEventBus } from '../gamification/domain-event-bus'; import { XpService } from '../gamification/xp.service'; import { GRADING_PASS_THRESHOLD } from './grading.types'; import { GradingService } from './grading.service'; +import { GradingController } from './grading.controller'; +import { TutorReviewDto } from './dto/tutor-review.dto'; const PASSING_AI_RESPONSE = { text: '{"score": 82, "feedback": "Clear ownership handling; add error handling for the unwrap."}', @@ -290,4 +292,118 @@ describe('GradingService', () => { xpService.onModuleDestroy(); }); }); + + // ── BA-043: Validate tutor review scores and feedback ──────────────────── + + describe('BA-043 — Validate tutor review scores and feedback', () => { + let controller: GradingController; + let service: GradingService; + + beforeEach(async () => { + const created = createService(); + service = created.service; + controller = new GradingController(service); + }); + + it('accepts override reviews at score boundaries (0 and 100)', async () => { + const id1 = await gradedSubmission(service); + const res1 = controller.tutorReview(id1, { + tutorId: 'tutor-1', + decision: 'override', + score: 0, + reason: 'Complete rewrite needed.', + }); + expect(res1.finalScore).toBe(0); + expect(res1.status).toBe('tutor_overridden'); + + const id2 = await gradedSubmission(service); + const res2 = controller.tutorReview(id2, { + tutorId: 'tutor-1', + decision: 'override', + score: 100, + feedback: 'Flawless submission with edge tests.', + }); + expect(res2.finalScore).toBe(100); + expect(res2.status).toBe('tutor_overridden'); + }); + + it('rejects override reviews with score below 0 with BadRequestException', async () => { + const id = await gradedSubmission(service); + expect(() => + controller.tutorReview(id, { + tutorId: 'tutor-1', + decision: 'override', + score: -1, + reason: 'Below minimum score', + }), + ).toThrow(BadRequestException); + }); + + it('rejects override reviews with score above 100 with BadRequestException', async () => { + const id = await gradedSubmission(service); + expect(() => + controller.tutorReview(id, { + tutorId: 'tutor-1', + decision: 'override', + score: 101, + reason: 'Above maximum score', + }), + ).toThrow(BadRequestException); + }); + + it('rejects fractional/non-integer scores with BadRequestException', async () => { + const id = await gradedSubmission(service); + expect(() => + controller.tutorReview(id, { + tutorId: 'tutor-1', + decision: 'override', + score: 85.5, + reason: 'Non-integer score', + }), + ).toThrow(BadRequestException); + }); + + it('rejects override when both reason and feedback are missing or empty', async () => { + const id1 = await gradedSubmission(service); + expect(() => + controller.tutorReview(id1, { + tutorId: 'tutor-1', + decision: 'override', + score: 75, + }), + ).toThrow(BadRequestException); + + const id2 = await gradedSubmission(service); + expect(() => + controller.tutorReview(id2, { + tutorId: 'tutor-1', + decision: 'override', + score: 75, + reason: ' ', + }), + ).toThrow(BadRequestException); + }); + + it('accepts override with feedback field in place of reason', async () => { + const id = await gradedSubmission(service); + const res = controller.tutorReview(id, { + tutorId: 'tutor-1', + decision: 'override', + score: 90, + feedback: 'Good implementation with clean types.', + }); + expect(res.finalScore).toBe(90); + expect(res.tutorReview?.reason).toBe('Good implementation with clean types.'); + }); + + it('allows confirm review without override score or feedback', async () => { + const id = await gradedSubmission(service); + const res = controller.tutorReview(id, { + tutorId: 'tutor-1', + decision: 'confirm', + }); + expect(res.status).toBe('tutor_confirmed'); + expect(res.finalScore).toBe(82); + }); + }); });