Exam mode: Add save exercise button to exercises
#9569
Conversation
github-actions
bot
added
the
client
edkaya
changed the title
Exam Mode: Add save exercise buttonExam mode: Add save exercise button
edkaya
changed the title
Exam mode: Add save exercise buttonExam mode: Add save exercise button to exercises
edkaya
temporarily deployed
to
artemis-test5.artemis.cit.tum.de
GitHub Actions
Inactive
Pull latest changes
WalkthroughThe changes in this pull request primarily enhance the exam participation interface by introducing the Changes
Suggested labels
Suggested reviewers
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Outside diff range and nitpick comments (15)
src/main/webapp/app/exam/participate/exercises/exercise-save-button/exercise-save-button.component.html (1)
1-1: Enhance button accessibility and user feedback.Consider adding
aria-labelandtitleattributes to improve accessibility and provide tooltip feedback.-<button id="save-exam" type="submit" [disabled]="submission()?.isSynced" class="btn btn-primary" (click)="onSave()"> +<button + id="save-exam" + type="submit" + [disabled]="submission()?.isSynced" + class="btn btn-primary" + (click)="onSave()" + [attr.aria-label]="submission()?.submitted && submission()?.isSynced ? 'Exercise saved' : 'Save exercise'" + [title]="submission()?.submitted && submission()?.isSynced ? 'Exercise is saved' : 'Click to save exercise'">src/main/webapp/app/exam/participate/exercises/exercise-save-button/exercise-save-button.component.ts (1)
15-25: Add type safety and documentation to the component.While the implementation is functional, it could benefit from improved type safety and documentation.
Consider these improvements:
export class ExerciseSaveButtonComponent { protected readonly faFloppyDisk = faFloppyDisk; protected readonly facSaveSuccess = facSaveSuccess; - submission = input<Submission>(); - save = output<void>(); + submission = input.required<Submission>(); // Make submission required + save = output<void>({ alias: 'exerciseSave' }); // Add alias for clearer API + /** + * Emits a save event when the user clicks the save button. + * This triggers the parent component to handle the actual save operation. + */ onSave() { + if (!this.submission()) { + console.warn('Attempting to save without a submission'); + return; + } this.save.emit(); } }src/main/webapp/app/exam/participate/exercises/modeling/modeling-exam-submission.component.html (1)
8-14: Consider extracting nested template logic for better readability.The nested conditional for bonus points and score badge makes the template harder to read. Consider extracting these into separate template variables or methods.
- <!-- prettier-ignore --> - <span left-header> ({{ exercise.maxPoints }} {{ 'artemisApp.examParticipation.points' | artemisTranslate }}@if (exercise.bonusPoints) { - <span - >, {{ exercise.bonusPoints }} {{ 'artemisApp.examParticipation.bonus' | artemisTranslate }}</span> - }) @if (exercise.includedInOverallScore !== IncludedInOverallScore.INCLUDED_COMPLETELY) { - <jhi-included-in-score-badge - [includedInOverallScore]="exercise.includedInOverallScore" /> - }</span> + <span left-header> + ({{ exercise.maxPoints }} {{ 'artemisApp.examParticipation.points' | artemisTranslate }} + @if (exercise.bonusPoints) { + <span>, {{ exercise.bonusPoints }} {{ 'artemisApp.examParticipation.bonus' | artemisTranslate }}</span> + }) + @if (exercise.includedInOverallScore !== IncludedInOverallScore.INCLUDED_COMPLETELY) { + <jhi-included-in-score-badge [includedInOverallScore]="exercise.includedInOverallScore" /> + } + </span>src/main/webapp/app/exam/participate/exercises/text/text-exam-submission.component.html (2)
8-14: Consider improving template readability.The nested conditional rendering for bonus points and score inclusion could be more readable.
Consider restructuring like this:
- <!-- prettier-ignore --> - <span left-header> ({{ exercise.maxPoints }} {{ 'artemisApp.examParticipation.points' | artemisTranslate }}@if (exercise.bonusPoints) { - <span - >, {{ exercise.bonusPoints }} {{ 'artemisApp.examParticipation.bonus' | artemisTranslate }}</span> - }) @if (exercise.includedInOverallScore !== IncludedInOverallScore.INCLUDED_COMPLETELY) { - <jhi-included-in-score-badge - [includedInOverallScore]="exercise.includedInOverallScore" /> - }</span> + <span left-header> + ({{ exercise.maxPoints }} {{ 'artemisApp.examParticipation.points' | artemisTranslate }} + @if (exercise.bonusPoints) { + <span>, {{ exercise.bonusPoints }} {{ 'artemisApp.examParticipation.bonus' | artemisTranslate }}</span> + }) + @if (exercise.includedInOverallScore !== IncludedInOverallScore.INCLUDED_COMPLETELY) { + <jhi-included-in-score-badge [includedInOverallScore]="exercise.includedInOverallScore" /> + } + </span>
Line range hint
28-36: Add accessibility attributes to the text editor.The textarea should have an aria-label for better accessibility.
<textarea #textEditor id="text-editor" class="text-editor-textarea" + aria-label="Exercise answer text editor" [maxLength]="maxCharacterCount" [(ngModel)]="answer" [readOnly]="readonly || !studentSubmission" [disabled]="readonly || !studentSubmission"src/main/webapp/app/exam/participate/exercises/text/text-exam-submission.component.ts (2)
29-29: Consider adding void type parameter for better type safety.While the output works, explicitly declaring the type parameter improves code clarity and type safety.
- saveCurrentExercise = output<void>(); + saveCurrentExercise = output<void, void>();
127-132: Consider using arrow function for consistency.While the implementation is correct, using an arrow function would be more consistent with modern TypeScript practices.
- /** - * Trigger save action in exam participation component - */ - notifyTriggerSave() { - this.saveCurrentExercise.emit(); - } + /** + * Trigger save action in exam participation component + */ + notifyTriggerSave = (): void => { + this.saveCurrentExercise.emit(); + };src/main/webapp/app/exam/participate/exercises/quiz/quiz-exam-submission.component.html (2)
1-14: LGTM! Clean layout structure with good component organization.The flexbox layout provides good alignment between the title and save button, matching the PR objectives for improving the exam mode interface.
Consider adding an aria-label to the save button section for better accessibility:
- <jhi-exercise-save-button [submission]="studentSubmission" (save)="notifyTriggerSave()" /> + <jhi-exercise-save-button + [submission]="studentSubmission" + (save)="notifyTriggerSave()" + aria-label="Save exercise progress" + />
Line range hint
82-122: Consider refactoring repeated question rendering patterns.The question rendering logic follows similar patterns for each type (MC, DD, SA). Consider extracting this into a shared component to improve maintainability.
Example approach:
// question-renderer.component.ts interface QuestionConfig { type: string; component: Type<any>; idPrefix: string; } const QUESTION_COMPONENTS: Record<string, QuestionConfig> = { MULTIPLE_CHOICE: { type: 'MULTIPLE_CHOICE', component: MultipleChoiceQuestionComponent, idPrefix: 'MC' }, // ... similar for other types };<!-- question-renderer.component.html --> <ng-container [ngComponentOutlet]="questionConfig.component" [ngComponentOutletInputs]="{ question: question, questionIndex: index, // ... other common props }"> </ng-container>src/main/webapp/app/exam/participate/exercises/modeling/modeling-exam-submission.component.ts (2)
37-41: Consider consistent decorator usage across the component.The component mixes the new input()/output() syntax with older @Input/@output decorators. Consider updating all properties to use the new syntax for consistency.
Apply this pattern to other properties:
studentSubmission = input<ModelingSubmission>(); exercise = input<ModelingExercise>();
37-39: Enhance documentation for isSubmissionSynced.While the technical reason for the property is documented, consider adding its business purpose for better maintainability.
/** * Tracks the synchronization state of the current submission with the server. * Explicitly needed due to OnPush change detection strategy. * Used by the save button to indicate whether changes are pending. */ isSubmissionSynced = input<boolean>();src/test/javascript/spec/component/exam/participate/exercises/modeling-exam-submission.component.spec.ts (3)
Line range hint
20-53: Add tests for save button functionality.The test suite is missing coverage for the new save button functionality described in the PR objectives:
- Button activation when student starts working
- Button state changes after saving
- Auto-save functionality after 30 seconds of inactivity
Consider adding these test cases:
describe('Save button functionality', () => { beforeEach(() => { resetComponent(); fixture.detectChanges(); }); it('should enable save button when model changes', () => { const saveButton = fixture.debugElement.query(By.directive(ExerciseSaveButtonComponent)); comp.modelChanged({} as UMLModel); fixture.detectChanges(); expect(saveButton.componentInstance.isDisabled).toBeFalse(); }); it('should disable save button after saving', () => { const saveButton = fixture.debugElement.query(By.directive(ExerciseSaveButtonComponent)); comp.onSave(); fixture.detectChanges(); expect(saveButton.componentInstance.isDisabled).toBeTrue(); expect(saveButton.componentInstance.label).toBe('Exercise Saved'); }); it('should auto-save after 30 seconds of inactivity', fakeAsync(() => { const saveButton = fixture.debugElement.query(By.directive(ExerciseSaveButtonComponent)); comp.modelChanged({} as UMLModel); fixture.detectChanges(); tick(30000); expect(saveButton.componentInstance.isDisabled).toBeTrue(); expect(saveButton.componentInstance.label).toBe('Exercise Saved'); })); });
Line range hint
171-196: Add save button state update to modelChanged tests.The
modelChangedtest should verify that the save button becomes enabled when the model changes.Update the test:
describe('modelChanged', () => { beforeEach(() => { resetComponent(); fixture.detectChanges(); }); it('should set isSynced to false and enable save button', () => { const saveButton = fixture.debugElement.query(By.directive(ExerciseSaveButtonComponent)); comp.studentSubmission.isSynced = true; comp.modelChanged({} as UMLModel); fixture.detectChanges(); expect(comp.studentSubmission.isSynced).toBeFalse(); expect(saveButton.componentInstance.isDisabled).toBeFalse(); }); });
Line range hint
197-209: Add save button state update to explanationChanged tests.The
explanationChangedtest should verify that the save button becomes enabled when the explanation changes.Update the test:
describe('explanationChanged', () => { beforeEach(() => { resetComponent(); fixture.detectChanges(); }); it('should set explanation text, update isSynced, and enable save button', () => { const explanationText = 'New Explanation Text'; const saveButton = fixture.debugElement.query(By.directive(ExerciseSaveButtonComponent)); comp.studentSubmission.isSynced = true; comp.explanationChanged(explanationText); fixture.detectChanges(); expect(comp.studentSubmission.isSynced).toBeFalse(); expect(comp.explanationText).toEqual(explanationText); expect(saveButton.componentInstance.isDisabled).toBeFalse(); }); });src/test/javascript/spec/component/exam/participate/exercises/quiz-exam-submission.component.spec.ts (1)
Line range hint
28-196: Add test coverage for the new save button functionality.The test suite needs additional test cases to cover the new save button functionality requirements from the PR:
- Button activation when student starts working
- Button state changes after saving
- Auto-save behavior after 30 seconds of inactivity
Consider adding these test cases:
it('should enable save button when student makes changes', () => { // Setup component.quizConfiguration = { quizQuestions: [multipleChoiceQuestion] }; fixture.detectChanges(); // Act component.onSelectionChanged(); fixture.detectChanges(); // Assert const saveButton = fixture.debugElement.query(By.directive(ExerciseSaveButtonComponent)); expect(saveButton.componentInstance.disabled).toBeFalse(); }); it('should disable save button with "Exercise Saved" label after saving', () => { // Setup component.quizConfiguration = { quizQuestions: [multipleChoiceQuestion] }; fixture.detectChanges(); // Act component.onSelectionChanged(); fixture.detectChanges(); component.saveExercise(); fixture.detectChanges(); // Assert const saveButton = fixture.debugElement.query(By.directive(ExerciseSaveButtonComponent)); expect(saveButton.componentInstance.disabled).toBeTrue(); expect(saveButton.componentInstance.label).toBe('Exercise Saved'); }); it('should auto-save and update button state after 30 seconds of inactivity', fakeAsync(() => { // Setup component.quizConfiguration = { quizQuestions: [multipleChoiceQuestion] }; fixture.detectChanges(); // Act component.onSelectionChanged(); fixture.detectChanges(); tick(30000); // Assert const saveButton = fixture.debugElement.query(By.directive(ExerciseSaveButtonComponent)); expect(saveButton.componentInstance.disabled).toBeTrue(); expect(saveButton.componentInstance.label).toBe('Exercise Saved'); }));
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (17)
- src/main/webapp/app/exam/participate/exam-participation.component.html (2 hunks)
- src/main/webapp/app/exam/participate/exercises/exam-submission-components.module.ts (2 hunks)
- src/main/webapp/app/exam/participate/exercises/exercise-save-button/exercise-save-button.component.html (1 hunks)
- src/main/webapp/app/exam/participate/exercises/exercise-save-button/exercise-save-button.component.scss (1 hunks)
- src/main/webapp/app/exam/participate/exercises/exercise-save-button/exercise-save-button.component.ts (1 hunks)
- src/main/webapp/app/exam/participate/exercises/modeling/modeling-exam-submission.component.html (1 hunks)
- src/main/webapp/app/exam/participate/exercises/modeling/modeling-exam-submission.component.ts (3 hunks)
- src/main/webapp/app/exam/participate/exercises/quiz/quiz-exam-submission.component.html (1 hunks)
- src/main/webapp/app/exam/participate/exercises/quiz/quiz-exam-submission.component.ts (3 hunks)
- src/main/webapp/app/exam/participate/exercises/text/text-exam-submission.component.html (1 hunks)
- src/main/webapp/app/exam/participate/exercises/text/text-exam-submission.component.ts (4 hunks)
- src/main/webapp/i18n/de/exam.json (1 hunks)
- src/main/webapp/i18n/en/exam.json (1 hunks)
- src/test/javascript/spec/component/exam/participate/exercises/modeling-exam-submission.component.spec.ts (2 hunks)
- src/test/javascript/spec/component/exam/participate/exercises/quiz-exam-submission.component.spec.ts (2 hunks)
- src/test/javascript/spec/component/exam/participate/exercises/text-exam-submission.component.spec.ts (2 hunks)
- src/test/playwright/e2e/exam/ExamDateVerification.spec.ts (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/main/webapp/app/exam/participate/exercises/exercise-save-button/exercise-save-button.component.scss
🧰 Additional context used
📓 Path-based instructions (14)
src/main/webapp/app/exam/participate/exam-participation.component.html (1)
Pattern
src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.src/main/webapp/app/exam/participate/exercises/exam-submission-components.module.ts (1)
src/main/webapp/app/exam/participate/exercises/exercise-save-button/exercise-save-button.component.html (1)
Pattern
src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.src/main/webapp/app/exam/participate/exercises/exercise-save-button/exercise-save-button.component.ts (1)
src/main/webapp/app/exam/participate/exercises/modeling/modeling-exam-submission.component.html (1)
Pattern
src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.src/main/webapp/app/exam/participate/exercises/modeling/modeling-exam-submission.component.ts (1)
src/main/webapp/app/exam/participate/exercises/quiz/quiz-exam-submission.component.html (1)
Pattern
src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.src/main/webapp/app/exam/participate/exercises/quiz/quiz-exam-submission.component.ts (1)
src/main/webapp/app/exam/participate/exercises/text/text-exam-submission.component.html (1)
Pattern
src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.src/main/webapp/app/exam/participate/exercises/text/text-exam-submission.component.ts (1)
src/main/webapp/i18n/de/exam.json (1)
Pattern
src/main/webapp/i18n/de/**/*.json: German language translations should be informal (dutzen) and should never be formal (sietzen). So the user should always be addressed with "du/dein" and never with "sie/ihr".src/test/javascript/spec/component/exam/participate/exercises/modeling-exam-submission.component.spec.ts (1)
Pattern
src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}src/test/javascript/spec/component/exam/participate/exercises/quiz-exam-submission.component.spec.ts (1)
Pattern
src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}src/test/javascript/spec/component/exam/participate/exercises/text-exam-submission.component.spec.ts (1)
Pattern
src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
📓 Learnings (1)
src/test/playwright/e2e/exam/ExamDateVerification.spec.ts (2)
Learnt from: SimonEntholzer PR: ls1intum/Artemis#8946 File: src/test/playwright/e2e/exam/ExamDateVerification.spec.ts:109-110 Timestamp: 2024-10-08T15:35:48.768Z Learning: For the pull request `General: Fix and re-enable exam mode e2e tests` (PR #8946), avoid providing "safer alternative" tips as requested by SimonEntholzer.Learnt from: SimonEntholzer PR: ls1intum/Artemis#8946 File: src/test/playwright/e2e/exam/ExamDateVerification.spec.ts:109-110 Timestamp: 2024-07-02T20:47:05.487Z Learning: For the pull request `General: Fix and re-enable exam mode e2e tests` (PR #8946), avoid providing "safer alternative" tips as requested by SimonEntholzer.
🔇 Additional comments (28)
src/main/webapp/app/exam/participate/exercises/exercise-save-button/exercise-save-button.component.html (3)
2-2: LGTM! Icon implementation provides clear visual feedback.The icon changes appropriately based on the submission state, providing good visual feedback to users.
3-7: LGTM! Responsive design handles mobile views appropriately.The text is properly hidden on extra small screens while maintaining the icon visible, which is a good responsive design practice.
1-8: Verify the submission state handling.The button's behavior relies heavily on the submission state. Let's verify the implementation of the submission state management.
✅ Verification successful
Let me gather more information about the component implementation to verify the submission state handling properly.
Let me gather more information about the submission model and its implementation to verify the submission state handling properly.
Based on the code analysis, I can now provide a final response about the submission state handling verification.
The submission state handling is correctly implemented.
The implementation shows a well-structured pattern for handling submission states:
- The
ExerciseSaveButtonComponentreceives a submission input signal and properly uses it for button state management- The submission model (
submission.model.ts) defines theisSyncedproperty used for tracking save state- Each exercise type component (text, quiz, modeling, etc.) implements the required submission state handling through:
hasUnsavedChanges(): Returns!submission.isSyncedupdateSubmissionFromView(): Updates submission content from UIupdateViewFromSubmission(): Updates UI from submission dataThe button's behavior correctly reflects the submission state by:
- Disabling when
submission()?.isSyncedis true- Showing appropriate icon based on
submission()?.submitted && submission()?.isSynced- Displaying correct translation key based on the same condition
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the submission state handling implementation # Expected: Find the submission state management in the component class # Look for the submission signal/property definition ast-grep --pattern 'submission = signal<$_>($$$)' # Look for submission state updates rg -A 5 'submission\(\)\?' src/main/webapp/app/exam/participate/exercisesLength of output: 1591
Script:
#!/bin/bash # Look for the component class implementation fd exercise-save-button.component.ts --exec cat {} # Look for any submission-related services or models rg -l "class.*Submission" --type ts # Look for submission state updates in parent components fd exercise.component.ts --exec cat {}Length of output: 48478
Script:
#!/bin/bash # Look for the submission model definition cat src/main/webapp/app/entities/submission.model.ts # Look for the parent component that provides the submission signal fd exam-submission.component.ts --exec cat {}Length of output: 49509
src/main/webapp/app/exam/participate/exercises/exercise-save-button/exercise-save-button.component.ts (3)
8-14: LGTM! Component decorator follows Angular best practices.The component is properly configured as standalone with minimal dependencies and follows Angular's style guide.
15-25: LGTM! Component is lightweight and memory-efficient.The component has no subscriptions requiring cleanup and maintains a simple, focused responsibility.
15-25: Verify exam mode requirements implementation.The component seems to be missing some required functionality mentioned in the PR objectives:
- Auto-save after 30s inactivity
- Button state management (active/disabled)
- "Exercise Saved" state handling
Let's verify if these are implemented elsewhere:
Would you like help implementing these features in this component?
src/main/webapp/app/exam/participate/exercises/modeling/modeling-exam-submission.component.html (3)
1-1: LGTM! Proper usage of new Angular control flow syntax.The code correctly uses the new
@ifsyntax instead of*ngIfas per guidelines.Also applies to: 8-14
Line range hint
19-39: LGTM! Well-structured editor implementation.The modeling editor section is well organized with proper conditional rendering and component bindings.
Line range hint
40-54: Verify XSS protection for problemStatementHtml.The implementation looks good, but ensure that
problemStatementHtmlis properly sanitized before being rendered withinnerHTML.✅ Verification successful
Let me try a different approach to verify the XSS protection implementation.
Let me try one more search to find how the
problemStatementHtmlis processed before being rendered.
Let me search for how the problemStatementHtml is processed before being rendered.
I can now generate the final response based on the findings. The code shows that the problemStatementHtml is properly sanitized through multiple layers:
XSS protection is properly implemented for problemStatementHtml
The implementation is secure because:
The problemStatementHtml is processed through the
htmlForMarkdownutility which:
- Uses DOMPurify.sanitize() for HTML sanitization (as seen in markdown.conversion.util.ts)
- Has configurable allowed HTML tags and attributes
- Is thoroughly tested with various security scenarios (as seen in markdown.service.spec.ts)
The component initializes problemStatementHtml safely:
this.problemStatementHtml = htmlForMarkdown(this.exercise?.problemStatement);
- The update method also uses the same safe conversion:
updateProblemStatement(newProblemStatementHtml: string): void { this.problemStatementHtml = newProblemStatementHtml; this.changeDetectorReference.detectChanges(); }The implementation follows Angular security best practices by using proper HTML sanitization before rendering content with [innerHTML].
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for DomSanitizer usage in the component ast-grep --pattern 'DomSanitizer.bypassSecurityTrustHtml($_)'Length of output: 187
Script:
#!/bin/bash # Search for the component file to analyze sanitization fd "modeling-exam-submission.component.ts" --type f # Search for DomSanitizer imports and usage rg "DomSanitizer|sanitize|sanitization" -A 3Length of output: 74504
Script:
#!/bin/bash # Search for the component file to analyze how problemStatementHtml is processed cat src/main/webapp/app/exam/participate/exercises/modeling/modeling-exam-submission.component.ts # Search for updateProblemStatement method rg "updateProblemStatement" -A 5Length of output: 337020
Script:
#!/bin/bash # Search for how problemStatementHtml is initialized and processed rg "problemStatementHtml" -A 5 src/main/webapp/app/exam/participate/exercises/modeling/modeling-exam-submission.component.ts # Search for htmlForMarkdown utility rg "htmlForMarkdown" -A 5Length of output: 58969
src/main/webapp/app/exam/participate/exercises/text/text-exam-submission.component.html (2)
1-1: LGTM! Proper usage of new Angular control flow syntax.The @if directives are correctly used instead of *ngIf as per the coding guidelines.
Also applies to: 8-14
Line range hint
31-31: Verify maxLength implementation.The text editor has a maxLength binding to maxCharacterCount. Let's verify this implementation to ensure proper character limit enforcement.
✅ Verification successful
Let me gather more information to verify the maxLength implementation completely.
Let me search for the actual template implementation to verify how the maxLength is bound to the editor.
maxLength is properly implemented and enforced
The verification shows that the maxLength implementation is correct and consistent:
- The
maxCharacterCountconstant is defined as 30,000 characters in both frontend (MAX_SUBMISSION_TEXT_LENGTH = 30 * 1000) and backend (MAX_SUBMISSION_TEXT_LENGTH = 30_000).- The text editor component correctly binds this limit using
[maxLength]="maxCharacterCount"in the template.- The backend also enforces this limit through the
@Sizeannotation on the text field.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for maxCharacterCount definition and usage ast-grep --pattern 'maxCharacterCount = $_' rg -A 2 'maxCharacterCount.*='Length of output: 973
Script:
#!/bin/bash # Search for MAX_SUBMISSION_TEXT_LENGTH constant definition rg "MAX_SUBMISSION_TEXT_LENGTH.*=" -A 2 # Search for maxLength binding in the template rg "maxLength.*textEditor" -B 2 -A 2 # Search for any text editor related configurations ast-grep --pattern 'textEditor = $_'Length of output: 1720
Script:
#!/bin/bash # Search for the template file content cat src/main/webapp/app/exam/participate/exercises/text/text-exam-submission.component.html # Search for any text editor related directives or bindings rg -l "TextEditorComponent" rg "\[maxLength\]" -B 2 -A 2Length of output: 14331
src/main/webapp/app/exam/participate/exercises/exam-submission-components.module.ts (1)
21-21: Verify the ExerciseSaveButtonComponent implementationLet's verify that the component is properly implemented with the required functionality as per PR objectives.
✅ Verification successful
Let's gather more information about the component implementation to make a proper verification.
Based on the gathered information, I can now provide a final response about the component implementation.
ExerciseSaveButtonComponent implementation is properly structured and functional
The component implementation shows a well-structured Angular component with:
- Proper input handling for
Submissionobject using the new input syntax- Output event emitter for save actions
- Reactive template with conditional rendering for save/saved states
- Appropriate button states (disabled when synced)
- Internationalization support through
jhiTranslate- Icon handling for both saving and saved states
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the ExerciseSaveButtonComponent implementation # Test 1: Check if the component file exists and contains required features echo "Checking ExerciseSaveButtonComponent implementation..." rg -l --type ts "export class ExerciseSaveButtonComponent" # Test 2: Verify template contains save button with proper states echo "Checking template for save button implementation..." rg -l --type html "Exercise Saved" ./src/main/webapp/app/exam/participate/exercises/exercise-save-button/ # Test 3: Check for test file existence echo "Checking for component tests..." rg -l --type ts "describe.*ExerciseSaveButtonComponent" ./src/main/webapp/app/exam/participate/exercises/exercise-save-button/Length of output: 704
Script:
#!/bin/bash # Let's examine the actual implementation of the component # Check the component implementation echo "Component Implementation:" rg -A 20 "export class ExerciseSaveButtonComponent" src/main/webapp/app/exam/participate/exercises/exercise-save-button/exercise-save-button.component.ts # Check the template content echo -e "\nTemplate Content:" cat src/main/webapp/app/exam/participate/exercises/exercise-save-button/exercise-save-button.component.html # Check for any Input/Output decorators and dependencies echo -e "\nDecorators and Dependencies:" rg "@(Input|Output|Injectable)" src/main/webapp/app/exam/participate/exercises/exercise-save-button/exercise-save-button.component.tsLength of output: 1361
src/main/webapp/app/exam/participate/exercises/text/text-exam-submission.component.ts (2)
1-1: LGTM: Import changes are correct and follow Angular style guide.The addition of
outputimport and the switch to solid icons are appropriate for the new save functionality.Also applies to: 9-9
40-40: LGTM: Good use of readonly for immutable icon reference.Making the icon property readonly prevents accidental modifications and follows Angular best practices.
src/test/javascript/spec/component/exam/participate/exercises/text-exam-submission.component.spec.ts (2)
20-20: LGTM: Import statement correctly added.The import for ExerciseSaveButtonComponent is properly placed with other component imports.
43-43: LGTM: Component correctly mocked.The ExerciseSaveButtonComponent is properly mocked using NgMocks.MockComponent as per coding guidelines.
src/main/webapp/app/exam/participate/exercises/quiz/quiz-exam-submission.component.html (1)
Line range hint
16-52: Address technical debt regarding Map vs Array consistency.The TODO comments suggest inconsistencies in data structure usage. This should be addressed to maintain code quality.
Let's check for similar patterns in the codebase:
Would you like help creating a GitHub issue to track the Map vs Array consistency improvements?
src/main/webapp/app/exam/participate/exercises/modeling/modeling-exam-submission.component.ts (2)
1-1: LGTM! Import changes align with Angular style guide.The addition of
inputandoutputimports from @angular/core is appropriate for the new functionality.
40-40: Verify integration with ExerciseSaveButtonComponent.Let's verify that the saveCurrentExercise output is properly connected to the ExerciseSaveButtonComponent in the template.
✅ Verification successful
Let me try different commands to verify this integration.
Let me check the content of the template file to verify the integration.
Let me verify the connection between
notifyTriggerSave()andsaveCurrentExercisein the component.
Integration with ExerciseSaveButtonComponent is properly implemented
The verification shows that:
- The template includes
<jhi-exercise-save-button>with a(save)event binding tonotifyTriggerSave()- The
notifyTriggerSave()method correctly emits thesaveCurrentExerciseoutput event- The component properly connects the save button's event to the parent component through the output binding
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the usage of saveCurrentExercise in the template rg -t html "saveCurrentExercise" "src/main/webapp/app/exam/participate/exercises/modeling" # Verify ExerciseSaveButtonComponent is properly imported rg -t typescript "ExerciseSaveButtonComponent" "src/main/webapp/app/exam/participate/exercises/modeling"Length of output: 226
Script:
#!/bin/bash # Search for the template file corresponding to the component fd modeling-exam-submission.component.html src/main/webapp/app/exam/participate/exercises/modeling # Search for saveCurrentExercise in HTML files (using just the extension) rg "saveCurrentExercise" --type-add 'html:*.html' -t html src/main/webapp/app/exam/participate/exercises/modeling # Search for ExerciseSaveButtonComponent in TypeScript files rg "ExerciseSaveButtonComponent" --type-add 'ts:*.ts' -t ts src/main/webapp/app/exam/participate/exercises/modelingLength of output: 426
Script:
#!/bin/bash # Check the content of the template file cat src/main/webapp/app/exam/participate/exercises/modeling/modeling-exam-submission.component.html # Also search for any exercise-save-button usage in HTML files rg "<jhi-exercise-save-button" --type-add 'html:*.html' -t html src/main/webapp/app/exam/participate/exercises/modelingLength of output: 3321
Script:
#!/bin/bash # Check the TypeScript component implementation for notifyTriggerSave method rg "notifyTriggerSave" --type-add 'ts:*.ts' -t ts src/main/webapp/app/exam/participate/exercises/modeling/modeling-exam-submission.component.ts -A 5Length of output: 223
src/test/javascript/spec/component/exam/participate/exercises/modeling-exam-submission.component.spec.ts (1)
20-20: LGTM: Component properly imported and mocked.The ExerciseSaveButtonComponent is correctly imported and declared using MockComponent in the TestBed configuration, following the coding guidelines for mocking components.
Also applies to: 53-53
src/test/javascript/spec/component/exam/participate/exercises/quiz-exam-submission.component.spec.ts (1)
28-28: LGTM! Proper integration of ExerciseSaveButtonComponent in test setup.The import and declaration of the ExerciseSaveButtonComponent follow the correct patterns and testing guidelines.
Also applies to: 59-59
src/main/webapp/app/exam/participate/exam-participation.component.html (4)
62-62: LGTM: Save functionality added to quiz submission.The addition of the save event handler is consistent with the PR objectives and matches the implementation pattern used across other exercise types.
72-76: LGTM: Save functionality added to text editor.The save event handler is properly implemented and follows the same pattern as other exercise types.
Line range hint
1-276: LGTM: Template follows latest Angular control flow syntax.The template correctly uses the new
@ifand@forsyntax as required by the coding guidelines, replacing the older*ngIfand*ngFordirectives.
79-84:⚠️ Potential issueConsider adding null safety checks for submissions array access.
While the save functionality is properly implemented, the
submissions![0]access could be unsafe if the array is empty. Consider adding a null check or using optional chaining.Consider updating the sync status binding to include null checks:
-[isSubmissionSynced]="exercise.studentParticipations[0].submissions![0].isSynced" +[isSubmissionSynced]="exercise.studentParticipations[0]?.submissions?.[0]?.isSynced"src/main/webapp/app/exam/participate/exercises/quiz/quiz-exam-submission.component.ts (2)
291-296: LGTM! Well-documented and follows single responsibility principle.The
notifyTriggerSavemethod is implemented correctly with proper documentation and follows good practices.
56-56: Consider implementing OnDestroy for proper cleanup.To prevent potential memory leaks, consider implementing the
OnDestroyinterface and unsubscribing from theEventEmitterwhen the component is destroyed.Example implementation:
export class QuizExamSubmissionComponent extends ExamSubmissionComponent implements OnInit, OnDestroy { // ... existing code ... ngOnDestroy() { this.saveCurrentExercise.complete(); // Call parent's ngOnDestroy if it exists if (super.ngOnDestroy) { super.ngOnDestroy(); } } }src/main/webapp/i18n/en/exam.json (1)
342-343: LGTM! The new translation entries are clear and consistent.The added translations for the save exercise functionality are well-implemented:
- Clear and concise button labels
- Consistent capitalization with other UI elements
- Matches the requirements from PR objectives
...webapp/app/exam/participate/exercises/exercise-save-button/exercise-save-button.component.ts
Show resolved
Hide resolved
src/main/webapp/app/exam/participate/exercises/exam-submission-components.module.ts
Show resolved
Hide resolved
src/main/webapp/app/exam/participate/exercises/text/text-exam-submission.component.ts
Show resolved
Hide resolved
.../javascript/spec/component/exam/participate/exercises/text-exam-submission.component.spec.ts
Show resolved
Hide resolved
src/main/webapp/app/exam/participate/exercises/modeling/modeling-exam-submission.component.ts
Show resolved
Hide resolved
src/main/webapp/app/exam/participate/exercises/quiz/quiz-exam-submission.component.ts
Show resolved
Hide resolved
src/main/webapp/app/exam/participate/exercises/quiz/quiz-exam-submission.component.ts
Show resolved
Hide resolved
Pull latest changes
|
github-actions
bot
added
the
deployment-error
edkaya
removed
the
deployment-error
Checklist
General
Client
Motivation and Context
Students can save their exercises only by clicking on a different/same exercise in the sidebar after they are done with the exercise (only in modeling, quiz, and text exercises). However, it may be confusing for the students who are not familiar with Artemis and its exam mode.
Description
Steps for Testing
Prerequisites:
(I created an exam with modeling, text and quiz exercises called "Exam Save Exercise Button" under my course on TS5. Test users 1-5 are registered, so you can test the PR with them.)
Testserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.
Test Coverage
Review Progress
Code Review
Manual Tests
Exam Mode Test
Screenshots
Save Exercise
Exercise Saved
Summary by CodeRabbit
Summary by CodeRabbit
Release Notes
New Features
ExerciseSaveButtoncomponent for saving exercises during participation.Bug Fixes
Documentation
ExerciseSaveButtoncomponent.These updates enhance the user experience by providing better interaction capabilities and clearer feedback during exam participation.