Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exam mode: Migrate manage and shared modules to signals #10436

Open
wants to merge 26 commits into
base: develop
Choose a base branch
from

Conversation

coolchock
Copy link
Contributor

@coolchock coolchock commented Mar 3, 2025

Checklist

General

Client

Description

This pull request is a key step in our Angular 19 migration. It focuses on migrating all components within the manage package of the exam mode to utilize signal inputs, signal outputs, and signal queries.

Steps for Testing

Prerequisites:

  • 1 Instructor
  • 1 Student

Please verify that everything works as expected in Exam Management. In particular, check that it is possible to:

  1. Create a normal exam, a test exam, and a test run.
  2. Register students for the exam.
  3. Generate individual student exams.
  4. Edit the overall working time and individual working times.
  5. Create exam announcements.
  6. Open individual student exams.

Additionally, please verify that the exam checklist and exam status bar function 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

Performance Review

  • I (as a reviewer) confirm that the client changes (in particular related to REST calls and UI responsiveness) are implemented with a very good performance even for very large courses with more than 2000 students.
  • I (as a reviewer) confirm that the server changes (in particular related to database calls) are implemented with a very good performance even for very large courses with more than 2000 students.

Code Review

  • Code Review 1
  • Code Review 2

Manual Tests

  • Test 1
  • Test 2

Summary by CodeRabbit

  • Refactor

    • Enhanced exam management and student exam interfaces for a more dynamic and reliable display of exam details. These improvements ensure that exam information—such as timelines, checklists, and event notifications—stays current and interactive.
    • Transitioned from direct property access to function calls for various components, improving data retrieval and management.
  • Tests

    • Updated testing practices to reflect the new reactive data handling approach, thereby improving overall stability and quality assurance.
    • Adjusted test setups to utilize the input function for property assignments, enhancing dependency injection and state management in tests.

@github-actions github-actions bot added tests client Pull requests that update TypeScript code. (Added Automatically!) labels Mar 3, 2025
@coolchock coolchock changed the title Exam mode: Migrate manage module to signals Exam mode: Migrate manage and shared modules to signals Mar 4, 2025
@helios-aet helios-aet bot temporarily deployed to artemis-test2.artemis.cit.tum.de March 8, 2025 22:21 Inactive
…ration

# Conflicts:
#	src/main/webapp/app/exam/manage/students/upload-images/students-upload-images-button.component.ts
#	src/main/webapp/app/exam/shared/events/exam-live-event.component.html
@coolchock coolchock marked this pull request as ready for review March 9, 2025 19:30
@coolchock coolchock requested a review from a team as a code owner March 9, 2025 19:30
Copy link

coderabbitai bot commented Mar 9, 2025

"""

Walkthrough

This PR refactors multiple Angular components, templates, and tests to replace direct property access with function calls. HTML templates now invoke functions (e.g., exam() instead of exam?.) for conditional checks and property bindings. In TypeScript files, decorators such as @Input(), @Output(), and @ViewChild() are replaced by function‐based declarations (e.g., input.required(), output(), viewChild.required()). Test setups have also been updated to use Angular’s injection context with the input function. Overall, the refactoring ensures a consistent reactive access pattern across the codebase without modifying the underlying business logic.

Changes

File(s) Change Summary
HTML Templates
src/main/webapp/app/exam/manage/exam-status.component.html
src/main/webapp/app/exam/manage/exams/exam-checklist-component/...*.html
src/main/webapp/app/exam/manage/student-exams/...*.html
src/main/webapp/app/exam/shared/events/exam-live-event.component.html
src/main/webapp/app/exam/shared/working-time-change/...control.component.html
Replaced direct property access (e.g., exam?.property, variable) with function call syntax (e.g., exam().property, variable()) in conditionals and data bindings.
Component TypeScript Files
src/main/webapp/app/exam/manage/exam-status.component.ts
src/main/webapp/app/exam/manage/exams/exam-checklist-component/...*.ts
src/main/webapp/app/exam/manage/exams/exam-edit-workingtime-dialog/...*.ts
src/main/webapp/app/exam/manage/exams/exam-mode-picker/exam-mode-picker.component.ts
src/main/webapp/app/exam/manage/exams/exam-update.component.ts
src/main/webapp/app/exam/shared/...*(exam-live-event, student-exam-working-time, test-exam-working-time, working-time-change, working-time-control).ts
Updated property declarations by replacing Angular decorators with function-based patterns (using input.required(), output(), and viewChild.required()), and changed property accesses (e.g., this.exam) to function calls (e.g., this.exam()) for consistent reactivity.
Test Files
src/test/javascript/spec/component/exam/... (multiple test suites)
Modified test setups to use TestBed.runInInjectionContext() and wrapped property assignments with the input() function, ensuring that component inputs are initialized reactively during testing.

Sequence Diagram(s)

Possibly related PRs

Suggested labels

small, ready to merge
"""

✨ Finishing Touches
  • 📝 Generate Docstrings

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai or @coderabbitai title anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🔭 Outside diff range comments (2)
src/main/webapp/app/exam/manage/exams/exam-checklist-component/exam-edit-workingtime-dialog/exam-edit-working-time-dialog.component.ts (2)

37-39: ⚠️ Potential issue

Missing function call syntax for exam property

The oldWorkingTime getter uses this.exam directly instead of this.exam(), which would cause errors when using signals.

Apply this change to fix the property access:

    get oldWorkingTime() {
-        return examWorkingTime(this.exam);
+        return examWorkingTime(this.exam());
    }

52-57: ⚠️ Potential issue

Missing function call syntax for exam property in API call

The exam properties are accessed directly instead of using function call syntax required by signals.

Apply this change:

-        this.examManagementService.updateWorkingTime(this.exam.course!.id!, this.exam.id!, this.workingTimeSeconds).subscribe({
+        this.examManagementService.updateWorkingTime(this.exam().course!.id!, this.exam().id!, this.workingTimeSeconds).subscribe({
            next: (res: HttpResponse<Exam>) => {
                this.isLoading = false;
                if (res.body) {
                    this.examChange.emit(res.body);
                }
                this.clear();
            },
🧹 Nitpick comments (5)
src/main/webapp/app/exam/manage/exams/exam-detail.component.ts (1)

76-76: Method binding ensures correct context

The addition of binding getExamRoutesByIdentifier to this ensures that the method maintains the correct context when invoked in other components or templates, which is essential for the Angular 19 signals migration. This helps prevent potential this context loss when the method is passed as a reference.

While binding in ngOnInit works perfectly, consider refactoring the method declaration to use an arrow function which automatically preserves the this context:

-    getExamRoutesByIdentifier(identifier: string) {
+    getExamRoutesByIdentifier = (identifier: string) => {
        return ['/course-management', this.exam.course?.id, 'exams', this.exam.id, identifier];
-    }
+    }

This approach would eliminate the need for explicit binding and align with modern TypeScript practices.

src/main/webapp/app/exam/manage/exam-status.component.ts (2)

257-264: Consider using optional chaining for nested conditional

The conditional check on line 261 could benefit from using optional chaining to make the code more readable.

-            ((!exam.examStudentReviewStart && exam.examStudentReviewEnd && exam.examStudentReviewEnd.isAfter(dayjs())) ||
-                (exam.examStudentReviewStart && exam.examStudentReviewStart.isBefore(dayjs()) && exam.examStudentReviewEnd!.isAfter(dayjs()))) ??
+            ((!exam.examStudentReviewStart && exam.examStudentReviewEnd?.isAfter(dayjs())) ||
+                (exam.examStudentReviewStart?.isBefore(dayjs()) && exam.examStudentReviewEnd?.isAfter(dayjs()))) ??
🧰 Tools
🪛 Biome (1.9.4)

[error] 261-261: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)


269-272: Consider using optional chaining for date check

The property check on line 271 could benefit from using optional chaining to make the code more readable.

-        return (exam.examStudentReviewStart && exam.examStudentReviewStart.isAfter(dayjs())) ?? false;
+        return exam.examStudentReviewStart?.isAfter(dayjs()) ?? false;
🧰 Tools
🪛 Biome (1.9.4)

[error] 271-271: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

src/main/webapp/app/exam/manage/student-exams/student-exam-detail-table-row/student-exam-detail-table-row.component.ts (1)

55-57: Use optional chaining to improve code safety

The static analysis tool correctly identified that this could be improved with optional chaining.

-        if (this.course() && this.course().id) {
+        if (this.course()?.id) {
             this.courseId = this.course().id!;
         }
🧰 Tools
🪛 Biome (1.9.4)

[error] 55-55: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

src/main/webapp/app/exam/manage/exams/exam-checklist-component/exam-checklist-exercisegroup-table/exam-checklist-exercisegroup-table.component.ts (1)

33-33: Function call with null assertion operator.

The code correctly uses this.exerciseGroups()! to access the array, but consider removing the non-null assertion operator (!) since you've already checked that this.exerciseGroups() is truthy on line 31.

-            this.exerciseGroups()!.forEach((exerciseGroup) => {
+            this.exerciseGroups().forEach((exerciseGroup) => {
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 06f2da4 and 42dc259.

📒 Files selected for processing (47)
  • src/main/webapp/app/exam/manage/exam-status.component.html (6 hunks)
  • src/main/webapp/app/exam/manage/exam-status.component.ts (11 hunks)
  • src/main/webapp/app/exam/manage/exams/exam-checklist-component/exam-announcement-dialog/exam-live-announcement-create-button.component.ts (4 hunks)
  • src/main/webapp/app/exam/manage/exams/exam-checklist-component/exam-checklist-exercisegroup-table/exam-checklist-exercisegroup-table.component.html (2 hunks)
  • src/main/webapp/app/exam/manage/exams/exam-checklist-component/exam-checklist-exercisegroup-table/exam-checklist-exercisegroup-table.component.ts (3 hunks)
  • src/main/webapp/app/exam/manage/exams/exam-checklist-component/exam-checklist.component.html (21 hunks)
  • src/main/webapp/app/exam/manage/exams/exam-checklist-component/exam-checklist.component.ts (7 hunks)
  • src/main/webapp/app/exam/manage/exams/exam-checklist-component/exam-edit-workingtime-dialog/exam-edit-working-time-dialog.component.ts (2 hunks)
  • src/main/webapp/app/exam/manage/exams/exam-checklist-component/exam-edit-workingtime-dialog/exam-edit-working-time.component.ts (4 hunks)
  • src/main/webapp/app/exam/manage/exams/exam-detail.component.ts (1 hunks)
  • src/main/webapp/app/exam/manage/exams/exam-mode-picker/exam-mode-picker.component.html (1 hunks)
  • src/main/webapp/app/exam/manage/exams/exam-mode-picker/exam-mode-picker.component.ts (2 hunks)
  • src/main/webapp/app/exam/manage/exams/exam-update.component.ts (5 hunks)
  • src/main/webapp/app/exam/manage/student-exams/student-exam-detail-table-row/student-exam-detail-table-row.component.html (2 hunks)
  • src/main/webapp/app/exam/manage/student-exams/student-exam-detail-table-row/student-exam-detail-table-row.component.ts (5 hunks)
  • src/main/webapp/app/exam/manage/student-exams/student-exam-status/student-exam-status.component.html (1 hunks)
  • src/main/webapp/app/exam/manage/student-exams/student-exam-status/student-exam-status.component.ts (2 hunks)
  • src/main/webapp/app/exam/manage/student-exams/student-exam-timeline/programming-exam-diff/programming-exercise-exam-diff.component.html (1 hunks)
  • src/main/webapp/app/exam/manage/student-exams/student-exam-timeline/programming-exam-diff/programming-exercise-exam-diff.component.ts (6 hunks)
  • src/main/webapp/app/exam/manage/student-exams/student-exam-timeline/student-exam-timeline.component.ts (8 hunks)
  • src/main/webapp/app/exam/manage/students/exam-students.component.ts (3 hunks)
  • src/main/webapp/app/exam/manage/students/upload-images/students-upload-images-button.component.ts (4 hunks)
  • src/main/webapp/app/exam/manage/students/upload-images/students-upload-images-dialog.component.ts (3 hunks)
  • src/main/webapp/app/exam/shared/events/exam-live-event.component.html (3 hunks)
  • src/main/webapp/app/exam/shared/events/exam-live-event.component.ts (3 hunks)
  • src/main/webapp/app/exam/shared/student-exam-working-time/student-exam-working-time.component.html (1 hunks)
  • src/main/webapp/app/exam/shared/student-exam-working-time/student-exam-working-time.component.ts (2 hunks)
  • src/main/webapp/app/exam/shared/testExam-workingTime/test-exam-working-time.component.ts (3 hunks)
  • src/main/webapp/app/exam/shared/working-time-change/working-time-change.component.html (1 hunks)
  • src/main/webapp/app/exam/shared/working-time-change/working-time-change.component.ts (2 hunks)
  • src/main/webapp/app/exam/shared/working-time-control/working-time-control.component.html (2 hunks)
  • src/main/webapp/app/exam/shared/working-time-control/working-time-control.component.ts (5 hunks)
  • src/test/javascript/spec/component/exam/exam-update.component.spec.ts (3 hunks)
  • src/test/javascript/spec/component/exam/manage/exam-mode-picker.component.spec.ts (2 hunks)
  • src/test/javascript/spec/component/exam/manage/exams/exam-announcement-dialog/exam-live-announcement-create-button.component.spec.ts (2 hunks)
  • src/test/javascript/spec/component/exam/manage/exams/exam-checklist-exercisegroup-table.component.spec.ts (4 hunks)
  • src/test/javascript/spec/component/exam/manage/exams/exam-checklist.component.spec.ts (3 hunks)
  • src/test/javascript/spec/component/exam/manage/programming-exam-diff.component.spec.ts (5 hunks)
  • src/test/javascript/spec/component/exam/manage/student-exams/exam-status.component.spec.ts (7 hunks)
  • src/test/javascript/spec/component/exam/manage/student-exams/student-exam-detail-table-row.component.spec.ts (2 hunks)
  • src/test/javascript/spec/component/exam/manage/student-exams/student-exam-timeline.component.spec.ts (5 hunks)
  • src/test/javascript/spec/component/exam/manage/student-exams/students-upload-images-button.component.spec.ts (2 hunks)
  • src/test/javascript/spec/component/exam/manage/student-exams/students-upload-images-dialog.component.spec.ts (2 hunks)
  • src/test/javascript/spec/component/exam/shared/events/exam-live-event.component.spec.ts (6 hunks)
  • src/test/javascript/spec/component/exam/shared/student-exam-working-time.component.spec.ts (3 hunks)
  • src/test/javascript/spec/component/exam/shared/testexam-working-time.component.spec.ts (2 hunks)
  • src/test/javascript/spec/component/exam/shared/working-time-control.component.spec.ts (7 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
`src/main/webapp/**/*.html`: @if and @for are new and valid ...

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/manage/student-exams/student-exam-status/student-exam-status.component.html
  • src/main/webapp/app/exam/shared/working-time-change/working-time-change.component.html
  • src/main/webapp/app/exam/manage/student-exams/student-exam-detail-table-row/student-exam-detail-table-row.component.html
  • src/main/webapp/app/exam/manage/exams/exam-checklist-component/exam-checklist-exercisegroup-table/exam-checklist-exercisegroup-table.component.html
  • src/main/webapp/app/exam/shared/events/exam-live-event.component.html
  • src/main/webapp/app/exam/manage/exam-status.component.html
  • src/main/webapp/app/exam/shared/working-time-control/working-time-control.component.html
  • src/main/webapp/app/exam/manage/exams/exam-mode-picker/exam-mode-picker.component.html
  • src/main/webapp/app/exam/manage/exams/exam-checklist-component/exam-checklist.component.html
  • src/main/webapp/app/exam/shared/student-exam-working-time/student-exam-working-time.component.html
  • src/main/webapp/app/exam/manage/student-exams/student-exam-timeline/programming-exam-diff/programming-exercise-exam-diff.component.html
`src/test/javascript/spec/**/*.ts`: jest: true; mock: NgMock...

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/manage/student-exams/students-upload-images-dialog.component.spec.ts
  • src/test/javascript/spec/component/exam/manage/exam-mode-picker.component.spec.ts
  • src/test/javascript/spec/component/exam/shared/student-exam-working-time.component.spec.ts
  • src/test/javascript/spec/component/exam/manage/exams/exam-announcement-dialog/exam-live-announcement-create-button.component.spec.ts
  • src/test/javascript/spec/component/exam/manage/student-exams/exam-status.component.spec.ts
  • src/test/javascript/spec/component/exam/manage/exams/exam-checklist.component.spec.ts
  • src/test/javascript/spec/component/exam/manage/exams/exam-checklist-exercisegroup-table.component.spec.ts
  • src/test/javascript/spec/component/exam/manage/programming-exam-diff.component.spec.ts
  • src/test/javascript/spec/component/exam/exam-update.component.spec.ts
  • src/test/javascript/spec/component/exam/shared/working-time-control.component.spec.ts
  • src/test/javascript/spec/component/exam/shared/testexam-working-time.component.spec.ts
  • src/test/javascript/spec/component/exam/manage/student-exams/student-exam-detail-table-row.component.spec.ts
  • src/test/javascript/spec/component/exam/manage/student-exams/student-exam-timeline.component.spec.ts
  • src/test/javascript/spec/component/exam/manage/student-exams/students-upload-images-button.component.spec.ts
  • src/test/javascript/spec/component/exam/shared/events/exam-live-event.component.spec.ts
`src/main/webapp/**/*.ts`: angular_style:https://angular.io/...

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:PascalCase;funcs:camelCase;props:camelCase;no_priv_prefix:true;strings:single_quotes;localize:true;btns:functionality;links:navigation;icons_text:newline;labels:associate;code_style:arrow_funcs,curly_braces,open_braces_same_line,indent_4;memory_leak_prevention:true;routes:naming_schema;chart_framework:ngx-charts;responsive_layout:true

  • src/main/webapp/app/exam/manage/exams/exam-detail.component.ts
  • src/main/webapp/app/exam/manage/student-exams/student-exam-status/student-exam-status.component.ts
  • src/main/webapp/app/exam/manage/students/exam-students.component.ts
  • src/main/webapp/app/exam/manage/exams/exam-checklist-component/exam-announcement-dialog/exam-live-announcement-create-button.component.ts
  • src/main/webapp/app/exam/shared/working-time-change/working-time-change.component.ts
  • src/main/webapp/app/exam/manage/exams/exam-checklist-component/exam-edit-workingtime-dialog/exam-edit-working-time.component.ts
  • src/main/webapp/app/exam/shared/student-exam-working-time/student-exam-working-time.component.ts
  • src/main/webapp/app/exam/manage/exams/exam-checklist-component/exam-edit-workingtime-dialog/exam-edit-working-time-dialog.component.ts
  • src/main/webapp/app/exam/manage/exams/exam-checklist-component/exam-checklist.component.ts
  • src/main/webapp/app/exam/manage/exams/exam-checklist-component/exam-checklist-exercisegroup-table/exam-checklist-exercisegroup-table.component.ts
  • src/main/webapp/app/exam/manage/exam-status.component.ts
  • src/main/webapp/app/exam/manage/exams/exam-mode-picker/exam-mode-picker.component.ts
  • src/main/webapp/app/exam/shared/testExam-workingTime/test-exam-working-time.component.ts
  • src/main/webapp/app/exam/manage/students/upload-images/students-upload-images-dialog.component.ts
  • src/main/webapp/app/exam/manage/exams/exam-update.component.ts
  • src/main/webapp/app/exam/shared/events/exam-live-event.component.ts
  • src/main/webapp/app/exam/shared/working-time-control/working-time-control.component.ts
  • src/main/webapp/app/exam/manage/students/upload-images/students-upload-images-button.component.ts
  • src/main/webapp/app/exam/manage/student-exams/student-exam-timeline/student-exam-timeline.component.ts
  • src/main/webapp/app/exam/manage/student-exams/student-exam-detail-table-row/student-exam-detail-table-row.component.ts
  • src/main/webapp/app/exam/manage/student-exams/student-exam-timeline/programming-exam-diff/programming-exercise-exam-diff.component.ts
🧠 Learnings (2)
src/test/javascript/spec/component/exam/exam-update.component.spec.ts (1)
Learnt from: pzdr7
PR: ls1intum/Artemis#9505
File: src/test/javascript/spec/integration/code-editor/code-editor-container.integration.spec.ts:179-181
Timestamp: 2024-11-12T12:51:58.049Z
Learning: In `src/test/javascript/spec/integration/code-editor/code-editor-container.integration.spec.ts`, `ResizeObserver` is mocked within the `beforeEach` block.
src/main/webapp/app/exam/manage/student-exams/student-exam-timeline/programming-exam-diff/programming-exercise-exam-diff.component.ts (1)
Learnt from: pzdr7
PR: ls1intum/Artemis#9443
File: src/main/webapp/app/exam/manage/student-exams/student-exam-timeline/programming-exam-diff/programming-exercise-exam-diff.component.ts:126-129
Timestamp: 2024-11-12T12:51:40.391Z
Learning: In Angular applications using `NgbModal`, wrapping data with `signal()` when passing inputs to modal components (e.g., `GitDiffReportModalComponent`) is necessary until `NgbModal` supports signal inputs.
🪛 Biome (1.9.4)
src/main/webapp/app/exam/manage/exam-status.component.ts

[error] 261-261: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)


[error] 271-271: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

src/main/webapp/app/exam/manage/student-exams/student-exam-detail-table-row/student-exam-detail-table-row.component.ts

[error] 55-55: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

⏰ Context from checks skipped due to timeout of 90000ms (7)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: Call Build Workflow / Build .war artifact
  • GitHub Check: Call Build Workflow / Build and Push Docker Image
  • GitHub Check: client-tests-selected
  • GitHub Check: client-tests
  • GitHub Check: server-tests
  • GitHub Check: Analyse
🔇 Additional comments (239)
src/main/webapp/app/exam/manage/student-exams/student-exam-status/student-exam-status.component.html (1)

2-2: LGTM: Successfully migrated to function calls in conditionals

The template has been updated to call isTestExam() and hasStudentsWithoutExam() as functions rather than accessing them as properties, aligning with the Angular signal-based approach. The template also correctly uses the new @if/@else syntax as required by our coding guidelines.

Also applies to: 8-8

src/main/webapp/app/exam/shared/student-exam-working-time/student-exam-working-time.component.html (1)

2-2: LGTM: Successfully migrated to signal-based access

The template has been updated to call studentExam() as a function before accessing the workingTime property, aligning with the Angular signal-based approach for reactive property access.

src/test/javascript/spec/component/exam/shared/events/exam-live-event.component.spec.ts (2)

13-13: LGTM: Added input import

Added the input function import from '@angular/core', which is required for the Angular signal-based testing approach.


33-36: LGTM: Successfully migrated tests to use Angular's injection context

The test setup has been updated to use TestBed.runInInjectionContext() with the input() function to properly set component inputs within Angular's dependency injection system. This change is consistent across all test cases and follows Angular 19's approach for handling component inputs.

Also applies to: 49-52, 65-68, 83-86, 107-110, 126-129, 145-148

src/test/javascript/spec/component/exam/shared/student-exam-working-time.component.spec.ts (2)

6-6: LGTM: Added input import

Added the input function import from '@angular/core', which is required for the Angular signal-based testing approach.


36-38: LGTM: Successfully migrated tests to use Angular's injection context

The test setup has been updated to use TestBed.runInInjectionContext() with the input() function to properly set component inputs within Angular's dependency injection system. This change is consistent with Angular 19's approach for handling component inputs.

The updated code correctly uses Boolean assertion methods (toBeTrue()/toBeFalse()) as per the coding guidelines for Jest expectations.

Also applies to: 63-65

src/main/webapp/app/exam/manage/exams/exam-mode-picker/exam-mode-picker.component.html (3)

2-2: Property access consistently replaced with function call.

The use of disableInput() as a function call instead of direct property access aligns with the Angular component input migration pattern.


6-8: Property access successfully migrated to function calls.

The template now correctly accesses the exam property through the function call syntax exam() instead of direct property access, maintaining reactivity.


15-17: Button state bindings correctly migrated.

The class bindings now properly use function calls for both exam() and disableInput() to determine button states, which follows the Angular signal-based pattern.

src/main/webapp/app/exam/shared/working-time-change/working-time-change.component.html (2)

4-4: Old working time access correctly migrated.

The template now accesses the old working time through oldWorkingTime() function call instead of direct property access, aligning with the Angular signal-based pattern.


8-8: New working time access correctly migrated.

The template now accesses the new working time through newWorkingTime() function call instead of direct property access, aligning with the Angular signal-based pattern.

src/test/javascript/spec/component/exam/manage/student-exams/students-upload-images-dialog.component.spec.ts (2)

19-19: Angular input function properly imported.

The import of the input function from @angular/core is correctly added to support the new Angular input API.


58-61: Tests successfully updated to use Angular's input API.

The test setup has been properly migrated to use TestBed.runInInjectionContext() with the input() function, replacing direct property assignment. This follows Angular's new approach for handling component inputs in tests.

src/main/webapp/app/exam/manage/exams/exam-checklist-component/exam-checklist-exercisegroup-table/exam-checklist-exercisegroup-table.component.html (3)

25-25: Successfully migrated to new Angular control flow syntax.

The template correctly uses the new @if control flow syntax instead of the deprecated *ngIf, while also properly accessing quizExamMaxPoints as a function call. This aligns with both aspects of the migration.


38-38: Points display correctly migrated to function call.

The quiz exam max points are now correctly accessed through the function call quizExamMaxPoints() instead of direct property access, maintaining reactivity.


42-106: Modern Angular syntax properly implemented.

The template successfully implements the new Angular control flow syntax with @for and nested @if/@else blocks, replacing the deprecated *ngFor and *ngIf directives. This follows best practices and the provided coding guidelines.

src/test/javascript/spec/component/exam/shared/working-time-control.component.spec.ts (7)

7-7: Correct import of Angular's input signal

The addition of the input import from @angular/core is appropriate for migrating to Angular signals.


50-54: Properly migrated test setup to use Angular signals

The test now correctly uses TestBed.runInInjectionContext to set up the exam input using Angular's signal-based input function.


72-77: Correctly configured multiple input signals

The test properly sets up both exam and relative inputs using TestBed.runInInjectionContext.


86-92: Proper setup of multiple input signals for relative working time test

The test correctly configures both exam and relative input signals.


101-107: Successfully migrated disabled property to use input signal

The test correctly sets up all three input signals (exam, relative, disabled) using the new Angular signals approach.


119-123: Proper test setup for working time changes test

The exam input is correctly configured for testing interactions with working time duration changes.


150-154: Correctly migrated percent difference test setup

The exam input is properly configured using Angular signals, maintaining the test's functionality.

src/test/javascript/spec/component/exam/manage/student-exams/students-upload-images-button.component.spec.ts (2)

10-10: Appropriate import of Angular input signal

The import of the input function from @angular/core is correctly added for signal-based inputs.


40-44: Successfully migrated component inputs to signals

The test now properly uses TestBed.runInInjectionContext to initialize the exam and courseId inputs using Angular signals, which aligns with the signal migration objective.

src/test/javascript/spec/component/exam/manage/exam-mode-picker.component.spec.ts (6)

7-7: Correct import of Angular input signal

The import of input from @angular/core is appropriate for the migration to Angular signals.


25-29: Properly migrated component initialization to use signals

The test correctly uses TestBed.runInInjectionContext to set up the exam and disableInput properties as signals.


34-37: Correct implementation of dynamic signal input update

The test properly updates the disableInput signal for testing the readonly mode functionality.


39-40: Successfully migrated property access to signal function call

The test now correctly uses component.exam() to access the exam signal value rather than direct property access.


45-47: Properly updated assertions to use signal function calls

The assertions correctly use component.exam().testExam and component.exam().numberOfCorrectionRoundsInExam instead of direct property access, aligning with Angular signals pattern.


52-54: Correctly updated assertions for exam mode test

The test now properly accesses the exam properties through the signal function call (component.exam()), maintaining the test's functionality while adapting to Angular signals.

src/test/javascript/spec/component/exam/manage/student-exams/exam-status.component.spec.ts (9)

17-17: Appropriate import of Angular input signal

The import of input from @angular/core is correctly added for migrating to signal-based inputs.


42-46: Successfully migrated exam preparation function to use signals

The test helper function now correctly uses TestBed.runInInjectionContext to set up the exam and course inputs using Angular signals.


53-57: Properly migrated test exam preparation function

The helper function for test exam preparation now correctly uses signals for exam and course inputs.


61-65: Correctly updated exam review state test preparation

The exam review state test preparation function now properly uses Angular signals for component inputs.


107-111: Successfully migrated course update for finished exam test

The test correctly updates the course input using Angular signals within TestBed.runInInjectionContext.


117-121: Properly configured inputs for exam review state test

The test correctly sets up both exam and course inputs using Angular signals.


135-138: Successfully migrated exam update for review phase test

The test properly updates the exam input signal after modifying the exam object's properties.


166-174: Correctly migrated exam and course setup for preparation test

The test now properly initializes both exam and course inputs using Angular signals in separate injection contexts, maintaining proper test isolation.


222-226: Successfully migrated course setup for test exam preparation

The test correctly sets up the course input using Angular signals for the test exam preparation scenario.

src/test/javascript/spec/component/exam/manage/student-exams/student-exam-detail-table-row.component.spec.ts (3)

27-28: Good addition of the required Angular 19 imports

These imports support the migration to signals for inputs in Angular 19, which is consistent with the PR objectives.


88-97: Properly migrated to Angular's new input API using signals

The code correctly uses TestBed.runInInjectionContext() to set up inputs with the new signal-based approach. This aligns with the Angular 19 migration requirements.


97-99: Good implementation of Angular component lifecycle with detectChanges

Calling detectChanges() after setting inputs ensures the component is properly updated, and setting courseId after the injection context is the correct approach.

src/main/webapp/app/exam/shared/working-time-control/working-time-control.component.html (6)

3-4: Correctly migrated conditional to use @if with function-based property access

The code correctly uses the new Angular control flow syntax with @if instead of *ngIf and properly accesses the property as a function call with durationLabelText().


10-11: Properly updated to function-based property access for disabled state

The template correctly uses disabled() with parentheses to access the property as a function call, which is the correct pattern for signal-based inputs in Angular 19.

Also applies to: 23-24, 37-38


15-16: Correctly updated allowNegative property to function call syntax

The template properly accesses allowNegative() as a function, consistent with the signal-based approach in Angular 19.

Also applies to: 28-30, 42-44


50-51: Properly migrated to @if with function calls for conditional checks

The code correctly uses the new Angular control flow syntax with @if and properly accesses properties as function calls with exam() and relative().


52-54: Correctly updated to function-based property access with non-null assertion

The template properly uses relativeLabelText() with parentheses and includes the non-null assertion operator, which is appropriate for translation keys.


64-65: Properly updated allowNegative property in percentage input

The template correctly uses allowNegative() as a function call for the percentage input's min attribute.

src/test/javascript/spec/component/exam/manage/exams/exam-checklist.component.spec.ts (3)

22-23: Good addition of Angular's input function import

The import supports the migration to signals for component inputs in Angular 19.


91-95: Properly migrated exam input initialization to use Angular's injection context

The code correctly uses TestBed.runInInjectionContext() to set up the exam input with the signal-based approach, which is consistent with Angular 19 best practices.

🧰 Tools
🪛 Biome (1.9.4)

[error] 90-95: Disallow duplicate setup and teardown hooks.

Disallow beforeEach duplicacy inside the describe function.

(lint/suspicious/noDuplicateTestHooks)


101-102: Correctly updated property access to function call syntax

The test now properly uses exam() as a function call instead of direct property access, which is the correct pattern for accessing signal-based inputs in Angular 19.

Also applies to: 108-109, 128-129

src/test/javascript/spec/component/exam/manage/exams/exam-checklist-exercisegroup-table.component.spec.ts (2)

4-5: Properly updated Angular imports to include input function

The import has been correctly updated to include the input function from Angular core, which is needed for the signal-based input approach.


99-102: Correctly migrated to Angular's new input API in all test cases

The tests properly use TestBed.runInInjectionContext() to set up the exerciseGroups input with the signal-based approach across all test cases. This is consistent with the Angular 19 migration guidelines.

Also applies to: 120-123, 135-138

src/main/webapp/app/exam/manage/student-exams/student-exam-detail-table-row/student-exam-detail-table-row.component.html (15)

2-2: Properly migrated data binding syntax

The property access has correctly been updated to use the function-call approach exercise()?.id instead of direct property access, which aligns with Angular signals pattern.


5-6: Angular syntax update looks good

You've properly implemented the new Angular @if syntax replacing *ngIf and correctly migrated the property access to the function-call pattern with exercise().


10-10: Property access correctly updated

The title binding is properly updated to use the function-call syntax exercise()?.title.


13-15: Properly migrated conditional and property access

The conditional logic and nested property access have been properly updated to use function calls for both exercise() and achievedPointsPerExercise().


22-22: Function parameter correctly updated

The parameter passed to getMaxPoints() has been properly updated to use the function-call pattern.


25-25: Function parameter correctly updated

The parameter passed to getBonusPoints() has been properly updated to use the function-call pattern.


40-40: Complex conditional properly migrated

The conditional statement with multiple conditions has been properly updated to use function calls for course(), exercise(), and studentExam().


42-42: Conditional check properly updated

The nested conditional check now correctly uses the function-call pattern for exercise().


46-46: Class binding correctly updated

The class binding now uses the function-call pattern with busy().


48-48: Router link binding correctly updated

The router link binding now uses the function-call pattern for the exercise() parameter.


56-56: Conditional logic updated with correct function calls

The conditional check with multiple conditions has been properly migrated to use the function-call pattern for exercise().


60-60: Class binding correctly updated

The class binding now correctly uses the function-call pattern with busy().


62-62: Router link binding properly updated

The router link binding now uses the function-call pattern for the parameters.


70-70: Type checking condition properly updated

The type checking conditional now uses the function-call pattern for exercise().


72-72: Multiple bindings correctly updated

All bindings (class binding and router link) have been properly updated to use the function-call pattern.

src/test/javascript/spec/component/exam/shared/testexam-working-time.component.spec.ts (10)

5-7: Import and path updates look good

The addition of the input import from @angular/core is correct and necessary for the updated test approach using signals.


42-47: Test setup properly migrated to use signals

The test setup has been correctly updated to use Angular's injection context for setting input properties, which is the recommended approach when using signals.


52-57: Test setup properly migrated to use signals

This test case has been correctly updated to use the new pattern with TestBed.runInInjectionContext() and the input() function.


62-67: Test setup properly migrated to use signals

This test case has been correctly updated to use the new pattern with TestBed.runInInjectionContext() and the input() function.


72-77: Test setup properly migrated to use signals

This test case has been correctly updated to use the new pattern with TestBed.runInInjectionContext() and the input() function.


82-87: Test setup properly migrated to use signals

This test case has been correctly updated to use the new pattern with TestBed.runInInjectionContext() and the input() function.


92-97: Test setup properly migrated to use signals

This test case has been correctly updated to use the new pattern with TestBed.runInInjectionContext() and the input() function.


102-107: Test setup properly migrated to use signals

This test case has been correctly updated to use the new pattern with TestBed.runInInjectionContext() and the input() function.


111-116: Test setup properly migrated to use signals

This test case has been correctly updated to use the new pattern with TestBed.runInInjectionContext() and the input() function.


120-125: Test setup properly migrated to use signals

This test case has been correctly updated to use the new pattern with TestBed.runInInjectionContext() and the input() function.

src/main/webapp/app/exam/shared/events/exam-live-event.component.html (12)

1-1: Properly migrated data binding syntax

The class binding has been correctly updated to use the function-call pattern with event().


4-4: String interpolation correctly updated

The string interpolation now correctly uses the function-call pattern with event().


8-10: Property access and conditional rendering properly updated

The property access and conditional rendering have been properly updated to use the function-call pattern with event().


14-14: Switch statement properly updated

The switch statement now correctly uses the function-call pattern with event().


32-32: Conditional check properly updated

The conditional check now correctly uses the function-call pattern with event().


37-37: Property access correctly updated

The nested property access has been properly updated to use the function-call pattern with event().


41-41: Property access correctly updated

The nested property access has been properly updated to use the function-call pattern with event().


45-45: Property access correctly updated

The nested property access has been properly updated to use the function-call pattern with event().


47-47: Conditional check and property access properly updated

The conditional check and nested property access have been properly updated to use the function-call pattern with event().


50-50: Property access correctly updated

The nested property access has been properly updated to use the function-call pattern with event().


73-73: Conditional rendering properly updated

The conditional rendering now correctly uses the function-call pattern with showAcknowledge().


79-79: Type check condition properly updated

The type check condition now correctly uses the function-call pattern with event().

src/main/webapp/app/exam/shared/working-time-change/working-time-change.component.ts (2)

1-1: Proper import for Angular signal inputs

The import statement correctly adds input from @angular/core which is required for the signals-based input approach.


11-12: Correctly migrated @input properties to signals

The component has been properly migrated from traditional @Input() decorators to the new signals-based approach using input.required<number>(). This matches the Angular 19 migration pattern being implemented across the codebase.

src/main/webapp/app/exam/manage/students/exam-students.component.ts (3)

1-1: Imports updated correctly for signal-based API.

The imports have been correctly updated to include viewChild from Angular core, which is needed for the signal-based approach to component references.


58-58: ViewChild correctly migrated to signal-based API.

The ViewChild decorator has been properly replaced with the new signal-based approach using viewChild.required(DataTableComponent).


181-184: Property access correctly updated to function call.

The code has been properly updated to use function call syntax dataTable() instead of property access, consistent with the signal-based approach. This ensures reactive access to the DataTableComponent instance.

src/main/webapp/app/exam/manage/exam-status.component.html (14)

1-1: Property access correctly updated to function call.

The template correctly uses course()?.isAtLeastInstructor function call syntax instead of property access, consistent with the signal-based approach.


3-3: Updated to modern Angular control flow syntax.

The code correctly uses the new @if control flow syntax instead of the deprecated *ngIf directive, as specified in the coding guidelines.


20-20: Property access correctly updated to function call.

The code has been properly updated to use exam().exerciseGroups instead of direct property access, consistent with the signal-based approach.


31-32: Property access correctly updated to function call.

The conditional check and interpolation properly use exam().examMaxPoints function call syntax instead of property access.


35-35: Property access correctly updated to function call.

The condition correctly uses exam().examMaxPoints function call syntax instead of property access.


43-43: Property access correctly updated to function call.

The translateValues parameter correctly uses exam().numberOfExamUsers function call syntax.


64-64: Property access correctly updated to function call.

The translateValues object correctly uses exam().numberOfExamUsers function call syntax.


117-117: Property access correctly updated to function call.

The date interpolation correctly uses exam().startDate and exam().endDate function call syntax.


125-125: Property access correctly updated to function call.

The condition correctly uses course()?.isAtLeastInstructor function call syntax and maintains the proper logical flow.


128-128: Property access correctly updated to function call.

The interpolation correctly uses exam().workingTime function call syntax.


132-132: Property access correctly updated to function call.

The interpolation correctly uses exam().numberOfExamUsers function call syntax.


209-209: Property access correctly updated to function call.

The condition correctly uses exam().publishResultsDate function call syntax.


211-211: Property access correctly updated to function call.

The date pipe correctly uses exam().publishResultsDate function call syntax.


215-215: Property access correctly updated to function call.

The condition correctly uses exam().publishResultsDate function call syntax.

src/main/webapp/app/exam/manage/student-exams/student-exam-status/student-exam-status.component.ts (2)

1-1: Import updated correctly for signal-based API.

The import statement has been correctly updated to include input from Angular core, which is needed for the signal-based approach to input properties.


18-19: Input properties correctly migrated to signal-based API.

The input properties have been properly converted from traditional @Input() decorator to the new signal-based approach using input.required<boolean>().

src/main/webapp/app/exam/shared/testExam-workingTime/test-exam-working-time.component.ts (5)

1-1: Import updated correctly for signal-based API.

The import statement has been correctly updated to include input from Angular core, which is needed for the signal-based approach to input properties.


14-14: Input property correctly migrated to signal-based API.

The studentExam input property has been properly converted from traditional @Input() decorator to the new signal-based approach using input.required<StudentExam>().


25-31: Property access correctly updated to function calls.

The conditional checks have been properly updated to use function call syntax (this.studentExam().property) instead of property access.


32-32: Property access correctly updated to function call.

The code correctly retrieves the working time using this.studentExam().workingTime function call syntax.


34-36: Property access correctly updated to function calls.

The calculations for usedWorkingTime and percentUsedWorkingTime correctly use function call syntax to access studentExam properties, with appropriate null checks.

src/test/javascript/spec/component/exam/manage/exams/exam-announcement-dialog/exam-live-announcement-create-button.component.spec.ts (3)

12-12: Importing the Angular input function for signal-based inputs.

The import of the input function from '@angular/core' aligns with the Angular 19 migration to signals, replacing the traditional @Input() decorator.


34-41: Updated exam input initialization to use signal-based approach.

The test setup now properly uses the Angular 19 signal-based approach:

  1. Creates an exam object
  2. Uses TestBed.runInInjectionContext to set the input in the correct context
  3. Uses input() function to create the signal

This change correctly updates the component test to align with the Angular 19 migration.


48-48: Updated component property access to use function call syntax.

Changed from accessing component.exam.visibleDate to component.exam().visibleDate to correctly invoke the input signal accessor function, ensuring proper reactivity in the test.

src/main/webapp/app/exam/manage/exams/exam-checklist-component/exam-checklist.component.html (11)

1-1: Updated conditional check to use signal-based exam access.

Changed from optional chaining on property (exam?.course) to function call with optional chaining (exam()?.course), correctly implementing the Angular signal pattern.


54-54: Updated attribute binding to use signal-based exam access.

Changed all references to exam properties to use the function call pattern (exam().exerciseGroups, exam().numberOfExercisesInExam) ensuring reactive updates when the exam data changes.

Also applies to: 65-67


101-101: Updated component input binding to use signal-based access.

Changed property binding to child component to use the function call syntax to access the exam properties ([quizExamMaxPoints]="exam().quizExamMaxPoints", [exerciseGroups]="exam().exerciseGroups || []").


104-104: Updated router link bindings to use signal-based function pattern.

Changed router link binding from getExamRoutesByIdentifier('route') to getExamRoutesByIdentifier()('route'), correctly implementing the function call pattern for router links.

Also applies to: 128-128, 160-160, 184-184, 211-211, 252-252, 342-342, 456-456, 507-507, 586-586, 615-615, 647-647, 662-662


147-149: Updated conditional check attributes with signal-based access.

Changed attribute binding to correctly use function call syntax with optional chaining and non-null assertions (!!exam().numberOfExamUsers && exam().numberOfExamUsers! > 0).


155-155: Updated text interpolation to use signal-based access.

Changed text interpolation from {{ exam.numberOfExamUsers }} to {{ exam().numberOfExamUsers }} for proper reactivity.


265-266: Updated translation value binding to use signal-based access.

Changed the date values in the translateValues object to use function calls (exam().startDate and exam().endDate) ensuring reactive updates when dates change.


297-299: Updated conditional rendering and text interpolation with signal pattern.

Changed ngIf condition and text interpolation to use function call syntax (@if (exam().numberOfExamUsers) and {{ exam().numberOfExamUsers! }}).


357-357: Updated component input binding to use signal-based access.

Changed input binding from direct property access to function call ([exam]="exam()") when passing the exam to child components.

Also applies to: 371-371


474-474: Updated conditional checks with signal-based property access.

Changed attribute binding and conditional expression to use function call syntax ([checkAttribute]="!!exam().publishResultsDate" and @if (exam().publishResultsDate)).

Also applies to: 477-477


602-602: Updated checklist attribute binding with signal-based access.

Changed attribute binding for the review date checks to use function call syntax for accessing the exam properties ([checkAttribute]="!!exam().examStudentReviewStart" and [checkAttribute]="!!exam().examStudentReviewEnd").

Also applies to: 608-608

src/main/webapp/app/exam/shared/student-exam-working-time/student-exam-working-time.component.ts (3)

1-1: Updated import to include Angular signal input function.

Added the input import from '@angular/core' to support the migration to signal-based inputs.


13-13: Converted @input property to signal-based input.

Changed from @Input() studentExam: StudentExam; to studentExam = input.required<StudentExam>();, implementing the new Angular 19 signal-based input pattern.


20-25: Updated property access to use signal-based function calls.

Improved the code by:

  1. Using function call syntax to access the input (this.studentExam())
  2. Creating local constants for better readability (workingTime and exam)
  3. Maintaining the same logic while adapting to the signal-based pattern

This makes the code more reactive and easier to understand.

src/test/javascript/spec/component/exam/exam-update.component.spec.ts (3)

624-626: Added ResizeObserver mock to support component testing.

Added a mock implementation for ResizeObserver, which is needed for components that interact with DOM sizing events. This follows best practices for Angular testing by ensuring DOM interactions work correctly in the test environment.


692-692: Updated component property access to use function call syntax.

Changed from direct property access to function call with component.examExerciseImportComponent().selectedExercises, correctly implementing the signal-based pattern for accessing component references.


739-741: Updated exam input initialization to use signal-based approach.

Properly set up the exam input for the child component using:

  1. TestBed.runInInjectionContext to provide the correct context
  2. The input() function to create the signal
  3. Function call syntax to access the component reference

This correctly implements the Angular 19 signal pattern in the test setup.

src/test/javascript/spec/component/exam/manage/programming-exam-diff.component.spec.ts (11)

22-23: Appropriate imports added for Angular signals

The additions of input from @angular/core and StudentParticipation model are correctly imported to support the migration to signals.


61-64: Component inputs properly initialized using signals

The component's input properties are correctly set using the signal pattern with .set() method instead of direct assignment.


75-78: Proper update pattern for signal values

The update pattern using .update(() => value) is correctly implemented to modify signal values, maintaining reactivity.


91-95: Consistent signal updates throughout the test

The signal update pattern is consistently applied for all input properties, ensuring proper testing of the component's reactive behavior.


103-104: Signal update method used correctly

The update() method is correctly used to set the exercise value, maintaining consistency with the signals approach.


128-130: Proper testing context for input signals

The use of TestBed.runInInjectionContext() is the correct approach for initializing inputs in Angular tests when using signals.


132-135: Properly updating subject using the update pattern

The update method for exerciseIdSubject correctly maintains the reference to the subject while updating its value.


138-138: Correct function-based access for signal values

The property access has been updated to use function call syntax (exercise()) as required when using signals.


152-154: Consistent initialization of input signals across tests

The test maintains consistency in how input signals are initialized across different test cases.


156-159: Proper subject update pattern

The update pattern for the subject is correctly implemented, preserving the subject reference while updating its value.


162-162: Function-based property access maintained

The property access consistently uses function call syntax as required by the signals pattern.

src/main/webapp/app/exam/manage/exams/exam-checklist-component/exam-edit-workingtime-dialog/exam-edit-working-time-dialog.component.ts (1)

2-2: Appropriate imports added for Angular output signals

The component correctly imports inject and output from @angular/core to support the migration to signals.

src/main/webapp/app/exam/manage/exams/exam-checklist-component/exam-announcement-dialog/exam-live-announcement-create-button.component.ts (5)

2-2: Appropriate imports added for Angular signals

The component correctly imports inject and input from @angular/core to support the migration to signals.


22-22: Proper implementation of required input signal

The exam property is correctly implemented as a required input signal using input.required<Exam>().


47-47: Correct function call syntax for exam property

The exam property is correctly accessed using function call syntax (this.exam()) as required by signals.


51-51: Consistent use of function call syntax

The function call syntax is consistently used to access the exam property's visibleDate.


66-67: Proper access to nested properties in signals

The component correctly accesses nested properties from the exam signal using function call syntax.

src/main/webapp/app/exam/manage/student-exams/student-exam-timeline/programming-exam-diff/programming-exercise-exam-diff.component.html (6)

2-4: Correct function call syntax for signals in templates

The template correctly uses function call syntax (exercise()) to access the signal values.


5-6: Proper conditional rendering with signals

The conditional rendering and property binding correctly use function call syntax with signals.


12-16: Consistent input binding with signals

All input bindings to the child component correctly use function call syntax for accessing signal values.


18-18: Proper conditional rendering with optional chaining

The conditional rendering correctly uses function call syntax with optional chaining for the signal value.


23-29: Consistent translation value binding with signals

The translation value bindings correctly use function call syntax to access signal values.


25-25: Proper conditional check with signals

The template correctly uses function call syntax for conditional checks with the signal value.

src/main/webapp/app/exam/manage/exams/exam-checklist-component/exam-checklist.component.ts (7)

1-1: Successfully migrated to Angular Input API

The component correctly imports and uses the new Angular Input API with input.required<Exam>() syntax instead of the decorator pattern, which aligns with the overall Angular 19 migration effort.

Also applies to: 48-49


85-94: Properly updated property access pattern in ngOnInit

The function correctly uses the new functional form this.exam() instead of direct property access, maintaining consistency with the new Angular 19 input API.


101-122: Successfully updated property access in ngOnChanges

All references to the exam property have been correctly changed to use the functional form this.exam() throughout the method, ensuring consistent data access patterns.


125-133: Properly updated function calls in ngOnDestroy and related methods

The WebSocket topic retrieval and unsubscription logic now properly uses the functional form this.exam() for property access.


138-157: Correctly implemented property access in evaluateQuizExercises

The logic to evaluate quiz exercises now properly uses the functional access pattern with this.exam() and correctly extracts properties like course ID and exam ID.


162-181: Successfully updated property access in assessUnsubmittedExamModelingAndTextParticipations

All references to exam properties now correctly use the functional form this.exam() when accessing course ID and exam ID.


183-192: Properly updated calculateIsExamOver method

The method correctly checks for the longestWorkingTime and accesses exam properties using the functional form this.exam() when calculating if the exam is over.

src/main/webapp/app/exam/manage/exam-status.component.ts (10)

1-1: Successfully migrated to Angular Input API

The component correctly imports and implements the new Angular Input API with input.required<Exam>() and input.required<Course>() instead of decorators, which aligns with the Angular 19 migration effort.

Also applies to: 41-42


79-86: Properly updated WebSocket topic initialization in ngOnInit

The method properly uses the functional form this.exam() when retrieving WebSocket topics for submission and start events, maintaining consistency with the new input API pattern.


88-111: Successfully updated property access in ngOnChanges

All references to exam and course properties are correctly updated to use the functional form (this.exam() and this.course()) when retrieving statistics and checking various conditions.


113-118: Properly updated WebSocket topic unsubscription in ngOnDestroy

The method properly uses the functional form this.exam() when retrieving WebSocket topics for unsubscription, maintaining consistency with the Angular input API pattern.


124-139: Successfully updated property access in exercise configuration methods

All references to exam properties have been correctly updated to use the functional form this.exam() throughout the exam exercise configuration checks.


168-180: Properly updated property access in setConductionState

The method correctly uses the functional form this.course() and applies the appropriate conditional checks for determining the exam conduction state.


185-195: Successfully updated property access in setReviewState

The method properly uses this.exam() to access the examStudentReviewEnd property when determining the exam review state.


200-212: Properly updated property access in setCorrectionState

The method correctly uses this.exam() to access the publishResultsDate property when determining the exam correction state.


218-236: Successfully updated property access in setExamPreparation

The method properly uses this.exam() when calling examChecklistService methods to verify various exam preparation status checks.


241-252: Properly updated property access in exam timing methods

The methods correctly extract the exam reference using this.exam() before checking date properties, ensuring consistency with the new input API pattern.

src/main/webapp/app/exam/manage/exams/exam-mode-picker/exam-mode-picker.component.ts (2)

1-1: Successfully migrated to Angular Input and Output API

The component correctly imports and implements the new Angular Input and Output APIs with input.required<Exam>(), input.required<boolean>() and output() instead of decorators, which aligns with the Angular 19 migration effort.

Also applies to: 13-16


22-28: Properly updated property access in setExamMode

The method correctly uses the functional forms this.disableInput() and this.exam() when checking and updating exam mode properties, maintaining consistency with the new Angular input API pattern.

src/test/javascript/spec/component/exam/manage/student-exams/student-exam-timeline.component.spec.ts (7)

5-5: Imports correctly updated for signal-based testing.

The test file imports have been properly updated to include Subject from rxjs and signal from '@angular/core', which are necessary for the signal-based approach.

Also applies to: 30-30


143-143: ViewChild access correctly updated to function call.

The test correctly uses the functional approach to access the examNavigationBarComponent by calling component.examNavigationBarComponent().


185-185: QueryList replaced with signal.

The currentPageComponents has been correctly updated to use signal([]) instead of a QueryList, aligning with the signal-based architecture.


193-196: Test case structure simplified.

The test cases have been simplified by flattening the structure, which makes them easier to read and maintain.


199-215: Proper mock signal creation utility added.

A well-designed helper function createMockSignal has been added to make testing signal-based components easier by providing a mock implementation that supports the update() method.


208-221: Component mock updated to support signals.

The mock component has been properly updated to use signal-based properties, ensuring tests accurately reflect the component's new structure.


254-256: Variable type refinement improved.

The code now properly types the submission as a SubmissionVersion when handling text exercises, improving type safety.

src/main/webapp/app/exam/manage/exams/exam-update.component.ts (5)

6-6: Imports updated for signal-based dependency injection.

The imports have been correctly updated to include inject, viewChild and other required functions for the signal-based approach.


82-82: ViewChild decorator correctly migrated to viewChild function.

The @ViewChild decorator for examExerciseImportComponent has been properly replaced with viewChild.required(), aligning with the signal-based approach.


84-84: Template reference correctly migrated to viewChild function.

The @ViewChild decorator for the template reference has been properly replaced with viewChild<TemplateRef<any>>() with the correct selector.


239-239: Template reference access updated to function call.

The code now correctly calls this.workingTimeConfirmationContent() as a function instead of accessing it as a property.


271-271: Component reference access updated to function calls.

All references to examExerciseImportComponent have been properly updated to use the function call syntax this.examExerciseImportComponent().

Also applies to: 276-276, 306-306, 311-311

src/main/webapp/app/exam/manage/students/upload-images/students-upload-images-dialog.component.ts (5)

1-1: Imports correctly updated for signal-based component.

The import statement has been properly updated to include input, viewChild, and inject for the signal-based approach.


36-36: ViewChild correctly migrated to viewChild function.

The @ViewChild decorator for importForm has been properly replaced with viewChild<NgForm>() with the correct selector.


40-41: Input properties correctly migrated to signal-based inputs.

The @Input() decorator properties have been properly converted to use the new input.required<T>() approach.


87-88: Property access updated to function call with proper null checking.

The code has been improved by first retrieving the exam with const exam = this.exam() and then performing null checking, which is more readable and maintainable.


92-92: Method parameter access updated to function call.

The code now correctly calls this.courseId() as a function when passing it as a parameter to the service method.

src/main/webapp/app/exam/shared/events/exam-live-event.component.ts (4)

1-1: Proper signal import added.

The import statement has been correctly updated to include the new input and output functions from @angular/core, which are required for Angular's reactive signal-based approach.


26-32: Input and output properties correctly migrated to signals.

The component properties have been properly migrated from decorator-based (@Input, @Output) to signal-based approach:

  • Required input event uses input.required<ExamLiveEvent>()
  • Optional input showAcknowledge uses input(false) with default value
  • Output events use output<ExamLiveEvent>()

This aligns with Angular's new reactive approach.


42-42: Getter methods correctly updated to use signal syntax.

The getter methods have been properly updated to call the event input as a function (this.event()) instead of accessing it as a property, ensuring proper reactivity with the new signal-based approach.

Also applies to: 46-46, 50-50, 54-54


58-58: Event emission correctly updated to use signal syntax.

The event emission now properly passes the result of calling this.event() rather than accessing it as a property, maintaining reactivity with the new signal-based approach.

Also applies to: 62-62

src/main/webapp/app/exam/manage/student-exams/student-exam-timeline/student-exam-timeline.component.ts (7)

1-1: Component import statements correctly updated for signals.

The import statements have been properly updated to include:

  • viewChild and viewChildren for template queries
  • toObservable from Angular's rxjs-interop package

These imports support the migration to the signal-based approach.

Also applies to: 26-26


73-74: ViewChild references correctly migrated to signal-based queries.

The component's view queries have been properly migrated from decorator-based approach to signal-based:

  • @ViewChildren(ExamSubmissionComponent)viewChildren(ExamSubmissionComponent)
  • @ViewChild('examNavigationBar')viewChild.required<ExamNavigationBarComponent>('examNavigationBar')

This correctly implements Angular's new reactive template query pattern.


107-107: Method calls updated to use signal function syntax.

References to examNavigationBarComponent have been properly updated to use function call syntax (examNavigationBarComponent()) to access the signal value, ensuring proper reactivity.

Also applies to: 346-346


112-112: ViewChildren changes subscription updated to use toObservable.

The subscription to changes in currentPageComponents now uses currentPageComponentsChanges, which is created using toObservable. This follows Angular's recommended pattern for observing changes to signal-based queries.


130-140: Component property updates migrated to use the update pattern.

The activeProgrammingComponent property assignments now use the update() method, which is the recommended pattern for updating signal values in Angular. This ensures proper reactivity when the properties change.


324-324: Component access updated to use signal function syntax.

The access to currentPageComponents has been properly updated to use function call syntax (currentPageComponents()) to retrieve the current value of the signal.


437-437: Observable created from ViewChildren signal.

A new property currentPageComponentsChanges has been added that creates an observable from the currentPageComponents signal using toObservable. This follows the recommended pattern for observing changes to signal-based template queries.

src/main/webapp/app/exam/manage/students/upload-images/students-upload-images-button.component.ts (5)

1-1: Import statement correctly updated for signals.

The import statement has been properly updated to include the new input and output functions from @angular/core, which are required for Angular's reactive signal-based approach.


15-15: Template binding updated to use signal function syntax.

The template binding for [btnSize] has been correctly updated to use function call syntax (buttonSize()) to access the current value of the signal.


29-32: Input properties correctly migrated to signals.

The component's input properties have been properly migrated from decorator-based to signal-based approach:

  • Required inputs use input.required<Type>()
  • Optional input with default value uses input<Type>(defaultValue)

This aligns with Angular's new reactive approach.


33-33: Output event correctly migrated to signal.

The output event uploadDone has been properly migrated from @Output() with EventEmitter to using the new output<void>() function, which is the recommended pattern for signal-based outputs.


46-47: Modal component instance properties correctly updated to use signal values.

The assignments to the modal component instance properties now correctly call the signal functions (this.courseId() and this.exam()) to access their current values.

src/main/webapp/app/exam/shared/working-time-control/working-time-control.component.ts (5)

1-1: Import statement correctly updated for signals and effects.

The import statement has been properly updated to include effect and input from @angular/core, which are required for Angular's reactive signal-based approach and responding to signal changes.


28-37: Input properties correctly migrated to signals.

The component's input properties have been properly migrated from decorator-based to signal-based approach:

  • Boolean inputs use input(defaultValue)
  • String inputs use input<string>()

This aligns with Angular's new reactive approach.


38-38: Exam input correctly migrated to signal.

The exam input property has been properly migrated to use the signal-based approach with optional typing.


40-45: Effect added to react to exam signal changes.

A constructor with an effect() has been added to respond to changes in the exam signal. This replaces the previous setter approach and ensures the component reacts properly when the exam changes, following Angular's recommended reactive pattern.


105-105: References to exam property updated to use signal function syntax.

All references to the exam property have been properly updated to use function call syntax (this.exam()) to access the current value of the signal, ensuring proper reactivity throughout the component.

Also applies to: 137-140, 148-153

src/main/webapp/app/exam/manage/student-exams/student-exam-detail-table-row/student-exam-detail-table-row.component.ts (4)

1-1: Import changes look good

The import statement has been properly updated to include input from @angular/core. This is necessary for the migration to Angular 19's signal-based inputs.


22-30: Successfully migrated from @Input() decorators to signal-based inputs

The component is correctly migrated to use the new Angular 19 signal-based inputs with input.required<T>() syntax. This approach ensures type safety and makes it clear that these properties are required inputs to the component.


44-46: Function calls replacing direct property access

You've correctly updated the code to access input properties as functions (e.g., this.exercise() instead of this.exercise). This is part of the Angular 19 migration to signals.


80-80: Function calls correctly applied in methods

The method parameter access is correctly updated to use the function call syntax this.examId().

src/main/webapp/app/exam/manage/student-exams/student-exam-timeline/programming-exam-diff/programming-exercise-exam-diff.component.ts (9)

1-1: Import statements properly updated

The import statement now correctly includes all the necessary Angular features for signal-based programming: inject, input, model, output, and signal.


36-43: Successfully migrated to model-based component properties

The migration from @Input() and @Output() decorators to the new model-based approach using model, input, and output functions is correctly implemented. This makes the component more reactive and aligns with Angular 19's signal-based architecture.


60-71: Properly updated subscription logic for signal-based access

The subscription to exerciseIdSubject now correctly calls it as a function with exerciseIdSubject(). The checks for cached reports and map operations also use the function call syntax (cachedDiffReports().has(key), cachedDiffReports().get(key)).


81-87: Function calls correctly applied in methods

The loadGitDiffReport method now properly accesses properties as functions, including previousSubmission(), currentSubmission(), and exercise().id.


92-93: Correct updating of cached diff reports

The update to use cachedDiffReports().set(key, gitDiffReport) and cachedDiffReportsChange.emit(this.cachedDiffReports()) properly follows the signal pattern.


100-105: Updated property assignments using function calls

The assignment operations now correctly access properties as functions, including exercise(), previousSubmission(), and currentSubmission().


134-134: Correctly using signal() for NgbModal component input

You've properly implemented the passing of data to the modal using signal() according to the retrieved learning which indicates this is necessary until NgbModal supports signal inputs.


143-143: Function calls in utility methods

The calculateMapKey method correctly uses function calls to access previousSubmission()?.id and currentSubmission()?.id.


146-156: Interface implementation updated

The implementation of interface methods getExercise(), getExerciseId(), and getSubmission() now correctly return the values by calling the signal functions.

src/main/webapp/app/exam/manage/exams/exam-checklist-component/exam-edit-workingtime-dialog/exam-edit-working-time.component.ts (5)

2-2: Import statement correctly updated for signals.

The import statement now includes inject, input, and output from '@angular/core', which are necessary for the signal-based approach in Angular 19.


22-23: Successfully migrated to signal-based inputs and outputs.

The component has been properly updated to use the new signal-based approach with input.required<Exam>() and output<Exam>() instead of the traditional @Input() and @Output() decorators.


46-46: Property access correctly converted to function call.

The code now correctly uses this.exam() to access the exam property, which is necessary when working with signal-based inputs.


64-64: Property assignment correctly updated for signals.

The code now properly assigns this.exam() to the modal component instance, ensuring that the current value of the exam signal is passed.


65-65:

✅ Verification successful

Verify subscription cleanup.

This line correctly subscribes to the examChange event from the modal, but please ensure this subscription is properly cleaned up when the modal is closed. The current unsubscribe in ngOnDestroy should handle this, but it's important to verify to prevent potential memory leaks.


🏁 Script executed:

#!/bin/bash
# Check if the subscription is properly cleaned up elsewhere in the codebase
rg -A 3 -B 3 "subscription.*\.unsubscribe\(\)" src/main/webapp/app/exam/manage/exams/exam-checklist-component/exam-edit-workingtime-dialog/

Length of output: 1185


Subscription cleanup verified.
The subscription.unsubscribe() call in ngOnDestroy has been confirmed in src/main/webapp/app/exam/manage/exams/exam-checklist-component/exam-edit-workingtime-dialog/exam-edit-working-time.component.ts. No memory leak issue was found based on the current implementation. Please continue ensuring that any future modifications maintain this cleanup pattern.

src/main/webapp/app/exam/manage/exams/exam-checklist-component/exam-checklist-exercisegroup-table/exam-checklist-exercisegroup-table.component.ts (4)

1-1: Import statement correctly updated for signals.

The import statement now includes input from '@angular/core', which is necessary for the signal-based approach in Angular 19.


19-20: Successfully migrated to signal-based inputs.

The component has been properly updated to use the new signal-based approach with input.required<T>() instead of the traditional @Input() decorators.


31-31: Property check correctly converted to function call.

The code now correctly uses this.exerciseGroups() to check if the exerciseGroups input has a value.


29-79:

❓ Verification inconclusive

Verify ngOnChanges behavior with signal-based inputs.

The ngOnChanges() lifecycle hook traditionally works with @Input() properties, but may behave differently with signal-based inputs. It's important to verify that changes to the inputs still trigger this method correctly.

Consider subscribing to the signal values directly or using a different approach if ngOnChanges isn't consistently triggered with signal inputs.


🏁 Script executed:

#!/bin/bash
# Check how other components handle changes with signal-based inputs
rg -A 5 "ngOnChanges.*input\." --type=ts src/main/webapp/

Length of output: 59


Action: Confirm Signal Behavior in ngOnChanges

The current implementation of ngOnChanges() is intended to handle updates for traditional @Input() properties. However, if you're now using signal-based inputs, it's important to manually verify that changes to these signal values are appropriately detected by ngOnChanges(). There was no evidence found in the codebase showing how other components manage signal-based inputs with this lifecycle hook. Please ensure that:

  • Signal-based inputs (if any) in this component indeed trigger ngOnChanges().
  • If not, consider subscribing directly to the signal values or switch to an alternative approach that aligns with Angular’s signal reactivity model.

coderabbitai[bot]
coderabbitai bot previously approved these changes Mar 9, 2025
ole-ve
ole-ve previously approved these changes Mar 9, 2025
Copy link
Contributor

@ole-ve ole-ve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reviewed the code, looks good. added some questions for understanding

@coolchock coolchock dismissed stale reviews from ole-ve and coderabbitai[bot] via 8222715 March 9, 2025 21:21
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
src/main/webapp/app/exam/manage/exam-status.component.ts (2)

258-262: Optimize optional chaining in isExamReviewRunning

The property accesses have been properly updated to use function calls, but the static analysis tool suggests optimizing the optional chaining.

 return (
-    ((!exam.examStudentReviewStart && exam.examStudentReviewEnd && exam.examStudentReviewEnd.isAfter(dayjs())) ||
-        (exam.examStudentReviewStart && exam.examStudentReviewStart.isBefore(dayjs()) && exam.examStudentReviewEnd!.isAfter(dayjs()))) ??
+    ((!exam.examStudentReviewStart && exam.examStudentReviewEnd?.isAfter(dayjs())) ||
+        (exam.examStudentReviewStart?.isBefore(dayjs()) && exam.examStudentReviewEnd?.isAfter(dayjs()))) ??
     false
 );
🧰 Tools
🪛 Biome (1.9.4)

[error] 261-261: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)


269-272: Optimize optional chaining in isExamReviewPlanned

The property access has been updated to use function calls, but the static analysis tool suggests optimizing the optional chaining here as well.

-return (exam.examStudentReviewStart && exam.examStudentReviewStart.isAfter(dayjs())) ?? false;
+return exam.examStudentReviewStart?.isAfter(dayjs()) ?? false;
🧰 Tools
🪛 Biome (1.9.4)

[error] 271-271: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 42dc259 and 8222715.

📒 Files selected for processing (2)
  • src/main/webapp/app/exam/manage/exam-status.component.html (6 hunks)
  • src/main/webapp/app/exam/manage/exam-status.component.ts (11 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/main/webapp/app/exam/manage/exam-status.component.html
🧰 Additional context used
📓 Path-based instructions (1)
`src/main/webapp/**/*.ts`: angular_style:https://angular.io/...

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:PascalCase;funcs:camelCase;props:camelCase;no_priv_prefix:true;strings:single_quotes;localize:true;btns:functionality;links:navigation;icons_text:newline;labels:associate;code_style:arrow_funcs,curly_braces,open_braces_same_line,indent_4;memory_leak_prevention:true;routes:naming_schema;chart_framework:ngx-charts;responsive_layout:true

  • src/main/webapp/app/exam/manage/exam-status.component.ts
🪛 Biome (1.9.4)
src/main/webapp/app/exam/manage/exam-status.component.ts

[error] 261-261: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)


[error] 271-271: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

⏰ Context from checks skipped due to timeout of 90000ms (9)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: Call Build Workflow / Build and Push Docker Image
  • GitHub Check: Call Build Workflow / Build .war artifact
  • GitHub Check: client-style
  • GitHub Check: client-tests-selected
  • GitHub Check: server-style
  • GitHub Check: client-tests
  • GitHub Check: server-tests
  • GitHub Check: Analyse
🔇 Additional comments (13)
src/main/webapp/app/exam/manage/exam-status.component.ts (13)

1-1: Updated imports to support signals

The imports have been properly updated to include inject and input functions that are part of Angular's newer approach to component inputs.


41-42: Input declarations migrated to signals

The component's inputs have been correctly migrated from the traditional @Input() decorator pattern to the modern signals approach using input.required<T>(). This is aligned with the Angular 19 migration goal.


79-86: Updated access pattern in ngOnInit

The property access has been correctly updated to use function calls (this.exam()) instead of direct property access.


88-93: Access pattern in ngOnChanges needs review

The property accesses have been properly updated to function calls, but the non-null assertion on testExam! should be reviewed. Since you're using the function call pattern, consider checking for undefined before accessing properties.

Instead of using non-null assertion, consider:

-this.isTestExam = this.exam().testExam!;
+this.isTestExam = !!this.exam().testExam;

94-97: Optional chaining check improved

The check for isAtLeastInstructor using optional chaining with the function call pattern looks good.


113-118: Signal access in ngOnDestroy correctly implemented

The component properly cleans up subscriptions in ngOnDestroy and has updated the access pattern to use function calls.


124-136: Access pattern updated in private methods

All property accesses have been properly updated to use function calls in the areAllExercisesConfigured method.


171-173: Conditional check with signal access

The conditional logic has been correctly updated to access properties through function calls.


186-186: Property access in setReviewState correctly updated

The property access has been properly updated to use function calls.


203-205: Signal access pattern in setCorrectionState

Property accesses have been correctly updated to use function calls.


224-226: Exam service calls updated correctly

The calls to the examChecklistService methods have been updated to pass the result of this.exam() function call.


241-244: Local variable for signal value improves readability

Good practice to store the signal value in a local variable before using it multiple times.


249-252: The examAlreadyEnded method uses correct signal access

This follows the same pattern as the previous method by storing the signal value locally. The implementation matches previous reviewer suggestions.

Copy link
Contributor

@ole-ve ole-ve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approve code, thanks for answering my questions 👍

Copy link
Member

@BBesrour BBesrour left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code

@helios-aet helios-aet bot temporarily deployed to artemis-test1.artemis.cit.tum.de March 10, 2025 15:02 Inactive
Copy link
Contributor

@tobias-lippert tobias-lippert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on ts1. All exam management functionality still works without issues

Copy link
Contributor

@chrisknedl chrisknedl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

☝️ What Tobias said.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client Pull requests that update TypeScript code. (Added Automatically!) ready to merge tests
Projects
Status: Ready For Review
Development

Successfully merging this pull request may close these issues.

5 participants