Development: Migrate assessment module client to angular decoratorless API
#10860
Conversation
…igrate-client-to-signals
github-actions
bot
added
client
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
End-to-End (E2E) Test Results Summary |
… from emitted event
github-actions
bot
added
atlas
End-to-End (E2E) Test Results Summary |
End-to-End (E2E) Test Results Summary |
maximiliansoelch
moved this from Work In Progress
to Ready For Review
in Artemis Development
WalkthroughThis update refactors a large set of Angular components to use the new standalone reactive input/output API ( Changes
Sequence Diagram(s)sequenceDiagram
participant ParentComponent
participant ChildComponent
Note over ParentComponent,ChildComponent: New Reactive Input/Output API
ParentComponent->>ChildComponent: [inputProp]="value" (template)
Note right of ChildComponent: inputProp declared via input()/model()
ChildComponent-->>ParentComponent: value accessed as inputProp()
ParentComponent->>ChildComponent: (outputEvent)="handler()" (template)
Note right of ChildComponent: outputEvent declared via output()
ChildComponent-->>ParentComponent: outputEvent.emit(data)
sequenceDiagram
participant Test
participant ComponentFixture
participant Component
Test->>ComponentFixture: setInput('inputName', value)
ComponentFixture->>Component: updates reactive input
Note right of Component: input accessed as inputName()
Test->>ComponentFixture: detectChanges()
Component->>Component: uses inputName() in logic/template
sequenceDiagram
participant Component
participant Template
Template->>Component: Calls inputProp() for value
Template->>Component: Calls outputEvent.emit(data) for events
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 10
🔭 Outside diff range comments (3)
src/main/webapp/app/assessment/overview/complaint-form/complaints-form.component.ts (1)
84-87: 🛠️ Refactor suggestionPrefer template reference or
ViewChildover directdocument.querySelectorManipulating the DOM via
document.querySelectorbypasses Angular’s change-detection and is hard to unit-test. Consider binding the<textarea>to a template reference (#complainText) and reading its.value.length, or expose the length directly through[(ngModel)].- const textArea: HTMLTextAreaElement = document.querySelector('#complainTextArea') as HTMLTextAreaElement; - return textArea.value.length; + // e.g. using a template reference variable + return this.complaintText?.length ?? 0;src/main/webapp/app/assessment/manage/assessment-instructions/collapsable-assessment-instructions/collapsable-assessment-instructions.component.ts (1)
32-55:⚠️ Potential issueDetach
interact.jslisteners on destroy to prevent memory leaks
interact('.expanded-instructions')…registers global listeners but they are never removed. On repeated navigation this can cause duplicated handlers and leaks.@@ export class CollapsableAssessmentInstructionsComponent implements AfterViewInit, OnDestroy { @@ ngAfterViewInit(): void { - interact('.expanded-instructions') + this.interactable = interact('.expanded-instructions') .resizable({ … }); } + + private interactable?: ReturnType<typeof interact>; + + ngOnDestroy(): void { + this.interactable?.unset(); // clean-up + }Remember to add
implements OnDestroyin the class declaration and import it from@angular/core.src/main/webapp/app/assessment/manage/complaints-for-tutor/complaints-for-tutor.component.ts (1)
90-101:⚠️ Potential issueNull-safety check before locking a complaint.
this.complaint().id!assumes both the complaint and its id exist.
Add a guard clause to avoid 500 responses or crashes when the id is missing:- this.complaintResponseService - .createLock(this.complaint().id!) +const complaint = this.complaint(); +if (!complaint?.id) { + this.alertService.error('artemisApp.complaint.missingId'); + return; +} +this.complaintResponseService + .createLock(complaint.id)
🧹 Nitpick comments (23)
src/main/webapp/app/assessment/manage/assessment-note/assessment-note.component.ts (1)
15-25: Consider migrating the input property to the decoratorless API for consistency.While you've successfully migrated the output property to use the new reactive API, the
assessmentNoteinput property still uses the traditional@Inputdecorator with setter/getter pattern. For consistency with the rest of the codebase migration, consider refactoring this to use themodel()API which supports two-way binding:readonly assessmentNote = model<AssessmentNote>(new AssessmentNote()); // Remove the setter/getter and instead update your component to use assessmentNote() and assessmentNote.set()Alternatively, if you need the same logic in the setter, you could use:
private _assessmentNoteInternal = signal<AssessmentNote>(new AssessmentNote()); readonly assessmentNote = input<AssessmentNote | undefined>({ transform: (value) => { if (value === undefined) { this._assessmentNoteInternal.set(new AssessmentNote()); } else { this._assessmentNoteInternal.set(value); } return value; } }); // And use _assessmentNoteInternal() to access the valuesrc/main/webapp/app/assessment/manage/unreferenced-feedback-detail/unreferenced-feedback-detail.component.html (1)
86-102: Consider simplifying conditional rendering with @if/@elseThe template uses multiple separate
@ifblocks with mutually exclusive conditions that could be simplified.Consider refactoring to use the more concise
@if/@elsepattern:@if (feedback().correctionStatus !== undefined) { <div> - @if (feedback().correctionStatus === 'CORRECT') { - <span class="text-success">{{ 'artemisApp.exampleSubmission.feedback.' + feedback().correctionStatus! | artemisTranslate }} </span> - } - @if (feedback().correctionStatus !== 'CORRECT') { - <span class="text-danger">{{ 'artemisApp.exampleSubmission.feedback.' + feedback().correctionStatus! | artemisTranslate }} </span> - } + @if (feedback().correctionStatus === 'CORRECT') { + <span class="text-success">{{ 'artemisApp.exampleSubmission.feedback.' + feedback().correctionStatus! | artemisTranslate }} </span> + } @else { + <span class="text-danger">{{ 'artemisApp.exampleSubmission.feedback.' + feedback().correctionStatus! | artemisTranslate }} </span> + <!-- :warning: emoji was rendered as a black-white glyph, hence the solution with the fa-icon --> + <fa-layers> + <fa-icon class="text-warning" [icon]="faExclamationTriangle" /> + <fa-icon class="text-dark exclamation-icon" [icon]="faExclamation" size="2x" transform="shrink-10" /> + </fa-layers> + } - <!-- :warning: emoji was rendered as a black-white glyph, hence the solution with the fa-icon --> - @if (feedback().correctionStatus !== 'CORRECT') { - <fa-layers> - <fa-icon class="text-warning" [icon]="faExclamationTriangle" /> - <fa-icon class="text-dark exclamation-icon" [icon]="faExclamation" size="2x" transform="shrink-10" /> - </fa-layers> - } </div> }src/main/webapp/app/assessment/overview/complaint-form/complaints-form.component.spec.ts (1)
51-51: Remove redundant input settingThis line is redundant as the same input is already set in the
beforeEachblock.-fixture.componentRef.setInput('exercise', exercise);src/main/webapp/app/assessment/shared/assessment-dashboard/assessment-dashboard-information.component.spec.ts (1)
59-60: Remove redundant isExamMode input settingThe
isExamModeinput is already set tofalsein thebeforeEachblock, making this line redundant.-fixture.componentRef.setInput('isExamMode', false); fixture.componentRef.setInput('examId', 42);src/main/webapp/app/assessment/overview/complaints-for-students/complaint-student-view.component.spec.ts (3)
170-171: Simplified DOM element mocking approachGood improvement on the DOM element mocking strategy by directly assigning the mock function to the DOM element instead of creating an ElementRef. This is cleaner and more straightforward.
Consider adding a null check before assigning the scrollIntoView function to handle potential cases where the element might not be found:
-fixture.nativeElement.querySelector('#complaintScrollpoint').scrollIntoView = scrollIntoViewMock; +const scrollElement = fixture.nativeElement.querySelector('#complaintScrollpoint'); +if (scrollElement) { + scrollElement.scrollIntoView = scrollIntoViewMock; +}
262-263: Same DOM mocking approach used consistentlyThe same simplified DOM element mocking strategy is used here, which is good for consistency, but the same null check recommendation applies.
See suggestion from previous comment about adding a null check.
286-287: Consistent DOM mocking approachSame simplified DOM element mocking strategy is used consistently throughout the tests.
See suggestion from previous comments about adding a null check.
src/main/webapp/app/assessment/shared/assessment-dashboard/assessment-dashboard-information.component.html (1)
9-23: Consider using a local template variable for frequently accessed values.The
numberOfSubmissions().totalis called multiple times in the template. For better performance, consider storing it in a local template variable:-@if (numberOfSubmissions().total !== 0) { +@if (numberOfSubmissions(); as submissions) { +@if (submissions.total !== 0) { <div class="pie-chart pb-2" style="margin-top: -10px"> <ngx-charts-pie-chart [customColors]="customColors" [view]="view" [results]="assessments" [legend]="true" [legendTitle]="''" [legendPosition]="legendPosition" [animations]="false" > - <ng-template #tooltipTemplate let-model="model"> {{ ((model.value * 100) / this.numberOfSubmissions().total).toFixed(2) }}% </ng-template> + <ng-template #tooltipTemplate let-model="model"> {{ ((model.value * 100) / submissions.total).toFixed(2) }}% </ng-template> </ngx-charts-pie-chart> </div> }src/main/webapp/app/assessment/manage/assessment-warning/assessment-warning.component.ts (1)
56-58: Use optional chaining for safer property access.The
reduceoperation could benefit from optional chaining for safer property access:return this.submissions() .map((submission) => submission.participation?.individualDueDate) - .reduce((latest, next) => (next && next.isAfter(latest) ? next : latest), this.exercise().dueDate); + .reduce((latest, next) => (next && next?.isAfter(latest) ? next : latest), this.exercise().dueDate);This addresses the static analysis hint and prevents potential errors if
nextis defined butisAfteris not a method on it.🧰 Tools
🪛 Biome (1.9.4)
[error] 58-58: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/main/webapp/app/assessment/overview/complaint-form/complaints-form.component.ts (2)
38-45: Avoid re-invoking signal accessors inside the same method
this.exercise()is called three times insidengOnInit. Although signal access is cheap, reading it once and re-using the local reference improves readability and avoids surprises if the accessor ever becomes reactive.- this.course = getCourseFromExercise(this.exercise()); + const exercise = this.exercise(); // single read + this.course = getCourseFromExercise(exercise); this.maxComplaintTextLimit = this.course?.maxComplaintTextLimit ?? 0; - const exercise = this.exercise();
55-60: Guard against undefinedexamIdto keep payload minimal
examIdis optional. If it isundefined, adding the field to the DTO serialises it asnull, which might break strict-null REST contracts.- complaintRequest.examId = this.examId(); + const examId = this.examId(); + if (examId !== undefined) { + complaintRequest.examId = examId; + }src/main/webapp/app/assessment/manage/assessment-header/assessment-header.component.ts (3)
1-1: Mixing decorator-less API with classic@Input()– deliberate but be consistentThe file now uses both
input()/output()and an@Input()setter (highlightDifferences).
That is perfectly valid when two-way binding of a primitive is required, but please keep the dual approach documented for future maintainers.
56-58: Consider initial value for_highlightDifferences
_highlightDifferencesisbooleanbut never initialised, so it starts asundefined, which could leak into consumers before the first setter call. Initialising tofalseis safer.- private _highlightDifferences: boolean; + private _highlightDifferences = false;
98-106: Minor readability: cacheresult()call
result()is invoked repeatedly inside the same getter. Storing it locally improves readability.- const result = this.result(); - if (result?.completionDate) { + const res = this.result(); + if (res?.completionDate) { return true; - } else if (Result.hasNonEmptyAssessmentNote(result)) { + } else if (Result.hasNonEmptyAssessmentNote(res)) {src/main/webapp/app/assessment/manage/assessment-header/assessment-header.component.html (2)
27-82: Consider extracting complex ‑and duplicate- conditions into computed helpers.The block contains 7 highly-nested
@ifstatements that repeat the same signal calls (isAssessor(),hasComplaint(),complaintType(),exercise.assessmentType,exercise.teamMode, …).
Besides reducing readability, every call re-evaluates the signal getter during change detection.Suggestion (illustrative):
readonly showMoreFeedbackRequest = computed(() => isAssessor() && !exercise().isAtLeastInstructor && hasComplaint() && complaintType() !== ComplaintType.COMPLAINT && !complaintHandled() && exercise().assessmentType !== AssessmentType.AUTOMATIC, );The template then becomes:
@if (showMoreFeedbackRequest()) { <span id="moreFeedbackRequest" …></span> }This removes duplication, shortens the template and avoids unnecessary signal look-ups.
88-102: Minor: group busy-state checks into a single helper to avoid repetition.
[disabled]="saveBusy() || submitBusy() || cancelBusy()"(and similar) is repeated for several buttons.
A small computed helper (anyBusy()) or areadonly busy = computed(() => …)would centralize the logic and prevent future divergence.src/main/webapp/app/assessment/manage/assessment-instructions/assessment-instructions/assessment-instructions.component.ts (1)
1-1: Avoid mixing classic@Input()with the new signal-based API for the same data source.The file imports both
Inputand the newinput/modelhelpers, and keepsexerciseas a classic setter while all other inputs are signals.
For consistency and to prevent two different change-detection paths, migrateexercisetoinput.required<Exercise>()(plus acomputed()view model), or keep all inputs decorator-based until the full component is migrated.src/main/webapp/app/assessment/manage/complaints-for-tutor/complaints-for-tutor.component.ts (2)
35-41: Mark required inputs as such to avoid unnecessary non-null assertions.
exercise,submission, and especiallycomplaintare accessed with!later (exercise()!,complaint().id!).
Declare them withinput.required<T>()(ormodel.required) so the compiler enforces presence and you can drop the bang operator:-readonly exercise = input<Exercise>(); +readonly exercise = input.required<Exercise>();
171-179:complaint().complaintTypeis accessed without null checks.Inside the acceptance branch the signal is called multiple times.
Cache the complaint once, or use the already-available variable from the guard above to avoid inconsistencies and extra calls.src/main/webapp/app/assessment/manage/assessment-layout/assessment-layout.component.ts (1)
53-60: Consider migrating the remaininghighlightDifferencesproperty to decoratorless API.While most properties have been migrated to the new API,
highlightDifferencesstill uses the traditional@Inputdecorator. This creates an inconsistency in the component's API style.-@Input() set highlightDifferences(highlightDifferences: boolean) { - this._highlightDifferences = highlightDifferences; - this.highlightDifferencesChange.emit(this.highlightDifferences); -} - -get highlightDifferences() { - return this._highlightDifferences; -} +readonly highlightDifferences = model<boolean>(false, { + transform: (value) => { + this.highlightDifferencesChange.emit(value); + return value; + } +});src/main/webapp/app/assessment/manage/unreferenced-feedback-detail/unreferenced-feedback-detail.component.ts (3)
42-48: Provide defaults for optional boolean inputs
isSuggestionis declared asinput<boolean>()without a default value, resulting inundefinedwhen the attribute is omitted. The template often treats it as a boolean flag, soundefined→ falsy may look okay, but it complicates strict type checks (strictNullChecks).-readonly isSuggestion = input<boolean>(); +// default to false to avoid undefined checks everywhere +readonly isSuggestion = input<boolean>(false);Same argument applies to
highlightDifferences. Giving it an explicit default (false) is already done – nice job!
49-53: Unify access-modifiers & align with naming convention
You mixpublic readonly(lines 49-50) withreadonly(lines 51-52). The signals are already immutable references; “public” adds no value because the class is exported. A consistent modifier set improves readability.-public readonly onFeedbackChange = output<Feedback>(); -public readonly onFeedbackDelete = output<Feedback>(); -readonly onAcceptSuggestion = output<Feedback>(); -readonly onDiscardSuggestion = output<Feedback>(); +readonly onFeedbackChange = output<Feedback>(); +readonly onFeedbackDelete = output<Feedback>(); +readonly onAcceptSuggestion = output<Feedback>(); +readonly onDiscardSuggestion = output<Feedback>();
109-112: CallpreventDefault()when hijacking a drop event
event.stopPropagation()prevents bubbling but still lets the browser execute its default action (e.g. opening a link when files are dropped). AddingpreventDefault()ensures no accidental navigation occurs.-event.stopPropagation(); +event.preventDefault(); +event.stopPropagation();
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (57)
src/main/webapp/app/assessment/manage/assessment-complaint-alert/assessment-complaint-alert.component.html(1 hunks)src/main/webapp/app/assessment/manage/assessment-complaint-alert/assessment-complaint-alert.component.spec.ts(1 hunks)src/main/webapp/app/assessment/manage/assessment-complaint-alert/assessment-complaint-alert.component.ts(2 hunks)src/main/webapp/app/assessment/manage/assessment-header/assessment-header.component.html(5 hunks)src/main/webapp/app/assessment/manage/assessment-header/assessment-header.component.spec.ts(13 hunks)src/main/webapp/app/assessment/manage/assessment-header/assessment-header.component.ts(6 hunks)src/main/webapp/app/assessment/manage/assessment-instructions/assessment-instructions/assessment-instructions.component.html(4 hunks)src/main/webapp/app/assessment/manage/assessment-instructions/assessment-instructions/assessment-instructions.component.ts(3 hunks)src/main/webapp/app/assessment/manage/assessment-instructions/collapsable-assessment-instructions/collapsable-assessment-instructions.component.html(2 hunks)src/main/webapp/app/assessment/manage/assessment-instructions/collapsable-assessment-instructions/collapsable-assessment-instructions.component.spec.ts(1 hunks)src/main/webapp/app/assessment/manage/assessment-instructions/collapsable-assessment-instructions/collapsable-assessment-instructions.component.ts(2 hunks)src/main/webapp/app/assessment/manage/assessment-instructions/expandable-section/expandable-section.component.html(1 hunks)src/main/webapp/app/assessment/manage/assessment-instructions/expandable-section/expandable-section.component.spec.ts(2 hunks)src/main/webapp/app/assessment/manage/assessment-instructions/expandable-section/expandable-section.component.ts(3 hunks)src/main/webapp/app/assessment/manage/assessment-layout/assessment-layout.component.html(2 hunks)src/main/webapp/app/assessment/manage/assessment-layout/assessment-layout.component.spec.ts(2 hunks)src/main/webapp/app/assessment/manage/assessment-layout/assessment-layout.component.ts(3 hunks)src/main/webapp/app/assessment/manage/assessment-note/assessment-note.component.ts(2 hunks)src/main/webapp/app/assessment/manage/assessment-warning/assessment-warning.component.spec.ts(3 hunks)src/main/webapp/app/assessment/manage/assessment-warning/assessment-warning.component.ts(3 hunks)src/main/webapp/app/assessment/manage/complaint-response/complaint-response.component.html(1 hunks)src/main/webapp/app/assessment/manage/complaint-response/complaint-response.component.ts(2 hunks)src/main/webapp/app/assessment/manage/complaints-for-tutor/complaints-for-tutor.component.html(5 hunks)src/main/webapp/app/assessment/manage/complaints-for-tutor/complaints-for-tutor.component.spec.ts(9 hunks)src/main/webapp/app/assessment/manage/complaints-for-tutor/complaints-for-tutor.component.ts(11 hunks)src/main/webapp/app/assessment/manage/grading-system/grading-key/grading-key-table.component.html(1 hunks)src/main/webapp/app/assessment/manage/grading-system/grading-key/grading-key-table.component.spec.ts(2 hunks)src/main/webapp/app/assessment/manage/grading-system/grading-key/grading-key-table.component.ts(4 hunks)src/main/webapp/app/assessment/manage/rating/star-rating/star-rating.component.ts(3 hunks)src/main/webapp/app/assessment/manage/structured-grading-instructions-assessment-layout/structured-grading-instructions-assessment-layout.component.html(2 hunks)src/main/webapp/app/assessment/manage/structured-grading-instructions-assessment-layout/structured-grading-instructions-assessment-layout.component.spec.ts(2 hunks)src/main/webapp/app/assessment/manage/structured-grading-instructions-assessment-layout/structured-grading-instructions-assessment-layout.component.ts(2 hunks)src/main/webapp/app/assessment/manage/unreferenced-feedback-detail/assessment-correction-round-badge/assessment-correction-round-badge.component.html(1 hunks)src/main/webapp/app/assessment/manage/unreferenced-feedback-detail/assessment-correction-round-badge/assessment-correction-round-badge.component.ts(2 hunks)src/main/webapp/app/assessment/manage/unreferenced-feedback-detail/unreferenced-feedback-detail.component.html(3 hunks)src/main/webapp/app/assessment/manage/unreferenced-feedback-detail/unreferenced-feedback-detail.component.spec.ts(2 hunks)src/main/webapp/app/assessment/manage/unreferenced-feedback-detail/unreferenced-feedback-detail.component.ts(3 hunks)src/main/webapp/app/assessment/overview/complaint-form/complaints-form.component.html(3 hunks)src/main/webapp/app/assessment/overview/complaint-form/complaints-form.component.spec.ts(4 hunks)src/main/webapp/app/assessment/overview/complaint-form/complaints-form.component.ts(4 hunks)src/main/webapp/app/assessment/overview/complaint-request/complaint-request.component.html(1 hunks)src/main/webapp/app/assessment/overview/complaint-request/complaint-request.component.ts(2 hunks)src/main/webapp/app/assessment/overview/complaints-for-students/complaint-student-view.component.spec.ts(21 hunks)src/main/webapp/app/assessment/overview/complaints-for-students/complaints-student-view.component.html(2 hunks)src/main/webapp/app/assessment/overview/complaints-for-students/complaints-student-view.component.ts(7 hunks)src/main/webapp/app/assessment/shared/assessment-dashboard/assessment-dashboard-information.component.html(4 hunks)src/main/webapp/app/assessment/shared/assessment-dashboard/assessment-dashboard-information.component.spec.ts(4 hunks)src/main/webapp/app/assessment/shared/assessment-dashboard/assessment-dashboard-information.component.ts(4 hunks)src/main/webapp/app/assessment/shared/assessment-dashboard/exercise-dashboard/language-table-cell/language-table-cell.component.ts(1 hunks)src/main/webapp/app/assessment/shared/assessment-dashboard/exercise-dashboard/second-correction-button/second-correction-enable-button.component.html(1 hunks)src/main/webapp/app/assessment/shared/assessment-dashboard/exercise-dashboard/second-correction-button/second-correction-enable-button.component.ts(2 hunks)src/main/webapp/app/assessment/shared/info-panel/info-panel.component.html(1 hunks)src/main/webapp/app/assessment/shared/info-panel/info-panel.component.ts(1 hunks)src/main/webapp/app/atlas/overview/judgement-of-learning-rating/judgement-of-learning-rating-component.spec.ts(2 hunks)src/main/webapp/app/atlas/overview/judgement-of-learning-rating/judgement-of-learning-rating.component.ts(1 hunks)src/main/webapp/app/exercise/rating/rating.component.spec.ts(0 hunks)src/main/webapp/app/exercise/rating/rating.component.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/main/webapp/app/exercise/rating/rating.component.spec.ts
🧰 Additional context used
📓 Path-based instructions (2)
`src/main/webapp/**/*.ts`: angular_style:https://angular.io/guide/styleguide;methods_in_html:false;lazy_loading:true;code_reuse:true;tests:meaningful;types:PascalCase;enums:PascalC...
src/main/webapp/app/assessment/manage/assessment-complaint-alert/assessment-complaint-alert.component.spec.tssrc/main/webapp/app/assessment/overview/complaints-for-students/complaint-student-view.component.spec.tssrc/main/webapp/app/assessment/overview/complaint-form/complaints-form.component.tssrc/main/webapp/app/assessment/shared/assessment-dashboard/exercise-dashboard/second-correction-button/second-correction-enable-button.component.tssrc/main/webapp/app/atlas/overview/judgement-of-learning-rating/judgement-of-learning-rating-component.spec.tssrc/main/webapp/app/assessment/manage/assessment-note/assessment-note.component.tssrc/main/webapp/app/assessment/manage/structured-grading-instructions-assessment-layout/structured-grading-instructions-assessment-layout.component.spec.tssrc/main/webapp/app/assessment/manage/unreferenced-feedback-detail/unreferenced-feedback-detail.component.spec.tssrc/main/webapp/app/assessment/manage/assessment-instructions/expandable-section/expandable-section.component.spec.tssrc/main/webapp/app/assessment/manage/assessment-warning/assessment-warning.component.spec.tssrc/main/webapp/app/assessment/manage/assessment-layout/assessment-layout.component.spec.tssrc/main/webapp/app/atlas/overview/judgement-of-learning-rating/judgement-of-learning-rating.component.tssrc/main/webapp/app/assessment/manage/grading-system/grading-key/grading-key-table.component.spec.tssrc/main/webapp/app/assessment/shared/assessment-dashboard/assessment-dashboard-information.component.spec.tssrc/main/webapp/app/assessment/manage/complaints-for-tutor/complaints-for-tutor.component.spec.tssrc/main/webapp/app/assessment/shared/info-panel/info-panel.component.tssrc/main/webapp/app/exercise/rating/rating.component.tssrc/main/webapp/app/assessment/manage/assessment-instructions/collapsable-assessment-instructions/collapsable-assessment-instructions.component.spec.tssrc/main/webapp/app/assessment/manage/grading-system/grading-key/grading-key-table.component.tssrc/main/webapp/app/assessment/overview/complaint-form/complaints-form.component.spec.tssrc/main/webapp/app/assessment/manage/rating/star-rating/star-rating.component.tssrc/main/webapp/app/assessment/manage/assessment-header/assessment-header.component.spec.tssrc/main/webapp/app/assessment/manage/assessment-complaint-alert/assessment-complaint-alert.component.tssrc/main/webapp/app/assessment/shared/assessment-dashboard/exercise-dashboard/language-table-cell/language-table-cell.component.tssrc/main/webapp/app/assessment/manage/complaint-response/complaint-response.component.tssrc/main/webapp/app/assessment/overview/complaint-request/complaint-request.component.tssrc/main/webapp/app/assessment/manage/assessment-warning/assessment-warning.component.tssrc/main/webapp/app/assessment/manage/assessment-instructions/expandable-section/expandable-section.component.tssrc/main/webapp/app/assessment/manage/assessment-header/assessment-header.component.tssrc/main/webapp/app/assessment/manage/assessment-instructions/assessment-instructions/assessment-instructions.component.tssrc/main/webapp/app/assessment/manage/unreferenced-feedback-detail/assessment-correction-round-badge/assessment-correction-round-badge.component.tssrc/main/webapp/app/assessment/manage/assessment-instructions/collapsable-assessment-instructions/collapsable-assessment-instructions.component.tssrc/main/webapp/app/assessment/overview/complaints-for-students/complaints-student-view.component.tssrc/main/webapp/app/assessment/manage/complaints-for-tutor/complaints-for-tutor.component.tssrc/main/webapp/app/assessment/manage/assessment-layout/assessment-layout.component.tssrc/main/webapp/app/assessment/manage/structured-grading-instructions-assessment-layout/structured-grading-instructions-assessment-layout.component.tssrc/main/webapp/app/assessment/shared/assessment-dashboard/assessment-dashboard-information.component.tssrc/main/webapp/app/assessment/manage/unreferenced-feedback-detail/unreferenced-feedback-detail.component.ts
`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/**/*.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/assessment/shared/info-panel/info-panel.component.htmlsrc/main/webapp/app/assessment/overview/complaint-form/complaints-form.component.htmlsrc/main/webapp/app/assessment/manage/assessment-header/assessment-header.component.htmlsrc/main/webapp/app/assessment/manage/grading-system/grading-key/grading-key-table.component.htmlsrc/main/webapp/app/assessment/overview/complaints-for-students/complaints-student-view.component.htmlsrc/main/webapp/app/assessment/manage/assessment-instructions/expandable-section/expandable-section.component.htmlsrc/main/webapp/app/assessment/manage/structured-grading-instructions-assessment-layout/structured-grading-instructions-assessment-layout.component.htmlsrc/main/webapp/app/assessment/manage/unreferenced-feedback-detail/assessment-correction-round-badge/assessment-correction-round-badge.component.htmlsrc/main/webapp/app/assessment/shared/assessment-dashboard/exercise-dashboard/second-correction-button/second-correction-enable-button.component.htmlsrc/main/webapp/app/assessment/manage/assessment-complaint-alert/assessment-complaint-alert.component.htmlsrc/main/webapp/app/assessment/manage/assessment-instructions/collapsable-assessment-instructions/collapsable-assessment-instructions.component.htmlsrc/main/webapp/app/assessment/manage/complaints-for-tutor/complaints-for-tutor.component.htmlsrc/main/webapp/app/assessment/manage/unreferenced-feedback-detail/unreferenced-feedback-detail.component.htmlsrc/main/webapp/app/assessment/manage/complaint-response/complaint-response.component.htmlsrc/main/webapp/app/assessment/manage/assessment-instructions/assessment-instructions/assessment-instructions.component.htmlsrc/main/webapp/app/assessment/shared/assessment-dashboard/assessment-dashboard-information.component.htmlsrc/main/webapp/app/assessment/manage/assessment-layout/assessment-layout.component.htmlsrc/main/webapp/app/assessment/overview/complaint-request/complaint-request.component.html
🪛 Biome (1.9.4)
src/main/webapp/app/assessment/manage/assessment-warning/assessment-warning.component.ts
[error] 58-58: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (128)
src/main/webapp/app/assessment/shared/assessment-dashboard/exercise-dashboard/language-table-cell/language-table-cell.component.ts (1)
1-1: Well-executed migration to Angular decoratorless API!The component has been successfully refactored to use Angular's new reactive primitives instead of traditional decorators. The changes align perfectly with the PR objective of modernizing the codebase:
- Replaced
@Input()decorator withinput.required<Submission>()- Created a computed property for derived state
- Updated the template to use function call syntax for accessing reactive values
These changes improve type safety and make reactivity more explicit while maintaining the same component behavior.
Also applies to: 8-8, 12-13
src/main/webapp/app/assessment/shared/assessment-dashboard/exercise-dashboard/second-correction-button/second-correction-enable-button.component.html (1)
3-3: Good use of new Angular control flow syntax.The migration from
*ngIfto@ifand property access to method calls (secondCorrectionEnabled()andtogglingSecondCorrectionButton()) correctly implements Angular's modern control flow syntax and aligns with the reactive input pattern.Also applies to: 6-6, 12-12, 15-15
src/main/webapp/app/assessment/shared/assessment-dashboard/exercise-dashboard/second-correction-button/second-correction-enable-button.component.ts (3)
1-1: Correct import updates for decoratorless API.The imports have been properly updated to include
inputandoutputfrom '@angular/core' for the reactive API.
13-14: Properly implemented reactive inputs.The input properties have been correctly converted from
@Input()decorators to the new reactiveinput<boolean>()API, maintaining their functionality while modernizing the implementation.
16-16: Properly implemented reactive output.The output has been correctly converted from
@Output()decorator withEventEmitterto the new reactiveoutput()API, maintaining the same event emission functionality.src/main/webapp/app/assessment/shared/info-panel/info-panel.component.html (1)
3-3: Good implementation of the decoratorless API pattern.The template correctly uses function call syntax
panelHeader()instead of property access, which aligns with the reactive input pattern being implemented. This ensures proper reactivity when the input value changes.src/main/webapp/app/assessment/shared/info-panel/info-panel.component.ts (2)
1-1: Correctly imported the new input API.The import statement has been properly updated to include the
inputfunction from Angular core, which is required for the decoratorless API implementation.
9-10: Well-implemented migration to decoratorless inputs.The code properly:
- Uses
input.required<string>()for the mandatory panelHeader input- Uses
input<string>()for the optional panelDescriptionHeader input- Marks both properties as readonly to prevent reassignment
This implementation follows Angular's best practices for the new reactive input API.
src/main/webapp/app/atlas/overview/judgement-of-learning-rating/judgement-of-learning-rating-component.spec.ts (2)
93-93: LGTM! Event object simplified correctly.The test has been updated to use the simplified event object structure that aligns with the component's updated
onRatemethod signature, removing the unnecessarystarRatingproperty.
107-108: LGTM! Consistent event object simplification.This change correctly maintains consistency with the prior test case and matches the component's updated event object structure.
src/main/webapp/app/atlas/overview/judgement-of-learning-rating/judgement-of-learning-rating.component.ts (1)
30-30: LGTM! Method signature simplified appropriately.The
onRatemethod signature has been successfully simplified to only include the necessary data (oldValueandnewValue), removing the dependency on theStarRatingComponenttype. This reduces coupling between components and aligns with the decoratorless API migration.src/main/webapp/app/exercise/rating/rating.component.ts (1)
50-50: LGTM! Method signature simplified correctly.The
onRatemethod signature has been properly updated to use the simplified event object, consistent with other components in the codebase. This change maintains the method's functionality while reducing component coupling.src/main/webapp/app/assessment/manage/rating/star-rating/star-rating.component.ts (3)
1-1: LGTM! Import updated for decoratorless API.Successfully added the
outputfunction import which is needed for the decoratorless API implementation.
78-81: LGTM! Output migrated to decoratorless API.The component has been properly updated to use the new
output()function instead of the traditional@Output()decorator withEventEmitter. This aligns with the PR objective to migrate to the Angular decoratorless API.
191-192: LGTM! Event emission simplified appropriately.The event payload has been correctly simplified to only include the essential data (
oldValueandnewValue), removing the redundant component instance reference. This reduces coupling and aligns with modern Angular practices.src/main/webapp/app/assessment/manage/assessment-complaint-alert/assessment-complaint-alert.component.spec.ts (1)
38-38: Good migration to the Angular reactive input APIThe test has been properly updated to use
fixture.componentRef.setInput('complaint', complaint)instead of direct property assignment. This approach aligns with Angular's reactive input API standards and ensures that proper input lifecycle hooks are triggered during testing.src/main/webapp/app/assessment/overview/complaints-for-students/complaints-student-view.component.html (3)
7-7: Correctly migrated property access to function callThe property access has been properly updated to use the function call syntax
exercise()for the reactive input API.
54-57: Properly updated input bindings to decoratorless APIAll input bindings have been correctly migrated to use the reactive function calls (
exercise(),result(),exam()) with appropriate null assertions where needed.
63-63: Replaced ViewChild reference with ID-based approachThe template reference variable has been correctly replaced with an ID attribute. This change supports the migration away from ViewChild decorators to a more reactive approach using element selection by ID.
src/main/webapp/app/assessment/manage/unreferenced-feedback-detail/assessment-correction-round-badge/assessment-correction-round-badge.component.html (1)
3-3: Successfully migrated to Angular reactive input APIThe conditional statements have been correctly updated to call
highlightDifferences()andfeedback()as functions rather than accessing them as properties. This aligns with Angular's reactive input pattern.Also applies to: 10-10
src/main/webapp/app/assessment/manage/assessment-complaint-alert/assessment-complaint-alert.component.html (1)
1-2: Well-executed migration to modern Angular syntaxThe template has been effectively updated to:
- Use the new
@ifsyntax instead of*ngIfas required per the coding guidelines- Call
complaint()as a function and assign its value to a local variable usingas complaint- Access properties directly on the local variable without optional chaining
This implementation follows modern Angular best practices and matches the decoratorless API approach being adopted throughout the codebase.
Also applies to: 5-5
src/main/webapp/app/assessment/manage/structured-grading-instructions-assessment-layout/structured-grading-instructions-assessment-layout.component.html (3)
1-1: Good use of new Angular control flow syntax!You've correctly migrated from
*ngIfto the new@ifcontrol flow syntax while also updatingcriteriato be accessed as a function call.
17-51: Good job implementing Angular's new control flow syntax!You've correctly updated:
- From
*ngForto@forwith the new track syntax- Using
$indexinstead ofindex- Accessing
criteria()as a function call- Nested
@forloop implementation is correctThese changes are aligned with Angular's new control flow syntax and the reactive API pattern.
11-16:❌ Incorrect review comment
Check if allowDrop should be accessed as a function
While you've updated
criteriato be accessed as a function call,allowDropis still being accessed as a property. IfallowDropis implemented as a reactive input or signal in the component, it should be accessed asallowDrop().
🏁 Script executed:
#!/bin/bash # Search for the definition of allowDrop in the component file rg -A 3 "allowDrop" src/main/webapp/app/assessment/manage/structured-grading-instructions-assessment-layout/structured-grading-instructions-assessment-layout.component.tsLength of output: 497
allowDrop is a plain boolean property, not a signal or function
The component declaresallowDrop: boolean; ngOnInit(): void { this.allowDrop = !this.readonly(); } collapseAll() { return this.allowDrop; }so accessing it as
allowDrop(notallowDrop()) is correct.Likely an incorrect or invalid review comment.
src/main/webapp/app/assessment/overview/complaint-request/complaint-request.component.html (3)
4-8: Good reactive API implementation!You've correctly updated the template to access the
complaintinput as a function callcomplaint()for all property accesses. This aligns with Angular's new reactive API pattern.
9-17: Well implemented control flow and reactive properties!The migration from
*ngIfdirectives to the new@ifsyntax is done correctly. You're also consistently accessing thecomplaint()input as a function throughout the conditional blocks.
19-27: Consistent reactive input pattern for form bindings!The form control bindings have been correctly updated to use function calls:
[maxLength]="maxComplaintTextLimit()"[(ngModel)]="complaint().complaintText"This maintains consistency with the reactive input pattern throughout the template.
src/main/webapp/app/assessment/manage/assessment-warning/assessment-warning.component.spec.ts (3)
21-28: Good test refactoring for reactive inputs!You've correctly updated the test to use
fixture.componentRef.setInput('exercise', ...)instead of direct property assignment. This matches the testing pattern needed for components using the reactive input API.
30-39: Consistent test input setting approach!The test case for checking behavior before exercise due date has been properly updated to use
fixture.componentRef.setInput('exercise', exercise)rather than direct property assignment.
63-65: Well-implemented test setup for multiple inputs!You've correctly updated both inputs:
fixture.componentRef.setInput('exercise', exercise)fixture.componentRef.setInput('submissions', [submission2, submission4, submission3, submission1])This ensures proper testing of components using the reactive input API pattern.
src/main/webapp/app/assessment/manage/assessment-instructions/expandable-section/expandable-section.component.html (3)
1-6: Great implementation of reactive patterns!You've successfully:
- Migrated from
*ngIfto the new@ifsyntax- Updated all input properties to be accessed as functions:
hasTranslation()headerKey()isSubHeader()- Maintained consistent reactive access pattern throughout the condition
This aligns perfectly with Angular's new control flow and reactive API.
7-12: Consistent reactive property access pattern!The second conditional block correctly implements:
- The new
@ifsyntax- Function call access for all input properties
- Properly maintains the pipe syntax with
headerKey() | artemisTranslateThis demonstrates a thorough understanding of Angular's reactive API.
13-18: Well-maintained pattern throughout the template!The third conditional section maintains consistency with the reactive pattern, accessing all input properties as functions. Great attention to detail in ensuring every input property is accessed properly.
src/main/webapp/app/assessment/manage/grading-system/grading-key/grading-key-table.component.spec.ts (2)
115-115: Correct adaptation to reactive input model.The test has been properly updated to invoke
studentGradeOrBonusPointsOrGradeBonus()as a method rather than accessing it as a property, aligning with the component's migration to Angular's decoratorless API.
193-193: Correct adaptation to reactive input model.Similar to the earlier change, this test assertion has been properly updated to invoke
studentGradeOrBonusPointsOrGradeBonus()as a method, ensuring consistency throughout the test file.src/main/webapp/app/assessment/manage/grading-system/grading-key/grading-key-table.component.html (1)
14-14: Good optimization using template variable.Using
@letto cache the result ofstudentGradeOrBonusPointsOrGradeBonus()is an excellent optimization. This reduces redundant method calls and improves template performance by storing the value once and reusing it throughout the template's conditional logic and highlighting.src/main/webapp/app/assessment/manage/structured-grading-instructions-assessment-layout/structured-grading-instructions-assessment-layout.component.spec.ts (6)
39-41: Correct implementation of input binding in tests.The change from direct property assignment to
fixture.componentRef.setInput()correctly follows Angular's recommended approach for setting inputs in component tests when using the decoratorless API.
45-46: Proper input binding for readonly property.Using
fixture.componentRef.setInput()here ensures the component's reactive input lifecycle hooks are properly triggered.
77-77: Consistent input binding approach for criteria.Continuing the pattern of using
setInput()for criteria maintains consistency with Angular's recommended testing approach.
80-81: Correct adaptation to view children as method.The test has been properly updated to access
expandableSections()as a method rather than a property, reflecting its migration from a@ViewChildrenQueryList to a reactiveviewChildren()function.
85-86: Consistent usage of expandableSections() as method.The change maintains consistency in accessing
expandableSections()as a method throughout the test.
89-90: Consistent usage of expandableSections() as method.The final usage of
expandableSections()is correctly updated to method invocation, ensuring consistency across the entire test file.src/main/webapp/app/assessment/manage/complaint-response/complaint-response.component.html (2)
1-2: Excellent safe binding pattern with reactive inputs.Using
@if (complaint(); as complaint)followed by an explicit undefined check creates a robust pattern that:
- Properly accesses the reactive input as a method
- Creates a local variable for convenient access
- Prevents null reference errors through careful safety checks
This approach follows Angular best practices for working with the decoratorless API.
20-20: Correct adaptation to reactive method call.The change to invoke
maxComplaintResponseTextLimit()as a method properly aligns with the component's migration to the decoratorless API.src/main/webapp/app/assessment/manage/assessment-instructions/expandable-section/expandable-section.component.spec.ts (2)
30-31: Good migration to Angular's recommended testing approach for reactive inputs.The changes correctly update test cases to use
fixture.componentRef.setInput()instead of direct property assignment when setting input values. This properly aligns with the component's migration to the decoratorless API and ensures that the input lifecycle hooks are correctly triggered during testing.Also applies to: 39-39, 51-51
35-35: Properly adapted assertion to match the new input pattern.The assertion has been correctly updated to reference the local
headerKeyvariable instead of accessing it through the component, which is consistent with the reactive input pattern used throughout the PR.src/main/webapp/app/assessment/manage/assessment-note/assessment-note.component.ts (1)
1-1: Good migration to the decoratorless API for outputs.The change from
@Output() with EventEmittertooutput()function correctly implements Angular's new reactive output API.Also applies to: 13-13
src/main/webapp/app/assessment/manage/assessment-complaint-alert/assessment-complaint-alert.component.ts (1)
1-1: Clean migration to the decoratorless API.The component has been correctly updated to use the new reactive
input()function instead of the@Input()decorator. Thereadonlymodifier is appropriate here to prevent the signal from being reassigned.Also applies to: 17-17
src/main/webapp/app/assessment/overview/complaint-form/complaints-form.component.html (2)
1-1: Well implemented migration to Angular's new control flow syntax.The template correctly uses the new
@ifsyntax instead of the older*ngIfdirective, which aligns with Angular's recommended approach and the specified coding guidelines.Also applies to: 6-6, 17-17, 24-24, 30-30
1-1: Properly updated property access for the reactive decoratorless API.All property accesses have been correctly updated to use method calls (with parentheses) to access the values from the reactive inputs, which is required when using the new input/output API.
Also applies to: 5-5, 6-6, 8-8, 17-17, 20-20, 24-24, 26-26, 30-30, 46-46
src/main/webapp/app/assessment/manage/complaints-for-tutor/complaints-for-tutor.component.html (10)
8-9: LGTM! Property access pattern updated correctly.The template now correctly uses function invocation for
complaint()and accesses its properties, which aligns with Angular's decoratorless API using signals.
11-12: Correctly updated to use function calls.Both
complaint()andzeroIndent()are now properly invoked as functions rather than accessed as properties, following Angular's new reactive input pattern.
16-18: Template expression properly updated.Good job converting the nested property access to use function invocation for
complaint().
41-41: Function invocation pattern correctly applied.The nested property access has been properly updated to use function invocation for
complaint().
44-44: Optional chaining pattern updated correctly.The template now correctly uses function invocation with optional chaining for
complaint()?.accepted.
47-48: Property access correctly updated.The conditional expression now properly uses function invocation for
complaint().
53-53: Optional chaining updated correctly.The template now properly uses function invocation with optional chaining for the negated condition.
64-64: Properly updated template binding.The
jhiTranslatebinding now correctly uses function invocation forcomplaint().
80-80: Condition correctly updated to use function invocation.The
@ifcondition now properly uses function invocation forcomplaint().
106-106: Complex condition properly updated.The nested conditions now correctly use function invocation for
complaint(), both for the.acceptedand.complaintTypeproperties.src/main/webapp/app/assessment/overview/complaint-request/complaint-request.component.ts (2)
1-1: Import correctly updated for decoratorless API.The
inputimport from Angular core has been correctly added to support the new decoratorless API.
17-18: Properly migrated to input.required API.The component has been successfully migrated from
@Input()decorators to the new decoratorless API usinginput.required<T>(), maintaining type safety and required input validation.src/main/webapp/app/assessment/manage/assessment-layout/assessment-layout.component.spec.ts (2)
47-53: Properly updated test input setup.Test inputs are now correctly set using
fixture.componentRef.setInput()instead of direct property assignment, which is the recommended pattern when using Angular's new decoratorless API.
80-80: Test correctly updated for new input pattern.The
complaintinput is now properly set usingfixture.componentRef.setInput(), ensuring that Angular's input lifecycle hooks are respected during testing.src/main/webapp/app/assessment/manage/assessment-instructions/collapsable-assessment-instructions/collapsable-assessment-instructions.component.html (4)
1-1: Correctly updated to use function invocation.The
@ifdirective now properly uses function invocation forcollapsed().
5-5: State update correctly migrated to use .set() pattern.The click handler now properly uses
collapsed.set()instead of direct property assignment, which is the correct pattern for updating model state in Angular's decoratorless API.
14-19: Input bindings correctly updated to use function invocation.All input bindings on the
jhi-assessment-instructionscomponent have been properly updated to use function invocation for reactive inputs:
[readOnly]="readOnly()"[exercise]="exercise()"[isAssessmentTraining]="isAssessmentTraining()"[showAssessmentInstructions]="showAssessmentInstructions()"
24-24: State update correctly migrated to use .set() pattern.The click handler in the collapsed state now properly uses
collapsed.set()instead of direct property assignment.src/main/webapp/app/assessment/manage/unreferenced-feedback-detail/unreferenced-feedback-detail.component.html (1)
1-104: Good job adopting the new @if syntaxThe template correctly uses the new Angular
@ifstructural directive instead of the older*ngIfsyntax, which follows the provided coding guidelines.src/main/webapp/app/assessment/overview/complaint-form/complaints-form.component.spec.ts (1)
44-47: Properly adopting the Angular reactive input testing patternGood job updating the test to use
fixture.componentRef.setInput()instead of direct property assignment, which correctly follows Angular's input lifecycle hooks when testing components with signals.src/main/webapp/app/assessment/shared/assessment-dashboard/assessment-dashboard-information.component.spec.ts (1)
22-23: Correctly implemented test setup for Angular signalsGreat work updating the test to use Angular's recommended
fixture.componentRef.setInput()method for setting input properties. This ensures proper initialization and change detection when working with signals.Also applies to: 39-41, 71-71
src/main/webapp/app/assessment/manage/complaints-for-tutor/complaints-for-tutor.component.spec.ts (1)
54-56: Well-implemented test setup for Angular signalsExcellent work updating the test to use Angular's recommended
fixture.componentRef.setInput()method for setting input properties. This approach correctly handles Angular's input lifecycle when testing components that use signals.Also applies to: 78-79, 115-116
src/main/webapp/app/assessment/manage/unreferenced-feedback-detail/unreferenced-feedback-detail.component.spec.ts (4)
35-36: Good implementation of Angular's new input API in testsThe test has been properly updated to use
fixture.componentRef.setInput()for setting component inputs instead of direct property assignment, which correctly follows Angular's input lifecycle hooks.
50-51: Consistent use of the new input APIProperly sets the feedback input using
setInput()method, maintaining consistent usage of Angular's new reactive API across tests.
64-69: Correctly implemented reactive input patternThe test has been updated to properly use
setInput()for passing the feedback object to the component, following Angular's recommended patterns for testing components with the new decoratorless API.
76-83: Well-structured feedback object with appropriate setInput usageThe feedback object is properly structured with all required properties and correctly passed to the component using the
setInput()method.src/main/webapp/app/assessment/overview/complaints-for-students/complaint-student-view.component.spec.ts (2)
105-108: Good migration to Angular's new input APIThe test initialization has been properly updated to use
fixture.componentRef.setInput()for setting component inputs, which is the recommended approach for the new decoratorless API.
376-381: Good use of object literal with setInputProperly uses an object literal with the spread operator and additional properties within the
setInput()method, which is a clean and efficient approach.src/main/webapp/app/assessment/manage/assessment-instructions/collapsable-assessment-instructions/collapsable-assessment-instructions.component.spec.ts (3)
26-27: Good initialization with setInputProperly initializes the component with the required exercise input using
setInput().
30-34: Well-structured input settingsThe test properly sets all the required inputs using
fixture.componentRef.setInput(), which is the correct approach with the new decoratorless API.
35-39: Correctly updated expectations for reactive inputsThe test expectations have been properly updated to call inputs as functions instead of accessing them as properties, which correctly reflects how inputs behave with the new decoratorless API.
src/main/webapp/app/assessment/manage/complaint-response/complaint-response.component.ts (1)
1-1: Correct import for new Angular APIThe import has been updated to use
inputfrom '@angular/core' instead of the traditionalInputdecorator, which is required for the decoratorless API.src/main/webapp/app/assessment/shared/assessment-dashboard/assessment-dashboard-information.component.html (1)
3-7: Correctly using the new @if syntax.The code properly implements Angular's new control flow syntax with
@if/@elsereplacing the older*ngIfdirective, which aligns with the provided coding guidelines.src/main/webapp/app/assessment/manage/grading-system/grading-key/grading-key-table.component.ts (4)
1-1: Correctly importing model from Angular core.The code properly imports the
modelfunction from @angular/core for the new decoratorless API.
36-37: Successful migration to Angular's reactive model API.The input properties have been correctly converted from
@Input()decorators to reactivemodel<T>()wrappers, maintaining the component's state management capabilities.
57-59: Proper initialization of reactive models in ngOnInit.The reactive models are correctly initialized using the
.set()method with fallback values provided from route parameters.
88-88: Conditional logic updated to use function call syntax.The conditional check has been properly updated to use the function call syntax
this.forBonus()instead of direct property access.src/main/webapp/app/assessment/manage/assessment-warning/assessment-warning.component.ts (3)
1-1: Correctly importing input from Angular core.The import statement has been properly updated to use
inputinstead ofInputfor the new decoratorless API.
34-35: Successful migration to input() API with proper type constraints.The input properties have been correctly converted to use the new reactive inputs API:
exerciseis properly marked as required usinginput.required<Exercise>()submissionshas a sensible default value withinput<Submission[]>([])
47-52: Proper use of function call syntax in lifecycle hooks.The
ngOnChangesmethod correctly uses function call syntax to access the reactive inputs, and stores the result in a local variable for better readability.src/main/webapp/app/assessment/manage/assessment-instructions/expandable-section/expandable-section.component.ts (3)
1-1: Correctly importing input from Angular core.The import statement has been properly updated to use
inputinstead ofInputfor the new decoratorless API.
16-18: Well-structured inputs with appropriate defaults.The input properties have been properly migrated to use the new reactive input API:
headerKeyis correctly marked as required withinput.required<string>()hasTranslationandisSubHeaderhave sensible default values
44-44: Getter updated to use function call syntax.The
storageKeygetter has been correctly updated to use the function call syntaxthis.headerKey()to access the reactive input.src/main/webapp/app/assessment/manage/assessment-header/assessment-header.component.spec.ts (4)
87-98: Good migration to setInput() for component inputsThe test setup correctly uses
fixture.componentRef.setInput()to set input values, which is the recommended approach for testing components with Angular's new signal-based inputs.
107-111: Properly updated test to use setInput() for complex objectsGood job setting the exercise input with appropriate typing
as Exercise. This maintains type safety while using the new input API.
117-119: Correctly handling test case setup with reactive inputsThe test case appropriately sets up component inputs using
setInput()before checking component behavior.
413-422: Good handling of object mutation in testsThe test correctly updates the nested object property and then calls
setInput()again to ensure the component receives the updated value. This is necessary with the reactive API since inputs are immutable.src/main/webapp/app/assessment/manage/assessment-instructions/assessment-instructions/assessment-instructions.component.html (3)
1-2: Great use of template variable with @let syntaxThe template effectively uses the modern
@letsyntax to store the result of thegradingCriteria()call, improving readability and performance by avoiding multiple function calls.
21-22: Good template variable usage for programmingParticipationCreating a local template variable for
programmingParticipation()follows best practices for readability and preventing repetitive function calls.
52-56: Properly updated conditional with function callThe
@ifdirective correctly uses the function call syntaxisAssessmentTraining()instead of property access, consistent with Angular's reactive input API.src/main/webapp/app/assessment/manage/unreferenced-feedback-detail/assessment-correction-round-badge/assessment-correction-round-badge.component.ts (1)
1-15: Correctly implemented signal-based inputsThe component has been successfully migrated to use Angular's reactive input API:
- Properly imports
inputfrom@angular/core- Uses
input.required<Feedback>()for mandatory inputs with type safety- Uses
input(false)to provide a default value for optional inputs- Declares inputs as readonly to maintain immutability
This implementation aligns perfectly with Angular's latest best practices.
src/main/webapp/app/assessment/manage/assessment-layout/assessment-layout.component.html (4)
1-1: Good use of @let for complaint variableUsing the
@letsyntax to create a local template variable forcomplaint()improves template readability and performance by avoiding repetitive function calls.
3-30: Properly updated input bindings to use function callsAll input bindings have been correctly updated to use function calls (e.g.,
[isLoading]="isLoading()") instead of property access. This is consistent with Angular's reactive input API.
34-34: Correctly accessing result with function callThe binding
[assessmentNote]="result()?.assessmentNote"properly uses the function call syntax to access the result input.
36-46: Well-structured complaint-related bindingsThe conditional rendering of the complaints form appropriately uses the local template variable and function calls for input bindings, making the template more readable and maintainable.
src/main/webapp/app/assessment/overview/complaint-form/complaints-form.component.ts (1)
25-30: Decorator-less inputs/outputs are correctly declared – nice!The migration to
input.*()/output()is syntactically correct, strongly-typed, and improves testability.
No follow-up required here.src/main/webapp/app/assessment/manage/assessment-instructions/collapsable-assessment-instructions/collapsable-assessment-instructions.component.ts (1)
17-21: Good use ofmodel()for two-waycollapsedstateThe conversion of
collapsedto a reactive model keeps external inputs intact while still allowing internal updates through.set().
No issues spotted.src/main/webapp/app/assessment/manage/assessment-header/assessment-header.component.ts (1)
35-40: Excellent signal typing and default valuesAll critical flags (
isLoading,saveBusy, …) are declared withinput.required<T>(), ensuring the template fails fast if forgotten.src/main/webapp/app/assessment/manage/assessment-header/assessment-header.component.html (1)
13-15: Verify thathasAssessmentDueDatePassedis a writable signal.
hasAssessmentDueDatePassed()is used for reading, while(close)="hasAssessmentDueDatePassed.set(false)"mutates it.
If the field is declared withinput()instead ofmodel(), the.set()call will raise a type error at compile-time.Please ensure it is declared via
model<boolean>()(or another writable-signal factory) in the component class.src/main/webapp/app/assessment/manage/assessment-layout/assessment-layout.component.ts (4)
1-1: Imports updated correctly for Angular decoratorless API.The import statement now includes
inputandoutputfrom '@angular/core', which are required for using Angular's new reactive input/output API.
29-49: Input properties successfully migrated to decoratorless API.All input properties have been correctly converted to use Angular's new reactive input API with
input()andinput.required()functions, maintaining the same functionality while enabling Angular's new reactive paradigm.
62-67: Property access correctly updated for reactive API.The access to
resultproperty has been correctly updated to use function call syntaxthis.result()to retrieve the value from the reactive input.
69-77: Output properties successfully migrated to decoratorless API.All output properties have been correctly converted to use Angular's new reactive output API with
output()functions, maintaining the same event emission functionality.src/main/webapp/app/assessment/overview/complaints-for-students/complaints-student-view.component.ts (5)
1-1: Imports updated correctly for Angular decoratorless API.The import statement now includes
inputandRenderer2from '@angular/core', which are required for using Angular's new reactive input API and safer DOM manipulation.
36-36: Added Renderer2 for safer DOM manipulation.Using the
Renderer2service instead of direct DOM access throughViewChildis a better practice for DOM manipulation in Angular, providing better abstraction and TestBed compatibility.
38-43: Input properties successfully migrated to decoratorless API.All input properties have been correctly converted to use Angular's new reactive input API with
input()andinput.required()functions, maintaining the same functionality.
64-95: Property access correctly updated for reactive API.The property access has been properly updated to use function call syntax (e.g.,
this.exercise(),this.participation()) to retrieve values from reactive inputs.
173-173: Improved DOM access using Renderer2.The implementation now uses
this.renderer.selectRootElementinstead of direct DOM access via ViewChild, which is safer and more aligned with Angular best practices for DOM manipulation.src/main/webapp/app/assessment/shared/assessment-dashboard/assessment-dashboard-information.component.ts (4)
1-1: Imports updated correctly for Angular decoratorless API.The import statement now includes
inputfrom '@angular/core', which is required for using Angular's new reactive input API.
45-64: Input properties successfully migrated to decoratorless API.All input properties have been correctly converted to use Angular's new reactive input API with
input()andinput.required()functions, maintaining the same functionality while enabling Angular's new reactive paradigm.
113-119: Property access correctly updated for reactive API.The property access in the
setupLinksmethod has been properly updated to use function call syntax (e.g.,this.isExamMode(),this.course().id) to retrieve values from reactive inputs.
125-134: Property access correctly updated in mathematical expressions.The property access within the graph data setup has been correctly updated to use function call syntax when accessing reactive inputs in mathematical expressions, ensuring proper reactivity.
src/main/webapp/app/assessment/manage/structured-grading-instructions-assessment-layout/structured-grading-instructions-assessment-layout.component.ts (5)
3-3: Imports updated correctly for Angular decoratorless API.The import statement now includes
inputandviewChildrenfrom '@angular/core', which are required for using Angular's new reactive input and view children API.
19-20: Input properties successfully migrated to decoratorless API.The input properties have been correctly converted to use Angular's new reactive input API with
input.required()andinput()functions.
27-27: ViewChildren successfully migrated to decoratorless API.The traditional ViewChildren decorator has been properly replaced with the new
viewChildren()function, maintaining the same functionality while enabling Angular's new reactive paradigm.
33-33: Property access correctly updated for reactive API.The property access in the
ngOnInitmethod has been properly updated to use function call syntax (this.readonly()) to retrieve the value from the reactive input.
37-50: Property access correctly updated for view children API.The access to expandable sections has been correctly updated to use function call syntax (
this.expandableSections()) to retrieve the query list from the reactive view children.src/main/webapp/app/assessment/manage/unreferenced-feedback-detail/unreferenced-feedback-detail.component.ts (1)
1-1: Import is correct – watch for upcoming API rename
input,outputand especiallymodelare still marked experimental in Angular 17. Keep an eye on the Angular 18 migration guide, becausemodelis very likely to be renamed tomodelSignal(RFC-16) and the current import will become deprecated.Please double-check the next Angular pre-release notes to avoid a breaking change during the next upgrade.
...p/assessment/manage/unreferenced-feedback-detail/unreferenced-feedback-detail.component.html
Show resolved
Hide resolved
...p/assessment/manage/unreferenced-feedback-detail/unreferenced-feedback-detail.component.html
Show resolved
Hide resolved
...ain/webapp/app/assessment/manage/complaints-for-tutor/complaints-for-tutor.component.spec.ts
Show resolved
Hide resolved
src/main/webapp/app/assessment/manage/complaint-response/complaint-response.component.ts
Show resolved
Hide resolved
...p/app/assessment/shared/assessment-dashboard/assessment-dashboard-information.component.html
Show resolved
Hide resolved
...p/app/assessment/shared/assessment-dashboard/assessment-dashboard-information.component.html
Show resolved
Hide resolved
.../manage/assessment-instructions/assessment-instructions/assessment-instructions.component.ts
Show resolved
Hide resolved
src/main/webapp/app/assessment/manage/complaints-for-tutor/complaints-for-tutor.component.ts
Show resolved
Hide resolved
...app/assessment/manage/unreferenced-feedback-detail/unreferenced-feedback-detail.component.ts
Show resolved
Hide resolved
...app/assessment/manage/unreferenced-feedback-detail/unreferenced-feedback-detail.component.ts
Show resolved
Hide resolved
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||
|
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. |
Checklist
General
Client
Motivation and Context
Description
With this PR, the majority of the components are migrated to use signals instead of @input, @output, @ViewChild, etc.
Steps for Testing
Note
The changes in this PR affect a lot of different components relevant for assessment. The provided testing steps only cover the main assessment functionality, but there could be workflows not covered by them. Please also think about other possible testing steps yourself when testing.
Exercise Assessment (Text / Modeling / Programming / File Upload)
Preconditions:
Steps:
✅ Expected:
✅ Expected: Complaint UI is functional and response can be saved successfully.
Structured Grading Instructions (SGIs)
Steps:
✅ Expected: Applied SGIs reflect in the feedback list and affect the score correctly.
Assessment Statistics and Feedback Requests
Steps:
✅ Expected: All values and links render correctly and navigate to valid submissions.
Grading Key
✅ Expected: Creation, customization, and saving of grading keys is possible. Also, deleting it should still be possible.
Exam Mode Testing
Preconditions:
Steps:
✅ Expected: Assessment UI behaves identically to course mode. No layout bugs or interaction regressions. Complaints should also work as expected.
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
Exam Mode Test
Test Coverage
Unchanged
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Tests
Style
Chores