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 @@ -69,6 +69,7 @@ describe('ExamUpdateComponent', () => {
beforeEach(() => {
examWithoutExercises = new Exam();
examWithoutExercises.id = 1;
examWithoutExercises.title = 'Test Exam';
course = new Course();
course.id = 1;
course.courseInformationSharingConfiguration = CourseInformationSharingConfiguration.COMMUNICATION_AND_MESSAGING;
Expand Down Expand Up @@ -146,6 +147,23 @@ describe('ExamUpdateComponent', () => {
expect(component.exam.workingTime).toBe(0);
});

it('should show the title validation message for a missing or whitespace-only title and hide it once a title is set', () => {
fixture.detectChanges();
const titleValidationSelector = By.css('[data-testid="title-validation-message"]');

const examWithBlankTitle = cloneDeep(component.exam);
examWithBlankTitle.title = ' ';
component.exam = examWithBlankTitle;
fixture.detectChanges();
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 Expand Up @@ -243,6 +261,7 @@ describe('ExamUpdateComponent', () => {
it('should validate the example solution publication date correctly', () => {
const newExamWithoutExercises = new Exam();
newExamWithoutExercises.id = 2;
newExamWithoutExercises.title = 'Test Exam';
component.exam = newExamWithoutExercises;

const now = dayjs();
Expand All @@ -263,6 +282,7 @@ describe('ExamUpdateComponent', () => {
it('should validate the exam summary publication date correctly', () => {
const newExam = new Exam();
newExam.id = 3;
newExam.title = 'Test Exam';
component.exam = newExam;

const now = dayjs();
Expand Down Expand Up @@ -802,6 +822,25 @@ describe('ExamUpdateComponent', () => {
expect(button.disabled()).toBe(true);
});

it('should disable the save button when the title is only whitespace', async () => {
examWithoutExercises.visibleDate = dayjs().add(1, 'hours');
examWithoutExercises.startDate = dayjs().add(2, 'hours');
examWithoutExercises.endDate = dayjs().add(3, 'hours');
examWithoutExercises.workingTime = 3600;
examWithoutExercises.title = ' ';

fixture.changeDetectorRef.detectChanges();
// Force the reactive form itself to report valid, so only the whitespace-only title can disable the save button
const ngForm = fixture.debugElement.query(By.directive(NgForm)).injector.get(NgForm);
vi.spyOn(ngForm.form, 'invalid', 'get').mockReturnValue(false);
fixture.changeDetectorRef.markForCheck();
await fixture.whenStable();

expect(component.isValidConfiguration).toBe(false);
const button = fixture.debugElement.query(By.directive(ButtonComponent)).componentInstance;
expect(button.disabled()).toBe(true);
});

it('should open the confirmation dialog when dates changed for an ongoing exam', async () => {
// Set up an ongoing exam
examWithoutExercises.id = 1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,16 @@ export class ExamUpdateComponent implements OnInit, OnDestroy {
return !!(this.exam.id && this.originalStartDate && this.originalEndDate && dayjs().isBetween(this.originalStartDate, this.originalEndDate));
}

/**
* Returns whether the exam title is present after trimming. The title input is required, but Angular's required validator
* accepts a whitespace-only value, so the trimmed check is needed to keep the save button in sync with the title validation message.
*/
get isValidTitle(): boolean {
return !!this.exam.title?.trim();
}

get isValidConfiguration(): boolean {
const examTitleValid = this.isValidTitle;
const examConductionDatesValid =
this.isVisibleDateSet && this.isStartDateSet && this.isValidStartDate && this.isEndDateSet && this.isValidEndDate && this.isValidVisibleDateValue;
const examReviewDatesValid = this.isValidPublishResultsDate && this.isValidExamStudentReviewStart && this.isValidExamStudentReviewEnd;
Expand All @@ -406,6 +415,7 @@ export class ExamUpdateComponent implements OnInit, OnDestroy {
const examValidNumberOfExercises = this.isValidNumberOfExercises;
const examValidGracePeriod = this.isValidGracePeriod;
return (
examTitleValid &&
examConductionDatesValid &&
examReviewDatesValid &&
examNumberOfCorrectionsValid &&
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 @@ -651,6 +651,17 @@ public void putAndExpectError(String path, Object body, HttpStatus expectedStatu
assertThat(response.getHeader(errorHeader)).isEqualTo(fullErrorKey);
}

public void postAndExpectError(String path, Object body, HttpStatus expectedStatus, String expectedErrorKey) throws Exception {
final var jsonBody = mapper.writeValueAsString(body);
final var response = performMvcRequest(MockMvcRequestBuilders.post(new URI(path)).contentType(MediaType.APPLICATION_JSON).content(jsonBody))
.andExpect(status().is(expectedStatus.value())).andReturn().getResponse();
restoreSecurityContext();

final var fullErrorKey = "error." + expectedErrorKey;
final var errorHeader = "X-" + APPLICATION_NAME + "-error";
assertThat(response.getHeader(errorHeader)).isEqualTo(fullErrorKey);
}

public <T> T get(String path, HttpStatus expectedStatus, Class<T> responseType) throws Exception {
return get(path, expectedStatus, responseType, new LinkedMultiValueMap<>());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import org.junit.jupiter.params.provider.NullSource;
import org.junit.jupiter.params.provider.ValueSource;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.HttpStatus;
Expand Down Expand Up @@ -678,6 +679,55 @@ void testCreateExam_failsWithExamMaxPointsTooHigh() throws Exception {
request.post("/api/exam/courses/" + course1.getId() + "/exams", ExamUpdateDTO.of(exam), HttpStatus.BAD_REQUEST);
}

@ParameterizedTest(name = "title=\"{0}\"")
@NullSource
@ValueSource(strings = { "", " " })
@WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR")
void testCreateExam_failsWithMissingOrBlankTitle(String title) throws Exception {
// A missing (null), empty or whitespace-only title has to be rejected with a clean 400, not persisted and not failing while mapping the null title to the entity
ObjectNode examJson = examBodyWithTitle(ExamUpdateDTO.of(ExamFactory.generateExam(course1, "examTitleValidationTest")), title);

request.postAndExpectError("/api/exam/courses/" + course1.getId() + "/exams", examJson, HttpStatus.BAD_REQUEST, "examTitleEmpty");
}

@ParameterizedTest(name = "title=\"{0}\"")
@NullSource
@ValueSource(strings = { "", " " })
@WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR")
void testUpdateExam_failsWithMissingOrBlankTitle(String title) throws Exception {
ObjectNode examJson = examBodyWithTitle(ExamUpdateDTO.of(exam1), title);

request.putAndExpectError("/api/exam/courses/" + course1.getId() + "/exams", examJson, HttpStatus.BAD_REQUEST, "examTitleEmpty");
}

@ParameterizedTest(name = "title=\"{0}\"")
@NullSource
@ValueSource(strings = { "", " " })
@WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR")
void testImportExam_failsWithMissingOrBlankTitle(String title) throws Exception {
ObjectNode examJson = examBodyWithTitle(ExamImportDTO.of(exam1, course1.getId()), title);

request.postAndExpectError("/api/exam/courses/" + course1.getId() + "/exam-import", examJson, HttpStatus.BAD_REQUEST, "examTitleEmpty");
}

/**
* Serialises the given exam DTO and overwrites its title, so a missing (null), empty or whitespace-only title can be sent as a raw request body.
*
* @param examDto the exam create/update/import DTO to serialise
* @param title the title to set, or null to omit it
* @return the request body as a JSON object with the adjusted title
*/
private ObjectNode examBodyWithTitle(Object examDto, String title) {
ObjectNode examJson = request.getObjectMapper().valueToTree(examDto);
if (title == null) {
examJson.putNull("title");
}
else {
examJson.put("title", title);
}
return examJson;
}

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