Modeling exercises: Add Athena module access checks
#11203
Conversation
…odules: client side
…g-exercises/choose-preliminary-feedback-model
…iminary-feedback-model
…oose-preliminary-feedback-model' into feature/programming-exercises/choose-preliminary-feedback-model
…iminary-feedback-model
…g-exercises/choose-preliminary-feedback-model # Conflicts: # src/main/webapp/app/exercises/modeling/manage/modeling-exercise.module.ts # src/main/webapp/app/exercises/programming/shared/lifecycle/programming-exercise-lifecycle.module.ts # src/main/webapp/app/exercises/shared/feedback-suggestion/exercise-feedback-suggestion-options.component.ts # src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise.module.ts # src/main/webapp/app/overview/exercise-details/exercise-details-student-actions.component.html # src/main/webapp/i18n/de/exercise.json # src/test/javascript/spec/component/exercises/shared/feedback/feedback-suggestion-option.component.spec.ts # src/test/javascript/spec/component/modeling-exercise/modeling-exercise-update.component.spec.ts # src/test/javascript/spec/component/overview/exercise-details/exercise-details-student-actions.component.spec.ts # src/test/javascript/spec/component/overview/exercise-details/request-feedback-button/request-feedback-button.component.spec.ts # src/test/javascript/spec/component/programming-exercise/programming-exercise-lifecycle.component.spec.ts # src/test/javascript/spec/component/text-exercise/text-exercise-update.component.spec.ts
WalkthroughThe changes extend the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ModelingExerciseResource
participant AthenaApi (optional)
participant Database
User->>ModelingExerciseResource: createModelingExercise / updateModelingExercise / importExercise
alt AthenaApi present
ModelingExerciseResource->>AthenaApi: check access to FEEDBACK_SUGGESTIONS & PRELIMINARY_FEEDBACK
AthenaApi-->>ModelingExerciseResource: access allowed/denied
ModelingExerciseResource->>ModelingExerciseResource: enable/disable feedback modules accordingly
else AthenaApi absent
ModelingExerciseResource->>ModelingExerciseResource: disable feedback modules
end
ModelingExerciseResource->>Database: save or update exercise
Database-->>ModelingExerciseResource: confirmation
ModelingExerciseResource-->>User: response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Possibly related PRs
Suggested labels
Suggested reviewers
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. 🔧 ESLint
npm error Exit handler never called! Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/de/tum/cit/aet/artemis/modeling/web/ModelingExerciseResource.java(6 hunks)src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/main/webapp/**/*.ts
⚙️ CodeRabbit Configuration File
Files:
src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.ts
src/main/java/**/*.java
⚙️ CodeRabbit Configuration File
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
Files:
src/main/java/de/tum/cit/aet/artemis/modeling/web/ModelingExerciseResource.java
🧠 Learnings (3)
📓 Common learnings
Learnt from: Wallenstein61
PR: ls1intum/Artemis#10989
File: src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java:122-135
Timestamp: 2025-06-06T14:47:54.300Z
Learning: In Artemis sharing platform integration (under "sharing" profile), endpoints in ExerciseSharingResource use checksum-based authentication with shared secrets instead of @PreAuthorize annotations. These endpoints (like getProblemStatement, getExerciseDetails, loadShoppingBasket) validate security through sharingInfo.checkChecksum() calls rather than role-based access control, as they serve as API interfaces for external sharing platform communication.
Learnt from: magaupp
PR: ls1intum/Artemis#8802
File: src/main/resources/templates/rust/exercise/src/context.rs:1-1
Timestamp: 2024-08-05T00:11:50.650Z
Learning: Code inside the `exercise` directories in the Artemis platform is provided to students and is intended for them to implement. TODO comments in these directories are meant to guide students and should not be addressed in the PR.
Learnt from: magaupp
PR: ls1intum/Artemis#8802
File: src/main/resources/templates/rust/exercise/src/context.rs:1-1
Timestamp: 2024-10-08T15:35:42.972Z
Learning: Code inside the `exercise` directories in the Artemis platform is provided to students and is intended for them to implement. TODO comments in these directories are meant to guide students and should not be addressed in the PR.
Learnt from: eceeeren
PR: ls1intum/Artemis#10533
File: src/main/java/de/tum/cit/aet/artemis/core/web/CourseResource.java:839-857
Timestamp: 2025-04-04T09:39:24.832Z
Learning: In the Artemis application, filtering of unreleased exercises is handled at the database level through SQL queries in the repository methods rather than in the application code. The method findByCourseIdWithFutureDueDatesAndCategories in ExerciseRepository applies this filtering, making additional application-level filtering unnecessary.
Learnt from: eceeeren
PR: ls1intum/Artemis#10533
File: src/main/java/de/tum/cit/aet/artemis/core/web/CourseResource.java:839-857
Timestamp: 2025-04-04T09:39:24.832Z
Learning: When filtering unreleased exercises in the Artemis application, the preferred approach is to handle it at the database level through SQL queries rather than filtering at the application level, as this is more efficient for performance.
Learnt from: janthoXO
PR: ls1intum/Artemis#9406
File: src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseParticipationResource.java:209-209
Timestamp: 2025-02-11T12:05:49.151Z
Learning: In ProgrammingExerciseParticipationResource, exam-related authorization checks and sensitive information filtering for results and feedbacks are handled by resultService.filterSensitiveInformationIfNecessary().
Learnt from: SimonEntholzer
PR: ls1intum/Artemis#8929
File: src/main/java/de/tum/in/www1/artemis/web/rest/UserResource.java:229-243
Timestamp: 2024-10-08T15:35:42.972Z
Learning: In the Artemis project, the VCS access token is bound to both the user and the participation, ensuring that only the owning user can access the token. Therefore, additional authorization checks in the `getVcsAccessToken` method are unnecessary.
Learnt from: Wallenstein61
PR: ls1intum/Artemis#9909
File: src/main/webapp/app/exercises/programming/manage/services/programming-exercise-sharing.service.ts:120-124
Timestamp: 2025-02-11T16:46:26.037Z
Learning: In the Artemis sharing platform integration, passing callBackUrl in exportProgrammingExerciseToSharing is safe as it's returned to the same authenticated user session through internal API endpoints.
Learnt from: SimonEntholzer
PR: ls1intum/Artemis#8929
File: src/test/java/de/tum/in/www1/artemis/authentication/UserLocalVcIntegrationTest.java:29-33
Timestamp: 2024-07-16T19:27:15.402Z
Learning: When addressing missing test coverage, ensure that new tests specifically validate the functionality introduced in the PR, such as token-based authentication.
Learnt from: iyannsch
PR: ls1intum/Artemis#8965
File: src/main/java/de/tum/in/www1/artemis/domain/ProgrammingExercise.java:97-98
Timestamp: 2024-10-08T15:35:48.767Z
Learning: For the Artemis project, the field `allowOnlineIde` in the `ProgrammingExercise` class should use the primitive type `boolean` to ensure it is non-nullable.
Learnt from: JohannesStoehr
PR: ls1intum/Artemis#8976
File: src/main/webapp/app/entities/quiz/quiz-exercise.model.ts:0-0
Timestamp: 2024-07-07T13:57:07.670Z
Learning: When defining the `resetQuizForImport` function in `src/main/webapp/app/entities/quiz/quiz-exercise.model.ts`, ensure to directly refer to the `exercise` parameter instead of using `this`.
Learnt from: JohannesStoehr
PR: ls1intum/Artemis#8976
File: src/main/webapp/app/entities/quiz/quiz-exercise.model.ts:0-0
Timestamp: 2024-10-08T15:35:42.972Z
Learning: When defining the `resetQuizForImport` function in `src/main/webapp/app/entities/quiz/quiz-exercise.model.ts`, ensure to directly refer to the `exercise` parameter instead of using `this`.
Learnt from: iyannsch
PR: ls1intum/Artemis#9379
File: src/main/webapp/app/shared/components/code-button/code-button.component.ts:364-372
Timestamp: 2024-10-08T18:07:58.425Z
Learning: In the `initTheia` method of `CodeButtonComponent` (`code-button.component.ts`), removing the null check for `this.exercise` inside the `subscribe` callback causes a compile-time error, so it should be kept.
src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.ts (16)
Learnt from: SimonEntholzer
PR: #9397
File: src/main/webapp/app/shared/user-settings/user-settings-container/user-settings-container.component.ts:1-1
Timestamp: 2024-10-08T15:35:52.595Z
Learning: In this project, refrain from suggesting the replacement of the inject() function with constructor injection in components.
Learnt from: SimonEntholzer
PR: #10147
File: src/main/webapp/app/shared/components/code-button/code-button.component.ts:1-1
Timestamp: 2025-01-18T08:49:30.693Z
Learning: In Angular 19+ projects, component inputs should use the new input() function from @angular/core instead of the @input() decorator, as it's part of the new signals architecture that improves performance and type safety.
Learnt from: florian-glombik
PR: #8858
File: src/main/webapp/app/shared/exercise-filter/exercise-filter-modal.component.ts:185-188
Timestamp: 2024-07-09T19:07:38.801Z
Learning: For the PR #8858, avoid suggesting moving assignments out of expressions within the resetFilter method in exercise-filter-modal.component.ts.
Learnt from: iyannsch
PR: #9379
File: src/main/webapp/app/shared/components/code-button/code-button.component.ts:364-372
Timestamp: 2024-10-08T18:07:58.425Z
Learning: In the initTheia method of CodeButtonComponent (code-button.component.ts), removing the null check for this.exercise inside the subscribe callback causes a compile-time error, so it should be kept.
Learnt from: iyannsch
PR: #9379
File: src/main/webapp/app/shared/components/code-button/code-button.component.ts:143-170
Timestamp: 2024-10-08T15:35:42.972Z
Learning: In code-button.component.ts, this.exercise.id is checked before use, so it's acceptable to use the non-null assertion operator ! on this.exercise.id!.
Learnt from: iyannsch
PR: #9379
File: src/main/webapp/app/shared/components/code-button/code-button.component.ts:143-170
Timestamp: 2024-09-30T08:34:49.363Z
Learning: In code-button.component.ts, this.exercise.id is checked before use, so it's acceptable to use the non-null assertion operator ! on this.exercise.id!.
Learnt from: tobias-lippert
PR: #10714
File: src/main/webapp/app/quiz/manage/create-buttons/quiz-exercise-create-buttons.component.ts:11-15
Timestamp: 2025-04-22T10:19:41.546Z
Learning: In Angular 19, standalone components are the default, so components with an imports array don't need to explicitly declare standalone: true property in the component decorator.
Learnt from: pzdr7
PR: #9230
File: src/main/webapp/app/admin/standardized-competencies/standardized-competency-management.component.ts:90-90
Timestamp: 2024-08-20T19:00:23.461Z
Learning: In the StandardizedCompetencyManagementComponent, the ChangeDetectorRef is named changeDetectorRef for clarity, as per the user's preference.
Learnt from: JohannesStoehr
PR: #8976
File: src/main/webapp/app/entities/quiz/quiz-exercise.model.ts:0-0
Timestamp: 2024-07-07T13:57:07.670Z
Learning: When defining the resetQuizForImport function in src/main/webapp/app/entities/quiz/quiz-exercise.model.ts, ensure to directly refer to the exercise parameter instead of using this.
Learnt from: JohannesStoehr
PR: #8976
File: src/main/webapp/app/entities/quiz/quiz-exercise.model.ts:0-0
Timestamp: 2024-10-08T15:35:42.972Z
Learning: When defining the resetQuizForImport function in src/main/webapp/app/entities/quiz/quiz-exercise.model.ts, ensure to directly refer to the exercise parameter instead of using this.
Learnt from: janthoXO
PR: #9201
File: src/main/webapp/app/shared/user-settings/ide-preferences/ide-settings.component.html:12-23
Timestamp: 2024-08-19T21:24:00.705Z
Learning: In Angular components, using the OnPush change detection strategy can mitigate performance issues related to method calls in templates, as it reduces unnecessary checks by only updating the view when specific conditions are met.
Learnt from: Wallenstein61
PR: #9909
File: src/main/webapp/app/sharing/search-result-dto.model.ts:50-55
Timestamp: 2025-02-11T15:46:35.616Z
Learning: The IExerciseType enum in src/main/webapp/app/sharing/search-result-dto.model.ts must maintain its current naming (with 'I' prefix) and lowercase string values to ensure compatibility with the external sharing platform connector interface. This is an intentional exception to our TypeScript naming conventions.
Learnt from: ekayandan
PR: #10885
File: src/main/webapp/app/programming/shared/git-diff-report/git-diff-report/git-diff-report.component.ts:63-74
Timestamp: 2025-07-03T14:51:57.407Z
Learning: In the GitDiffReportComponent (src/main/webapp/app/programming/shared/git-diff-report/git-diff-report/git-diff-report.component.ts), directly mutating the diffInformation array from repositoryDiffInformation().diffInformations to update the diffReady property works correctly with Angular's OnPush change detection strategy in this codebase.
Learnt from: coolchock
PR: #10456
File: src/main/webapp/app/exam/overview/summary/exercises/quiz-exam-summary/quiz-exam-summary.component.ts:68-75
Timestamp: 2025-03-26T12:29:47.826Z
Learning: Components with inputs defined using input.required<T>() in Angular don't need optional chaining when accessing the properties of the input value, as these inputs are guaranteed to have a value when the component is properly used.
Learnt from: tobias-lippert
PR: #10184
File: src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise.route.ts:8-8
Timestamp: 2025-02-03T10:40:17.951Z
Learning: In the Artemis application's routing architecture, route parameters like 'courseId' are declared in the files that import the routes rather than in the route definitions themselves, allowing for more modular and reusable route configurations.
Learnt from: tobias-lippert
PR: #10610
File: src/main/webapp/app/core/course/overview/course-overview.component.ts:98-117
Timestamp: 2025-04-01T22:05:07.175Z
Learning: Error handling is not needed in component initialization methods (like ngOnInit) in Artemis components. If initialization fails, the component is considered not usable at all, and the error should propagate to higher-level handlers rather than being caught and handled at the component level.
src/main/java/de/tum/cit/aet/artemis/modeling/web/ModelingExerciseResource.java (21)
Learnt from: janthoXO
PR: #9406
File: src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseParticipationResource.java:209-209
Timestamp: 2025-02-11T12:05:49.151Z
Learning: In ProgrammingExerciseParticipationResource, exam-related authorization checks and sensitive information filtering for results and feedbacks are handled by resultService.filterSensitiveInformationIfNecessary().
Learnt from: Wallenstein61
PR: #10989
File: src/main/java/de/tum/cit/aet/artemis/programming/web/ExerciseSharingResource.java:122-135
Timestamp: 2025-06-06T14:47:54.300Z
Learning: In Artemis sharing platform integration (under "sharing" profile), endpoints in ExerciseSharingResource use checksum-based authentication with shared secrets instead of @PreAuthorize annotations. These endpoints (like getProblemStatement, getExerciseDetails, loadShoppingBasket) validate security through sharingInfo.checkChecksum() calls rather than role-based access control, as they serve as API interfaces for external sharing platform communication.
Learnt from: magaupp
PR: #8802
File: src/main/resources/templates/rust/exercise/src/context.rs:1-1
Timestamp: 2024-08-05T00:11:50.650Z
Learning: Code inside the exercise directories in the Artemis platform is provided to students and is intended for them to implement. TODO comments in these directories are meant to guide students and should not be addressed in the PR.
Learnt from: magaupp
PR: #8802
File: src/main/resources/templates/rust/exercise/src/context.rs:1-1
Timestamp: 2024-10-08T15:35:42.972Z
Learning: Code inside the exercise directories in the Artemis platform is provided to students and is intended for them to implement. TODO comments in these directories are meant to guide students and should not be addressed in the PR.
Learnt from: magaupp
PR: #9751
File: src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseFactory.java:143-148
Timestamp: 2024-11-26T20:43:17.588Z
Learning: In src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseFactory.java, the test package name assigned in populateUnreleasedProgrammingExercise does not need to adhere to naming conventions.
Learnt from: iyannsch
PR: #8965
File: src/main/java/de/tum/in/www1/artemis/domain/ProgrammingExercise.java:97-98
Timestamp: 2024-07-10T11:39:26.373Z
Learning: For the Artemis project, the field allowOnlineIde in the ProgrammingExercise class should use the primitive type boolean to ensure it is non-nullable.
Learnt from: iyannsch
PR: #8965
File: src/main/java/de/tum/in/www1/artemis/domain/ProgrammingExercise.java:97-98
Timestamp: 2024-10-08T15:35:48.767Z
Learning: For the Artemis project, the field allowOnlineIde in the ProgrammingExercise class should use the primitive type boolean to ensure it is non-nullable.
Learnt from: eceeeren
PR: #10533
File: src/main/java/de/tum/cit/aet/artemis/core/web/CourseResource.java:839-857
Timestamp: 2025-04-04T09:39:24.832Z
Learning: In the Artemis application, filtering of unreleased exercises is handled at the database level through SQL queries in the repository methods rather than in the application code. The method findByCourseIdWithFutureDueDatesAndCategories in ExerciseRepository applies this filtering, making additional application-level filtering unnecessary.
Learnt from: julian-christl
PR: #7829
File: src/main/java/de/tum/in/www1/artemis/web/rest/ComplaintResponseResource.java:0-0
Timestamp: 2024-10-08T15:35:42.972Z
Learning: The user has fixed the issue regarding the redundant wildcard import in ComplaintResponseResource.java by removing it in commit 7e392e0.
Learnt from: julian-christl
PR: #7829
File: src/main/java/de/tum/in/www1/artemis/web/rest/ComplaintResponseResource.java:0-0
Timestamp: 2024-06-10T19:44:09.116Z
Learning: The user has fixed the issue regarding the redundant wildcard import in ComplaintResponseResource.java by removing it in commit 7e392e0.
Learnt from: julian-christl
PR: #8052
File: src/test/java/de/tum/in/www1/artemis/lecture/CompetencyIntegrationTest.java:310-310
Timestamp: 2024-06-10T19:44:09.116Z
Learning: Modifications to parameters in competencyProgressUtilService.createCompetencyProgress for debugging purposes are considered irrelevant to the test outcomes but helpful for clarity during debugging.
Learnt from: Wallenstein61
PR: #9909
File: src/main/webapp/app/exercises/programming/manage/services/programming-exercise-sharing.service.ts:120-124
Timestamp: 2025-02-11T16:46:26.037Z
Learning: In the Artemis sharing platform integration, passing callBackUrl in exportProgrammingExerciseToSharing is safe as it's returned to the same authenticated user session through internal API endpoints.
Learnt from: julian-christl
PR: #9322
File: src/main/java/de/tum/cit/aet/artemis/programming/repository/ProgrammingExerciseStudentParticipationRepository.java:0-0
Timestamp: 2024-10-08T15:35:42.972Z
Learning: In Artemis, an ExerciseGroup always has an associated Exam, so exerciseGroup.exam is never null.
Learnt from: KonstiAnon
PR: #11163
File: src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizExerciseService.java:677-690
Timestamp: 2025-07-15T20:58:06.751Z
Learning: In QuizExerciseService.copyFieldsForUpdate(), shallow copying using BeanUtils.copyProperties is intentional and sufficient because the update logic overwrites references without modifying the instances in-place, making deep copying unnecessary.
Learnt from: valentin-boehm
PR: #7384
File: src/test/java/de/tum/in/www1/artemis/exam/StudentExamIntegrationTest.java:988-993
Timestamp: 2024-06-10T19:44:09.116Z
Learning: The testSubmitStudentExam_differentUser method does not require additional checks to verify the state of studentExam1 after receiving a HttpStatus.FORBIDDEN because the control flow in the StudentExamResource is straightforward and ensures no state change occurs.
Learnt from: valentin-boehm
PR: #7384
File: src/test/java/de/tum/in/www1/artemis/exam/StudentExamIntegrationTest.java:975-980
Timestamp: 2024-06-10T19:44:09.116Z
Learning: The testSubmitStudentExam_notInTime method does not require additional checks to verify the state of studentExam1 after receiving a HttpStatus.FORBIDDEN because the control flow in the StudentExamResource is straightforward and ensures no state change occurs.
Learnt from: ahmetsenturk
PR: #10187
File: src/main/java/de/tum/cit/aet/artemis/core/web/UserResource.java:158-170
Timestamp: 2025-01-28T12:24:34.419Z
Learning: The update query for external LLM acceptance in UserRepository already has proper annotations (@Modifying and @transactional) to handle concurrent updates safely.
Learnt from: Wallenstein61
PR: #10989
File: src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.java:108-117
Timestamp: 2025-06-10T12:26:42.449Z
Learning: In ProgrammingExerciseImportFromFileService, the current directory traversal logic for sharing imports (walking importExerciseDir before extraction) is working correctly in practice and should not be changed to more complex solutions, even if there are theoretical issues with the approach.
Learnt from: theblobinthesky
PR: #10581
File: src/main/webapp/app/exercise/import/from-file/exercise-import-from-file.component.ts:57-60
Timestamp: 2025-05-11T22:58:13.489Z
Learning: The ProgrammingExerciseBuildConfig constructor initializes all fields with default values, including new fields like allowBranching (false) and branchRegex ('.*'), so explicit handling of these fields isn't needed when importing old exercise configurations.
Learnt from: JohannesStoehr
PR: #8976
File: src/main/webapp/app/entities/quiz/quiz-exercise.model.ts:0-0
Timestamp: 2024-10-08T15:35:42.972Z
Learning: When defining the resetQuizForImport function in src/main/webapp/app/entities/quiz/quiz-exercise.model.ts, ensure to directly refer to the exercise parameter instead of using this.
Learnt from: JohannesStoehr
PR: #8976
File: src/main/webapp/app/entities/quiz/quiz-exercise.model.ts:0-0
Timestamp: 2024-07-07T13:57:07.670Z
Learning: When defining the resetQuizForImport function in src/main/webapp/app/entities/quiz/quiz-exercise.model.ts, ensure to directly refer to the exercise parameter instead of using this.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: server-tests
- GitHub Check: client-tests
🔇 Additional comments (2)
src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.ts (1)
1-1: LGTM! Correct use of Angular's inject() function.The refactoring from constructor-based dependency injection to the
inject()function is properly implemented and follows the project's preferred approach.Also applies to: 35-37
src/main/java/de/tum/cit/aet/artemis/modeling/web/ModelingExerciseResource.java (1)
367-383: Well-implemented exception handling for Athena access checks.The import method correctly handles potential exceptions from Athena access checks, ensuring graceful degradation when access is denied. This implementation pattern should be applied to the create and update methods for consistency.
| athenaApi.ifPresentOrElse(api -> api.checkHasAccessToAthenaModule(modelingExercise, course, AthenaModuleMode.FEEDBACK_SUGGESTIONS, ENTITY_NAME), | ||
| () -> modelingExercise.setFeedbackSuggestionModule(null)); | ||
| athenaApi.ifPresentOrElse(api -> api.checkHasAccessToAthenaModule(modelingExercise, course, AthenaModuleMode.PRELIMINARY_FEEDBACK, ENTITY_NAME), | ||
| () -> modelingExercise.setPreliminaryFeedbackModule(null)); | ||
|
|
There was a problem hiding this comment.
Consider consistent exception handling for Athena access checks.
The current implementation doesn't catch potential BadRequestAlertException that might be thrown by checkHasAccessToAthenaModule, unlike the import method (lines 369-383). This could lead to different behavior between create and import operations. Consider wrapping these checks in try-catch blocks for consistency.
Apply this diff to ensure consistent exception handling:
- athenaApi.ifPresentOrElse(api -> api.checkHasAccessToAthenaModule(modelingExercise, course, AthenaModuleMode.FEEDBACK_SUGGESTIONS, ENTITY_NAME),
- () -> modelingExercise.setFeedbackSuggestionModule(null));
- athenaApi.ifPresentOrElse(api -> api.checkHasAccessToAthenaModule(modelingExercise, course, AthenaModuleMode.PRELIMINARY_FEEDBACK, ENTITY_NAME),
- () -> modelingExercise.setPreliminaryFeedbackModule(null));
+ try {
+ athenaApi.ifPresentOrElse(api -> api.checkHasAccessToAthenaModule(modelingExercise, course, AthenaModuleMode.FEEDBACK_SUGGESTIONS, ENTITY_NAME),
+ () -> modelingExercise.setFeedbackSuggestionModule(null));
+ }
+ catch (BadRequestAlertException e) {
+ modelingExercise.setFeedbackSuggestionModule(null);
+ }
+ try {
+ athenaApi.ifPresentOrElse(api -> api.checkHasAccessToAthenaModule(modelingExercise, course, AthenaModuleMode.PRELIMINARY_FEEDBACK, ENTITY_NAME),
+ () -> modelingExercise.setPreliminaryFeedbackModule(null));
+ }
+ catch (BadRequestAlertException e) {
+ modelingExercise.setPreliminaryFeedbackModule(null);
+ }📝 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.
| athenaApi.ifPresentOrElse(api -> api.checkHasAccessToAthenaModule(modelingExercise, course, AthenaModuleMode.FEEDBACK_SUGGESTIONS, ENTITY_NAME), | |
| () -> modelingExercise.setFeedbackSuggestionModule(null)); | |
| athenaApi.ifPresentOrElse(api -> api.checkHasAccessToAthenaModule(modelingExercise, course, AthenaModuleMode.PRELIMINARY_FEEDBACK, ENTITY_NAME), | |
| () -> modelingExercise.setPreliminaryFeedbackModule(null)); | |
| try { | |
| athenaApi.ifPresentOrElse(api -> api.checkHasAccessToAthenaModule(modelingExercise, course, AthenaModuleMode.FEEDBACK_SUGGESTIONS, ENTITY_NAME), | |
| () -> modelingExercise.setFeedbackSuggestionModule(null)); | |
| } catch (BadRequestAlertException e) { | |
| modelingExercise.setFeedbackSuggestionModule(null); | |
| } | |
| try { | |
| athenaApi.ifPresentOrElse(api -> api.checkHasAccessToAthenaModule(modelingExercise, course, AthenaModuleMode.PRELIMINARY_FEEDBACK, ENTITY_NAME), | |
| () -> modelingExercise.setPreliminaryFeedbackModule(null)); | |
| } catch (BadRequestAlertException e) { | |
| modelingExercise.setPreliminaryFeedbackModule(null); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/de/tum/cit/aet/artemis/modeling/web/ModelingExerciseResource.java
around lines 177 to 181, the calls to checkHasAccessToAthenaModule are not
wrapped in try-catch blocks, unlike similar calls in the import method at lines
369-383. To fix this, wrap each checkHasAccessToAthenaModule call inside a
try-catch block that catches BadRequestAlertException and handles it
consistently, ensuring uniform exception handling behavior between create and
import operations.
| athenaApi.ifPresentOrElse(api -> api.checkHasAccessToAthenaModule(modelingExercise, course, AthenaModuleMode.FEEDBACK_SUGGESTIONS, ENTITY_NAME), | ||
| () -> modelingExercise.setFeedbackSuggestionModule(null)); | ||
| athenaApi.ifPresentOrElse(api -> api.checkHasAccessToAthenaModule(modelingExercise, course, AthenaModuleMode.PRELIMINARY_FEEDBACK, ENTITY_NAME), | ||
| () -> modelingExercise.setPreliminaryFeedbackModule(null)); | ||
| // Changing Athena module after the due date has passed is not allowed |
There was a problem hiding this comment.
Apply consistent exception handling for Athena access checks in update method.
Similar to the create method, these Athena access checks should handle potential BadRequestAlertException for consistency with the import method implementation.
Apply this diff:
- athenaApi.ifPresentOrElse(api -> api.checkHasAccessToAthenaModule(modelingExercise, course, AthenaModuleMode.FEEDBACK_SUGGESTIONS, ENTITY_NAME),
- () -> modelingExercise.setFeedbackSuggestionModule(null));
- athenaApi.ifPresentOrElse(api -> api.checkHasAccessToAthenaModule(modelingExercise, course, AthenaModuleMode.PRELIMINARY_FEEDBACK, ENTITY_NAME),
- () -> modelingExercise.setPreliminaryFeedbackModule(null));
+ try {
+ athenaApi.ifPresentOrElse(api -> api.checkHasAccessToAthenaModule(modelingExercise, course, AthenaModuleMode.FEEDBACK_SUGGESTIONS, ENTITY_NAME),
+ () -> modelingExercise.setFeedbackSuggestionModule(null));
+ }
+ catch (BadRequestAlertException e) {
+ modelingExercise.setFeedbackSuggestionModule(null);
+ }
+ try {
+ athenaApi.ifPresentOrElse(api -> api.checkHasAccessToAthenaModule(modelingExercise, course, AthenaModuleMode.PRELIMINARY_FEEDBACK, ENTITY_NAME),
+ () -> modelingExercise.setPreliminaryFeedbackModule(null));
+ }
+ catch (BadRequestAlertException e) {
+ modelingExercise.setPreliminaryFeedbackModule(null);
+ }📝 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.
| athenaApi.ifPresentOrElse(api -> api.checkHasAccessToAthenaModule(modelingExercise, course, AthenaModuleMode.FEEDBACK_SUGGESTIONS, ENTITY_NAME), | |
| () -> modelingExercise.setFeedbackSuggestionModule(null)); | |
| athenaApi.ifPresentOrElse(api -> api.checkHasAccessToAthenaModule(modelingExercise, course, AthenaModuleMode.PRELIMINARY_FEEDBACK, ENTITY_NAME), | |
| () -> modelingExercise.setPreliminaryFeedbackModule(null)); | |
| // Changing Athena module after the due date has passed is not allowed | |
| try { | |
| athenaApi.ifPresentOrElse(api -> api.checkHasAccessToAthenaModule(modelingExercise, course, AthenaModuleMode.FEEDBACK_SUGGESTIONS, ENTITY_NAME), | |
| () -> modelingExercise.setFeedbackSuggestionModule(null)); | |
| } | |
| catch (BadRequestAlertException e) { | |
| modelingExercise.setFeedbackSuggestionModule(null); | |
| } | |
| try { | |
| athenaApi.ifPresentOrElse(api -> api.checkHasAccessToAthenaModule(modelingExercise, course, AthenaModuleMode.PRELIMINARY_FEEDBACK, ENTITY_NAME), | |
| () -> modelingExercise.setPreliminaryFeedbackModule(null)); | |
| } | |
| catch (BadRequestAlertException e) { | |
| modelingExercise.setPreliminaryFeedbackModule(null); | |
| } | |
| // Changing Athena module after the due date has passed is not allowed |
🤖 Prompt for AI Agents
In
src/main/java/de/tum/cit/aet/artemis/modeling/web/ModelingExerciseResource.java
around lines 246 to 250, the Athena access checks use ifPresentOrElse without
handling BadRequestAlertException, unlike the create method. Modify these checks
to catch and handle BadRequestAlertException consistently by wrapping the calls
in try-catch blocks or using a similar exception handling approach as in the
create method to ensure uniform error handling.
github-project-automation
bot
moved this from Work In Progress
to Ready For Review
in Artemis Development
End-to-End (E2E) Test Results Summary |
helios-aet
bot
temporarily deployed
to
artemis-test3.artemis.cit.tum.de
helios-aet
bot
temporarily deployed
to
artemis-test1.artemis.cit.tum.de
helios-aet
bot
temporarily deployed
to
artemis-test1.artemis.cit.tum.de
helios-aet
bot
temporarily deployed
to
artemis-test2.artemis.cit.tum.de
helios-aet
bot
temporarily deployed
to
artemis-test4.artemis.cit.tum.de
I also tried with artemis-test4, the accounts cannot login. |
helios-aet
bot
temporarily deployed
to
artemis-test1.artemis.cit.tum.de
Cannot deploy on TS1 either |
helios-aet
bot
temporarily deployed
to
artemis-test3.artemis.cit.tum.de
helios-aet
bot
temporarily deployed
to
artemis-test3.artemis.cit.tum.de
helios-aet
bot
temporarily deployed
to
artemis-test1.artemis.cit.tum.de
helios-aet
bot
temporarily deployed
to
artemis-test2.artemis.cit.tum.de
|
Can confirm issue - deployment itself works but there is always an issue (can't login, Maintenance, ...) |
|
Closing in favor of #11211 |
Checklist
General
Server
Client
Motivation and Context
Previously, the client-side blocked instructors from enabling Athena-powered features in unauthorized courses for modeling exercises. However, this restriction was not enforced on the server. This pull request adds server-side validation to ensure Athena modules are only used in authorized contexts.
Description
This pull request introduces server-side validation for Athena module usage in modeling exercises and includes a minor client-side refactoring.
Server-side Changes:
In ModelingExerciseResource.java, we now use the AthenaApi to enforce access control for Athena-related features during exercise creation, updates, and imports.
Create:
When creating a new modeling exercise, the server now verifies if the course has access to the selected Athena modules. If access is not granted, the corresponding feature (e.g., feedbackSuggestionModule) is disabled for the exercise.
Update:
Similar server-side checks are performed when an exercise is updated. This also prevents changing the Athena module configuration after the exercise's due date has passed.
Import:
When importing a modeling exercise, the same access checks are applied to the destination course. If the course lacks permissions, Athena-related features are disabled on the imported exercise, ensuring a valid configuration.
Client-side Changes:
In exercise-preliminary-feedback-options.component.ts, constructor-based dependency injection has been replaced with inject()
Steps for Testing
Prerequisites:
Testserver States
You can manage test servers using Helios. Check environment statuses in the environment list. To deploy to a test server, go to the CI/CD page, find your PR or branch, and trigger the deployment.
Review Progress
Performance Review
Code Review
Manual Tests
Test Coverage
Screenshots
Summary by CodeRabbit
New Features
Refactor