Quiz exercises: Validate short answer question title length#12533
Quiz exercises: Validate short answer question title length#12533MaximilianAnzinger wants to merge 2 commits intodevelopfrom
Quiz exercises: Validate short answer question title length#12533Conversation
Quiz exercises: Validate short answer question title length
|
@MaximilianAnzinger Test coverage has been automatically updated in the PR description. |
WalkthroughInput length validation was added to short answer question title fields in a quiz editing component, implementing a maximum character limit constraint ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/webapp/app/quiz/manage/short-answer-question/short-answer-question-edit.component.spec.ts (1)
654-659: Add a companion maxlength assertion for the re-evaluation title input.This test currently covers only the default edit branch; the re-evaluation branch has a separate input and should be asserted too.
Suggested test addition
+ it('should enforce the title maxlength on the re-evaluation short answer question input', () => { + fixture.componentRef.setInput('reEvaluationInProgress', true); + fixture.detectChanges(); + + const titleInput = fixture.nativeElement.querySelector<HTMLInputElement>( + 'input[name="short-answer-question-title-reevaluate-0"]', + ); + + expect(titleInput).not.toBeNull(); + expect(titleInput?.getAttribute('maxlength')).toBe(String(MAX_QUIZ_QUESTION_LENGTH_THRESHOLD - 1)); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/webapp/app/quiz/manage/short-answer-question/short-answer-question-edit.component.spec.ts` around lines 654 - 659, Add a companion assertion that verifies the maxlength on the re-evaluation title input: query the DOM for the re-evaluation input (e.g., the element with id '#short-answer-question-re-evaluation-title' or the component's re-evaluation title selector used in the template), assert it is not null, and check its maxlength equals String(MAX_QUIZ_QUESTION_LENGTH_THRESHOLD - 1) just like the existing check for '#short-answer-question-title' so both the default edit and re-evaluation branches are covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@src/main/webapp/app/quiz/manage/short-answer-question/short-answer-question-edit.component.spec.ts`:
- Around line 654-659: Add a companion assertion that verifies the maxlength on
the re-evaluation title input: query the DOM for the re-evaluation input (e.g.,
the element with id '#short-answer-question-re-evaluation-title' or the
component's re-evaluation title selector used in the template), assert it is not
null, and check its maxlength equals String(MAX_QUIZ_QUESTION_LENGTH_THRESHOLD -
1) just like the existing check for '#short-answer-question-title' so both the
default edit and re-evaluation branches are covered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 90ccfc63-e232-4962-96b9-03525d268c62
📒 Files selected for processing (3)
src/main/webapp/app/quiz/manage/short-answer-question/short-answer-question-edit.component.htmlsrc/main/webapp/app/quiz/manage/short-answer-question/short-answer-question-edit.component.spec.tssrc/main/webapp/app/quiz/manage/short-answer-question/short-answer-question-edit.component.ts
|
@MaximilianAnzinger Your PR description needs attention before it can be reviewed: Issues Found
How to Fix
This check validates that your PR description follows the PR template. A complete description helps reviewers understand your changes and speeds up the review process.
|
|
@MaximilianAnzinger Test coverage has been automatically updated in the PR description. |
End-to-End Test Results
Test Strategy: Two-phase execution
Overall: ❌ Phase 2 (remaining tests) failed 🔗 Workflow Run · 📊 Test Report |
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@MaximilianAnzinger Clean and focused change. The maxlength of 249 correctly matches the existing >= 250 threshold, both inputs (edit and re-evaluation) are covered, and the translation key wires up properly. Nice work.
SedaOran
left a comment
There was a problem hiding this comment.
Looks good overall.
One small suggestion: since you're using MAX_QUIZ_QUESTION_LENGTH_THRESHOLD - 1 to derive MAX_QUESTION_TITLE_LENGTH, it might be helpful to add a short comment explaining the -1 (to align with the < threshold validation).
I was also wondering if it might be worth considering server-side validation for this as well (and possibly tests for it), just to ensure consistency and avoid relying only on client-side checks.
Otherwise, really clean and well-structured change. Thanks.
Summary
Add inline title length validation to the short answer question editor so excessively long titles are blocked directly in the quiz creation flow instead of only surfacing later through overall quiz validation.
Checklist
General
Client
Motivation and Context
Fixes #12452.
The short answer question title input currently accepts arbitrarily long text even though the quiz editor already treats long question titles as invalid. This means instructors can enter very large titles without immediate feedback in the short answer creation flow.
Description
Steps for Testing
Prerequisites:
Exam Mode Testing
This change only affects the quiz exercise editing UI for instructors and does not affect exam mode participation.
Review Progress
Performance Review
Code Review
Manual Tests
Test Coverage
Client
Last updated: 2026-04-14 15:12:55 UTC