Programming exercises: Allow to choose preliminary feedback model
#10946
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
|
""" WalkthroughThis change introduces a new "preliminary feedback" module type alongside the existing "feedback suggestions" module throughout both backend and frontend. It updates APIs, service logic, entity models, and UI to support, configure, and display both module types distinctly. Numerous method signatures, conditions, and UI elements are adapted to handle the new module type and related permissions. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI
participant Backend
participant Athena
User->>UI: Open Exercise Configuration
UI->>Backend: GET available Athena modules (with ModuleType param)
Backend->>Athena: Query modules (filtered by ModuleType)
Athena-->>Backend: Return module list
Backend-->>UI: Return filtered module list
UI->>User: Display options for Feedback Suggestions and Preliminary Feedback
User->>UI: Enable Preliminary Feedback
UI->>Backend: Save exercise with preliminaryFeedbackModule set
Backend->>Athena: Validate module access (ModuleType.PRELIMINARY_FEEDBACK)
Athena-->>Backend: Confirm access
Backend-->>UI: Save successful
User->>UI: Request Preliminary Feedback (as student)
UI->>Backend: Request preliminary feedback (isPreliminary=true)
Backend->>Athena: Forward request to preliminary feedback module
Athena-->>Backend: Return feedback
Backend-->>UI: Display feedback to user
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (3)
src/main/webapp/app/text/overview/text-editor/text-editor.component.html (1)
27-36:⚠️ Potential issueRemove
thisreferences in template expressions.
In Angular templates, dropthis.and bind directly. Refactor the snippet to:@if ( textExercise.preliminaryFeedbackModule && (!textExercise.dueDate || !hasExerciseDueDatePassed(textExercise, participation)) ) { <!-- … --> }This prevents template parsing errors.
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaRepositoryExportService.java (1)
67-72: 🛠️ Refactor suggestionUpdate method name and documentation to reflect dual module type check.
The method now checks for both feedback suggestions and preliminary feedback, but the method name and documentation only reference feedback suggestions. Consider updating the method name and documentation to accurately reflect that it validates either module type is enabled.
- private void checkFeedbackSuggestionsOrAutomaticFeedbackEnabledElseThrow(Exercise exercise) { + private void checkAthenaFeedbackModulesEnabledElseThrow(Exercise exercise) { if (!(exercise.areFeedbackSuggestionsEnabled() || exercise.isPreliminaryFeedbackEnabled())) { - log.error("Feedback suggestions are not enabled for exercise {}", exercise.getId()); - throw new ServiceUnavailableException("Feedback suggestions are not enabled for exercise"); + log.error("Neither feedback suggestions nor preliminary feedback are enabled for exercise {}", exercise.getId()); + throw new ServiceUnavailableException("Neither feedback suggestions nor preliminary feedback are enabled for exercise"); } }Also update the method call on line 89 to use the new method name.
src/test/javascript/spec/component/exercises/shared/feedback/preliminary-feedback-options.component.spec.ts (1)
1-110: 💡 Verification agent🧩 Analysis chain
Request verification of component implementation.
The tests reference properties and methods that may not exist on the actual component. Please verify that the
ExercisePreliminaryFeedbackOptionsComponenthas the expected interface.
🏁 Script executed:
#!/bin/bash # Verify the component interface matches the test expectations ast-grep --pattern $'export class ExercisePreliminaryFeedbackOptionsComponent { $$$ isAthenaEnabled$$$ $$$ }' # Also check for the isAthenaEnabled property rg -A 10 "isAthenaEnabled" src/main/webapp/app/exercises/shared/preliminary-feedback/Length of output: 5073
🏁 Script executed:
#!/bin/bash rg -nE "inputControlsDisabled|getCheckboxLabelStyle|togglePreliminaryFeedback|exercise\(" src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.tsLength of output: 318
🏁 Script executed:
#!/bin/bash # Extract method and property declarations to verify component interface rg -n 'inputControlsDisabled|getCheckboxLabelStyle|togglePreliminaryFeedback|exercise\(' -C3 src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.tsLength of output: 2175
🏁 Script executed:
#!/bin/bash # Verify existence of exercise() and readOnly() methods in the component rg -nE "exercise\(\)|readOnly\(\)" -C5 src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.tsLength of output: 212
#!/bin/bash file="src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.ts" echo "===== File Header =====" sed -n '1,50p' "$file" echo echo "===== Looking for exercise() method or property =====" grep -n "exercise\s*(" "$file" || echo "No occurrences of exercise() found" echo echo "===== Looking for readOnly() method or property =====" grep -n "readOnly\s*(" "$file" || echo "No occurrences of readOnly() found"
#!/bin/bash file="src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.ts" echo "=== Class Declaration ===" rg -n "export class ExercisePreliminaryFeedbackOptionsComponent" "$file" echo -e "\n=== @Input Decorators ===" rg -n "@Input" "$file" || echo "No @Input decorators found" echo -e "\n=== exercise() Method/Call Occurrences ===" rg -n "exercise\(\)" "$file" || echo "No exercise() occurrences found" echo -e "\n=== readOnly() Method/Call Occurrences ===" rg -n "readOnly\(\)" "$file" || echo "No readOnly() occurrences found"
#!/bin/bash file="src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.ts" echo "=== @Input Decorators, exercise(), readOnly() Definitions/Calls ===" grep -nE "@Input|exercise\s*\(|readOnly\s*\(" -C3 "$file" || echo "No matches found"
Component interface mismatches tests – fixes required
The test suite references several inputs and methods that don’t exist (or don’t match) on ExercisePreliminaryFeedbackOptionsComponent. Please align the implementation or the tests:
- Missing @input() declarations
• Tests call fixture.componentRef.setInput('exercise', …) and .setInput('readOnly', …), but the component has no corresponding@Input() exercise: Exerciseor@Input() readOnly: boolean.- Observable vs. boolean property
• Tests expectisAthenaEnabled$(an Observable) set from the service; the component currently declaresisAthenaEnabled: boolean.- Method references not defined
• The component callsthis.exercise()andthis.readOnly(), but neither method is declared.- Array helper mismatch
• In togglePreliminaryFeedback, the code usesthis.availableAthenaModules.first(), butstring[]has nofirst()method (tests expect the first element).- Service method signature
• Tests spy onathenaService.getAvailableModules()without arguments, whereas the component calls it with(courseId, this.exercise(), ModuleType.PRELIMINARY_FEEDBACK).Suggested fixes:
- Add or rename inputs/methods to match test expectations (or update tests).
- Expose
isAthenaEnabled$as an Observable if that’s the intended API.- Replace
first()with[0](or add a helper) to select the first module.- Ensure service calls and spies use the same signature.
File(s):
- src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.ts
🧰 Tools
🪛 GitHub Check: client-tests
[failure] 52-52:
Parameter 'result' implicitly has an 'any' type.
[failure] 52-52:
Property 'isAthenaEnabled$' does not exist on type 'ExercisePreliminaryFeedbackOptionsComponent'. Did you mean 'isAthenaEnabled'?
[failure] 51-51:
Property 'isAthenaEnabled$' does not exist on type 'ExercisePreliminaryFeedbackOptionsComponent'. Did you mean 'isAthenaEnabled'?
[failure] 46-46:
Argument of type 'Observable' is not assignable to parameter of type 'boolean'.
[failure] 21-21:
No overload matches this call.🪛 GitHub Check: client-tests-selected
[failure] 52-52:
Parameter 'result' implicitly has an 'any' type.
[failure] 52-52:
Property 'isAthenaEnabled$' does not exist on type 'ExercisePreliminaryFeedbackOptionsComponent'. Did you mean 'isAthenaEnabled'?
[failure] 51-51:
Property 'isAthenaEnabled$' does not exist on type 'ExercisePreliminaryFeedbackOptionsComponent'. Did you mean 'isAthenaEnabled'?
[failure] 46-46:
Argument of type 'Observable' is not assignable to parameter of type 'boolean'.
[failure] 21-21:
No overload matches this call.
🧹 Nitpick comments (15)
src/main/webapp/app/modeling/manage/detail/modeling-exercise-detail.component.ts (1)
128-129: Clarify translation key usage for consistency.The translation key
artemisApp.exercise.allowFeedbackRequestson line 129 is potentially confusing since:
- This property was renamed to
allowManualFeedbackRequestselsewhere in the codebase- Here it's displaying data from
preliminaryFeedbackModule, not manual feedback requestsConsider using a more specific translation key like
artemisApp.exercise.allowPreliminaryFeedbackto clearly distinguish this from manual feedback requests.Apply this diff to improve clarity:
-{ type: DetailType.Boolean, title: 'artemisApp.exercise.allowFeedbackRequests', data: { boolean: !!exercise.preliminaryFeedbackModule } }, +{ type: DetailType.Boolean, title: 'artemisApp.exercise.allowPreliminaryFeedback', data: { boolean: !!exercise.preliminaryFeedbackModule } },src/main/webapp/app/programming/manage/update/programming-exercise-update.helper.ts (1)
43-44: Consider grouping related feedback fields together for better organization.The new feedback-related enum members are inconsistently placed.
FEEDBACK_SUGGESTIONSis mixed within the grading section whilePRELIMINARY_FEEDBACK_REQUESTSis placed separately under a "Feedback section" comment. For better code organization, consider grouping all feedback-related fields together.COMPLAINT_ON_AUTOMATIC_ASSESSMENT = 'complaintOnAutomaticAssessment', - FEEDBACK_SUGGESTIONS = 'feedbackSuggestions', - PRELIMINARY_FEEDBACK_REQUESTS = 'preliminaryFeedbackRequests', MANUAL_FEEDBACK_REQUESTS = 'manualFeedbackRequests', + + // Feedback section + FEEDBACK_SUGGESTIONS = 'feedbackSuggestions', + PRELIMINARY_FEEDBACK_REQUESTS = 'preliminaryFeedbackRequests',Apply the same grouping in the mapping object for consistency.
Also applies to: 93-93, 100-101
src/main/webapp/app/modeling/manage/update/modeling-exercise-update.component.ts (1)
47-48: LGTM! Proper Angular component integration for preliminary feedback support.The imports and component integration follow Angular conventions correctly. The
ExercisePreliminaryFeedbackOptionsComponentis properly added to enable preliminary feedback functionality in modeling exercises.Note: The
ExerciseUpdatePlagiarismComponentViewChild is declared but not used in the visible code. Ensure it's properly utilized in the template or consider removing if unused.Also applies to: 72-72, 93-93
src/main/webapp/app/core/course/overview/exercise-details/student-actions/exercise-details-student-actions.component.spec.ts (1)
537-556: Consistent test implementation for modeling exercises.The test follows the same pattern as the text exercise test, which is good for consistency. However, there's a minor type inconsistency.
Apply this fix to use the correct exercise type:
- const exercise = { type: ExerciseType.MODELING } as ProgrammingExercise; + const exercise = { type: ExerciseType.MODELING } as Exercise;src/main/webapp/app/assessment/shared/services/athena.service.ts (1)
23-28: Consider returning Observable for consistency.The new
isEnabled()method is useful, but it returns a boolean directly while other methods in the service return Observables. Consider whether this should return an Observable for consistency with the reactive pattern used throughout the service.If consistency is preferred, consider this approach:
- public isEnabled(): boolean { - return this.profileService.isProfileActive(PROFILE_ATHENA); - } + public isEnabled(): Observable<boolean> { + return of(this.profileService.isProfileActive(PROFILE_ATHENA)); + }src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.html (1)
18-18: Use i18n translation key instead of hardcoded label.The label "Athena Preliminary Feedback Module" should use a translation key for internationalization consistency.
- <label for="athenaModuleSelect">Athena Preliminary Feedback Module</label> + <label for="athenaModuleSelect" jhiTranslate="artemisApp.exercise.athenaPreliminaryFeedbackModule"></label>src/test/java/de/tum/cit/aet/artemis/exercise/participation/ParticipationIntegrationTest.java (1)
918-921: Use constant instead of string literal for modeling module
The test hardcodes"module_modeling_preliminary_test"instead of using the importedATHENA_MODULE_MODELING_PRELIMINARY_TESTconstant. To ensure consistency and avoid typos, replace the literal with the constant.src/test/java/de/tum/cit/aet/artemis/athena/service/connectors/AthenaFeedbackSuggestionsServiceTest.java (1)
142-147: Consider testing ID conflicts for both feedback types.The test now only verifies ID conflicts for preliminary feedback (
isPreliminary=true). Since ID conflicts could occur for both feedback types, consider parameterizing this test to cover both scenarios.Consider using
@ParameterizedTestto test both feedback types:-@Test -void testFeedbackSuggestionsIdConflict() { - athenaRequestMockProvider.mockGetFeedbackSuggestionsAndExpect("text", true); +@ParameterizedTest +@ValueSource(booleans = {true, false}) +void testFeedbackSuggestionsIdConflict(boolean isPreliminary) { + athenaRequestMockProvider.mockGetFeedbackSuggestionsAndExpect("text", isPreliminary); var otherExercise = new TextExercise(); textSubmission.setParticipation(new StudentParticipation().exercise(otherExercise)); // Add submission to wrong exercise - assertThatExceptionOfType(ConflictException.class).isThrownBy(() -> athenaFeedbackSuggestionsService.getTextFeedbackSuggestions(textExercise, textSubmission, true)); + assertThatExceptionOfType(ConflictException.class).isThrownBy(() -> athenaFeedbackSuggestionsService.getTextFeedbackSuggestions(textExercise, textSubmission, isPreliminary)); }src/main/webapp/app/exercise/feedback-suggestion/feedback-suggestion-option.component.spec.ts (1)
6-6: Remove unused import.The
MockProviderimport is not used in the test file and should be removed to clean up the code.-import { MockDirective, MockProvider } from 'ng-mocks'; +import { MockDirective } from 'ng-mocks';🧰 Tools
🪛 GitHub Check: client-tests
[failure] 6-6:
'MockProvider' is declared but its value is never read.🪛 GitHub Check: client-tests-selected
[failure] 6-6:
'MockProvider' is declared but its value is never read.src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.ts (1)
86-88: Suggest refactoring date comparison logic.The current implementation only checks the exercise's due date, but the component also accepts a
dueDateinput. Consider using the input parameter if provided.private hasDueDatePassed() { - return dayjs(this.exercise().dueDate).isBefore(dayjs()); + const effectiveDueDate = this.dueDate() || this.exercise().dueDate; + return effectiveDueDate ? dayjs(effectiveDueDate).isBefore(dayjs()) : false; }src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.java (4)
87-87: Consider renaming theisGradedfield to match the new semantics.Since the parameter throughout the service has been renamed from
isGradedtoisPreliminary, the RequestDTO field should be updated accordingly to maintain consistency and avoid confusion.- private record RequestDTO(ExerciseBaseDTO exercise, SubmissionBaseDTO submission, boolean isGraded) { + private record RequestDTO(ExerciseBaseDTO exercise, SubmissionBaseDTO submission, boolean isGraded) {Consider renaming to either
isPreliminary(and adjust the logic where it's used) orisGraded(keeping the current inverted logic).
111-111: Fix inconsistent hyphenation in logging messages.The logging messages use different formats for "non-graded": "Non Graded" (with space) vs "Non-Graded" (with hyphen).
- log.debug("Start Athena {} Feedback Suggestions Service for Exercise '{}' (#{}).", isPreliminary ? "Non Graded" : "Graded", exercise.getTitle(), exercise.getId()); + log.debug("Start Athena {} Feedback Suggestions Service for Exercise '{}' (#{}).", isPreliminary ? "Non-Graded" : "Graded", exercise.getTitle(), exercise.getId());- log.info("Athena responded to '{}' feedback suggestions request: {}", isPreliminary ? "Non Graded" : "Graded", response.data); + log.info("Athena responded to '{}' feedback suggestions request: {}", isPreliminary ? "Non-Graded" : "Graded", response.data);Also applies to: 125-125
140-140: Fix inconsistent hyphenation within the same method.Line 140 uses "Non Graded" while line 147 uses "Non-Graded". Should be consistent.
- log.debug("Start Athena {} Feedback Suggestions Service for Exercise '{}' (#{}).", isPreliminary ? "Non Graded" : "Graded", exercise.getTitle(), exercise.getId()); + log.debug("Start Athena {} Feedback Suggestions Service for Exercise '{}' (#{}).", isPreliminary ? "Non-Graded" : "Graded", exercise.getTitle(), exercise.getId());
161-161: Maintain consistent hyphenation for "non-graded".For consistency with the programming feedback method, use "Non-Graded" with a hyphen.
- log.debug("Start Athena {} Feedback Suggestions Service for Modeling Exercise '{}' (#{}).", isPreliminary ? "Non Graded" : "Graded", exercise.getTitle(), exercise.getId()); + log.debug("Start Athena {} Feedback Suggestions Service for Modeling Exercise '{}' (#{}).", isPreliminary ? "Non-Graded" : "Graded", exercise.getTitle(), exercise.getId());- log.info("Athena responded to '{}' feedback suggestions request: {}", isPreliminary ? "Non Graded" : "Graded", response.data); + log.info("Athena responded to '{}' feedback suggestions request: {}", isPreliminary ? "Non-Graded" : "Graded", response.data);Also applies to: 174-174
src/main/java/de/tum/cit/aet/artemis/athena/web/AthenaResource.java (1)
142-143: Clarify the comment to match the new parameter semantics.The comment could be clearer about the relationship between the boolean value and feedback type.
- // this resource is only for graded feedback suggestions - return ResponseEntity.ok(feedbackProvider.apply(exercise, submission, false)); + // this resource is only for graded feedback suggestions (isPreliminary = false) + return ResponseEntity.ok(feedbackProvider.apply(exercise, submission, false));
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/main/resources/config/liquibase/changelog/20250602083137_changelog.xmlis excluded by!**/*.xmlsrc/main/resources/config/liquibase/master.xmlis excluded by!**/*.xml
📒 Files selected for processing (61)
src/main/java/de/tum/cit/aet/artemis/athena/api/AthenaApi.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/athena/api/AthenaFeedbackApi.java(1 hunks)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(7 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(5 hunks)src/main/java/de/tum/cit/aet/artemis/exercise/domain/Exercise.java(4 hunks)src/main/java/de/tum/cit/aet/artemis/exercise/repository/ExerciseRepository.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/exercise/service/SubmissionService.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/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/web/TextExerciseResource.java(4 hunks)src/main/webapp/app/assessment/manage/assessment-warning/assessment-warning.component.ts(1 hunks)src/main/webapp/app/assessment/shared/assessment-dashboard/exercise-dashboard/exercise-assessment-dashboard.component.spec.ts(1 hunks)src/main/webapp/app/assessment/shared/assessment-dashboard/exercise-dashboard/exercise-assessment-dashboard.component.ts(1 hunks)src/main/webapp/app/assessment/shared/entities/athena.model.ts(1 hunks)src/main/webapp/app/assessment/shared/services/athena.service.ts(2 hunks)src/main/webapp/app/core/course/overview/exercise-details/request-feedback-button/request-feedback-button.component.spec.ts(2 hunks)src/main/webapp/app/core/course/overview/exercise-details/request-feedback-button/request-feedback-button.component.ts(1 hunks)src/main/webapp/app/core/course/overview/exercise-details/student-actions/exercise-details-student-actions.component.html(2 hunks)src/main/webapp/app/core/course/overview/exercise-details/student-actions/exercise-details-student-actions.component.spec.ts(2 hunks)src/main/webapp/app/exercise/feedback-suggestion/exercise-feedback-suggestion-options.component.html(2 hunks)src/main/webapp/app/exercise/feedback-suggestion/exercise-feedback-suggestion-options.component.ts(5 hunks)src/main/webapp/app/exercise/feedback-suggestion/feedback-suggestion-option.component.spec.ts(4 hunks)src/main/webapp/app/exercise/shared/entities/exercise/exercise.model.ts(4 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/modeling/manage/detail/modeling-exercise-detail.component.ts(1 hunks)src/main/webapp/app/modeling/manage/update/modeling-exercise-update.component.html(1 hunks)src/main/webapp/app/modeling/manage/update/modeling-exercise-update.component.ts(3 hunks)src/main/webapp/app/modeling/overview/modeling-submission/modeling-submission.component.html(1 hunks)src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.ts(1 hunks)src/main/webapp/app/programming/manage/update/programming-exercise-update.component.spec.ts(2 hunks)src/main/webapp/app/programming/manage/update/programming-exercise-update.helper.ts(2 hunks)src/main/webapp/app/programming/shared/lifecycle/programming-exercise-lifecycle.component.html(3 hunks)src/main/webapp/app/programming/shared/lifecycle/programming-exercise-lifecycle.component.spec.ts(5 hunks)src/main/webapp/app/programming/shared/lifecycle/programming-exercise-lifecycle.component.ts(4 hunks)src/main/webapp/app/text/manage/detail/text-exercise-detail.component.ts(1 hunks)src/main/webapp/app/text/manage/text-exercise/update/text-exercise-update.component.html(1 hunks)src/main/webapp/app/text/manage/text-exercise/update/text-exercise-update.component.ts(2 hunks)src/main/webapp/app/text/overview/text-editor/text-editor.component.html(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/exercises/shared/feedback/preliminary-feedback-options.component.spec.ts(1 hunks)
💤 Files with no reviewable changes (2)
- src/main/webapp/i18n/en/programmingExercise.json
- src/main/webapp/i18n/de/programmingExercise.json
🧰 Additional context used
📓 Path-based instructions (6)
`src/main/java/**/*.java`: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,de...
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/ProgrammingExerciseImportService.javasrc/main/java/de/tum/cit/aet/artemis/athena/api/AthenaFeedbackApi.javasrc/main/java/de/tum/cit/aet/artemis/athena/service/AthenaSubmissionSelectionService.javasrc/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingAssessmentService.javasrc/main/java/de/tum/cit/aet/artemis/programming/domain/ProgrammingExercise.javasrc/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSendingService.javasrc/main/java/de/tum/cit/aet/artemis/exercise/repository/ExerciseRepository.javasrc/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.javasrc/main/java/de/tum/cit/aet/artemis/athena/service/AthenaRepositoryExportService.javasrc/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingSubmissionResource.javasrc/main/java/de/tum/cit/aet/artemis/athena/api/AthenaApi.javasrc/main/java/de/tum/cit/aet/artemis/athena/domain/ModuleType.javasrc/main/java/de/tum/cit/aet/artemis/exercise/service/SubmissionService.javasrc/main/java/de/tum/cit/aet/artemis/athena/service/AthenaSubmissionSendingService.javasrc/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.javasrc/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseExportImportResource.javasrc/main/java/de/tum/cit/aet/artemis/exercise/domain/Exercise.javasrc/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.javasrc/main/java/de/tum/cit/aet/artemis/athena/service/AthenaModuleService.javasrc/main/java/de/tum/cit/aet/artemis/athena/web/AthenaResource.java
`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/**/*.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/text/overview/text-editor/text-editor.component.htmlsrc/main/webapp/app/text/manage/text-exercise/update/text-exercise-update.component.htmlsrc/main/webapp/app/core/course/overview/exercise-details/student-actions/exercise-details-student-actions.component.htmlsrc/main/webapp/app/modeling/overview/modeling-submission/modeling-submission.component.htmlsrc/main/webapp/app/modeling/manage/update/modeling-exercise-update.component.htmlsrc/main/webapp/app/programming/shared/lifecycle/programming-exercise-lifecycle.component.htmlsrc/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.htmlsrc/main/webapp/app/exercise/feedback-suggestion/exercise-feedback-suggestion-options.component.html
`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/...
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/textExercise.jsonsrc/main/webapp/i18n/de/exercise.json
`src/main/webapp/**/*.ts`: angular_style:https://angular.io/guide/styleguide;methods_in_html:false;lazy_loading:true;code_reuse:true;tests:meaningful;types:PascalCase;enums:PascalC...
src/main/webapp/app/modeling/manage/detail/modeling-exercise-detail.component.tssrc/main/webapp/app/assessment/shared/assessment-dashboard/exercise-dashboard/exercise-assessment-dashboard.component.spec.tssrc/main/webapp/app/assessment/manage/assessment-warning/assessment-warning.component.tssrc/main/webapp/app/text/manage/text-exercise/update/text-exercise-update.component.tssrc/main/webapp/app/programming/manage/update/programming-exercise-update.helper.tssrc/main/webapp/app/assessment/shared/assessment-dashboard/exercise-dashboard/exercise-assessment-dashboard.component.tssrc/main/webapp/app/core/course/overview/exercise-details/request-feedback-button/request-feedback-button.component.tssrc/main/webapp/app/modeling/manage/update/modeling-exercise-update.component.tssrc/main/webapp/app/programming/shared/lifecycle/programming-exercise-lifecycle.component.tssrc/main/webapp/app/programming/manage/update/programming-exercise-update.component.spec.tssrc/main/webapp/app/text/manage/detail/text-exercise-detail.component.tssrc/main/webapp/app/core/course/overview/exercise-details/student-actions/exercise-details-student-actions.component.spec.tssrc/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.tssrc/main/webapp/app/assessment/shared/entities/athena.model.tssrc/main/webapp/app/core/course/overview/exercise-details/request-feedback-button/request-feedback-button.component.spec.tssrc/main/webapp/app/exercise/shared/entities/exercise/exercise.model.tssrc/main/webapp/app/programming/shared/lifecycle/programming-exercise-lifecycle.component.spec.tssrc/main/webapp/app/assessment/shared/services/athena.service.tssrc/main/webapp/app/exercise/feedback-suggestion/feedback-suggestion-option.component.spec.tssrc/main/webapp/app/exercise/feedback-suggestion/exercise-feedback-suggestion-options.component.tssrc/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.ts
`src/test/java/**/*.java`: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_...
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/programming/ProgrammingAssessmentIntegrationTest.javasrc/test/java/de/tum/cit/aet/artemis/athena/AthenaResourceIntegrationTest.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
`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_logi...
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/exercises/shared/feedback/preliminary-feedback-options.component.spec.ts
🧬 Code Graph Analysis (3)
src/main/java/de/tum/cit/aet/artemis/athena/api/AthenaFeedbackApi.java (1)
src/main/webapp/app/assessment/shared/services/athena.service.ts (3)
getTextFeedbackSuggestions(94-124)getProgrammingFeedbackSuggestions(133-158)getModelingFeedbackSuggestions(167-201)
src/test/java/de/tum/cit/aet/artemis/athena/service/connectors/AthenaFeedbackSuggestionsServiceTest.java (1)
src/main/webapp/app/assessment/shared/services/athena.service.ts (2)
getTextFeedbackSuggestions(94-124)getProgrammingFeedbackSuggestions(133-158)
src/main/webapp/app/assessment/shared/services/athena.service.ts (1)
src/main/webapp/app/app.constants.ts (1)
PROFILE_ATHENA(66-66)
🪛 GitHub Check: client-tests
src/test/javascript/spec/component/exercises/shared/feedback/preliminary-feedback-options.component.spec.ts
[failure] 52-52:
Parameter 'result' implicitly has an 'any' type.
[failure] 52-52:
Property 'isAthenaEnabled$' does not exist on type 'ExercisePreliminaryFeedbackOptionsComponent'. Did you mean 'isAthenaEnabled'?
[failure] 51-51:
Property 'isAthenaEnabled$' does not exist on type 'ExercisePreliminaryFeedbackOptionsComponent'. Did you mean 'isAthenaEnabled'?
[failure] 46-46:
Argument of type 'Observable' is not assignable to parameter of type 'boolean'.
[failure] 21-21:
No overload matches this call.
src/main/webapp/app/programming/shared/lifecycle/programming-exercise-lifecycle.component.spec.ts
[failure] 417-417:
Argument of type 'Observable' is not assignable to parameter of type 'boolean'.
[failure] 405-405:
Property 'isAthenaEnabled$' does not exist on type 'ProgrammingExerciseLifecycleComponent'. Did you mean 'isAthenaEnabled'?
[failure] 400-400:
Argument of type 'Observable' is not assignable to parameter of type 'boolean'.
src/main/webapp/app/exercise/feedback-suggestion/feedback-suggestion-option.component.spec.ts
[failure] 6-6:
'MockProvider' is declared but its value is never read.
🪛 GitHub Check: client-tests-selected
src/test/javascript/spec/component/exercises/shared/feedback/preliminary-feedback-options.component.spec.ts
[failure] 52-52:
Parameter 'result' implicitly has an 'any' type.
[failure] 52-52:
Property 'isAthenaEnabled$' does not exist on type 'ExercisePreliminaryFeedbackOptionsComponent'. Did you mean 'isAthenaEnabled'?
[failure] 51-51:
Property 'isAthenaEnabled$' does not exist on type 'ExercisePreliminaryFeedbackOptionsComponent'. Did you mean 'isAthenaEnabled'?
[failure] 46-46:
Argument of type 'Observable' is not assignable to parameter of type 'boolean'.
[failure] 21-21:
No overload matches this call.
src/main/webapp/app/programming/shared/lifecycle/programming-exercise-lifecycle.component.spec.ts
[failure] 417-417:
Argument of type 'Observable' is not assignable to parameter of type 'boolean'.
[failure] 405-405:
Property 'isAthenaEnabled$' does not exist on type 'ProgrammingExerciseLifecycleComponent'. Did you mean 'isAthenaEnabled'?
[failure] 400-400:
Argument of type 'Observable' is not assignable to parameter of type 'boolean'.
src/main/webapp/app/exercise/feedback-suggestion/feedback-suggestion-option.component.spec.ts
[failure] 6-6:
'MockProvider' is declared but its value is never read.
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Build and Push Docker Image / Build Docker Image for ls1intum/artemis
- GitHub Check: Build and Push Docker Image / Build Docker Image for ls1intum/artemis
- GitHub Check: server-tests
- GitHub Check: Analyse
🔇 Additional comments (104)
src/main/webapp/i18n/en/textExercise.json (1)
48-48: LGTM: Localization cleanup aligns with architectural changes.The removal of preliminary feedback request-related translation entries is consistent with the shift from boolean flags to the new module-based approach for handling preliminary feedback.
src/main/webapp/app/text/manage/text-exercise/update/text-exercise-update.component.ts (2)
46-46: LGTM: Proper import of preliminary feedback component.The import statement follows Angular conventions and the established pattern for shared exercise components.
72-72: LGTM: Component properly added to imports array.The ExercisePreliminaryFeedbackOptionsComponent is correctly included in the standalone component's imports array, enabling the template to use the preliminary feedback configuration UI.
src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingAssessmentIntegrationTest.java (1)
1007-1007: LGTM: Property renamed to reflect updated domain model.The change from
allowFeedbackRequeststoallowManualFeedbackRequestsaligns with the refactoring that distinguishes manual feedback requests from Athena feedback modules. The test logic remains unchanged.src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportService.java (2)
226-226: LGTM: Clear comment explaining extended logic.The comment appropriately clarifies that the disabling logic applies to both feedback suggestion and preliminary feedback modules for exam exercises.
228-228: LGTM: Consistent handling of preliminary feedback modules.The addition of
setPreliminaryFeedbackModule(null)mirrors the existing pattern for feedback suggestion modules, ensuring exam exercises don't inherit preliminary feedback modules during import operations.src/main/java/de/tum/cit/aet/artemis/athena/domain/ModuleType.java (1)
3-5: LGTM! Clean enum implementation.The enum correctly defines the two module types with proper Java naming conventions. This provides a clear type-safe way to distinguish between feedback suggestion and preliminary feedback modules throughout the codebase.
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingSubmissionResource.java (1)
385-385: LGTM! Consistent property rename.The change from
getAllowFeedbackRequests()togetAllowManualFeedbackRequests()aligns with the broader refactoring to distinguish manual feedback requests from preliminary feedback modules. The conditional logic remains intact.src/main/webapp/app/assessment/shared/assessment-dashboard/exercise-dashboard/exercise-assessment-dashboard.component.spec.ts (1)
565-565: LGTM! Test update aligns with backend changes.The property rename from
allowFeedbackRequeststoallowManualFeedbackRequestsin the test setup correctly reflects the backend model changes and ensures test compatibility.src/main/webapp/i18n/de/textExercise.json (1)
48-48: Approve removal of obsolete localization keys.
The old entries forallowPreliminaryAthenaFeedbackRequests*are no longer needed, as preliminary feedback is now handled via the new feedback section.src/main/webapp/app/assessment/shared/assessment-dashboard/exercise-dashboard/exercise-assessment-dashboard.component.ts (1)
369-372: Property rename toallowManualFeedbackRequestsis correct.
This update replaces the oldallowFeedbackRequestsflag to specifically gate manual feedback requests, distinct from the new preliminary-feedback modules.src/main/webapp/app/text/manage/text-exercise/update/text-exercise-update.component.html (1)
219-222: Verify translation existence.
You added:<h3 jhiTranslate="artemisApp.assessment.feedback" id="artemisApp.exercise.sections.feedback"></h3> @if (!isExamMode) { <jhi-exercise-preliminary-feedback-options [exercise]="textExercise" [dueDate]="textExercise.dueDate" /> }Ensure that the key
artemisApp.assessment.feedbackis defined in all locale files (e.g.,i18n/en/*.jsonandi18n/de/*.json) so the header renders properly.src/main/java/de/tum/cit/aet/artemis/exercise/service/SubmissionService.java (1)
694-694: LGTM! Method name change improves clarity.The rename from
getAllowFeedbackRequests()togetAllowManualFeedbackRequests()better reflects the distinction between manual feedback requests and Athena preliminary feedback modules, aligning with the overall refactoring goals.src/main/java/de/tum/cit/aet/artemis/programming/domain/ProgrammingExercise.java (1)
628-628: LGTM! Consistent method name improvements.Both changes from
getAllowFeedbackRequests()togetAllowManualFeedbackRequests()are consistent with the broader refactoring and improve semantic clarity in the context of checking manual feedback request permissions.Also applies to: 722-722
src/main/webapp/app/assessment/shared/entities/athena.model.ts (1)
1-4: LGTM! Well-structured enum following TypeScript conventions.The
ModuleTypeenum is properly implemented with clear naming and explicit string values. It follows TypeScript/Angular conventions and aligns with the backend implementation to support the new preliminary feedback module feature.src/main/java/de/tum/cit/aet/artemis/athena/api/AthenaFeedbackApi.java (1)
41-41: LGTM! Correct boolean inversion for parameter semantic change.The negation of
isGradedto!isGradedcorrectly handles the semantic change fromisGradedtoisPreliminaryin the service layer. When feedback is graded (final), preliminary feedback should be false, and vice versa. The consistent application across all three feedback suggestion methods maintains the expected behavior.Also applies to: 46-46, 50-50
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingAssessmentService.java (1)
138-139: LGTM! Property rename aligns with the new feedback model.The change from
getAllowFeedbackRequests()togetAllowManualFeedbackRequests()is consistent with the broader refactoring to distinguish between manual feedback requests and Athena preliminary feedback modules. The logic remains unchanged and correctly handles manual feedback request resolution.src/main/webapp/app/assessment/manage/assessment-warning/assessment-warning.component.ts (1)
50-50: LGTM! Frontend property update maintains consistency with backend changes.The property rename from
allowFeedbackRequeststoallowManualFeedbackRequestscorrectly aligns the frontend component with the backend model changes. The warning logic remains intact and will continue to function as expected for manual feedback requests.src/main/webapp/app/core/course/overview/exercise-details/student-actions/exercise-details-student-actions.component.html (2)
133-135:✅ Verification successful
Verify the logic change for showing feedback request buttons.
The condition was simplified by removing the
exercise.allowFeedbackRequestscheck. This means the feedback request button will now be shown for all programming exercises with graded participation, regardless of the exercise's feedback request settings.Please verify this logic change is intentional and aligns with the new feedback model. The button might now appear even when feedback requests should be disabled, potentially causing user confusion.
🏁 Script executed:
#!/bin/bash # Search for the request-feedback-button component to understand its internal logic ast-grep --pattern 'export class $_FeedbackButton$_Component { $$$ }' # Check if the component has its own validation logic for when to enable the button rg -A 10 -B 5 "request-feedback-button"Length of output: 37048
🏁 Script executed:
#!/bin/bash # Check if the RequestFeedbackButtonComponent uses exercise.allowFeedbackRequests rg -n "allowFeedbackRequests" src/main/webapp/app/core/course/overview/exercise-details/request-feedback-button/request-feedback-button.component.ts # Dump the beginning of the component to inspect initialization logic sed -n '1,200p' src/main/webapp/app/core/course/overview/exercise-details/request-feedback-button/request-feedback-button.component.tsLength of output: 9548
🏁 Script executed:
#!/bin/bash # Inspect the request-feedback-button component template sed -n '1,200p' src/main/webapp/app/core/course/overview/exercise-details/request-feedback-button/request-feedback-button.component.htmlLength of output: 3657
Encapsulate feedback visibility in RequestFeedbackButtonComponent
The
allowFeedbackRequestscheck was intentionally removed from the host template becauseRequestFeedbackButtonComponentnow manages its own visibility via therequestFeedbackEnabledflag (driven byexercise.allowManualFeedbackRequestsfor manual requests orexercise.preliminaryFeedbackModulewhen Athena is active). The component template wraps its buttons in an@if (!isExamExercise && requestFeedbackEnabled)guard, so the host-level duplication of that check is no longer necessary. Existing unit tests cover both programming and text/modeling scenarios, confirming the component still respects the original enablement rules.No further changes required.
225-233:✅ Verification successful
Verify the simplified condition for text/modeling exercise feedback requests.
The condition was simplified by removing the
exercise.allowFeedbackRequestscheck, now only requiringathenaEnabled, exercise type, andbeforeDueDate. This change should be verified to ensure it aligns with the intended behavior of the preliminary feedback system.Confirm that this simplification is correct and won't show the feedback request button inappropriately:
🏁 Script executed:
#!/bin/bash # Check if there are any related validation checks in the component logic rg -A 5 -B 5 "athenaEnabled|preliminaryFeedback" --type ts # Look for any access control logic in the request feedback button component ast-grep --pattern 'class $_RequestFeedbackButton$_ { $$$ $_method($_) { $$$ } $$$ }'Length of output: 36605
🏁 Script executed:
#!/bin/bash # Inspect the RequestFeedbackButtonComponent template for *ngIf or disabled logic TEMPLATE_PATH="src/main/webapp/app/core/course/overview/exercise-details/request-feedback-button/request-feedback-button.component.html" echo "Template *ngIf occurrences:" rg -n "ngIf" "$TEMPLATE_PATH" || true echo -e "\nFull template (first 200 lines):" sed -n '1,200p' "$TEMPLATE_PATH" # Inspect the TS file for requestFeedbackEnabled and ngOnInit TS_PATH="src/main/webapp/app/core/course/overview/exercise-details/request-feedback-button/request-feedback-button.component.ts" echo -e "\nngOnInit and Input definitions:" rg -n -C3 "ngOnInit" "$TS_PATH" || true echo -e "\nrequestFeedbackEnabled usage:" rg -n -C3 "requestFeedbackEnabled" "$TS_PATH" || trueLength of output: 5754
🏁 Script executed:
#!/bin/bash # Display context around feedback request button in student-actions template FILE="src/main/webapp/app/core/course/overview/exercise-details/student-actions/exercise-details-student-actions.component.html" echo "Lines 180–260 of student-actions.component.html:" sed -n '180,260p' "$FILE"Length of output: 4435
Condition check is correct and no changes needed
The
jhi-request-feedback-buttoncomponent itself enforces both Athena-based (exercise.preliminaryFeedbackModule) and manual (exercise.allowManualFeedbackRequests) gates in its ownngOnInitand template. The parent view only needs to mount the component when Athena is active, the exercise is TEXT or MODELING, and it’s before the due date. Removing the extraexercise.allowFeedbackRequestscheck here won’t expose the button in unintended cases, since the button stays hidden internally when requests aren’t actually allowed.src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSendingService.java (1)
98-102: LGTM! Clear refactoring for Athena module type support.The changes correctly update the URL construction to explicitly specify the exercise type and feedback suggestion module, aligning with the new architecture that distinguishes between different Athena module types. The added comment clarifies the scope of this service.
src/main/webapp/app/programming/manage/update/programming-exercise-update.component.spec.ts (2)
1255-1258: LGTM! Test comments updated for new property names.The comment correctly reflects the property renaming from
allowFeedbackRequeststoallowManualFeedbackRequestsand addition of the new Athena module properties.
1282-1284: LGTM! Test data properly updated for new domain model.The test data correctly includes the new
allowManualFeedbackRequests,feedbackSuggestionModule, andpreliminaryFeedbackModuleproperties, ensuring comprehensive test coverage for the refactored functionality.src/main/webapp/app/core/course/overview/exercise-details/request-feedback-button/request-feedback-button.component.ts (1)
69-73: LGTM! Proper conditional logic for feedback request enablement.The implementation correctly distinguishes between Athena-enabled environments (using
preliminaryFeedbackModule) and non-Athena environments (usingallowManualFeedbackRequests). The fallback tofalsewhenallowManualFeedbackRequestsis undefined is appropriate defensive programming.src/main/webapp/app/text/manage/detail/text-exercise-detail.component.ts (1)
112-113: LGTM! Clean implementation of the new module type distinction.The changes correctly implement the separation between feedback suggestions and preliminary feedback modules. The boolean conversion using
!!is appropriate, and the updated translation keys align with the new terminology.src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaSubmissionSendingService.java (1)
129-131:✅ Verification successful
Verify the updated method signature for getAthenaModuleUrl.
The URL construction has been updated to use
exercise.getExerciseType()andexercise.getFeedbackSuggestionModule()instead of passing the entire exercise. Verify that this method signature change is consistent across the codebase.
🏁 Script executed:
#!/bin/bash # Description: Verify the method signature change for getAthenaModuleUrl in AthenaModuleService # Expected: Method should accept ExerciseType and String parameters # Search for the method definition ast-grep --pattern $'getAthenaModuleUrl($$$) { $$$ }' # Search for all usages of getAthenaModuleUrl rg -A 2 "getAthenaModuleUrl"Length of output: 3716
Confirmed: getAthenaModuleUrl signature and usage are consistent
- AthenaModuleService now defines
public String getAthenaModuleUrl(ExerciseType exerciseType, String module)- All invocations in
- AthenaSubmissionSendingService
- AthenaFeedbackSuggestionsService
- AthenaSubmissionSelectionService
- AthenaFeedbackSendingService
passexercise.getExerciseType()and the module name String.- No remaining calls use an outdated signature.
src/main/webapp/app/exercise/feedback-suggestion/exercise-feedback-suggestion-options.component.html (4)
8-8: LGTM! Improved checkbox logic with showDropdownList condition.The updated checkbox condition
showDropdownList || !!this.exercise.feedbackSuggestionModuleprovides better UX by allowing the dropdown to be visible when toggling the checkbox, even before a module is selected.
17-19: LGTM! Translation keys updated to match new terminology.The translation key changes from 'feedbackSuggestionsEnabled' to 'allowFeedbackSuggestions' align with the updated terminology throughout the application and improve clarity.
21-21: LGTM! Consistent conditional rendering logic.The conditional rendering for the dropdown now matches the checkbox condition, ensuring UI consistency.
23-23: LGTM! Clearer label for module type distinction.The label change from "Athena Module" to "Athena Feedback Suggestions Module" provides better clarity and distinguishes this dropdown from other potential Athena module dropdowns (like preliminary feedback modules).
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaSubmissionSelectionService.java (1)
86-88: LGTM! Improved clarity with explicit parameter passing.The change to pass
exercise.getExerciseType()andexercise.getFeedbackSuggestionModule()explicitly instead of the whole exercise object improves code clarity and aligns with the module type distinction. The added comment clarifies that this service specifically handles feedback suggestions.src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.ts (2)
581-581: LGTM! Improved naming for manual feedback requests.The change from
allowFeedbackRequeststoallowManualFeedbackRequestsprovides better clarity and distinguishes manual feedback requests from Athena-based feedback modules.
588-589: LGTM! Added visibility for Athena feedback modules.The new boolean details for feedback suggestions and preliminary feedback modules provide clear visibility into the available Athena modules, aligning with the module type distinction introduced in this PR.
src/main/webapp/app/modeling/manage/update/modeling-exercise-update.component.html (1)
251-254: LGTM! Properly implemented preliminary feedback section.The new feedback section follows Angular best practices with proper conditional rendering and component input binding. The structure is consistent with other sections in the template.
src/main/webapp/app/programming/shared/lifecycle/programming-exercise-lifecycle.component.ts (4)
25-25: LGTM! Added import for preliminary feedback component.The import addition is necessary for the new preliminary feedback functionality and follows TypeScript import conventions.
39-39: LGTM! Added component to imports array.The component addition to the imports array is correctly placed and follows Angular module patterns.
158-164: LGTM! Consistent method and property renaming.The method rename from
toggleFeedbackRequests()totoggleManualFeedbackRequests()and the property updates consistently reflect the distinction between manual feedback requests and Athena-based feedback modules. The logic remains unchanged, which is appropriate.
180-180: LGTM! Consistent property naming update.The change from
allowFeedbackRequeststoallowManualFeedbackRequestsmaintains consistency with the refactoring throughout the component and improves code clarity.src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java (3)
46-46: Import addition looks good.The ModuleType import is correctly added to support the new explicit module type checking functionality.
259-262: Excellent separation of module type access checks.The implementation correctly handles both module types with explicit access validation. The pattern of setting the module to null when access is denied provides proper security and fallback behavior.
356-359: Consistent access check implementation in update method.The update method correctly mirrors the create method's access check pattern for both module types, ensuring consistent security behavior throughout the exercise lifecycle.
src/test/java/de/tum/cit/aet/artemis/athena/AthenaResourceIntegrationTest.java (3)
3-7: Import statements correctly updated for module type separation.The import changes properly reflect the new distinction between general test modules and suggestion-specific test modules, aligning with the broader refactoring to separate feedback suggestions from preliminary feedback.
257-291: Test setup properly uses suggestion-specific modules.The tests correctly:
- Use suggestion-specific module constants for exercise configuration
- Include the boolean parameter (false) in mock expectations to indicate non-preliminary feedback context
- Maintain comprehensive test coverage for the feedback suggestion functionality
This ensures the tests accurately reflect the new module type handling.
303-331: Mock provider calls consistently updated.All mock provider calls properly include the boolean parameter to distinguish the feedback context, ensuring tests remain accurate with the new API signature.
src/main/webapp/app/exercise/shared/entities/exercise/exercise.model.ts (4)
90-90: Property renaming improves clarity.The rename from
allowFeedbackRequeststoallowManualFeedbackRequestsbetter distinguishes between manual feedback requests and the new automatic preliminary feedback functionality.
132-132: New property addition aligns with backend changes.The
preliminaryFeedbackModuleproperty correctly supports the new preliminary feedback functionality, maintaining consistency with the backend domain model.
163-163: Constructor update maintains consistency.The constructor properly initializes the renamed property with the same default value (false).
283-285: Import reset logic properly handles both module types.The resetForImport function correctly:
- Updates the property name reference
- Explicitly sets both feedback module types to undefined
- Ensures clean state for imported exercises
src/main/webapp/i18n/en/exercise.json (1)
176-179: Translation keys properly distinguish feedback functionalities.The new translation keys clearly separate:
allowFeedbackSuggestions: For Athena-generated suggestions that assist tutorsallowFeedbackRequests: For student-initiated preliminary feedback requestsThe tooltips provide clear explanations of each feature's purpose, improving user understanding of the distinct functionalities.
src/main/webapp/app/core/course/overview/exercise-details/student-actions/exercise-details-student-actions.component.spec.ts (3)
28-28: LGTM: Proper import for Athena profile constant.The import of
PROFILE_ATHENAis correctly placed and follows the existing import pattern.
516-535: Well-structured test for text exercises feedback button visibility.The test correctly verifies that the request feedback button is displayed for text exercises when the Athena profile is active and before the due date. The setup and assertions are appropriate.
558-574: Programming exercise test correctly handles different behavior.The test appropriately handles the different behavior for programming exercises where the feedback button is shown with graded participation regardless of Athena profile or due date. Good implementation.
src/main/webapp/i18n/de/exercise.json (1)
176-179: Excellent German translations following informal addressing guidelines.The new translations correctly distinguish between feedback suggestions and feedback requests. The German text follows the required informal addressing ("du/dein") and uses appropriate technical terminology. The tooltip explanations are clear and helpful.
src/main/java/de/tum/cit/aet/artemis/exercise/repository/ExerciseRepository.java (2)
599-613: Good clarification comment for existing method.The added comment clarifies that this method is specifically for "restricted feedback suggestion module", which helps distinguish it from the new preliminary feedback method.
615-630: Well-implemented repository method following established patterns.The new method correctly follows the same pattern as the existing feedback suggestion method:
- Proper use of
@Transactionaland@Modifyingannotations- Consistent parameter naming and types
- Correct JPQL syntax for updating the
preliminaryFeedbackModulefield- Clear method documentation
The implementation maintains consistency with the existing codebase.
src/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.java (4)
42-42: Correct import for ModuleType enum.The import statement for
ModuleTypeis properly placed and necessary for the enhanced Athena module type handling.
238-241: Excellent separation of module type handling in create method.The code correctly performs separate access checks for both
FEEDBACK_SUGGESTIONSandPRELIMINARY_FEEDBACKmodule types. The use ofifPresentOrElseensures that if Athena API is not available or access is denied, the corresponding module is set to null, maintaining security.
295-298: Consistent module type handling in update method.The update method correctly mirrors the same pattern as the create method, ensuring both module types are validated independently. This maintains consistency and security across all operations.
541-556: Robust exception handling for import method.The import method includes comprehensive error handling for both module types:
- Clear documentation explaining the behavior
- Separate try-catch blocks for each module type
- Graceful degradation by setting modules to null when access is denied
- Maintains security while providing good user experience
This approach ensures the import operation can proceed even if one module type is restricted.
src/main/webapp/app/core/course/overview/exercise-details/request-feedback-button/request-feedback-button.component.spec.ts (2)
67-78: LGTM: Clean refactoring to support new feedback module architecture.The helper function correctly reflects the domain model changes, replacing the boolean
allowFeedbackRequestswith the stringpreliminaryFeedbackModuleproperty. The default value of'module'ensures backward compatibility for existing tests.
276-288: LGTM: Good test coverage for manual feedback requests.This test properly verifies the behavior when Athena is disabled but manual feedback requests are enabled, ensuring the component renders the appropriate anchor element.
src/test/javascript/spec/component/exercises/shared/feedback/preliminary-feedback-options.component.spec.ts (1)
21-25:⚠️ Potential issueFix MockProvider configuration and type mismatches.
The MockProvider setup has issues that are causing TypeScript compilation errors:
- The
isEnabledmethod should return an Observable to match the actual service- There's a type mismatch in the mock configuration
Apply this diff to fix the MockProvider setup:
- MockProvider(AthenaService, { - isEnabled: () => of(true), - }), + MockProvider(AthenaService, { + isEnabled: () => of(true), + getAvailableModules: () => of([]), + }),Likely an incorrect or invalid review comment.
🧰 Tools
🪛 GitHub Check: client-tests
[failure] 21-21:
No overload matches this call.🪛 GitHub Check: client-tests-selected
[failure] 21-21:
No overload matches this call.src/main/java/de/tum/cit/aet/artemis/athena/api/AthenaApi.java (3)
11-11: LGTM: Appropriate import for module type support.The import correctly adds the
ModuleTypeenum needed for the enhanced method signature.
48-50: LGTM: Method signature correctly enhanced for module type filtering.The addition of the
ModuleTypeparameter allows for proper distinction between feedback suggestion and preliminary feedback modules, aligning with the PR objectives.
57-57: LGTM: Method name accurately reflects expanded scope.The rename from
revokeAccessToRestrictedFeedbackSuggestionModulestorevokeAccessToRestrictedFeedbackModulescorrectly indicates that the method now handles both feedback suggestion and preliminary feedback modules.src/main/webapp/app/assessment/shared/services/athena.service.ts (2)
13-14: LGTM: Appropriate imports for enhanced functionality.The new imports correctly support HTTP parameter handling and module type filtering.
36-49: LGTM: Excellent implementation of module type filtering.The enhancement correctly implements optional module type filtering using HTTP parameters, maintaining backward compatibility while enabling the new functionality. The conditional parameter construction is clean and efficient.
src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.html (1)
1-33: LGTM! Clean Angular template implementation.The template correctly uses the new Angular @if and @for syntax as required by the coding guidelines. The conditional logic for checkbox state and dropdown visibility is well-structured.
src/main/webapp/app/programming/shared/lifecycle/programming-exercise-lifecycle.component.html (2)
89-89: Good refactoring to clarify manual vs. Athena feedback requests.The change from
allowFeedbackRequeststoallowManualFeedbackRequestsprovides better semantic clarity in the context of distinguishing manual feedback from Athena-based feedback.
206-224: Well-structured conditional logic for feedback options.The implementation properly separates Athena-enabled functionality from manual feedback with clear conditional branching. The component properly delegates to specialized components when Athena is enabled while providing a fallback manual checkbox.
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseExportImportResource.java (2)
247-262: Excellent defensive programming for Athena module access validation.The implementation properly validates access to both module types independently, gracefully handling service unavailability and access denial by disabling the corresponding functionality. This prevents import failures while maintaining security boundaries.
45-45: LGTM! Proper import addition for new enum.The ModuleType import is correctly placed and follows the existing import organization pattern.
src/test/java/de/tum/cit/aet/artemis/exercise/participation/ParticipationIntegrationTest.java (9)
3-5: Import new preliminary feedback module constants
The tests now importATHENA_MODULE_*_PRELIMINARY_TESTto assign the new preliminary feedback modules.
667-671: Configure programming exercise with preliminary feedback module and mock
Correctly sets the newpreliminaryFeedbackModuleand callsmockGetFeedbackSuggestionsAndExpect("programming", true)to simulate preliminary feedback context.
693-693: Verify broadcasting of new results for programming preliminary feedback
The test now asserts thatresultWebsocketService.broadcastNewResultis invoked twice, matching the new feedback flow.
713-717: Configure programming exercise for graded feedback with preliminary module
Similarly sets thepreliminaryFeedbackModuleand mocksmockGetFeedbackSuggestionsAndExpect("programming", true)in the graded feedback success scenario.
738-738: Verify broadcasting of new results for graded programming feedback
EnsuresbroadcastNewResultis called twice, consistent with the preliminary feedback integration.
758-762: Configure text exercise with preliminary feedback module and mock
SetstextExercise.setPreliminaryFeedbackModule(ATHENA_MODULE_TEXT_PRELIMINARY_TEST)and mocksmockGetFeedbackSuggestionsAndExpect("text", true)for the text feedback success path.
798-802: Configure modeling exercise with preliminary feedback module and mock
SetsmodelingExercise.setPreliminaryFeedbackModule(ATHENA_MODULE_MODELING_PRELIMINARY_TEST)and mocksmockGetFeedbackSuggestionsAndExpect("modeling", true)for modeling feedback success.
836-839: Configure programming exercise for Athena failure scenario
Correctly sets the preliminary module and mocksmockGetFeedbackSuggestionsWithFailure("programming", true)to test the failure path.
880-884: Configure text exercise for Athena failure scenario
Assigns the preliminary module and invokesmockGetFeedbackSuggestionsWithFailure("text", true)to simulate a text feedback failure.src/test/java/de/tum/cit/aet/artemis/athena/service/connectors/AthenaFeedbackSuggestionsServiceTest.java (4)
3-8: LGTM! Module constants follow consistent naming pattern.The new module constants properly distinguish between feedback suggestions and preliminary feedback modules for each exercise type.
50-52: Good addition of modeling exercise support.The modeling exercise setup follows the same pattern as text and programming exercises, maintaining consistency across all exercise types.
Also applies to: 61-64, 82-89
94-114: Correct handling of backward compatibility.The existing tests properly maintain their original behavior by passing
isPreliminary=false, ensuring that feedback suggestions continue to work as before.
116-139: Well-structured tests for preliminary feedback functionality.The new tests properly verify preliminary feedback for both text and programming exercises with the correct USER role for student scenarios.
src/main/webapp/app/exercise/feedback-suggestion/exercise-feedback-suggestion-options.component.ts (2)
1-1: Good implementation of change detection and lifecycle hooks.The addition of
AfterViewCheckedandChangeDetectorRefproperly handles async module loading and ensures the UI updates correctly. ThengAfterViewCheckedmethod appropriately manages dropdown visibility based on control state.Also applies to: 20-20, 24-24, 43-43, 46-46, 60-65
39-39: Well-implemented toggle behavior with proper state management.The implementation correctly:
- Controls dropdown visibility based on checkbox state
- Ensures mutual exclusivity by disabling manual feedback requests when suggestions are enabled
- Follows Angular conventions with property naming and event handling
Also applies to: 88-95
src/test/java/de/tum/cit/aet/artemis/core/connector/AthenaRequestMockProvider.java (2)
55-77: Well-organized module constants for test scenarios.The module constants are properly organized with clear distinctions between SUGGESTIONS and PRELIMINARY variants for each exercise type, making test scenarios more explicit and maintainable.
112-121: Clean implementation of module name generation and mock updates.The changes effectively:
- Introduce a helper method that reduces code duplication
- Update mock methods to support both feedback types consistently
- Maintain clear JavaDoc documentation for the new parameters
Also applies to: 210-216, 248-254
src/main/webapp/app/programming/shared/lifecycle/programming-exercise-lifecycle.component.spec.ts (3)
21-22: LGTM: Import updates look correct.The import path correction for
MockProfileServiceand addition ofAthenaServicealign with the new preliminary feedback functionality.
155-162: LGTM: Property rename correction.The test correctly updates from
allowFeedbackRequeststoallowManualFeedbackRequests, aligning with the domain model changes.
394-427: Comprehensive test coverage for preliminary feedback rendering.These tests provide good coverage for the new preliminary feedback functionality, ensuring proper conditional rendering based on Athena service state and exam mode.
🧰 Tools
🪛 GitHub Check: client-tests
[failure] 417-417:
Argument of type 'Observable' is not assignable to parameter of type 'boolean'.
[failure] 405-405:
Property 'isAthenaEnabled$' does not exist on type 'ProgrammingExerciseLifecycleComponent'. Did you mean 'isAthenaEnabled'?
[failure] 400-400:
Argument of type 'Observable' is not assignable to parameter of type 'boolean'.🪛 GitHub Check: client-tests-selected
[failure] 417-417:
Argument of type 'Observable' is not assignable to parameter of type 'boolean'.
[failure] 405-405:
Property 'isAthenaEnabled$' does not exist on type 'ProgrammingExerciseLifecycleComponent'. Did you mean 'isAthenaEnabled'?
[failure] 400-400:
Argument of type 'Observable' is not assignable to parameter of type 'boolean'.src/main/java/de/tum/cit/aet/artemis/exercise/domain/Exercise.java (4)
97-97: LGTM: Clear field rename for better semantics.The rename from
allowFeedbackRequeststoallowManualFeedbackRequestsprovides better clarity about the field's purpose.
139-140: LGTM: Well-designed new field with proper annotations.The
preliminaryFeedbackModulefield is properly annotated with@Nullableand follows the same pattern as the existingfeedbackSuggestionModulefield.
237-243: LGTM: Consistent getter/setter implementation.The getter and setter methods for
allowManualFeedbackRequestsfollow the established patterns in the class.
682-696: LGTM: Well-implemented preliminary feedback methods.The getter, setter, and helper method for preliminary feedback functionality are consistent with the existing
feedbackSuggestionModuleimplementation pattern.src/main/webapp/app/exercise/feedback-suggestion/feedback-suggestion-option.component.spec.ts (2)
112-113: LGTM: Updated test assertions for new property names.The test correctly checks the
showDropdownListstate and uses the renamedallowManualFeedbackRequestsproperty.Also applies to: 118-119
127-165: LGTM: Comprehensive enhanced test case.This test provides thorough coverage of the component behavior with different assessment types, including proper DOM element checking and state validation.
src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.ts (4)
1-17: LGTM: Modern Angular component with proper imports.The component uses Angular's modern signal-based inputs and has all necessary imports for the functionality.
18-34: LGTM: Well-structured component properties.The component follows good practices with readonly properties for constants and proper property initialization.
41-50: LGTM: Proper initialization with Athena service integration.The
ngOnInitmethod correctly fetches available modules with the preliminary feedback type and sets up the component state.
63-65: Consider consistency with programming exercise restrictions.The
inputControlsDisabledmethod only checks for readOnly and due date, but doesn't include programming exercise automatic assessment restrictions like the feedback suggestion component does. Consider if this is intentional or if similar restrictions should apply.Please verify if preliminary feedback should also be disabled for programming exercises with automatic assessment type, similar to how it's handled in the feedback suggestion component.
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaModuleService.java (1)
128-141: Good refactoring of the method signature!The change from accepting an
Exerciseobject toExerciseTypeand module name reduces coupling and makes the method more flexible. The switch statement properly handles all cases with appropriate error handling.src/main/java/de/tum/cit/aet/artemis/athena/web/AthenaResource.java (1)
219-221: Well-implemented optional module type filtering!The addition of the optional
ModuleTypeparameter to all three endpoint methods is consistent and follows Spring best practices with@RequestParam(required = false). This provides good flexibility for clients to filter modules by type.Also applies to: 232-234, 245-247
src/main/webapp/app/modeling/overview/modeling-submission/modeling-submission.component.html
Show resolved
Hide resolved
...ript/spec/component/exercises/shared/feedback/preliminary-feedback-options.component.spec.ts
Outdated
Show resolved
Hide resolved
...ain/webapp/app/programming/shared/lifecycle/programming-exercise-lifecycle.component.spec.ts
Outdated
Show resolved
Hide resolved
...ain/webapp/app/programming/shared/lifecycle/programming-exercise-lifecycle.component.spec.ts
Outdated
Show resolved
Hide resolved
...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
Show resolved
Hide resolved
github-project-automation
bot
moved this from Work In Progress
to Ready For Review
in Artemis Development
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/main/webapp/app/programming/shared/lifecycle/programming-exercise-lifecycle.component.spec.ts (1)
400-400:⚠️ Potential issueFix TypeScript type errors in mock return values.
The static analysis correctly identifies type mismatches where
Observable<boolean>is being passed instead ofboolean.Apply this diff to fix the type errors:
- jest.spyOn(athenaService, 'isEnabled').mockReturnValue(of(true)); + jest.spyOn(athenaService, 'isEnabled').mockReturnValue(true);- jest.spyOn(athenaService, 'isEnabled').mockReturnValue(of(false)); + jest.spyOn(athenaService, 'isEnabled').mockReturnValue(of(false));Also applies to: 417-417
src/test/javascript/spec/component/exercises/shared/feedback/preliminary-feedback-options.component.spec.ts (1)
44-53: Fix inconsistent mocking of isEnabled method.The test has conflicting mock configurations - MockProvider expects Observable but the spy returns a boolean.
Based on the MockProvider fix above, update the spy to be consistent:
jest.spyOn(athenaService, 'getAvailableModules').mockReturnValue(of()); - jest.spyOn(athenaService, 'isEnabled').mockReturnValue(true); + jest.spyOn(athenaService, 'isEnabled').mockReturnValue(of(true));And update the expectation to handle the Observable:
- expect(component.isAthenaEnabled).toBeDefined(); - expect(component.isAthenaEnabled).toBeTrue(); + expect(component.isAthenaEnabled).toBeDefined(); + component.isAthenaEnabled.subscribe((result: boolean) => { + expect(result).toBeTrue(); + });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/webapp/app/exercise/feedback-suggestion/feedback-suggestion-option.component.spec.ts(4 hunks)src/main/webapp/app/programming/shared/lifecycle/programming-exercise-lifecycle.component.spec.ts(5 hunks)src/test/javascript/spec/component/exercises/shared/feedback/preliminary-feedback-options.component.spec.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
`src/main/webapp/**/*.ts`: angular_style:https://angular.io/guide/styleguide;methods_in_html:false;lazy_loading:true;code_reuse:true;tests:meaningful;types:PascalCase;enums:PascalC...
src/main/webapp/app/exercise/feedback-suggestion/feedback-suggestion-option.component.spec.tssrc/main/webapp/app/programming/shared/lifecycle/programming-exercise-lifecycle.component.spec.ts
`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_logi...
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/exercises/shared/feedback/preliminary-feedback-options.component.spec.ts
🪛 GitHub Check: client-tests-selected
src/main/webapp/app/exercise/feedback-suggestion/feedback-suggestion-option.component.spec.ts
[failure] 17-17:
Cannot find module '@angular/platform-brisAthenaEnabledowser' or its corresponding type declarations.
src/main/webapp/app/programming/shared/lifecycle/programming-exercise-lifecycle.component.spec.ts
[failure] 417-417:
Argument of type 'Observable' is not assignable to parameter of type 'boolean'.
src/test/javascript/spec/component/exercises/shared/feedback/preliminary-feedback-options.component.spec.ts
[failure] 21-21:
No overload matches this call.
🪛 GitHub Check: client-tests
src/main/webapp/app/exercise/feedback-suggestion/feedback-suggestion-option.component.spec.ts
[failure] 17-17:
Cannot find module '@angular/platform-brisAthenaEnabledowser' or its corresponding type declarations.
src/main/webapp/app/programming/shared/lifecycle/programming-exercise-lifecycle.component.spec.ts
[failure] 417-417:
Argument of type 'Observable' is not assignable to parameter of type 'boolean'.
src/test/javascript/spec/component/exercises/shared/feedback/preliminary-feedback-options.component.spec.ts
[failure] 21-21:
No overload matches this call.
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Build and Push Docker Image / Build Docker Image for ls1intum/artemis
- GitHub Check: Build and Push Docker Image / Build Docker Image for ls1intum/artemis
- GitHub Check: server-tests
- GitHub Check: Analyse
🔇 Additional comments (12)
src/main/webapp/app/programming/shared/lifecycle/programming-exercise-lifecycle.component.spec.ts (5)
21-22: LGTM: Import statements are correct.The import statements for MockProfileService and AthenaService are properly added to support the new test functionality.
33-33: LGTM: AthenaService injection is properly implemented.The service declaration and injection are correctly set up to support the new test cases.
Also applies to: 77-77
155-162: LGTM: Test updated to reflect property name change.The test correctly uses the new
allowManualFeedbackRequestsproperty and the test name accurately reflects the functionality being tested.
368-379: LGTM: Feedback suggestion component rendering tests are well-structured.The tests properly verify conditional rendering of the
ExerciseFeedbackSuggestionOptionsComponentbased on exam mode and feedback suggestion settings.Also applies to: 381-392
394-409: Good test coverage for preliminary feedback component rendering.The tests effectively verify the conditional rendering logic for
ExercisePreliminaryFeedbackOptionsComponentbased on Athena service state, exam mode, and preliminary feedback settings. The use offakeAsyncandtickis appropriate for handling asynchronous behavior.Also applies to: 411-427
src/main/webapp/app/exercise/feedback-suggestion/feedback-suggestion-option.component.spec.ts (3)
5-6: LGTM: Import and provider updates enhance test capabilities.The addition of RxJS
of,MockDirective, translation-related imports, and providers properly supports the enhanced test scenarios.Also applies to: 14-16, 32-33
112-112: LGTM: Enhanced assertions improve test coverage.The new assertions for
showDropdownListandallowManualFeedbackRequestsprovide better verification of the component's state management.Also applies to: 118-119
127-165: Excellent comprehensive test scenario.This test effectively covers the complex interaction between assessment type changes and feedback suggestion controls. It properly:
- Sets up a realistic programming exercise scenario
- Mocks the Athena service response
- Verifies UI state changes through assessment type toggles
- Uses proper DOM querying to verify checkbox states
- Tests the complete cycle of enabling/disabling controls
The test logic is sound and provides valuable coverage for the component's behavior.
src/test/javascript/spec/component/exercises/shared/feedback/preliminary-feedback-options.component.spec.ts (4)
33-42: Well-structured initialization test.The test properly verifies module loading and availability state. Good use of specific assertions and proper setup.
55-71: Comprehensive input control testing.Both test cases effectively verify the input control disabling logic for different scenarios (programming exercises with automatic assessment and due date validation).
73-87: Good styling logic verification.The tests properly validate the conditional styling behavior for different exercise types and assessment modes.
89-106: Excellent toggle functionality testing.This test comprehensively covers the preliminary feedback toggle behavior, verifying both enabling and disabling scenarios with proper state management.
src/main/webapp/app/exercise/feedback-suggestion/feedback-suggestion-option.component.spec.ts
Outdated
Show resolved
Hide resolved
...ript/spec/component/exercises/shared/feedback/preliminary-feedback-options.component.spec.ts
Outdated
Show resolved
Hide resolved
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/main/webapp/app/exercise/feedback-suggestion/feedback-suggestion-option.component.spec.ts (1)
128-128: Fix TypeScript type error with ProfileService method call.The static analysis correctly identifies that
setActiveProfilesdoesn't exist on theProfileServiceinterface. The test is calling this method on what is actually aMockProfileServiceinstance.Cast the service to the mock type to fix the type error:
- profileService.setActiveProfiles(PROFILE_ATHENA); + (profileService as MockProfileService).setActiveProfiles([PROFILE_ATHENA]);Note: Also ensure the argument is an array as expected by the method signature.
🧰 Tools
🪛 GitHub Check: client-tests-selected
[failure] 128-128:
Property 'setActiveProfiles' does not exist on type 'ProfileService'.🪛 GitHub Check: client-tests
[failure] 128-128:
Property 'setActiveProfiles' does not exist on type 'ProfileService'.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/webapp/app/core/course/overview/exercise-details/student-actions/exercise-details-student-actions.component.spec.ts(2 hunks)src/main/webapp/app/exercise/feedback-suggestion/feedback-suggestion-option.component.spec.ts(3 hunks)src/main/webapp/app/programming/shared/lifecycle/programming-exercise-lifecycle.component.spec.ts(5 hunks)src/test/javascript/spec/helpers/mocks/service/mock-profile.service.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/webapp/app/core/course/overview/exercise-details/student-actions/exercise-details-student-actions.component.spec.ts
🧰 Additional context used
📓 Path-based instructions (2)
`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_logi...
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/helpers/mocks/service/mock-profile.service.ts
`src/main/webapp/**/*.ts`: angular_style:https://angular.io/guide/styleguide;methods_in_html:false;lazy_loading:true;code_reuse:true;tests:meaningful;types:PascalCase;enums:PascalC...
src/main/webapp/app/exercise/feedback-suggestion/feedback-suggestion-option.component.spec.tssrc/main/webapp/app/programming/shared/lifecycle/programming-exercise-lifecycle.component.spec.ts
🧬 Code Graph Analysis (3)
src/test/javascript/spec/helpers/mocks/service/mock-profile.service.ts (1)
src/main/webapp/app/core/layouts/profiles/profile-info.model.ts (1)
ProfileInfo(126-166)
src/main/webapp/app/exercise/feedback-suggestion/feedback-suggestion-option.component.spec.ts (2)
src/test/javascript/spec/helpers/mocks/service/mock-profile.service.ts (1)
MockProfileService(4-37)src/main/webapp/app/app.constants.ts (1)
PROFILE_ATHENA(66-66)
src/main/webapp/app/programming/shared/lifecycle/programming-exercise-lifecycle.component.spec.ts (1)
src/main/webapp/app/app.constants.ts (1)
PROFILE_ATHENA(66-66)
🪛 GitHub Check: client-tests-selected
src/test/javascript/spec/helpers/mocks/service/mock-profile.service.ts
[failure] 5-5:
Type '{ programmingLanguageFeatures: never[]; activeProfiles: never[]; activeModuleFeatures: never[]; testServer: false; }' is missing the following properties from type 'ProfileInfo': build, buildTimeoutDefault, buildTimeoutMax, buildTimeoutMin, and 21 more.
src/main/webapp/app/exercise/feedback-suggestion/feedback-suggestion-option.component.spec.ts
[failure] 128-128:
Property 'setActiveProfiles' does not exist on type 'ProfileService'.
src/main/webapp/app/programming/shared/lifecycle/programming-exercise-lifecycle.component.spec.ts
[failure] 398-398:
Property 'setActiveProfiles' does not exist on type 'ProfileService'.
🪛 GitHub Check: client-tests
src/test/javascript/spec/helpers/mocks/service/mock-profile.service.ts
[failure] 5-5:
Type '{ programmingLanguageFeatures: never[]; activeProfiles: never[]; activeModuleFeatures: never[]; testServer: false; }' is missing the following properties from type 'ProfileInfo': build, buildTimeoutDefault, buildTimeoutMax, buildTimeoutMin, and 21 more.
src/main/webapp/app/exercise/feedback-suggestion/feedback-suggestion-option.component.spec.ts
[failure] 128-128:
Property 'setActiveProfiles' does not exist on type 'ProfileService'.
src/main/webapp/app/programming/shared/lifecycle/programming-exercise-lifecycle.component.spec.ts
[failure] 398-398:
Property 'setActiveProfiles' does not exist on type 'ProfileService'.
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Build and Push Docker Image / Build Docker Image for ls1intum/artemis
- GitHub Check: Build .war artifact
- GitHub Check: Build and Push Docker Image / Build Docker Image for ls1intum/artemis
- GitHub Check: server-tests
- GitHub Check: Analyse
🔇 Additional comments (9)
src/main/webapp/app/programming/shared/lifecycle/programming-exercise-lifecycle.component.spec.ts (5)
21-21: Fix import path to use relative path.The import path has been corrected to use a relative path, which is good practice for test files.
22-23: Good addition of required imports for new functionality.The imports for
AthenaServiceandPROFILE_ATHENAare necessary for the new tests that verify Athena-related functionality.
158-165: Excellent update to reflect the renamed property.The test correctly reflects the renamed property from
allowFeedbackRequeststoallowManualFeedbackRequests, which aligns with the broader PR changes to distinguish between feedback types.
371-395: Well-structured tests for conditional component rendering.The new tests effectively verify that feedback-related components are conditionally rendered based on the appropriate flags and exam mode. The test structure follows Angular testing best practices.
413-429: Good test coverage for disabled Athena scenarios.This test effectively verifies that the preliminary feedback component is not rendered when Athena is disabled, while ensuring the manual feedback requests checkbox remains available. The test demonstrates good attention to edge cases.
src/main/webapp/app/exercise/feedback-suggestion/feedback-suggestion-option.component.spec.ts (4)
5-18: Excellent enhancement of imports and mocking utilities.The addition of necessary imports including
MockDirective,MockTranslateService, and platform browser utilities properly supports the enhanced test functionality. The imports are well-organized and follow Angular testing best practices.
32-34: Good use of proper mocking utilities in test configuration.The test configuration correctly uses
MockDirectivefor the translate directive and properly injects the mock services, following the coding guidelines for Angular testing.
105-122: Well-structured test for feedback suggestions toggling.The test effectively verifies the toggling behavior including the dropdown visibility state and proper module assignment. The expectations follow the coding guidelines using
toBeTrue()andtoBeFalse().
124-162: Comprehensive test with good DOM element verification.This test provides excellent coverage of the component's behavior across different assessment types, properly verifying both the component state and DOM element states. The test structure demonstrates good Angular testing practices.
🧰 Tools
🪛 GitHub Check: client-tests-selected
[failure] 128-128:
Property 'setActiveProfiles' does not exist on type 'ProfileService'.🪛 GitHub Check: client-tests
[failure] 128-128:
Property 'setActiveProfiles' does not exist on type 'ProfileService'.
src/test/javascript/spec/helpers/mocks/service/mock-profile.service.ts
Outdated
Show resolved
Hide resolved
...ain/webapp/app/programming/shared/lifecycle/programming-exercise-lifecycle.component.spec.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/webapp/app/exercise/feedback-suggestion/feedback-suggestion-option.component.spec.ts (1)
18-18: Import typo fix confirmed.This line fixes the previously identified typo in the import statement.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/webapp/app/exercise/feedback-suggestion/feedback-suggestion-option.component.spec.ts(3 hunks)src/main/webapp/app/programming/shared/lifecycle/programming-exercise-lifecycle.component.spec.ts(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/webapp/app/programming/shared/lifecycle/programming-exercise-lifecycle.component.spec.ts
🧰 Additional context used
📓 Path-based instructions (1)
`src/main/webapp/**/*.ts`: angular_style:https://angular.io/guide/styleguide;methods_in_html:false;lazy_loading:true;code_reuse:true;tests:meaningful;types:PascalCase;enums:PascalC...
src/main/webapp/app/exercise/feedback-suggestion/feedback-suggestion-option.component.spec.ts
🧬 Code Graph Analysis (1)
src/main/webapp/app/exercise/feedback-suggestion/feedback-suggestion-option.component.spec.ts (3)
src/test/javascript/spec/helpers/mocks/service/mock-profile.service.ts (1)
MockProfileService(4-22)src/main/webapp/app/app.constants.ts (1)
PROFILE_ATHENA(66-66)src/main/webapp/app/core/layouts/profiles/profile-info.model.ts (1)
ProfileInfo(126-166)
🪛 GitHub Check: client-tests-selected
src/main/webapp/app/exercise/feedback-suggestion/feedback-suggestion-option.component.spec.ts
[failure] 131-131:
Cannot find name 'ProfileInfo'.
🪛 GitHub Check: client-tests
src/main/webapp/app/exercise/feedback-suggestion/feedback-suggestion-option.component.spec.ts
[failure] 131-131:
Cannot find name 'ProfileInfo'.
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Build and Push Docker Image / Build Docker Image for ls1intum/artemis
- GitHub Check: Build and Push Docker Image / Build Docker Image for ls1intum/artemis
- GitHub Check: Build .war artifact
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: server-tests
- GitHub Check: Analyse
🔇 Additional comments (5)
src/main/webapp/app/exercise/feedback-suggestion/feedback-suggestion-option.component.spec.ts (5)
5-7: LGTM! Good import additions for enhanced testing.The added imports for
of,MockDirective,Exercise, andExerciseTypeare appropriate for the enhanced test functionality.
14-16: LGTM! Translation service mocking setup is correct.The imports for translation directive and service mocking are properly added to support the component's translation functionality in tests.
25-25: LGTM! Proper mock service setup.The spy instance declaration and mock provider configuration for the profile service follows Angular testing best practices.
Also applies to: 33-35, 45-45
111-111: LGTM! Enhanced test assertions improve coverage.The additional assertions for
showDropdownListandallowManualFeedbackRequestsprovide better verification of component state changes.Also applies to: 117-118
126-165: Well-structured comprehensive test case.The rewritten test provides excellent coverage of the component's behavior across different assessment types and states. The test logic correctly verifies:
- Input control states based on assessment type
- Dropdown visibility changes
- Checkbox enabled/disabled states
Once the ProfileInfo import is added, this test will provide robust verification of the component's functionality.
🧰 Tools
🪛 GitHub Check: client-tests-selected
[failure] 131-131:
Cannot find name 'ProfileInfo'.🪛 GitHub Check: client-tests
[failure] 131-131:
Cannot find name 'ProfileInfo'.
src/main/webapp/app/exercise/feedback-suggestion/feedback-suggestion-option.component.spec.ts
Show resolved
Hide resolved
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
…iminary-feedback-model
…ck-model' of https://github.com/ls1intum/Artemis into feature/programming-exercises/choose-preliminary-feedback-model
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/test/java/de/tum/cit/aet/artemis/athena/service/connectors/AthenaFeedbackSuggestionsServiceTest.java(6 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`src/test/java/**/*.java`: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_...
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/service/connectors/AthenaFeedbackSuggestionsServiceTest.java
🧬 Code Graph Analysis (1)
src/test/java/de/tum/cit/aet/artemis/athena/service/connectors/AthenaFeedbackSuggestionsServiceTest.java (1)
src/main/webapp/app/assessment/shared/services/athena.service.ts (2)
getTextFeedbackSuggestions(94-124)getProgrammingFeedbackSuggestions(133-158)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: client-tests
- GitHub Check: server-tests
🔇 Additional comments (5)
src/test/java/de/tum/cit/aet/artemis/athena/service/connectors/AthenaFeedbackSuggestionsServiceTest.java (5)
3-8: LGTM: Well-organized module constant imports.The imports clearly distinguish between suggestions and preliminary feedback modules for all exercise types (text, programming, modeling), which aligns well with the dual-module architecture introduced in this PR.
70-71: Excellent dual-module configuration setup.The setup correctly configures both feedback suggestion and preliminary feedback modules for each exercise type, which perfectly supports the new dual-module architecture. This ensures tests can verify behavior for both feedback types independently.
Also applies to: 76-77, 84-85
94-94: Parameter addition aligns with service method signature changes.The addition of the
falseparameter for preliminary feedback correctly indicates these tests are for feedback suggestions rather than preliminary feedback. The mock expectations are also properly updated to handle the boolean flag.Also applies to: 97-97
116-139: Well-designed preliminary feedback tests with appropriate user roles.The new preliminary feedback tests properly:
- Use
truefor the preliminary feedback flag- Test with "USER" role instead of "TA", which makes sense for student-facing preliminary feedback
- Cover both text and programming exercises
- Follow the same assertion patterns as the suggestion tests
This provides good coverage for the new preliminary feedback functionality.
143-143: ID conflict test updated appropriately.The test correctly uses the new method signature with the preliminary feedback flag. The choice to use
truefor preliminary feedback is reasonable since the test focuses on the conflict detection logic rather than the specific feedback type.Also applies to: 146-146
| import de.tum.cit.aet.artemis.modeling.domain.ModelingExercise; | ||
| import de.tum.cit.aet.artemis.modeling.domain.ModelingSubmission; | ||
| import de.tum.cit.aet.artemis.modeling.util.ModelingExerciseUtilService; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Setup for modeling exercises looks good, but tests are missing.
The modeling exercise and submission setup follows the same pattern as text and programming exercises, but there are no corresponding test methods that actually use these modeling objects.
Should I help generate the missing modeling exercise tests to complete the test coverage?
Also applies to: 50-51, 61-64
🏁 Script executed:
#!/bin/bash
# Search for any modeling-specific test methods that might exist elsewhere
ast-grep --pattern $'@Test
$$$
void $_(${MATCH}ModelingExercise$_) {
$$$
}'
# Also search for any references to modelingExercise or modelingSubmission in test methods
rg -A 5 -B 2 'modelingExercise|modelingSubmission' --type javaLength of output: 85948
Add missing modeling exercise tests in AthenaFeedbackSuggestionsServiceTest
The test class imports ModelingExercise, ModelingSubmission, and ModelingExerciseUtilService but doesn’t include any @Test methods that exercise modeling feedback suggestions. We should mirror the existing text- and programming-exercise tests by adding dedicated modeling tests.
• File: src/test/java/de/tum/cit/aet/artemis/athena/service/connectors/AthenaFeedbackSuggestionsServiceTest.java
– Around imports (lines 27–29): import setup is correct but no tests follow.
– Add @Test methods that invoke getModelingFeedbackSuggestions(...) with a ModelingExercise and ModelingSubmission created via ModelingExerciseUtilService.
– Also address the sections around lines 50–51 and 61–64 where text/programming tests exist—insert corresponding modeling variants there.
🤖 Prompt for AI Agents
In
src/test/java/de/tum/cit/aet/artemis/athena/service/connectors/AthenaFeedbackSuggestionsServiceTest.java
around lines 27-29, 50-51, and 61-64, the imports for modeling exercise classes
are present but there are no test methods exercising modeling feedback
suggestions. To fix this, add new @Test methods that create ModelingExercise and
ModelingSubmission instances using ModelingExerciseUtilService and call
getModelingFeedbackSuggestions(...) on them. Mirror the structure and assertions
of the existing text and programming exercise tests to ensure modeling feedback
suggestion logic is properly covered.
| athenaRequestMockProvider.mockGetFeedbackSuggestionsAndExpect("programming", false, jsonPath("$.exercise.id").value(programmingExercise.getId()), | ||
| jsonPath("$.exercise.title").value(programmingExercise.getTitle()), jsonPath("$.submission.id").value(programmingSubmission.getId()), | ||
| jsonPath("$.submission.repositoryUri") | ||
| .value("https://artemislocal.cit.tum.de/api/athena/public/programming-exercises/" + programmingExercise.getId() + "/submissions/3/repository")); | ||
| List<ProgrammingFeedbackDTO> suggestions = athenaFeedbackSuggestionsService.getProgrammingFeedbackSuggestions(programmingExercise, programmingSubmission, false); | ||
| assertThat(suggestions.getFirst().title()).isEqualTo("Not so good"); | ||
| assertThat(suggestions.getFirst().lineStart()).isEqualTo(3); | ||
| athenaRequestMockProvider.verify(); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
URL inconsistency between test environments.
The programming feedback suggestions test uses artemislocal.cit.tum.de (line 109) while the preliminary feedback test uses localhost (line 134). This inconsistency could indicate different test environments or could be unintentional.
Please verify if the URL difference is intentional for testing different environments or if it should be consistent.
🏁 Script executed:
#!/bin/bash
# Search for URL patterns in test files to understand the expected format
rg 'artemislocal\.cit\.tum\.de|localhost.*api/athena' --type java -A 2 -B 2
# Check if there are configuration differences for different test scenarios
rg 'repositoryUri' --type java -A 3 -B 3Length of output: 68729
🏁 Script executed:
#!/bin/bash
set -e
# Locate and display the test class to inspect its setup and properties
test_file=$(rg -l 'class AthenaFeedbackSuggestionsServiceTest' -g 'src/test/java/*.java')
echo "=== Test file: $test_file ==="
sed -n '1,200p' "$test_file"
# Locate and display the service implementation to see how the URL is constructed
service_file=$(rg -l 'class AthenaFeedbackSuggestionsService' -g 'src/main/java/*.java')
echo -e "\n=== Service file: $service_file ==="
rg -n 'getProgrammingFeedbackSuggestions' -n "$service_file"
sed -n '1,200p' "$service_file"Length of output: 250
🏁 Script executed:
#!/bin/bash
set -e
# Find and display the top of the test class to see how AthenaProperties/restTemplate are configured
test_file=$(fd -e java AthenaFeedbackSuggestionsServiceTest.java src/test/java)
echo "=== Test class: $test_file ==="
sed -n '1,80p' "$test_file"
# Find and display the service implementation to inspect URL construction logic
service_file=$(fd -e java AthenaFeedbackSuggestionsService.java src/main/java)
echo -e "\n=== Service class: $service_file ==="
rg -n 'getProgrammingFeedbackSuggestions' -n "$service_file"
sed -n '1,200p' "$service_file"Length of output: 17144
🏁 Script executed:
#!/bin/bash
set -e
# Locate and inspect AthenaRequestMockProvider to understand module constants
reqprov=$(fd -e java AthenaRequestMockProvider.java src)
echo "=== AthenaRequestMockProvider: $reqprov ==="
sed -n '1,200p' "$reqprov"
sed -n '200,350p' "$reqprov"
# Locate and inspect AthenaDTOConverterService to see how submission.repositoryUri is built
dto=$(fd -e java AthenaDTOConverterService.java src)
echo -e "\n=== AthenaDTOConverterService: $dto ==="
rg -n 'ofSubmission' -n "$dto"
sed -n '1,200p' "$dto"Length of output: 26168
🏁 Script executed:
#!/bin/bash
set -e
# Locate and display AbstractAthenaTest to inspect test property configuration
abstract_test=$(fd -e java AbstractAthenaTest.java src/test/java)
echo "=== AbstractAthenaTest: $abstract_test ==="
sed -n '1,200p' "$abstract_test"Length of output: 1248
🏁 Script executed:
#!/bin/bash
set -e
# Search for server.url and artemis.athena.url in configuration files
echo "== Main resources =="
rg -n 'server\.url' -n src/main/resources
rg -n 'artemis\.athena\.url' -n src/main/resources
echo -e "\n== Test resources =="
rg -n 'server\.url' -n src/test/resources
rg -n 'artemis\.athena\.url' -n src/test/resourcesLength of output: 103
Align host in programming preliminary feedback test
The testPreliminaryFeedbackProgramming method is still asserting a localhost-based URL, whereas the testFeedbackSuggestionsProgramming uses https://artemislocal.cit.tum.de. These should be consistent.
Please update the expected URI in testPreliminaryFeedbackProgramming:
File:
src/test/java/de/tum/cit/aet/artemis/athena/service/connectors/AthenaFeedbackSuggestionsServiceTest.java
Around line 134:
- .value("http://localhost/api/athena/public/programming-exercises/" + programmingExercise.getId() + "/submissions/3/repository"));
+ .value("https://artemislocal.cit.tum.de/api/athena/public/programming-exercises/" + programmingExercise.getId() + "/submissions/3/repository"));📝 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.
| athenaRequestMockProvider.mockGetFeedbackSuggestionsAndExpect("programming", false, jsonPath("$.exercise.id").value(programmingExercise.getId()), | |
| jsonPath("$.exercise.title").value(programmingExercise.getTitle()), jsonPath("$.submission.id").value(programmingSubmission.getId()), | |
| jsonPath("$.submission.repositoryUri") | |
| .value("https://artemislocal.cit.tum.de/api/athena/public/programming-exercises/" + programmingExercise.getId() + "/submissions/3/repository")); | |
| List<ProgrammingFeedbackDTO> suggestions = athenaFeedbackSuggestionsService.getProgrammingFeedbackSuggestions(programmingExercise, programmingSubmission, false); | |
| assertThat(suggestions.getFirst().title()).isEqualTo("Not so good"); | |
| assertThat(suggestions.getFirst().lineStart()).isEqualTo(3); | |
| athenaRequestMockProvider.verify(); | |
| } | |
| athenaRequestMockProvider.mockGetPreliminaryFeedbackProgrammingAndExpect("programming", false, | |
| jsonPath("$.exercise.id").value(programmingExercise.getId()), | |
| jsonPath("$.exercise.title").value(programmingExercise.getTitle()), | |
| jsonPath("$.submission.id").value(programmingSubmission.getId()), | |
| jsonPath("$.submission.repositoryUri") | |
| - .value("http://localhost/api/athena/public/programming-exercises/" + programmingExercise.getId() + "/submissions/3/repository")); | |
| + .value("https://artemislocal.cit.tum.de/api/athena/public/programming-exercises/" + programmingExercise.getId() + "/submissions/3/repository")); | |
| List<ProgrammingFeedbackDTO> feedback = athenaFeedbackSuggestionsService.getProgrammingPreliminaryFeedback(programmingExercise, programmingSubmission, false); |
🤖 Prompt for AI Agents
In
src/test/java/de/tum/cit/aet/artemis/athena/service/connectors/AthenaFeedbackSuggestionsServiceTest.java
around line 134, the testPreliminaryFeedbackProgramming method asserts a
repositoryUri using "localhost" while the testFeedbackSuggestionsProgramming
method uses "https://artemislocal.cit.tum.de". To fix this inconsistency, update
the expected repositoryUri in testPreliminaryFeedbackProgramming to use
"https://artemislocal.cit.tum.de" so both tests align on the same host URL.
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
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).Motivation and Context
At the moment, Artemis only supports assigning a single Athena Feedback Request module, regardless of whether the feedback is preliminary or graded.
Description
This PR adds the ability for instructors to assign separate Athena Feedback modules for preliminary and graded feedback. This provides the flexibility to choose the module that best fits the type of feedback needed for a specific exercise.
Note: This PR is based on a closed, unmerged PR. It includes resolved merge conflicts and introduces new features, such as filtering the selectable Athena modules based on their feedback type capabilities.
Steps for Testing
Prerequisites:
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
You can manage test servers using Helios. Check environment statuses in the environment list. To deploy to a test server, go to the CI/CD page, find your PR or branch, and trigger the deployment.
Review Progress
Performance Review
Code Review
Manual Tests
Performance Tests
Test Coverage
Screenshots
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Tests
Chores