Programming exercises: Fix broken download button for auxiliary repositories

[iyannsch]

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.

Server

• Important: I implemented the changes with a very good performance and prevented too many (unnecessary) database calls.
• I strictly followed the server coding and design guidelines.
• I added multiple integration tests (Spring) related to the features (with a high test coverage).
• I documented the Java code using JavaDoc style.

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.
• 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.

Changes affecting Programming Exercises

• High priority: I tested all changes and their related features with all corresponding user types on a test server configured with the integrated lifecycle setup (LocalVC and LocalCI).

Motivation and Context

Issue #8605 describes the current state in which the download of non-existent auxiliary repositories fail silently. This is not a problem for building or testing programming exercises, but heavily limit the usability for novice users.

When auxiliary repos are created during exercise creation, the auxiliary repos are created and linked correctly. In this case, the current implementation successfully provided a download.
However, when aux. repos are added to existing exercises, no directory is created in LocalVC, but the DB is kept in sync and represents the changes. The instructor performing the action is informed about this situation but no fix is suggested nor is a permanent error message raised. When trying to clone or download the non-existent repository, an internal server error (500) arises but no information is shown in the UI. This leaves teams of multiple instructors with the situation that they might not understand why an aux. repo does not work.

Description

I propose a more sophisticated solution changing the way how auxiliary repositories are added to already existing exercises.
Adding new aux repos to existing exercises (and a set of already existing aux repos), poses the challenge of not overwriting those aux repos that are already referenced in tests etc.
This code contains the logic for detecting "illegal" auxiliary repo changes. As soon as a name or checkoutDirectory are changed, it is considered illegal.

As a first step, I implemented an error handler for failing repository downloads. Users are not informed about the situation and know that their repository does not exist. This fights the symptoms but not the root-cause.

To solve the underlying problem, I added the functionality in the server to keep the filesystem in sync with the database when deleting and creating auxiliary repositories. The instructor can add and remove auxiliary repositories during both exercise creation and modification. Editing existing auxiliary repositories (e.g., changing the repo's name) is not supported and will still throw the error.

Discussion for further improvement

Currently, the VersionControlService-Interface does not offer the functionality to modify existing repositories. Systems like GitLab that were used in the past do not offer such functionality with Artemis as well. It could make sense to extend the LocalVC with editing functionality for auxiliary repos - currently, I'll prioritize more pressing issues and leave this for future work.

Steps for Testing

Prerequisites:

• 1 Instructor
• 1 Course

Verify successful download on creation

1. Log in to Artemis
2. Create a new programming exercise and add an auxiliary repository before saving the exercise
3. Verify that you can download the aux repo

4. Verify that the banner is shown

Verify successful download on modification

1. Log in to Artemis
2. Edit an existing programming exercise and add an auxiliary repository before saving the exercise
3. Verify that you can download the aux repo

4. Verify that the banner is shown

Verify error alert when misconfiguring repositories

1. Log in to Artemis
2. Edit an existing programming exercise
3. Rename an existing auxiliary repository and save
4. Verify that the banner is shown
   

Verify error handling on misconfigured repositories

1. Log in to Artemis
2. Edit an existing programming exercise
3. Rename an existing auxiliary repository and save
4. Click on the Download Repository button and verify that you see an error banner
   

Exam Mode Testing

Testserver States

Note

These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.

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
• I (as a reviewer) confirm that the server changes (in particular related to database calls) are implemented with a very good performance

Code Review

• Code Review 1
• Code Review 2

Manual Tests

• Test 1
• Test 2

Exam Mode Test

• Test 1
• Test 2

Test Coverage

Screenshots

Summary by CodeRabbit

• Style

  • Improved the layout of the detail overview list by adding top margin to list items.

• Bug Fixes

  • Enhanced error handling in repository export functionality for programming exercises, providing better feedback when an export fails.

• Documentation

  • Added error messages for failed repository exports in both German and English to improve user feedback during export processes.

[coderabbitai]

Walkthrough

The recent changes in the project focus on enhancing the user interface, improving error handling in exporting repositories, and adding new user feedback messages for repository export errors. Specifically, the styling of list items in a component was updated, error handling in repository export was fortified with RxJS, and new multilingual error messages were added for better user communication in case of export failures.

Changes

File	Change Summary
src/main/webapp/app/detail-overview-list/detail-overview-list.component.html	Added class="mt-2" to <li> elements to adjust the styling by adding a top margin.
src/main/webapp/app/exercises/programming/shared/actions/programming-exercise-instructor-repo-download.component.ts	Enhanced exportRepository method with error handling using RxJS operators.
src/main/webapp/i18n/de/programmingExercise.json, src/main/webapp/i18n/en/programmingExercise.json	Added new error messages for failed repository export to improve user feedback in both German and English.

Sequence Diagram(s)

No sequence diagrams needed as the changes primarily involve stylistic adjustments and error handling improvements without significantly altering control flows.

---

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>.
  • 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 generate interesting stats about this repository and render them as a table.
  • @coderabbitai show all the console.log statements in this repository.
  • @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 as 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 resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

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.

[coderabbitai]

Actionable comments posted: 1

[JohannesStoehr]

Regarding the root cause:
With external VCS like Bitbucket or Gitlab it was hard to track what users do without the knowledge of Artemis (creating additional repositories, changing build plans, ...). So it was hard for Artemis to track these changes accurately. Additionally it was hard to adapt the potentially custom build plans to correctly checkout newly added auxiliary repositories later on.

With LocalVC these problems should no longer occur, so we can provide instructors with the option to manage auxiliary repositories right in Artemis and then make sure that no additional repositories exist that are not accounted for or that auxiliary repositories exist in the Artemis database but not the LocalVC

[github-actions]

There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions.

[iyannsch]

Don´t close it. @krusche is this a relevant feature to implement as @JohannesStoehr suggested?

[krusche]

I suggest we fix the root cause and allow more options for adding / editing / removing aux repos in LocalVC

[github-actions]

There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions.

[krusche]

Ideally, we provide more options in a follow-up and fix the root cause / bug first quickly, this PR is already open too long

[JohannesStoehr]

Code

[marlon-luca-bu]

Code

[BBesrour]

Tested on TS1. works as described.

Should the warning be displayed when changing the checkoutPath?

[iyannsch]

@BBesrour I think for more complicated Build Jobs this could make sense. Regardless of this, it was initially implemented like this - I only removed the alert for creation and deletion 👍