Skip to content

Commit 383bbab

Browse files
authored
Exam mode: Improve exam validation (#11960)
1 parent 62cf410 commit 383bbab

7 files changed

Lines changed: 293 additions & 11 deletions

File tree

src/main/java/de/tum/cit/aet/artemis/exam/web/ExamResource.java

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -410,6 +410,7 @@ public ResponseEntity<Exam> importExamWithExercises(@PathVariable Long courseId,
410410
private void checkForExamConflictsElseThrow(Long courseId, Exam exam) {
411411
checkExamCourseIdElseThrow(courseId, exam);
412412
checkExamForDatesConflictsElseThrow(exam);
413+
checkExamNumericFieldLimitsElseThrow(exam);
413414
checkExamForWorkingTimeConflictsElseThrow(exam);
414415
checkExamPointsAndCorrectionRoundsElseThrow(exam);
415416

@@ -432,6 +433,40 @@ private void checkExamCourseIdElseThrow(Long courseId, Exam exam) {
432433
}
433434
}
434435

436+
/**
437+
* Validates numeric field limits for exam configuration.
438+
* Maximum values:
439+
* - Working time: 2592000 seconds (30 days)
440+
* - Grace period: 3600 seconds (1 hour)
441+
* - Exam max points: 9999
442+
* - Number of exercises: 100
443+
*
444+
* @param exam the exam to be checked
445+
*/
446+
private void checkExamNumericFieldLimitsElseThrow(Exam exam) {
447+
// Max working time: 30 days = 2592000 seconds
448+
final int maxWorkingTimeSeconds = 2_592_000;
449+
final int workingTimeToCheck = exam.isTestExam() ? exam.getWorkingTime() : exam.getDuration();
450+
if (workingTimeToCheck > maxWorkingTimeSeconds) {
451+
throw new BadRequestAlertException("The working time is too long. Maximum allowed is 30 days (43200 minutes).", ENTITY_NAME, "examWorkingTimeTooHigh");
452+
}
453+
454+
// Grace period: max 1 hour = 3600 seconds
455+
if (exam.getGracePeriod() != null && exam.getGracePeriod() > 3600) {
456+
throw new BadRequestAlertException("The grace period is too long. Maximum allowed is 3600 seconds.", ENTITY_NAME, "examGracePeriodTooHigh");
457+
}
458+
459+
// Max points: max 9999
460+
if (exam.getExamMaxPoints() > 9999) {
461+
throw new BadRequestAlertException("The maximum points value is too high. Maximum allowed is 9999.", ENTITY_NAME, "examMaxPointsTooHigh");
462+
}
463+
464+
// Number of exercises: max 100
465+
if (exam.getNumberOfExercisesInExam() != null && exam.getNumberOfExercisesInExam() > 100) {
466+
throw new BadRequestAlertException("The number of exercises is too high. Maximum allowed is 100.", ENTITY_NAME, "examNumberOfExercisesTooHigh");
467+
}
468+
}
469+
435470
/**
436471
* Checks that the visible/start/end-dates are present and in the correct order.
437472
* For real exams: visibleDate < startDate < endDate

src/main/webapp/app/exam/manage/exams/update/exam-update.component.html

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,10 @@ <h4 jhiTranslate="artemisApp.examManagement.editExam"></h4>
1717
<div>
1818
<hr />
1919
<h5 class="pb-1" jhiTranslate="artemisApp.examManagement.sections.configuration"></h5>
20-
<label class="form-check-label" jhiTranslate="artemisApp.examManagement.examTitle"></label>
2120
<jhi-title-channel-name
2221
[(title)]="exam.title"
2322
[(channelName)]="exam.channelName"
2423
channelNamePrefix="exam-"
25-
[hideTitleLabel]="true"
2624
[hideChannelName]="hideChannelNameInput"
2725
[initChannelName]="isImport || exam.id === undefined"
2826
/>
@@ -127,6 +125,9 @@ <h5 class="pb-1" jhiTranslate="artemisApp.examManagement.sections.conduction"></
127125
<jhi-help-icon [text]="'artemisApp.examManagement' + (exam.testExam ? '.testExam' : '') + '.workingTimeTooltip'" />
128126
@if (!exam.testExam) {
129127
<input readonly disabled type="text" class="form-control" [value]="workingTimeInMinutesRounded" />
128+
@if (isWorkingTimeTooHigh) {
129+
<div class="alert alert-danger" jhiTranslate="error.examWorkingTimeTooHigh"></div>
130+
}
130131
}
131132
@if (exam.testExam) {
132133
<input
@@ -139,13 +140,20 @@ <h5 class="pb-1" jhiTranslate="artemisApp.examManagement.sections.conduction"></
139140
[(ngModel)]="workingTimeInMinutes"
140141
(focusin)="roundWorkingTime()"
141142
(focusout)="roundWorkingTime()"
143+
#workingTimeInput="ngModel"
142144
/>
145+
@if (workingTimeInput.errors?.['max'] || isWorkingTimeTooHigh) {
146+
<div class="alert alert-danger" jhiTranslate="error.examWorkingTimeTooHigh"></div>
147+
}
143148
}
144149
</div>
145150
<div class="col-sm-6">
146151
<label class="form-check-label" for="gracePeriod" jhiTranslate="artemisApp.examManagement.gracePeriod"></label>
147152
<jhi-help-icon text="artemisApp.examManagement.gracePeriodTooltip" />
148-
<input id="gracePeriod" name="gracePeriod" class="form-control" type="number" min="0" [(ngModel)]="exam.gracePeriod" />
153+
<input id="gracePeriod" name="gracePeriod" class="form-control" type="number" min="0" max="3600" [(ngModel)]="exam.gracePeriod" #gracePeriodInput="ngModel" />
154+
@if (gracePeriodInput.errors?.['max']) {
155+
<div class="alert alert-danger" jhiTranslate="error.examGracePeriodTooHigh"></div>
156+
}
149157
</div>
150158
</div>
151159
<hr />
@@ -155,14 +163,38 @@ <h5 class="pb-1" jhiTranslate="artemisApp.examManagement.sections.exercises"></h
155163
<div class="col-sm-6">
156164
<label class="form-check-label" for="examMaxPoints" jhiTranslate="artemisApp.examManagement.maxPoints.title"></label>
157165
<fa-icon [icon]="faExclamationTriangle" class="text-warning" ngbTooltip="{{ 'artemisApp.examManagement.maxPoints.warning' | artemisTranslate }}" />
158-
<input id="examMaxPoints" name="examMaxPoints" class="form-control" type="number" min="1" [(ngModel)]="exam.examMaxPoints" />
166+
<input
167+
id="examMaxPoints"
168+
name="examMaxPoints"
169+
class="form-control"
170+
type="number"
171+
min="1"
172+
max="9999"
173+
[(ngModel)]="exam.examMaxPoints"
174+
#examMaxPointsInput="ngModel"
175+
/>
176+
@if (examMaxPointsInput.errors?.['max']) {
177+
<div class="alert alert-danger" jhiTranslate="error.examMaxPointsTooHigh"></div>
178+
}
159179
</div>
160180
<div class="col-sm-6">
161181
<label class="form-check-label" for="numberOfExercisesInExam" jhiTranslate="artemisApp.examManagement.numberOfExercisesInExam"
162182
>Number of Exercises in Exam</label
163183
>
164184
<jhi-help-icon [text]="'artemisApp.examManagement.numberOfExercisesInExamTooltip'" />
165-
<input id="numberOfExercisesInExam" name="numberOfExercisesInExam" class="form-control" type="number" min="1" [(ngModel)]="exam.numberOfExercisesInExam" />
185+
<input
186+
id="numberOfExercisesInExam"
187+
name="numberOfExercisesInExam"
188+
class="form-control"
189+
type="number"
190+
min="1"
191+
max="100"
192+
[(ngModel)]="exam.numberOfExercisesInExam"
193+
#numberOfExercisesInput="ngModel"
194+
/>
195+
@if (numberOfExercisesInput.errors?.['max']) {
196+
<div class="alert alert-danger" jhiTranslate="error.examNumberOfExercisesTooHigh"></div>
197+
}
166198
</div>
167199
</div>
168200
<div class="form-check mb-3">

src/main/webapp/app/exam/manage/exams/update/exam-update.component.spec.ts

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -575,6 +575,79 @@ describe('ExamUpdateComponent', () => {
575575
fixture.changeDetectorRef.detectChanges();
576576
expect(component.exam.numberOfExercisesInExam).toBe(40);
577577
expect(component.isValidNumberOfExercises).toBeTrue();
578+
579+
// Test max limit of 100
580+
examWithoutExercises.numberOfExercisesInExam = 100;
581+
fixture.changeDetectorRef.detectChanges();
582+
expect(component.exam.numberOfExercisesInExam).toBe(100);
583+
expect(component.isValidNumberOfExercises).toBeTrue();
584+
585+
examWithoutExercises.numberOfExercisesInExam = 101;
586+
fixture.changeDetectorRef.detectChanges();
587+
expect(component.exam.numberOfExercisesInExam).toBe(101);
588+
expect(component.isValidNumberOfExercises).toBeFalse();
589+
});
590+
591+
it('should correctly validate max points with upper limit of 9999', () => {
592+
fixture.detectChanges();
593+
594+
examWithoutExercises.examMaxPoints = 100;
595+
fixture.changeDetectorRef.detectChanges();
596+
expect(component.exam.examMaxPoints).toBe(100);
597+
expect(component.isValidMaxPoints).toBeTrue();
598+
599+
examWithoutExercises.examMaxPoints = 9999;
600+
fixture.changeDetectorRef.detectChanges();
601+
expect(component.exam.examMaxPoints).toBe(9999);
602+
expect(component.isValidMaxPoints).toBeTrue();
603+
604+
examWithoutExercises.examMaxPoints = 10000;
605+
fixture.changeDetectorRef.detectChanges();
606+
expect(component.exam.examMaxPoints).toBe(10000);
607+
expect(component.isValidMaxPoints).toBeFalse();
608+
});
609+
610+
it('should correctly validate grace period with upper limit of 3600 seconds', () => {
611+
fixture.detectChanges();
612+
613+
examWithoutExercises.gracePeriod = 180;
614+
fixture.changeDetectorRef.detectChanges();
615+
expect(component.exam.gracePeriod).toBe(180);
616+
expect(component.isValidGracePeriod).toBeTrue();
617+
618+
examWithoutExercises.gracePeriod = 3600;
619+
fixture.changeDetectorRef.detectChanges();
620+
expect(component.exam.gracePeriod).toBe(3600);
621+
expect(component.isValidGracePeriod).toBeTrue();
622+
623+
examWithoutExercises.gracePeriod = 3601;
624+
fixture.changeDetectorRef.detectChanges();
625+
expect(component.exam.gracePeriod).toBe(3601);
626+
expect(component.isValidGracePeriod).toBeFalse();
627+
});
628+
629+
it('should correctly validate working time with upper limit of 30 days (2592000 seconds)', () => {
630+
fixture.detectChanges();
631+
632+
// Set up exam as a test exam with a long exam window
633+
examWithoutExercises.testExam = true;
634+
examWithoutExercises.startDate = dayjs().add(0, 'hours');
635+
examWithoutExercises.endDate = dayjs().add(35, 'days'); // Long exam window
636+
637+
// Valid working time
638+
examWithoutExercises.workingTime = 86400; // 1 day
639+
expect(component.exam.workingTime).toBe(86400);
640+
expect(component.validateWorkingTime).toBeTrue();
641+
642+
// Working time at limit (30 days = 2592000 seconds)
643+
examWithoutExercises.workingTime = 2592000;
644+
expect(component.exam.workingTime).toBe(2592000);
645+
expect(component.validateWorkingTime).toBeTrue();
646+
647+
// Working time exceeds limit
648+
examWithoutExercises.workingTime = 2592001;
649+
expect(component.exam.workingTime).toBe(2592001);
650+
expect(component.validateWorkingTime).toBeFalse();
578651
});
579652

580653
it('should bind correct title into jhi-button and compute correct save title text', () => {

src/main/webapp/app/exam/manage/exams/update/exam-update.component.ts

Lines changed: 54 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,11 @@ export class ExamUpdateComponent implements OnInit, OnDestroy, AfterViewInit {
115115
this.course = data.course;
116116
this.exam.course = data.course;
117117

118+
// Prefill course name with course title for new exams
119+
if (!this.exam.id && !this.exam.courseName && this.course.title) {
120+
this.exam.courseName = this.course.title;
121+
}
122+
118123
if (!this.exam.startText) {
119124
this.exam.startText = this.examDefaultStartText;
120125
}
@@ -356,25 +361,30 @@ export class ExamUpdateComponent implements OnInit, OnDestroy, AfterViewInit {
356361
const examValidWorkingTime = this.validateWorkingTime;
357362
const examValidExampleSolutionPublicationDate = this.isValidExampleSolutionPublicationDate;
358363
const examValidNumberOfExercises = this.isValidNumberOfExercises;
364+
const examValidGracePeriod = this.isValidGracePeriod;
359365
return (
360366
examConductionDatesValid &&
361367
examReviewDatesValid &&
362368
examNumberOfCorrectionsValid &&
363369
examMaxPointsValid &&
364370
examValidWorkingTime &&
365371
examValidExampleSolutionPublicationDate &&
366-
examValidNumberOfExercises
372+
examValidNumberOfExercises &&
373+
examValidGracePeriod
367374
);
368375
}
369376

370377
/**
371378
* Returns a boolean indicating whether the exam's number of exercises is valid.
372-
* The number of exercises is valid if it's not set, or if it's at least 1.
379+
* The number of exercises is valid if it's not set, or if it's between 1 and 100.
373380
*
374381
* @returns {boolean} `true` if the exam's number of exercises is valid, `false` otherwise.
375382
*/
376383
get isValidNumberOfExercises(): boolean {
377-
return this.exam.numberOfExercisesInExam === undefined || this.exam.numberOfExercisesInExam === null || this.exam.numberOfExercisesInExam! >= 1;
384+
if (this.exam.numberOfExercisesInExam === undefined || this.exam.numberOfExercisesInExam === null) {
385+
return true;
386+
}
387+
return this.exam.numberOfExercisesInExam >= 1 && this.exam.numberOfExercisesInExam <= 100;
378388
}
379389

380390
/**
@@ -405,7 +415,20 @@ export class ExamUpdateComponent implements OnInit, OnDestroy, AfterViewInit {
405415
}
406416

407417
get isValidMaxPoints(): boolean {
408-
return !!this.exam?.examMaxPoints && this.exam?.examMaxPoints > 0;
418+
return !!this.exam?.examMaxPoints && this.exam?.examMaxPoints > 0 && this.exam?.examMaxPoints <= 9999;
419+
}
420+
421+
/**
422+
* Returns a boolean indicating whether the exam's grace period is valid.
423+
* The grace period is valid if it's not set, or if it's between 0 and 3600 seconds.
424+
*
425+
* @returns {boolean} `true` if the exam's grace period is valid, `false` otherwise.
426+
*/
427+
get isValidGracePeriod(): boolean {
428+
if (this.exam.gracePeriod === undefined || this.exam.gracePeriod === null) {
429+
return true;
430+
}
431+
return this.exam.gracePeriod >= 0 && this.exam.gracePeriod <= 3600;
409432
}
410433

411434
/**
@@ -470,27 +493,52 @@ export class ExamUpdateComponent implements OnInit, OnDestroy, AfterViewInit {
470493
return true;
471494
}
472495

496+
/**
497+
* Maximum working time in seconds (30 days).
498+
*/
499+
readonly maxWorkingTimeSeconds = 2592000;
500+
473501
/**
474502
* Validates the WorkingTime.
475-
* For test exams, the WorkingTime should be at least 1 and smaller / equal to the working window
476-
* For real exams, the WorkingTime is calculated based on the startDate and EndDate and should match the time difference.
503+
* For test exams, the WorkingTime should be at least 1 and smaller / equal to the working window,
504+
* and must not exceed 30 days (2592000 seconds).
505+
* For real exams, the WorkingTime is calculated based on the startDate and EndDate and should match the time difference,
506+
* and must not exceed 30 days (2592000 seconds).
477507
*/
478508
get validateWorkingTime(): boolean {
479509
if (this.exam.testExam) {
480510
if (this.exam.workingTime === undefined || this.exam.workingTime < 1) {
481511
return false;
482512
}
513+
// Check 30-day limit
514+
if (this.exam.workingTime > this.maxWorkingTimeSeconds) {
515+
return false;
516+
}
483517
if (this.exam.startDate && this.exam.endDate) {
484518
return this.exam.workingTime <= dayjs(this.exam.endDate).diff(this.exam.startDate, 's');
485519
}
486520
return false;
487521
}
488522
if (this.exam.workingTime && this.exam.startDate && this.exam.endDate) {
523+
// Check 30-day limit for real exams as well
524+
if (this.exam.workingTime > this.maxWorkingTimeSeconds) {
525+
return false;
526+
}
489527
return this.exam.workingTime === dayjs(this.exam.endDate).diff(this.exam.startDate, 's');
490528
}
491529
return false;
492530
}
493531

532+
/**
533+
* Returns true if the working time exceeds the maximum allowed limit of 30 days.
534+
*/
535+
get isWorkingTimeTooHigh(): boolean {
536+
if (this.exam.workingTime === undefined || this.exam.workingTime === null) {
537+
return false;
538+
}
539+
return this.exam.workingTime > this.maxWorkingTimeSeconds;
540+
}
541+
494542
get isValidPublishResultsDate(): boolean {
495543
// allow instructors to set publishResultsDate later
496544
if (!this.exam.publishResultsDate) {

src/main/webapp/i18n/de/error.json

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,16 @@
204204
"courseIdMissing": "Die Kurs ID ist erforderlich.",
205205
"forbidChangeCourseId": "Die Übungskurs-ID stimmt nicht mit der gespeicherten Kurs-ID überein"
206206
},
207+
"noCourse": "Eine Klausur muss zu einem Kurs gehören.",
208+
"wrongCourseId": "Die Kurs-ID stimmt nicht mit dem Kurs der Klausur überein.",
209+
"examTimes": "Ungültige Klausurzeiten: Bitte stelle sicher, dass das Sichtbarkeitsdatum vor dem Startdatum liegt, das Startdatum vor dem Enddatum liegt und das Veröffentlichungsdatum der Beispiellösung (falls gesetzt) nach dem Enddatum liegt.",
210+
"negativePoints": "Die maximale Punktzahl einer Klausur muss größer als 0 sein.",
211+
"correctionRoundViolation": "Ungültige Anzahl an Korrekturläufen: Testklausuren müssen 0 Korrekturläufe haben, echte Klausuren müssen 1 oder 2 Korrekturläufe haben.",
212+
"attendanceCheckViolation": "Testklausuren können keine Anwesenheitskontrolle haben.",
213+
"examWorkingTimeTooHigh": "Die Bearbeitungszeit ist zu lang. Das Maximum beträgt 43200 Minuten (30 Tage). Bitte passe den Start und/oder das Ende der Bearbeitungszeit an.",
214+
"examGracePeriodTooHigh": "Die Nachfrist ist zu lang. Bitte gib einen Wert zwischen 0 und 3600 Sekunden ein.",
215+
"examMaxPointsTooHigh": "Die maximale Punktzahl ist zu hoch. Bitte gib einen Wert zwischen 1 und 9999 ein.",
216+
"examNumberOfExercisesTooHigh": "Die Anzahl der Aufgaben ist zu hoch. Bitte gib einen Wert zwischen 1 und 100 ein.",
207217
"userManagement": {
208218
"onlySuperAdminCanCreateSuperAdmin": "Du bist kein Super-Administrator und kannst daher keinen Super-Administrator-Account erstellen.",
209219
"onlySuperAdminCanManageSuperAdmins": "Du bist kein Super-Administrator und kannst daher keine Super-Administratoren-Accounts verwalten."

0 commit comments

Comments
 (0)