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

Integrated code lifecycle: Add auxiliary repository view #9321

Merged
merged 48 commits into from
Oct 19, 2024

Conversation

SimonEntholzer
Copy link
Contributor

@SimonEntholzer SimonEntholzer commented Sep 16, 2024

Checklist

General

Server

  • Important: I implemented the changes with a very good performance and prevented too many (unnecessary) and too complex database calls.
  • I strictly followed the principle of data economy for all 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 added pre-authorization annotations according to the guidelines and checked the course groups for all new REST Calls (security).
  • 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 (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.

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

It is possible to view all repositories in the repository view, except auxiliary repositories.

Description

This PR adds the capability to view the auxiliary repository.

Disclaimer: as this PR makes changes on various levels of the code-editor, we could refactor it to adhere to the new client guidelines.
We will refactor the code editor to use the new client guidelines in a follow-up, as the change is very drastic.

Steps for Testing

Prerequisites:

  • 1 Instructor
  1. Create a programming exercise and add an auxiliary repository.
  2. Clone the auxiliary repository and commit something.
  3. In the exercise details page, use the code button to go to the auxiliary repositories repository view (the link icon on the right of the link)
    image

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

Code Review

  • Code Review 1
  • Code Review 2

Manual Tests

  • Test 1
  • Test 2

Test Coverage

Client

Class/File Line Coverage Confirmation (assert/expect)
programming-exercise.service.ts 90.18% ✅ ❌
code-editor.model.ts 100%
code-editor-conflict-state.service.ts 96.87% ✅ ❌
code-editor-domain.service.ts 100%
commit-history.component.ts 100%
repository-view.component.ts 98.03% ✅ ❌
Class/File Line Coverage Confirmation (assert/expect)
ProgrammingExerciseParticipationService.java 74% ✅ ❌
ProgrammingExerciseService.java 87% ✅ ❌
ProgrammingExerciseParticipationResource.java 88% ✅ ❌
ProgrammingExerciseResource.java 79% ✅ ❌
AuxiliaryRepositoryResource.java 97% ✅ ❌

Screenshots

image

Summary by CodeRabbit

Release Notes

  • New Features

    • Introduced a comprehensive set of RESTful endpoints for managing auxiliary repositories, allowing users to perform actions such as creating, deleting, and renaming files and folders.
    • Added functionality for pulling changes, committing changes, and checking repository status.
    • Enhanced commit history retrieval specifically for auxiliary repositories.
    • Implemented a new method to retrieve programming exercises along with their associated auxiliary repositories.
    • Added routing configurations for improved access to repository-related views.
  • Bug Fixes

    • Improved error handling for repository operations, ensuring robust management of access violations and state conflicts.
  • Improvements

    • Enhanced security measures for endpoint access, ensuring only authorized users can perform repository actions.
    • Refactored commit handling logic for better organization and clarity in the application.
  • Documentation

    • Updated documentation to reflect new functionalities and endpoint usage for auxiliary repositories.

@SimonEntholzer SimonEntholzer requested a review from a team as a code owner September 16, 2024 16:25
Copy link

coderabbitai bot commented Sep 16, 2024

Walkthrough

The changes introduce a new AuxiliaryRepositoryResource class that acts as a REST controller for managing auxiliary repositories within a programming exercise framework. It extends RepositoryResource and provides various endpoints for authorized users to perform operations such as file management, committing changes, and retrieving repository status. Additionally, the ProgrammingExerciseParticipationResource class has been updated to support auxiliary repositories, and modifications have been made to several components and services to handle commit retrieval and repository interactions, ensuring cohesive integration throughout the application.

Changes

File Change Summary
src/main/java/de/tum/cit/aet/artemis/programming/web/repository/AuxiliaryRepositoryResource.java Created a new resource for managing auxiliary repository operations, including methods for file management and repository status.
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseParticipationResource.java Updated to support auxiliary repositories, including a new repository dependency and modified commit history retrieval.
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java Added a method to retrieve programming exercises along with their associated auxiliary repositories.
src/main/webapp/app/exercises/programming/manage/services/programming-exercise-participation.service.ts Introduced a method for retrieving commit history from auxiliary repositories.
src/main/webapp/app/localvc/commit-history/commit-history.component.ts Refactored commit handling to support auxiliary repositories with new methods for commit retrieval.
src/main/webapp/app/localvc/repository-view/repository-view.component.ts Enhanced functionality to handle auxiliary repositories, including a method to load auxiliary repository details.
src/test/javascript/spec/helpers/mocks/service/mock-programming-exercise-participation.service.ts Added a mock method for retrieving commit history for auxiliary repositories.
src/test/javascript/spec/helpers/mocks/service/mock-programming-exercise.service.ts Added a mock method to find programming exercises with auxiliary repositories.
src/test/javascript/spec/service/programming-exercise.service.spec.ts Introduced a test case for the new method that retrieves programming exercises along with auxiliary repositories.
src/main/webapp/app/exercises/programming/manage/programming-exercise-management-routing.module.ts Added new routes for accessing repository views and commit history for auxiliary repositories.
src/test/javascript/spec/component/localvc/repository-view.component.spec.ts Added a test case to validate loading of auxiliary repository type in the component.
src/test/java/de/tum/cit/aet/artemis/exercise/programming/AuxiliaryRepositoryResourceIntegrationTest.java Created integration tests for the AuxiliaryRepositoryResource functionality, covering file operations and access control.
src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseIntegrationTestService.java Added a method for adding auxiliary repositories to programming exercises within the integration test service.
src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseParticipationIntegrationTest.java Enhanced integration tests for commit history retrieval, including new tests for auxiliary repositories.

Possibly related PRs

Suggested labels

feature, assessment, atlas, communication

Suggested reviewers

  • JohannesStoehr
  • pzdr7
  • florian-glombik
  • EneaGore
  • undernagruzez
  • BBesrour

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

@SimonEntholzer SimonEntholzer self-assigned this Sep 16, 2024
@github-actions github-actions bot added server Pull requests that update Java code. (Added Automatically!) client Pull requests that update TypeScript code. (Added Automatically!) labels Sep 16, 2024
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: 8

Review details

Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE

Commits

Files that changed from the base of the PR and between 8ff8197 and a6d003e.

Files selected for processing (16)
  • src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java (2 hunks)
  • src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseService.java (1 hunks)
  • src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseParticipationResource.java (5 hunks)
  • src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java (1 hunks)
  • src/main/java/de/tum/cit/aet/artemis/programming/web/repository/AuxiliaryRepositoryResource.java (1 hunks)
  • src/main/webapp/app/detail-overview-list/components/programming-auxiliary-repository-buttons-detail/programming-auxiliary-repository-buttons-detail.component.html (1 hunks)
  • src/main/webapp/app/detail-overview-list/detail-overview-list.component.html (1 hunks)
  • src/main/webapp/app/exercises/programming/manage/programming-exercise-management-routing.module.ts (2 hunks)
  • src/main/webapp/app/exercises/programming/manage/services/programming-exercise-participation.service.ts (1 hunks)
  • src/main/webapp/app/exercises/programming/manage/services/programming-exercise.service.ts (1 hunks)
  • src/main/webapp/app/exercises/programming/shared/code-editor/model/code-editor.model.ts (3 hunks)
  • src/main/webapp/app/exercises/programming/shared/code-editor/service/code-editor-conflict-state.service.ts (1 hunks)
  • src/main/webapp/app/exercises/programming/shared/code-editor/service/code-editor-domain-dependent-endpoint.service.ts (1 hunks)
  • src/main/webapp/app/exercises/programming/shared/code-editor/service/code-editor-domain.service.ts (2 hunks)
  • src/main/webapp/app/localvc/commit-history/commit-history.component.ts (4 hunks)
  • src/main/webapp/app/localvc/repository-view/repository-view.component.ts (3 hunks)
Additional context used
Path-based instructions (16)
src/main/webapp/app/exercises/programming/shared/code-editor/service/code-editor-domain.service.ts (1)

Pattern 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/detail-overview-list/components/programming-auxiliary-repository-buttons-detail/programming-auxiliary-repository-buttons-detail.component.html (1)

Pattern src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.

src/main/webapp/app/exercises/programming/shared/code-editor/service/code-editor-domain-dependent-endpoint.service.ts (1)

Pattern 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/exercises/programming/shared/code-editor/service/code-editor-conflict-state.service.ts (1)

Pattern 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/exercises/programming/shared/code-editor/model/code-editor.model.ts (1)

Pattern 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/detail-overview-list/detail-overview-list.component.html (1)

Pattern src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.

src/main/webapp/app/exercises/programming/manage/services/programming-exercise-participation.service.ts (1)

Pattern 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/localvc/commit-history/commit-history.component.ts (1)

Pattern 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/localvc/repository-view/repository-view.component.ts (1)

Pattern 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/exercises/programming/manage/programming-exercise-management-routing.module.ts (1)

Pattern 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/java/de/tum/cit/aet/artemis/programming/web/repository/AuxiliaryRepositoryResource.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseParticipationResource.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/webapp/app/exercises/programming/manage/services/programming-exercise.service.ts (1)

Pattern 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/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseService.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

Biome
src/main/webapp/app/exercises/programming/shared/code-editor/service/code-editor-conflict-state.service.ts

[error] 70-70: Forbidden non-null assertion.

Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator

(lint/style/noNonNullAssertion)

src/main/webapp/app/localvc/commit-history/commit-history.component.ts

[error] 94-94: Forbidden non-null assertion.

(lint/style/noNonNullAssertion)


[error] 148-148: Forbidden non-null assertion.

(lint/style/noNonNullAssertion)


[error] 161-161: Forbidden non-null assertion.

(lint/style/noNonNullAssertion)

src/main/webapp/app/localvc/repository-view/repository-view.component.ts

[error] 190-190: Forbidden non-null assertion.

(lint/style/noNonNullAssertion)


[error] 194-194: Forbidden non-null assertion.

(lint/style/noNonNullAssertion)

Additional comments not posted (42)
src/main/webapp/app/exercises/programming/shared/code-editor/service/code-editor-domain.service.ts (1)

17-17: LGTM! The changes align with the PR objectives and adhere to the Angular style guidelines.

The addition of DomainAuxiliaryRepositoryChange to the subject property type allows the DomainService to handle auxiliary repository changes, which is a key objective of this PR. The property naming and access modifier follow the Angular style guidelines specified in the additional instructions.

The use of a union type provides flexibility in handling different domain change scenarios while ensuring type safety.

src/main/webapp/app/detail-overview-list/components/programming-auxiliary-repository-buttons-detail/programming-auxiliary-repository-buttons-detail.component.html (2)

2-2: Good use of track by expression!

Using auxiliaryRepository.id for tracking in the @for loop is a good practice. It uniquely identifies each repository and should improve performance by ensuring efficient rendering.


6-11: Great enhancement to the navigation capabilities!

The new input property [routerLinkForRepositoryView] in the jhi-code-button component improves the user experience by allowing users to navigate directly to the detailed view of each auxiliary repository.

Setting the router link to an array that includes the current route, the repository type ('AUXILIARY'), and the repository's ID is a clear and direct way to construct the navigation path.

src/main/webapp/app/exercises/programming/shared/code-editor/service/code-editor-domain-dependent-endpoint.service.ts (1)

38-39: LGTM!

The new case for DomainType.AUXILIARY_REPOSITORY follows the existing pattern and generates the URL in the expected format. This change aligns with the PR objective of enabling visibility of auxiliary repositories.

src/main/webapp/app/exercises/programming/shared/code-editor/model/code-editor.model.ts (3)

5-5: LGTM!

The import statement for AuxiliaryRepository is correctly added and follows the project's directory structure.


53-53: LGTM!

The new AUXILIARY_REPOSITORY value is correctly added to the DomainType enum and follows the naming convention of the existing values.


99-100: LGTM!

The DomainAuxiliaryRepositoryChange type alias is correctly defined, and the DomainChange type is correctly modified to include the new type. The changes are necessary to represent the domain changes related to auxiliary repositories.

src/main/webapp/app/detail-overview-list/detail-overview-list.component.html (1)

7-7: LGTM! The change to track by index aligns with Angular best practices.

The update to the @for loop to track by $index instead of the detail object itself is a good change that aligns with the new Angular syntax and best practices. This change can potentially enhance performance by allowing Angular to better manage the rendering of the list, particularly in scenarios where the list may change dynamically.

By tracking the index instead of the object, the framework can optimize the rendering process and reduce the overhead associated with change detection. This is because when tracking by object, Angular needs to perform a deep comparison of the object properties to determine if the object has changed, which can be expensive for large or complex objects. In contrast, tracking by index is a simple and fast operation.

Additionally, using the new @for syntax instead of the old *ngFor style is recommended and should be used consistently throughout the codebase.

src/main/webapp/app/localvc/commit-history/commit-history.component.ts (8)

27-27: LGTM!

The addition of the optional repositoryId property is a necessary change to support the visibility of auxiliary repositories. The type and optional nature of the property are appropriate.


57-57: LGTM!

Extracting the repositoryId from the route parameters and assigning it to the component property is a logical way to obtain the repository ID. The type conversion using Number is appropriate.


93-94: LGTM!

The addition of the else if block to handle the 'AUXILIARY' repository type is a necessary change to support auxiliary repositories. Assigning the templateParticipation to the participation property for auxiliary repositories seems appropriate.

Tools
Biome

[error] 94-94: Forbidden non-null assertion.

(lint/style/noNonNullAssertion)


128-140: LGTM!

The refactoring of the handleCommits method to handle commits based on the repositoryType is a great improvement in terms of code readability and maintainability. The separation of logic into specific handler methods for each repository type makes the code more modular and easier to understand. The conditional checks ensure that the appropriate handler method is called based on the repository type.


142-152: LGTM!

The introduction of the handleParticipationCommits method is a good practice to improve code organization and readability. Filtering out commits without submissions and ensuring the presence of the template commit as the last commit are appropriate steps in handling participation commits.

Tools
Biome

[error] 148-148: Forbidden non-null assertion.

(lint/style/noNonNullAssertion)


154-165: LGTM!

The introduction of the handleAuxiliaryRepositoryCommits method is a good practice to improve code organization and readability. Utilizing the repositoryId to fetch the relevant commit history for auxiliary repositories aligns with the purpose of introducing the repositoryId property.

Tools
Biome

[error] 161-161: Forbidden non-null assertion.

(lint/style/noNonNullAssertion)


167-180: LGTM!

The introduction of the handleTemplateSolutionTestRepositoryCommits method is a good practice to improve code organization and readability. Encapsulating the existing logic for handling template, solution, or test repository commits in a dedicated method makes the code more modular and easier to understand. The method name clearly conveys its purpose, enhancing the self-explanatory nature of the code.


Line range hint 1-218: Overall, the changes in this file are well-structured and align with the PR objectives.

The refactoring of the handleCommits method and the introduction of separate handler methods for different repository types greatly improve code modularity, readability, and maintainability. The addition of the repositoryId property and its usage in retrieving commit history for auxiliary repositories effectively enables the visibility of auxiliary repositories.

The code changes follow good practices such as meaningful method names, appropriate type declarations, and consistent logic. The separation of concerns and the encapsulation of specific logic in dedicated methods make the code more understandable and easier to maintain.

Great job with the implementation!

Tools
Biome

[error] 148-148: Forbidden non-null assertion.

(lint/style/noNonNullAssertion)


[error] 161-161: Forbidden non-null assertion.

(lint/style/noNonNullAssertion)

src/main/webapp/app/localvc/repository-view/repository-view.component.ts (2)

45-45: LGTM!

The property follows the naming convention and is correctly typed.


88-94: LGTM!

The code segment correctly handles the 'AUXILIARY' repository type and calls the appropriate method with the correct arguments.

src/main/webapp/app/exercises/programming/manage/programming-exercise-management-routing.module.ts (2)

174-185: LGTM!

The new route configuration for accessing the repository view with an auxiliary repository ID is implemented correctly. It follows the existing code conventions and includes the necessary access controls and caching metadata.


198-209: Looks good!

The modification to the commit history route to include the auxiliary repository ID is implemented correctly. It maintains consistency with the changes made in the previous code segment and includes the necessary access controls and caching metadata.

src/main/java/de/tum/cit/aet/artemis/programming/web/repository/AuxiliaryRepositoryResource.java (13)

63-69: LGTM!

The constructor is using constructor injection to inject the required dependencies, which is a good practice. The dependencies are being passed to the super constructor and the auxiliaryRepositoryRepository is being set correctly.


72-78: LGTM!

The method is correctly retrieving the AuxiliaryRepository, checking user access, and returning the repository using the gitService. The logic and syntax look good.


81-84: LGTM!

The method is correctly retrieving the AuxiliaryRepository and returning its VcsRepositoryUri. The logic and syntax look good.


87-96: LGTM!

The method is correctly checking the user's access to the repository using the repositoryAccessService. It handles the AccessForbiddenException appropriately and returns the correct boolean value. The logic and syntax look good.


109-113: LGTM!

The method is correctly calling the getFiles method of the super class with the provided auxiliaryRepositoryId. The logic and syntax look good.


116-120: LGTM!

The method is correctly calling the getFile method of the super class with the provided auxiliaryRepositoryId and filename. The logic and syntax look good.


123-128: LGTM!

The method is correctly calling the createFile method of the super class with the provided auxiliaryRepositoryId, filePath, and request. The logic and syntax look good.


131-136: LGTM!

The method is correctly calling the createFolder method of the super class with the provided auxiliaryRepositoryId, folderPath, and request. The logic and syntax look good.


139-144: LGTM!

The method is correctly calling the renameFile method of the super class with the provided auxiliaryRepositoryId and fileMove. The logic and syntax look good.


147-152: LGTM!

The method is correctly calling the deleteFile method of the super class with the provided auxiliaryRepositoryId and filename. The logic and syntax look good.


155-159: LGTM!

The method is correctly calling the pullChanges method of the super class with the provided auxiliaryRepositoryId. The logic and syntax look good.


162-167: LGTM!

The method is correctly calling the commitChanges method of the super class with the provided auxiliaryRepositoryId. The logic and syntax look good.


170-175: LGTM!

The method is correctly calling the resetToLastCommit method of the super class with the provided auxiliaryRepositoryId. The logic and syntax look good.

src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseParticipationResource.java (4)

Line range hint 83-101: LGTM!

The constructor changes are consistent with the new auxiliaryRepositoryRepository field addition. The dependency injection of the AuxiliaryRepositoryRepository will enable the class to interact with auxiliary repositories.


343-349: The new logic for handling auxiliary repositories looks good!

The changes properly handle the case when the repositoryType is AUXILIARY:

  • The auxiliary repository is retrieved using the repositoryId.
  • It is verified that the auxiliary repository belongs to the correct exercise.
  • The commit information is retrieved using the new getAuxiliaryRepositoryCommitInfos service method.

Line range hint 384-414: Skipped review.

This method does not seem to be directly impacted by the changes related to auxiliary repositories. The existing logic for handling different repository types and participations remains unchanged.


324-325: Verify the API usage and documentation for the new repositoryId parameter.

Please ensure that the API clients have been updated to provide the repositoryId parameter when calling this method for auxiliary repositories. Also, verify that the API documentation has been updated to reflect this change.

src/main/webapp/app/exercises/programming/manage/services/programming-exercise.service.ts (1)

286-295: LGTM!

The new findWithAuxiliaryRepository function looks good. It enhances the service's capability by allowing the retrieval of a programming exercise along with its associated auxiliary repository in a single call.

The implementation is straightforward and follows the existing patterns in the service. The URL construction and usage of observe: 'response' are correct.

This addition can be particularly useful for scenarios where auxiliary repositories are relevant to the exercise being handled.

src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java (2)

541-543: Endpoint definition and access control look good!

The @GetMapping annotation defines a clear endpoint for retrieving a programming exercise with a specific auxiliary repository. The @EnforceAtLeastTutorInExercise annotation ensures proper access control.


547-550: Validation logic and error handling are appropriate.

The function correctly validates that the provided auxiliaryRepositoryId belongs to the exercise's list of auxiliary repositories. If validation fails, it throws a BadRequestAlertException with a clear error message, providing appropriate feedback to the client.

src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseService.java (2)

1012-1022: LGTM!

The function provides a clean and convenient way to load a programming exercise with specific related data based on the provided flags. It encapsulates the loading logic well, making it easier to use and maintain.


1019-1022: LGTM!

The new function provides a clean and convenient way to load a programming exercise with only its auxiliary repositories. It encapsulates the loading logic well, making it easier to use and maintain.

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: 7

Review details

Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE

Commits

Files that changed from the base of the PR and between 8ff8197 and a6d003e.

Files selected for processing (16)
  • src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java (2 hunks)
  • src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseService.java (1 hunks)
  • src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseParticipationResource.java (5 hunks)
  • src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java (1 hunks)
  • src/main/java/de/tum/cit/aet/artemis/programming/web/repository/AuxiliaryRepositoryResource.java (1 hunks)
  • src/main/webapp/app/detail-overview-list/components/programming-auxiliary-repository-buttons-detail/programming-auxiliary-repository-buttons-detail.component.html (1 hunks)
  • src/main/webapp/app/detail-overview-list/detail-overview-list.component.html (1 hunks)
  • src/main/webapp/app/exercises/programming/manage/programming-exercise-management-routing.module.ts (2 hunks)
  • src/main/webapp/app/exercises/programming/manage/services/programming-exercise-participation.service.ts (1 hunks)
  • src/main/webapp/app/exercises/programming/manage/services/programming-exercise.service.ts (1 hunks)
  • src/main/webapp/app/exercises/programming/shared/code-editor/model/code-editor.model.ts (3 hunks)
  • src/main/webapp/app/exercises/programming/shared/code-editor/service/code-editor-conflict-state.service.ts (1 hunks)
  • src/main/webapp/app/exercises/programming/shared/code-editor/service/code-editor-domain-dependent-endpoint.service.ts (1 hunks)
  • src/main/webapp/app/exercises/programming/shared/code-editor/service/code-editor-domain.service.ts (2 hunks)
  • src/main/webapp/app/localvc/commit-history/commit-history.component.ts (4 hunks)
  • src/main/webapp/app/localvc/repository-view/repository-view.component.ts (3 hunks)
Additional context used
Path-based instructions (16)
src/main/webapp/app/exercises/programming/shared/code-editor/service/code-editor-domain.service.ts (1)

Pattern 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/detail-overview-list/components/programming-auxiliary-repository-buttons-detail/programming-auxiliary-repository-buttons-detail.component.html (1)

Pattern src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.

src/main/webapp/app/exercises/programming/shared/code-editor/service/code-editor-domain-dependent-endpoint.service.ts (1)

Pattern 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/exercises/programming/shared/code-editor/service/code-editor-conflict-state.service.ts (1)

Pattern 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/exercises/programming/shared/code-editor/model/code-editor.model.ts (1)

Pattern 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/detail-overview-list/detail-overview-list.component.html (1)

Pattern src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.

src/main/webapp/app/exercises/programming/manage/services/programming-exercise-participation.service.ts (1)

Pattern 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/localvc/commit-history/commit-history.component.ts (1)

Pattern 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/localvc/repository-view/repository-view.component.ts (1)

Pattern 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/exercises/programming/manage/programming-exercise-management-routing.module.ts (1)

Pattern 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/java/de/tum/cit/aet/artemis/programming/web/repository/AuxiliaryRepositoryResource.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseParticipationResource.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/webapp/app/exercises/programming/manage/services/programming-exercise.service.ts (1)

Pattern 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/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseService.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

Biome
src/main/webapp/app/exercises/programming/shared/code-editor/service/code-editor-conflict-state.service.ts

[error] 70-70: Forbidden non-null assertion.

Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator

(lint/style/noNonNullAssertion)

src/main/webapp/app/localvc/commit-history/commit-history.component.ts

[error] 94-94: Forbidden non-null assertion.

(lint/style/noNonNullAssertion)


[error] 148-148: Forbidden non-null assertion.

(lint/style/noNonNullAssertion)


[error] 161-161: Forbidden non-null assertion.

(lint/style/noNonNullAssertion)

src/main/webapp/app/localvc/repository-view/repository-view.component.ts

[error] 190-190: Forbidden non-null assertion.

(lint/style/noNonNullAssertion)


[error] 194-194: Forbidden non-null assertion.

(lint/style/noNonNullAssertion)

Additional comments not posted (36)
src/main/webapp/app/exercises/programming/shared/code-editor/service/code-editor-domain.service.ts (1)

17-17: Looks good! Verify the impact on subscribers.

The addition of DomainAuxiliaryRepositoryChange to the subject property allows the DomainService to handle auxiliary repository changes, which is consistent with the PR objectives.

Please ensure that the subscribers of subject are updated to handle the new type appropriately.

Run the following script to verify the usage of subject:

Verification successful

Changes are correctly implemented. Subscribers are handling the new type.

The addition of DomainAuxiliaryRepositoryChange to the subject property in DomainService has been properly implemented. The subscribeDomainChange() method returns this updated subject, and other components and services are subscribing to it correctly. The new type is also properly defined in the model file.

Subscribers are set up to handle any type of DomainChange, which now includes DomainAuxiliaryRepositoryChange. They use filtering and type checking to process the domain changes appropriately.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of `subject` property in `DomainService`.

# Test: Search for the subscribers of `subject`. Expect: Subscribers handle `DomainAuxiliaryRepositoryChange` type.
rg --type typescript -A 5 $'subscribeDomainChange\(\)'

Length of output: 90


Script:

#!/bin/bash
# Description: Verify the existence of TypeScript files and usage of `subscribeDomainChange()` method

# Test 1: Check for TypeScript files
echo "Checking for TypeScript files:"
fd -e ts

# Test 2: Search for subscribeDomainChange() method in TypeScript files
echo -e "\nSearching for subscribeDomainChange() method:"
rg -t ts -A 5 'subscribeDomainChange\(\)'

# Test 3: Search for DomainAuxiliaryRepositoryChange usage
echo -e "\nSearching for DomainAuxiliaryRepositoryChange usage:"
rg -t ts 'DomainAuxiliaryRepositoryChange'

Length of output: 256862

src/main/webapp/app/detail-overview-list/components/programming-auxiliary-repository-buttons-detail/programming-auxiliary-repository-buttons-detail.component.html (2)

2-2: LGTM!

Tracking by auxiliaryRepository.id in the @for loop is a good practice that improves performance and ensures efficient change detection in Angular.


6-11: LGTM!

The addition of the [routerLinkForRepositoryView] input property to the jhi-code-button component is a great enhancement that improves user experience by providing a clear navigation path to the repository view.

src/main/webapp/app/exercises/programming/shared/code-editor/service/code-editor-domain-dependent-endpoint.service.ts (1)

38-39: LGTM! Verify the usage of the new endpoint across the codebase.

The addition of the new case for DomainType.AUXILIARY_REPOSITORY in the calculateRestResourceURL method is a valid enhancement that broadens the functionality of the service to accommodate additional repository types. The change is consistent with the existing code structure and follows the same pattern as other cases, improving the versatility and maintainability of the service.

Please ensure that all components or services that consume the generated API endpoints are updated to handle the new AUXILIARY_REPOSITORY domain type correctly.

Run the following script to verify the usage of the new endpoint:

Verification successful

Verified: 'AUXILIARY_REPOSITORY' usage is consistent and well-integrated

The search results confirm that the new AUXILIARY_REPOSITORY case in the code-editor-domain-dependent-endpoint.service.ts file is properly integrated across the codebase. Here's a summary of the findings:

  • TypeScript: The AUXILIARY_REPOSITORY domain type is used in multiple components and services, including the repository view, code editor model, and conflict state service.
  • Java: Backend services and resources have corresponding implementations for handling auxiliary repositories, including proper error handling and validation.
  • Database: The Liquibase changelog indicates that the database schema supports auxiliary repositories.

This consistent usage across different layers of the application (frontend, backend, and database) suggests that the feature has been thoroughly implemented and integrated.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of the new `AUXILIARY_REPOSITORY` endpoint across the codebase.

# Test: Search for the usage of the `AUXILIARY_REPOSITORY` domain type. Expect: Relevant components or services are updated to handle the new domain type.
rg --type typescript $'DomainType.AUXILIARY_REPOSITORY'

Length of output: 89


Script:

#!/bin/bash
# Description: Verify the usage of the new `AUXILIARY_REPOSITORY` endpoint across the codebase.

# Search for 'AUXILIARY_REPOSITORY' in TypeScript files
echo "Searching for 'AUXILIARY_REPOSITORY' in TypeScript files:"
fd -e ts -e tsx | xargs grep -ni 'AUXILIARY_REPOSITORY' || echo "No matches found in TypeScript files."

# Search for 'AUXILIARY_REPOSITORY' in all files (case-insensitive)
echo -e "\nSearching for 'AUXILIARY_REPOSITORY' in all files (case-insensitive):"
grep -Rni 'AUXILIARY_REPOSITORY' . || echo "No matches found in any files."

Length of output: 5258

src/main/webapp/app/exercises/programming/shared/code-editor/model/code-editor.model.ts (3)

5-5: LGTM!

The import statement is correct and necessary to use the AuxiliaryRepository model in this file.


53-53: LGTM!

The new enum value AUXILIARY_REPOSITORY is added correctly to the DomainType enum and follows the naming convention. It is necessary to represent the new domain type for auxiliary repositories.


99-100: Verify the usage of the DomainChange type in the codebase.

The new type alias DomainAuxiliaryRepositoryChange and the updated DomainChange type are implemented correctly and follow the naming conventions. These changes are necessary to represent the changes related to auxiliary repositories in the domain model.

However, the changes to the DomainChange type could affect other parts of the codebase that use this type. Please ensure that all the code that uses DomainChange is updated to handle the new DomainAuxiliaryRepositoryChange type.

Run the following script to verify the usage of the DomainChange type:

Verification successful

Verification successful: DomainAuxiliaryRepositoryChange is handled in the codebase

The codebase has been updated to handle the new DomainAuxiliaryRepositoryChange type. Evidence of this includes:

  • Usage of DomainType.AUXILIARY_REPOSITORY in multiple files, including:
    • A switch statement in code-editor-domain-dependent-endpoint.service.ts
    • An if statement in code-editor-conflict-state.service.ts
    • A method call in repository-view.component.ts

These findings suggest that the necessary updates have been made to accommodate the new type in various parts of the codebase.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all usages of the `DomainChange` type are updated to handle the new `DomainAuxiliaryRepositoryChange` type.

# Test: Search for the type usage. Expect: Code that uses `DomainChange` should handle the new type.
rg --type typescript -A 5 $'DomainChange'

Length of output: 75


Script:

#!/bin/bash
# Description: Search for usage of the DomainChange type in TypeScript files

# Use ast-grep to find TypeScript files that use DomainChange
ast-grep --lang typescript --pattern 'DomainChange'

# Additionally, search for switch statements or if-else chains that might be handling DomainChange
ast-grep --lang typescript --pattern 'switch ($expr) { $$$case DomainType.$_: $$$ }'

Length of output: 2184


Script:

#!/bin/bash
# Description: Search for usage of DomainType.AUXILIARY_REPOSITORY in TypeScript files

# Use ast-grep to find TypeScript files that use DomainType.AUXILIARY_REPOSITORY
ast-grep --lang typescript --pattern 'DomainType.AUXILIARY_REPOSITORY'

# Additionally, search for if statements that might be handling DomainType.AUXILIARY_REPOSITORY
ast-grep --lang typescript --pattern 'if ($expr === DomainType.AUXILIARY_REPOSITORY) { $$$ }'

Length of output: 690

src/main/webapp/app/detail-overview-list/detail-overview-list.component.html (1)

7-7: LGTM!

The change from tracking detail to tracking $index in the for loop aligns with the recommended Angular syntax and best practices. Using $index for tracking can provide the following benefits:

  • Improved performance
  • Enhanced code clarity
  • Easier debugging when the index is needed for further operations

The overall functionality of the component remains intact, and this change is a positive refinement.

src/main/webapp/app/localvc/commit-history/commit-history.component.ts (7)

27-27: LGTM!

The new optional property repositoryId is appropriately typed and named, following the Angular style guide.


57-57: LGTM!

The repositoryId is correctly assigned from the route parameters and converted to a number.


93-94: LGTM!

The code correctly handles the case when the repository type is 'AUXILIARY' by assigning the templateParticipation to the participation property.

Tools
Biome

[error] 94-94: Forbidden non-null assertion.

(lint/style/noNonNullAssertion)


128-140: LGTM!

The refactoring of the handleCommits method improves code readability and maintainability by separating the logic for handling commits based on the repository type. The method names are clear and self-explanatory.


142-152: LGTM!

The new handleParticipationCommits method improves code organization and readability by encapsulating the logic for handling participation commits. The method name is clear and self-explanatory.

Regarding the static analysis hint:

The non-null assertion operator used on participation.id is safe here since the participation property is always assigned before calling this method. The hint can be ignored in this case.

Tools
Biome

[error] 148-148: Forbidden non-null assertion.

(lint/style/noNonNullAssertion)


154-165: LGTM!

The new handleAuxiliaryRepositoryCommits method improves code organization and readability by encapsulating the logic for handling auxiliary repository commits. The method name is clear and self-explanatory.

Regarding the static analysis hint:

The non-null assertion operator used on repositoryId is safe here since the repositoryId property is always assigned before calling this method when the repository type is 'AUXILIARY'. The hint can be ignored in this case.

Tools
Biome

[error] 161-161: Forbidden non-null assertion.

(lint/style/noNonNullAssertion)


167-180: LGTM!

The new handleTemplateSolutionTestRepositoryCommits method improves code organization and readability by encapsulating the logic for handling template, solution, and test repository commits. The method name is clear and self-explanatory.

The check for !this.isTestRepository ensures that commit results are set only for non-test repositories, which is appropriate.

src/main/webapp/app/localvc/repository-view/repository-view.component.ts (1)

45-45: LGTM!

The new property follows the coding style guidelines and has a descriptive name.

src/main/webapp/app/exercises/programming/manage/programming-exercise-management-routing.module.ts (2)

174-185: LGTM!

The new route configuration for the RepositoryViewComponent follows the Angular routing conventions and includes the necessary path parameters for granular access to the auxiliary repository view. The authority checks and cache management metadata ensure security and optimize performance.


198-209: LGTM!

The modification to the existing route configuration for the CommitHistoryComponent enhances the user experience by providing a more granular access to the commit history based on the specific repository. The addition of the repositoryId parameter in the route path achieves this without compromising the security or performance, as the authority checks and cache management metadata remain unchanged.

src/main/java/de/tum/cit/aet/artemis/programming/web/repository/AuxiliaryRepositoryResource.java (11)

63-69: LGTM!

The constructor is using constructor injection to inject the required dependencies, which is a good practice. It is also assigning the auxiliaryRepositoryRepository to a class variable for later use.


72-78: LGTM!

The method is performing the necessary access checks before returning the repository. It is using the repositoryAccessService to check if the user has access to the repository and the gitService to get or checkout the repository.


81-84: LGTM!

The method is simply returning the VcsRepositoryUri of the AuxiliaryRepository.


87-96: LGTM!

The method is performing the necessary access checks to determine if the user can access the repository. It is using the repositoryAccessService to check if the user has access to the repository and catching the AccessForbiddenException to return false if the user does not have access.


109-113: LGTM!

The method is simply delegating the call to the super class method.


116-120: LGTM!

The method is simply delegating the call to the super class method.


123-128: LGTM!

The method is simply delegating the call to the super class method.


131-136: LGTM!

The method is simply delegating the call to the super class method.


139-144: LGTM!

The method is simply delegating the call to the super class method.


147-152: LGTM!

The method is simply delegating the call to the super class method.


193-222: LGTM!

The method is performing the necessary access checks before updating the files. It is using the repositoryAccessService to check if the user has access to the repository and the gitService to retrieve the repository. It is handling various exceptions and throwing appropriate ResponseStatusExceptions. It is delegating the actual file update and commit logic to the saveFilesAndCommitChanges method.

src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseParticipationResource.java (2)

Line range hint 83-101: Approved: Constructor changes to support auxiliary repositories.

The addition of the AuxiliaryRepositoryRepository dependency to the constructor is necessary to support the new functionality related to auxiliary repositories. The dependency is properly injected and stored, maintaining consistency with the existing design and best practices.


Line range hint 319-350: Approved: Extension of getCommitHistoryForTemplateSolutionOrTestRepo to support auxiliary repositories.

The changes to the getCommitHistoryForTemplateSolutionOrTestRepo method correctly extend its functionality to support retrieving commit history for auxiliary repositories. The new repositoryId parameter allows specifying the specific auxiliary repository, and the added logic properly verifies that the repository belongs to the correct programming exercise. The commit information is retrieved using the appropriate service method, maintaining consistency with the existing design.

src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java (1)

527-535: LGTM!

The new getAuxiliaryRepositoryCommitInfos method looks good:

  • It retrieves commit information for the given auxiliary repository using the gitService.
  • Handles exceptions gracefully by catching GitAPIException, logging an error, and returning an empty list to prevent disruption.
  • Expands the service's functionality to support auxiliary repositories.
src/main/webapp/app/exercises/programming/manage/services/programming-exercise.service.ts (1)

286-295: LGTM!

The new findWithAuxiliaryRepository function looks good. It enhances the service's capability to retrieve a programming exercise along with a specific auxiliary repository by making a GET request to the appropriate endpoint.

The function signature, parameter names, and return type are clear and descriptive. The endpoint URL construction is straightforward and follows the expected pattern.

Overall, this addition improves the granularity and flexibility of fetching programming exercises with their associated resources.

src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java (2)

541-543: New endpoint looks good!

The new endpoint allows retrieving a programming exercise with template and solution participation for a given exercise id and auxiliary repository id. The @EnforceAtLeastTutorInExercise annotation ensures proper access control.


545-551: LGTM!

The method implementation is correct:

  • It first loads the programming exercise with auxiliary repositories using the service method.
  • It then validates that the provided auxiliaryRepositoryId belongs to the loaded exercise and throws an appropriate exception if the validation fails.
  • Finally, it returns the loaded programming exercise if the validation passes.
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseService.java (1)

1012-1022: LGTM!

The method correctly loads the programming exercise with the specified fetch options, loads the template and solution participation with the specified fetch options, sets the loaded template and solution participation on the programming exercise, and replaces test IDs with names in the programming exercise.

coderabbitai[bot]
coderabbitai bot previously approved these changes Sep 16, 2024
coderabbitai[bot]
coderabbitai bot previously approved these changes Sep 16, 2024
@SimonEntholzer
Copy link
Contributor Author

I will increase the test coverage some more

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: 13

🧹 Outside diff range and nitpick comments (3)
src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseParticipationIntegrationTest.java (3)

87-90: Address the TODO: Remove programmingExerciseIntegrationTestService

The TODO comment indicates that programmingExerciseIntegrationTestService should be removed after refactoring. Please complete the refactoring to eliminate this dependency and clean up the test code.

Would you like assistance in preparing this refactoring or opening a GitHub issue to track this task?


725-730: Address the TODO: Move tests to the appropriate test file

The TODO comment suggests moving these tests to a different test file since they don't use the programming-exercise-participations/.. endpoint. Organizing tests in the correct files improves code maintainability and clarity.

Would you like assistance in refactoring these tests or opening a GitHub issue to track this task?


806-809: Address the TODO: Refactor endpoint to include participation

The TODO comment indicates that the endpoint should be refactored to programming-exercise-participations. Refactoring according to this suggestion will enhance endpoint consistency and maintainability.

Would you like assistance in planning this refactoring or opening a GitHub issue to track this task?

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between a053f1a and 0b8eec9.

📒 Files selected for processing (3)
  • src/test/java/de/tum/cit/aet/artemis/exercise/programming/AuxiliaryRepositoryResourceIntegrationTest.java (1 hunks)
  • src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseIntegrationTestService.java (1 hunks)
  • src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseParticipationIntegrationTest.java (5 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/test/java/de/tum/cit/aet/artemis/exercise/programming/AuxiliaryRepositoryResourceIntegrationTest.java (1)

Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true

src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseIntegrationTestService.java (1)

Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true

src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseParticipationIntegrationTest.java (1)

Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true

🔇 Additional comments (1)
src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseIntegrationTestService.java (1)

2191-2197: New method added to add an auxiliary repository to an exercise.

This new method addAuxiliaryRepositoryToExercise adds functionality to associate an auxiliary repository with a programming exercise. It creates a default auxiliary repository, saves it, and then adds it to the given exercise.

A few suggestions for improvement:

  1. Consider adding a JavaDoc comment to explain the method's purpose and parameters.
  2. The method could be more flexible by allowing custom auxiliary repository properties to be passed as parameters.
  3. Consider adding error handling or validation for the case where the exercise already has auxiliary repositories.

@krusche krusche merged commit 34f4410 into develop Oct 19, 2024
26 of 28 checks passed
@krusche krusche deleted the feature/add-auxiliary-repository-view branch October 19, 2024 07:43
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!) component:Programming exercise Pull requests that affect the corresponding module programming Pull requests that affect the corresponding module ready for review server Pull requests that update Java code. (Added Automatically!) tests
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.