Integrated code lifecycle: Add auxiliary repository view
#9321
Conversation
WalkthroughThe changes introduce a new Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
github-actions
bot
added
server
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
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)
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)
src/main/webapp/app/exercises/programming/shared/code-editor/service/code-editor-conflict-state.service.ts (1)
src/main/webapp/app/exercises/programming/shared/code-editor/model/code-editor.model.ts (1)
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)
src/main/webapp/app/localvc/commit-history/commit-history.component.ts (1)
src/main/webapp/app/localvc/repository-view/repository-view.component.ts (1)
src/main/webapp/app/exercises/programming/manage/programming-exercise-management-routing.module.ts (1)
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_importssrc/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_importssrc/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_importssrc/main/webapp/app/exercises/programming/manage/services/programming-exercise.service.ts (1)
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_importssrc/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
DomainAuxiliaryRepositoryChangeto thesubjectproperty type allows theDomainServiceto 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 oftrack byexpression!Using
auxiliaryRepository.idfor tracking in the@forloop 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 thejhi-code-buttoncomponent 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_REPOSITORYfollows 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
AuxiliaryRepositoryis correctly added and follows the project's directory structure.
53-53: LGTM!The new
AUXILIARY_REPOSITORYvalue is correctly added to theDomainTypeenum and follows the naming convention of the existing values.
99-100: LGTM!The
DomainAuxiliaryRepositoryChangetype alias is correctly defined, and theDomainChangetype 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
@forloop to track by$indexinstead of thedetailobject 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
@forsyntax instead of the old*ngForstyle 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
repositoryIdproperty 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
repositoryIdfrom the route parameters and assigning it to the component property is a logical way to obtain the repository ID. The type conversion usingNumberis appropriate.
93-94: LGTM!The addition of the
else ifblock to handle the 'AUXILIARY' repository type is a necessary change to support auxiliary repositories. Assigning thetemplateParticipationto theparticipationproperty for auxiliary repositories seems appropriate.Tools
Biome
[error] 94-94: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
128-140: LGTM!The refactoring of the
handleCommitsmethod to handle commits based on therepositoryTypeis 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
handleParticipationCommitsmethod 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
handleAuxiliaryRepositoryCommitsmethod is a good practice to improve code organization and readability. Utilizing therepositoryIdto fetch the relevant commit history for auxiliary repositories aligns with the purpose of introducing therepositoryIdproperty.Tools
Biome
[error] 161-161: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
167-180: LGTM!The introduction of the
handleTemplateSolutionTestRepositoryCommitsmethod 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
handleCommitsmethod and the introduction of separate handler methods for different repository types greatly improve code modularity, readability, and maintainability. The addition of therepositoryIdproperty 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
auxiliaryRepositoryRepositoryis being set correctly.
72-78: LGTM!The method is correctly retrieving the
AuxiliaryRepository, checking user access, and returning the repository using thegitService. The logic and syntax look good.
81-84: LGTM!The method is correctly retrieving the
AuxiliaryRepositoryand returning itsVcsRepositoryUri. 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 theAccessForbiddenExceptionappropriately and returns the correct boolean value. The logic and syntax look good.
109-113: LGTM!The method is correctly calling the
getFilesmethod of the super class with the providedauxiliaryRepositoryId. The logic and syntax look good.
116-120: LGTM!The method is correctly calling the
getFilemethod of the super class with the providedauxiliaryRepositoryIdandfilename. The logic and syntax look good.
123-128: LGTM!The method is correctly calling the
createFilemethod of the super class with the providedauxiliaryRepositoryId,filePath, andrequest. The logic and syntax look good.
131-136: LGTM!The method is correctly calling the
createFoldermethod of the super class with the providedauxiliaryRepositoryId,folderPath, andrequest. The logic and syntax look good.
139-144: LGTM!The method is correctly calling the
renameFilemethod of the super class with the providedauxiliaryRepositoryIdandfileMove. The logic and syntax look good.
147-152: LGTM!The method is correctly calling the
deleteFilemethod of the super class with the providedauxiliaryRepositoryIdandfilename. The logic and syntax look good.
155-159: LGTM!The method is correctly calling the
pullChangesmethod of the super class with the providedauxiliaryRepositoryId. The logic and syntax look good.
162-167: LGTM!The method is correctly calling the
commitChangesmethod of the super class with the providedauxiliaryRepositoryId. The logic and syntax look good.
170-175: LGTM!The method is correctly calling the
resetToLastCommitmethod of the super class with the providedauxiliaryRepositoryId. 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
auxiliaryRepositoryRepositoryfield addition. The dependency injection of theAuxiliaryRepositoryRepositorywill 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
repositoryTypeisAUXILIARY:
- 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
getAuxiliaryRepositoryCommitInfosservice 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 newrepositoryIdparameter.Please ensure that the API clients have been updated to provide the
repositoryIdparameter 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
findWithAuxiliaryRepositoryfunction 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
@GetMappingannotation defines a clear endpoint for retrieving a programming exercise with a specific auxiliary repository. The@EnforceAtLeastTutorInExerciseannotation ensures proper access control.
547-550: Validation logic and error handling are appropriate.The function correctly validates that the provided
auxiliaryRepositoryIdbelongs to the exercise's list of auxiliary repositories. If validation fails, it throws aBadRequestAlertExceptionwith 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.
...p/app/exercises/programming/shared/code-editor/service/code-editor-conflict-state.service.ts
Show resolved
Hide resolved
...bapp/app/exercises/programming/manage/services/programming-exercise-participation.service.ts
Outdated
Show resolved
Hide resolved
...bapp/app/exercises/programming/manage/services/programming-exercise-participation.service.ts
Outdated
Show resolved
Hide resolved
...bapp/app/exercises/programming/manage/services/programming-exercise-participation.service.ts
Outdated
Show resolved
Hide resolved
src/main/webapp/app/localvc/repository-view/repository-view.component.ts
Outdated
Show resolved
Hide resolved
...main/java/de/tum/cit/aet/artemis/programming/web/repository/AuxiliaryRepositoryResource.java
Outdated
Show resolved
Hide resolved
...java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseParticipationService.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 7
Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
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)
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)
src/main/webapp/app/exercises/programming/shared/code-editor/service/code-editor-conflict-state.service.ts (1)
src/main/webapp/app/exercises/programming/shared/code-editor/model/code-editor.model.ts (1)
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)
src/main/webapp/app/localvc/commit-history/commit-history.component.ts (1)
src/main/webapp/app/localvc/repository-view/repository-view.component.ts (1)
src/main/webapp/app/exercises/programming/manage/programming-exercise-management-routing.module.ts (1)
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_importssrc/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_importssrc/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_importssrc/main/webapp/app/exercises/programming/manage/services/programming-exercise.service.ts (1)
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_importssrc/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
DomainAuxiliaryRepositoryChangeto thesubjectproperty allows theDomainServiceto handle auxiliary repository changes, which is consistent with the PR objectives.Please ensure that the subscribers of
subjectare 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
DomainAuxiliaryRepositoryChangeto thesubjectproperty inDomainServicehas been properly implemented. ThesubscribeDomainChange()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 includesDomainAuxiliaryRepositoryChange. 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.idin the@forloop 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 thejhi-code-buttoncomponent 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_REPOSITORYin thecalculateRestResourceURLmethod 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_REPOSITORYdomain 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_REPOSITORYcase in thecode-editor-domain-dependent-endpoint.service.tsfile is properly integrated across the codebase. Here's a summary of the findings:
- TypeScript: The
AUXILIARY_REPOSITORYdomain 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
AuxiliaryRepositorymodel in this file.
53-53: LGTM!The new enum value
AUXILIARY_REPOSITORYis added correctly to theDomainTypeenum and follows the naming convention. It is necessary to represent the new domain type for auxiliary repositories.
99-100: Verify the usage of theDomainChangetype in the codebase.The new type alias
DomainAuxiliaryRepositoryChangeand the updatedDomainChangetype 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
DomainChangetype could affect other parts of the codebase that use this type. Please ensure that all the code that usesDomainChangeis updated to handle the newDomainAuxiliaryRepositoryChangetype.Run the following script to verify the usage of the
DomainChangetype:Verification successful
Verification successful: DomainAuxiliaryRepositoryChange is handled in the codebase
The codebase has been updated to handle the new
DomainAuxiliaryRepositoryChangetype. Evidence of this includes:
- Usage of
DomainType.AUXILIARY_REPOSITORYin 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.tsThese 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
detailto tracking$indexin theforloop aligns with the recommended Angular syntax and best practices. Using$indexfor 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
repositoryIdis appropriately typed and named, following the Angular style guide.
57-57: LGTM!The
repositoryIdis 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
templateParticipationto theparticipationproperty.Tools
Biome
[error] 94-94: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
128-140: LGTM!The refactoring of the
handleCommitsmethod 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
handleParticipationCommitsmethod 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.idis safe here since theparticipationproperty 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
handleAuxiliaryRepositoryCommitsmethod 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
repositoryIdis safe here since therepositoryIdproperty 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
handleTemplateSolutionTestRepositoryCommitsmethod 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.isTestRepositoryensures 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
RepositoryViewComponentfollows 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
CommitHistoryComponentenhances the user experience by providing a more granular access to the commit history based on the specific repository. The addition of therepositoryIdparameter 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
auxiliaryRepositoryRepositoryto 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
repositoryAccessServiceto check if the user has access to the repository and thegitServiceto get or checkout the repository.
81-84: LGTM!The method is simply returning the
VcsRepositoryUriof theAuxiliaryRepository.
87-96: LGTM!The method is performing the necessary access checks to determine if the user can access the repository. It is using the
repositoryAccessServiceto check if the user has access to the repository and catching theAccessForbiddenExceptionto returnfalseif 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
repositoryAccessServiceto check if the user has access to the repository and thegitServiceto retrieve the repository. It is handling various exceptions and throwing appropriateResponseStatusExceptions. It is delegating the actual file update and commit logic to thesaveFilesAndCommitChangesmethod.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
AuxiliaryRepositoryRepositorydependency 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 ofgetCommitHistoryForTemplateSolutionOrTestRepoto support auxiliary repositories.The changes to the
getCommitHistoryForTemplateSolutionOrTestRepomethod correctly extend its functionality to support retrieving commit history for auxiliary repositories. The newrepositoryIdparameter 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
getAuxiliaryRepositoryCommitInfosmethod 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
findWithAuxiliaryRepositoryfunction 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
@EnforceAtLeastTutorInExerciseannotation 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
auxiliaryRepositoryIdbelongs 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.
...p/app/exercises/programming/shared/code-editor/service/code-editor-conflict-state.service.ts
Show resolved
Hide resolved
...bapp/app/exercises/programming/manage/services/programming-exercise-participation.service.ts
Outdated
Show resolved
Hide resolved
...bapp/app/exercises/programming/manage/services/programming-exercise-participation.service.ts
Outdated
Show resolved
Hide resolved
src/main/webapp/app/localvc/repository-view/repository-view.component.ts
Outdated
Show resolved
Hide resolved
...main/java/de/tum/cit/aet/artemis/programming/web/repository/AuxiliaryRepositoryResource.java
Outdated
Show resolved
Hide resolved
SimonEntholzer
requested review from
Hialus,
eceeeren,
JohannesStoehr,
pzdr7,
DominikRemo,
PaRangger,
BBesrour,
b-fein,
cremertim,
bensofficial,
EneaGore,
janthoXO and
Jan-Thurner
|
This PR is open for a long time. Let's please get it |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
📒 Files selected for processing (2)
- src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseService.java (1 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
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_importssrc/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/web/ProgrammingExerciseResource.java
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseService.java
Show resolved
Hide resolved
SimonEntholzer
temporarily deployed
to
artemis-test2.artemis.cit.tum.de
GitHub Actions
Inactive
BBesrour
temporarily deployed
to
artemis-test2.artemis.cit.tum.de
GitHub Actions
Inactive
Checklist
General
Server
Client
authoritiesto all new routes and checked the course groups for displaying navigation elements (links, buttons).Changes affecting Programming Exercises
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:
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
Manual Tests
Test Coverage
Screenshots
Summary by CodeRabbit
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Improvements
Documentation