Integrated code lifecycle: Add editing of auxiliary repositories in the online code editor for instructors
#9585
Conversation
github-actions
bot
added
server
WalkthroughThe pull request introduces enhancements to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (3)
src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-instructor-and-editor-container.component.html (1)
84-97: Consider adding translations and improving disabled state feedback.The implementation is solid and follows the new Angular syntax guidelines correctly. However, there are two potential improvements:
- Add translation support for consistency with other repository buttons
- Enhance the disabled state feedback
Apply these changes to improve the implementation:
@for (auxiliaryRepository of exercise.auxiliaryRepositories; track exercise.auxiliaryRepositories; let i = $index) { @if (auxiliaryRepository.id) { <button - [disabled]="!exercise" + [disabled]="!exercise || !auxiliaryRepository.repositoryUrl" (click)="selectAuxiliaryRepository(auxiliaryRepository.id)" ngbDropdownItem [style.background-color]=" selectedRepository === REPOSITORY.AUXILIARY && selectedRepositoryId === auxiliaryRepository.id ? '#3e8acc' : 'transparent' " > - {{ auxiliaryRepository.name }} + <span [jhiTranslate]="'artemisApp.editor.repoSelect.auxiliaryRepo'" [translateValues]="{ name: auxiliaryRepository.name }"></span> </button> } }Add this translation key to your i18n files:
{ "artemisApp.editor.repoSelect.auxiliaryRepo": "Auxiliary Repository: {{name}}" }src/main/java/de/tum/cit/aet/artemis/programming/repository/ProgrammingExerciseRepository.java (1)
779-782: LGTM, but consider refactoring the "dark hack".The changes correctly integrate auxiliary repositories into the data retrieval. However, as noted in the TODO comment, this method uses a hack to load participations separately.
Consider refactoring this into a service layer that properly handles the data loading in a more maintainable way:
- ProgrammingExercise programmingExerciseWithAuxiliaryRepositories = findByIdWithAuxiliaryRepositoriesElseThrow(programmingExerciseId); - - programmingExerciseWithTemplate.setAuxiliaryRepositories(programmingExerciseWithAuxiliaryRepositories.getAuxiliaryRepositories()); + // Move to ProgrammingExerciseService + @Transactional(readOnly = true) + public ProgrammingExercise getProgrammingExerciseWithAllParticipations(Long exerciseId) { + ProgrammingExercise exercise = programmingExerciseRepository + .findWithTemplateParticipationLatestResultFeedbackTestCasesById(exerciseId) + .orElseThrow(() -> new EntityNotFoundException("Programming exercise not found")); + + // Load solution participation and auxiliary repositories in a single query + ProgrammingExercise exerciseWithSolutionAndAuxRepos = programmingExerciseRepository + .findWithSolutionParticipationAndAuxiliaryRepositoriesById(exerciseId) + .orElseThrow(() -> new EntityNotFoundException("Programming exercise not found")); + + exercise.setSolutionParticipation(exerciseWithSolutionAndAuxRepos.getSolutionParticipation()); + exercise.setAuxiliaryRepositories(exerciseWithSolutionAndAuxRepos.getAuxiliaryRepositories()); + + return exercise; + }src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-instructor-base-container.component.ts (1)
31-31: Consider using PascalCase for enum membersThe
REPOSITORYenum uses uppercase naming for its members. According to the coding guidelines, enums should use PascalCase. Renaming the enum members to PascalCase will improve consistency.Apply this diff to update the enum member names:
export enum REPOSITORY { - ASSIGNMENT = 'ASSIGNMENT', - TEMPLATE = 'TEMPLATE', - SOLUTION = 'SOLUTION', - TEST = 'TEST', - AUXILIARY = 'AUXILIARY', + Assignment = 'ASSIGNMENT', + Template = 'TEMPLATE', + Solution = 'SOLUTION', + Test = 'TEST', + Auxiliary = 'AUXILIARY', }Ensure to update all references to these enum members throughout the codebase to reflect the changes.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (6)
- src/main/java/de/tum/cit/aet/artemis/programming/repository/ProgrammingExerciseRepository.java (1 hunks)
- src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-instructor-and-editor-container.component.html (2 hunks)
- src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-instructor-and-editor-container.component.ts (1 hunks)
- src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-instructor-and-editor-container.scss (1 hunks)
- src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-instructor-base-container.component.ts (6 hunks)
- src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-management-routing.module.ts (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-instructor-and-editor-container.scss
🧰 Additional context used
📓 Path-based instructions (5)
src/main/java/de/tum/cit/aet/artemis/programming/repository/ProgrammingExerciseRepository.java (1)
Pattern
src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/webapp/app/exercises/programming/manage/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/exercises/programming/manage/code-editor/code-editor-instructor-and-editor-container.component.ts (1)
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-management-routing.module.ts (1)
🔇 Additional comments (5)
src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-instructor-and-editor-container.component.ts (2)
20-20: LGTM! The styleUrl addition follows Angular best practices.The addition of the styleUrl property follows Angular's component styling best practices and adheres to the coding guidelines.
Line range hint
1-50: Verify auxiliary repository handling implementation.Given that this PR's main objective is to add support for auxiliary repositories editing, we should verify that this functionality is properly implemented, either in this component or its parent class.
Let's verify the implementation:
✅ Verification successful
Let me gather more information about the auxiliary repository implementation by checking the base component and related files:
Based on the comprehensive evidence from the shell script outputs, I can now provide the final response:
Auxiliary repository support is properly implemented in this component and its parent class
The implementation includes:
- Proper routing setup for auxiliary repositories via
auxiliary/:repositoryIdpath- Complete UI integration in the repository dropdown menu
- Full handling in the base component including:
- Repository selection logic
- Domain change management
- Navigation methods
- State management for auxiliary repositories
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for auxiliary repository handling in the base component and related files # Check base component implementation echo "Checking base component for auxiliary repository handling..." rg -A 5 "auxiliary.*repository" "src/main/webapp/app/exercises/programming/manage/code-editor/" # Check for route configuration echo "Checking route configuration for auxiliary repository..." rg -A 5 "auxiliary/:repositoryId" "src/main/webapp/app/exercises/programming/" # Check for related service methods echo "Checking service implementations..." ast-grep --pattern 'class $_ { $$$ auxiliary$_($$$) { $$$ } $$$ }'Length of output: 5135
Script:
#!/bin/bash # Check the base component implementation echo "Checking CodeEditorInstructorBaseContainerComponent..." cat src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-instructor-base-container.component.ts # Check for auxiliary repository related models and interfaces echo -e "\nChecking for auxiliary repository models..." rg -l "auxiliaryRepositories?" src/main/webapp/app/exercises/programming/ # Check for auxiliary repository related template echo -e "\nChecking template for auxiliary repository UI..." cat src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-instructor-and-editor-container.component.htmlLength of output: 28924
src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-management-routing.module.ts (1)
57-68: LGTM! Route configuration follows established patterns.The new route for auxiliary repositories maintains consistency with existing routes in terms of:
- Authorization controls
- Cache configuration
- Component reuse
- Data structure
src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-instructor-and-editor-container.component.html (1)
40-42: LGTM: Repository selection button implementation is clean and functional.The button text dynamically updates to show either the auxiliary repository name or the selected repository type, providing clear visual feedback to users.
src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-instructor-base-container.component.ts (1)
68-69: Properties are correctly named using camelCaseThe new properties
selectedRepositoryIdandselectedAuxiliaryRepositoryNamefollow the camelCase naming convention, adhering to the coding guidelines.
...webapp/app/exercises/programming/manage/code-editor/code-editor-management-routing.module.ts
Show resolved
Hide resolved
.../exercises/programming/manage/code-editor/code-editor-instructor-base-container.component.ts
Show resolved
Hide resolved
.../exercises/programming/manage/code-editor/code-editor-instructor-base-container.component.ts
Show resolved
Hide resolved
.../exercises/programming/manage/code-editor/code-editor-instructor-base-container.component.ts
Show resolved
Hide resolved
.../exercises/programming/manage/code-editor/code-editor-instructor-base-container.component.ts
Outdated
Show resolved
Hide resolved
…ode-editor-instructor-base-container.component.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
SimonEntholzer
requested review from
JohannesStoehr,
asliayk,
BBesrour,
b-fein and
cremertim
sachmii
temporarily deployed
to
artemis-test4.artemis.cit.tum.de
GitHub Actions
Inactive
src/main/java/de/tum/cit/aet/artemis/programming/repository/ProgrammingExerciseRepository.java
Show resolved
Hide resolved
...es/programming/manage/code-editor/code-editor-instructor-and-editor-container.component.html
Outdated
Show resolved
Hide resolved
|
github-actions
bot
added
the
deployment-error
SimonEntholzer
temporarily deployed
to
artemis-test4.artemis.cit.tum.de
GitHub Actions
Inactive
SimonEntholzer
dismissed stale reviews from JohannesStoehr, coderabbitai[bot], Feras797, sachmii, HawKhiem, and sarpsahinalp
via
e230bc4
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- src/test/javascript/spec/integration/code-editor/code-editor-instructor.integration.spec.ts (12 hunks)
🧰 Additional context used
📓 Path-based instructions (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}}
🔇 Additional comments (3)
src/test/javascript/spec/integration/code-editor/code-editor-instructor.integration.spec.ts (3)
72-72: Variable renaming improves code clarity.
The renaming of container to comp follows Angular testing conventions and makes the code more readable. The subsequent usage maintains consistency throughout the test suite.
Also applies to: 147-147, 209-270
177-177: LGTM: Navigation spy setup follows Jest best practices.
The navigation spy is correctly set up using Jest's spyOn with proper injection of the Router service.
487-493: Test coverage for navigation path helper is comprehensive.
The up() method test cases effectively verify the navigation path logic for different repository types.
src/test/javascript/spec/integration/code-editor/code-editor-instructor.integration.spec.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
src/main/java/de/tum/cit/aet/artemis/programming/repository/ProgrammingExerciseRepository.java(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/main/java/de/tum/cit/aet/artemis/programming/repository/ProgrammingExerciseRepository.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
🔇 Additional comments (1)
src/main/java/de/tum/cit/aet/artemis/programming/repository/ProgrammingExerciseRepository.java (1)
791-792: LGTM: Method name accurately reflects its functionality.
The method name has been updated to include "AuxiliaryRepos", which clearly indicates the additional data being fetched.
src/main/java/de/tum/cit/aet/artemis/programming/repository/ProgrammingExerciseRepository.java
Show resolved
Hide resolved
krusche
changed the title
Integrated code lifecycle: Add editing of auxiliary repositories for instructorsIntegrated code lifecycle: Add editing of auxiliary repositories in the online code editor for instructors
krusche
deleted the
feature/auxiliary-repositories/enable-in-edit-in-editor
branch
Checklist
General
Server
Client
authoritiesto all new routes and checked the course groups for displaying navigation elements (links, buttons).Changes affecting Programming Exercises
Motivation and Context
Instructors can edit Template, Assignment, Solution and Test repositories in Artemis, by using the "edit in editor" view.
This is not yet possible for auxiliary repositories.
Description
This PR adds support for auxiliary repositories in the edit in editor view.
Also fixes a small UI bug, where the text on the drop-down button was not readable, when not hovering.
Steps for Testing
Testserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.
Review Progress
Code Review
Manual Tests
Test Coverage
Client
Server
Screenshots
small Bugfix:
vs.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Style
Chores