Exam mode: Show a validation message for a missing exam title - #13362
Exam mode: Show a validation message for a missing exam title#13362HannesHarbeck wants to merge 8 commits into
Exam mode: Show a validation message for a missing exam title#13362Conversation
…eject blank titles The exam create and edit form only disabled the save button when the title was empty, without the missing/invalid hint shown for the required date fields. Add a red validation message under the title field so an empty title is flagged like the dates. Mirror the requirement on the server: reject a null or blank title with a 400 on create, update and import before the request is mapped to an entity. This closes two gaps where the server persisted an empty-string title and threw a 500 while mapping a null title.
HannesHarbeck
changed the title
Exam mode: Show a validation message for a missing exam title and r…Exam mode: Show a validation message for a missing exam title
github-actions
Bot
added
tests
server
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughExam title validation now rejects null or whitespace-only values in exam creation, update, and import endpoints. The exam update form validates titles, displays localized feedback, and disables saving for invalid titles. Backend and frontend tests cover the behavior. ChangesExam title validation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/webapp/app/exam/manage/exams/update/exam-update.component.spec.ts (1)
154-154: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
deepClonefor the exam copy.Line [154] uses
cloneDeepfromlodash-es. UsedeepClonefromapp/foundation/util/deep-clone.utilfor entity-like objects and update the import.As per coding guidelines, copy entity-like objects with
deepClonefromapp/foundation/util/deep-clone.util.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/webapp/app/exam/manage/exams/update/exam-update.component.spec.ts` at line 154, Replace the cloneDeep usage in the examWithTitle setup with deepClone from app/foundation/util/deep-clone.util, and update the corresponding import while preserving the existing copied-exam behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/test/java/de/tum/cit/aet/artemis/exam/ExamIntegrationTest.java`:
- Around line 681-699: Strengthen the exam title validation tests around
testCreateExam_failsWithBlankTitle and testCreateExam_failsWithMissingTitle by
asserting the response contains the required examTitleEmpty error key, not only
BAD_REQUEST. Add equivalent blank and null title cases for exam update (PUT) and
import endpoints, using the project’s specific response assertions and
preserving the expected BAD_REQUEST status.
---
Nitpick comments:
In `@src/main/webapp/app/exam/manage/exams/update/exam-update.component.spec.ts`:
- Line 154: Replace the cloneDeep usage in the examWithTitle setup with
deepClone from app/foundation/util/deep-clone.util, and update the corresponding
import while preserving the existing copied-exam behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 92a3d761-2bb1-473b-af83-79fb0493712f
📒 Files selected for processing (6)
src/main/java/de/tum/cit/aet/artemis/exam/web/ExamResource.javasrc/main/webapp/app/exam/manage/exams/update/exam-update.component.htmlsrc/main/webapp/app/exam/manage/exams/update/exam-update.component.spec.tssrc/main/webapp/i18n/de/exam.jsonsrc/main/webapp/i18n/en/exam.jsonsrc/test/java/de/tum/cit/aet/artemis/exam/ExamIntegrationTest.java
github-project-automation
Bot
moved this from Work In Progress
to Ready For Review
in Artemis Development
GitHub Actions
Inactive
End-to-End Test Results
❌ Failed Tests (Phase 1)
Test Strategy: Two-phase execution
Overall: ❌ E2E: real (non-flaky) test failure |
… import in the exam title tests Address the review feedback on the exam title validation tests: assert the specific examTitleEmpty error key instead of only the 400 status, and cover the update and import endpoints in addition to create. Add a reusable RequestUtilService#postAndExpectError mirroring putAndExpectError, and replace the two create-only tests with three parameterised tests (create, update, import) that each check a null, empty and whitespace-only title.
github-actions
Bot
added
the
core
GitHub Actions
Inactive
Claudia-Anthropica
left a comment
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@HannesHarbeck The title validation is consistently applied to create, update, and import and has focused client and server coverage. However, the required All required CI Passed check is failing because E2E / Report E2E Overall Status failed; please investigate the E2E failure and restore the required check before approval.
GitHub Actions
Inactive
GitHub Actions
Inactive
Claudia-Anthropica
left a comment
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@HannesHarbeck Required CI now passes, and the server validation covers create, update, and import. One client-side gap remains: a whitespace-only title displays the invalid message but leaves Save enabled, resulting in a rejected server request instead of preventing submission.
A whitespace-only title showed the missing/invalid message but left the save button enabled, because Angular's required validator accepts a non-empty whitespace string and the save validity check did not consider the title. The request then reached the server and was rejected with a 400 instead of being prevented on the client. Include the trimmed-title check in isValidConfiguration, which the save button is already disabled by, and add a test asserting that a whitespace-only title disables saving.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/webapp/app/exam/manage/exams/update/exam-update.component.spec.ts (1)
154-156: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the repository
deepClonehelper for the copiedExamfixture.
component.examis an entity-like object. ReplacecloneDeepwithdeepClonefromapp/foundation/util/deep-clone.utiland update the import.Proposed change
- const examWithBlankTitle = cloneDeep(component.exam); + const examWithBlankTitle = deepClone(component.exam);As per coding guidelines, copy entity-like objects with
deepClonefromapp/foundation/util/deep-clone.util.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/webapp/app/exam/manage/exams/update/exam-update.component.spec.ts` around lines 154 - 156, Replace the cloneDeep call used to copy component.exam in the examWithBlankTitle fixture with the repository deepClone helper, and update the import to use app/foundation/util/deep-clone.util. Preserve the existing fixture mutation and test behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/main/webapp/app/exam/manage/exams/update/exam-update.component.spec.ts`:
- Around line 154-156: Replace the cloneDeep call used to copy component.exam in
the examWithBlankTitle fixture with the repository deepClone helper, and update
the import to use app/foundation/util/deep-clone.util. Preserve the existing
fixture mutation and test behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a986876d-bb0e-4783-a478-2848b6d19f50
📒 Files selected for processing (2)
src/main/webapp/app/exam/manage/exams/update/exam-update.component.spec.tssrc/main/webapp/app/exam/manage/exams/update/exam-update.component.ts
GitHub Actions
Inactive
|
Tested locally and works as described. However, I think it would look better if the error message was closer to the input field(like with the dates) |
…title field Reduce the gap between the title field and its "missing/invalid" message by replacing the top margin with a small negative one, so the message sits directly under the field like the date field messages. Layout only, no behaviour change.
2b13db6
Claudia-Anthropica
left a comment
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@HannesHarbeck The server consistently rejects null, empty, and whitespace-only exam titles across create, update, and import, with focused contract tests. The client applies the same trimmed-title rule to its message and Save-button state, and the latest layout adjustment uses an available spacing utility. The snapshot lists no required checks and no completed failing checks.
GitHub Actions
Inactive
|
@SultanTheHL I adjusted the layout accordingly |
SedaOran
left a comment
SedaOran
left a comment
There was a problem hiding this comment.
Now UI works well and It's aligned. Approving.
WoH
left a comment
WoH
left a comment
There was a problem hiding this comment.
tested commit 2b13db6. setup: local dev, e2e seed, instructor artemis_test_user_16, course 9023 E2E Exam Management Course.
- create exam, leave title empty, red 'Title' is missing/invalid message under the field like the date fields: yes
- type a title, message disappears, save enabled once dates valid too: yes, per-keystroke, no fill()
- delete the title again, message reappears, save disabled: yes, tested on both create and the edit form (step not in your list but same code path)
pr13362-v01-create-exam-title-validation.webm
pr13362-v02-edit-exam-clear-title.webm
also checked:
- whitespace-only title (3 spaces, real keystrokes + blur): native
requiredreports the field valid (green border) but your trim check still shows the message and blocks save. isolated it by filling all three dates first so title was the only failing field. this is the actual bug the PR fixes, good catch.
-
full create round trip: POST /api/exam/courses/9023/exams -> 201, title matches in request/response/UI, no console errors, no toast
-
crafted PUT with title:null on the saved exam -> 400, error.examTitleEmpty, exam title unchanged after (confirmed by reload). see inline comment, this error key has no en/de translation, unlike its neighbors
| */ | ||
| private void checkExamTitleIsPresentElseThrow(String title) { | ||
| if (title == null || title.isBlank()) { | ||
| throw new BadRequestAlertException("The exam title must not be empty.", ENTITY_NAME, "examTitleEmpty"); |
There was a problem hiding this comment.
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.
|
There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions. |
develop replaced the manual exam conduction date chain with the timeline status signal (#13473). Keep that check and combine it with the new title validation instead of the removed date getters. develop also replaced lodash cloneDeep with deepClone (#13351), so the two new spec call sites use deepClone as well.
7d00130
GitHub Actions
Inactive
Claudia-Anthropica
left a comment
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@HannesHarbeck The exact head consistently rejects null/blank titles for create, update, and import, while the client blocks whitespace-only titles with focused coverage. The failed E2E test concerns post-reload quiz-answer re-sending; its only call through the changed create endpoint uses a valid title, so this change does not reach the failed behavior. Test / Server Tests (PostgreSQL) and therefore All required CI Passed remain red, but the captured server evidence names no failing test or compiler error, so that failure cannot be attributed. The existing unresolved thread identifies one [low] localization gap: error.examTitleEmpty is absent from the English and German error.json, leaving the raw English fallback for non-UI invalid requests.
ShudongCai
left a comment
ShudongCai
left a comment
There was a problem hiding this comment.
Tested locally, works as expected
GitHub Actions
Inactive
SedaOran
left a comment
SedaOran
left a comment
There was a problem hiding this comment.
Tested locally, works as expected. Approved.
7 participants
Summary
The exam create/edit form now flags an empty title with a red "missing/invalid" message like the date fields, and the server rejects a null or blank exam title with a 400 on create, update and import. This closes two server gaps: an empty-string title was persisted, and a crafted null title threw a 500 while being mapped to the entity.
Checklist
General
Server
Client
Motivation and Context
The exam create and edit form only disabled the save button when the title was empty, without the missing/invalid hint shown for the required date fields. A user emptying the title got no visible feedback about why saving was blocked.
On top of that, the server did not enforce the requirement it relies on: the
titlecolumn isnullable = false, which only catchesnull, so an empty-string title was persisted, and a crafted request with anulltitle threw a 500 (NPE) while mapping the DTO to the entity.Description
text-state-dangerstyling.ExamResourcerejects anullor blank title with400 examTitleEmptyon create, update and import, validated on the request before it is mapped to an entity (so thenullcase can no longer NPE inExam#setTitle).Steps for Testing
Prerequisites:
Note: this PR only changes the instructor-facing exam configuration form and the server-side validation. The student exam-taking UI (exam mode) is not affected.
Testserver States
You can manage test servers using Helios. Check environment statuses in the environment list. To deploy to a test server, go to the CI/CD page, find your PR or branch, and trigger the deployment.
Review Progress
Code Review
Manual Tests
Test Coverage
Client
Server
Last updated: 2026-08-24 13:20:53 UTC
Screenshots
To be added: the exam create form with an empty title showing the red "'Title' is missing/invalid" message (light and dark theme).
Summary by CodeRabbit
Summary by CodeRabbit
Bug Fixes
Tests