Programming exercises: Allow to choose preliminary feedback model
#10441
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
github-actions
bot
added
tests
server
github-actions
bot
added
client
github-actions
bot
added
exercise
dmytropolityka
changed the title
Programming exercises: Allow to choose preliminary feedback model
WalkthroughThis change introduces a new enum, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Client
participant FeedbackService
participant ModuleService
participant AthenaAPI
User->>Client: Request feedback suggestions
Client->>FeedbackService: getFeedbackSuggestions(exercise, isPreliminary)
FeedbackService->>ModuleService: getAthenaModuleUrl(exerciseType, module)
ModuleService->>AthenaAPI: Send feedback request (with proper module)
AthenaAPI-->>ModuleService: Return feedback data
ModuleService-->>FeedbackService: Return processed data
FeedbackService-->>Client: Deliver feedback suggestions
Possibly related PRs
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)`src/main/java/**/*.java`: naming:CamelCase; principles:{sin...
⏰ Context from checks skipped due to timeout of 90000ms (8)
🔇 Additional comments (1)
✨ Finishing Touches
🪧 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: 3
🔭 Outside diff range comments (1)
src/test/java/de/tum/cit/aet/artemis/athena/AthenaResourceIntegrationTest.java (1)
259-287: 🛠️ Refactor suggestionAdd tests for preliminary feedback.
The current tests only verify behavior with
falsefor the preliminary feedback parameter. Consider adding tests that usetrueto verify that preliminary feedback functionality also works correctly.Add tests similar to the existing ones but with the preliminary feedback parameter set to
true:@Test @WithMockUser(username = TEST_PREFIX + "tutor1", roles = "TA") void testGetPreliminaryFeedbackSuccessText() throws Exception { // Enable Athena preliminary feedback for the exercise textExercise.setPreliminaryFeedbackModule(ATHENA_MODULE_TEXT_TEST); textExerciseRepository.save(textExercise); athenaRequestMockProvider.mockGetFeedbackSuggestionsAndExpect("text", true); List<Feedback> response = request.getList("/api/athena/text-exercises/" + textExercise.getId() + "/submissions/" + textSubmission.getId() + "/preliminary-feedback", HttpStatus.OK, Feedback.class); assertThat(response).as("response is not empty").isNotEmpty(); }
🧹 Nitpick comments (12)
src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise-update.component.html (1)
217-220: Well-structured UI addition for preliminary feedback options.The new feedback section is appropriately positioned with a proper heading and translation key. The conditional rendering based on
isExamModecorrectly aligns with the backend behavior where preliminary feedback is disabled for exam exercises.One minor suggestion would be to consider adding a help icon with tooltip explanation similar to other sections in the form.
<h3 jhiTranslate="artemisApp.assessment.feedback" id="artemisApp.exercise.sections.feedback"></h3> +<jhi-help-icon text="artemisApp.exercise.feedbackOptionsTooltip" /> @if (!isExamMode) { <jhi-exercise-preliminary-feedback-options [exercise]="textExercise" [dueDate]="textExercise.dueDate" /> }src/main/webapp/app/exercises/modeling/manage/modeling-exercise-update.component.ts (1)
339-339: Fixed typo in comment.Corrected "digram" to "diagram" in the comment.
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaRepositoryExportService.java (1)
68-68: Updated condition to use preliminary feedback check.The condition now checks for
exercise.isPreliminaryFeedbackEnabled()instead of the previousexercise.getAllowFeedbackRequests(). This correctly implements the new approach to handle preliminary feedback as a separate concept from general feedback requests.This change aligns with the PR's objective to distinguish between different types of feedback modules. The method name
checkFeedbackSuggestionsOrAutomaticFeedbackEnabledElseThrowshould ideally be updated to reflect this change in logic from "automatic feedback" to "preliminary feedback".-private void checkFeedbackSuggestionsOrAutomaticFeedbackEnabledElseThrow(Exercise exercise) { +private void checkFeedbackSuggestionsOrPreliminaryFeedbackEnabledElseThrow(Exercise exercise) {src/main/webapp/app/overview/exercise-details/exercise-details-student-actions.component.html (1)
146-146: Improved readability with blank lineThe addition of a blank line improves code readability by separating logical blocks.
src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.html (2)
7-8: Update name attribute to match property name.The input uses
name="allowFeedbackRequests"but the model property is nowallowManualFeedbackRequests. Consider updating for consistency.-name="allowFeedbackRequests" +name="preliminaryFeedback"
18-18: Add i18n support for the dropdown label.The dropdown label "Athena Preliminary Feedback Module" should use the jhiTranslate directive for internationalization consistency.
-<label for="athenaModuleSelect">Athena Preliminary Feedback Module</label> +<label for="athenaModuleSelect" jhiTranslate="artemisApp.exercise.athenaPreliminaryFeedbackModule"></label>src/main/webapp/i18n/de/exercise.json (1)
178-179: Add translation for module selection dropdown.Missing translation key for the "Athena Preliminary Feedback Module" label that should be added to support the dropdown in the new component.
Add a new entry after line 179:
"allowFeedbackRequestsTooltip": "Studierende können vor dem Abgabetermin Feedback anfordern. Die Anfragen werden von Athena bearbeitet.", +"athenaPreliminaryFeedbackModule": "Athena-Modul für vorläufiges Feedback",src/test/javascript/spec/component/programming-exercise/programming-exercise-lifecycle.component.spec.ts (1)
394-427: Add test for exam mode with preliminary feedback.For completeness, consider adding a test that specifically verifies the preliminary feedback component is not rendered when in exam mode, similar to how the feedback suggestions component is tested.
it('should not render ExercisePreliminaryFeedbackOptionsComponent in exam mode', fakeAsync(() => { comp.exercise = exercise; fixture.componentRef.setInput('isEditFieldDisplayedRecord', { preliminaryFeedbackRequests: true, }); comp.isExamMode = true; jest.spyOn(athenaService, 'isEnabled').mockReturnValue(of(true)); tick(); fixture.detectChanges(); const preliminaryFeedbackElement = fixture.debugElement.nativeElement.querySelector('jhi-exercise-preliminary-feedback-options'); expect(preliminaryFeedbackElement).toBeFalsy(); }));src/test/javascript/spec/component/overview/exercise-details/exercise-details-student-actions.component.spec.ts (1)
695-714: Test descriptions should be unique and accurately reflect the test purposeThe test has exactly the same description as the previous test, even though this one tests modeling exercises rather than text exercises.
Consider updating the test description to clearly indicate it's testing modeling exercises:
-it('should display the request feedback button for modeling exercises when Athena is enabled and before due date', () => { +it('should display the request feedback button for MODELING exercises when Athena is enabled and before due date', () => {src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.ts (1)
85-87: Consider using the inputdueDateproperty instead of relying on the exercise's dueDateThe component accepts a
dueDateinput parameter, but thehasDueDatePassed()method usesthis.exercise.dueDateinstead. This could lead to inconsistencies if the inputdueDatediffers from the exercise's due date.- return dayjs(this.exercise.dueDate).isBefore(dayjs()); + const dateToCheck = this.dueDate || this.exercise.dueDate; + return dayjs(dateToCheck).isBefore(dayjs());src/main/java/de/tum/cit/aet/artemis/athena/web/AthenaResource.java (1)
114-114: Parameter name changed to better reflect its purpose.The parameter name has been changed from
isGradedtoisPreliminaryto align with the new feature allowing separate preliminary feedback modules.However, I noticed a discrepancy between the Javadoc comment (lines 111-113), which still references "graded" feedback provider, and the updated method signature. Consider updating the documentation to reflect this change.
/** - * Method to apply the (graded) feedback provider. Examples: AthenaFeedbackSuggestionsService::getTextFeedbackSuggestions, + * Method to apply the feedback provider (preliminary or graded). Examples: AthenaFeedbackSuggestionsService::getTextFeedbackSuggestions, * AthenaFeedbackSuggestionsService::getProgrammingFeedbackSuggestions */src/test/java/de/tum/cit/aet/artemis/exercise/participation/ParticipationIntegrationTest.java (1)
798-799: Use constant instead of string literal for module name.The test uses a string literal ("module_modeling_preliminary_test") instead of the constant ATHENA_MODULE_MODELING_PRELIMINARY_TEST that is used in other tests. This is inconsistent with the pattern used elsewhere in the class.
- this.modelingExercise.setPreliminaryFeedbackModule("module_modeling_preliminary_test"); + this.modelingExercise.setPreliminaryFeedbackModule(ATHENA_MODULE_MODELING_PRELIMINARY_TEST);Also applies to: 801-801
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/main/resources/config/liquibase/changelog/20250304004831_changelog.xmlis excluded by!**/*.xmlsrc/main/resources/config/liquibase/master.xmlis excluded by!**/*.xml
📒 Files selected for processing (60)
src/main/java/de/tum/cit/aet/artemis/athena/domain/ModuleType.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSendingService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaModuleService.java(3 hunks)src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaRepositoryExportService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaSubmissionSelectionService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaSubmissionSendingService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/athena/web/AthenaResource.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/core/config/Constants.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exercise/domain/Exercise.java(4 hunks)src/main/java/de/tum/cit/aet/artemis/exercise/service/SubmissionService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/modeling/service/ModelingExerciseFeedbackService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/domain/ProgrammingExercise.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingAssessmentService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseCodeReviewFeedbackService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseExportImportResource.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java(3 hunks)src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingSubmissionResource.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/text/service/TextExerciseFeedbackService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.java(4 hunks)src/main/webapp/app/assessment/assessment-warning/assessment-warning.component.ts(1 hunks)src/main/webapp/app/entities/exercise.model.ts(4 hunks)src/main/webapp/app/exercises/modeling/manage/modeling-exercise-detail.component.ts(1 hunks)src/main/webapp/app/exercises/modeling/manage/modeling-exercise-update.component.html(1 hunks)src/main/webapp/app/exercises/modeling/manage/modeling-exercise-update.component.ts(3 hunks)src/main/webapp/app/exercises/modeling/participate/modeling-submission.component.html(1 hunks)src/main/webapp/app/exercises/programming/manage/programming-exercise-detail.component.ts(1 hunks)src/main/webapp/app/exercises/programming/manage/update/programming-exercise-update.helper.ts(2 hunks)src/main/webapp/app/exercises/programming/shared/lifecycle/programming-exercise-lifecycle.component.html(3 hunks)src/main/webapp/app/exercises/programming/shared/lifecycle/programming-exercise-lifecycle.component.ts(4 hunks)src/main/webapp/app/exercises/shared/dashboards/tutor/exercise-assessment-dashboard.component.ts(1 hunks)src/main/webapp/app/exercises/shared/feedback-suggestion/exercise-feedback-suggestion-options.component.html(2 hunks)src/main/webapp/app/exercises/shared/feedback-suggestion/exercise-feedback-suggestion-options.component.ts(5 hunks)src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.html(1 hunks)src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.ts(1 hunks)src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise-detail.component.ts(1 hunks)src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise-update.component.html(1 hunks)src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise-update.component.ts(2 hunks)src/main/webapp/app/exercises/text/participate/text-editor.component.html(1 hunks)src/main/webapp/app/overview/exercise-details/exercise-details-student-actions.component.html(2 hunks)src/main/webapp/app/overview/exercise-details/request-feedback-button/request-feedback-button.component.ts(1 hunks)src/main/webapp/i18n/de/exercise.json(1 hunks)src/main/webapp/i18n/de/programmingExercise.json(0 hunks)src/main/webapp/i18n/de/textExercise.json(1 hunks)src/main/webapp/i18n/en/exercise.json(1 hunks)src/main/webapp/i18n/en/programmingExercise.json(0 hunks)src/main/webapp/i18n/en/textExercise.json(1 hunks)src/test/java/de/tum/cit/aet/artemis/athena/AthenaResourceIntegrationTest.java(6 hunks)src/test/java/de/tum/cit/aet/artemis/athena/service/connectors/AthenaFeedbackSuggestionsServiceTest.java(6 hunks)src/test/java/de/tum/cit/aet/artemis/core/connector/AthenaRequestMockProvider.java(4 hunks)src/test/java/de/tum/cit/aet/artemis/exercise/participation/ParticipationIntegrationTest.java(11 hunks)src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingAssessmentIntegrationTest.java(1 hunks)src/test/javascript/spec/component/assessment-dashboard/exercise-assessment-dashboard.component.spec.ts(1 hunks)src/test/javascript/spec/component/exercises/shared/feedback/feedback-suggestion-option.component.spec.ts(3 hunks)src/test/javascript/spec/component/exercises/shared/feedback/preliminary-feedback-options.component.spec.ts(1 hunks)src/test/javascript/spec/component/overview/exercise-details/exercise-details-student-actions.component.spec.ts(12 hunks)src/test/javascript/spec/component/overview/exercise-details/request-feedback-button/request-feedback-button.component.spec.ts(2 hunks)src/test/javascript/spec/component/programming-exercise/programming-exercise-lifecycle.component.spec.ts(5 hunks)src/test/javascript/spec/component/programming-exercise/programming-exercise-update.component.spec.ts(2 hunks)
💤 Files with no reviewable changes (2)
- src/main/webapp/i18n/en/programmingExercise.json
- src/main/webapp/i18n/de/programmingExercise.json
✅ Files skipped from review due to trivial changes (1)
- src/main/java/de/tum/cit/aet/artemis/athena/domain/ModuleType.java
🧰 Additional context used
📓 Path-based instructions (6)
`src/main/java/**/*.java`: naming:CamelCase; principles:{sin...
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/ProgrammingAssessmentService.javasrc/main/java/de/tum/cit/aet/artemis/athena/service/AthenaSubmissionSelectionService.javasrc/main/java/de/tum/cit/aet/artemis/modeling/service/ModelingExerciseFeedbackService.javasrc/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingSubmissionResource.javasrc/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSendingService.javasrc/main/java/de/tum/cit/aet/artemis/athena/service/AthenaRepositoryExportService.javasrc/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseCodeReviewFeedbackService.javasrc/main/java/de/tum/cit/aet/artemis/text/service/TextExerciseFeedbackService.javasrc/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportService.javasrc/main/java/de/tum/cit/aet/artemis/core/config/Constants.javasrc/main/java/de/tum/cit/aet/artemis/athena/service/AthenaSubmissionSendingService.javasrc/main/java/de/tum/cit/aet/artemis/programming/domain/ProgrammingExercise.javasrc/main/java/de/tum/cit/aet/artemis/exercise/service/SubmissionService.javasrc/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.javasrc/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.javasrc/main/java/de/tum/cit/aet/artemis/athena/service/AthenaModuleService.javasrc/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseExportImportResource.javasrc/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.javasrc/main/java/de/tum/cit/aet/artemis/athena/web/AthenaResource.javasrc/main/java/de/tum/cit/aet/artemis/exercise/domain/Exercise.java
`src/main/webapp/**/*.ts`: angular_style:https://angular.io/...
src/main/webapp/app/assessment/assessment-warning/assessment-warning.component.tssrc/main/webapp/app/exercises/modeling/manage/modeling-exercise-detail.component.tssrc/main/webapp/app/exercises/shared/dashboards/tutor/exercise-assessment-dashboard.component.tssrc/main/webapp/app/exercises/text/manage/text-exercise/text-exercise-update.component.tssrc/main/webapp/app/overview/exercise-details/request-feedback-button/request-feedback-button.component.tssrc/main/webapp/app/exercises/programming/manage/update/programming-exercise-update.helper.tssrc/main/webapp/app/exercises/modeling/manage/modeling-exercise-update.component.tssrc/main/webapp/app/exercises/programming/manage/programming-exercise-detail.component.tssrc/main/webapp/app/exercises/programming/shared/lifecycle/programming-exercise-lifecycle.component.tssrc/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.tssrc/main/webapp/app/exercises/text/manage/text-exercise/text-exercise-detail.component.tssrc/main/webapp/app/exercises/shared/feedback-suggestion/exercise-feedback-suggestion-options.component.tssrc/main/webapp/app/entities/exercise.model.ts
`src/main/webapp/**/*.html`: @if and @for are new and valid ...
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/text/participate/text-editor.component.htmlsrc/main/webapp/app/exercises/text/manage/text-exercise/text-exercise-update.component.htmlsrc/main/webapp/app/exercises/modeling/participate/modeling-submission.component.htmlsrc/main/webapp/app/exercises/shared/feedback-suggestion/exercise-feedback-suggestion-options.component.htmlsrc/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.htmlsrc/main/webapp/app/exercises/programming/shared/lifecycle/programming-exercise-lifecycle.component.htmlsrc/main/webapp/app/overview/exercise-details/exercise-details-student-actions.component.htmlsrc/main/webapp/app/exercises/modeling/manage/modeling-exercise-update.component.html
`src/test/javascript/spec/**/*.ts`: jest: true; mock: NgMock...
src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/test/javascript/spec/component/assessment-dashboard/exercise-assessment-dashboard.component.spec.tssrc/test/javascript/spec/component/programming-exercise/programming-exercise-lifecycle.component.spec.tssrc/test/javascript/spec/component/exercises/shared/feedback/preliminary-feedback-options.component.spec.tssrc/test/javascript/spec/component/overview/exercise-details/request-feedback-button/request-feedback-button.component.spec.tssrc/test/javascript/spec/component/exercises/shared/feedback/feedback-suggestion-option.component.spec.tssrc/test/javascript/spec/component/programming-exercise/programming-exercise-update.component.spec.tssrc/test/javascript/spec/component/overview/exercise-details/exercise-details-student-actions.component.spec.ts
`src/main/webapp/i18n/de/**/*.json`: German language transla...
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".
src/main/webapp/i18n/de/exercise.jsonsrc/main/webapp/i18n/de/textExercise.json
`src/test/java/**/*.java`: test_naming: descriptive; test_si...
src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true
src/test/java/de/tum/cit/aet/artemis/athena/AthenaResourceIntegrationTest.javasrc/test/java/de/tum/cit/aet/artemis/programming/ProgrammingAssessmentIntegrationTest.javasrc/test/java/de/tum/cit/aet/artemis/athena/service/connectors/AthenaFeedbackSuggestionsServiceTest.javasrc/test/java/de/tum/cit/aet/artemis/core/connector/AthenaRequestMockProvider.javasrc/test/java/de/tum/cit/aet/artemis/exercise/participation/ParticipationIntegrationTest.java
🧠 Learnings (4)
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingAssessmentService.java (1)
Learnt from: undernagruzez
PR: ls1intum/Artemis#8498
File: src/main/java/de/tum/in/www1/artemis/service/programming/ProgrammingExerciseCodeReviewFeedbackService.java:236-236
Timestamp: 2024-11-12T12:51:40.391Z
Learning: Rate limit exceptions for AI feedback requests are handled in the client app.
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseCodeReviewFeedbackService.java (1)
Learnt from: undernagruzez
PR: ls1intum/Artemis#8498
File: src/main/java/de/tum/in/www1/artemis/service/programming/ProgrammingExerciseCodeReviewFeedbackService.java:236-236
Timestamp: 2024-11-12T12:51:40.391Z
Learning: Rate limit exceptions for AI feedback requests are handled in the client app.
src/main/webapp/app/exercises/programming/shared/lifecycle/programming-exercise-lifecycle.component.ts (1)
Learnt from: florian-glombik
PR: ls1intum/Artemis#8451
File: src/main/webapp/app/exercises/programming/shared/lifecycle/programming-exercise-lifecycle.component.html:187-187
Timestamp: 2024-11-12T12:51:58.050Z
Learning: Use `artemisTranslate` for handling translations in Angular templates within the Artemis platform, as per the project-specific requirements.
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java (1)
Learnt from: Hialus
PR: ls1intum/Artemis#8607
File: src/main/java/de/tum/in/www1/artemis/web/rest/programming/ProgrammingExerciseResource.java:64-64
Timestamp: 2024-11-12T12:51:51.201Z
Learning: For the Artemis project, import statements are automatically managed by formatters and should not be commented on.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (137)
src/main/java/de/tum/cit/aet/artemis/text/service/TextExerciseFeedbackService.java (1)
120-120:✅ Verification successful
Parameter updated to support new preliminary feedback mode
This change correctly updates the parameter from
falsetotrueto indicate that this is now preliminary feedback rather than graded feedback, aligning with the PR's objective of allowing separate modules for preliminary feedback requests.
🏁 Script executed:
#!/bin/bash # Let's verify the Athena feedback service interface to confirm the parameter meaning has changed # Check the interface definition of the getTextFeedbackSuggestions method rg -A 5 "getTextFeedbackSuggestions.*?\(" src/main/java/de/tum/cit/aet/artemis/athena/service/Length of output: 1403
Verified Parameter Update for Preliminary Feedback Mode
The code update correctly switches the parameter to
true, which aligns with the Athena service's expectations. The interface now interpretstrueas triggering a "Non Graded" (preliminary) feedback mode—as confirmed by the debug log message in the Athena service interface—ensuring that preliminary feedback is handled as intended.src/main/java/de/tum/cit/aet/artemis/modeling/service/ModelingExerciseFeedbackService.java (1)
162-162:✅ Verification successful
Parameter updated to use preliminary feedback mode
The parameter was correctly updated from
falsetotrueto indicate that this is now preliminary feedback rather than graded feedback. This change is consistent with similar updates in other service classes and aligns with the PR's goal of distinguishing between preliminary and manual feedback.
🏁 Script executed:
#!/bin/bash # Verify that this change is consistent with other similar service implementations # Check all feedback suggestion method calls across the codebase rg -A 3 "get.*?FeedbackSuggestions.*?\(.+,.+,\s*true" src/main/java/de/tum/cit/aet/artemis/Length of output: 1322
Preliminary feedback parameter update verified
The change from
falsetotruein the call togetModelingFeedbackSuggestionsinModelingExerciseFeedbackService.javais consistent with similar implementations (e.g., inTextExerciseFeedbackService.java). This verifies that the parameter correctly indicates preliminary feedback as intended.src/main/webapp/app/assessment/assessment-warning/assessment-warning.component.ts (1)
50-50:✅ Verification successful
Updated condition to check allowManualFeedbackRequests
The change correctly updates the condition to check for manual feedback requests specifically, rather than general feedback requests. This aligns with the new separation between preliminary and manual feedback introduced in this PR.
🏁 Script executed:
#!/bin/bash # Verify that the Exercise model has been updated to include the new property # Check the Exercise model definition rg "allowManualFeedbackRequests" src/main/webapp/app/entities/exercise.model.tsLength of output: 247
Verification Passed: Manual Feedback Request Check Validated
- Verified that the
Exercisemodel includes theallowManualFeedbackRequestsproperty (defaulting tofalse) insrc/main/webapp/app/entities/exercise.model.ts.- Confirmed that the condition in
assessment-warning.component.tsnow correctly utilizes this property:this.showWarning = now.isBefore(this.getLatestDueDate()) && !this.exercise.allowManualFeedbackRequests;- The update aligns with the separation between preliminary and manual feedback requests as intended.
src/main/webapp/app/exercises/text/participate/text-editor.component.html (1)
27-27:✅ Verification successful
Updated conditional to use allowManualFeedbackRequests
The change correctly updates the template to check for
allowManualFeedbackRequestspermission instead of the generalallowFeedbackRequests. This ensures the request feedback button is displayed only when manual feedback is permitted, aligned with the new separation between preliminary and manual feedback.
🏁 Script executed:
#!/bin/bash # Verify that similar changes have been made in other components for consistency # Check other template files for similar updates rg "allowManualFeedbackRequests" --glob="*.html" src/main/webapp/app/Length of output: 1347
Consistency Verified: Manual Feedback Conditional Correctly Updated
The change in
src/main/webapp/app/exercises/text/participate/text-editor.component.htmlnow properly checks forallowManualFeedbackRequests, ensuring that the request feedback button appears only when manual feedback is permitted. Verification in similar components (e.g., in the programming exercise lifecycle component) confirms consistent application of this update across the codebase.src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportService.java (1)
295-297: Consistent handling of feedback modules for exam exercises.The new line correctly sets
preliminaryFeedbackModuleto null for exam exercises, following the same pattern as with feedback suggestion modules. The added comment clearly explains that this applies to preliminary feedback as well.src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise-update.component.ts (2)
48-48: Good import of the new preliminary feedback component.The import follows the established pattern and Angular naming conventions.
71-71: Properly registered the component in the imports array.The component is correctly added to the imports array, which is required for Angular standalone components.
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSendingService.java (2)
98-99: Useful clarification comment about feedback scope.The added comment improves code readability by explicitly stating that this method currently only applies to feedback suggestions based on the current design.
101-102: Improved URL construction for Athena service.The URL construction has been properly refactored to use
athenaModuleService.getAthenaModuleUrl()which provides a more modular approach to handling different types of feedback modules. This implementation aligns with the PR objective of distinguishing between feedback suggestion and preliminary feedback modules.src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseCodeReviewFeedbackService.java (1)
135-136: Updated parameter to enable preliminary feedbackThe parameter for
getProgrammingFeedbackSuggestionswas changed fromfalsetotrue, reflecting the new distinction between manual and preliminary feedback as outlined in the PR objectives. This aligns with the broader change of introducing separate modules for preliminary feedback and graded feedback.src/test/javascript/spec/component/assessment-dashboard/exercise-assessment-dashboard.component.spec.ts (1)
595-595: Property name updated to reflect more specific functionalityThe property has been renamed from
allowFeedbackRequeststoallowManualFeedbackRequeststo better specify the type of feedback being configured. This is part of the broader change to distinguish between manual and preliminary feedback requests.src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaSubmissionSelectionService.java (1)
86-88: Enhanced URL construction for Athena moduleThe implementation now uses specific methods (
getExerciseType()andgetFeedbackSuggestionModule()) to construct the Athena module URL, rather than relying on the exercise object directly. This refactoring supports the new functionality of configuring different modules for different types of feedback.The added comment "applies only to feedback suggestions" provides clarity about the scope of this implementation.
src/main/java/de/tum/cit/aet/artemis/core/config/Constants.java (1)
73-73: API path structure updated for consistencyThe order of segments in the API path has been changed from
/api/public/athena/programming-exercises/to/api/athena/public/programming-exercises/, which provides a more consistent and logical organization of the API endpoints. This change aligns with the separation of feedback module types introduced in this PR.src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingAssessmentService.java (1)
153-154: Property rename aligned with feedback separation logic.The change from
getAllowFeedbackRequests()togetAllowManualFeedbackRequests()correctly implements the distinction between manual and preliminary feedback requests as per the PR objectives.src/main/webapp/app/exercises/shared/dashboards/tutor/exercise-assessment-dashboard.component.ts (1)
387-387: Property rename aligned with feedback separation logic.The condition has been updated from using the deprecated
allowFeedbackRequeststo the more specificallowManualFeedbackRequestsproperty, which aligns with the new distinction between manual and preliminary feedback requests.src/main/webapp/app/exercises/modeling/manage/modeling-exercise-detail.component.ts (1)
143-144: UI display for feedback module settings.These additions appropriately display whether feedback suggestions and preliminary feedback are enabled based on the presence of their respective modules. This enhances the UI with the new feedback configuration capabilities.
The use of
!!to convert the module presence to a boolean value is a good practice. The display titles use the correct translation keys for consistency across the application.src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingAssessmentIntegrationTest.java (1)
1013-1013: Test method updated to use the renamed property setter.The test method has been correctly updated to use
setAllowManualFeedbackRequests(true)instead ofsetAllowFeedbackRequests(true), maintaining consistency with the domain model changes.src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingSubmissionResource.java (1)
387-387:✅ Verification successful
Condition updated to check for manual feedback requests
The change from
getAllowFeedbackRequests()togetAllowManualFeedbackRequests()correctly aligns with the PR objective of enabling separate configuration for preliminary and manual feedback requests.
🏁 Script executed:
#!/bin/bash # Verify the consistent implementation of the renamed method across the codebase # Search for other occurrences of allowManualFeedbackRequests to ensure consistency rg "getAllowManualFeedbackRequests" --type javaLength of output: 1122
Update Verified: Consistent Manual Feedback Request Method
The change from
getAllowFeedbackRequests()togetAllowManualFeedbackRequests()in the condition is correctly applied. Our search confirms that this method is used uniformly across the codebase (e.g., inProgrammingExercise.java,Exercise.java, andSubmissionService.java). The updated condition now properly supports the PR objective of enabling a separate configuration for manual feedback requests.src/main/webapp/app/exercises/modeling/participate/modeling-submission.component.html (1)
27-29:✅ Verification successful
Updated conditional to use preliminaryFeedbackModule
Good change that properly implements the PR objective. The button's visibility now depends on the availability of the preliminary feedback module rather than the generic feedback request setting.
🏁 Script executed:
#!/bin/bash # Check for consistent usage of the preliminaryFeedbackModule across other components # Search for other occurrences in HTML files rg "preliminaryFeedbackModule" --type html -A 2 -B 2Length of output: 3505
Conditional update for preliminaryFeedbackModule verified.
The usage of
preliminaryFeedbackModulein the modeling submission component is consistent with its usage in shared preliminary feedback components. The conditional now properly depends on the availability of the preliminary feedback module rather than the generic feedback request setting, aligning with the PR objective.
- Confirmed that the updated conditional in
src/main/webapp/app/exercises/modeling/participate/modeling-submission.component.htmlis consistent with other parts of the codebase.src/main/webapp/app/overview/exercise-details/request-feedback-button/request-feedback-button.component.ts (1)
69-73:✅ Verification successful
Feature-aware feedback request enablement logic
The component now correctly differentiates between Athena-based preliminary feedback and manual feedback requests based on the active profile, which aligns perfectly with the PR objectives.
🏁 Script executed:
#!/bin/bash # Check if the preliminaryFeedbackModule property is defined in the Exercise model # Search for the preliminaryFeedbackModule property in the Exercise model rg "preliminaryFeedbackModule" --type ts -A 2 -B 2 src/main/webapp/app/entities/exercise.model.tsLength of output: 501
Code logic verified: Feedback request toggle behaves as intended
The shell script output confirms that the
preliminaryFeedbackModuleproperty is correctly defined in the Exercise model. The condition leveragingthis.athenaEnabledproperly checks for the existence of this property, while the fallback toallowManualFeedbackRequestsis also valid. No issues were found with the current implementation.
- Verified that
preliminaryFeedbackModuleexists insrc/main/webapp/app/entities/exercise.model.ts- The conditional logic in
src/main/webapp/app/overview/exercise-details/request-feedback-button/request-feedback-button.component.tsaligns with the intended feature separation based on the active profilesrc/main/webapp/i18n/en/textExercise.json (1)
48-48:✅ Verification successful
Removed translation keys for preliminary Athena feedback
Based on the AI summary, the keys for "allowPreliminaryAthenaFeedbackRequests" and its tooltip have been removed. This is consistent with the PR's approach of separating preliminary and manual feedback options.
🏁 Script executed:
#!/bin/bash # Check if the same keys were removed from other locale files # Check German locale file echo "Checking German locale file:" rg -A 2 -B 2 "allowPreliminaryAthenaFeedbackRequests" --type json src/main/webapp/i18n/de/textExercise.json || echo "Not found in German locale file" # Check if new keys were added for the new functionality echo "Checking for new keys related to preliminary feedback:" rg "preliminary.*feedback" --type json src/main/webapp/i18n/Length of output: 553
Verification Successful: Translation Keys Removal Confirmed
- The key
"allowPreliminaryAthenaFeedbackRequests"and its tooltip have been removed as expected in the EnglishtextExercise.json.- The same key is absent in the German locale file (
src/main/webapp/i18n/de/textExercise.json).- New keys (e.g.,
"allowFeedbackRequests") appear in the English exercise file, aligning with the PR's approach to separate preliminary and manual feedback options.src/main/webapp/app/exercises/modeling/manage/modeling-exercise-update.component.ts (2)
50-50: Import addition for preliminary feedback support.The addition of
ExercisePreliminaryFeedbackOptionsComponentaligns with the PR objective to allow configuring separate modules for preliminary feedback in exercises.
74-74: Component integration for preliminary feedback configuration.Good addition of
ExercisePreliminaryFeedbackOptionsComponentto the imports array. This enables users to configure preliminary feedback options separately from graded feedback, which meets the PR objective.src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaSubmissionSendingService.java (1)
130-131:✅ Verification successful
Enhanced Athena module URL construction with explicit module type.
The URL construction now properly specifies both the exercise type and the feedback suggestion module. This change is necessary to support the PR's objective of configuring separate modules for different types of feedback.
The added comment clearly indicates that this applies specifically to feedback suggestions.
I recommend verifying that the
getFeedbackSuggestionModule()method returns the correct module based on whether the request is for preliminary or graded feedback.
🏁 Script executed:
#!/bin/bash # Verify how getFeedbackSuggestionModule is implemented and called # Search for getFeedbackSuggestionModule method implementation rg -t java "getFeedbackSuggestionModule\(" --no-heading -A 5 -B 5Length of output: 17664
Enhanced Athena Module URL Construction Verified
The URL construction in
AthenaSubmissionSendingService.javanow explicitly uses the exercise type and the feedback suggestion module. Our verification confirms that:
- The
getFeedbackSuggestionModule()method (found inExercise.java) properly returns the configured module value.- In services handling feedback suggestions, the module is derived either directly (for graded feedback) or conditionally (for preliminary feedback, as seen in
AthenaFeedbackSuggestionsService.java), which aligns with the intended design.- Integration tests in
AthenaExerciseIntegrationTest.javafurther confirm that the module remains consistent for exercises with unchanged configurations and is correctly unset when access is revoked.No further changes are needed regarding this functionality.
src/main/webapp/app/exercises/programming/manage/update/programming-exercise-update.helper.ts (2)
43-44: Added new enum values for feedback configuration.The addition of
FEEDBACK_SUGGESTIONSandPRELIMINARY_FEEDBACK_REQUESTSto theProgrammingExerciseInputFieldenum properly extends the UI to support configuring both types of feedback separately.
93-94: Configuration for feedback options display in UI.Setting both
feedbackSuggestionsandpreliminaryFeedbackRequeststofalsein theIS_DISPLAYED_IN_SIMPLE_MODEmap ensures these options are only displayed in advanced mode, which is appropriate for these specialized configuration options.The comment "// Feedback section" helps organize the code by grouping related feedback options.
Also applies to: 100-101
src/main/webapp/i18n/de/textExercise.json (1)
48-48: Removed translation entries need to match English versionThe entries "allowPreliminaryAthenaFeedbackRequests" and "allowPreliminaryAthenaFeedbackRequestsTooltip" have been removed. This correctly matches the changes made to the English localization file.
src/main/webapp/app/overview/exercise-details/exercise-details-student-actions.component.html (2)
141-143: Refactored condition for programming exercises feedback request buttonThe condition has been simplified to only check for the existence of
gradedParticipationand the exercise type, removing the dependency onallowFeedbackRequests. This change aligns with the PR objectives to differentiate between preliminary and manual feedback modules.
236-244: Refactored condition for text/modeling exercises feedback request buttonThe condition has been updated to only check for Athena being enabled, the exercise type, and if it's before the due date. The dependency on
allowFeedbackRequestshas been removed, aligning with the PR objectives to differentiate between preliminary and manual feedback modules.src/main/java/de/tum/cit/aet/artemis/exercise/service/SubmissionService.java (1)
700-700:✅ Verification successful
Updated method parameter name for clarity and consistency
The condition now checks
!exercise.getAllowManualFeedbackRequests()instead of the previous check forgetAllowFeedbackRequests(), providing better specificity about what type of feedback requests are being verified.
🏁 Script executed:
#!/bin/bash # Check if getAllowManualFeedbackRequests is consistently used throughout the codebase echo "Searching for getAllowManualFeedbackRequests method usage..." rg "getAllowManualFeedbackRequests\(\)" --type java echo "Searching for getAllowFeedbackRequests method usage (should be removed or limited)..." rg "getAllowFeedbackRequests\(\)" --type javaLength of output: 1488
Feedback Parameter Name Update Verified
The updated condition using
!exercise.getAllowManualFeedbackRequests()has been verified across the codebase. Our search confirms that references togetAllowFeedbackRequests()no longer exist, ensuring consistency and clarity.
- Verified in
src/main/java/de/tum/cit/aet/artemis/exercise/service/SubmissionService.java(line 700) along with consistent usage in other files.src/main/webapp/app/exercises/modeling/manage/modeling-exercise-update.component.html (2)
271-271: Added clear section header for feedback configurationThe new header improves UI organization by clearly separating the feedback configuration section with proper translation support.
272-274:✅ Verification successful
Replaced feedback suggestions with preliminary feedback options
The component
<jhi-exercise-feedback-suggestion-options>has been replaced with<jhi-exercise-preliminary-feedback-options>, which aligns with the PR objective of allowing instructors to configure separate modules for preliminary feedback.This change maintains the same conditional logic for exam mode, ensuring that preliminary feedback options are only shown in non-exam contexts.
🏁 Script executed:
#!/bin/bash # Check if the ExercisePreliminaryFeedbackOptionsComponent is properly declared and used echo "Checking for ExercisePreliminaryFeedbackOptionsComponent declaration..." rg -A 5 "ExercisePreliminaryFeedbackOptionsComponent" --type typescript echo "Checking for jhi-exercise-preliminary-feedback-options usage in templates..." rg "jhi-exercise-preliminary-feedback-options" --type htmlLength of output: 1141
Preliminary Feedback Options Replacement Verified
The change from
<jhi-exercise-feedback-suggestion-options>to<jhi-exercise-preliminary-feedback-options>in the Modeling Exercise update component is correctly implemented. The shell search confirms that the new component is used consistently across different exercise update templates (modeling, text, and programming), and the conditional logic to display it only in non-exam scenarios remains intact.
- The updated component usage in
src/main/webapp/app/exercises/modeling/manage/modeling-exercise-update.component.htmlis properly applied.- Similar updates were successfully applied in related exercise update templates, ensuring consistency in the preliminary feedback configuration.
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseExportImportResource.java (3)
43-44: LGTM! The ModuleType import is necessary.The import addition for
ModuleTypeis appropriate for the new functionality being added.
246-247: Clear and accurate comment for both feedback functionality types.The comment has been updated to accurately reflect that both feedback suggestions and preliminary feedback functionality can be disabled based on Athena access rights.
255-261: Properly implemented preliminary feedback access check.The implementation of the preliminary feedback access check follows the same pattern as the feedback suggestions check. This ensures consistent behavior for both module types.
The try-catch block appropriately:
- Checks if the user has access to the preliminary feedback module
- Sets the module to null if either the service isn't present or access is denied
- Handles exceptions consistently with the existing pattern
src/main/webapp/app/entities/exercise.model.ts (4)
90-90: Property renamed for greater clarity.The property has been renamed from
allowFeedbackRequeststoallowManualFeedbackRequests, which better describes its actual purpose and distinguishes it from the new preliminary feedback functionality.
132-132: Added new property for selecting preliminary feedback module.The new
preliminaryFeedbackModuleproperty enables storing the selection of which Athena module to use for preliminary feedback.
163-163: Constructor initialization updated to match renamed property.The constructor has been correctly updated to initialize the renamed property
allowManualFeedbackRequestsinstead of the oldallowFeedbackRequests.
283-286: Reset function properly updated for all feedback-related properties.The
resetForImportfunction has been updated to set all feedback-related properties to their initial states:
allowManualFeedbackRequeststo falsefeedbackSuggestionModuleto undefinedpreliminaryFeedbackModuleto undefinedThis ensures a clean state when importing exercises.
src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.html (2)
1-15: Clear and well-structured UI for enabling preliminary feedback.The checkbox UI follows Angular best practices with:
- Proper conditional rendering based on Athena availability
- Appropriate bindings for checked state and disabled state
- Clear labeling with internationalization support
- Helpful tooltip inclusion
16-31: Well-implemented dropdown for module selection.The dropdown for selecting the Athena module:
src/main/webapp/i18n/de/exercise.json (2)
176-177: Clear German translation for feedback suggestions feature.The translation for
allowFeedbackSuggestionsand its tooltip is clear and appropriate for German users, using informal tone (du/dein) as required by the guidelines.
178-179: Clear German translation for feedback requests feature.The translation for
allowFeedbackRequestsand its tooltip clearly explains the preliminary feedback functionality in German, using informal tone (du/dein) as required.src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java (3)
44-44: Import for ModuleType added correctlyThe import for ModuleType has been correctly added to support the new differentiation between feedback suggestions and preliminary feedback.
249-253: Module access checks correctly implemented for both module typesThe code correctly implements separate access checks for both feedback module types:
- First check for
FEEDBACK_SUGGESTIONSmodule- Second check for new
PRELIMINARY_FEEDBACKmoduleThis ensures proper access control for each module type during exercise creation.
347-350: Module access checks consistently implemented in update methodThe update method correctly mirrors the same pattern as the create method, ensuring consistent access checks for both module types during exercise update.
src/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.java (4)
43-43: Import for ModuleType added correctlyThe import for ModuleType has been correctly added to support differentiation between feedback suggestions and preliminary feedback in text exercises as well.
229-232: Module access checks correctly implemented for text exercisesThe code correctly implements separate access checks for both feedback module types in the text exercise creation flow, mirroring the implementation in programming exercises.
286-289: Consistent module checks in text exercise update methodThe update method correctly applies the same access check pattern ensuring consistency across different exercise types and operations.
535-550: Error handling for module access checks in import flowThe implementation includes proper error handling for both module types when importing exercises. This ensures that even if a module is not available or accessible, the import process can continue with those modules disabled rather than failing completely.
The structure with separate try-catch blocks for each module type provides good isolation of failures.
src/main/webapp/app/exercises/shared/feedback-suggestion/exercise-feedback-suggestion-options.component.html (3)
8-8: Improved checkbox logic for feedback suggestionsThe checkbox now correctly represents the combined state of whether feedback suggestions are shown in the dropdown or directly enabled via the module.
17-19: Updated translation keys for better consistencyThe translation keys have been updated from "feedbackSuggestionsEnabled" to "allowFeedbackSuggestions" and a corresponding tooltip update, which better reflects the purpose of this setting and aligns with naming conventions used elsewhere.
21-23: Improved condition for showing module dropdownThe dropdown visibility condition now includes both the
showDropdownListflag and module availability check.The label has been updated to be more specific with "Athena Feedback Suggestions Module" which clearly indicates this is for feedback suggestions specifically, not preliminary feedback.
src/main/java/de/tum/cit/aet/artemis/programming/domain/ProgrammingExercise.java (2)
682-684: Updated check for manual feedback request permissionsThe method has been correctly updated to use
getAllowManualFeedbackRequests()instead of the previousgetAllowFeedbackRequests()to align with the new separation of feedback types.
796-798: Validation updated for manual feedback requestsThe validation method correctly uses the new
getAllowManualFeedbackRequests()method to check if manual feedback is enabled before validating related settings.src/main/webapp/app/exercises/shared/feedback-suggestion/exercise-feedback-suggestion-options.component.ts (6)
1-1: Implementation of AfterViewChecked lifecycle hook is appropriate for UI state management.Adding
AfterViewCheckedto the imports and implementing it in the component allows for proper UI state management after Angular's change detection completes.
15-15: Good utilization of Angular standalone component pattern.The component is correctly configured as standalone, which aligns with modern Angular practices and improves bundle size optimization.
19-19: Implementation of multiple lifecycle interfaces with proper ordering.The component now implements
OnInit,OnChanges, andAfterViewCheckedwhich is appropriate for the component's functionality and follows Angular's lifecycle sequence.
37-37: Boolean property is correctly initialized.The
showDropdownListproperty is properly initialized tofalsewhich ensures a defined initial state for the UI.
84-92: Improved toggle feedback flow and mutual exclusivity behavior.The
toggleFeedbackSuggestionsmethod now correctly:
- Updates the dropdown visibility state
- Ensures mutual exclusivity between feedback suggestions and manual feedback requests
- Properly resets state when toggled off
This implementation aligns with the PR goal of separating preliminary feedback from manual feedback.
57-61:✅ Verification successful
Be cautious with ngAfterViewChecked implementation.
The
ngAfterViewCheckedmethod is correctly implemented to maintain UI state consistency, but be aware that it runs after every change detection cycle, which could potentially impact performance in complex UIs.
🌐 Web query:
What are the performance considerations for ngAfterViewChecked in Angular?💡 Result:
The
ngAfterViewCheckedlifecycle hook in Angular is triggered after each change detection cycle completes for a component's view and child views. While useful for responding to view updates, it poses significant performance risks if misused. Here are key considerations and best practices:1. Frequent Execution
ngAfterViewCheckedruns after every change detection cycle, which can occur frequently due to user events, timers, or data updates[1][6][7].- Even minor inefficiencies here compound over time, degrading app performance[2][5].
- Example: A looped
setTimeout()inside this hook can trigger infinite change detection cycles[3].2. Heavy Computations
- Avoid computationally expensive tasks (e.g., complex DOM manipulation, data processing) in this hook. Angular executes lifecycle hooks sequentially, so slow operations delay the entire change detection process[1][6].
- Optimization:
- Memoization: Cache results of repeated calculations[1].
- Pure Pipes: Move heavy logic to pure pipes, which Angular reevaluates only when inputs change[1][5].
3. Change Detection Loops
- Modifying component state (e.g., updating properties) inside
ngAfterViewCheckedcan trigger additional change detection cycles, creating infinite loops[3][7].- Solution: Use flags to prevent reentrancy:
ngAfterViewChecked() { if (!this.viewUpdated) { this.updateView(); this.viewUpdated = true; } }4. Impact of
OnPushStrategy
- While
ChangeDetectionStrategy.OnPushreduces unnecessary checks,ngAfterViewCheckedstill runs if events or observables trigger change detection[4][6].- Combine
OnPushwith manual change detection usingChangeDetectorRef.detectChanges()for finer control[5].5. DOM Reflows and Repaints
- Operations that force synchronous layout recalculations (e.g., reading
offsetHeight) in this hook can cause browser reflows, slowing rendering[1][5].- Mitigation: Batch DOM updates or defer non-critical work using
setTimeout/requestAnimationFrame.6. Debugging Tools
- Use Angular DevTools' profiler to identify components with slow
ngAfterViewCheckedexecution[1].- Monitor the "View Checks" metric to pinpoint bottlenecks.
When to Use
ngAfterViewCheckedReserve this hook for lightweight tasks that require post-update actions, like:
- Adjusting third-party library layouts after view changes[2].
- Logging debug information during development[7].
For most use cases, prefer
ngAfterViewInit(runs once after view initialization) or reactive patterns like observables to minimize unnecessary checks[2][5].Citations:
- 1: https://angular.dev/best-practices/slow-computations
- 2: https://zerotomastery.io/blog/angular-lifecycle-hooks/
- 3: https://hackernoon.com/angular-application-performance-fixes-story-1-9bfa353d0fc4
- 4: fix(Changedetection): ngAfterViewChecked is called when component is set to OnPush and no input properties changed angular/angular#7055
- 5: https://www.miquido.com/blog/how-to-improve-the-performance-of-your-angular-app/
- 6: https://blog.angular-university.io/how-does-angular-2-change-detection-really-work/
- 7: https://dev.to/chukwuma1976/angular-lifecycle-hooks-a-high-level-overview-11i
- 8: https://v17.angular.io/guide/lifecycle-hooks
Attention: Verify Performance Efficiency in ngAfterViewChecked
The current implementation in
src/main/webapp/app/exercises/shared/feedback-suggestion/exercise-feedback-suggestion-options.component.ts(lines 57-61) correctly maintains UI state by hiding the dropdown when input controls are disabled. However, keep in mind thatngAfterViewCheckedis called after every change detection cycle. Please ensure that theinputControlsDisabled()method is lightweight and does not trigger additional change detection cycles (e.g., via heavy computations, side effects, or repeated state updates). This will help prevent performance issues in more complex UIs.src/test/javascript/spec/component/exercises/shared/feedback/feedback-suggestion-option.component.spec.ts (4)
4-12: Import organization is appropriate for the extended test requirements.The changes properly include additional testing imports that are needed for the enhanced test coverage:
MockDirectivefor isolating component under testTranslateDirectiveandMockTranslateServicefor i18n testingByfrom platform-browser for DOM querying
29-30: Good test isolation with mock directive and service.Properly providing mock implementations for
TranslateDirectiveandTranslateServiceensures tests are isolated and don't depend on actual translation functionality.
101-119: Comprehensive test case for toggling feedback suggestions.The test has been improved to verify:
- Initial state of
showDropdownList(false)- Setting of
feedbackSuggestionModulewhen toggled on- Updated state of
showDropdownList(true) after toggle- Setting of
allowManualFeedbackRequeststo false (mutual exclusivity)- Behavior when toggled off
This test thoroughly covers the component's updated behavior.
121-160: Well-structured test for assessment type switching.This test thoroughly validates that:
- Component correctly responds to assessment type changes
- UI elements are properly enabled/disabled based on assessment type
- Component correctly maintains state through assessment type transitions
The test follows a clear pattern of setup → action → assertion → next action → assertion, which makes it easy to understand and maintain.
src/test/javascript/spec/component/overview/exercise-details/request-feedback-button/request-feedback-button.component.spec.ts (2)
66-77: Properly updated factory function to support preliminary feedback.The
createBaseExercisefunction has been updated to:
- Accept a new optional parameter for
preliminaryModule- Provide a sensible default value ('module')
- Set
preliminaryFeedbackModuleon the returned exercise objectThis change aligns with the domain model updates and supports testing of the new feedback behavior.
275-287: Valuable test for non-Athena manual feedback scenario.This new test case verifies an important user scenario where:
- Athena is disabled (not available)
- The exercise is not an exam
- Manual feedback requests are enabled
Ensuring the feedback request button is properly displayed in this scenario is crucial for maintaining user access to feedback features when Athena is unavailable.
src/main/webapp/app/exercises/programming/manage/programming-exercise-detail.component.ts (2)
600-600: Updated property reference to use correct manual feedback property.The component now correctly references
exercise.allowManualFeedbackRequestsinstead of the deprecated property, which aligns with the domain model changes.
607-608: Proper implementation of distinct feedback option display.The component now correctly displays two separate boolean values:
allowFeedbackSuggestionsbased on the presence of a feedback suggestion moduleallowFeedbackRequestsbased on the presence of a preliminary feedback moduleThis implementation properly reflects the PR's goal of separating preliminary feedback from feedback suggestions.
The use of double negation (
!!) to convert potentially undefined module references to boolean values is a common and effective JavaScript pattern.src/main/webapp/app/exercises/programming/shared/lifecycle/programming-exercise-lifecycle.component.ts (4)
24-24: Appropriate import of the preliminary feedback component.The import of
ExercisePreliminaryFeedbackOptionsComponentis correctly added to support the new functionality for configuring preliminary feedback modules.
38-38: Component properly registered in the imports array.The
ExercisePreliminaryFeedbackOptionsComponentis correctly added to the imports array, enabling its use in the template.
158-164: Method appropriately renamed for clarity.The method has been correctly renamed from
toggleFeedbackRequeststotoggleManualFeedbackRequeststo clearly distinguish it from preliminary feedback functionality. The implementation correctly togglesallowManualFeedbackRequestsinstead of the previousallowFeedbackRequests.
180-180: Consistent property usage in toggleAssessmentType method.The update to set
allowManualFeedbackRequeststo false instead ofallowFeedbackRequestsmaintains consistency with the renamed property throughout the component.src/test/java/de/tum/cit/aet/artemis/athena/AthenaResourceIntegrationTest.java (5)
3-8: Updated imports for module distinction.The imports have been correctly updated to use the specific constants for feedback suggestion modules with the
_SUGGESTIONS_TESTsuffix, which aligns with the new module type distinction.
256-259: Module constant updated for feedback suggestions.The test is correctly updated to use
ATHENA_MODULE_TEXT_SUGGESTIONS_TESTinstead of the previous constant, and the mock call now includes thefalseparameter to specify non-preliminary feedback.
269-273: Module constant updated for programming feedback suggestions.Similar to the text exercise test, the programming exercise test correctly uses the updated constant and mock call parameters.
283-287: Module constant updated for modeling feedback suggestions.The test for modeling feedback suggestions is properly updated to use the new constant and mock parameter.
302-302: Consistent parameter updates across all mock calls.All calls to
mockGetFeedbackSuggestionsAndExpecthave been consistently updated to include thefalseparameter, ensuring uniform testing behavior.Also applies to: 315-315, 322-323, 330-332
src/test/javascript/spec/component/programming-exercise/programming-exercise-lifecycle.component.spec.ts (4)
22-23: AthenaService dependency properly added.The AthenaService is correctly imported and injected to support the new functionality for preliminary feedback options.
Also applies to: 33-33, 77-78
155-162: Updated test for manual feedback requests.The test case has been correctly updated to test
allowManualFeedbackRequestsinstead ofallowFeedbackRequests, maintaining consistency with the component changes.
368-392: Good test coverage for feedback suggestion component rendering.These tests thoroughly verify that the ExerciseFeedbackSuggestionOptionsComponent renders correctly based on the feedback suggestions setting and exam mode.
394-427: Comprehensive testing for preliminary feedback component.These tests verify that the ExercisePreliminaryFeedbackOptionsComponent renders correctly based on preliminary feedback settings, exam mode, and Athena service availability.
src/main/webapp/i18n/en/exercise.json (1)
176-180: Clear and descriptive internationalization keys added.The new internationalization keys for feedback options are well-named and provide clear descriptions for users:
allowFeedbackSuggestionsand its tooltip explain the automatic Athena-generated feedback suggestions for tutors.allowFeedbackRequestsand its tooltip explain the preliminary feedback request functionality for students.These additions support the UI for the new feedback configuration options and help users understand the different feedback types.
src/test/javascript/spec/component/exercises/shared/feedback/preliminary-feedback-options.component.spec.ts (8)
1-10: Component test file has appropriate imports and setupThe imports look complete and appropriate for the new component test file.
11-32: TestBed configuration is correctly implementedThe setup of the test environment is well structured with proper mocking of external dependencies (AthenaService and ActivatedRoute).
33-42: Good test for module initializationThe test effectively verifies that the component initializes with available modules and correctly sets the component state.
44-55: Observable subscription test follows best practicesGood job testing the observable subscription behavior. The test verifies that the component correctly subscribes to the AthenaService's
isEnabledmethod.
57-66: Test correctly verifies disabled state conditionsThe test appropriately checks the conditions for disabling input controls based on exercise type, assessment type, and read-only status.
68-73: Test for date-based disabling is thoroughGood verification of the due date logic for disabling input controls.
75-89: Comprehensive style testingThese tests properly verify that the component applies the correct styles to checkbox labels based on the exercise and assessment type conditions.
91-108: Toggle behavior test is well structuredThe test thoroughly verifies the toggling behavior, checking both the state changes and UI visibility changes when toggling preliminary feedback.
src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise-detail.component.ts (1)
112-113: Appropriate refactoring of feedback module referencesThe changes correctly:
- Update the title to use more consistent naming
- Change the data binding to use the new
preliminaryFeedbackModuleproperty instead ofallowFeedbackRequestsThis aligns with the PR objective to separate preliminary feedback and graded feedback modules.
src/main/webapp/app/exercises/programming/shared/lifecycle/programming-exercise-lifecycle.component.html (3)
89-89: Condition updated to use the renamed propertyThe condition now correctly checks for
exercise.allowManualFeedbackRequestsinstead of the previous generic property.
203-205: Updated feedback section conditionsThe component now correctly separates
feedbackSuggestionsfrom other types of feedback requests, which aligns with the PR objective to distinguish between different types of feedback.
206-224: Well-structured implementation of feedback options with Athena fallbackThe implementation correctly:
- Uses the modern
@ifand@forAngular syntax as required in the coding guidelines- Provides a clean separation between Athena-enabled feedback and manual feedback
- Handles fallback gracefully when Athena is not available
This implementation gives instructors flexibility in configuring feedback modules as intended in the PR objectives.
src/test/javascript/spec/component/overview/exercise-details/exercise-details-student-actions.component.spec.ts (5)
34-34: Added necessary import for new profile constantThe addition of
PROFILE_ATHENAalongsidePROFILE_THEIAsupports the new tests for Athena-enabled functionality.
66-76: Improved object literal formattingThe reformatting of object literals improves code readability without changing functionality, which is a good practice.
119-124: Better formatted mock return valueThe mockReturnValue call is now more readable with proper indentation.
674-693: Good test for feedback button display with Athena enabledThe test verifies that the feedback request button is displayed when Athena is enabled for text exercises before the due date.
716-732: Complete coverage of feedback button display requirementsThis test properly verifies that the feedback button is displayed for programming exercises with an initialized participation, which covers another important use case.
src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.ts (2)
13-22: Component declaration follows Angular style guidelinesThe component is correctly set up as standalone, with proper imports and selector naming following the
jhi-prefix convention.
41-49: Initialization logic retrieves Athena modules correctlyThe component properly retrieves available Athena modules based on course ID and exercise. The observable subscription pattern is correctly implemented.
src/test/javascript/spec/component/programming-exercise/programming-exercise-update.component.spec.ts (1)
1262-1265: Test expectations correctly validate feedback-related propertiesThe test expectations accurately verify that
allowManualFeedbackRequests,feedbackSuggestionModule, andpreliminaryFeedbackModuleare reset to their default values when dates are reset. This ensures proper behavior of the component when importing exercises.src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaModuleService.java (2)
108-127: Method signature refactored to accept direct parameters rather than exercise objectThe method signature has been improved to take the exercise type and module name directly, making it more flexible and reusable. The switch statement properly handles different exercise types.
139-148: Access check method updated to handle different module typesThe method has been enhanced to verify access to different types of modules using the ModuleType enum. This provides better flexibility for different feedback scenarios.
src/test/java/de/tum/cit/aet/artemis/athena/service/connectors/AthenaFeedbackSuggestionsServiceTest.java (4)
3-8: Tests properly import constants for all module typesThe test correctly imports all necessary constants for different module types (modeling, programming, text) for both preliminary and suggestion feedback modules.
61-89: Complete setup for testing preliminary feedback with all exercise typesThe setup is comprehensive, including configuration for modeling exercises and submissions with their respective feedback modules. This ensures proper test coverage.
94-101: Test for text feedback suggestions updated to include preliminary flagThe test method for text feedback suggestions has been properly updated to include the new
preliminaryparameter, which is set tofalsefor this test case.
116-139: Tests for preliminary feedback implemented for different exercise typesNew test methods have been added to verify preliminary feedback functionality for text and programming exercises. The tests properly set up mock expectations and verify results.
src/main/java/de/tum/cit/aet/artemis/exercise/domain/Exercise.java (5)
104-104: Field renamed for clarity.The field has been renamed from
allowFeedbackRequeststoallowManualFeedbackRequeststo more precisely reflect its purpose, distinguishing between manual and preliminary feedback requests.
148-150: New field to support separate preliminary feedback module configuration.This addition is central to the PR's purpose of allowing instructors to configure separate modules for preliminary feedback. The column name follows the same pattern as the existing
feedbackSuggestionModulefield.
253-259: Renamed getter and setter to match the field change.The getter and setter methods have been updated to match the renamed field, maintaining consistency throughout the codebase.
839-845: New getter and setter for the preliminary feedback module.These methods follow the Java Bean convention and provide appropriate access to the new field.
851-853: New convenience method to check if preliminary feedback is enabled.This helper method provides a clean, readable way to check if the preliminary feedback feature is enabled by verifying the module isn't null, similar to the existing
areFeedbackSuggestionsEnabled()method.src/main/java/de/tum/cit/aet/artemis/athena/web/AthenaResource.java (3)
108-108: Updated interface to support distinction between feedback types.The interface has been modified to include an additional type parameter for distinguishing between different feedback types.
118-118: Updated method signature to match the interface change.The method signature has been updated to use the new interface with the additional type parameter.
134-134: Inverted boolean parameter to maintain logical consistency.The call now passes
falsetofeedbackProvider.applyto represent that this resource is for graded feedback suggestions, aligning with the new terminology whereisPreliminary=falsemeans graded feedback.The comment has been updated to reflect that this resource is only for graded feedback suggestions, which helps clarify the purpose of the method.
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.java (5)
107-108: Updated method signature to use the new parameter name.The parameter name has been changed from
isGradedtoisPreliminary, improving clarity about the purpose of the parameter.
110-111: Updated method implementation and logging to reflect the new parameter.The method and its logging statement now properly handle the
isPreliminaryflag, logging the feedback type more clearly as "Non Graded" or "Graded" based on the flag value.
119-123: Updated request construction to use the appropriate Athena module.The request now:
- Passes the negated
isPreliminaryvalue to maintain compatibility with the existing API contract- Selects between
preliminaryFeedbackModuleandfeedbackSuggestionModulebased on theisPreliminaryflagThis implementation properly supports the PR's objective of allowing different modules for different feedback types.
140-147: Consistent implementation of the isPreliminary pattern in programming feedback.The changes to the programming feedback suggestions method mirror the changes to the text feedback method, maintaining consistency across exercise types.
161-175: Consistent implementation of the isPreliminary pattern in modeling feedback.The changes to the modeling feedback suggestions method also follow the same pattern as the other methods, ensuring consistency throughout the service.
src/test/java/de/tum/cit/aet/artemis/core/connector/AthenaRequestMockProvider.java (4)
55-77: Added new module type constants for testing.These new constants provide test modules for both feedback suggestions and preliminary feedback for each exercise type (text, programming, modeling), enabling comprehensive testing of the new functionality.
118-120: Helper method to generate appropriate test module names.This utility method cleanly generates the correct module name based on the module type and feedback type, improving code maintainability and reducing duplication.
210-216: Updated mock method to support preliminary feedback testing.The method has been updated to include the
isPreliminaryparameter and uses the new helper method to determine the correct test module name, allowing tests to verify both feedback types.
248-254: Updated failure mock to match the signature of the success mock.The failure mock method has also been updated with the same parameter and URL construction logic as the success mock, maintaining consistency.
src/test/java/de/tum/cit/aet/artemis/exercise/participation/ParticipationIntegrationTest.java (11)
3-5: Updated imports for preliminary feedback modules.The imports have been appropriately updated to use the preliminary feedback module constants instead of the standard feedback module constants, which aligns with the PR's objective to add support for distinct preliminary feedback modules.
548-549: LGTM! Setting preliminary feedback module for programming exercises.The programming exercise now correctly uses the preliminary feedback module constant, which is consistent with the changes across this PR.
551-551: Added isPreliminary parameter to mock method call.The mock method now includes a boolean parameter to indicate this is a preliminary feedback request, which matches the new feature being implemented.
573-573: Updated verification to use resultWebsocketService.The verification has been changed from
programmingMessagingServicetoresultWebsocketService, which is the appropriate service for broadcasting results independent of exercise type.
593-594: LGTM! Preliminary feedback module configuration for programming exercises.The test properly configures the preliminary feedback module and mocks the feedback suggestions request with the correct isPreliminary parameter.
Also applies to: 596-596
618-618: Consistent use of resultWebsocketService for feedback notifications.This change ensures that result notifications are verified against the correct service.
638-639: LGTM! Preliminary feedback module configuration for text exercises.The test correctly sets up the text exercise with the appropriate preliminary feedback module constant.
Also applies to: 641-641
678-679: LGTM! Preliminary feedback module configuration for modeling exercises.The test correctly sets up the modeling exercise with the appropriate preliminary feedback module constant.
Also applies to: 681-681
716-718: LGTM! Testing failure scenario with preliminary feedback.The test correctly configures the preliminary feedback module and mocks a failure response for feedback suggestions.
740-740: Consistent verification of result broadcasting.The verification is consistent with other tests that check result notifications.
760-761: LGTM! Text exercise failure testing with preliminary feedback.The test properly configures the preliminary feedback module and mocks a feedback suggestions failure for text exercises.
Also applies to: 764-764
...app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.ts
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaModuleService.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaModuleService.java (2)
169-175: Consider adding validation for preliminary feedback module changesThe
checkValidAthenaModuleChangemethod only validates changes to the feedback suggestion module, not the preliminary feedback module. If similar validation is needed for preliminary feedback modules, update this method accordingly.public void checkValidAthenaModuleChange(Exercise originalExercise, Exercise updatedExercise, String entityName) throws BadRequestAlertException { var dueDate = originalExercise.getDueDate(); if (!Objects.equals(originalExercise.getFeedbackSuggestionModule(), updatedExercise.getFeedbackSuggestionModule()) && dueDate != null && dueDate.isBefore(ZonedDateTime.now())) { throw new BadRequestAlertException("Athena module can't be changed after due date has passed", entityName, "athenaModuleChangeAfterDueDate"); } + // Add similar validation for preliminary feedback module if needed + if (!Objects.equals(originalExercise.getPreliminaryFeedbackModule(), updatedExercise.getPreliminaryFeedbackModule()) && dueDate != null + && dueDate.isBefore(ZonedDateTime.now())) { + throw new BadRequestAlertException("Preliminary feedback module can't be changed after due date has passed", entityName, "preliminaryFeedbackModuleChangeAfterDueDate"); + } }
182-184: Consider handling restricted preliminary feedback modulesThe
revokeAccessToRestrictedFeedbackSuggestionModulesmethod only revokes access to restricted feedback suggestion modules. If preliminary feedback modules can also be restricted, consider creating a similar method or extending this one to handle both module types.public void revokeAccessToRestrictedFeedbackSuggestionModules(Course course) { exerciseRepository.revokeAccessToRestrictedFeedbackSuggestionModulesByCourseId(course.getId(), restrictedModules); } + /** + * Revokes the access to restricted Athena modules for preliminary feedback for all exercises of a course. + * + * @param course The course for which the access to restricted modules should be revoked + */ + public void revokeAccessToRestrictedPreliminaryFeedbackModules(Course course) { + exerciseRepository.revokeAccessToRestrictedPreliminaryFeedbackModulesByCourseId(course.getId(), restrictedModules); + }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaModuleService.java(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`src/main/java/**/*.java`: naming:CamelCase; principles:{sin...
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/athena/service/AthenaModuleService.java
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: Call Build Workflow / Build .war artifact
- GitHub Check: Call Build Workflow / Build and Push Docker Image
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: client-tests-selected
- GitHub Check: client-style
- GitHub Check: server-tests
- GitHub Check: server-style
- GitHub Check: client-tests
- GitHub Check: Analyse
🔇 Additional comments (7)
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaModuleService.java (7)
25-25: LGTM - Added import for ModuleTypeThe import is appropriately added to support the new ModuleType enum functionality.
108-115: Approve method signature change for better modularityThe method signature change improves modularity by accepting
ExerciseTypeandmoduledirectly instead of anExerciseobject, making the method more focused and reusable.
116-127: LGTM - Simplified switch statementThe switch statement has been properly updated to use the passed
exerciseTypeparameter directly.
135-135: LGTM - Updated parameter documentationDocumentation properly updated to reflect the new ModuleType parameter.
139-147: LGTM - Enhanced module access check with module typeThe method has been properly updated to handle different module types and provides clear error messages that include the module type information.
150-157: LGTM - Well-designed module selection helper methodThe new private helper method follows single responsibility principle and effectively encapsulates the logic for selecting the appropriate module based on the module type.
162-162: Consider extending module change validation to preliminary feedback modulesThe documentation specifies that module changes cannot be made after the due date, but this only applies to feedback suggestion modules. Consider whether similar validation should be implemented for preliminary feedback modules.
Do you need this restriction for preliminary feedback modules as well, or is it intentional that only feedback suggestion modules have this limitation?
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/java/de/tum/cit/aet/artemis/exercise/repository/ExerciseRepository.java (2)
608-623: Update the method documentation to accurately reflect its purpose.The documentation for this new method is identical to the one for
revokeAccessToRestrictedFeedbackSuggestionModulesByCourseIdbut should specifically mention that it revokes access to preliminary feedback modules rather than feedback suggestion modules./** - * Revokes the access by setting all exercises that currently utilize a restricted feedback suggestion module to null. + * Revokes the access by setting all exercises that currently utilize a restricted preliminary feedback module to null. * * @param courseId The course for which the access should be revoked - * @param restrictedFeedbackSuggestionModule Collection of restricted modules + * @param restrictedFeedbackSuggestionModule Collection of restricted preliminary feedback modules */
622-623: Consider renaming the parameter to match its purpose.The parameter name
restrictedFeedbackSuggestionModuleis used for both methods but serves different purposes. A more specific name would improve code clarity.void revokeAccessToRestrictedPreliminaryFeedbackModulesByCourseId(@Param("courseId") Long courseId, - @Param("restrictedFeedbackSuggestionModule") Collection<String> restrictedFeedbackSuggestionModule); + @Param("restrictedFeedbackSuggestionModule") Collection<String> restrictedPreliminaryFeedbackModules);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaModuleService.java(5 hunks)src/main/java/de/tum/cit/aet/artemis/core/web/CourseResource.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exercise/repository/ExerciseRepository.java(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaModuleService.java
🧰 Additional context used
📓 Path-based instructions (1)
`src/main/java/**/*.java`: naming:CamelCase; principles:{sin...
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/CourseResource.javasrc/main/java/de/tum/cit/aet/artemis/exercise/repository/ExerciseRepository.java
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: Call Build Workflow / Build and Push Docker Image
- GitHub Check: Call Build Workflow / Build .war artifact
- GitHub Check: server-style
- GitHub Check: client-tests
- GitHub Check: client-style
- GitHub Check: client-tests-selected
- GitHub Check: server-tests
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Analyse
🔇 Additional comments (1)
src/main/java/de/tum/cit/aet/artemis/core/web/CourseResource.java (1)
336-336: Method name update aligns with new feature supportThe method call has been updated from
revokeAccessToRestrictedFeedbackSuggestionModulestorevokeAccessToRestrictedFeedbackModules, which makes it more general to handle both feedback suggestions and preliminary feedback modules as part of the new functionality to allow separate modules for different feedback types.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaModuleService.java(5 hunks)src/main/java/de/tum/cit/aet/artemis/exercise/repository/ExerciseRepository.java(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaModuleService.java
🧰 Additional context used
📓 Path-based instructions (1)
`src/main/java/**/*.java`: naming:CamelCase; principles:{sin...
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/repository/ExerciseRepository.java
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: Call Build Workflow / Build .war artifact
- GitHub Check: Call Build Workflow / Build and Push Docker Image
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: client-tests-selected
- GitHub Check: client-style
- GitHub Check: client-tests
- GitHub Check: server-style
- GitHub Check: server-tests
- GitHub Check: Analyse
| /** | ||
| * Revokes the access by setting all exercises that currently utilize a restricted preliminary feedback module to null. | ||
| * | ||
| * @param courseId The course for which the access should be revoked | ||
| * @param restrictedPreliminaryFeedbackModule Collection of restricted modules | ||
| */ | ||
| @Transactional // ok because of modifying query | ||
| @Modifying | ||
| @Query(""" | ||
| UPDATE Exercise e | ||
| SET e.preliminaryFeedbackModule = NULL | ||
| WHERE e.course.id = :courseId | ||
| AND e.preliminaryFeedbackModule IN :restrictedFeedbackSuggestionModule | ||
| """) | ||
| void revokeAccessToRestrictedPreliminaryFeedbackModulesByCourseId(@Param("courseId") Long courseId, | ||
| @Param("restrictedFeedbackSuggestionModule") Collection<String> restrictedPreliminaryFeedbackModule); |
There was a problem hiding this comment.
Fix parameter name inconsistency in SQL query
There's a mismatch between the method parameter name and the parameter name used in the SQL query:
- The method parameter is defined as
restrictedPreliminaryFeedbackModule(line 623) - But the SQL query references
restrictedFeedbackSuggestionModule(line 621)
This inconsistency will cause runtime errors when the query is executed.
Apply this diff to fix the parameter name in the SQL query:
@Query("""
UPDATE Exercise e
SET e.preliminaryFeedbackModule = NULL
WHERE e.course.id = :courseId
- AND e.preliminaryFeedbackModule IN :restrictedFeedbackSuggestionModule
+ AND e.preliminaryFeedbackModule IN :restrictedPreliminaryFeedbackModule
""")
void revokeAccessToRestrictedPreliminaryFeedbackModulesByCourseId(@Param("courseId") Long courseId,
@Param("restrictedPreliminaryFeedbackModule") Collection<String> restrictedPreliminaryFeedbackModule);📝 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.
| /** | |
| * Revokes the access by setting all exercises that currently utilize a restricted preliminary feedback module to null. | |
| * | |
| * @param courseId The course for which the access should be revoked | |
| * @param restrictedPreliminaryFeedbackModule Collection of restricted modules | |
| */ | |
| @Transactional // ok because of modifying query | |
| @Modifying | |
| @Query(""" | |
| UPDATE Exercise e | |
| SET e.preliminaryFeedbackModule = NULL | |
| WHERE e.course.id = :courseId | |
| AND e.preliminaryFeedbackModule IN :restrictedFeedbackSuggestionModule | |
| """) | |
| void revokeAccessToRestrictedPreliminaryFeedbackModulesByCourseId(@Param("courseId") Long courseId, | |
| @Param("restrictedFeedbackSuggestionModule") Collection<String> restrictedPreliminaryFeedbackModule); | |
| /** | |
| * Revokes the access by setting all exercises that currently utilize a restricted preliminary feedback module to null. | |
| * | |
| * @param courseId The course for which the access should be revoked | |
| * @param restrictedPreliminaryFeedbackModule Collection of restricted modules | |
| */ | |
| @Transactional // ok because of modifying query | |
| @Modifying | |
| @Query(""" | |
| UPDATE Exercise e | |
| SET e.preliminaryFeedbackModule = NULL | |
| WHERE e.course.id = :courseId | |
| AND e.preliminaryFeedbackModule IN :restrictedPreliminaryFeedbackModule | |
| """) | |
| void revokeAccessToRestrictedPreliminaryFeedbackModulesByCourseId(@Param("courseId") Long courseId, | |
| @Param("restrictedPreliminaryFeedbackModule") Collection<String> restrictedPreliminaryFeedbackModule); |
Please test only on TS3
!!! Contains database migrations !!!
Checklist
General
Server
Client
authoritiesto all new routes and checked the course groups for displaying navigation elements (links, buttons).Changes affecting Programming Exercises
Motivation and Context
Currently, Artemis allows only one module to be set for Athena Feedback Requests, be it for preliminary or graded feedback. This PR addresses the limitation and enables setting separate modules for each type of feedback.
Description
This PR introduces functionality that allows instructors to configure different Athena Feedback Request modules for preliminary feedback and graded feedback. Preliminary feedback can now be assigned a dedicated module without affecting the graded feedback settings.
Steps for Testing
Disclaimer:
Currently, only LLM modules support preliminary feedback. You can easily test this functionality with non-LLM modules to verify if the correct module is being used (non-LLM modules will not work).
As an instructor:
2. Navigate to the course settings.
3. Create a Programming, Text, or Modeling exercise.
4. Select a module for preliminary feedback (no due date required). For graded feedback, enable Feedback Suggestions, set a due date, and select a module(text or programming).
5. Save the changes.
As a student:
6. Open the created exercise.
7. Submit something.
8. Submit an AI feedback request.
9. Verify that the appropriate module is used based on the type of feedback.
Testserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.
Review Progress
Performance Review
Code Review
Manual Tests
Exam Mode Test
Performance Tests
Screenshots
Summary by CodeRabbit
New Features
Enhancements
Documentation