Programming exercises: Improve repository view routing
#10249
Conversation
github-actions
bot
added
tests
client
WalkthroughThis pull request refactors the codebase to replace hardcoded repository type strings and the legacy Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant C as ProgrammingExamSummaryComponent
participant R as Router
participant V as Repository View
U->>C: Click repository button
alt isInCourseManagement TRUE
C->>C: Build route with RepositoryType.USER segment
else isInCourseManagement FALSE
C->>C: Build default route without course management override
end
C->>R: Navigate using constructed route
R->>V: Load the appropriate repository view
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used📓 Path-based instructions (1)`src/main/webapp/**/*.ts`: angular_style:https://angular.io/...
🧠 Learnings (2)src/main/webapp/app/exercises/programming/participate/programming-repository.route.ts (1)src/main/webapp/app/exercises/programming/manage/programming-exercise-management.route.ts (1)⏰ Context from checks skipped due to timeout of 90000ms (8)
🔇 Additional comments (3)
✨ Finishing Touches
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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🔭 Outside diff range comments (3)
src/main/webapp/app/localvc/vcs-repository-access-log-view/vcs-repository-access-log-view.component.ts (1)
23-29: Rename misleading property to match its new purpose.The computed property is named
participationIdbut actually represents a repository ID. This could be confusing for maintainers.Consider this refactoring:
- private readonly participationId = computed(() => { - const participationId = this.params().repositoryId; - if (participationId) { - return Number(participationId); + private readonly repositoryId = computed(() => { + const repoId = this.params().repositoryId; + if (repoId) { + return Number(repoId); } return undefined; });Then update the references in the effect:
- if (this.participationId()) { - await this.loadVcsAccessLogForParticipation(this.participationId()!); + if (this.repositoryId()) { + await this.loadVcsAccessLogForParticipation(this.repositoryId()!);src/test/javascript/spec/service/orion/orion-connector.service.spec.ts (1)
193-196: Update test to use the newRepositoryTypeenum.The test is still using the old
REPOSITORY.SOLUTIONconstant. Update it to use the newRepositoryTypeenum for consistency with the changes in the service.Apply this diff to fix the test:
- serviceUnderTest.selectRepository(REPOSITORY.SOLUTION); + serviceUnderTest.selectRepository(RepositoryType.SOLUTION); - expect((window as any).orionVCSConnector.selectRepository).toHaveBeenCalledWith(REPOSITORY.SOLUTION); + expect((window as any).orionVCSConnector.selectRepository).toHaveBeenCalledWith(RepositoryType.SOLUTION);src/test/javascript/spec/component/orion/orion-code-editor-instructor-and-editor-container.component.spec.ts (1)
105-105: Update selectedRepository assignment.The test is still using the old
REPOSITORYenum.-comp.selectedRepository = REPOSITORY.SOLUTION; +comp.selectedRepository = RepositoryType.SOLUTION;🧰 Tools
🪛 GitHub Check: client-tests-selected
[failure] 105-105:
Cannot find name 'REPOSITORY'.🪛 GitHub Check: client-tests
[failure] 105-105:
Cannot find name 'REPOSITORY'.
🧹 Nitpick comments (13)
src/main/webapp/app/exercises/programming/shared/code-editor/model/code-editor.model.ts (1)
56-66: Enhance documentation for the RepositoryType enum.The enum is well-structured, but the important note about
ASSIGNMENTbeing an instructor's USER repository is hidden in a comment. Consider moving this information to the enum's JSDoc description for better visibility./** * Enumeration specifying the repository type + * + * Repository types: + * - ASSIGNMENT: The USER repository of an instructor for an exercise + * - TEMPLATE: Template repository containing the exercise template + * - SOLUTION: Solution repository containing the exercise solution + * - TESTS: Test repository containing the exercise tests + * - AUXILIARY: Auxiliary repository for additional resources + * - USER: User repository for student submissions */ export enum RepositoryType { - ASSIGNMENT = 'ASSIGNMENT', // The ASSIGNMENT repository is the USER repository of an instructor for an exercise + ASSIGNMENT = 'ASSIGNMENT', TEMPLATE = 'TEMPLATE', SOLUTION = 'SOLUTION', TESTS = 'TESTS', AUXILIARY = 'AUXILIARY', USER = 'USER', }src/main/webapp/app/orion/management/code-editor-instructor-and-editor-orion-container.component.ts (1)
44-45: Remove redundant REPOSITORY constant.The
REPOSITORYconstant is just an alias forRepositoryType. Consider usingRepositoryTypedirectly throughout the component for better clarity.- REPOSITORY = RepositoryType;Then update the usage in the template file to use
RepositoryTypedirectly.src/main/webapp/app/exam/manage/exercise-groups/programming-exercise-cell/programming-exercise-group-cell.component.ts (1)
76-83: Enhance method documentation.The JSDoc comment should include information about the
repositoryTypeparameter and its allowed values./** * Downloads the instructor repository. Used when the "localvc" profile is active. * For the local VCS, linking to an external site displaying the repository does not work. * Instead, the repository is downloaded. * - * - * @param repositoryType + * @param repositoryType The type of repository to download (ASSIGNMENT, TEMPLATE, SOLUTION, TESTS). + * Note: AUXILIARY repositories are not supported for local VCS. */src/main/webapp/app/exam/participate/summary/exercises/programming-exam-summary/programming-exam-summary.component.ts (4)
76-76: Consider using a more robust URL check.The current implementation using
includescould lead to false positives if 'course-management' appears elsewhere in the URL.Consider using a more specific check:
-this.isInCourseManagement = this.router.url.includes('course-management'); +this.isInCourseManagement = this.router.url.startsWith('/course-management') || this.router.url.includes('/course-management/');
70-70: Consider using a service for route context.The boolean flag for course management context could be moved to a shared service for better reusability across components.
76-76: Consider using Angular Router events.Instead of checking the URL directly, consider using Router events to react to route changes:
-this.isInCourseManagement = this.router.url.includes('course-management'); +this.router.events.pipe( + filter((event): event is NavigationEnd => event instanceof NavigationEnd) +).subscribe((event) => { + this.isInCourseManagement = event.url.includes('course-management'); +});
70-70: Consider using Angular's route data or guards for context detection.Using URL string includes for context detection is fragile and could break if URL patterns change. Consider using Angular's route data or guards to determine the context.
-isInCourseManagement = false; +isInCourseManagement = false; + +constructor(private activatedRoute: ActivatedRoute) { + this.isInCourseManagement = this.activatedRoute.snapshot.data['context'] === 'course-management'; +}src/main/webapp/app/shared/orion/orion-connector.service.ts (1)
219-227: Update method documentation to reflect the new type.The method documentation should be updated to reference
RepositoryTypeinstead of "instructor repository".Apply this diff to update the documentation:
/** * Selects an instructor repository in the IDE. The selected repository will be used for all future actions * that reference an instructor repo s.a. submitting the code. * - * @param repository The repository to be selected for all future interactions + * @param repository The repository type (from RepositoryType enum) to be selected for all future interactions */src/main/webapp/app/localvc/repository-view/repository-view.component.ts (1)
159-168: Consider extracting common error handling logic.The error handling logic is duplicated across multiple methods. Consider extracting it into a reusable method to improve maintainability and reduce code duplication.
Create a new private method:
private handleLoadError() { this.participationCouldNotBeFetched = true; this.loadingParticipation = false; }Then update the error handlers:
- error: () => { - this.participationCouldNotBeFetched = true; - this.loadingParticipation = false; - }, + error: () => this.handleLoadError(),Also applies to: 228-235
src/main/webapp/app/exercises/programming/manage/programming-exercise.component.ts (1)
248-264: Update method documentation to reflect the new type.The method documentation should be updated to reference
RepositoryTypeinstead of "instructor repository". Also, the comment about 'AUXILIARY' should be updated to use the enum value.Apply this diff to update the documentation:
/** * Downloads the instructor repository. Used when the "localvc" profile is active. * For the local VCS, linking to an external site displaying the repository does not work. * Instead, the repository is downloaded. * * @param programmingExerciseId - * @param repositoryType + * @param repositoryType The type of repository to download (from RepositoryType enum) */ downloadRepository(programmingExerciseId: number | undefined, repositoryType: RepositoryType) { if (programmingExerciseId) { - // Repository type cannot be 'AUXILIARY' as auxiliary repositories are currently not supported for the local VCS. + // Repository type cannot be RepositoryType.AUXILIARY as auxiliary repositories are currently not supported for the local VCS.src/main/webapp/app/exercises/programming/shared/code-editor/actions/code-editor-actions.component.ts (1)
96-105: Consider using route parameter types for better type safety.The route parameter handling could benefit from stronger typing to prevent potential runtime errors.
- this.route.params.subscribe((params) => { - const repositoryType = params['repositoryType'] ?? 'USER'; - const courseId = Number(params['courseId']); - const repositoryId = Number(params['repositoryId']); - const exerciseId = Number(params['exerciseId']); - const examId = Number(params['examId']); - const exerciseGroupId = Number(params['exerciseGroupId']); + interface RouteParams { + repositoryType: string; + courseId: string; + repositoryId: string; + exerciseId: string; + examId: string; + exerciseGroupId: string; + } + this.route.params.subscribe((params: Partial<RouteParams>) => { + const repositoryType = params.repositoryType ?? 'USER'; + const courseId = params.courseId ? Number(params.courseId) : undefined; + const repositoryId = params.repositoryId ? Number(params.repositoryId) : undefined; + const exerciseId = params.exerciseId ? Number(params.exerciseId) : undefined; + const examId = params.examId ? Number(params.examId) : undefined; + const exerciseGroupId = params.exerciseGroupId ? Number(params.exerciseGroupId) : undefined;src/main/webapp/app/overview/exercise-details/exercise-details-student-actions.component.ts (1)
94-95: Consider removing unused variables.The variables
routerLinkandrepositoryLinkappear to be unused after the router logic removal.- routerLink: string; - repositoryLink: string;src/main/webapp/app/exercises/programming/manage/programming-exercise.component.html (1)
90-132: Redundant routerLink Attribute in Repository Download Links
Multiple anchor tags trigger the download action via the(click)event while specifying an empty[routerLink]. If navigation isn’t intended here, consider either removing the[routerLink]attribute or switching to a<button>element. This small refactor could improve semantic correctness and accessibility.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (46)
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/components/programming-auxiliary-repository-buttons-detail/programming-auxiliary-repository-buttons-detail.component.ts(2 hunks)src/main/webapp/app/detail-overview-list/detail.model.ts(2 hunks)src/main/webapp/app/exam/manage/exam-management.route.ts(3 hunks)src/main/webapp/app/exam/manage/exercise-groups/programming-exercise-cell/programming-exercise-group-cell.component.html(3 hunks)src/main/webapp/app/exam/manage/exercise-groups/programming-exercise-cell/programming-exercise-group-cell.component.ts(4 hunks)src/main/webapp/app/exam/participate/summary/exercises/programming-exam-summary/programming-exam-summary.component.html(1 hunks)src/main/webapp/app/exam/participate/summary/exercises/programming-exam-summary/programming-exam-summary.component.ts(1 hunks)src/main/webapp/app/exercises/programming/assess/code-editor-tutor-assessment-container.component.ts(1 hunks)src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-instructor-and-editor-container.component.html(6 hunks)src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-instructor-and-editor-container.component.ts(2 hunks)src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-instructor-base-container.component.ts(8 hunks)src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-management-routing.module.ts(2 hunks)src/main/webapp/app/exercises/programming/manage/programming-exercise-detail.component.html(1 hunks)src/main/webapp/app/exercises/programming/manage/programming-exercise-detail.component.ts(5 hunks)src/main/webapp/app/exercises/programming/manage/programming-exercise-management-routing.module.ts(2 hunks)src/main/webapp/app/exercises/programming/manage/programming-exercise.component.html(3 hunks)src/main/webapp/app/exercises/programming/manage/programming-exercise.component.ts(4 hunks)src/main/webapp/app/exercises/programming/manage/services/programming-exercise.service.ts(2 hunks)src/main/webapp/app/exercises/programming/participate/programming-repository-routing.module.ts(3 hunks)src/main/webapp/app/exercises/programming/shared/actions/programming-exercise-instructor-repo-download.component.ts(2 hunks)src/main/webapp/app/exercises/programming/shared/code-editor/actions/code-editor-actions.component.html(1 hunks)src/main/webapp/app/exercises/programming/shared/code-editor/actions/code-editor-actions.component.ts(5 hunks)src/main/webapp/app/exercises/programming/shared/code-editor/model/code-editor.model.ts(1 hunks)src/main/webapp/app/exercises/shared/exercise-scores/exercise-scores.component.html(1 hunks)src/main/webapp/app/exercises/shared/participation/participation.component.html(1 hunks)src/main/webapp/app/localvc/commit-details-view/commit-details-view.component.ts(7 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(5 hunks)src/main/webapp/app/localvc/vcs-repository-access-log-view/vcs-repository-access-log-view.component.ts(1 hunks)src/main/webapp/app/orion/management/code-editor-instructor-and-editor-orion-container.component.html(2 hunks)src/main/webapp/app/orion/management/code-editor-instructor-and-editor-orion-container.component.ts(4 hunks)src/main/webapp/app/overview/exercise-details/exercise-details-student-actions.component.html(1 hunks)src/main/webapp/app/overview/exercise-details/exercise-details-student-actions.component.ts(1 hunks)src/main/webapp/app/shared/orion/orion-connector.service.ts(2 hunks)src/main/webapp/app/shared/orion/orion.ts(2 hunks)src/main/webapp/app/utils/navigation.utils.ts(1 hunks)src/test/javascript/spec/component/localvc/commit-details-view.component.spec.ts(9 hunks)src/test/javascript/spec/component/localvc/commit-history.component.spec.ts(3 hunks)src/test/javascript/spec/component/localvc/repository-view.component.spec.ts(2 hunks)src/test/javascript/spec/component/localvc/vcs-repository-access-log-view.component.spec.ts(1 hunks)src/test/javascript/spec/component/orion/orion-code-editor-instructor-and-editor-container.component.spec.ts(1 hunks)src/test/javascript/spec/component/overview/exercise-details/exercise-details-student-actions.component.spec.ts(0 hunks)src/test/javascript/spec/component/programming-exercise/programming-exercise-instructor-repo-download-component.spec.ts(1 hunks)src/test/javascript/spec/integration/code-editor/code-editor-instructor.integration.spec.ts(5 hunks)src/test/javascript/spec/service/orion/orion-connector.service.spec.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/test/javascript/spec/component/overview/exercise-details/exercise-details-student-actions.component.spec.ts
🧰 Additional context used
📓 Path-based instructions (45)
src/main/webapp/app/detail-overview-list/components/programming-auxiliary-repository-buttons-detail/programming-auxiliary-repository-buttons-detail.component.ts (1)
src/test/javascript/spec/component/programming-exercise/programming-exercise-instructor-repo-download-component.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/main/webapp/app/exercises/programming/shared/code-editor/actions/code-editor-actions.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/test/javascript/spec/service/orion/orion-connector.service.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/main/webapp/app/detail-overview-list/detail.model.ts (1)
src/main/webapp/app/exercises/programming/manage/programming-exercise-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/participate/programming-repository-routing.module.ts (1)
src/main/webapp/app/localvc/vcs-repository-access-log-view/vcs-repository-access-log-view.component.ts (1)
src/main/webapp/app/exercises/programming/shared/actions/programming-exercise-instructor-repo-download.component.ts (1)
src/main/webapp/app/exercises/shared/exercise-scores/exercise-scores.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/assess/code-editor-tutor-assessment-container.component.ts (1)
src/main/webapp/app/exercises/programming/shared/code-editor/model/code-editor.model.ts (1)
src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-instructor-and-editor-container.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/test/javascript/spec/component/orion/orion-code-editor-instructor-and-editor-container.component.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/main/webapp/app/exam/participate/summary/exercises/programming-exam-summary/programming-exam-summary.component.ts (1)
src/main/webapp/app/orion/management/code-editor-instructor-and-editor-orion-container.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/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/orion/management/code-editor-instructor-and-editor-orion-container.component.ts (1)
src/main/webapp/app/exercises/programming/manage/programming-exercise.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/code-editor/code-editor-instructor-and-editor-container.component.ts (1)
src/main/webapp/app/shared/orion/orion-connector.service.ts (1)
src/main/webapp/app/overview/exercise-details/exercise-details-student-actions.component.ts (1)
src/main/webapp/app/exercises/programming/manage/programming-exercise.component.ts (1)
src/test/javascript/spec/component/localvc/repository-view.component.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/test/javascript/spec/component/localvc/commit-history.component.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/main/webapp/app/utils/navigation.utils.ts (1)
src/main/webapp/app/exercises/programming/shared/code-editor/actions/code-editor-actions.component.ts (1)
src/test/javascript/spec/component/localvc/commit-details-view.component.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/main/webapp/app/exercises/programming/manage/services/programming-exercise.service.ts (1)
src/test/javascript/spec/component/localvc/vcs-repository-access-log-view.component.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/test/javascript/spec/integration/code-editor/code-editor-instructor.integration.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-management-routing.module.ts (1)
src/main/webapp/app/localvc/commit-history/commit-history.component.ts (1)
src/main/webapp/app/exam/manage/exercise-groups/programming-exercise-cell/programming-exercise-group-cell.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/shared/participation/participation.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/localvc/commit-details-view/commit-details-view.component.ts (1)
src/main/webapp/app/overview/exercise-details/exercise-details-student-actions.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/programming-exercise-detail.component.ts (1)
src/main/webapp/app/exam/manage/exam-management.route.ts (1)
src/main/webapp/app/exam/manage/exercise-groups/programming-exercise-cell/programming-exercise-group-cell.component.ts (1)
src/main/webapp/app/exercises/programming/manage/programming-exercise-management-routing.module.ts (1)
src/main/webapp/app/exam/participate/summary/exercises/programming-exam-summary/programming-exam-summary.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/shared/orion/orion.ts (1)
src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-instructor-base-container.component.ts (1)
src/main/webapp/app/localvc/repository-view/repository-view.component.ts (1)
📓 Learnings (4)
src/main/webapp/app/exercises/programming/participate/programming-repository-routing.module.ts (1)
Learnt from: SimonEntholzer
PR: ls1intum/Artemis#10228
File: src/main/webapp/app/exercises/programming/participate/programming-repository-routing.module.ts:9-9
Timestamp: 2025-01-29T08:21:07.083Z
Learning: The programming repository routing module uses `:repositoryId` parameter which is correctly handled by the localvc components (commit-details-view, commit-history, and repository-view components).
src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-management-routing.module.ts (1)
Learnt from: SimonEntholzer
PR: ls1intum/Artemis#10228
File: src/main/webapp/app/exercises/programming/participate/programming-repository-routing.module.ts:9-9
Timestamp: 2025-01-29T08:21:07.083Z
Learning: The programming repository routing module uses `:repositoryId` parameter which is correctly handled by the localvc components (commit-details-view, commit-history, and repository-view components).
src/main/webapp/app/exam/manage/exam-management.route.ts (1)
Learnt from: SimonEntholzer
PR: ls1intum/Artemis#10228
File: src/main/webapp/app/exercises/programming/participate/programming-repository-routing.module.ts:9-9
Timestamp: 2025-01-29T08:21:07.083Z
Learning: The programming repository routing module uses `:repositoryId` parameter which is correctly handled by the localvc components (commit-details-view, commit-history, and repository-view components).
src/main/webapp/app/exercises/programming/manage/programming-exercise-management-routing.module.ts (1)
Learnt from: SimonEntholzer
PR: ls1intum/Artemis#10228
File: src/main/webapp/app/exercises/programming/participate/programming-repository-routing.module.ts:9-9
Timestamp: 2025-01-29T08:21:07.083Z
Learning: The programming repository routing module uses `:repositoryId` parameter which is correctly handled by the localvc components (commit-details-view, commit-history, and repository-view components).
🪛 GitHub Check: client-tests-selected
src/test/javascript/spec/component/orion/orion-code-editor-instructor-and-editor-container.component.spec.ts
[failure] 7-7:
'RepositoryType' is declared but its value is never read.
[failure] 7-7:
Module '"app/exercises/programming/manage/code-editor/code-editor-instructor-base-container.component"' declares 'RepositoryType' locally, but it is not exported.
src/test/javascript/spec/integration/code-editor/code-editor-instructor.integration.spec.ts
[failure] 69-69:
'RepositoryType' is declared but its value is never read.
[failure] 69-69:
Module '"app/exercises/programming/manage/code-editor/code-editor-instructor-base-container.component"' declares 'RepositoryType' locally, but it is not exported.
🪛 GitHub Check: client-tests
src/test/javascript/spec/component/orion/orion-code-editor-instructor-and-editor-container.component.spec.ts
[failure] 7-7:
'RepositoryType' is declared but its value is never read.
[failure] 7-7:
Module '"app/exercises/programming/manage/code-editor/code-editor-instructor-base-container.component"' declares 'RepositoryType' locally, but it is not exported.
src/test/javascript/spec/integration/code-editor/code-editor-instructor.integration.spec.ts
[failure] 69-69:
'RepositoryType' is declared but its value is never read.
[failure] 69-69:
Module '"app/exercises/programming/manage/code-editor/code-editor-instructor-base-container.component"' declares 'RepositoryType' locally, but it is not exported.
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: server-tests
- GitHub Check: Analyse
🔇 Additional comments (70)
src/main/webapp/app/localvc/vcs-repository-access-log-view/vcs-repository-access-log-view.component.ts (3)
1-14: LGTM! Well-structured component setup.The component follows Angular best practices with proper imports and standalone component setup.
16-19: LGTM! Modern dependency injection pattern.Excellent use of the new inject() function for dependency injection.
33-61: LGTM! Excellent implementation of async operations and error handling.The code demonstrates several good practices:
- Proper use of effects for reactive updates
- Robust error handling with translated messages
- Clean separation of concerns in methods
- Memory-safe signal updates
src/main/webapp/app/exam/manage/exam-management.route.ts (4)
586-587: Adopt consistent repository type routing approach.
Excellent job referencing the samerepositorySubRoutesstructure for exam-based exercises. This fosters maintainability.
277-278: Validate route parameter usage for exam-based repository paths.
The introduction of:repositoryTypealigns well with the new repository sub-routes. Ensure that all invocations conform to this parameter to avoid potential runtime mismatches.✅ Verification successful
Consistent Usage of
:repositoryTypeAcross Repository Routes Confirmed
- All occurrences of
:repositoryTypein exam-based repository paths (including routes with and without:repositoryId) are consistent with its usage in other parts of the codebase.- The exam management routes in
src/main/webapp/app/exam/manage/exam-management.route.tsfollow the same pattern as in the exercise management modules.- No discrepancies were found that suggest potential runtime mismatches.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify usage of the :repositoryType parameter across the codebase rg ":repositoryType" -A 5Length of output: 6537
281-282: Confirm no collision between routes.
When adding:repositoryIdafter:repositoryType, verify no overshadowing of sibling routes occurs. Angular's first-match-wins pattern can sometimes cause unexpected behavior.✅ Verification successful
No route collision detected for the repository dynamic segments.
The search shows that the ":repositoryType/:repositoryId" pattern appears consistently in different modules, with each instance being part of a clearly scoped route (with different parent segments). There is no evidence of sibling routes in conflict or unintended shadowing.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if route definitions might overshadow each other rg "path: '.*:repositoryType.*:repositoryId'" -A 3Length of output: 2121
590-591: Ensure consistent route ordering for multi-parameter paths.
If:repositoryIdis optional in any context, the route might unintentionally match the more generic path first. Confirm the ordering is correct inrepositorySubRoutes.✅ Verification successful
Route ordering for multi-parameter paths in repositorySubRoutes is consistent.
An investigation into the usage ofrepositorySubRoutesand the related route definitions shows that all routes that include:repositoryTypeand:repositoryIdfollow the same ordering, and there is no indication of any optional parameter (e.g. using a trailing “?”) that might cause improper matching. The ordering as imported intoexam-management.route.tsis verified to be correct.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that route ordering is correct and no optional params overshadow more specific routes rg "path: '.*:repositoryType.*:repositoryId'"Length of output: 779
Script:
#!/bin/bash # Search for the definition and usage of repositorySubRoutes to verify its ordering rg -n "repositorySubRoutes" .Length of output: 1074
src/main/webapp/app/detail-overview-list/components/programming-auxiliary-repository-buttons-detail/programming-auxiliary-repository-buttons-detail.component.ts (2)
8-8: Use of standardized RepositoryType import is appropriate.
Your import ofRepositoryTypeimproves consistency and type safety across the application.
19-19: Protected readonly field ensures consistent references in the template.
PersistingRepositoryTypeas a readonly field is a clean, maintainable approach.src/main/webapp/app/exercises/programming/participate/programming-repository-routing.module.ts (3)
18-18: Path extension to 'commit-history' is consistent.
Using:repositoryId/commit-historymaintains clarity around the resource. Good job.
27-27: Enhanced commit details path ensures coherence.
Replacing:participationIdwith:repositoryIdeliminates confusion and matches the new naming scheme.
9-9: Parameter rename aligns with the shift from participation to repository-based routing.
This clarifies the route’s purpose and ensures consistent naming across the app.✅ Verification successful
Routing Parameter Rename Verified.
The change insrc/main/webapp/app/exercises/programming/participate/programming-repository-routing.module.tscorrectly renames the parameter to:repositoryId, and no stale references to:participationIdremain in that file. The other instances of:participationIdin the repository are in files where the participation-based route naming is expected.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify no stale references to :participationId remain rg ":participationId"Length of output: 12175
src/test/javascript/spec/component/programming-exercise/programming-exercise-instructor-repo-download-component.spec.ts (1)
44-44: LGTM! Test cases properly updated.The repository type has been correctly updated from 'TESTS' to 'TEST' to align with the new standardized
RepositoryTypeenum.src/main/webapp/app/exercises/programming/shared/actions/programming-exercise-instructor-repo-download.component.ts (1)
4-4: LGTM! Clean transition to RepositoryType enum.The component has been properly updated to use the new
RepositoryTypeenum, improving type safety and maintainability.Also applies to: 11-11, 30-30
src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-management-routing.module.ts (2)
54-54: LGTM! Route parameter update aligns with new repository type handling.The route path has been correctly updated to use
:repositoryType/:repositoryId, which aligns with the PR's objective to improve repository view routing.
9-20: Verify the impact of removing these routes.The commented-out routes for 'test' and ':participationId' might affect existing functionality. Please confirm:
- Are these routes being replaced by alternative paths?
- Are there any components still depending on these routes?
Also applies to: 41-52
✅ Verification successful
Route Replacement Verification
The investigation confirms that the originally defined routes for
'test'and':participationId'have been replaced with updated paths:
'test'→'ide/test'':participationId'→'ide/:participationId'No active references remain for the old paths, and a review of components in the codebase (including active route definitions in
code-editor-management-routing.module.ts) indicates that no components still depend on the removed routes.
- The file
src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-management-routing.module.tsnow defines:
path: 'ide/test'path: 'ide/:participationId'path: ':repositoryType/:repositoryId'Thus, the functionality previously provided by the commented-out routes is being handled by the new routes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for route references in the codebase echo "Searching for references to removed routes..." rg -l "test'?\s*[,)]" --type ts rg -l "participationId'?\s*[,)]" --type tsLength of output: 13391
Script:
#!/bin/bash # Display active route definitions in the code-editor-management-routing.module.ts file excluding commented lines echo "Active route definitions in code-editor-management-routing.module.ts:" grep -E "^\s*path:" src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-management-routing.module.ts | grep -v "^\s*//"Length of output: 402
src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-instructor-and-editor-container.component.ts (1)
15-15: LGTM! Clean addition of RepositoryType enum.The component has been properly updated to expose the
RepositoryTypeenum, following Angular style guidelines with correct access modifiers.Also applies to: 49-49
src/test/javascript/spec/component/localvc/vcs-repository-access-log-view.component.spec.ts (1)
48-48: LGTM! Test setup correctly updated.The route parameter change from
participationIdtorepositoryIdaligns with the broader refactoring, and test coverage remains comprehensive.src/main/webapp/app/exam/participate/summary/exercises/programming-exam-summary/programming-exam-summary.component.ts (1)
70-70: LGTM!The property name is descriptive and follows Angular's naming conventions.
src/main/webapp/app/shared/orion/orion.ts (3)
87-87: LGTM!The parameter type change to
RepositoryTypealigns with the PR's objective to centralize repository type handling.
1-1: LGTM! Type safety improvement.The transition to using
RepositoryTypeenum improves type safety and maintainability.Also applies to: 87-87
1-1: LGTM! Good type safety improvement.The transition to using
RepositoryTypeenum improves type safety and maintainability.Also applies to: 87-87
src/main/webapp/app/detail-overview-list/detail.model.ts (3)
103-103: LGTM!The type update to
RepositoryTypeis consistent with the PR's objective to centralize repository type handling.
15-15: LGTM! Enhanced type safety.The transition to using
RepositoryTypeenum improves type safety and consistency across the codebase.Also applies to: 103-103
15-15: LGTM! Good type safety improvement.The transition to using
RepositoryTypeenum in the interface improves type safety and consistency.Also applies to: 103-103
src/main/webapp/app/utils/navigation.utils.ts (1)
167-169: Verify the exam routing condition change.The condition for exam routing has been changed from
examId < 0toexamId > 0. Please ensure this change is intentional and aligns with the expected behavior.Run the following script to verify the usage of
examIdin routing:✅ Verification successful
Exam routing condition change confirmed.
The shell script output shows that both occurrences insrc/main/webapp/app/utils/navigation.utils.tsuse the conditionexamId > 0, which confirms the intended change. There are no instances ofexamId < 0, so the update aligns with the expected behavior.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for patterns of examId usage in routing conditions. # Search for examId conditions in routing logic rg -A 5 'examId [<>]' # Search for examId parameter usage in route paths ast-grep --pattern $'examId [<>] 0'Length of output: 1272
src/main/webapp/app/localvc/commit-details-view/commit-details-view.component.ts (2)
75-80: LGTM!The conditional logic for handling different repository types is well-structured and clear.
114-115: LGTM!The method call has been correctly updated to use
repositoryIdandrepositoryType.src/main/webapp/app/exercises/programming/manage/programming-exercise-management-routing.module.ts (2)
28-65: LGTM!The
repositorySubRoutesconstant is well-structured and provides a clean separation of repository-related routes. Each route is properly configured with:
- Lazy-loaded components
- Appropriate authorities
- Required guards
- Consistent page titles
184-190: LGTM!The route configuration correctly uses the
repositorySubRoutesfor both repository type and ID-based paths, providing a consistent routing structure.src/main/webapp/app/localvc/commit-history/commit-history.component.ts (2)
55-61: LGTM!The initialization logic correctly handles the repository type and ID, with appropriate fallback to 'USER' type.
132-139: LGTM!The commit handling logic is well-structured with clear conditions for different repository types.
src/main/webapp/app/localvc/repository-view/repository-view.component.ts (1)
74-74: LGTM! Type safety improvements.The changes enhance type safety by using the
RepositoryTypeenum instead of string literals and union types. This makes the code more maintainable and less error-prone.Also applies to: 112-121
src/main/webapp/app/exercises/programming/shared/code-editor/actions/code-editor-actions.component.ts (2)
7-7: LGTM! Imports are well organized.The imports are logically grouped and include all necessary dependencies for the new routing functionality.
Also applies to: 16-16, 24-24
29-29: LGTM! Component imports are properly configured.RouterLink is correctly added to the imports array for handling navigation.
src/test/javascript/spec/component/localvc/commit-history.component.spec.ts (1)
113-113: LGTM! Route parameters are consistently updated.The migration from
participationIdtorepositoryIdis consistently applied across all test cases, maintaining test coverage for the new routing structure.Also applies to: 134-134, 155-155, 278-278
src/test/javascript/spec/component/localvc/commit-details-view.component.spec.ts (1)
140-140: LGTM! Route parameters are properly updated for repository type support.The test cases have been consistently updated to include the new
repositoryTypeparameter where applicable, ensuring proper test coverage for the routing changes.Also applies to: 158-158, 176-176, 193-193, 214-214, 231-231, 248-248, 269-269, 297-297
src/main/webapp/app/overview/exercise-details/exercise-details-student-actions.component.ts (1)
2-2: LGTM! Import simplified to RouterLink only.The change from Router to RouterLink import aligns with the component's reduced routing needs.
src/test/javascript/spec/component/localvc/repository-view.component.spec.ts (1)
159-159: Verify the change in participation initialization.The test expectations for the
participationproperty have been changed fromundefinedto{}for both 'TESTS' and 'AUXILIARY' repository types. This change suggests a modification in how these repository types handle the participation property.Run the following script to verify if this change is consistent with the component's implementation:
Also applies to: 198-198
✅ Verification successful
Participation Initialization Change Verified
The component assigns participation based on the exercise’s properties (templateParticipation, solutionParticipation, or participationWithResults). Although there isn’t an explicit assignment of an empty object (i.e.
{}) within the component file itself, the test changes are consistent with the updated test data—which provides a default empty object for TESTS and AUXILIARY repository types—ensuring thatcomponent.participationevaluates to{}as expected.
- The component sets participation using values from the exercise without defaulting to
{}explicitly.- The test data for TESTS and AUXILIARY now supplies the expected default
{}to match these changes.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check how participation is initialized in the component # Expected: Find evidence of empty object initialization for TESTS and AUXILIARY types # Search for participation initialization in the component ast-grep --pattern 'this.participation = $_' src/main/webapp/app/localvc/repository-view/repository-view.component.tsLength of output: 583
src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-instructor-base-container.component.ts (1)
18-18: LGTM! Good refactoring to use centralized RepositoryType enum.The changes improve type safety and maintainability by replacing string literals with a centralized
RepositoryTypeenum. This is a good practice that helps prevent typos and makes the codebase more maintainable.Also applies to: 65-65, 98-99, 100-101, 196-196, 330-331
src/main/webapp/app/exercises/programming/manage/services/programming-exercise.service.ts (1)
25-25: LGTM! Good use of RepositoryType enum in service layer.The changes improve type safety by using the
RepositoryTypeenum for theexportInstructorRepositorymethod signature and condition check. This aligns well with the overall refactoring effort.Also applies to: 437-438
src/main/webapp/app/exercises/programming/manage/programming-exercise-detail.component.ts (3)
407-407: LGTM! Good use of RepositoryType enumeration.The changes correctly replace string literals with
RepositoryTypeenumeration values, improving type safety and maintainability.Also applies to: 416-416, 425-425
407-407: LGTM! Great use of theRepositoryTypeenumeration.The changes enhance type safety and maintainability by replacing string literals with the
RepositoryTypeenumeration.Also applies to: 416-416, 425-425
407-407: LGTM! Good use ofRepositoryTypeenumeration.The changes correctly replace string literals with
RepositoryTypeenumeration, improving type safety and maintainability.Also applies to: 416-416, 425-425
src/main/webapp/app/detail-overview-list/components/programming-auxiliary-repository-buttons-detail/programming-auxiliary-repository-buttons-detail.component.html (3)
10-10: LGTM! Good use of RepositoryType enumeration.The changes correctly replace string literals with
RepositoryType.AUXILIARYenumeration value, improving type safety and maintainability.Also applies to: 15-15
10-10: LGTM! Great use of theRepositoryTypeenumeration.The changes enhance type safety and maintainability by replacing string literals with the
RepositoryTypeenumeration.Also applies to: 15-15
10-10: LGTM! Good use ofRepositoryTypeenumeration.The changes correctly use
RepositoryType.AUXILIARYinstead of string literals, improving type safety and maintainability.Also applies to: 15-15
src/main/webapp/app/exercises/programming/shared/code-editor/actions/code-editor-actions.component.html (3)
4-9: LGTM! Well-implemented repository navigation button.The new button is correctly implemented with:
- Proper conditional rendering based on
isInCourseManagement- Security attributes for external links (
target="_blank"withrel="noopener noreferrer")- Translated label and icon for better UX
4-9: LGTM! Well-structured repository navigation button.The new button enhances the user interface by providing direct access to the repository. Good use of:
- Security attributes (
target="_blank" rel="noopener noreferrer")- Translation directive
- Icon for visual feedback
4-9: LGTM! Well-implemented repository link button.The new button for navigating to repositories is well-implemented with:
- Proper conditional rendering based on
isInCourseManagement- Correct use of
routerLinkfor navigation- Good accessibility with
target="_blank"andrel="noopener noreferrer"- Clear icon and translated label
src/main/webapp/app/exam/participate/summary/exercises/programming-exam-summary/programming-exam-summary.component.html (1)
14-18: Conditional Routing for Repository View
The updated[routerLinkForRepositoryView]property now uses a clear conditional expression based onisInCourseManagementto determine the route. When in course management, the route includes the static'USER'segment; otherwise, it falls back to the default route structure. This change effectively centralizes the routing logic and improves clarity.src/main/webapp/app/exam/manage/exercise-groups/programming-exercise-cell/programming-exercise-group-cell.component.html (3)
18-18: Using RepositoryType Enum for Template Repository Download
The call todownloadRepositorynow passesRepositoryType.TEMPLATEinstead of a hardcoded string. This improves type safety and consistency across the application when handling repository downloads.
36-36: Using RepositoryType Enum for Solution Repository Download
Similarly, updating the download call to useRepositoryType.SOLUTIONhelps to enforce consistency in repository type references.
54-54: Using RepositoryType Enum for Test Repository Download
The revision to passRepositoryType.TESTSin the test repository download action aligns with the centralized enumeration approach and removes ambiguity caused by string literals.src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-instructor-and-editor-container.component.html (6)
20-20: Utilizing RepositoryType for Build Eligibility
The[buildable]input now checks whetherselectedRepository !== RepositoryType.TESTS, ensuring that the code editor container correctly disables building actions for test repositories. This change reinforces type safety and expected behavior in the UI.
43-43: Display Label Update Based on Repository Type
The button label now conditionally displaysselectedAuxiliaryRepositoryNamewhenselectedRepositoryequalsRepositoryType.AUXILIARY; otherwise, it falls back to showingselectedRepository. This enhances the clarity of repository selection in the dropdown.
50-50: Template Repository Button Style Adjustment
The inline style for the template repository button now dynamically applies a background color whenselectedRepository === RepositoryType.TEMPLATE. This visual cue improves user recognition of the current selection.
58-58: Solution Repository Button Style Adjustment
Changing the style binding to check againstRepositoryType.SOLUTIONensures that the solution repository button receives the proper styling when selected.
64-64: Assignment Repository Button Style Update
The assignment repository button now usesRepositoryType.ASSIGNMENTfor its background color condition. This update continues the overall move from string literals to a strongly typed enumeration.
129-139: Conditional Rendering Based on Repository Type
The sections that wrap result updating and build trigger components have been updated to check thatselectedRepository !== RepositoryType.TESTS. This ensures that these elements are hidden when the test repository is active, consistent with the design expectations.src/main/webapp/app/exercises/programming/manage/programming-exercise-detail.component.html (1)
23-24: Enhanced Router Link for "Edit in Editor" Button
The router link for editing in the code editor has been modified to include the static segment'TEMPLATE'in the path:[routerLink]="[baseResource, 'code-editor', 'TEMPLATE', programmingExercise.templateParticipation.id!]"This change clarifies that the editor route is specifically for the template repository and aligns with the overall refactoring towards a standardized repository type system.
src/main/webapp/app/overview/exercise-details/exercise-details-student-actions.component.html (1)
125-133: Routing Update for Repository View Link
The updated[routerLinkForRepositoryView]now builds the route with'/courses', courseId, 'exercises', exercise.id!, 'repository', exercise.studentParticipations![0].id!, which provides better contextual information for navigating to the repository view. This change aligns with the PR’s objective to improve routing consistency.src/main/webapp/app/exercises/shared/exercise-scores/exercise-scores.component.html (1)
315-323: Simplified Repository Routing in Exercise Scores
The repository view router link has been refactored from an absolute path (using'/courses', course and exercise IDs) to a relative link['..', 'repository', 'USER', value.id]. This simplifies the URL structure for repository views and enhances consistency with the centralized routing refactoring across the application.src/main/webapp/app/exercises/programming/manage/programming-exercise.component.html (3)
90-94: Refactoring Repository Download – Template Repository
Replaced the string literal'TEMPLATE'withRepositoryType.TEMPLATEin the download call. This change improves type safety and aligns with the new, centralized repository type handling.
110-114: Refactoring Repository Download – Solution Repository
The update to useRepositoryType.SOLUTIONinstead of a hardcoded string literal enhances clarity and maintains consistency with the overall refactoring.
130-132: Refactoring Repository Download – Test Repository
The download for the test repository now usesRepositoryType.TESTS, ensuring standardization across repository type references and reducing the risk of typos or mismatches.src/main/webapp/app/exercises/shared/participation/participation.component.html (1)
127-133: Updated Repository View Routing in Participation
The<jhi-code-button>now uses[routerLinkForRepositoryView]="['..', 'repository', 'USER', row.id]", introducing a relative path with a'USER'segment. This update is consistent with the new repository type handling and ensures a more standardized routing approach across the application.src/main/webapp/app/orion/management/code-editor-instructor-and-editor-orion-container.component.html (3)
22-22: Updated Repository Condition for Selected Participation Display
The condition now correctly checksselectedRepository !== REPOSITORY.TESTS, which aligns with the newRepositoryTypeenum and ensures that the updating result component is only rendered when participation exists and the repository type is not "TESTS".
69-69: Conditional Display of Student Trigger Build Button
The condition@if (selectedRepository !== REPOSITORY.TESTS)correctly prevents the build button from being displayed for the test repository. This change ensures that the UI only renders the build trigger when it’s applicable and maintains a proper routing experience.
73-73: Conditional Display of Local Test Button
The updated condition@if (selectedRepository !== REPOSITORY.TESTS)ensures that the "Orion Test Locally" button is only rendered when the selected repository is not the test repository. This behavior aligns with the overall refactor towards standardized repository type handling.
...ipt/spec/component/orion/orion-code-editor-instructor-and-editor-container.component.spec.ts
Outdated
Show resolved
Hide resolved
src/main/webapp/app/localvc/commit-details-view/commit-details-view.component.ts
Outdated
Show resolved
Hide resolved
src/main/webapp/app/localvc/commit-history/commit-history.component.ts
Outdated
Show resolved
Hide resolved
src/test/javascript/spec/integration/code-editor/code-editor-instructor.integration.spec.ts
Outdated
Show resolved
Hide resolved
...participate/summary/exercises/programming-exam-summary/programming-exam-summary.component.ts
Show resolved
Hide resolved
.../webapp/app/exercises/programming/assess/code-editor-tutor-assessment-container.component.ts
Outdated
Show resolved
Hide resolved
WalkthroughThis pull request replaces hardcoded string values for repository types with a dedicated Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Component
participant Router
User->>Component: Click on repository view button
Component->>Component: Check context (isInCourseManagement flag)
alt Course management context is true
Component->>Router: Navigate using path: /courses/.../repository/USER/…
else
Component->>Router: Navigate using default repository path with RepositoryType
end
Router-->>User: Load repository view
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
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
|
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (1)
src/test/javascript/spec/service/orion/orion-connector.service.spec.ts (1)
192-197: Update test case to use RepositoryType.The test case is still using the old
REPOSITORYtype which will cause compilation errors. Update the test to use the newRepositoryTypeenum.Apply this diff to fix the test:
- it('should forward selectRepository', () => { - serviceUnderTest.selectRepository(REPOSITORY.SOLUTION); - - expect((window as any).orionVCSConnector.selectRepository).toHaveBeenCalledOnce(); - expect((window as any).orionVCSConnector.selectRepository).toHaveBeenCalledWith(REPOSITORY.SOLUTION); + it('should forward selectRepository', () => { + serviceUnderTest.selectRepository(RepositoryType.SOLUTION); + + expect((window as any).orionVCSConnector.selectRepository).toHaveBeenCalledOnce(); + expect((window as any).orionVCSConnector.selectRepository).toHaveBeenCalledWith(RepositoryType.SOLUTION);
🧹 Nitpick comments (15)
src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-management-routing.module.ts (2)
9-20: Clean up commented code.Remove the commented-out route configuration as it's no longer needed. If this code needs to be referenced later, it should be tracked in version control history instead.
- // { - // path: 'test', - // loadComponent: () => - // import('app/exercises/programming/manage/code-editor/code-editor-instructor-and-editor-container.component').then( - // (m) => m.CodeEditorInstructorAndEditorContainerComponent, - // ), - // data: { - // authorities: [Authority.EDITOR, Authority.INSTRUCTOR, Authority.ADMIN], - // pageTitle: 'artemisApp.editor.home.title', - // }, - // canActivate: [UserRouteAccessService], - // },
41-52: LGTM! Good routing structure improvement.The change from
:participationIdto:repositoryType/:repositoryIdimproves the routing structure and aligns with the retrieved learning about repository routing. Remember to remove the commented-out code.- // { - // path: ':participationId', - // loadComponent: () => - // import('app/exercises/programming/manage/code-editor/code-editor-instructor-and-editor-container.component').then( - // (m) => m.CodeEditorInstructorAndEditorContainerComponent, - // ), - // data: { - // authorities: [Authority.EDITOR, Authority.INSTRUCTOR, Authority.ADMIN], - // pageTitle: 'artemisApp.editor.home.title', - // }, - // canActivate: [UserRouteAccessService], - // },Also applies to: 54-54
src/main/webapp/app/utils/navigation.utils.ts (1)
157-170: Consider using RepositoryType enum for type safety.The function signature change and the condition update for
examIdlook good. However, consider using theRepositoryTypeenum for better type safety.Apply this diff to enhance type safety:
-export const getLocalRepositoryLink = ( +import { RepositoryType } from 'app/exercises/programming/manage/code-editor/code-editor-instructor-base-container.component'; + +export const getLocalRepositoryLink = ( courseId: number, exerciseId: number, - repositoryType: string, + repositoryType: RepositoryType, participationId: number, exerciseGroupId: number = 0, examId = 0, ): string[] => {src/main/webapp/app/localvc/commit-history/commit-history.component.ts (2)
55-61: Consider using RepositoryType enum for type safety.The string literals for repository types could be replaced with an enum for better type safety and maintainability.
Apply this diff to enhance type safety:
+import { RepositoryType } from 'app/exercises/programming/manage/code-editor/code-editor-instructor-base-container.component'; -repositoryType: string; +repositoryType: RepositoryType; -this.repositoryType = params['repositoryType'] ?? 'USER'; +this.repositoryType = params['repositoryType'] ?? RepositoryType.USER; -if (this.repositoryId && this.repositoryType === 'USER') { +if (this.repositoryId && this.repositoryType === RepositoryType.USER) {
132-139: Update repository type comparisons to use enum.Continue using the
RepositoryTypeenum for consistency.Apply this diff:
-if (this.repositoryType === 'USER') { +if (this.repositoryType === RepositoryType.USER) { this.handleParticipationCommits(); -} else if (this.repositoryType === 'AUXILIARY') { +} else if (this.repositoryType === RepositoryType.AUXILIARY) { this.handleAuxiliaryRepositoryCommits();src/main/webapp/app/localvc/repository-view/repository-view.component.ts (1)
74-74: LGTM! Consider extracting magic strings to constants.The changes improve type safety by using the
RepositoryTypeenum. The use of nullish coalescing operator for the default value is a good practice.Consider extracting the URL path segment 'course-management' to a constant to avoid magic strings:
+const COURSE_MANAGEMENT_PATH = 'course-management'; + export class RepositoryViewComponent implements OnInit, OnDestroy { // ... existing code ... - this.enableVcsAccessLog = this.router.url.includes('course-management') && params['repositoryType'] !== RepositoryType.TESTS; + this.enableVcsAccessLog = this.router.url.includes(COURSE_MANAGEMENT_PATH) && params['repositoryType'] !== RepositoryType.TESTS;Also applies to: 112-114
src/main/webapp/app/exercises/programming/manage/programming-exercise.component.ts (1)
256-264: LGTM! Consider documenting the limitation in JSDoc.The changes improve type safety by using the
RepositoryTypeenum. The comment about auxiliary repositories not being supported is valuable but could be more visible.Consider adding the limitation to the JSDoc comment:
/** * Downloads the instructor repository. Used when the "localvc" profile is active. * For the local VCS, linking to an external site displaying the repository does not work. * Instead, the repository is downloaded. + * Note: Auxiliary repositories are currently not supported for the local VCS. * * @param programmingExerciseId * @param repositoryType */src/main/webapp/app/exercises/programming/assess/code-editor-tutor-assessment-container.component.ts (1)
226-226: UseRepositoryTypeenum for consistency.For consistency with the rest of the codebase, consider using the
RepositoryTypeenum instead of the string literal 'USER'.-this.localRepositoryLink = getLocalRepositoryLink(this.courseId, this.exerciseId, 'USER', this.participation.id!, this.exerciseGroupId, this.examId); +this.localRepositoryLink = getLocalRepositoryLink(this.courseId, this.exerciseId, RepositoryType.USER, this.participation.id!, this.exerciseGroupId, this.examId);src/main/webapp/app/exercises/programming/shared/code-editor/actions/code-editor-actions.component.html (1)
4-9: New Repository Navigation Button for Course Management
A new conditional block using the Angular@ifsyntax has been introduced to render a repository navigation button whenisInCourseManagementis true. The use of a button with an external link, proper translation viajhiTranslate, and therouterLinkbinding is well implemented.Suggestion: Consider adding an
aria-labelor other accessibility attributes to the link to improve accessibility.src/main/webapp/app/orion/management/code-editor-instructor-and-editor-orion-container.component.html (1)
22-23: Consistent Repository Type Checks in Conditional Blocks
The conditional checks and style bindings (e.g., for button background colors) have been updated to compareselectedRepositoryagainst the enum value for test repositories (here noted asREPOSITORY.TESTS). This contributes to improved type safety and clarity.Suggestion: Ensure that the naming is fully consistent—if the new enum is named
RepositoryType, update all occurrences to useRepositoryType.TESTSrather thanREPOSITORY.TESTSif not done already.Also applies to: 64-65, 69-73
src/main/webapp/app/exam/manage/exercise-groups/programming-exercise-cell/programming-exercise-group-cell.component.html (1)
18-19: Consistent Use of RepositoryType Enum for Download Links
The calls todownloadRepositoryhave been updated to pass enum values (RepositoryType.TEMPLATE,RepositoryType.SOLUTION, andRepositoryType.TESTS) instead of string literals. This change enhances type safety and improves code clarity.Note: Verify that the
downloadRepositorymethod and any related backend logic correctly handle these enum values.Also applies to: 36-37, 54-55
src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-instructor-and-editor-container.component.html (1)
42-44: Repository Select Button Label Enhancement
The button label now conditionally displaysselectedAuxiliaryRepositoryNamewhen the selected repository is auxiliary; otherwise, it shows the rawselectedRepositoryvalue. Consider mapping the raw enum value to a more user-friendly display string for consistency.src/main/webapp/app/exercises/programming/manage/programming-exercise.component.html (3)
91-93: Enum-Based Repository Download (Template)
Replacing the hardcoded string'TEMPLATE'withRepositoryType.TEMPLATEin the downloadRepository call improves type safety and maintainability.
111-113: Enum-Based Repository Download (Solution)
Similarly, usingRepositoryType.SOLUTIONensures the solution download call is consistent with the new enum-driven approach.
131-132: Enum-Based Repository Download (Test)
Switching toRepositoryType.TESTSfurther aligns this method call with the standardized repository types. Please verify that the backend or routing logic anticipates these enum values.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (46)
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/components/programming-auxiliary-repository-buttons-detail/programming-auxiliary-repository-buttons-detail.component.ts(2 hunks)src/main/webapp/app/detail-overview-list/detail.model.ts(2 hunks)src/main/webapp/app/exam/manage/exam-management.route.ts(3 hunks)src/main/webapp/app/exam/manage/exercise-groups/programming-exercise-cell/programming-exercise-group-cell.component.html(3 hunks)src/main/webapp/app/exam/manage/exercise-groups/programming-exercise-cell/programming-exercise-group-cell.component.ts(4 hunks)src/main/webapp/app/exam/participate/summary/exercises/programming-exam-summary/programming-exam-summary.component.html(1 hunks)src/main/webapp/app/exam/participate/summary/exercises/programming-exam-summary/programming-exam-summary.component.ts(1 hunks)src/main/webapp/app/exercises/programming/assess/code-editor-tutor-assessment-container.component.ts(1 hunks)src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-instructor-and-editor-container.component.html(6 hunks)src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-instructor-and-editor-container.component.ts(2 hunks)src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-instructor-base-container.component.ts(8 hunks)src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-management-routing.module.ts(2 hunks)src/main/webapp/app/exercises/programming/manage/programming-exercise-detail.component.html(1 hunks)src/main/webapp/app/exercises/programming/manage/programming-exercise-detail.component.ts(5 hunks)src/main/webapp/app/exercises/programming/manage/programming-exercise-management-routing.module.ts(2 hunks)src/main/webapp/app/exercises/programming/manage/programming-exercise.component.html(3 hunks)src/main/webapp/app/exercises/programming/manage/programming-exercise.component.ts(4 hunks)src/main/webapp/app/exercises/programming/manage/services/programming-exercise.service.ts(2 hunks)src/main/webapp/app/exercises/programming/participate/programming-repository-routing.module.ts(3 hunks)src/main/webapp/app/exercises/programming/shared/actions/programming-exercise-instructor-repo-download.component.ts(2 hunks)src/main/webapp/app/exercises/programming/shared/code-editor/actions/code-editor-actions.component.html(1 hunks)src/main/webapp/app/exercises/programming/shared/code-editor/actions/code-editor-actions.component.ts(5 hunks)src/main/webapp/app/exercises/programming/shared/code-editor/model/code-editor.model.ts(1 hunks)src/main/webapp/app/exercises/shared/exercise-scores/exercise-scores.component.html(1 hunks)src/main/webapp/app/exercises/shared/participation/participation.component.html(1 hunks)src/main/webapp/app/localvc/commit-details-view/commit-details-view.component.ts(7 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(5 hunks)src/main/webapp/app/localvc/vcs-repository-access-log-view/vcs-repository-access-log-view.component.ts(1 hunks)src/main/webapp/app/orion/management/code-editor-instructor-and-editor-orion-container.component.html(2 hunks)src/main/webapp/app/orion/management/code-editor-instructor-and-editor-orion-container.component.ts(4 hunks)src/main/webapp/app/overview/exercise-details/exercise-details-student-actions.component.html(1 hunks)src/main/webapp/app/overview/exercise-details/exercise-details-student-actions.component.ts(1 hunks)src/main/webapp/app/shared/orion/orion-connector.service.ts(2 hunks)src/main/webapp/app/shared/orion/orion.ts(2 hunks)src/main/webapp/app/utils/navigation.utils.ts(1 hunks)src/test/javascript/spec/component/localvc/commit-details-view.component.spec.ts(9 hunks)src/test/javascript/spec/component/localvc/commit-history.component.spec.ts(3 hunks)src/test/javascript/spec/component/localvc/repository-view.component.spec.ts(2 hunks)src/test/javascript/spec/component/localvc/vcs-repository-access-log-view.component.spec.ts(1 hunks)src/test/javascript/spec/component/orion/orion-code-editor-instructor-and-editor-container.component.spec.ts(1 hunks)src/test/javascript/spec/component/overview/exercise-details/exercise-details-student-actions.component.spec.ts(0 hunks)src/test/javascript/spec/component/programming-exercise/programming-exercise-instructor-repo-download-component.spec.ts(1 hunks)src/test/javascript/spec/integration/code-editor/code-editor-instructor.integration.spec.ts(5 hunks)src/test/javascript/spec/service/orion/orion-connector.service.spec.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/test/javascript/spec/component/overview/exercise-details/exercise-details-student-actions.component.spec.ts
🧰 Additional context used
📓 Path-based instructions (45)
src/main/webapp/app/detail-overview-list/detail.model.ts (1)
src/main/webapp/app/localvc/vcs-repository-access-log-view/vcs-repository-access-log-view.component.ts (1)
src/test/javascript/spec/component/programming-exercise/programming-exercise-instructor-repo-download-component.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/main/webapp/app/exercises/programming/shared/code-editor/actions/code-editor-actions.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/orion/management/code-editor-instructor-and-editor-orion-container.component.ts (1)
src/main/webapp/app/exercises/programming/manage/programming-exercise-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/exam/participate/summary/exercises/programming-exam-summary/programming-exam-summary.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/programming-exercise.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/shared/exercise-scores/exercise-scores.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/code-editor/code-editor-management-routing.module.ts (1)
src/main/webapp/app/shared/orion/orion.ts (1)
src/main/webapp/app/localvc/commit-history/commit-history.component.ts (1)
src/test/javascript/spec/service/orion/orion-connector.service.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/main/webapp/app/overview/exercise-details/exercise-details-student-actions.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/code-editor/code-editor-instructor-and-editor-container.component.ts (1)
src/main/webapp/app/exercises/shared/participation/participation.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/shared/orion/orion-connector.service.ts (1)
src/main/webapp/app/exercises/programming/shared/code-editor/model/code-editor.model.ts (1)
src/main/webapp/app/exam/manage/exercise-groups/programming-exercise-cell/programming-exercise-group-cell.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/exam/manage/exercise-groups/programming-exercise-cell/programming-exercise-group-cell.component.ts (1)
src/test/javascript/spec/component/localvc/commit-history.component.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/main/webapp/app/exam/participate/summary/exercises/programming-exam-summary/programming-exam-summary.component.ts (1)
src/main/webapp/app/exercises/programming/manage/programming-exercise.component.ts (1)
src/main/webapp/app/overview/exercise-details/exercise-details-student-actions.component.ts (1)
src/main/webapp/app/detail-overview-list/components/programming-auxiliary-repository-buttons-detail/programming-auxiliary-repository-buttons-detail.component.ts (1)
src/main/webapp/app/exercises/programming/assess/code-editor-tutor-assessment-container.component.ts (1)
src/main/webapp/app/exercises/programming/manage/services/programming-exercise.service.ts (1)
src/test/javascript/spec/integration/code-editor/code-editor-instructor.integration.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/main/webapp/app/localvc/commit-details-view/commit-details-view.component.ts (1)
src/test/javascript/spec/component/localvc/commit-details-view.component.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/test/javascript/spec/component/localvc/repository-view.component.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/main/webapp/app/exercises/programming/participate/programming-repository-routing.module.ts (1)
src/main/webapp/app/exercises/programming/shared/actions/programming-exercise-instructor-repo-download.component.ts (1)
src/test/javascript/spec/component/orion/orion-code-editor-instructor-and-editor-container.component.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/main/webapp/app/exercises/programming/shared/code-editor/actions/code-editor-actions.component.ts (1)
src/main/webapp/app/exercises/programming/manage/programming-exercise-detail.component.ts (1)
src/main/webapp/app/orion/management/code-editor-instructor-and-editor-orion-container.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/test/javascript/spec/component/localvc/vcs-repository-access-log-view.component.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/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/manage/code-editor/code-editor-instructor-base-container.component.ts (1)
src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-instructor-and-editor-container.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/localvc/repository-view/repository-view.component.ts (1)
src/main/webapp/app/exam/manage/exam-management.route.ts (1)
src/main/webapp/app/utils/navigation.utils.ts (1)
src/main/webapp/app/exercises/programming/manage/programming-exercise-management-routing.module.ts (1)
📓 Learnings (3)
src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-management-routing.module.ts (1)
Learnt from: SimonEntholzer
PR: ls1intum/Artemis#10228
File: src/main/webapp/app/exercises/programming/participate/programming-repository-routing.module.ts:9-9
Timestamp: 2025-01-29T08:21:07.083Z
Learning: The programming repository routing module uses `:repositoryId` parameter which is correctly handled by the localvc components (commit-details-view, commit-history, and repository-view components).
src/main/webapp/app/exercises/programming/participate/programming-repository-routing.module.ts (1)
Learnt from: SimonEntholzer
PR: ls1intum/Artemis#10228
File: src/main/webapp/app/exercises/programming/participate/programming-repository-routing.module.ts:9-9
Timestamp: 2025-01-29T08:21:07.083Z
Learning: The programming repository routing module uses `:repositoryId` parameter which is correctly handled by the localvc components (commit-details-view, commit-history, and repository-view components).
src/main/webapp/app/exercises/programming/manage/programming-exercise-management-routing.module.ts (1)
Learnt from: SimonEntholzer
PR: ls1intum/Artemis#10228
File: src/main/webapp/app/exercises/programming/participate/programming-repository-routing.module.ts:9-9
Timestamp: 2025-01-29T08:21:07.083Z
Learning: The programming repository routing module uses `:repositoryId` parameter which is correctly handled by the localvc components (commit-details-view, commit-history, and repository-view components).
🪛 GitHub Check: client-tests-selected
src/test/javascript/spec/integration/code-editor/code-editor-instructor.integration.spec.ts
[failure] 69-69:
'RepositoryType' is declared but its value is never read.
[failure] 69-69:
Module '"app/exercises/programming/manage/code-editor/code-editor-instructor-base-container.component"' declares 'RepositoryType' locally, but it is not exported.
src/test/javascript/spec/component/orion/orion-code-editor-instructor-and-editor-container.component.spec.ts
[failure] 7-7:
'RepositoryType' is declared but its value is never read.
[failure] 7-7:
Module '"app/exercises/programming/manage/code-editor/code-editor-instructor-base-container.component"' declares 'RepositoryType' locally, but it is not exported.
🪛 GitHub Check: client-tests
src/test/javascript/spec/integration/code-editor/code-editor-instructor.integration.spec.ts
[failure] 69-69:
'RepositoryType' is declared but its value is never read.
[failure] 69-69:
Module '"app/exercises/programming/manage/code-editor/code-editor-instructor-base-container.component"' declares 'RepositoryType' locally, but it is not exported.
src/test/javascript/spec/component/orion/orion-code-editor-instructor-and-editor-container.component.spec.ts
[failure] 7-7:
'RepositoryType' is declared but its value is never read.
[failure] 7-7:
Module '"app/exercises/programming/manage/code-editor/code-editor-instructor-base-container.component"' declares 'RepositoryType' locally, but it is not exported.
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: server-tests
- GitHub Check: Analyse
🔇 Additional comments (53)
src/main/webapp/app/localvc/commit-details-view/commit-details-view.component.ts (7)
10-10: Importing the GitDiffReportComponent is correctly integrated.
No issues – this import is used properly in the component’simportsarray.
26-26: Refactor from participation-based to repository-based ID is consistent.
RenamingparticipationIdtorepositoryIdaligns perfectly with the revised repository-centric approach.
36-36: EnsuringrepositoryTypeis no longer optional is a good decision.
The property is guaranteed a value (defaulting to 'USER'), avoiding undefined behavior.
61-63: Defaulting undefinedrepositoryTypeto 'USER' is appropriate.
This approach ensures consistent usage by eliminating null or undefined checks when handling the repository type.
115-115: Refactoring method call to usethis.repositoryIdis consistent.
Excellent job aligning the diff report fetching with repository-based identification.
76-76: Double-check handling of all non-'USER' repository types.
Ensure that future repository types (e.g., 'TEMPLATE', 'SOLUTION', 'AUXILIARY') remain valid under this condition.✅ Verification successful
Double-check on repository type handling confirmed.
The conditionif (this.repositoryType !== 'USER')incommit-details-view.component.tscleanly routes all non-'USER' repository types (including 'TEMPLATE', 'SOLUTION', 'TESTS', 'AUXILIARY', and any future types) to the service call. This approach is consistent with analogous patterns in other components (e.g., in commit-history and repository-view components), where non-'USER' values are handled via dedicated methods.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify usage of all possible repositoryType values to confirm coverage. rg -A 5 $'this\\.repositoryType\\b'Length of output: 15052
78-79: Verify behavior whenrepositoryIdis undefined for 'USER' repositories.
This else-if block only executes ifrepositoryIdis truthy. Confirm that failing to set it doesn't break other flows.✅ Verification successful
Verification of Safe Handling of Undefined
repositoryIdin 'USER' Repositories
- In both the commit-details-view and commit-history components,
this.repositoryIdis assigned viaNumber(params['repositoryId']), so if the parameter is missing, it becomes a falsy value (NaN).- In the commit-details-view component, the branch that calls
retrieveCommitHistoryForParticipation(this.repositoryId)only executes whenthis.repositoryIdis truthy. Otherwise, it returns early, preventing further operations that assume a valid ID.- In the commit-history component, when
this.repositoryIdis falsy (or undefined), the code follows the alternate flow (loadDifferentParticipation()), ensuring that the absence of a repository ID does not compromise the handling of 'USER' repositories.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search references for `this.repositoryId` to confirm safe usage throughout. rg -A 5 $'this\\.repositoryId\\b'Length of output: 5379
src/main/webapp/app/detail-overview-list/components/programming-auxiliary-repository-buttons-detail/programming-auxiliary-repository-buttons-detail.component.ts (2)
8-8: UsingRepositoryTypeensures stronger type safety.
Importing the enum elevates clarity and consistency across the codebase.
19-19: ExposingRepositoryTypeas a protected property is an effective approach.
This practice facilitates template usage while respecting encapsulation.src/main/webapp/app/exercises/programming/participate/programming-repository-routing.module.ts (3)
9-9: Updated the route parameter to:repositoryId.
Clear improvement over the old:participationIdapproach for repository-focused logic.
18-18: Consistent change to the commit-history path.
Switching to:repositoryId/commit-historyaligns with the new repository-centered routing.
27-27: Path update for commit details is consistent.
Renaming to:repositoryId/commit-history/:commitHashavoids confusion and improves clarity.src/main/webapp/app/exercises/programming/shared/actions/programming-exercise-instructor-repo-download.component.ts (1)
4-4: LGTM! Good use of the RepositoryType enum.The changes correctly implement the transition to using the RepositoryType enum, improving type safety and maintainability.
Also applies to: 11-11, 30-30
src/main/webapp/app/localvc/vcs-repository-access-log-view/vcs-repository-access-log-view.component.ts (1)
24-24: Verify the impact of parameter name change.The change from
participationIdtorepositoryIdin the route parameters is correct, but ensure that all consumers of this component are updated to pass the correct parameter.✅ Verification successful
Please run the above script to verify if any route configurations reference the component and check whether the parameter passed is still using the old naming convention or has been updated to the new "repositoryId".
Parameter name change verified for vcs-repository-access-log-view component.
- The component now correctly retrieves the parameter as
repositoryId.- No consumer (or route configuration) referencing this component is still using the old parameter name (
participationId).🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for route configurations and component usages that might still use participationId rg -l "participationId.*vcs-repository-access-log-view"Length of output: 57
Script:
#!/bin/bash # Search all route configuration files and other sources within the app for occurrences # of the old route parameter name ":participationId". This helps verify that all routes # and consumers now refer to the updated parameter name. rg -l ":participationId" src/main/webapp # Additionally, search for component usage contexts where vcs-repository-access-log-view # might be mentioned along with the new parameter "repositoryId" for cross-checking. rg -l "repositoryId.*vcs-repository-access-log-view" src/main/webappLength of output: 918
Script:
#!/bin/bash # Search for any references to the vcs-repository-access-log-view component in the route configurations rg -l "vcs-repository-access-log-view" src/main/webappLength of output: 256
src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-instructor-and-editor-container.component.ts (1)
15-15: LGTM! Type-safe enum implementation.The addition of
RepositoryTypeenum and its protected readonly property improves type safety and prevents accidental modifications.Also applies to: 49-49
src/main/webapp/app/exercises/programming/shared/code-editor/model/code-editor.model.ts (1)
56-66: LGTM! Well-documented enum implementation.The
RepositoryTypeenum is well-documented and follows TypeScript best practices. The comment forASSIGNMENTtype is particularly helpful in clarifying its specific use case.src/test/javascript/spec/component/localvc/vcs-repository-access-log-view.component.spec.ts (1)
48-48: LGTM! Updated route parameter aligns with new routing schema.The change from
participationIdtorepositoryIdin the route parameters aligns with the broader refactoring effort.src/main/webapp/app/orion/management/code-editor-instructor-and-editor-orion-container.component.ts (1)
74-74: LGTM! Type-safe condition using enum.The condition using
RepositoryType.TESTSimproves type safety and code clarity.src/main/webapp/app/exam/manage/exercise-groups/programming-exercise-cell/programming-exercise-group-cell.component.ts (3)
8-8: LGTM! Import changes align with the refactoring goal.The imports are correctly organized, with the new
RepositoryTypeimport replacing the old type.Also applies to: 17-17
83-91: LGTM! Method signature change maintains type safety.The
downloadRepositorymethod signature has been correctly updated to useRepositoryType, maintaining type safety while improving code consistency.
93-93: LGTM! Protected readonly property follows Angular best practices.The
protected readonly RepositoryTypeproperty is correctly exposed to the template, following Angular best practices for enum access.src/main/webapp/app/exam/participate/summary/exercises/programming-exam-summary/programming-exam-summary.component.ts (2)
70-70: LGTM! Property naming follows Angular conventions.The
isInCourseManagementboolean property follows Angular naming conventions for boolean flags.
76-76: LGTM! Initialization in ngOnInit is correct.The property is correctly initialized in ngOnInit using router URL check, which aligns with Angular lifecycle management.
src/main/webapp/app/shared/orion/orion.ts (1)
1-1: LGTM! Interface updates maintain type safety and documentation.The
selectRepositorymethod signature has been correctly updated to useRepositoryType, maintaining type safety while preserving documentation.Also applies to: 87-87
src/main/webapp/app/detail-overview-list/detail.model.ts (1)
15-15: LGTM! Interface updates maintain type safety.The
typeproperty inProgrammingRepositoryButtonsDetailinterface has been correctly updated to useRepositoryType, maintaining type safety and consistency.Also applies to: 103-103
src/main/webapp/app/exercises/programming/manage/programming-exercise-management-routing.module.ts (2)
28-65: Well-structured repository sub-routes with proper lazy loading.The extraction of common repository sub-routes improves code organization and maintainability. The routes are properly guarded with appropriate authorities and use lazy loading for better performance.
185-190: Clean route restructuring using sub-routes.The use of
children: repositorySubRouteseffectively reuses the common sub-routes for both repository paths.src/main/webapp/app/localvc/commit-history/commit-history.component.ts (1)
33-33: Clear documentation of repositoryId's dual role.The comment effectively clarifies the dual purpose of
repositoryIdfor different repository types.src/main/webapp/app/shared/orion/orion-connector.service.ts (1)
225-227: LGTM! Improved type safety with RepositoryType enum.The change from using
REPOSITORYtoRepositoryTypeenhances type safety and maintainability. This aligns with the broader refactoring effort to standardize repository type handling across the application.src/main/webapp/app/exercises/programming/shared/code-editor/actions/code-editor-actions.component.ts (1)
7-7: LGTM! Import statements are well-organized.The imports are logically grouped and include all necessary dependencies for the component's functionality.
Also applies to: 16-16, 24-24, 29-29
src/test/javascript/spec/component/localvc/commit-history.component.spec.ts (1)
113-113: LGTM! Test cases are well-updated.The test cases have been properly updated to use
repositoryIdandrepositoryTypeinstead ofparticipationId, maintaining consistency with the new repository type handling approach.Also applies to: 134-134, 155-155, 176-176, 193-193, 214-214, 231-231, 248-248, 269-269, 297-297
src/test/javascript/spec/component/localvc/commit-details-view.component.spec.ts (1)
140-140: LGTM! Test cases are well-updated.The test cases have been properly updated to use
repositoryIdandrepositoryTypeinstead ofparticipationId, maintaining consistency with the new repository type handling approach.Also applies to: 158-158, 176-176, 193-193, 214-214, 231-231, 248-248, 269-269, 297-297
src/main/webapp/app/overview/exercise-details/exercise-details-student-actions.component.ts (1)
2-2: Verify the impact of removing router-based repository link generation.The removal of router-based repository link generation might affect navigation functionality. Please ensure that:
- The
repositoryLinkproperty is still properly initialized elsewhere- All navigation features that previously relied on this link still work correctly
Run the following script to verify repository link usage:
Also applies to: 94-95
✅ Verification successful
Repository link functionality verified successfully.
- The
repositoryLinkis still initialized insrc/main/webapp/app/exercises/programming/shared/code-editor/actions/code-editor-actions.component.tsvia a call togetLocalRepositoryLink().- Templates (e.g., in
src/main/webapp/app/exercises/programming/shared/code-editor/actions/code-editor-actions.component.html) still reference it with[routerLink]="repositoryLink", ensuring navigation remains intact.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for repository link usage in templates and components # Search for repository link usage in templates echo "Searching for repository link usage in templates..." rg -A 5 'repositoryLink' --glob '*.html' # Search for repository link initialization in components echo "Searching for repository link initialization in components..." rg -A 5 'repositoryLink\s*=' --glob '*.ts'Length of output: 2429
src/test/javascript/spec/component/localvc/repository-view.component.spec.ts (1)
159-159: LGTM! Test expectations updated correctly.The test expectations have been updated to reflect that
component.participationis now initialized as an empty object for TESTS and AUXILIARY repository types, maintaining test coverage for the component's behavior.Also applies to: 198-198
src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-instructor-base-container.component.ts (1)
18-18: Great improvement in type safety!The changes replace string literals with the
RepositoryTypeenum, which:
- Enhances type safety by preventing typos in repository type strings
- Improves maintainability through centralized repository type definitions
- Makes the code more refactor-friendly
Also applies to: 65-65, 98-101, 190-191, 196-196, 208-209, 212-213, 216-217, 330-330
src/test/javascript/spec/integration/code-editor/code-editor-instructor.integration.spec.ts (1)
253-253: LGTM! Test expectations updated correctly.The test expectations have been updated to use the
RepositoryTypeenum, improving type safety in the tests.Also applies to: 317-317, 331-331, 364-364
src/main/webapp/app/exercises/programming/manage/services/programming-exercise.service.ts (1)
25-25: LGTM! Service layer updated for type safety.The changes enhance type safety by:
- Using the
RepositoryTypeenum instead of string literals- Ensuring consistent repository type handling across the service layer
Also applies to: 437-438
src/main/webapp/app/exam/manage/exam-management.route.ts (1)
277-278: LGTM! Routing configuration improvements.The changes improve the routing structure by:
- Using consistent repository type parameters
- Utilizing
repositorySubRoutesfor better code organization- Maintaining consistent patterns across different routes
Also applies to: 280-282, 586-588, 590-592
src/main/webapp/app/exercises/programming/manage/programming-exercise-detail.component.ts (1)
407-407: LGTM! Enhanced type safety withRepositoryTypeenum.The changes improve code quality by:
- Replacing string literals with enum values
- Enhancing type safety
- Making the code more maintainable
Also applies to: 416-416, 425-425
src/main/webapp/app/detail-overview-list/components/programming-auxiliary-repository-buttons-detail/programming-auxiliary-repository-buttons-detail.component.html (1)
10-10: LGTM! Improved type safety in template bindings.The changes enhance the template by:
- Using
RepositoryTypeenum instead of string literals- Maintaining consistency with the rest of the codebase
- Improving type safety in template bindings
Also applies to: 15-15
src/main/webapp/app/exam/participate/summary/exercises/programming-exam-summary/programming-exam-summary.component.html (1)
14-18: Updated Routing for Repository View
The routing logic for the repository view has been updated with a conditional expression based onisInCourseManagement. When true, the array route includes'USER'as a parameter; otherwise, it falls back to the standard routing path. This change enhances the flexibility of navigation depending on the view context.Note: Please verify that the hardcoded
'USER'segment is intentional and that downstream components interpret this parameter correctly.src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-instructor-and-editor-container.component.html (8)
20-20: Buildable Condition Update Using RepositoryType
The binding[buildable]="selectedRepository !== RepositoryType.TESTS"clearly indicates that repositories of typeTESTSare not buildable. This aligns with the expected business logic for repository operations.
48-51: Template Repository Button Styling
The template repository selection button now uses[style.background-color]="selectedRepository === RepositoryType.TEMPLATE ? '#3e8acc' : 'transparent'". This provides clear visual feedback for selection. Ensure that this color choice aligns with the overall UI theme.
55-59: Solution Repository Button Styling Consistency
Similarly, the solution repository button styling updates to useRepositoryType.SOLUTIONwith a theme variable (var(--link-color)). This update is consistent and visually coherent.
63-67: Assignment Participation Button Update
The assignment participation button now checks forRepositoryType.ASSIGNMENTin its styling logic. Confirm that both the disabled state and background-color conditions are aligned with the intended UI behavior.
80-85: Test Repository Button Styling
The test repository button is styled using[style.background-color]="selectedRepository === RepositoryType.TESTS ? 'var(--link-color)' : 'transparent'". This ensures uniformity across repository buttons.
129-133: Conditional Display of Updating Result Component
Thejhi-updating-resultcomponent is conditionally displayed only whenselectedRepositoryis notRepositoryType.TESTS, which aligns with the behavior that test repositories are non-buildable.
139-141: Conditional Display of IDE Build Trigger
Similarly, the IDE build trigger button is only displayed when the selected repository is not of typeTESTS. This conditional logic is appropriate and prevents unnecessary actions for non-buildable repositories.
143-153: Overall Code Editor Container and Editable Instructions
The code editor container and the subsequent editable instructions are well structured with the updated repository type checks. Ensure that any changes in repository handling are also reflected in the component’s TypeScript logic and services, maintaining consistency across the application.src/main/webapp/app/exercises/programming/manage/programming-exercise-detail.component.html (1)
23-23: Routing Update in "Edit in Editor" Button
Adding the static"TEMPLATE"segment to the router link makes the route explicit and aligns with the new RepositoryType (and similar enum-based) approach used elsewhere in the application. Please verify that the downstream route is adapted to expect this new segment.src/main/webapp/app/overview/exercise-details/exercise-details-student-actions.component.html (1)
125-130: Explicit Route Construction for Repository View
The updated routerLink for the repository view now uses an explicit array (i.e.
['/courses', courseId, 'exercises', exercise.id!, 'repository', exercise.studentParticipations![0].id!]) which enhances clarity and type safety. Please confirm that this new construction correctly reflects the intended navigation path.src/main/webapp/app/exercises/shared/exercise-scores/exercise-scores.component.html (1)
311-318: Relative Routing for Repository View in Scores Component
Switching from an absolute route (using course-related segments) to the relative route['..', 'repository', 'USER', value.id]standardizes repository navigation for programming exercises. Ensure that the parent route correctly resolves the relative path and that the added"USER"segment is correctly handled.src/main/webapp/app/exercises/shared/participation/participation.component.html (1)
129-132: Standardized Repository View Routing in Participation Component
Changing the routerLink from[row.id, 'repository']to the relative form['..', 'repository', 'USER', row.id]enhances consistency across the application. Confirm that this updated relative path works as expected within the current routing context.
src/main/webapp/app/localvc/commit-details-view/commit-details-view.component.ts
Show resolved
Hide resolved
...mponent/programming-exercise/programming-exercise-instructor-repo-download-component.spec.ts
Outdated
Show resolved
Hide resolved
...n/webapp/app/orion/management/code-editor-instructor-and-editor-orion-container.component.ts
Outdated
Show resolved
Hide resolved
...ipt/spec/component/orion/orion-code-editor-instructor-and-editor-container.component.spec.ts
Outdated
Show resolved
Hide resolved
...webapp/app/exercises/programming/shared/code-editor/actions/code-editor-actions.component.ts
Show resolved
Hide resolved
src/test/javascript/spec/integration/code-editor/code-editor-instructor.integration.spec.ts
Outdated
Show resolved
Hide resolved
SindiBuklaji
temporarily deployed
to
artemis-test5.artemis.cit.tum.de
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/test/javascript/spec/integration/code-editor/code-editor-instructor.integration.spec.ts (1)
95-95: Remove unused constant.The
courseIdconstant is declared but never used in the test file.- const courseId = 1;🧰 Tools
🪛 GitHub Check: client-tests
[failure] 95-95:
'courseId' is declared but its value is never read.🪛 GitHub Check: client-tests-selected
[failure] 95-95:
'courseId' is declared but its value is never read.src/main/webapp/app/localvc/commit-history/commit-history.component.ts (2)
56-62: Consider using RepositoryType enum for default value.Instead of using the string literal
'USER', use the enum value for consistency.- this.repositoryType = params['repositoryType'] ?? 'USER'; + this.repositoryType = params['repositoryType'] ?? RepositoryType.USER;
133-139: Use RepositoryType enum for string comparisons.For consistency and type safety, use enum values instead of string literals in the conditions.
- if (this.repositoryType === 'USER') { + if (this.repositoryType === RepositoryType.USER) { this.handleParticipationCommits(); - } else if (this.repositoryType === 'AUXILIARY') { + } else if (this.repositoryType === RepositoryType.AUXILIARY) { this.handleAuxiliaryRepositoryCommits(); } else { this.handleTemplateSolutionTestRepositoryCommits(); }src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-instructor-base-container.component.ts (2)
86-86: Consider adding type safety for route parameters.The route parameters could benefit from type safety using a strongly-typed route configuration.
interface RepositoryRouteParams { exerciseId: string; repositoryType: RepositoryType; repositoryId: string; } // Then use it in the route subscription this.paramSub = this.route!.params.subscribe((params: RepositoryRouteParams) => { const exerciseId = Number(params.exerciseId); const repositoryType = params.repositoryType; const repositoryId = Number(params.repositoryId); // ... });Also applies to: 98-101
274-274: Consider extracting route segments to constants.The navigation paths could be more maintainable if route segments were extracted to constants.
private readonly ROUTE_SEGMENTS = { PARENT: '../..', TEST_PLACEHOLDER: 'test', } as const; // Usage example: this.router.navigate([this.ROUTE_SEGMENTS.PARENT, RepositoryType.TEMPLATE, this.exercise.templateParticipation!.id], { relativeTo: this.route });Also applies to: 281-281, 288-288, 296-296, 303-303
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-instructor-base-container.component.ts(8 hunks)src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-management-routing.module.ts(1 hunks)src/main/webapp/app/localvc/commit-history/commit-history.component.ts(5 hunks)src/test/javascript/spec/component/orion/orion-code-editor-instructor-and-editor-container.component.spec.ts(3 hunks)src/test/javascript/spec/integration/code-editor/code-editor-instructor.integration.spec.ts(9 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/test/javascript/spec/component/orion/orion-code-editor-instructor-and-editor-container.component.spec.ts
- src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-management-routing.module.ts
🧰 Additional context used
📓 Path-based instructions (3)
src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-instructor-base-container.component.ts (1)
src/main/webapp/app/localvc/commit-history/commit-history.component.ts (1)
src/test/javascript/spec/integration/code-editor/code-editor-instructor.integration.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
🪛 GitHub Check: client-tests
src/test/javascript/spec/integration/code-editor/code-editor-instructor.integration.spec.ts
[failure] 95-95:
'courseId' is declared but its value is never read.
🪛 GitHub Check: client-tests-selected
src/test/javascript/spec/integration/code-editor/code-editor-instructor.integration.spec.ts
[failure] 95-95:
'courseId' is declared but its value is never read.
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: server-tests
- GitHub Check: Call Build Workflow / Build and Push Docker Image
- GitHub Check: Analyse
🔇 Additional comments (7)
src/test/javascript/spec/integration/code-editor/code-editor-instructor.integration.spec.ts (3)
213-215: LGTM! Clean implementation of flexible test initialization.The updated
initContainerfunction with optional route parameters improves test flexibility and maintainability.
252-252: LGTM! Consistent use of RepositoryType enum.The test cases have been correctly updated to use the centralized
RepositoryTypeenum and the new routing structure.Also applies to: 307-309, 316-316, 356-359, 364-364
432-491: LGTM! Comprehensive test coverage for repository navigation.The repository navigation test cases have been thoroughly updated to use the
RepositoryTypeenum and cover all navigation scenarios.src/main/webapp/app/localvc/commit-history/commit-history.component.ts (1)
15-15: LGTM! Good transition to strongly-typed enum.The change from string literals to
RepositoryTypeenum improves type safety and maintainability.Also applies to: 33-34
src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-instructor-base-container.component.ts (3)
18-18: LGTM! Good use of centralized enum.The transition to using the centralized
RepositoryTypeenum improves maintainability and type safety.Also applies to: 65-65
190-190: LGTM! Consistent use of RepositoryType enum.The assignments of
selectedRepositoryconsistently use theRepositoryTypeenum values.Also applies to: 196-196, 208-208, 212-212, 216-216
331-331: LGTM! Proper enum comparison.The condition correctly uses the
RepositoryTypeenum for comparison.
SimonEntholzer
temporarily deployed
to
artemis-test5.artemis.cit.tum.de
GitHub Actions
Inactive
helios-aet
bot
temporarily deployed
to
artemis-test1.artemis.cit.tum.de
helios-aet
bot
temporarily deployed
to
artemis-test6.artemis.cit.tum.de
helios-aet
bot
temporarily deployed
to
artemis-test1.artemis.cit.tum.de
helios-aet
bot
temporarily deployed
to
artemis-test6.artemis.cit.tum.de
SimonEntholzer
dismissed stale reviews from flbrgit, sachmii, simonbohnen, magaupp, az108, and coderabbitai[bot]
via
dffa7f9
krusche
changed the title
Programming exercises: Fix repository view routingProgramming exercises: Improve repository view routing
krusche
added
ready to merge
maintainer-approved
krusche
deleted the
feature/programming-exercises/fix-repository-view-routing
branch
Checklist
General
Client
authoritiesto all new routes and checked the course groups for displaying navigation elements (links, buttons).Changes affecting Programming Exercises
Motivation and Context
The routing to the repository view (iamge below), which is accessible through every code button, is broken in many places, very convoluted and inconsistent.
Description
Routing: From the many different ways the repository view was reached before, there are two main routes now distinguished by student and course management view:
/courses/:courseId/exercises/:exerciseId/repository/:repositoryId(repositoryId is synonymous with participation Id in this case)course-management/:courseId/programming-exercises/:exerciseId/repository/:repositoryType/:repositoryIdrepositoryIdSteps for Testing
Basically make sure, that in any place with a code button, the routing to the repository works correctly.
Check that you can access the repository view from every sub page of the
edit in editorinstructor view.->
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
Client
Screenshots
Refactored routing:
Button in the
edit in editorview:Summary by CodeRabbit
New Features
Refactor