Exam mode: Fix student exam timeline page

[SimonEntholzer]

Checklist

General

• I tested all changes and their related features with all corresponding user types on a test server.
• This is a small issue that I tested locally and was confirmed by another developer on a test server.
• Language: I followed the guidelines for inclusive, diversity-sensitive, and appreciative language.
• I chose a title conforming to the naming conventions for pull requests.

Client

• Important: I implemented the changes with a very good performance, prevented too many (unnecessary) REST calls and made sure the UI is responsive, even with large data (e.g. using paging).
• I strictly followed the principle of data economy for all client-server REST calls.
• I strictly followed the client coding and design guidelines.
• Following the theming guidelines, I specified colors only in the theming variable files and checked that the changes look consistent in both the light and the dark theme.
• I added multiple integration tests (Jest) related to the features (with a high test coverage), while following the test guidelines.
• I added authorities to all new routes and checked the course groups for displaying navigation elements (links, buttons).
• I documented the TypeScript code using JSDoc style.
• I added multiple screenshots/screencasts of my UI changes.
• I translated all newly inserted strings into English and German.

Motivation and Context

The exam timeline view is broken currently.
I added some comments below, explaining the individual problems.

Description

This PR fixes the exam timeline view.
This issue was caused by someone (or the automatic Angular19 update) deleting the getExercise() method in the ProgrammingExerciseExamDiffComponent, which showed up as unused, as it inherited from the ExamPageComponent, and not from the ExamSubmissionComponent. In the ExamTimelineComponent, somehow the type system managed to use the ProgrammingExerciseExamDiffComponent as a ExamSubmissionComponent, although it did not inherit from it. But: after the deletion of the getExercise method, this broke.

    get activePageComponent(): ExamPageComponent | undefined {
        // we have to find the current component based on the activeExercise because the queryList might not be full yet (e.g. only 2 of 5 components initialized)
        return this.currentPageComponents.find((submissionComponent) => (submissionComponent as ExamSubmissionComponent).getExercise().id === this.activeExamPage.exercise?.id);
    }

The fix:

Steps for Testing

Prerequisites:

1. Create an exam, participate with a student. Add multiple submissions to the exercises.
2. As an instructor, open the exam timeline of a student: Exam -> Student exams -> View -> Exam timeline
   
3. Check that the exam timeline works as intened

You can access an exam timeline on TS2, with some submissions:
https://artemis-test2.artemis.cit.tum.de/course-management/43/exams/58/student-exams/2196/exam-timeline
(use artemis_test_user_20)

Testserver States

You can manage test servers using Helios. Check environment statuses in the environment list. To deploy to a test server, go to the CI/CD page, find your PR or branch, and trigger the deployment.

Review Progress

Code Review

• Code Review 1
• Code Review 2

Manual Tests

• Test 1
• Test 2

Test Coverage

Screenshots

Before fix: (nothing is displayed)

After fix (text and programming exercise):

Summary by CodeRabbit

• Bug Fixes

  • Ensured commit result cells display consistently across different views.

• Refactor

  • Optimized the processing of commit details to run only when data is available.

• New Features

  • Enhanced the exam diff interface with additional capabilities for managing exercise details and submissions.

[coderabbitai]

Walkthrough

This pull request adjusts parts of the commit information display and exam diff functionalities. In the commit info components, a table cell is now rendered unconditionally and the component’s initialization logic is reordered to only process commit details when commits are available. In the programming exam diff component, the inheritance is updated and several new methods and properties are introduced to improve handling of exercise and submission data.

Changes

File(s)	Change Summary
src/main/webapp/app/exercises/programming/.../commits-info-row.component.html src/main/webapp/app/exercises/programming/shared/commits-info/commits-info.component.ts	In the HTML file, a <td> displaying commit results is now always rendered rather than conditionally based on isRepositoryView. In the component, the ngOnInit logic is reordered so that commit details are only set and grouped after confirming commits are available, with profile info retrieval moved earlier.
src/main/webapp/app/exam/manage/student-exams/student-exam-timeline/programming-exam-diff/programming-exercise-exam-diff.component.ts	The component’s inheritance changes from ExamPageComponent to ExamSubmissionComponent, new import statements are added, and several methods (e.g., getExercise(), getSubmission(), hasUnsavedChanges(), etc.) along with a new exerciseType property are introduced to expand functionality.

Sequence Diagram(s)

sequenceDiagram
    participant C as CommitsInfoComponent
    participant P as profileService
    participant R as Commit Retrieval Flow

    C->>P: getProfileInfo()
    P-->>C: profile information
    alt Commits are defined
        C->>C: setCommitDetails()
        C->>C: groupCommits()
    else Commits are not defined
        C->>R: Retrieve commits using participationId
        R-->>C: Retrieved commits
        C->>C: setCommitDetails()
        C->>C: groupCommits()
    end

Loading

Possibly related PRs

• Programming exercises: Improve commit history view #8716: Adjustments to commit result rendering in commits-info-row.component.ts with introduction of new inputs and properties for commit information.

Suggested labels

tests, component:Exam Mode

Suggested reviewers

• krusche
• BBesrour
• JohannesStoehr
• coolchock

✨ Finishing Touches

• 📝 Generate Docstrings (Beta)

---

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 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. (Beta)
• @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.

[az108]

One small thing otherwise LGTM

[az108]

Thanks for clarifying :D Code LGTM 👍

[HanyangXu0508]

Server 4 test, worked.

[magaupp]

I found the actual cause of the getExercise() problem.
Other changes look good.

[simonbohnen]

Tested on TS2, works!

[jfr2102]

Works on TS2

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

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

143-166: Add JSDoc comments to interface methods.

While the implementation is correct, adding JSDoc comments would improve code documentation and maintainability, as mentioned in the PR objectives.

+    /**
+     * Returns the current exercise.
+     * @returns {Exercise} The current programming exercise
+     */
     getExercise(): Exercise {
         return this.exercise;
     }

+    /**
+     * Returns the ID of the current exercise.
+     * @returns {number | undefined} The exercise ID if available
+     */
     getExerciseId(): number | undefined {
         return this.exercise.id;
     }

+    /**
+     * Returns the current submission.
+     * @returns {Submission | undefined} The current submission if available
+     */
     getSubmission(): Submission | undefined {
         return this.currentSubmission;
     }

+    /**
+     * Checks if there are unsaved changes.
+     * @returns {boolean} Always returns false as programming exercises don't support draft changes
+     */
     hasUnsavedChanges(): boolean {
         return false;
     }

+    /**
+     * Sets the submission version.
+     * @param {SubmissionVersion} submissionVersion - The version to set
+     */
     setSubmissionVersion(submissionVersion: SubmissionVersion): void {
         this.submissionVersion = submissionVersion;
     }

+    /**
+     * Updates the submission based on view changes.
+     * Not implemented as programming exercises don't support direct editing.
+     */
     updateSubmissionFromView(): void {}

+    /**
+     * Updates the view based on submission changes.
+     * Not implemented as programming exercises don't support direct editing.
+     */
     updateViewFromSubmission(): void {}

Consider implementing view update methods.

The empty methods updateSubmissionFromView() and updateViewFromSubmission() might need implementation if view updates are required for programming exercises.

Could you clarify if these methods should remain empty or if they need implementation for the exam timeline functionality?

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 63ee5e0 and 5d46237.

📒 Files selected for processing (1)

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

🧰 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/student-exams/student-exam-timeline/programming-exam-diff/programming-exercise-exam-diff.component.ts

🧠 Learnings (1)

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.

⏰ Context from checks skipped due to timeout of 90000ms (8)

• GitHub Check: Call Build Workflow / Build and Push Docker Image
• GitHub Check: Call Build Workflow / Build .war artifact
• GitHub Check: client-tests-selected
• GitHub Check: client-style
• GitHub Check: client-tests
• GitHub Check: server-style
• GitHub Check: server-tests
• GitHub Check: Analyse

🔇 Additional comments (3)

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

9-9: LGTM! The inheritance change aligns with component responsibility.

The switch from ExamPageComponent to ExamSubmissionComponent better reflects this component's role in handling exam submissions, which is crucial for the exam timeline functionality.

Also applies to: 22-23, 31-31

---

49-49: LGTM! Property initialization follows Angular guidelines.

The exerciseType property is correctly typed and initialized, following Angular naming conventions.

---

131-133: LGTM! Correct implementation of NgbModal inputs.

The modal inputs are properly wrapped with signal() as required by NgbModal, ensuring compatibility.

[simonbohnen]

Reapprove

[magaupp]

Thanks for the change, LGTM 👍

[jfr2102]

re-aprove

[az108]

reapprove