Programming exercises: Exchange of Artemis programming exercises via CodeAbility Sharing Platform
#9909
Conversation
github-actions
bot
added
server
Wallenstein61
changed the title
Programming exercises: Exchange of Artemis programming exercises via CodeAbility Sharing Platform
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 pmd (7.8.0)src/main/java/de/tum/cit/aet/artemis/core/config/Constants.javaThe following rules are missing or misspelled in your ruleset file category/vm/bestpractices.xml: BooleanInstantiation, DontImportJavaLang, DuplicateImports, EmptyFinallyBlock, EmptyIfStmt, EmptyInitializer, EmptyStatementBlock, EmptyStatementNotInLoop, EmptySwitchStatements, EmptySynchronizedBlock, EmptyTryBlock, EmptyWhileStmt, ExcessiveClassLength, ExcessiveMethodLength, ImportFromSamePackage, MissingBreakInSwitch, SimplifyBooleanAssertion. Please check your ruleset configuration. src/main/java/de/tum/cit/aet/artemis/core/config/SecurityConfiguration.javaThe following rules are missing or misspelled in your ruleset file category/vm/bestpractices.xml: BooleanInstantiation, DontImportJavaLang, DuplicateImports, EmptyFinallyBlock, EmptyIfStmt, EmptyInitializer, EmptyStatementBlock, EmptyStatementNotInLoop, EmptySwitchStatements, EmptySynchronizedBlock, EmptyTryBlock, EmptyWhileStmt, ExcessiveClassLength, ExcessiveMethodLength, ImportFromSamePackage, MissingBreakInSwitch, SimplifyBooleanAssertion. Please check your ruleset configuration. src/main/java/de/tum/cit/aet/artemis/core/dto/SharingInfoDTO.javaThe following rules are missing or misspelled in your ruleset file category/vm/bestpractices.xml: BooleanInstantiation, DontImportJavaLang, DuplicateImports, EmptyFinallyBlock, EmptyIfStmt, EmptyInitializer, EmptyStatementBlock, EmptyStatementNotInLoop, EmptySwitchStatements, EmptySynchronizedBlock, EmptyTryBlock, EmptyWhileStmt, ExcessiveClassLength, ExcessiveMethodLength, ImportFromSamePackage, MissingBreakInSwitch, SimplifyBooleanAssertion. Please check your ruleset configuration.
WalkthroughThe pull request introduces enhancements to the Artemis application to facilitate integration with the CodeAbility sharing platform. Key changes include new dependencies in the build configuration, updates to environment variables, and the introduction of several classes and methods that support sharing functionalities. This includes REST controllers for managing sharing operations, new constants for resource paths, and modifications to existing services and components to accommodate the sharing feature. Changes
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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: 55
🧹 Outside diff range and nitpick comments (68)
src/main/java/de/tum/cit/aet/artemis/exercise/service/sharing/SharingException.java (1)
5-7: Enhance exception documentationThe current documentation could be more descriptive to help developers understand when and how to use this exception.
Consider expanding the JavaDoc:
/** - * Sharing Exception during import or export to sharing platform. + * Exception thrown during programming exercise import/export operations with the sharing platform. + * + * This exception is thrown when: + * - Connection to the sharing platform fails + * - Invalid exercise data is received + * - Authentication/authorization errors occur + * - Data validation fails during import/export + * + * @see de.tum.cit.aet.artemis.exercise.service.sharing.ProgrammingExerciseImportFromSharingService */src/main/webapp/app/sharing/sharing.module.ts (2)
1-5: Consider organizing imports according to Angular style guideWhile the imports are correctly implemented, consider organizing them in the following order:
- Angular framework imports
- Third-party imports
- Application imports
import { NgModule } from '@angular/core'; import { RouterModule } from '@angular/router'; + import { SharingComponent } from 'app/sharing/sharing.component'; import { featureOverviewState } from 'app/sharing/sharing.route'; import { ArtemisSharedModule } from 'app/shared/shared.module';
7-7: Add type annotation for routes constantConsider adding a type annotation for better type safety and code maintainability.
-const SHARING_ROUTES = [...featureOverviewState]; +const SHARING_ROUTES: Routes = [...featureOverviewState];src/main/webapp/app/sharing/sharing.route.ts (2)
5-14: Consider enhancing route security and performance.While the basic route configuration is correct, consider these improvements:
- Add a route guard for additional security checks
- Implement data resolvers for pre-loading sharing data
Example implementation:
export const sharingRoutes: Routes = [ { path: '', component: SharingComponent, + canActivate: [AuthGuard], + resolve: { + sharingData: SharingResolver + }, data: { pageTitle: 'artemisApp.sharing.title', authorities: [Authority.EDITOR, Authority.INSTRUCTOR, Authority.ADMIN], }, }, ];
16-23: Simplify the routing structure.The current implementation has unnecessary complexity:
SHARING_ROUTESconstant with spread operator is redundant for a single route- Consider simplifying the structure by directly using
sharingRoutesSuggested simplification:
-const SHARING_ROUTES = [...sharingRoutes]; - export const featureOverviewState: Routes = [ { path: '', - children: SHARING_ROUTES, + children: sharingRoutes, }, ];src/main/webapp/app/sharing/sharing.model.ts (2)
1-4: Consider using providedIn for better tree-shakingThe
@Injectable()decorator should specify where the service is provided. UsingprovidedIn: 'root'ensures a singleton instance and enables tree-shaking.-@Injectable() +@Injectable({ + providedIn: 'root' +})
1-27: Consider implementing state managementGiven that this sharing functionality is part of a larger feature connecting to an external platform, consider using NgRx or another state management solution to handle the sharing state more robustly. This would provide better control over state changes, especially for the shopping basket feature mentioned in the PR objectives.
Key benefits would include:
- Centralized state management
- Better debugging capabilities
- Predictable state updates
- Easier integration with the REST-based connector
src/main/java/de/tum/cit/aet/artemis/sharing/SharingSetupInfo.java (3)
9-13: Enhance class-level documentation.While the documentation provides a basic description, it could be more comprehensive by including:
- The purpose and use cases of this class
- The relationship with the sharing platform
- Example usage
/** - * the sharing info, wrapping the original sharing Info from the sharing platform and adding course and exercise info. + * Represents the setup information required for sharing programming exercises between Artemis and the sharing platform. + * This class combines the sharing information from the external platform with local course and exercise details, + * facilitating the import/export process. + * + * Usage example: + * <pre> + * SharingSetupInfo setupInfo = new SharingSetupInfo(); + * setupInfo.setCourse(course); + * setupInfo.setExercise(exercise); + * setupInfo.setSharingInfo(sharingInfoDTO); + * </pre> */
30-70: Consider adding null safety checks and improving method documentation.The getters and setters lack null validation, which could lead to NPEs. Also, the documentation could be more descriptive.
Example improvement for one getter/setter pair:
/** - * the exercise + * Returns the programming exercise associated with this sharing setup. + * + * @return the programming exercise, may be null if not set */ public ProgrammingExercise getExercise() { return exercise; } /** - * sets the exercise + * Sets the programming exercise for this sharing setup. + * + * @param exercise the programming exercise to set, must not be null + * @throws IllegalArgumentException if exercise is null */ public void setExercise(ProgrammingExercise exercise) { + if (exercise == null) { + throw new IllegalArgumentException("Exercise must not be null"); + } this.exercise = exercise; }
1-71: Consider implementing the Builder pattern for better object construction.Given that this class requires multiple fields to be set up, consider implementing the Builder pattern to ensure all required fields are provided and to make object creation more fluent.
Example implementation:
public class SharingSetupInfo { private final ProgrammingExercise exercise; private final Course course; private final SharingInfoDTO sharingInfo; private SharingSetupInfo(Builder builder) { this.exercise = builder.exercise; this.course = builder.course; this.sharingInfo = builder.sharingInfo; } // Getters only, no setters for immutability public static class Builder { private ProgrammingExercise exercise; private Course course; private SharingInfoDTO sharingInfo; public Builder exercise(ProgrammingExercise exercise) { this.exercise = exercise; return this; } public Builder course(Course course) { this.course = course; return this; } public Builder sharingInfo(SharingInfoDTO sharingInfo) { this.sharingInfo = sharingInfo; return this; } public SharingSetupInfo build() { // Validate required fields if (exercise == null || course == null || sharingInfo == null) { throw new IllegalStateException("All fields are required"); } return new SharingSetupInfo(this); } } }src/main/webapp/app/sharing/search-result-dto.model.ts (3)
1-9: Consider improving type safety and naming clarity
- The property
ranking5seems ambiguous. Consider a more descriptive name that explains its purpose.- Numeric fields should have explicit type annotations for better type safety.
export interface SearchResultDTO { project: ProjectDTO; file: MetadataFileDTO; metadata: UserProvidedMetadataDTO; - ranking5: number; + rankingScore: number; - views: number; + views: number; - downloads: number; + downloads: number; }
11-15: Consider using string literal types for action fieldTo improve type safety and prevent invalid action values, consider using a string literal type for the
actionfield.export interface PluginActionInfo { plugin: string; - action: string; + action: 'import' | 'export' | 'validate'; // Add relevant action types commandName: string; }
44-48: Add email validation patternConsider using a more specific type for the email field to ensure valid email addresses.
+type Email = string & { readonly brand: unique symbol }; + +function validateEmail(email: string): Email { + if (!/^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email)) { + throw new Error('Invalid email format'); + } + return email as Email; +} + export interface Person { name: string; - email: string; + email: Email; affiliation: string; }src/main/java/de/tum/cit/aet/artemis/sharing/SharingMultipartZipFile.java (2)
9-9: Consider standardizing to Jakarta EE annotationsThe code mixes Jakarta (
jakarta.validation.constraints.NotNull) and JavaEE (javax.annotation.Nonnull) annotations. Consider standardizing to Jakarta EE annotations for consistency.-import jakarta.validation.constraints.NotNull; -import javax.annotation.Nonnull; +import jakarta.annotation.Nonnull;Also applies to: 7-7
15-17: Enhance class documentationThe current documentation is minimal. Consider adding:
- Purpose of this class in the sharing workflow
- Explanation of why a custom MultipartFile implementation is needed
- Usage examples or context
src/main/java/de/tum/cit/aet/artemis/core/dto/SharingInfoDTO.java (2)
7-10: Enhance class documentationThe current documentation could be more descriptive. Consider adding details about:
- The sharing platform this DTO interacts with
- The purpose of the basket concept
- The relationship with the CodeAbility Project
/** - * the sharing info to request an specific exercise from the sharing platform. + * Data Transfer Object for requesting exercises from the CodeAbility sharing platform. + * Contains information needed for basket-based exercise sharing, including authentication + * tokens, callback URLs, and exercise position within the basket. */
28-31: Fix documentation typoThere's a typo in the exercisePosition field documentation.
/** - * the index of the request exercise + * the index of the requested exercise */ private int exercisePosition;src/main/webapp/app/exercises/programming/manage/update/programming-exercise-creation-config.ts (1)
25-25: LGTM! Consider adding JSDoc documentation.The new property follows the established naming pattern and integrates well with existing import-related flags.
Consider adding JSDoc documentation to describe the property's purpose:
+ /** Indicates whether the exercise is being imported from the sharing platform */ isImportFromSharing: boolean;src/main/webapp/app/exercises/programming/shared/actions/programming-exercise-instructor-exercise-sharing.component.ts (2)
11-22: Consider extracting template to separate file for better maintainability.While inline templates are acceptable for simple components, as this component grows, maintaining the template in a separate file would improve readability and allow for better IDE support.
41-48: Remove commented code.The commented-out code in
preOpenSharingTabshould be removed if it's not being used. If this is a work in progress, consider using a TODO comment instead.-preOpenSharingTab() { - // the focus back to this window is not working, so we open in this window - /* - if(!this.sharingTab) { - this.sharingTab = window.open("about:blank", "sharing"); - } - */ -}src/main/java/de/tum/cit/aet/artemis/core/service/ProfileService.java (1)
134-141: Rename method and relocate to subsystem profiles section.To maintain consistency with other methods in this class:
- Rename the method to include "Active" suffix
- Move it to the "Sub-system profiles" section with other similar profile checks (around line 100)
- public boolean isSharing() { + public boolean isSharingActive() { return isProfileActive("sharing"); }src/main/webapp/app/exercises/programming/manage/programming-exercise-management.module.ts (1)
21-21: Consider lazy loading for the sharing feature.The import looks good and follows Angular conventions. However, since this is part of a new sharing feature, consider implementing lazy loading to optimize the initial bundle size.
Example approach:
// Create a separate sharing feature module const routes: Routes = [ { path: 'sharing', loadChildren: () => import('./sharing/sharing.module').then(m => m.SharingModule) } ];src/main/webapp/app/sharing/sharing.scss (3)
1-6: Enhance layout robustness and font fallbacksConsider the following improvements to strengthen the base layout:
body { margin: 0; padding: 0; - font-family: 'Ubuntu', sans-serif; + font-family: 'Ubuntu', system-ui, -apple-system, sans-serif; display: flex; + flex-direction: column; + min-height: 100vh; } .header { position: relative; text-align: center; background: #3070b3; + width: 100%; }Also applies to: 8-30
120-187: Add animation variables and respect user preferencesEnhance animation consistency and accessibility by implementing CSS variables and respecting user preferences.
+:root { + --transition-duration: 0.5s; + --primary-color: #3070b3; +} + +@media (prefers-reduced-motion: reduce) { + :root { + --transition-duration: 0.01s; + } +} .container .box .icon { position: absolute; top: 0; left: 0; width: 100%; height: 100%; background: #f2f2f2; - transition: 0.5s; + transition: all var(--transition-duration) ease-in-out; z-index: 1; }
244-262: Modernize utility classesReplace float-based utilities with modern flexbox/grid alternatives for better maintainability and reliability.
-.text-wrap-right { - float: right; - margin-left: 15px; - margin-bottom: 7px; +.text-wrap-right { + display: flex; + justify-content: flex-end; + gap: 15px; + margin-bottom: 7px; } -.text-wrap-left { - clear: right; - float: left; - margin-right: 15px; - margin-bottom: 7px; +.text-wrap-left { + display: flex; + justify-content: flex-start; + gap: 15px; + margin-bottom: 7px; } -.center { - clear: right; - display: block; - margin: 30px auto; - width: 50%; +.center { + display: flex; + justify-content: center; + align-items: center; + margin: 30px 0; + width: min(50%, 100%); }src/main/webapp/app/sharing/sharing.component.html (3)
1-9: Enhance table structure and stylingSeveral improvements can be made to the table structure:
- Move inline styles to a separate CSS file for better maintainability
- Add ARIA labels and roles for better accessibility
- Consider using CSS Grid or Flexbox instead of table layout for better responsiveness
-<table style="border-spacing: 50px 0px; border-collapse: separate"> +<table class="sharing-table" role="grid" aria-label="Exercise sharing options">
14-14: Move hardcoded German text to translation fileThe German text "Keine importierten Aufgaben gefunden" should be moved to the translation file and referenced using the translation key.
-<div *ngIf="!shoppingBasket" jhiTranslate="artemisApp.sharing.courseToImport">Keine importierten Aufgaben gefunden</div> +<div *ngIf="!shoppingBasket" jhiTranslate="artemisApp.sharing.noImportedTasks"></div>
65-67: Improve color representation and accessibilityThe color display implementation has several issues:
- Move the inline styles to CSS
- Add ARIA labels for color indication
- Consider moving ARTEMIS_DEFAULT_COLOR to a configuration file
-<div [ngStyle]="{ backgroundColor: course.color || ARTEMIS_DEFAULT_COLOR, width: '15px', height: '20px' }"> </div> +<div + class="course-color-indicator" + [style.backgroundColor]="course.color || ARTEMIS_DEFAULT_COLOR" + [attr.aria-label]="'Course color: ' + (course.color || 'default')" +> </div>src/main/webapp/app/app-routing.module.ts (1)
186-189: Consider grouping sharing routes together.For better maintainability, consider grouping the sharing route with related functionality or creating a dedicated section for sharing features, similar to how other features like "TEAM" and "ACCOUNT" are organized.
- { - path: 'sharing/import/:basketToken', - loadChildren: () => import('./sharing/sharing.module').then((m) => m.SharingModule), - }, + // ===== SHARING ===== + { + path: 'sharing/import/:basketToken', + loadChildren: () => import('./sharing/sharing.module').then((m) => m.SharingModule), + },src/main/webapp/app/exercises/programming/manage/programming-exercise-management-routing.module.ts (1)
98-109: Consider consolidating import routesThis route follows a similar pattern to other import routes. Consider consolidating the import routes using a shared parent route with child routes to reduce duplication and improve maintainability.
Example refactoring approach:
{ path: ':courseId/programming-exercises/import', data: { authorities: [Authority.EDITOR, Authority.INSTRUCTOR, Authority.ADMIN], }, children: [ { path: 'from-sharing', component: ProgrammingExerciseUpdateComponent, resolve: { programmingExercise: ProgrammingExerciseResolve, }, data: { pageTitle: 'artemisApp.programmingExercise.home.importLabel', } }, // other import routes as children ] }src/main/java/de/tum/cit/aet/artemis/core/config/Constants.java (1)
379-382: Improve Javadoc quality and consistencyThe Javadoc comments for the new constants could be more descriptive and consistent with other constants in the file:
- Use proper sentence structure
- Add more context about the purpose
- Fix typos ("configution" → "configuration")
Consider updating the Javadocs:
/** - * sharing configution resource path for sharing config request + * The REST API endpoint path for retrieving sharing configuration. + * Used by the sharing platform integration to manage configuration settings. */ public static final String SHARINGCONFIG_RESOURCE_PATH = "/sharing/config"; /** - * sharing configution resource path for sharing config import request + * The REST API endpoint path for importing shared programming exercises. + * Used by the sharing platform integration to import exercises from other institutions. */ public static final String SHARINGIMPORT_RESOURCE_PATH = "/sharing/import"; /** - * sharing configution resource path for sharing config export request + * The REST API endpoint path for exporting programming exercises. + * Used by the sharing platform integration to share exercises with other institutions. */ public static final String SHARINGEXPORT_RESOURCE_PATH = "/sharing/export"; /** - * sharing configution resource path for rest request, iff sharing profile is enabled + * The REST API endpoint path for checking if the sharing profile is enabled. + * Used to determine if the sharing platform integration features are available. */ public static final String SHARINGCONFIG_RESOURCE_IS_ENABLED = SHARINGCONFIG_RESOURCE_PATH + "/is-enabled";Also applies to: 384-387, 389-392, 394-397
build.gradle (1)
220-222: Consider adding repository credentials configuration.The Maven repository for the sharing platform connector might require authentication in production. Consider making the repository URL and credentials configurable through environment variables or a secure configuration mechanism.
maven { // required for sharing platform connector - url 'https://sharing-codeability.uibk.ac.at/api/v4/projects/26/packages/maven' + url = project.hasProperty('sharingPlatformUrl') ? project.property('sharingPlatformUrl') : 'https://sharing-codeability.uibk.ac.at/api/v4/projects/26/packages/maven' + credentials { + username = project.hasProperty('sharingPlatformUser') ? project.property('sharingPlatformUser') : '' + password = project.hasProperty('sharingPlatformPassword') ? project.property('sharingPlatformPassword') : '' + } }src/main/webapp/app/exercises/programming/manage/programming-exercise-detail.component.ts (1)
Line range hint
1-1000: Consider splitting this large component in future refactoring.This component has grown quite large (1000+ lines) with multiple responsibilities. While the current changes for sharing platform integration are focused and well-implemented, consider splitting this component into smaller, more focused components in future refactoring efforts. This would improve maintainability and testability.
Potential splits could include:
- Exercise General Details Component
- Exercise Language Details Component
- Exercise Grading Details Component
- Exercise Problem Details Component
src/main/webapp/i18n/de/programmingExercise.json (1)
759-762: Add descriptions to sharing translations.The translations for the sharing functionality are too brief and could benefit from more descriptive text to better guide users.
Consider expanding the translations to be more descriptive:
"sharing": { - "import": "Programmieraufgabe aus Sharing importieren", - "export": "Exportieren nach Sharing" + "import": "Programmieraufgabe von der Sharing-Plattform importieren", + "export": "Programmieraufgabe zur Sharing-Plattform exportieren" }src/main/java/de/tum/cit/aet/artemis/exercise/service/sharing/ProgrammingExerciseImportFromSharingService.java (2)
33-34: Correct the grammatical error in the field comment.In the comment on lines 33-34, it should read "this service strongly relies on the file import format" instead of "relies in the file import format."
67-75: Consider wrapping low-level exceptions into a custom exception.Exposing low-level exceptions like
IOException,GitAPIException,URISyntaxExceptionmay leak implementation details. Consider wrapping these exceptions into a higher-level custom exception (e.g.,SharingException) to provide a cleaner API and maintain abstraction.src/main/java/de/tum/cit/aet/artemis/core/web/SharingSupportResource.java (4)
50-50: Remove unnecessary@SuppressWarnings("unused")annotationThe
@SuppressWarnings("unused")annotation on the constructor is unnecessary because the parametersharingPluginServiceis being used. Removing this annotation keeps the code clean and avoids suppressing legitimate warnings.Apply this diff to remove the annotation:
- @SuppressWarnings("unused") public SharingSupportResource(SharingConnectorService sharingPluginService) { this.sharingPluginService = sharingPluginService; }
83-83: UseHttpStatus.UNAUTHORIZEDconstant instead of hardcoded status codeInstead of using the hardcoded status code
401, use theHttpStatus.UNAUTHORIZEDconstant for better readability and maintainability.Apply this diff to make the change:
- return ResponseEntity.status(401).body(null); + return ResponseEntity.status(HttpStatus.UNAUTHORIZED).body(null);
94-96: Return consistent HTTP status codes and simplify responsesReturning
503 Service Unavailablemay not be appropriate in this context, as it indicates the server is temporarily unable to handle requests. Consider returning200 OKwith a boolean body indicating the sharing status. Additionally, you can simplify the response by usingResponseEntity.ok().Apply this diff to update the method:
@GetMapping(SHARINGCONFIG_RESOURCE_IS_ENABLED) public ResponseEntity<Boolean> isSharingEnabled() { if (sharingPluginService.isSharingApiBaseUrlPresent()) { - return ResponseEntity.status(200).body(true); + return ResponseEntity.ok(true); } - return ResponseEntity.status(503).body(false); + return ResponseEntity.ok(false); }
72-72: Enhance logging with additional contextIncluding more details in the log messages improves traceability and debugging. Consider adding relevant information such as the
installationNameorapiBaseUrl.Apply this diff to enhance the log message:
- log.info("Delivered Sharing Config "); + log.info("Delivered Sharing Config for installation: {}", installationName);src/main/webapp/app/sharing/sharing.component.ts (3)
86-86: Simplify null checks using optional chainingYou can simplify the null check for
selectedCourseand itsidby using optional chaining.Apply this diff to streamline the condition:
- if (this.selectedCourse && this.selectedCourse.id) { + if (this.selectedCourse?.id) {🧰 Tools
🪛 Biome (1.9.4)
[error] 86-86: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
101-103: Specify the return type for the 'trackId' methodFor better type safety and code clarity, it's recommended to specify the return type of methods explicitly.
Apply this diff to add the return type:
- trackId(index: number, item: Course) { + trackId(index: number, item: Course): number {
105-107: Add explicit return type to the 'sortRows' methodAdding explicit return types improves readability and maintainability of the code.
Apply this diff:
- sortRows() { + sortRows(): void {src/main/java/de/tum/cit/aet/artemis/exercise/service/sharing/SharingConnectorService.java (4)
135-135: Typo in method comment: "authorizaion" should be "authorization"There's a spelling error in the method comment. Correcting it enhances readability and professionalism.
156-156: Typo in comment: "plattform" should be "platform"There's a minor spelling mistake in the inline comment. Please correct it for clarity.
35-35: Avoid exposing mutable shared state without synchronizationThe
sharingApiBaseUrlis a mutable object shared across methods. Directly exposing it without proper synchronization can lead to thread-safety issues.Suggested Improvement:
Instead of using
URL, consider using an immutable object or synchronizing access:
Use
StringInstead ofURL:- private URL sharingApiBaseUrl = null; + private String sharingApiBaseUrl = null;Synchronize Access:
public synchronized void setSharingApiBaseUrl(URL sharingApiBaseUrl) { this.sharingApiBaseUrl = sharingApiBaseUrl; } public synchronized URL getSharingApiBaseUrlOrNull() { return sharingApiBaseUrl; }Use Atomic Reference:
- private URL sharingApiBaseUrl = null; + private final AtomicReference<URL> sharingApiBaseUrl = new AtomicReference<>(); public void setSharingApiBaseUrl(URL sharingApiBaseUrl) { this.sharingApiBaseUrl.set(sharingApiBaseUrl); } public URL getSharingApiBaseUrlOrNull() { return this.sharingApiBaseUrl.get(); }Choose the approach that best fits your application's concurrency requirements.
Also applies to: 83-83, 92-92, 127-127
58-60: Ensure consistent placement of getters and settersThe
getInstallationName()method is placed between other fields and not grouped with related getters and setters. For better readability, consider grouping all fields together, followed by constructors, and then getters and setters.src/main/webapp/app/exercises/programming/manage/services/programming-exercise-sharing.service.ts (5)
19-19: Capitalize class commentFor consistency and readability, start the class comment with a capital letter.
Apply this diff:
-/** the programming exercise sharing service */ +/** The programming exercise sharing service */
30-31: Fix typo in comment: "Servire" should be "Server"There's a typo in the comment; "Servire" should be "Server".
Apply this diff to fix the typo:
/** - * loads the Shopping Basket via the Servire + * Loads the Shopping Basket via the server */
42-43: Fix typo in comment: "statment" should be "statement"There's a typo in the comment; "statment" should be "statement".
Apply this diff to fix the typo:
/** - * loads the problem statment via the server + * Loads the problem statement via the server */
45-46: UnnecessaryheadersvariableThe
headersvariable is declared but not used to set any headers. Consider removing it or setting necessary headers.Apply this diff if headers are not needed:
- const headers = new HttpHeaders(); - return this.http.post<string>(this.resourceUrlBasket + 'problemStatement', sharingInfo, { headers, responseType: 'text' as 'json' }); + return this.http.post<string>(this.resourceUrlBasket + 'problemStatement', sharingInfo, { responseType: 'text' as 'json' });If headers are needed, make sure to set the required headers.
54-58: Improve method comment for clarityThe method comment and parameter descriptions could be rephrased for better clarity.
Apply this diff to improve readability:
/** - * Sets a new programming exercise up - * @param programmingExercise which should be setup - * @param course in which the exercise should be imported + * Sets up a new programming exercise. + * @param programmingExercise The programming exercise to set up + * @param course The course in which the exercise should be imported * @param sharingInfo sharing related data needed for the import */src/main/java/de/tum/cit/aet/artemis/exercise/web/ExerciseSharingResource.java (9)
67-67: Typographical error in comment: "constuctor" should be "constructor".There's a typo in the comment on line 67. Correcting it enhances code readability.
Apply this diff to fix the typo:
- * constuctor for spring + * Constructor for Spring
68-71: Complete Javadoc comments for constructor parameters.The parameters in the constructor Javadoc are missing descriptions. Providing detailed comments improves documentation and helps other developers understand the code.
Apply this diff to complete the Javadoc:
/** - * Constructor for Spring - * - * @param exerciseSharingService - * @param programmingExerciseImportFromSharingService + * Constructor for Spring. + * + * @param exerciseSharingService the service for exercise sharing operations + * @param programmingExerciseImportFromSharingService the service for importing programming exercises from sharing */
115-115: Address the TODO comment regarding identical methods.The TODO comment indicates that
getExerciseDetailsmay return identical results togetProblemStatement. This should be investigated to avoid redundant code.Would you like assistance in refactoring these methods to eliminate duplication?
178-180: Add @OverRide annotation to the overriddenclosemethod.Adding the
@Overrideannotation helps with readability and ensures that the method properly overrides a superclass method.Apply this diff to add the annotation:
+@Override public void close() throws IOException {
181-182: Correct the log message to reflect the correct context.The log message mentions "imported file" instead of "exported file," which could cause confusion during debugging.
Apply this diff to correct the message:
if (!file.delete()) { - log.warn("Could not delete imported file from Sharing-Platform"); + log.warn("Could not delete exported exercise zip file after sharing"); }
157-158: Improve the response for unauthorized access by returning an appropriate status.Returning a
401 Unauthorizedstatus without a body is standard practice.Apply this diff to adjust the response:
if (sec.isEmpty() || !exerciseSharingService.validate(token, sec)) { - return ResponseEntity.status(401).body(null); + return ResponseEntity.status(HttpStatus.UNAUTHORIZED).build(); }
169-184: Consider refactoring the inner classNewFileInputStreamfor better code organization.Defining a class inside a method can reduce readability. Moving it to a separate file or making it a static nested class improves maintainability.
Would you like assistance in refactoring the
NewFileInputStreamto a dedicated class?
96-96: Handle exceptions within the method rather than throwing them.Throwing exceptions like
GitAPIException,SharingException,IOException, andURISyntaxExceptionfrom the controller may not be ideal. Handling them and returning appropriate HTTP responses enhances user experience.Apply this diff to handle exceptions:
public ResponseEntity<ProgrammingExercise> setUpFromSharingImport(@RequestBody SharingSetupInfo sharingSetupInfo) - throws GitAPIException, SharingException, IOException, URISyntaxException { + { try { ProgrammingExercise exercise = programmingExerciseImportFromSharingService.importProgrammingExerciseFromSharing(sharingSetupInfo); return ResponseEntity.ok().body(exercise); } catch (GitAPIException | SharingException | IOException | URISyntaxException e) { log.error("Error importing programming exercise from sharing", e); return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(null); } }
3-3: Avoid static imports for better code clarity.Using static imports can make it harder to identify where a constant or method comes from. Explicitly importing the class improves readability.
Apply this diff to change the import:
-import static de.tum.cit.aet.artemis.core.config.Constants.SHARINGEXPORT_RESOURCE_PATH; +import de.tum.cit.aet.artemis.core.config.Constants;And update the usage:
-@PostMapping(SHARINGEXPORT_RESOURCE_PATH + "/{exerciseId}") +@PostMapping(Constants.SHARINGEXPORT_RESOURCE_PATH + "/{exerciseId}")src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.java (1)
98-98: Clarify the Javadoc parameter descriptionThe description for
isImportFromSharingis unclear. Consider rephrasing it for better understanding.Suggestion:
* @param isImportFromSharing flag indicating whether the import is from sharing (true) or a regular file import (false)src/main/java/de/tum/cit/aet/artemis/sharing/ExerciseSharingService.java (5)
117-123: Inconsistent parameter naming in constructorThe parameter
sharingPluginServiceis assigned tothis.sharingConnectorService, which could lead to confusion. For clarity and consistency, consider renaming the parameter tosharingConnectorService.Apply this diff to rename the parameter:
public ExerciseSharingService(ProgrammingExerciseExportService programmingExerciseExportService, SharingConnectorService sharingPluginService, ProgrammingExerciseRepository programmingExerciseRepository, ProfileService profileService) { this.programmingExerciseExportService = programmingExerciseExportService; - this.sharingConnectorService = sharingPluginService; + this.sharingConnectorService = sharingConnectorService; this.programmingExerciseRepository = programmingExerciseRepository; this.profileService = profileService; }
86-87: Reduce field visibility to privateThe fields
artemisServerUrlandprofileServiceare declared asprotectedbut are not accessed outside this class. Following the principle of least privilege, consider changing their access modifiers toprivate.Apply this diff to adjust the access modifiers:
@Value("${server.url}") -protected String artemisServerUrl; +private String artemisServerUrl; /** * the profile service */ -protected ProfileService profileService; +private ProfileService profileService;Also applies to: 92-92
241-245: Typo in comment and caution on method usageThere's a typo in the comment: "It replaces an localhost wit host.docker.internal." Also, ensure that
correctLocalHostInDockeris not used in production environments unintentionally.Apply this diff to fix the typo:
/** - * this is just a weak implementation for local testing (within a docker). It replaces an localhost wit host.docker.internal. + * This is a weak implementation for local testing (within Docker). It replaces 'localhost' with 'host.docker.internal'. * * @param url the url to be corrected * @return an url that points to host.docker.internal if previously directed to localhost. */
275-275: Enhance regex pattern to match file extensionsThe pattern currently matches filenames starting with "Exercise-Details" but may miss files with extensions. Consider updating the pattern to include common file extensions.
Apply this diff to improve the regex pattern:
-Pattern pattern = Pattern.compile("^Exercise-Details", Pattern.CASE_INSENSITIVE); +Pattern pattern = Pattern.compile("^Exercise-Details(\\..+)?$", Pattern.CASE_INSENSITIVE);
266-274: Address the TODO comment and verify method usageThe method
getExerciseDetailsFromBaskethas a TODO indicating its usage needs checking. Ensure the method is necessary and properly utilized. If not in use, consider removing it to keep the codebase clean.Would you like assistance in determining the usage of this method or updating it accordingly?
src/main/webapp/app/exercises/programming/manage/update/programming-exercise-update.component.ts (1)
1274-1278: Remove commented-out code for cleaner codebaseThe commented-out code appears obsolete and can clutter the codebase. Removing it improves readability and maintainability.
Please remove the commented code to keep the codebase clean:
-/* -this.programmingExerciseSharingService.loadProblemStatementForExercises(this.sharingInfo).subscribe((statement: string) => { - this.programmingExercise.problemStatement = statement; -}); -*/
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (5)
.idea/runConfigurations/Artemis_Dev.xmlis excluded by!**/*.xmldocker/artemis-dev-local-vc-local-ci-mysql.ymlis excluded by!**/*.ymldocker/artemis.ymlis excluded by!**/*.ymlsrc/main/resources/config/application-dev.ymlis excluded by!**/*.ymlsrc/main/resources/config/application-sharing.ymlis excluded by!**/*.yml
📒 Files selected for processing (37)
build.gradle(2 hunks)docker/artemis/config/dev-local-vc-local-ci.env(1 hunks)src/main/java/de/tum/cit/aet/artemis/core/config/Constants.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/core/config/SecurityConfiguration.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/core/dto/SharingInfoDTO.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/core/service/ProfileService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/core/web/SharingSupportResource.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exercise/service/sharing/ProgrammingExerciseImportFromSharingService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exercise/service/sharing/SharingConnectorService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exercise/service/sharing/SharingException.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exercise/web/ExerciseSharingResource.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.java(7 hunks)src/main/java/de/tum/cit/aet/artemis/sharing/ExerciseSharingService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/sharing/SharingMultipartZipFile.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/sharing/SharingSetupInfo.java(1 hunks)src/main/webapp/app/app-routing.module.ts(1 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(4 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-management.module.ts(2 hunks)src/main/webapp/app/exercises/programming/manage/programming-exercise.component.ts(1 hunks)src/main/webapp/app/exercises/programming/manage/services/programming-exercise-sharing.service.ts(1 hunks)src/main/webapp/app/exercises/programming/manage/update/programming-exercise-creation-config.ts(1 hunks)src/main/webapp/app/exercises/programming/manage/update/programming-exercise-update.component.html(3 hunks)src/main/webapp/app/exercises/programming/manage/update/programming-exercise-update.component.ts(16 hunks)src/main/webapp/app/exercises/programming/shared/actions/programming-exercise-instructor-exercise-sharing.component.ts(1 hunks)src/main/webapp/app/sharing/search-result-dto.model.ts(1 hunks)src/main/webapp/app/sharing/sharing.component.html(1 hunks)src/main/webapp/app/sharing/sharing.component.ts(1 hunks)src/main/webapp/app/sharing/sharing.model.ts(1 hunks)src/main/webapp/app/sharing/sharing.module.ts(1 hunks)src/main/webapp/app/sharing/sharing.route.ts(1 hunks)src/main/webapp/app/sharing/sharing.scss(1 hunks)src/main/webapp/i18n/de/programmingExercise.json(2 hunks)src/main/webapp/i18n/de/sharing.json(1 hunks)src/main/webapp/i18n/en/programmingExercise.json(1 hunks)src/main/webapp/i18n/en/sharing.json(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/main/webapp/i18n/de/sharing.json
- src/main/webapp/i18n/en/sharing.json
🧰 Additional context used
📓 Path-based instructions (31)
src/main/java/de/tum/cit/aet/artemis/core/config/Constants.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/core/config/SecurityConfiguration.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/core/dto/SharingInfoDTO.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/core/service/ProfileService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/core/web/SharingSupportResource.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/exercise/service/sharing/ProgrammingExerciseImportFromSharingService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/exercise/service/sharing/SharingConnectorService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/exercise/service/sharing/SharingException.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/exercise/web/ExerciseSharingResource.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/sharing/ExerciseSharingService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/sharing/SharingMultipartZipFile.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/sharing/SharingSetupInfo.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/webapp/app/app-routing.module.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/manage/programming-exercise-detail.component.ts (1)
src/main/webapp/app/exercises/programming/manage/programming-exercise-management-routing.module.ts (1)
src/main/webapp/app/exercises/programming/manage/programming-exercise-management.module.ts (1)
src/main/webapp/app/exercises/programming/manage/programming-exercise.component.ts (1)
src/main/webapp/app/exercises/programming/manage/services/programming-exercise-sharing.service.ts (1)
src/main/webapp/app/exercises/programming/manage/update/programming-exercise-creation-config.ts (1)
src/main/webapp/app/exercises/programming/manage/update/programming-exercise-update.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/update/programming-exercise-update.component.ts (1)
src/main/webapp/app/exercises/programming/shared/actions/programming-exercise-instructor-exercise-sharing.component.ts (1)
src/main/webapp/app/sharing/search-result-dto.model.ts (1)
src/main/webapp/app/sharing/sharing.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/sharing/sharing.component.ts (1)
src/main/webapp/app/sharing/sharing.model.ts (1)
src/main/webapp/app/sharing/sharing.module.ts (1)
src/main/webapp/app/sharing/sharing.route.ts (1)
src/main/webapp/i18n/de/programmingExercise.json (1)
Pattern src/main/webapp/i18n/de/**/*.json: German language translations should be informal (dutzen) and should never be formal (sietzen). So the user should always be addressed with "du/dein" and never with "sie/ihr".
🪛 Biome (1.9.4)
src/main/webapp/app/sharing/sharing.component.ts
[error] 86-86: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (41)
src/main/java/de/tum/cit/aet/artemis/exercise/service/sharing/SharingException.java (1)
8-8: Verify profile configuration across the codebase
The @Profile("sharing") annotation suggests this is part of an optional feature set.
Let's verify the profile usage consistency:
✅ Verification successful
Profile configuration for sharing feature is consistently applied
The verification shows that the @Profile("sharing") annotation is consistently used across all sharing-related components:
- All classes in the
sharingpackages are properly annotated - The annotation is applied to both service layer (
ExerciseSharingService,SharingConnectorService) and web layer (ExerciseSharingResource,SharingSupportResource) components - Supporting classes like DTOs and exceptions also maintain the same profile
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent usage of the "sharing" profile
# and related configuration across the codebase
echo "Checking for sharing profile usage..."
rg --type java "@Profile\(.*sharing.*\)"
echo -e "\nChecking for sharing profile configuration..."
fd -e yml -e properties | xargs rg "spring\.profiles.*sharing"
echo -e "\nChecking for other sharing-related classes..."
rg --type java "package.*sharing"
Length of output: 2132
src/main/webapp/app/sharing/sharing.module.ts (2)
8-12: LGTM! Module implementation follows Angular best practices
The module implementation is well-structured with:
- Proper lazy loading implementation using
RouterModule.forChild() - Clean module structure following Angular guidelines
- Correct component declarations
1-12: Verify lazy loading implementation in main routing
Let's verify that this module is properly lazy loaded from the main routing module.
✅ Verification successful
Lazy loading is correctly implemented ✅
The module is properly lazy loaded in app-routing.module.ts using the recommended Angular syntax:
loadChildren: () => import('./sharing/sharing.module').then((m) => m.SharingModule)🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for lazy loading implementation of SharingModule in the main routing
ast-grep --pattern 'loadChildren: () => import($path).then((m) => m.SharingModule)'
# Verify there are no eager imports of SharingModule
rg -l "import.*SharingModule" | grep -v "sharing.module.ts"
Length of output: 311
Script:
#!/bin/bash
# Let's check the actual implementation in app-routing.module.ts
rg "SharingModule" src/main/webapp/app/app-routing.module.ts -A 3 -B 3
Length of output: 372
docker/artemis/config/dev-local-vc-local-ci.env (2)
5-5: Security configuration for sharing platform integration
The PR objectives mention that the sharing platform integration requires pre-shared keys for secure communication. Consider adding the necessary security-related environment variables to this configuration file for local development and testing.
5-5: Verify the sharing profile dependencies
The addition of the 'sharing' profile looks correct. However, let's verify that all required dependencies and components for the sharing functionality are properly configured.
✅ Verification successful
The sharing profile is properly configured with required components
Based on the verification results, the sharing profile is well-structured with appropriate components:
- Multiple sharing-related classes are properly annotated with
@Profile("sharing") - Core components like
SharingSupportResourceandExerciseSharingServiceare present - The sharing functionality is organized across multiple packages including core, exercise, and service layers
- Basic YAML configuration for sharing is present
The addition of the 'sharing' profile to SPRING_PROFILES_ACTIVE is valid and will correctly activate these components.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for sharing-related components and their profile annotations
# Expected: Find classes/components that require the 'sharing' profile
# Search for sharing profile annotations
echo "Checking for @Profile(\"sharing\") annotations:"
rg -l "@Profile.*sharing"
# Search for sharing-related configuration properties
echo -e "\nChecking for sharing-related configuration properties:"
rg -l "sharing\.(enabled|url|key|config)"
# Search for sharing-related beans and components
echo -e "\nChecking for sharing-related components:"
ast-grep --pattern 'class $_ {
$$$
@Profile("sharing")
$$$
}'
Length of output: 1307
Script:
#!/bin/bash
# Let's check the content of key sharing-related files to verify proper configuration
# Check SharingSupportResource for configuration properties
echo "=== SharingSupportResource configuration ==="
rg -A 5 "@Profile.*sharing" src/main/java/de/tum/cit/aet/artemis/core/web/SharingSupportResource.java
# Check ExerciseSharingService for dependencies
echo -e "\n=== ExerciseSharingService configuration ==="
rg -A 10 "@Profile.*sharing" src/main/java/de/tum/cit/aet/artemis/sharing/ExerciseSharingService.java
# Check application properties for sharing configuration
echo -e "\n=== Application properties for sharing ==="
fd "application.*properties" --type f --exec rg -l "sharing\."
# If properties files found, show their sharing-related content
fd "application.*properties" --type f --exec rg "sharing\." {}
Length of output: 963
Script:
#!/bin/bash
# Let's check for configuration classes and component dependencies
# Check for sharing configuration classes
echo "=== Sharing Configuration Classes ==="
rg -l "class.*SharingConfig"
# Check for autowired sharing components
echo -e "\n=== Autowired Sharing Dependencies ==="
rg -A 3 "@Autowired.*Sharing"
# Check for sharing-related beans
echo -e "\n=== Sharing Bean Definitions ==="
ast-grep --pattern '@Bean
$$$
$_Sharing$_'
# Check for sharing-related yml configurations
echo -e "\n=== Sharing YAML Configurations ==="
fd -e yml -e yaml --type f --exec rg "sharing:" {}
Length of output: 641
src/main/webapp/app/sharing/sharing.route.ts (2)
1-4: LGTM! Import statements are well-organized.
The imports follow Angular style guide recommendations, using appropriate path aliases and logical grouping.
1-23: Verify route configuration meets sharing platform requirements.
The route configuration should align with the sharing platform integration requirements mentioned in PR #9909.
✅ Verification successful
Route configuration aligns with sharing platform integration requirements
The route configuration is correctly implemented for the sharing platform integration:
- Proper authority checks are in place, restricting access to EDITOR, INSTRUCTOR, and ADMIN roles
- Component implementation validates these authorities during initialization
- Integration with ProgrammingExerciseSharingService is properly configured
- Route paths are correctly set up for both import and export functionality
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for other sharing-related components and services
echo "Checking for related sharing components..."
rg -t typescript "import.*Sharing" src/main/webapp/app/
# Verify consistent authority usage
echo "Checking authority usage across sharing features..."
rg -t typescript "Authority\.(EDITOR|INSTRUCTOR|ADMIN)" src/main/webapp/app/sharing/
Length of output: 422
Script:
#!/bin/bash
# Check for other sharing-related components and services without file type restriction
echo "Checking for related sharing components..."
rg "import.*Sharing" src/main/webapp/app/
# Verify authority usage
echo "Checking authority usage across sharing features..."
rg "Authority\.(EDITOR|INSTRUCTOR|ADMIN)" src/main/webapp/app/
# Check for sharing-related components and configurations
echo "Looking for sharing-related files..."
fd "sharing" src/main/webapp/app/
# Check for route configurations referencing sharing
echo "Checking route configurations..."
rg "path.*sharing" src/main/webapp/app/
Length of output: 47531
Script:
#!/bin/bash
# Check sharing component implementation
echo "Checking sharing component implementation..."
cat src/main/webapp/app/sharing/sharing.component.ts
# Check sharing model
echo "Checking sharing model..."
cat src/main/webapp/app/sharing/sharing.model.ts
# Check programming exercise sharing service
echo "Checking programming exercise sharing service..."
cat src/main/webapp/app/exercises/programming/manage/services/programming-exercise-sharing.service.ts
Length of output: 11908
src/main/java/de/tum/cit/aet/artemis/sharing/SharingSetupInfo.java (1)
1-8: LGTM! Imports are clean and well-organized.
The imports are specific and avoid star imports, following the coding guidelines.
src/main/java/de/tum/cit/aet/artemis/sharing/SharingMultipartZipFile.java (2)
30-44: LGTM! Clean and straightforward implementation
The metadata methods are implemented correctly and follow the interface contract.
18-18: Verify sharing profile integration
Let's verify that this class is properly integrated with other sharing components.
✅ Verification successful
Profile integration is properly configured
The @Profile("sharing") annotation on SharingMultipartZipFile is correctly integrated with the sharing functionality:
- The class is part of a complete sharing subsystem with proper configuration and dependencies:
- Dedicated sharing profile configuration exists in
application-sharing.yml - The class is used by
ExerciseSharingServicefor file handling operations - It's properly integrated with other sharing components like
ProgrammingExerciseImportFromSharingService
- Dedicated sharing profile configuration exists in
- The implementation correctly fulfills the
MultipartFileinterface contract for file handling within the sharing context
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check integration with other sharing components
# Find all files using the sharing profile
echo "Files using sharing profile:"
rg -l "@Profile.*sharing.*"
# Find all references to SharingMultipartZipFile
echo -e "\nReferences to SharingMultipartZipFile:"
rg "SharingMultipartZipFile"
# Find potential file handling in sharing components
echo -e "\nFile handling in sharing components:"
rg -l "MultipartFile.*sharing"
Length of output: 2966
Script:
#!/bin/bash
# Let's verify the implementation and integration more thoroughly
# Check the implementation of SharingMultipartZipFile
echo "SharingMultipartZipFile implementation:"
ast-grep --pattern 'class SharingMultipartZipFile implements MultipartFile {
$$$
}'
# Check for configuration properties related to sharing
echo -e "\nSharing configuration properties:"
rg "sharing" src/main/resources/config/application*.yml || true
# Check for sharing-related beans and configurations
echo -e "\nSharing configurations and beans:"
rg -l "EnableConfigurationProperties.*sharing|@Configuration.*sharing"
# Check for sharing-related tests
echo -e "\nSharing-related tests:"
fd -e java -e groovy -e kotlin . test/ -x rg -l "sharing|SharingMultipartZipFile" {} || true
Length of output: 1689
Script:
#!/bin/bash
# Let's check the implementation and test coverage
# Check the actual implementation of SharingMultipartZipFile
echo "SharingMultipartZipFile implementation:"
cat src/main/java/de/tum/cit/aet/artemis/sharing/SharingMultipartZipFile.java
# Find test files related to sharing
echo -e "\nSharing-related tests:"
fd -e java -e groovy -e kotlin . src/test -x rg -l "sharing|SharingMultipartZipFile" {} || true
# Check sharing profile configuration
echo -e "\nSharing profile configuration:"
cat src/main/resources/config/application-sharing.yml || true
# Check for sharing-related beans
echo -e "\nSharing-related Spring configurations:"
rg "@Configuration" -A 5 -B 1 $(rg -l "sharing.*@Configuration")
Length of output: 31281
src/main/webapp/app/exercises/programming/shared/actions/programming-exercise-instructor-exercise-sharing.component.ts (2)
36-39: LGTM!
The constructor follows Angular's dependency injection pattern correctly with appropriate access modifiers.
1-77: Verify component testing coverage.
As this component is crucial for the sharing platform integration, ensure comprehensive testing is in place:
- Unit tests for the export functionality
- Integration tests with the sharing service
- Error handling scenarios
- Feature toggle behavior
src/main/webapp/app/exercises/programming/manage/update/programming-exercise-update.component.html (3)
3-9: LGTM! Proper usage of new Angular syntax.
The conditional logic correctly uses the new @if syntax as per coding guidelines, and properly handles all cases (create, edit, and import scenarios) with clear separation.
64-64: Verify form footer behavior with sharing imports.
The binding logic maintains consistency with other components, but we should verify the form footer's behavior when handling sharing imports.
✅ Verification successful
Form footer correctly handles sharing imports
The form footer component properly handles the import case, including sharing imports:
- The component uses Angular signals for
isImportinput and adapts its behavior accordingly - The save button text changes to "Import" when
isImportis true - Test coverage confirms the correct behavior for import scenarios
- The notification component is correctly hidden during imports
- The implementation is consistent with how other components handle sharing imports
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if FormFooter component handles the sharing import case appropriately
# Look for specific handling of isImport property in the component
ast-grep --pattern 'class $_ {
$$$
@Input() isImport: $_
$$$
}'
# Check for any specific handling of sharing imports in form footer
rg -A 5 "isImport.*sharing" "src/main/webapp/app/shared/form-footer/**/*.ts"
Length of output: 1819
Script:
#!/bin/bash
# Let's try to find the form-footer component and check its implementation
fd "form-footer" --type f
# After finding the component, let's check its content
rg -l "isImport" $(fd "form-footer" --type f)
# Let's also check where this component is used with isImport
rg "@Input.*isImport|\\[isImport\\]"
Length of output: 3821
Script:
#!/bin/bash
# Let's check the form-footer component implementation and its tests
cat src/main/webapp/app/forms/form-footer/form-footer.component.ts
cat src/test/javascript/spec/component/forms/form-footer.component.spec.ts
cat src/main/webapp/app/forms/form-footer/form-footer.component.html
Length of output: 7655
19-19: Verify handling of sharing imports in ProgrammingExerciseInfo component.
The binding logic is correct, but we should ensure the component properly handles the sharing import case.
✅ Verification successful
Based on the search results, I can now generate the final response. The code shows that isImportFromSharing is properly handled in the ProgrammingExerciseInfo component through the isImport binding, which combines all import scenarios (from existing exercise, file, or sharing). The component's behavior is consistent across all import types, and the sharing import case is properly integrated into the existing import logic.
The isImport binding correctly handles the sharing import case
The binding logic is properly implemented and the ProgrammingExerciseInfo component handles all import scenarios consistently, including the sharing case. The code shows:
- The
isImportbinding combines all import types:isImportFromExistingExercise || isImportFromFile || isImportFromSharing - The sharing import case is properly integrated into the component's initialization and update logic
- The component's behavior is consistent across all import scenarios
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if ProgrammingExerciseInfo component handles the sharing import case
# Look for specific handling of isImport property in the component
ast-grep --pattern 'class ProgrammingExerciseInfo {
$$$
@Input() isImport: $_
$$$
}'
# Check for any specific handling of sharing imports
rg -A 5 "isImport.*sharing" "src/main/webapp/app/exercises/**/*.ts"
Length of output: 364
Script:
#!/bin/bash
# Let's try a broader search to locate the ProgrammingExerciseInfo component first
fd -e ts -e html "ProgrammingExerciseInfo"
# Then search for isImport usage in the found files
rg "isImport" -A 5
# Also search for any sharing-related imports or logic
rg "sharing|isImport" -A 5
Length of output: 103495
Script:
#!/bin/bash
# Let's search for the ProgrammingExerciseInfo component and its usage
fd -e ts -e html "ProgrammingExerciseInfo"
# Also search for the specific file mentioned in the review comment
fd "programming-exercise-update.component.html"
# Let's also check for any sharing-related logic in programming exercise components
rg -A 5 "isImportFromSharing" "src/main/webapp/app/exercises/programming/"
Length of output: 16284
src/main/webapp/app/exercises/programming/manage/programming-exercise-management.module.ts (1)
66-66: Verify component placement in module structure.
The component declaration is correctly placed. However, let's verify if other sharing-related components exist or are planned that might warrant a dedicated sharing feature module.
✅ Verification successful
No need for a dedicated sharing module at this time
The codebase search reveals only one sharing-related component (ProgrammingExerciseInstructorExerciseSharingComponent), which is already properly declared in the programming exercise management module. There's no evidence of other sharing components that would justify creating a separate feature module for sharing functionality.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other sharing-related components
rg -t ts "export class.*Sharing.*Component" src/main/webapp/app/exercises/
Length of output: 264
src/main/webapp/app/sharing/sharing.component.html (1)
1-103: Verify security measures for sharing platform integration
Since this component handles integration with an external sharing platform, we should verify:
- Token validation and secure storage
- API endpoint security
- Input sanitization for imported data
✅ Verification successful
Security measures for sharing platform integration are adequately implemented
The implementation includes several security measures:
- JWT-based authentication is properly configured in both dev and prod environments
- CORS is properly configured with explicit allowed origins, methods, and headers
- The
ExerciseSharingServicevalidates and sanitizes URLs throughcorrectLocalHostInDocker - Token-based access control is implemented with proper expiration handling
- The sharing platform integration uses secure communication with proper request/response validation
The component's security implementation aligns with best practices for external system integration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for secure communication patterns
rg -A 5 "apiBaseURL|basketToken" src/
# Check for HTTPS enforcement in configuration
rg -A 5 "sharing.*url" src/main/resources/config/
Length of output: 13482
Script:
#!/bin/bash
# Check for HTTPS validation and security patterns
rg -A 5 "correctLocalHostInDocker|validateUrl|https" src/main/java/de/tum/cit/aet/artemis/sharing/
# Check for input validation and sanitization
ast-grep --pattern 'class ExerciseSharingService {
$$$
getBasketInfo($_, $_) {
$$$
}
$$$
}'
# Check for security configurations
rg -A 5 "security|csrf|cors" src/main/resources/config/
Length of output: 7204
src/main/webapp/app/app-routing.module.ts (2)
186-189: Verify the sharing module structure.
Let's ensure the sharing module is properly configured and follows the application's conventions.
✅ Verification successful
The sharing module is properly configured and follows Angular conventions
The verification shows that:
- The sharing module exists and is properly structured with correct imports and declarations
- Routes are correctly configured in
sharing.route.tswith proper authority checks - The sharing component is properly decorated and implements the required functionality for exercise sharing
- The module is correctly lazy-loaded in the app routing configuration
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the sharing module structure and configuration
# Check if the sharing module exists and follows naming conventions
echo "Checking sharing module structure..."
fd -e ts "sharing.module.ts" src/main/webapp/app/sharing
# Verify the module follows Angular conventions
ast-grep --pattern 'class $module implements NgModule {
$$$
}' src/main/webapp/app/sharing/sharing.module.ts
# Check for proper route configuration in the sharing module
ast-grep --pattern 'const routes: Routes = [
$$$
];' src/main/webapp/app/sharing/sharing-routing.module.ts
Length of output: 510
Script:
#!/bin/bash
# Let's examine the sharing module content to verify its configuration
# Check the content of sharing.module.ts
echo "=== Sharing Module Content ==="
cat src/main/webapp/app/sharing/sharing.module.ts
# Check if routes are defined within the sharing module itself
echo -e "\n=== Checking for route definitions in sharing.module.ts ==="
ast-grep --pattern 'const routes: Routes = [
$$$
]' src/main/webapp/app/sharing/sharing.module.ts
# Look for any component declarations in the module
echo -e "\n=== Checking component declarations ==="
ast-grep --pattern '@NgModule({
$$$
})' src/main/webapp/app/sharing/sharing.module.ts
Length of output: 1361
Script:
#!/bin/bash
# Let's check the route configuration and component implementation
# Check the sharing.route.ts content
echo "=== Sharing Route Configuration ==="
cat src/main/webapp/app/sharing/sharing.route.ts
# Verify the sharing component exists and its basic structure
echo -e "\n=== Sharing Component Content ==="
cat src/main/webapp/app/sharing/sharing.component.ts
# Check if the component has the correct decorator
echo -e "\n=== Component Decorator Check ==="
ast-grep --pattern '@Component({
$$$
})' src/main/webapp/app/sharing/sharing.component.ts
Length of output: 5812
186-189: Verify integration with the sharing platform configuration.
The route implementation should align with the sharing platform's authentication and configuration requirements.
✅ Verification successful
Route implementation aligns with sharing platform integration requirements
The implementation shows proper integration with the sharing platform:
- Handles basket token from route parameters
- Integrates with programming exercise sharing service
- Implements proper authentication checks
- Manages sharing state and shopping basket functionality
- Supports API base URL configuration via query parameters
The route implementation in app-routing.module.ts correctly loads the sharing module which contains all the necessary components for secure communication and configuration handling.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the sharing platform integration points
# Check for sharing configuration service
echo "Checking for sharing configuration service..."
fd -e ts "sharing.*service.ts" src/main/webapp/app/sharing
# Verify authentication handling
ast-grep --pattern 'class $service {
$$$
basketToken($$$) {
$$$
}
$$$
}' src/main/webapp/app/sharing
# Check for environment configuration
echo "Checking for sharing platform configuration..."
fd -g "*environment*.ts" src/main/webapp/environments
Length of output: 551
Script:
#!/bin/bash
# Let's verify the sharing module implementation and configuration
# Check for sharing module and its components
echo "Checking sharing module structure..."
fd -e ts "sharing" src/main/webapp/app
# Look for sharing-related configuration in the codebase
echo "Checking sharing configuration..."
rg -l "sharing" src/main/webapp/app/config
# Check for basket token handling in sharing components
echo "Checking basket token implementation..."
rg -l "basketToken" src/main/webapp/app/sharing
# Look for environment configuration in correct path
echo "Checking environment configuration..."
fd -e ts "environment" src/main/webapp
# Examine the sharing module implementation
echo "Checking sharing module contents..."
ast-grep --pattern 'export class SharingModule {
$$$
}'
Length of output: 1429
Script:
#!/bin/bash
# Let's examine the specific implementation details
# Check sharing module implementation
echo "Examining sharing module..."
cat src/main/webapp/app/sharing/sharing.module.ts
# Check sharing component implementation for basket token handling
echo "Examining sharing component..."
cat src/main/webapp/app/sharing/sharing.component.ts
# Check sharing model
echo "Examining sharing model..."
cat src/main/webapp/app/sharing/sharing.model.ts
# Check environment configuration
echo "Examining environment configuration..."
cat src/main/webapp/app/environments/environment.ts
Length of output: 6264
src/main/webapp/app/exercises/programming/manage/programming-exercise.component.ts (2)
39-39: Verify the sharing model import path
The import statement follows Angular conventions correctly.
✅ Verification successful
Import path for SharingInfo is correct and class exists
The import statement import { SharingInfo } from 'app/sharing/sharing.model'; is verified to be correct as:
- The file exists at
src/main/webapp/app/sharing/sharing.model.ts - The file exports the
SharingInfoclass with the expected properties
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence and exports of the sharing model
# Test: Check if the sharing model exists and exports SharingInfo
ast-grep --pattern 'export class SharingInfo {
$$$
}'
Length of output: 181
Script:
#!/bin/bash
# Let's try to find the sharing model file first
fd "sharing.model.ts"
# Then let's use ripgrep to search for SharingInfo class definition
# as ast-grep had encoding issues
rg "export (class|interface|type) SharingInfo" -A 5
Length of output: 541
44-44: Consider the scope and usage of SharingInfo provider
While adding SharingInfo as a component provider is syntactically correct, there are some considerations:
- Component-level provider creates a new instance per component
- No visible usage of SharingInfo in the component yet
- Missing integration with sharing platform functionality
Consider:
- Moving the provider to a higher level (module) if sharing info needs to be shared across components
- Adding the necessary dependency injection and usage code
- Documenting the sharing functionality integration plan
src/main/webapp/app/exercises/programming/manage/programming-exercise-management-routing.module.ts (2)
51-51: Verify component compatibility with sharing functionality
Both routes use ProgrammingExerciseUpdateComponent. Ensure this component has been updated to handle the sharing platform integration requirements mentioned in the PR objectives.
Also applies to: 100-100
✅ Verification successful
Based on the results, I can generate the final response as we have sufficient evidence showing that the component has proper sharing functionality integration.
Component is properly integrated with sharing functionality
The ProgrammingExerciseUpdateComponent has comprehensive sharing platform integration:
- Imports required sharing services and models (
ProgrammingExerciseSharingService,SharingInfo) - Handles sharing-specific routes with
isImportFromSharingflag - Implements sharing-related operations:
- Sets up exercises from sharing imports
- Loads problem statements
- Manages sharing details (basket token, API URLs, etc.)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if ProgrammingExerciseUpdateComponent has sharing-related code
rg -A 5 "sharing|import.*sharing" "src/main/webapp/app/exercises/programming/manage/update/programming-exercise-update.component.ts"
Length of output: 3793
49-61: 🛠️ Refactor suggestion
Review authority configuration and route naming
A few concerns about this route configuration:
- The authority set (
TA, INSTRUCTOR, ADMIN) differs from other import routes which use (EDITOR, INSTRUCTOR, ADMIN). This inconsistency might cause confusion. - The route path
importSharingdoesn't follow the kebab-case naming convention used by other routes (e.g.,import-from-file).
Consider applying these changes:
- path: ':courseId/programming-exercises/importSharing',
+ path: ':courseId/programming-exercises/import-sharing',
component: ProgrammingExerciseUpdateComponent,
resolve: {
programmingExercise: ProgrammingExerciseResolve,
},
data: {
- authorities: [Authority.TA, Authority.INSTRUCTOR, Authority.ADMIN],
+ authorities: [Authority.EDITOR, Authority.INSTRUCTOR, Authority.ADMIN],
usePathForBreadcrumbs: true,
pageTitle: 'artemisApp.programmingExercise.home.title',
},✅ Verification successful
Let me search for other import routes to verify the authority configuration.
Let me try a different search pattern to find all routes with their authorities in the routing module.
Review authority configuration and route naming
The review comment is correct. Looking at the routing module, there are inconsistencies:
-
The
importSharingroute uses[Authority.TA, Authority.INSTRUCTOR, Authority.ADMIN]while all other import-related routes use[Authority.EDITOR, Authority.INSTRUCTOR, Authority.ADMIN]:import/:exerciseIdimport-from-fileimport-from-sharing
-
The route path
importSharingdoesn't follow the kebab-case naming convention used by other routes likeimport-from-fileandimport-from-sharing.
The suggested changes to maintain consistency are appropriate:
- path: ':courseId/programming-exercises/importSharing',
+ path: ':courseId/programming-exercises/import-sharing',
component: ProgrammingExerciseUpdateComponent,
resolve: {
programmingExercise: ProgrammingExerciseResolve,
},
data: {
- authorities: [Authority.TA, Authority.INSTRUCTOR, Authority.ADMIN],
+ authorities: [Authority.EDITOR, Authority.INSTRUCTOR, Authority.ADMIN],
usePathForBreadcrumbs: true,
pageTitle: 'artemisApp.programmingExercise.home.title',
},🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify authority consistency across import routes
rg "import.*authorities.*\[.*\]" -A 2
Length of output: 131112
Script:
#!/bin/bash
# Search for import routes and their authority configurations in the routing module
rg "import.*path:.*authorities.*\[.*\]" -A 3 src/main/webapp/app/exercises/programming/manage/programming-exercise-management-routing.module.ts
Length of output: 145
Script:
#!/bin/bash
# Search for all route configurations in the programming exercise management routing module
cat src/main/webapp/app/exercises/programming/manage/programming-exercise-management-routing.module.ts
Length of output: 13019
src/main/java/de/tum/cit/aet/artemis/core/config/SecurityConfiguration.java (1)
211-212: Verify security measures for the sharing endpoint
While the comment indicates that sharing is protected by explicit security tokens, permitting all requests to /api/sharing/** without additional security layers (like IP restrictions) could potentially expose the endpoint to abuse.
Let's verify the security implementation:
Consider:
- Documenting the security design and token validation mechanism
- Adding rate limiting for these endpoints
- Implementing IP allowlisting similar to the Prometheus endpoint if applicable
- Adding request logging for security audit purposes
src/main/webapp/app/exercises/programming/manage/programming-exercise-detail.component.html (2)
110-110: LGTM: Well-placed sharing component with appropriate access control
The new sharing component is correctly positioned next to the download component and properly restricted to instructors only via the isAtLeastInstructor check, which aligns with the PR objectives for secure sharing platform integration.
Line range hint 3-3: LGTM: Proper usage of new Angular syntax
The template correctly uses the new @if directive instead of the deprecated *ngIf, following the latest Angular syntax guidelines.
Also applies to: 13-13, 39-39, 69-69, 101-101, 133-133, 141-141, 169-169, 182-182, 196-196
src/main/java/de/tum/cit/aet/artemis/core/config/Constants.java (1)
379-398: Verify the impact of these endpoints on security configuration
Since these are new REST endpoints, ensure they are properly secured in the security configuration.
Let's verify the security configuration for these endpoints:
✅ Verification successful
Let's search for security configurations specifically:
The sharing endpoints are properly secured with explicit security tokens
The security configuration in SecurityConfiguration.java explicitly handles the sharing endpoints with the following rule:
.requestMatchers("/api/sharing/**").permitAll()This is accompanied by a comment that explains these endpoints are "protected by explicit security tokens, thus we can permitAll here". This means the endpoints have their own security mechanism using tokens rather than relying on Spring Security's authentication.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the new sharing endpoints are properly secured in the security configuration
# Search for security configuration related to sharing endpoints
rg -A 5 "sharing.*config" --type java
Length of output: 5471
Script:
#!/bin/bash
# Search for security configuration files and their content
rg -l "SecurityConfiguration|WebSecurityConfigurerAdapter|SecurityFilterChain" --type java
# Then search for the actual security rules
rg -A 10 "antMatchers|requestMatchers|authorizeRequests|authorizeHttpRequests" --type java
Length of output: 8364
src/main/webapp/app/exercises/programming/manage/programming-exercise-detail.component.ts (3)
19-19: LGTM!
The import statement follows Angular conventions and is properly placed with other service imports.
120-120: LGTM!
The property declaration follows TypeScript best practices with proper typing and initialization.
158-158: LGTM!
The dependency injection follows Angular best practices.
src/main/webapp/i18n/en/programmingExercise.json (1)
757-759: LGTM! The translations for sharing functionality are clear and concise.
The new translations align well with the PR objectives for integrating with the sharing platform. The keys are properly structured and the text is grammatically correct.
src/main/webapp/i18n/de/programmingExercise.json (1)
649-649: LGTM! Translation change improves clarity.
The change from "Abgaben" to "Abgabelimit" makes the meaning clearer and more precise, while maintaining informal language as required.
src/main/java/de/tum/cit/aet/artemis/exercise/service/sharing/ProgrammingExerciseImportFromSharingService.java (1)
54-59: Good use of constructor injection for dependency management.
The dependencies are correctly injected via the constructor, following best practices.
src/main/java/de/tum/cit/aet/artemis/exercise/service/sharing/SharingConnectorService.java (2)
117-117: Ensure thread-safety when accessing shared resources
The method isSharingApiBaseUrlPresent() checks sharingApiBaseUrl without synchronization. If sharingApiBaseUrl can be modified by another thread, this could lead to inconsistent behavior.
Suggested Action:
Verify if sharingApiBaseUrl is accessed or modified by multiple threads simultaneously. If so, consider making it volatile or synchronizing access to it.
- private URL sharingApiBaseUrl = null;
+ private volatile URL sharingApiBaseUrl = null;Alternatively, synchronize the methods that modify or access it.
1-174: Overall assessment
The SharingConnectorService class is well-structured and implements the required functionality for integrating with the Sharing Platform. It follows the naming conventions and dependency injection practices.
src/main/java/de/tum/cit/aet/artemis/exercise/web/ExerciseSharingResource.java (2)
85-86: Validate inputs and handle errors in loadShoppingBasket method.
Ensure that basketToken and apiBaseUrl are validated, and handle any potential exceptions from the service layer to prevent unexpected errors.
Would you like me to verify the input validation and error handling in the getBasketInfo method?
154-156:
Ensure secure access to the export endpoint by implementing authentication.
The exportExerciseToSharing GET endpoint is accessible without authentication, which could pose a security risk. It's important to protect this endpoint from unauthorized access.
Please verify if this endpoint should be publicly accessible. If not, consider adding appropriate authentication mechanisms.
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.java (4)
6-6: Import statement is appropriate
The addition of java.io.FileWriter is necessary for file writing operations.
18-19: Import statements added for JSON processing
The imports for javax.json.JsonObject and javax.json.JsonString are introduced. Ensure consistency in JSON library usage.
183-190: Overloaded method for backward compatibility is appropriate
The overloaded method without the isImportFromSharing parameter maintains backward compatibility and defaults the flag to false.
101-102: Ensure all method calls are updated to include the new parameter
The method importProgrammingExerciseFromFile now includes a new parameter isImportFromSharing. Verify that all calls to this method are updated accordingly to prevent any runtime errors.
Run the following script to find all usages of the method:
✅ Verification successful
All method calls are properly updated with the new parameter
The verification shows that all calls to importProgrammingExerciseFromFile are correctly handled:
- The method has an overloaded version without the
isImportFromSharingparameter that defaults tofalse - Direct calls are properly implemented:
ProgrammingExerciseExportImportResourceuses the overloaded method (defaults tofalse)ProgrammingExerciseImportFromSharingServiceexplicitly passestrue
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all usages of importProgrammingExerciseFromFile to ensure they are updated with the new parameter.
# Search for method calls in the codebase
rg --no-heading 'importProgrammingExerciseFromFile\(' -A 2
Length of output: 3269
| public class SharingException extends Exception { | ||
|
|
||
| public SharingException(String message) { | ||
| super(message); | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add essential exception handling capabilities
The current implementation could benefit from additional exception handling features commonly expected in production-grade code.
Consider applying these enhancements:
@Profile("sharing")
public class SharingException extends Exception {
+ private static final long serialVersionUID = 1L;
public SharingException(String message) {
super(message);
}
+
+ public SharingException(String message, Throwable cause) {
+ super(message, cause);
+ }
+
+ /**
+ * Creates a SharingException for connection failures.
+ *
+ * @param endpoint The endpoint that failed
+ * @param cause The underlying cause
+ * @return A new SharingException
+ */
+ public static SharingException connectionError(String endpoint, Throwable cause) {
+ return new SharingException("Failed to connect to sharing platform at " + endpoint, cause);
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public class SharingException extends Exception { | |
| public SharingException(String message) { | |
| super(message); | |
| } | |
| } | |
| public class SharingException extends Exception { | |
| private static final long serialVersionUID = 1L; | |
| public SharingException(String message) { | |
| super(message); | |
| } | |
| public SharingException(String message, Throwable cause) { | |
| super(message, cause); | |
| } | |
| /** | |
| * Creates a SharingException for connection failures. | |
| * | |
| * @param endpoint The endpoint that failed | |
| * @param cause The underlying cause | |
| * @return A new SharingException | |
| */ | |
| public static SharingException connectionError(String endpoint, Throwable cause) { | |
| return new SharingException("Failed to connect to sharing platform at " + endpoint, cause); | |
| } | |
| } |
| public basketToken = ''; | ||
| public returnURL: string; | ||
| public apiBaseURL: string; | ||
| public selectedExercise = 0; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add type safety and documentation to properties
The properties need type annotations and JSDoc comments explaining their purpose, especially since they're public. Consider adding URL validation for the URL properties.
+/** Token representing the current shopping basket */
public basketToken = '';
+/** URL to return to after completing sharing operation */
-public returnURL: string;
+public returnURL = '';
+/** Base URL for the sharing platform API */
-public apiBaseURL: string;
+public apiBaseURL = '';
+/** ID of the currently selected exercise */
public selectedExercise = 0;Also consider adding URL validation:
private validateURL(url: string): void {
try {
new URL(url);
} catch {
throw new Error('Invalid URL provided');
}
}| export type ShoppingBasket = { | ||
| exerciseInfo: Array<SearchResultDTO>; | ||
| userInfo: UserInfo; | ||
| tokenValidUntil: number; | ||
| }; | ||
|
|
||
| export type UserInfo = { | ||
| email: string; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Convert types to documented interfaces with validation
Consider converting these types to interfaces with proper documentation and validation.
-export type ShoppingBasket = {
+/**
+ * Represents a shopping basket containing exercises to be shared
+ */
+export interface ShoppingBasket {
exerciseInfo: Array<SearchResultDTO>;
userInfo: UserInfo;
- tokenValidUntil: number;
+ tokenValidUntil: Date;
};
-export type UserInfo = {
+/**
+ * Represents user information for sharing operations
+ */
+export interface UserInfo {
+ /** User's email address for sharing notifications */
email: string;
};
+/**
+ * Validates an email address
+ * @param email The email address to validate
+ * @throws Error if the email is invalid
+ */
+export function validateEmail(email: string): void {
+ const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/;
+ if (!emailRegex.test(email)) {
+ throw new Error('Invalid email address');
+ }
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export type ShoppingBasket = { | |
| exerciseInfo: Array<SearchResultDTO>; | |
| userInfo: UserInfo; | |
| tokenValidUntil: number; | |
| }; | |
| export type UserInfo = { | |
| email: string; | |
| }; | |
| /** | |
| * Represents a shopping basket containing exercises to be shared | |
| */ | |
| export interface ShoppingBasket { | |
| exerciseInfo: Array<SearchResultDTO>; | |
| userInfo: UserInfo; | |
| tokenValidUntil: Date; | |
| }; | |
| /** | |
| * Represents user information for sharing operations | |
| */ | |
| export interface UserInfo { | |
| /** User's email address for sharing notifications */ | |
| email: string; | |
| }; | |
| /** | |
| * Validates an email address | |
| * @param email The email address to validate | |
| * @throws Error if the email is invalid | |
| */ | |
| export function validateEmail(email: string): void { | |
| const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; | |
| if (!emailRegex.test(email)) { | |
| throw new Error('Invalid email address'); | |
| } | |
| } |
src/main/java/de/tum/cit/aet/artemis/sharing/SharingSetupInfo.java
Outdated
Show resolved
Hide resolved
| String decodedToken = new String(Base64.getDecoder().decode(b64Token)); | ||
| Path parent = Paths.get(repoDownloadClonePath, decodedToken + ".zip"); | ||
| File exportedExercise = parent.toFile(); | ||
| if (exportedExercise.exists()) { | ||
| return exportedExercise; | ||
| } | ||
| else { | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
Potential path traversal vulnerability
Decoding the base64 token and using it directly in file paths without validation can lead to path traversal attacks. Validate and sanitize the decoded token before using it.
[security_issue]
Apply input validation to prevent path traversal:
public File getExportedExerciseByToken(String b64Token) {
String decodedToken = new String(Base64.getDecoder().decode(b64Token), StandardCharsets.UTF_8);
+ if (!isValidToken(decodedToken)) {
+ log.warn("Invalid token received: {}", decodedToken);
+ return null;
+ }
Path parent = Paths.get(repoDownloadClonePath, decodedToken + ".zip");
File exportedExercise = parent.toFile();
if (exportedExercise.exists()) {
return exportedExercise;
} else {
return null;
}
}
+
+private boolean isValidToken(String token) {
+ // Implement validation logic, e.g., check for illegal characters or patterns
+ return token.matches("^[a-zA-Z0-9_-]+$");
+}Committable suggestion skipped: line range outside the PR's diff.
src/main/java/de/tum/cit/aet/artemis/sharing/ExerciseSharingService.java
Outdated
Show resolved
Hide resolved
...main/webapp/app/exercises/programming/manage/update/programming-exercise-update.component.ts
Outdated
Show resolved
Hide resolved
...main/webapp/app/exercises/programming/manage/update/programming-exercise-update.component.ts
Show resolved
Hide resolved
...main/webapp/app/exercises/programming/manage/update/programming-exercise-update.component.ts
Outdated
Show resolved
Hide resolved
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
src/main/java/de/tum/cit/aet/artemis/exercise/service/sharing/SharingConnectorService.java
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/exercise/web/ExerciseSharingResource.java
Fixed
Show fixed
Hide fixed
src/main/java/de/tum/cit/aet/artemis/exercise/web/ExerciseSharingResource.java
Fixed
Show resolved
Hide resolved
| try { | ||
| URI uriRedirect = exerciseSharingService.exportExerciseToSharing(exerciseId).toURI(); | ||
| uriRedirect = UriBuilder.fromUri(uriRedirect).queryParam("callBack", callBackUrl).build(); | ||
| return ResponseEntity.ok().body("\"" + uriRedirect.toString() + "\""); |
Check warning
Code scanning / CodeQL
Cross-site scripting Medium
There was a problem hiding this comment.
The url is user provided and returned to the same user. Not a real risk
| return ResponseEntity.ok().body("\"" + uriRedirect.toString() + "\""); | ||
| } | ||
| catch (SharingException | URISyntaxException e) { | ||
| return ResponseEntity.internalServerError().body(e.getMessage()); |
Check warning
Code scanning / CodeQL
Information exposure through an error message Medium
src/main/java/de/tum/cit/aet/artemis/exercise/web/ExerciseSharingResource.java
Dismissed
Show dismissed
Hide dismissed
src/main/java/de/tum/cit/aet/artemis/sharing/ExerciseSharingService.java
Dismissed
Show dismissed
Hide dismissed
src/main/java/de/tum/cit/aet/artemis/sharing/ExerciseSharingService.java
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/sharing/ExerciseSharingService.java
Fixed
Show fixed
Hide fixed
...ses/programming/shared/actions/programming-exercise-instructor-exercise-sharing.component.ts
Show resolved
Hide resolved
|
There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions. |
Wallenstein61
force-pushed
the
feature/connecting-to-sharing-platform
branch
from
003804a to
fddbf2d
Compare
Wallenstein61
force-pushed
the
feature/connecting-to-sharing-platform
branch
from
fddbf2d to
e1c4d3f
Compare
Wallenstein61
force-pushed
the
feature/connecting-to-sharing-platform
branch
from
e1c4d3f to
1a6699a
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (11)
src/main/webapp/app/exercises/programming/shared/actions/programming-exercise-instructor-exercise-sharing.component.ts (1)
55-76: 🛠️ Refactor suggestionImprove type safety and error handling in exportExerciseToSharing.
The method needs several improvements:
- Replace non-null assertions with proper null checks
- Use template literals instead of string concatenation
- Use localized error messages
exportExerciseToSharing(programmingExerciseId: number) { this.sharingService.exportProgrammingExerciseToSharing(programmingExerciseId, window.location.href).subscribe({ next: (redirect: HttpResponse<string>) => { - if (redirect) { - const redirectURL = redirect.body?.toString(); + const redirectURL = redirect?.body; + if (redirectURL) { if (this.sharingTab) { if (!window.name) { window.name = 'artemis'; } - this.sharingTab.location.href = redirectURL! + '&window=' + window.name; + this.sharingTab.location.href = `${redirectURL}&window=${window.name}`; this.sharingTab.focus(); } else { - window.location.href = redirectURL!; + window.location.href = redirectURL; } + } else { + this.alertService.error('artemisApp.programmingExercise.sharing.error.noRedirect'); } }, error: (errorResponse) => { - this.alertService.error('Unable to export exercise. Error: ' + errorResponse.message); + this.alertService.error('artemisApp.programmingExercise.sharing.error.export', { message: errorResponse.message }); }, }); }src/main/java/de/tum/cit/aet/artemis/exercise/service/sharing/SharingPluginService.java (4)
114-115:⚠️ Potential issueIncorrect usage of the anyMatch() result.
The expression “metadata.format.stream().anyMatch(…).get()” is invalid because anyMatch(...) returns a boolean, not an Optional. This leads to a runtime error.Apply this diff to remove the “.get()” call:
- "metadata.format.stream().anyMatch(entry->entry=='artemis' || entry=='Artemis').get()" + "metadata.format.stream().anyMatch(entry->entry=='artemis' || entry=='Artemis')"
125-125:⚠️ Potential issuePrevent NullPointerException during API key validation.
Comparing strings directly with sharingApiKey.equals(apiKey) can throw an NPE if sharingApiKey is null. Use Objects.equals(...) to mitigate this risk.- return sharingApiKey.equals(apiKey); + return Objects.equals(sharingApiKey, apiKey);
139-139: 🛠️ Refactor suggestionUse Spring’s @async instead of creating your own Executor.
Creating a dedicated thread pool for a single task can lead to resource overhead. Leverage Spring’s @async annotation to handle asynchronous behavior.
145-148:⚠️ Potential issueAvoid sending sensitive data via query parameters.
Passing the API key as a query parameter can expose it in logs and network traces. Instead, send it as a request header (e.g., “Authorization: Bearer ”).src/main/java/de/tum/cit/aet/artemis/exercise/service/sharing/SharingConnectorService.java (4)
130-130:⚠️ Potential issueIncorrect usage of the anyMatch() result.
The string “metadata.format.stream().anyMatch(...).get()” is invalid because anyMatch(...) returns a boolean. Calling .get() will cause an error at runtime.
147-147:⚠️ Potential issueNull-safe comparison of API keys.
Calling sharingApiKey.equals(apiKey) may throw an NPE if sharingApiKey is null. Use Objects.equals(...) to handle null values safely.- return sharingApiKey.equals(apiKey); + return Objects.equals(sharingApiKey, apiKey);
158-158: 🛠️ Refactor suggestionReplace manual thread creation with Spring’s @async.
Using Executors.newFixedThreadPool(1) is unnecessary. Annotate the method with @async to simplify concurrency management and avoid resource leaks.
167-168:⚠️ Potential issueMove sensitive API key to HTTP header.
Attaching the API key as a query parameter leaks it in logs and can compromise security. Prefer sending it in the “Authorization” header.src/main/java/de/tum/cit/aet/artemis/sharing/ExerciseSharingService.java (2)
414-415:⚠️ Potential issueUse constant-time comparison to avoid timing attacks.
Calling equals(...) for HMAC validation could reveal timing differences. Use MessageDigest.isEqual(...) for a safer approach.- return computedHMAC.equals(sec); + return MessageDigest.isEqual( + computedHMAC.getBytes(StandardCharsets.UTF_8), + sec.getBytes(StandardCharsets.UTF_8) + );
429-434:⚠️ Potential issueValidate and sanitize decoded tokens to prevent path traversal.
Directly using decoded tokens in file paths can enable malicious users to traverse directories. Perform adequate checks or filtering.
🧹 Nitpick comments (4)
src/main/java/de/tum/cit/aet/artemis/exercise/service/sharing/SharingException.java (1)
11-42: Consider using a RuntimeException if forced handling is not required.
Wrapping exceptions in a checked exception (extends Exception) forces explicit try-catch blocks; if handling at the call-site is optional, a RuntimeException might be more convenient.src/main/java/de/tum/cit/aet/artemis/exercise/service/sharing/ProgrammingExerciseImportFromSharingService.java (1)
67-75: Enhance error logging for better debugging.Add detailed logging to track the import process and potential failures.
Apply this diff to improve logging:
public ProgrammingExercise importProgrammingExerciseFromSharing(SharingSetupInfo sharingSetupInfo) throws SharingException, IOException, GitAPIException, URISyntaxException { + log.debug("Starting import of programming exercise from sharing platform"); SharingMultipartZipFile zipFile = exerciseSharingService.getCachedBasketItem(sharingSetupInfo.getSharingInfo()); + log.debug("Retrieved cached basket item for sharing info"); User user = userRepository.getUserWithGroupsAndAuthorities(); + log.debug("Retrieved user with groups and authorities: {}", user.getLogin());src/main/java/de/tum/cit/aet/artemis/core/web/SharingSupportResource.java (1)
91-97: Use HttpStatus constants instead of magic numbers.Replace magic numbers with proper HTTP status constants for better maintainability.
Apply this diff:
@GetMapping(SHARINGCONFIG_RESOURCE_IS_ENABLED) public ResponseEntity<Boolean> isSharingEnabled() { if (sharingConnectorService.isSharingApiBaseUrlPresent()) { - return ResponseEntity.status(200).body(true); + return ResponseEntity.status(HttpStatus.OK).body(true); } - return ResponseEntity.status(503).body(false); + return ResponseEntity.status(HttpStatus.SERVICE_UNAVAILABLE).body(false); }src/main/java/de/tum/cit/aet/artemis/core/config/SecurityConfiguration.java (1)
211-212: Add debug logging for sharing endpoint access.While the security configuration is correct, adding debug logging would help track unauthorized access attempts.
Consider adding a custom filter to log access attempts to sharing endpoints:
public class SharingEndpointLoggingFilter extends OncePerRequestFilter { private final Logger log = LoggerFactory.getLogger(SharingEndpointLoggingFilter.class); @Override protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) throws ServletException, IOException { if (request.getRequestURI().startsWith("/api/sharing/")) { log.debug("Sharing endpoint accessed: {} from IP: {}", request.getRequestURI(), request.getRemoteAddr()); } filterChain.doFilter(request, response); } }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
.idea/runConfigurations/Artemis_Dev.xmlis excluded by!**/*.xmldocker/artemis-dev-local-vc-local-ci-mysql.ymlis excluded by!**/*.ymldocker/artemis.ymlis excluded by!**/*.ymlsrc/main/resources/config/application-dev.ymlis excluded by!**/*.ymlsrc/main/resources/config/application-sharing.ymlis excluded by!**/*.yml
📒 Files selected for processing (27)
build.gradle(2 hunks)docker/artemis/config/dev-local-vc-local-ci.env(1 hunks)src/main/java/de/tum/cit/aet/artemis/core/config/Constants.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/core/config/SecurityConfiguration.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/core/dto/SharingInfoDTO.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/core/service/ProfileService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/core/web/SharingSupportResource.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exercise/service/sharing/ProgrammingExerciseImportFromSharingService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exercise/service/sharing/SharingConnectorService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exercise/service/sharing/SharingException.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exercise/service/sharing/SharingPluginService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exercise/web/ExerciseSharingResource.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.java(7 hunks)src/main/java/de/tum/cit/aet/artemis/sharing/ExerciseSharingService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/sharing/SharingMultipartZipFile.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/sharing/SharingSetupInfo.java(1 hunks)src/main/webapp/app/app-routing.module.ts(1 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(4 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-management.module.ts(2 hunks)src/main/webapp/app/exercises/programming/manage/programming-exercise.component.ts(1 hunks)src/main/webapp/app/exercises/programming/manage/services/programming-exercise-sharing.service.ts(1 hunks)src/main/webapp/app/exercises/programming/manage/update/programming-exercise-creation-config.ts(1 hunks)src/main/webapp/app/exercises/programming/manage/update/programming-exercise-update.component.html(3 hunks)src/main/webapp/app/exercises/programming/manage/update/programming-exercise-update.component.ts(16 hunks)src/main/webapp/app/exercises/programming/shared/actions/programming-exercise-instructor-exercise-sharing.component.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (16)
- src/main/webapp/app/exercises/programming/manage/programming-exercise-management.module.ts
- src/main/webapp/app/exercises/programming/manage/update/programming-exercise-creation-config.ts
- src/main/java/de/tum/cit/aet/artemis/core/service/ProfileService.java
- src/main/webapp/app/exercises/programming/manage/programming-exercise.component.ts
- src/main/webapp/app/app-routing.module.ts
- build.gradle
- src/main/webapp/app/exercises/programming/manage/update/programming-exercise-update.component.html
- src/main/java/de/tum/cit/aet/artemis/core/config/Constants.java
- src/main/java/de/tum/cit/aet/artemis/sharing/SharingSetupInfo.java
- src/main/webapp/app/exercises/programming/manage/programming-exercise-detail.component.html
- docker/artemis/config/dev-local-vc-local-ci.env
- src/main/java/de/tum/cit/aet/artemis/exercise/web/ExerciseSharingResource.java
- src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.java
- src/main/java/de/tum/cit/aet/artemis/sharing/SharingMultipartZipFile.java
- src/main/java/de/tum/cit/aet/artemis/core/dto/SharingInfoDTO.java
- src/main/webapp/app/exercises/programming/manage/services/programming-exercise-sharing.service.ts
🧰 Additional context used
📓 Path-based instructions (11)
src/main/java/de/tum/cit/aet/artemis/exercise/service/sharing/SharingException.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/webapp/app/exercises/programming/manage/programming-exercise-management-routing.module.ts (1)
src/main/java/de/tum/cit/aet/artemis/exercise/service/sharing/ProgrammingExerciseImportFromSharingService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/core/config/SecurityConfiguration.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/webapp/app/exercises/programming/manage/programming-exercise-detail.component.ts (1)
src/main/webapp/app/exercises/programming/shared/actions/programming-exercise-instructor-exercise-sharing.component.ts (1)
src/main/webapp/app/exercises/programming/manage/update/programming-exercise-update.component.ts (1)
src/main/java/de/tum/cit/aet/artemis/exercise/service/sharing/SharingPluginService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/core/web/SharingSupportResource.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/exercise/service/sharing/SharingConnectorService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/sharing/ExerciseSharingService.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 (8)
src/main/webapp/app/exercises/programming/shared/actions/programming-exercise-instructor-exercise-sharing.component.ts (3)
1-11: LGTM! Clean imports and proper component setup.
The imports are well-organized and the component selector follows Angular naming conventions.
12-21: Remove the call to commented-out method.
The preOpenSharingTab() method is commented out internally, yet it's still being called in the template. This should be removed from the onClick handler.
-(onClick)="preOpenSharingTab(); exportExerciseToSharing(exerciseId)"
+(onClick)="exportExerciseToSharing(exerciseId)"28-31: Initialize properties and add input validation.
The sharingTab should be explicitly initialized, and exerciseId input should be validated.
src/main/webapp/app/exercises/programming/manage/update/programming-exercise-update.component.ts (1)
757-766: 🛠️ Refactor suggestion
Refactor nested subscriptions to improve readability and error handling.
The current code nests multiple subscriptions, which can make the code harder to read and maintain. Use RxJS operators like switchMap to chain Observables.
Consider refactoring to:
} else if (this.isImportFromSharing) {
- this.courseService.find(this.courseId).subscribe((res) => {
- this.programmingExerciseSharingService.setUpFromSharingImport(this.programmingExercise, res.body!, this.sharingInfo).subscribe({
- next: (response: HttpResponse<ProgrammingExercise>) => {
- this.alertService.success('artemisApp.programmingExercise.created', { param: this.programmingExercise.title });
- this.onSaveSuccess(response.body!);
- },
- error: (err) => this.onSaveError(err),
- });
- });
+ this.courseService.find(this.courseId).pipe(
+ switchMap((res) => this.programmingExerciseSharingService.setUpFromSharingImport(this.programmingExercise, res.body!, this.sharingInfo))
+ ).subscribe({
+ next: (response: HttpResponse<ProgrammingExercise>) => {
+ this.alertService.success('artemisApp.programmingExercise.created', { param: this.programmingExercise.title });
+ this.onSaveSuccess(response.body!);
+ },
+ error: (err) => this.onSaveError(err),
+ });
}Likely invalid or redundant comment.
src/main/java/de/tum/cit/aet/artemis/sharing/ExerciseSharingService.java (1)
252-252: 🛠️ Refactor suggestion
Correct the regex to match either “Problem-Statement” or “exercise.md”.
The alternation operator should be grouped to avoid partial matching. Escape “.” properly.
- Pattern pattern = Pattern.compile("^Problem-Statement|^exercise.md$", Pattern.CASE_INSENSITIVE);
+ Pattern pattern = Pattern.compile("^(Problem-Statement|exercise\\.md)$", Pattern.CASE_INSENSITIVE);Likely invalid or redundant comment.
src/main/java/de/tum/cit/aet/artemis/exercise/service/sharing/ProgrammingExerciseImportFromSharingService.java (2)
23-59: LGTM! Constructor follows Spring best practices.
The service is properly configured with appropriate annotations and uses constructor injection for dependencies.
67-75:
Add null checks and improve error handling.
The method has potential null pointer risks:
sharingSetupInfo.getExercise()could be null before accessinggetCourseViaExerciseGroupOrCourseMember()sharingSetupInfo.getCourse()could be null when setting the course
Apply this diff to add null checks:
public ProgrammingExercise importProgrammingExerciseFromSharing(SharingSetupInfo sharingSetupInfo) throws SharingException, IOException, GitAPIException, URISyntaxException {
+ if (sharingSetupInfo == null || sharingSetupInfo.getExercise() == null) {
+ throw new SharingException("Invalid sharing setup info: exercise data is missing");
+ }
+ if (sharingSetupInfo.getCourse() == null) {
+ throw new SharingException("Invalid sharing setup info: course data is missing");
+ }
SharingMultipartZipFile zipFile = exerciseSharingService.getCachedBasketItem(sharingSetupInfo.getSharingInfo());
User user = userRepository.getUserWithGroupsAndAuthorities();Likely invalid or redundant comment.
src/main/java/de/tum/cit/aet/artemis/core/web/SharingSupportResource.java (1)
68-84:
Improve error handling and security in getConfig endpoint.
- Use a more specific HTTP status code for unauthorized access (401 is used without proper constant)
- The IllegalArgumentException from URI.create() is not caught
Apply this diff to improve the implementation:
@GetMapping(SHARINGCONFIG_RESOURCE_PATH)
- public ResponseEntity<SharingPluginConfig> getConfig(@RequestHeader("Authorization") Optional<String> sharingApiKey, @RequestParam String apiBaseUrl,
+ public ResponseEntity<SharingPluginConfig> getConfig(@RequestHeader(value = "Authorization", required = false) String sharingApiKey, @RequestParam String apiBaseUrl,
@RequestParam String installationName) {
- if (sharingApiKey.isPresent() && sharingConnectorService.validate(sharingApiKey.get())) {
+ if (sharingApiKey != null && sharingConnectorService.validate(sharingApiKey)) {
log.info("Delivered Sharing Config ");
URL apiBaseUrl1;
try {
apiBaseUrl1 = URI.create(apiBaseUrl).toURL();
}
- catch (MalformedURLException e) {
+ catch (IllegalArgumentException | MalformedURLException e) {
+ log.warn("Invalid API base URL provided: {}", apiBaseUrl, e);
return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(null);
}
return ResponseEntity.ok(sharingConnectorService.getPluginConfig(apiBaseUrl1, installationName));
}
log.warn("Received wrong or missing api key");
- return ResponseEntity.status(401).body(null);
+ return ResponseEntity.status(HttpStatus.UNAUTHORIZED).body(null);
}Likely invalid or redundant comment.
| preOpenSharingTab() { | ||
| // the focus back to this window is not working, so we open in this window | ||
| /* | ||
| if(!this.sharingTab) { | ||
| this.sharingTab = window.open("about:blank", "sharing"); | ||
| } | ||
| */ | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove commented-out method.
Since the preOpenSharingTab method is commented out and not being used, it should be removed entirely from the codebase.
...in/webapp/app/exercises/programming/manage/programming-exercise-management-routing.module.ts
Show resolved
Hide resolved
src/main/webapp/app/exercises/programming/manage/programming-exercise-detail.component.ts
Show resolved
Hide resolved
| this.sharingInfo.basketToken = qparams['basketToken']; | ||
| this.sharingInfo.returnURL = qparams['returnUrl']; | ||
| this.sharingInfo.apiBaseURL = qparams['apiBaseUrl']; | ||
| this.sharingInfo.selectedExercise = qparams['selectedExercise']; |
There was a problem hiding this comment.
Validate query parameters for sharing import.
The code doesn't validate required query parameters for sharing import, which could lead to runtime errors.
Add parameter validation:
this.activatedRoute.queryParams.subscribe((qparams: Params) => {
+ const requiredParams = ['basketToken', 'returnUrl', 'apiBaseUrl', 'selectedExercise'];
+ const missingParams = requiredParams.filter(param => !qparams[param]);
+ if (missingParams.length > 0) {
+ this.alertService.error('artemisApp.programmingExercise.sharing.missingParams',
+ { params: missingParams.join(', ') });
+ return;
+ }
this.sharingInfo.basketToken = qparams['basketToken'];
this.sharingInfo.returnURL = qparams['returnUrl'];
this.sharingInfo.apiBaseURL = qparams['apiBaseUrl'];
this.sharingInfo.selectedExercise = qparams['selectedExercise'];Committable suggestion skipped: line range outside the PR's diff.
| private createProgrammingExerciseForImportFromSharing() { | ||
| this.activatedRoute.queryParams.subscribe((qparams: Params) => { | ||
| this.sharingInfo.basketToken = qparams['basketToken']; | ||
| this.sharingInfo.returnURL = qparams['returnUrl']; | ||
| this.sharingInfo.apiBaseURL = qparams['apiBaseUrl']; | ||
| this.sharingInfo.selectedExercise = qparams['selectedExercise']; | ||
| this.programmingExerciseSharingService.loadDetailsForExercises(this.sharingInfo).subscribe( | ||
| (exerciseDetails: ProgrammingExercise) => { | ||
| if (!exerciseDetails.buildConfig) { | ||
| exerciseDetails.buildConfig = new ProgrammingExerciseBuildConfig(); | ||
| } | ||
| history.state.programmingExerciseForImportFromFile = exerciseDetails; | ||
|
|
||
| this.createProgrammingExerciseForImportFromFile(); | ||
| }, | ||
| (error) => { | ||
| this.alertService.error('Failed to load exercise details from the sharing platform: ' + error); | ||
| }, | ||
| ); | ||
| }); | ||
| /* | ||
| this.programmingExerciseSharingService.loadProblemStatementForExercises(this.sharingInfo).subscribe((statement: string) => { | ||
| this.programmingExercise.problemStatement = statement; | ||
| }); | ||
| */ | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Clean up commented code and improve error handling.
The file contains commented-out code and could benefit from improved error handling.
- Remove the commented-out code block (lines 1283-1286)
- Improve error handling by providing more context:
- (error) => {
- this.alertService.error('Failed to load exercise details from the sharing platform: ' + error);
- },
+ (error: HttpErrorResponse) => {
+ const errorMessage = error.headers?.get('X-artemisApp-alert') || error.message;
+ this.alertService.error('artemisApp.programmingExercise.sharing.loadError', { error: errorMessage });
+ },Committable suggestion skipped: line range outside the PR's diff.
| String decodedToken = new String(Base64.getDecoder().decode(b64Token)); | ||
| Path parent = Paths.get(repoDownloadClonePath, decodedToken + ".zip"); | ||
| File exportedExercise = parent.toFile(); | ||
| if (exportedExercise.exists()) { |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI 8 days ago
To fix the problem, we need to ensure that the constructed file path is within a specific directory and does not allow access to unintended files. This can be achieved by resolving the path and checking that it remains within the intended directory.
- Normalize the constructed path to its absolute form.
- Ensure that the resolved path starts with the intended base directory.
- If the path is outside the intended directory, reject the input.
| @@ -429,3 +429,8 @@ | ||
| String decodedToken = new String(Base64.getDecoder().decode(b64Token)); | ||
| Path parent = Paths.get(repoDownloadClonePath, decodedToken + ".zip"); | ||
| Path basePath = Paths.get(repoDownloadClonePath).normalize().toAbsolutePath(); | ||
| Path parent = basePath.resolve(decodedToken + ".zip").normalize().toAbsolutePath(); | ||
| if (!parent.startsWith(basePath)) { | ||
| log.warn("Invalid path traversal attempt: {}", decodedToken); | ||
| return null; | ||
| } | ||
| File exportedExercise = parent.toFile(); |
Sorry, new try,: Old pull request reversed branches :-(
This pull request resolves issue #3592
Checklist
This pull request is "Work in Progress". Please bear with us.
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
The CodeAbility Project currently operates and maintains a platform for sharing (among other content) programming exercises between interested parties. It is open to the programming teaching community.
CodeAbility also operates an Artemis instance that serves its 6 Austrian partners as a platform for programming teaching.
Artemis is currently a major source of programming exercises, as well as a potential target for re-using programming exercises.
However the exchange between separated instances of Artemis is only supported by a manual exchange of zipped exercises files.
We have implemented a REST-based connector interface in the sharing platform to import and export exercises between interested applications.
This connector interface allows among other features the import and export of programming exercises in an appropriate format.
We have already implemented an prototype integration of Artemis with the sharing platform based on the connector interface in order to allow for the smooth sharing of exercises.
Description
An additional spring profile "sharing" is implemented, that enables the functionality to export/import programming exercises via the sharing plattform.
The sharing requires an registration of the artemis instance at artemis-support-informatik@uibk.ac.at. During the registration a pre shared key is exchanged, that allows the communication between artemis and the sharing plattform.
When the sharing profile is enabled, (and configuration is initialized properly), the the sharing platform connects regularly with the artemis instance to query the connector configuration.
Additional the artemis instance can trigger an configuration request for the sharing plattform at application startup.
Details can be found at https://sharing-codeability.uibk.ac.at/development/sharing/codeability-sharing-platform/-/wikis/Artemis-Connection/Connector-Interface
Steps for Testing
Export from Artemis to the sharing plattform
Prerequisites:
with additional access right to the sharing plattform gitlab instance (https://sharing-codeability.uibk.ac.at)
Expected Result:
Import from the sharing plattform to Artemis
Prerequisites:
with additional access right to the sharing plattform gitlab instance (https://sharing-codeability.uibk.ac.at)
https://user-images.githubusercontent.com/18215713/122518891-1aa5e300-d012-11eb-92a8-4c76f8fb31ac.png
Expected Result:
Testserver States
Work in Progress: It has to be clarified, against how the integration sharing platform instance at https://sharing.codeability-austria.uibk.ac.at/static/SharingCodeAbility.html
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
Work in Progress: It has to be clarified, against how the integration sharing platform instance at https://sharing.codeability-austria.uibk.ac.at/static/SharingCodeAbility.html
Performance Review
Code Review
Manual Tests
Exam Mode Test
Performance Tests
Test Coverage
Screenshots
Work in Progress: Please bear with us!
Summary by CodeRabbit
Release Notes
New Features
UI Enhancements
Bug Fixes
Documentation