Skip to content
Open
15 changes: 15 additions & 0 deletions src/main/java/de/tum/cit/aet/artemis/exam/web/ExamResource.java
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@ public ResponseEntity<ExamDTO> createExam(@PathVariable Long courseId, @RequestB
if (examDTO.id() != null) {
throw new BadRequestAlertException("A new exam cannot already have an ID", ENTITY_NAME, "idExists");
}
checkExamTitleIsPresentElseThrow(examDTO.title());

examAccessService.checkCourseAccessForInstructorElseThrow(courseId);

Expand Down Expand Up @@ -287,6 +288,7 @@ public ResponseEntity<ExamDTO> updateExam(@PathVariable Long courseId, @RequestB
if (examUpdateDTO.id() == null) {
throw new BadRequestAlertException("An exam update must have an ID", ENTITY_NAME, "idMissing");
}
checkExamTitleIsPresentElseThrow(examUpdateDTO.title());

examAccessService.checkCourseAndExamAccessForInstructorElseThrow(courseId, examUpdateDTO.id());

Expand Down Expand Up @@ -465,6 +467,7 @@ public ResponseEntity<ExamWideAnnouncementEventDTO> createExamAnnouncement(@Path
public ResponseEntity<ExamImportResultDTO> importExamWithExercises(@PathVariable Long courseId, @RequestBody ExamImportDTO examImportDTO,
@RequestParam(required = false) String importId) throws URISyntaxException, IOException {
log.debug("REST request to import an exam : {}", examImportDTO);
checkExamTitleIsPresentElseThrow(examImportDTO.title());

examAccessService.checkCourseAccessForInstructorElseThrow(courseId);

Expand Down Expand Up @@ -548,6 +551,18 @@ private void checkExamTitleLengthElseThrow(Exam exam) {
}
}

/**
* Checks that the exam title is present, so an exam is never created or updated with a missing or blank title. The client marks this too, but crafted requests and import
* payloads bypass the UI. This validates the raw request title before it is mapped to an entity, because {@link Exam#setTitle} would throw on a null title during mapping.
*
* @param title the exam title from the request
*/
private void checkExamTitleIsPresentElseThrow(String title) {
if (title == null || title.isBlank()) {
throw new BadRequestAlertException("The exam title must not be empty.", ENTITY_NAME, "examTitleEmpty");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

examTitleEmpty has no error.json entry in en or de.

checkExamTitleLengthElseThrow right above this (examTitleTooLong) has both, and so does every other exam validation error in this class (noCourse, wrongCourseId, examTimes, negativePoints, correctionRoundViolation, attendanceCheckViolation).

confirmed at runtime: fetch('/i18n/en.json') and de.json both lack error.examTitleEmpty while error.examTitleTooLong is present in both. means a crafted request or any non-UI caller gets the raw english exception title in the toast, untranslated for german users. two-line fix, same pattern as examTitleTooLong.

}
}

/**
* Validates numeric field limits for exam configuration.
* Maximum values:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ <h5 class="pb-1" jhiTranslate="artemisApp.examManagement.sections.configuration"
[hideChannelName]="hideChannelNameInput()"
[initChannelName]="isImport() || exam.id === undefined"
/>
@if (!exam.title?.trim()) {
Comment thread
Claudia-Anthropica marked this conversation as resolved.
<div class="text-state-danger text-sm mt-1 block" data-testid="title-validation-message" jhiTranslate="artemisApp.examManagement.titleMissingOrNotValid"></div>
}
</div>
<div class="form-group">
<div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,18 @@ describe('ExamUpdateComponent', () => {
expect(component.exam.workingTime).toBe(0);
});

it('should show the title validation message when the title is missing and hide it once a title is set', () => {
fixture.detectChanges();
const titleValidationSelector = By.css('[data-testid="title-validation-message"]');
expect(fixture.debugElement.query(titleValidationSelector)).not.toBeNull();

const examWithTitle = cloneDeep(component.exam);
examWithTitle.title = 'A valid exam title';
component.exam = examWithTitle;
fixture.detectChanges();
expect(fixture.debugElement.query(titleValidationSelector)).toBeNull();
});

it('should validate the dates correctly', () => {
examWithoutExercises.visibleDate = dayjs().add(1, 'hours');
examWithoutExercises.startDate = dayjs().add(2, 'hours');
Expand Down
1 change: 1 addition & 0 deletions src/main/webapp/i18n/de/exam.json
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,7 @@
"examManagement": {
"reviewDatesInvalidWarning": "Das Ende der Klausureinsicht ist falsch definiert oder fehlt",
"reviewDatesInvalidExplanation": "Ist ein Beginn der Klausureinsicht definiert, muss auch ein Ende definiert werden. Zusätzlich muss das Ende der Klausureinsicht immer nach dem Beginn datiert sein.",
"titleMissingOrNotValid": "'Titel' fehlt/ungültig",
"sections": {
"configuration": "Klausurkonfiguration",
"conduction": "Klausurdurchführung",
Expand Down
1 change: 1 addition & 0 deletions src/main/webapp/i18n/en/exam.json
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,7 @@
"examManagement": {
"reviewDatesInvalidWarning": "End of Student Review is set incorrect or missing",
"reviewDatesInvalidExplanation": "If the Begin of Student Review is set, the End of Student Review has to be set as well. Furthermore, the End of Student Review has to be chronologically after its Begin.",
"titleMissingOrNotValid": "'Title' is missing/invalid",
"sections": {
"configuration": "Exam Configuration",
"conduction": "Exam Conduction",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -678,6 +678,26 @@ void testCreateExam_failsWithExamMaxPointsTooHigh() throws Exception {
request.post("/api/exam/courses/" + course1.getId() + "/exams", ExamUpdateDTO.of(exam), HttpStatus.BAD_REQUEST);
}

@Test
@WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR")
void testCreateExam_failsWithBlankTitle() throws Exception {
Exam exam = ExamFactory.generateExam(course1, "examBlankTitleTest");
exam.setTitle(" "); // stripped to an empty title

request.post("/api/exam/courses/" + course1.getId() + "/exams", ExamUpdateDTO.of(exam), HttpStatus.BAD_REQUEST);
}

@Test
@WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR")
void testCreateExam_failsWithMissingTitle() throws Exception {
Exam exam = ExamFactory.generateExam(course1, "examMissingTitleTest");
// A crafted request without a title has to be rejected cleanly, not fail while mapping the null title to the entity
ObjectNode examJson = request.getObjectMapper().valueToTree(ExamUpdateDTO.of(exam));
examJson.putNull("title");

request.post("/api/exam/courses/" + course1.getId() + "/exams", examJson, HttpStatus.BAD_REQUEST);
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

@Test
@WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR")
void testCreateExam_failsWithGracePeriodTooHigh() throws Exception {
Expand Down
Loading