Programming exercises: Allow to choose preliminary feedback model
#10067
Conversation
…odules: client side
…g-exercises/choose-preliminary-feedback-model
github-actions
bot
added
tests
server
|
There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions. |
dmytropolityka
changed the title
Programming exercises Allow to choose preliminary feedback model
…iminary-feedback-model
…oose-preliminary-feedback-model' into feature/programming-exercises/choose-preliminary-feedback-model
dmytropolityka
changed the title
Programming exercises Allow to choose preliminary feedback modelProgramming exercises: Allow to choose preliminary feedback model
dmytropolityka
changed the title
Programming exercises: Allow to choose preliminary feedback modelProgramming exercises: Allow to choose preliminary feedback model
github-actions
bot
added
the
core
github-actions
bot
added
the
communication
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 pmd (7.8.0)src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.javaThe following rules are missing or misspelled in your ruleset file category/vm/bestpractices.xml: BooleanInstantiation, DontImportJavaLang, DuplicateImports, EmptyFinallyBlock, EmptyIfStmt, EmptyInitializer, EmptyStatementBlock, EmptyStatementNotInLoop, EmptySwitchStatements, EmptySynchronizedBlock, EmptyTryBlock, EmptyWhileStmt, ExcessiveClassLength, ExcessiveMethodLength, ImportFromSamePackage, MissingBreakInSwitch, SimplifyBooleanAssertion. Please check your ruleset configuration. WalkthroughThe pull request introduces significant changes to the Artemis application's feedback mechanism, focusing on refining how feedback suggestions and preliminary feedback are handled across different exercise types. The modifications include introducing a new Changes
Suggested labels
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (34)
src/test/java/de/tum/cit/aet/artemis/exercise/participation/ParticipationIntegrationTest.java (1)
594-594: Consider reducing repetition in test setup
This call seems identical to the one in line 550. If possible, you could refactor common test setup logic to improve maintainability.src/main/webapp/app/exercises/shared/feedback-suggestion/exercise-feedback-suggestion-options.component.ts (1)
79-79: Avoid usinganyfor event parameter
Consider strongly typing the event parameter to enhance clarity and maintainability, for example:toggleFeedbackSuggestions(event: Event) { const inputEl = event.target as HTMLInputElement; this.showDropdownList = inputEl.checked; // ... }src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaModuleService.java (1)
162-162: Clarification on feedback suggestions
The comment indicates that only feedback suggestion modules are restricted post-due-date. Explicitly document if new module types (like preliminary feedback) inherit the same restriction.src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.ts (2)
39-47: Checking for modules availability on init
Appropriate usage of an observable to fetch modules. Consider handling error cases if the request fails (e.g., logging or user notification).
74-81: Use stronger typing for event
Similar to other components, use a stricter event type to avoidany. This helps with consistency and type safety.src/main/webapp/app/exercises/modeling/manage/modeling-exercise.module.ts (1)
57-57: Ensure consistent module usage for shared components
IncludingExercisePreliminaryFeedbackOptionsComponentin the imports array implies that it's an exported component from a module or that you intend to declare it in this module. Re-check if this approach aligns with the architectural standards (e.g., using a shared module that declares and exports the component).src/test/javascript/spec/component/exercises/shared/feedback/feedback-suggestion-option.component.spec.ts (1)
101-113: Expand coverage for toggling feedback
The toggling logic is tested for the happy path. Consider adding a test for when no modules are available (e.g.,availableAthenaModulesis empty) to ensure no errors occur if toggled with an empty list.src/test/javascript/spec/component/exercises/shared/feedback/preliminary-feedback-options.component.spec.ts (2)
63-63: Unit test coverage
Tests forinputControlsDisabledlogic look comprehensive; consider including an additional scenario for standard (non-programming) exercises withMANUALassessment type to verify that the controls remain enabled.
81-81: Minor readability improvement
When returning the style object, you might consider returning a constant, e.g.{ color: 'grey' }, from a named const or a helper method for improved clarity.src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.java (9)
98-100: Clarify comment for theisPreliminaryparameter.
The Javadoc states "should Athena generate grade suggestions or not," but the parameter is namedisPreliminary. It might be clearer to adjust the wording to consistently reference "preliminary feedback" throughout the comment.
103-104: Consistent logging message.
The log statement uses "Graded" or "Non Graded" in quotes, but the code now treats the parameter as "preliminary" vs. "graded." Consider harmonizing terminology to avoid confusion (e.g., "Preliminary" vs. "Graded").
126-128: Clarify docstring.
The parameter reference in the docstring says "should Athena generate grade suggestions or not," but the variable is namedisPreliminary. Align these terms for consistency across the codebase.
131-131: Docstring mismatch.
The docstring says the function returns a list of feedback suggestions, but doesn’t explicitly mention how theisPreliminaryparameter modifies the returned suggestions. Consider clarifying.
133-135: Same naming consistency concern.
TheRequestDTOis created withisPreliminary, yet references like "Graded" or "Non Graded" appear in logging.
148-150: Improve doc consistency.
The doc block mentions "should Athena generate grade suggestions or not," while the actual parameter isisPreliminary. Matching vocabulary helps future contributors.
153-153: Naming alignment.
Rename the method doc referencing'feedback suggestions' for a modeling exerciseto clarify these can also be preliminary or non-graded suggestions.
154-155: Consistent logging message.
As above, the log references'Graded' : 'Non Graded', which differs from the parameter nameisPreliminary.
168-169: Minor style alignment.
Log statement references'Graded' : 'Non Graded'. Consider naming reuse forisPreliminaryto reduce confusion.src/main/webapp/app/exercises/programming/shared/lifecycle/programming-exercise-lifecycle.component.ts (1)
162-162: Maintain consistent approach for deactivating feedback.
This line setsallowManualFeedbackRequeststofalseunderSEMI_AUTOMATIC. Verify that other lines or toggles (liketoggleManualFeedbackRequests) cannot produce conflicting states.src/main/java/de/tum/cit/aet/artemis/athena/web/AthenaResource.java (2)
108-108: Verify generics usage in the interface.
Using<ExerciseType, SubmissionType, Boolean, OutputType>is somewhat confusing givenBooleanis often a known type, not a placeholder. Consider renaming to<ExerciseT, SubmissionT, PreliminaryFlag, OutputT>for clarity.
114-114: Method name alignment in comment.
The comment references "graded" feedback, but the parameter is nowisPreliminary. Consider removing references to "graded" or clarifying the context.src/test/javascript/spec/component/programming-exercise/programming-exercise-lifecycle.component.spec.ts (5)
30-30: Inject vs. create new variable.
athenaServiceis declared here and also injected into the test setup. It's correct, but ensure no overshadowing or confusion if referencing within scope.
157-159: Test coverage for toggling.
Testing the toggle fromfalsetotrueis good. Consider also verifying state changes fromtruetofalsefor complete coverage.
161-161: Test step expansion.
Add an assertion verifying that related fields (assessmentDueDate, etc.) are cleared or set properly when toggling.
163-163: Self-documenting code.
expect(comp.exercise.allowManualFeedbackRequests).toBeTrue();confirms the toggle was successful. Consider a small comment clarifying the final state of the exercise.
369-381: Improve naming of test scenario.
The test name references "feedback suggestions are set." Might clarify "should render component iffeedbackSuggestionsare enabled in non-exam mode."src/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.java (4)
286-289: Ensure second check forpreliminaryFeedbackModule
The approach mirrors the earlier check forfeedbackSuggestionModule. This symmetry helps avoid inconsistent module availability.
535-536: Improve comments for module checks
Expanding the comments might clarify the difference between the two modules (feedback suggestions vs. preliminary feedback).
546-546: Declarative style
Consider weaving the ifPresentOrElse calls consistently in one block for clarity if additional lines are added.
547-548: Avoid partial catch duplication
Consider grouping repeated exception-handling logic into a helper method, if it expands further.src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.html (1)
21-23: Remove redundant 'this' references.Angular templates don't require explicit 'this' references.
-[value]="this.exercise.preliminaryFeedbackModule" -[(ngModel)]="this.exercise.preliminaryFeedbackModule" -[disabled]="inputControlsDisabled()" +[value]="exercise.preliminaryFeedbackModule" +[(ngModel)]="exercise.preliminaryFeedbackModule" +[disabled]="inputControlsDisabled()"src/main/webapp/app/assessment/assessment-warning/assessment-warning.component.ts (1)
47-47: LGTM! Property rename reflects clearer intent.The change from
allowFeedbackRequeststoallowManualFeedbackRequestsbetter describes the purpose of the flag and aligns with the PR's objective of distinguishing between different types of feedback.However, consider extracting the complex condition to a readable method:
-this.showWarning = now.isBefore(this.getLatestDueDate()) && !this.exercise.allowManualFeedbackRequests; +this.showWarning = this.shouldShowWarning(now); + +private shouldShowWarning(currentTime: dayjs.Dayjs): boolean { + return currentTime.isBefore(this.getLatestDueDate()) && !this.exercise.allowManualFeedbackRequests; +}src/main/webapp/app/exercises/text/participate/text-editor.component.html (1)
Line range hint
27-36: Consider extracting complex condition to a component methodThe template contains a complex condition combining multiple checks. Consider extracting this logic to a method in the component class for better maintainability and testability.
- @if (textExercise.allowManualFeedbackRequests && (!this.textExercise.dueDate || !hasExerciseDueDatePassed(this.textExercise, this.participation))) { + @if (canRequestManualFeedback()) {Add to component class:
canRequestManualFeedback(): boolean { return this.textExercise.allowManualFeedbackRequests && (!this.textExercise.dueDate || !this.hasExerciseDueDatePassed(this.textExercise, this.participation)); }src/main/webapp/app/exercises/programming/shared/lifecycle/programming-exercise-lifecycle.component.html (1)
203-224: Consider simplifying the nested conditional structureWhile the logic is correct, the nested conditions could be simplified for better maintainability.
Consider restructuring the conditions using early returns or separate components:
-@if ((isEditFieldDisplayedRecord()?.feedbackSuggestions || !isEditFieldDisplayedRecord()) && !isExamMode) { - <jhi-exercise-feedback-suggestion-options [exercise]="exercise" [dueDate]="exercise.dueDate" [readOnly]="readOnly" /> -} -<h6 jhiTranslate="artemisApp.assessment.feedback"></h6> -@if ((isEditFieldDisplayedRecord()?.preliminaryFeedbackRequests || isEditFieldDisplayedRecord()?.manualFeedbackRequests || !isEditFieldDisplayedRecord()) && !isExamMode) { - @if (isAthenaEnabled$ | async) { - <jhi-exercise-preliminary-feedback-options [exercise]="exercise" [dueDate]="exercise.dueDate" [readOnly]="readOnly" /> - } @else { - <div class="form-check"> +@if (!isExamMode) { + <ng-container *ngTemplateOutlet="feedbackOptions"></ng-container> +} + +<ng-template #feedbackOptions> + @if (isEditFieldDisplayedRecord()?.feedbackSuggestions || !isEditFieldDisplayedRecord()) { + <jhi-exercise-feedback-suggestion-options [exercise]="exercise" [dueDate]="exercise.dueDate" [readOnly]="readOnly" /> + } + <h6 jhiTranslate="artemisApp.assessment.feedback"></h6> + @if (isEditFieldDisplayedRecord()?.preliminaryFeedbackRequests || isEditFieldDisplayedRecord()?.manualFeedbackRequests || !isEditFieldDisplayedRecord()) { + @if (isAthenaEnabled$ | async) { + <jhi-exercise-preliminary-feedback-options [exercise]="exercise" [dueDate]="exercise.dueDate" [readOnly]="readOnly" /> + } @else { + <div class="form-check">
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/main/resources/config/liquibase/changelog/20241212135412_changelog.xmlis excluded by!**/*.xmlsrc/main/resources/config/liquibase/master.xmlis excluded by!**/*.xml
📒 Files selected for processing (62)
src/main/java/de/tum/cit/aet/artemis/athena/domain/ModuleType.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSendingService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaModuleService.java(3 hunks)src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaRepositoryExportService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaSubmissionSelectionService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaSubmissionSendingService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/athena/web/AthenaResource.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/communication/dto/ChannelDTO.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exercise/domain/Exercise.java(4 hunks)src/main/java/de/tum/cit/aet/artemis/exercise/service/SubmissionService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/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/assessment-warning/assessment-warning.component.ts(1 hunks)src/main/webapp/app/entities/exercise.model.ts(4 hunks)src/main/webapp/app/exercises/modeling/manage/modeling-exercise-detail.component.ts(1 hunks)src/main/webapp/app/exercises/modeling/manage/modeling-exercise-update.component.html(1 hunks)src/main/webapp/app/exercises/modeling/manage/modeling-exercise-update.component.ts(1 hunks)src/main/webapp/app/exercises/modeling/manage/modeling-exercise.module.ts(2 hunks)src/main/webapp/app/exercises/modeling/participate/modeling-submission.component.html(1 hunks)src/main/webapp/app/exercises/programming/manage/programming-exercise-detail.component.ts(1 hunks)src/main/webapp/app/exercises/programming/manage/update/programming-exercise-update.helper.ts(2 hunks)src/main/webapp/app/exercises/programming/shared/lifecycle/programming-exercise-lifecycle.component.html(3 hunks)src/main/webapp/app/exercises/programming/shared/lifecycle/programming-exercise-lifecycle.component.ts(2 hunks)src/main/webapp/app/exercises/programming/shared/lifecycle/programming-exercise-lifecycle.module.ts(1 hunks)src/main/webapp/app/exercises/shared/dashboards/tutor/exercise-assessment-dashboard.component.ts(1 hunks)src/main/webapp/app/exercises/shared/feedback-suggestion/exercise-feedback-suggestion-options.component.html(2 hunks)src/main/webapp/app/exercises/shared/feedback-suggestion/exercise-feedback-suggestion-options.component.ts(3 hunks)src/main/webapp/app/exercises/shared/feedback-suggestion/exercise-feedback-suggestion-options.module.ts(0 hunks)src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.html(1 hunks)src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.ts(1 hunks)src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise-detail.component.ts(1 hunks)src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise-update.component.html(1 hunks)src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise.module.ts(2 hunks)src/main/webapp/app/exercises/text/participate/text-editor.component.html(1 hunks)src/main/webapp/app/overview/exercise-details/exercise-details-student-actions.component.html(2 hunks)src/main/webapp/app/overview/exercise-details/request-feedback-button/request-feedback-button.component.ts(1 hunks)src/main/webapp/i18n/de/exercise.json(1 hunks)src/main/webapp/i18n/de/programmingExercise.json(0 hunks)src/main/webapp/i18n/de/textExercise.json(1 hunks)src/main/webapp/i18n/en/exercise.json(1 hunks)src/main/webapp/i18n/en/programmingExercise.json(0 hunks)src/main/webapp/i18n/en/textExercise.json(1 hunks)src/test/java/de/tum/cit/aet/artemis/athena/AthenaResourceIntegrationTest.java(6 hunks)src/test/java/de/tum/cit/aet/artemis/athena/service/connectors/AthenaFeedbackSuggestionsServiceTest.java(6 hunks)src/test/java/de/tum/cit/aet/artemis/core/connector/AthenaRequestMockProvider.java(3 hunks)src/test/java/de/tum/cit/aet/artemis/exercise/participation/ParticipationIntegrationTest.java(4 hunks)src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingAssessmentIntegrationTest.java(1 hunks)src/test/javascript/spec/component/assessment-dashboard/exercise-assessment-dashboard.component.spec.ts(1 hunks)src/test/javascript/spec/component/exercises/shared/feedback/feedback-suggestion-option.component.spec.ts(3 hunks)src/test/javascript/spec/component/exercises/shared/feedback/preliminary-feedback-options.component.spec.ts(1 hunks)src/test/javascript/spec/component/modeling-exercise/modeling-exercise-update.component.spec.ts(0 hunks)src/test/javascript/spec/component/overview/exercise-details/exercise-details-student-actions.component.spec.ts(2 hunks)src/test/javascript/spec/component/overview/exercise-details/request-feedback-button/request-feedback-button.component.spec.ts(9 hunks)src/test/javascript/spec/component/programming-exercise/programming-exercise-lifecycle.component.spec.ts(6 hunks)src/test/javascript/spec/component/programming-exercise/programming-exercise-update.component.spec.ts(2 hunks)src/test/javascript/spec/component/text-exercise/text-exercise-update.component.spec.ts(0 hunks)
💤 Files with no reviewable changes (5)
- src/main/webapp/app/exercises/shared/feedback-suggestion/exercise-feedback-suggestion-options.module.ts
- src/test/javascript/spec/component/text-exercise/text-exercise-update.component.spec.ts
- src/test/javascript/spec/component/modeling-exercise/modeling-exercise-update.component.spec.ts
- src/main/webapp/i18n/en/programmingExercise.json
- src/main/webapp/i18n/de/programmingExercise.json
✅ Files skipped from review due to trivial changes (3)
- src/main/java/de/tum/cit/aet/artemis/athena/domain/ModuleType.java
- src/main/webapp/app/exercises/modeling/manage/modeling-exercise-update.component.ts
- src/test/javascript/spec/component/assessment-dashboard/exercise-assessment-dashboard.component.spec.ts
🧰 Additional context used
📓 Path-based instructions (52)
src/main/webapp/app/assessment/assessment-warning/assessment-warning.component.ts (1)
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingAssessmentService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaSubmissionSendingService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/test/javascript/spec/component/overview/exercise-details/exercise-details-student-actions.component.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/main/java/de/tum/cit/aet/artemis/communication/dto/ChannelDTO.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise-detail.component.ts (1)
src/main/webapp/app/exercises/modeling/manage/modeling-exercise-detail.component.ts (1)
src/main/webapp/app/exercises/shared/dashboards/tutor/exercise-assessment-dashboard.component.ts (1)
src/main/webapp/app/exercises/text/participate/text-editor.component.html (1)
Pattern src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.
src/test/javascript/spec/component/exercises/shared/feedback/preliminary-feedback-options.component.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/main/webapp/app/overview/exercise-details/exercise-details-student-actions.component.html (1)
Pattern src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaSubmissionSelectionService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/webapp/app/exercises/modeling/manage/modeling-exercise.module.ts (1)
src/main/webapp/app/exercises/programming/shared/lifecycle/programming-exercise-lifecycle.component.html (1)
Pattern src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.
src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise-update.component.html (1)
Pattern src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.
src/main/webapp/app/exercises/modeling/manage/modeling-exercise-update.component.html (1)
Pattern src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseExportImportResource.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/webapp/app/overview/exercise-details/request-feedback-button/request-feedback-button.component.ts (1)
src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.html (1)
Pattern src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/webapp/app/exercises/shared/feedback-suggestion/exercise-feedback-suggestion-options.component.html (1)
Pattern src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.
src/main/webapp/i18n/de/textExercise.json (1)
Pattern src/main/webapp/i18n/de/**/*.json: German language translations should be informal (dutzen) and should never be formal (sietzen). So the user should always be addressed with "du/dein" and never with "sie/ihr".
src/main/webapp/i18n/de/exercise.json (1)
Pattern src/main/webapp/i18n/de/**/*.json: German language translations should be informal (dutzen) and should never be formal (sietzen). So the user should always be addressed with "du/dein" and never with "sie/ihr".
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaRepositoryExportService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/test/javascript/spec/component/programming-exercise/programming-exercise-lifecycle.component.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/main/java/de/tum/cit/aet/artemis/exercise/service/SubmissionService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/test/javascript/spec/component/programming-exercise/programming-exercise-update.component.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/test/javascript/spec/component/overview/exercise-details/request-feedback-button/request-feedback-button.component.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/test/javascript/spec/component/exercises/shared/feedback/feedback-suggestion-option.component.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/test/java/de/tum/cit/aet/artemis/exercise/participation/ParticipationIntegrationTest.java (1)
Pattern 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/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingSubmissionResource.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/webapp/app/exercises/programming/shared/lifecycle/programming-exercise-lifecycle.module.ts (1)
src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise.module.ts (1)
src/main/webapp/app/exercises/programming/shared/lifecycle/programming-exercise-lifecycle.component.ts (1)
src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.ts (1)
src/main/webapp/app/exercises/modeling/participate/modeling-submission.component.html (1)
Pattern src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.
src/main/webapp/app/exercises/programming/manage/programming-exercise-detail.component.ts (1)
src/main/webapp/app/exercises/shared/feedback-suggestion/exercise-feedback-suggestion-options.component.ts (1)
src/main/webapp/app/exercises/programming/manage/update/programming-exercise-update.helper.ts (1)
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSendingService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/programming/domain/ProgrammingExercise.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/webapp/app/entities/exercise.model.ts (1)
src/test/java/de/tum/cit/aet/artemis/athena/service/connectors/AthenaFeedbackSuggestionsServiceTest.java (1)
Pattern src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true
src/test/java/de/tum/cit/aet/artemis/athena/AthenaResourceIntegrationTest.java (1)
Pattern 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/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaModuleService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/exercise/domain/Exercise.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/athena/web/AthenaResource.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/test/java/de/tum/cit/aet/artemis/core/connector/AthenaRequestMockProvider.java (1)
Pattern 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/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingAssessmentIntegrationTest.java (1)
Pattern 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
📓 Learnings (3)
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingAssessmentService.java (1)
Learnt from: undernagruzez
PR: ls1intum/Artemis#8498
File: src/main/java/de/tum/in/www1/artemis/service/programming/ProgrammingExerciseCodeReviewFeedbackService.java:236-236
Timestamp: 2024-11-12T12:51:40.391Z
Learning: Rate limit exceptions for AI feedback requests are handled in the client app.
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseExportImportResource.java (1)
Learnt from: Hialus
PR: ls1intum/Artemis#8607
File: src/main/java/de/tum/in/www1/artemis/web/rest/programming/ProgrammingExerciseResource.java:64-64
Timestamp: 2024-11-12T12:51:51.201Z
Learning: For the Artemis project, import statements are automatically managed by formatters and should not be commented on.
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java (1)
Learnt from: Hialus
PR: ls1intum/Artemis#8607
File: src/main/java/de/tum/in/www1/artemis/web/rest/programming/ProgrammingExerciseResource.java:64-64
Timestamp: 2024-11-12T12:51:51.201Z
Learning: For the Artemis project, import statements are automatically managed by formatters and should not be commented on.
🔇 Additional comments (131)
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportService.java (1)
296-298: Ensure proper module management in exam mode
Setting bothfeedbackSuggestionModuleandpreliminaryFeedbackModuletonullfor exam exercises makes sense to disable these features. However, please verify that no downstream logic expects these modules to be non-null. If any additional cleanup or conditional checks are needed elsewhere to handle the absence of these modules, consider adding them to ensure consistent application state.src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaSubmissionSendingService.java (1)
129-131: Suggest validating exercise module fields before building the URL.While appending the URL with
exercise.getExerciseType()andexercise.getFeedbackSuggestionModule(), consider adding a small safety check in case these fields are null or uninitialized. This helps ensure the request will not fail due to missing or invalid data, maintaining robustness and alignment with the KISS principle.src/test/java/de/tum/cit/aet/artemis/core/connector/AthenaRequestMockProvider.java (5)
58-61: Good addition of constants for clarity.The separate constants for text suggestions and text preliminary modules provide explicit, descriptive differentiation. This aligns with the requirement for clear, small-scoped test data.
66-69: Consistent naming for programming module constants.Defining separate constants for the programming module’s suggestions and preliminary types is consistent, improving readability and maintainability.
74-77: Modeling module constants likewise follow the established pattern.Your approach for the modeling module matches the naming and organization style used for text and programming modules. This consistency helps keep things coherent in test scenarios.
112-120: Use of a unified helper method is well-structured.The newly introduced
getTestModuleNamemethod elegantly centralizes module name generation based on whether it is a preliminary or suggestion request. This promotes DRY principles and lowers the risk of future naming errors.
212-214: Signature update to differentiate preliminary vs. suggestions is valid.By adding the
isPreliminaryparameter tomockGetFeedbackSuggestionsAndExpect, you have cleanly extended the functionality to handle both use cases. Mocking with static method calls and factoring in the new parameter helps ensure the test coverage remains both thorough and concise.src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingAssessmentService.java (1)
155-155: Confirm consistency with newly introduced manual feedback workflow.The logic correctly checks whether manual feedback requests are enabled and the participation’s individual due date has already passed. Ensure that any related checks in other parts of the application are also updated consistently to use
getAllowManualFeedbackRequests().You can run the following shell script to verify that no instances of
getAllowFeedbackRequests()remain in the code:✅ Verification successful
Manual feedback request workflow is consistently implemented
The codebase shows a complete and consistent implementation of the manual feedback request workflow using
getAllowManualFeedbackRequests(). Key findings:
- The method is correctly used in
ProgrammingAssessmentService.javaandProgrammingSubmissionResource.javafor controlling manual feedback requests- Frontend components properly handle the flag through:
- Exercise model definition (
allowManualFeedbackRequestsproperty)- Programming exercise lifecycle component for toggling the setting
- Assessment components respecting the flag for UI controls
- No instances of the old
getAllowFeedbackRequests()method were found in the codebase- Proper validation checks are in place in
ProgrammingExercise.javato ensure manual feedback settings are valid🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining references to the old feedback requests method rg 'getAllowFeedbackRequests\(\)' -A 3Length of output: 40
Script:
#!/bin/bash # Let's search for any references to manual feedback requests in the codebase # This will help us understand the implementation better # Search for method declarations and usages related to manual feedback rg -i "manualfeedback|allowmanualfeedback" -A 3Length of output: 46626
src/main/webapp/app/exercises/programming/shared/lifecycle/programming-exercise-lifecycle.module.ts (2)
7-8: Well-named standalone imports.
These new components follow Angular's PascalCase naming convention and use single quotes as required. Good job adhering to the style guidelines.
11-11: Confirm standalone component usage.
Including components in theimportsarray implies they are standalone. If they are not declared withstandalone: true, move them to thedeclarationsarray.src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaSubmissionSelectionService.java (1)
86-88: Validate null checks for the new parameters.You’re now retrieving the module URL with both
exercise.getExerciseType()andexercise.getFeedbackSuggestionModule(). Please ensure null-safety checks for these new parameters (e.g., in theathenaModuleService.getAthenaModuleUrl()implementation) to avoid potential NullPointerExceptions.src/test/java/de/tum/cit/aet/artemis/exercise/participation/ParticipationIntegrationTest.java (3)
550-550: Passingtrueas the second parameter for programming feedback
Good use of the new boolean parameter to indicate a specialized or preliminary feedback scenario. Ensure that test cases exist for the scenario when it'sfalseas well.
638-638: Maintain consistency across feedback requests
Just like "programming" and "modeling," the string "text" is also valid. All calls appear consistent with the newly introduced parameter.
677-677: Ensure coverage of modeling-specific behavior
You have a similar call with"modeling", true. Confirm that the modeling exercises' feedback retrieval is tested separately to capture domain differences between modeling and text/programming.src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingSubmissionResource.java (1)
387-387: Ensure consistency with the new feedback request logicThe replacement of
getAllowFeedbackRequests()withgetAllowManualFeedbackRequests()aligns with the updated naming across the application. Verify that any other references to the old method have been removed or updated elsewhere in the codebase to avoid potential mismatches.Use the following script to confirm that no other references to the old method remain:
✅ Verification successful
The change from
getAllowFeedbackRequests()togetAllowManualFeedbackRequests()is part of a broader system updateBased on the search results, I can see that the codebase is undergoing a transition in how feedback requests are handled. The changes appear to be part of a larger update where:
- The frontend (i18n and components) still uses the term
allowFeedbackRequestsin UI elements and properties- The backend (Java code) has moved to using
allowManualFeedbackRequests- There's a distinction being made between manual and automatic/preliminary feedback requests
Let's verify the complete implementation:
The change to
getAllowManualFeedbackRequests()is part of a complete system updateBased on the comprehensive search results, I can confirm that the codebase has been properly updated to use the new
allowManualFeedbackRequestsproperty. The changes are consistent across:
- The backend (Java) domain model in
Exercise.javawith proper getter/setter methods- The frontend components and templates using the new property name
- Test files that have been updated to use the new property name
- Service layer logic that checks for manual feedback requests
The change from
getAllowFeedbackRequests()togetAllowManualFeedbackRequests()is part of a broader update that distinguishes between manual and automatic feedback requests, where:
- Manual feedback requests are controlled by
allowManualFeedbackRequests- Automatic/preliminary feedback is handled by
preliminaryFeedbackModule🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for old references to getAllowFeedbackRequests and highlight potential mismatches. rg 'getAllowFeedbackRequests\('Length of output: 33
Script:
#!/bin/bash # Let's try a more comprehensive search including both the old and new method names # and also check for any related configuration or property references # Search for any variations of the method name rg -i "allowfeedbackrequest" -A 2 -B 2 # Also search in Java files specifically for the getter/setter pattern ast-grep --pattern 'get[A-Za-z]*FeedbackRequest[s]?()' ast-grep --pattern 'set[A-Za-z]*FeedbackRequest[s]?()' ast-grep --pattern 'is[A-Za-z]*FeedbackRequest[s]?()'Length of output: 8858
Script:
#!/bin/bash # Search for the new method name and related properties rg -i "allowmanualfeedbackrequest" -A 2 -B 2 # Search for the class definition to understand the property declaration ast-grep --pattern 'class Exercise { $$$ private boolean allowManualFeedbackRequests; $$$ }'Length of output: 23739
src/main/webapp/app/exercises/shared/feedback-suggestion/exercise-feedback-suggestion-options.component.ts (4)
8-9: Imports for standalone component look correct
These imports are aligned with the Angular standalone component approach, and no issues appear with module references.
13-14: Good use of the standalone component pattern
Declaringstandalone: trueand specifyingimportsaligns with Angular best practices, reducing boilerplate.
32-32: Informative property for dropdown visibility
InitializingshowDropdownListtofalseis straightforward and improves readability.
82-82: Double-check the forced property assignment
SettingallowManualFeedbackRequeststofalsewhenever the checkbox is checked might conflict with user expectations if manual feedback should remain available. Confirm the requirement or allow the user to configure this property independently.src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSendingService.java (1)
98-102: Confirm module validity and sanitize if necessary
Using the module name in URL construction can pose security risks (e.g., path traversal) if not validated. Ensure it is a trusted string or thoroughly sanitized upstream.src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaModuleService.java (4)
25-25: Introduction of ModuleType
ImportingModuleTypeclarifies the distinction between different Athena functionalities (e.g., feedback vs. preliminary). This is a maintainable approach.
111-126: Method signature refactor for module-specific URLs
Switching to(ExerciseType exerciseType, String module)is a clean approach, though ensure thatmoduleremains valid and present for the chosenexerciseType.
135-146: Access checks for exam exercises
Throwing an error for exam exercises that attempt to use Athena ensures compliance with exam constraints. Good approach.
150-157: Modular approach to retrieving the appropriate module
Using the privategetModulemethod centralizes logic and prevents duplication. Verify correctness of mapping for new module types in the future.src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.ts (4)
1-9: Imports and module references are coherent
All necessary modules and services for the new component are imported, matching the Angular style guidelines for standalone components.
11-16: Component declaration stands out
Utilizingstandalone: trueplus the requiredimportsfosters a modular structure.
34-37: Constructor injection best practice
InjectingAthenaServiceandActivatedRoutein the constructor follows recommended Angular design.
49-55: Due date changes
ResettingpreliminaryFeedbackModuleupon due date changes ensures consistency, but confirm whether clearing instead of resetting it toinitialAthenaModulewould be more logical in some scenarios.src/main/webapp/app/exercises/programming/manage/update/programming-exercise-update.helper.ts (2)
43-44: Enum extension
AddingFEEDBACK_SUGGESTIONSandPRELIMINARY_FEEDBACK_REQUESTSeffectively broadens the possible input fields; ensure thorough coverage in relevant forms and validators.
93-101: Visibility settings for simple mode
Setting these fields tofalseinIS_DISPLAYED_IN_SIMPLE_MODEmatches the requirement to hide them unless explicitly toggled in advanced settings. Confirm this matches all user workflow scenarios.src/main/webapp/app/exercises/modeling/manage/modeling-exercise.module.ts (1)
30-30: Importing a component directly in imports?
Usually, we import modules in theimportsarray, while components belong indeclarations. Ensure thatExercisePreliminaryFeedbackOptionsComponentis declared in a proper module and then import that module here, rather than directly importing the component.src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise.module.ts (1)
32-33: Clarify component declarations vs. imports
When importing bothExercisePreliminaryFeedbackOptionsComponentandExerciseFeedbackSuggestionOptionsComponent, confirm that they are properly declared in their own modules if you plan to import them here. If the intention is to declare them in the same module, move them to thedeclarationsarray to ensure they are recognized as part of this module.src/test/javascript/spec/component/exercises/shared/feedback/feedback-suggestion-option.component.spec.ts (2)
10-10: Good use of ArtemisTestModule
Pulling in theArtemisTestModulecan streamline test setup by reusing common mocks and test configs.
21-21: Check for potential conflicts
IncludingExerciseFeedbackSuggestionOptionsComponentinimportsmight override some configurations or re-declare the component. Ensure it’s not already declared elsewhere or in the same test.src/test/javascript/spec/component/exercises/shared/feedback/preliminary-feedback-options.component.spec.ts (1)
21-21: Validate component setup
Including the component directly inimportscan be valid if it’s marked as a standalone component (@Component({ standalone: true })). Otherwise, it belongs indeclarations. Confirm the usage pattern with your existing Angular architecture.src/main/webapp/app/overview/exercise-details/request-feedback-button/request-feedback-button.component.ts (1)
52-56: Logic Confirmed for Switching Between Preliminary and Manual FeedbackThis conditionally enables request feedback based on the presence of
preliminaryFeedbackModulewhen Athena is enabled or byallowManualFeedbackRequestsotherwise. The checks look consistent with the new feedback model introduction.src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise-detail.component.ts (1)
106-107: Consistent Mapping to New Feedback ModulesThese lines correctly reference
feedbackSuggestionModuleandpreliminaryFeedbackModulefor boolean display. This aligns with the refactoring and is coherent with the updated exercise model.src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaRepositoryExportService.java (1)
69-69: Renaming to Reflect Preliminary FeedbackSwitching from
allowFeedbackRequeststo checkingisPreliminaryFeedbackEnabled()is consistent with the new property naming. The condition properly ensures feedback suggestions or preliminary feedback is enabled.src/main/java/de/tum/cit/aet/artemis/communication/dto/ChannelDTO.java (1)
200-204: Consider Additional Fields When Building Channel ObjectsThe
toChannel()method does not include fields likesubType,subTypeReferenceId, or the tutorial group details. If that is intentional, it should be clearly documented; otherwise, consider incorporating them.src/test/java/de/tum/cit/aet/artemis/athena/service/connectors/AthenaFeedbackSuggestionsServiceTest.java (12)
3-8: Good usage of static imports for Athena modules.This approach keeps the code concise and highlights the direct references to the relevant modules.
27-29: Clear introduction of modeling exercise classes and utilities.The references to
ModelingExercise,ModelingSubmission, andModelingExerciseUtilServiceprovide consistency with the structure used for text and programming exercises. This helps maintain a unified workflow across different exercise types.
50-51: Naming consistency.The variable names
modelingExerciseUtilServiceproperly align with the established naming scheme in the code.
61-64: Initialization of modeling exercise and submission.The introduction of
modelingExerciseandmodelingSubmissionparallels the existing approach fortextExerciseandprogrammingExercise, keeping test coverage symmetrical.
70-71: Consistent setup of feedback modules.By setting both
feedbackSuggestionModuleandpreliminaryFeedbackModulein the@BeforeEachsetup, you're preserving the default test arrangement to reflect the new feedback configuration options.
76-77: Parallel structure for programming exercise modules.The approach for
programmingExercisemirrors the text exercise setup, reinforcing a consistent testing strategy across various exercise types.
81-88: New modeling exercise setup.Well-structured setup for the modeling exercise, ensuring parity with text and programming test coverage. This demonstrates a uniform testing approach for all exercise types.
94-97: Integration test coverage for text feedback suggestions.Ensures the correct behavior of
getTextFeedbackSuggestionswithisPreliminary = false. The mock JSON paths validate that the request payload matches the expected submission details.
106-114: Programming feedback suggestions coverage.Similar test approach as with text exercises, verifying the JSON paths and the final feedback. This maintains consistency and improves confidence in the code's reliability.
116-126: Preliminary feedback testing for text.Adding test cases for
isPreliminary = trueensures thorough coverage of the preliminary feedback path. Clear usage ofmockGetFeedbackSuggestionsAndExpectfurther reinforces test correctness.
Line range hint
128-138: Preliminary feedback testing for programming.Parallel approach to text-based testing for programming exercises. Consistency in structure helps easily compare results across different exercise types.
143-143: Conflict scenario test coverage.Verifying that an exception is thrown when there's a mismatch between submission and exercise ensures robust error handling.
src/main/webapp/app/exercises/modeling/manage/modeling-exercise-detail.component.ts (1)
139-140: Boolean details for feedback and requests.Introducing these boolean indicators in the details array effectively communicates the availability of feedback suggestions and preliminary feedback to the user. This integrates well with the new modules introduced in the backend and fosters a clear UI display.
src/test/javascript/spec/component/overview/exercise-details/request-feedback-button/request-feedback-button.component.spec.ts (9)
59-59: Renaming topreliminaryFeedbackModule.Discontinuing
allowFeedbackRequestsand moving topreliminaryFeedbackModulealigns the front-end with back-end changes, providing clarity in the feedback flow. Thorough testing ensures correct error handling flow when the request fails.
78-78: Ensuring button visibility logic.Properly reading
preliminaryFeedbackModuleensures the request button is displayed only when conditions are met. The test confirms that the element is present in the DOM and is disabled if no valid submission is found.
107-107: Check for missing participation.Ensures consistent behavior when participations are undefined. The test logic is aligned with the new property usage (
preliminaryFeedbackModule).
126-126: Stable label and styling checks with Athena enabled.The direct usage of
preliminaryFeedbackModuleensures the button’s display logic is correct. The label is tested for correctness, aiding in user clarity.
149-149: Verifying the click event triggersrequestFeedback().Ensures that the updated feedback logic still permits the user to request feedback. The test coverage approach is robust.
176-176: Showing a warning when feedback wouldn't be beneficial.The test scenario helps confirm that the user is properly informed about existing feedback. Good user experience coverage.
194-194: Button disabling condition.Appropriately honors the updated property for incomplete or generating feedback states, preserving consistent UI state logic.
216-216: Button enabling condition.Verifies the correct behavior when the submission is set and not generating feedback. Confirms accurate usage of
preliminaryFeedbackModule.
231-243: Fallback to manual feedback requests.Introducing
allowManualFeedbackRequestsclarifies that the button remains visible even with Athena disabled, as long as manual feedback is allowed. The relevant test ensures correct UI behavior.src/main/webapp/app/entities/exercise.model.ts (4)
90-90: Property rename for clarity.Changing
allowFeedbackRequeststoallowManualFeedbackRequestsclarifies the nature of these requests as manual requests. This improves readability in the code and fosters consistent usage.
132-132: New property to reference suggestion module.Including
feedbackSuggestionModulein theExercisemodel ensures dynamic support for retrieving and controlling feedback suggestion logic on the client side.
163-163: DefaultingallowManualFeedbackRequeststo false.Avoids unintentional enabling of manual requests. This ensures the exercise’s default behavior remains consistent.
283-285: Resetting feedback properties for import.Clearing out
feedbackSuggestionModuleandpreliminaryFeedbackModuleupon import provides a fresh baseline for new exercises, avoiding data leaks from previously exported exercises.src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.java (3)
114-119: Proper stream usage.
The method callsresponse.data.stream().toList(). This is fine and concise. For large volumes, consider potential overhead, but for typical usage, this is acceptable.
136-141: Prevent potential confusion.
Ensure the module URL and suffix/feedback_suggestionsremains consistent across newisPreliminarychanges. If module toggles differ amongisPreliminarystates, ensure that is well documented.
164-167: Module endpoint usage.
Double-check the correctness of the endpoint suffix. The value appended togetAthenaModuleUrlplus"/feedback_suggestions"must match the feature's intended routes for preliminary or final feedback.src/main/webapp/app/exercises/programming/shared/lifecycle/programming-exercise-lifecycle.component.ts (1)
140-142: Toggle logic sync.
The toggle setsallowManualFeedbackRequeststo the opposite boolean value. This is straightforward. However, ensure that any dependent UI state or calls (e.g., removing or setting due dates) properly reflect the chosen approach.src/main/java/de/tum/cit/aet/artemis/athena/web/AthenaResource.java (1)
118-118: Use domain terminology in typed function.
The function accepts aFeedbackProviderwith the newly introducedisPreliminary. Confirm that all call sites align with the intended usage (graded vs. preliminary).src/test/javascript/spec/component/programming-exercise/programming-exercise-lifecycle.component.spec.ts (6)
17-19: Use consistent naming for newly imported components.
Confirm naming alignment with code references likeExerciseFeedbackSuggestionOptionsComponentvs.exercise-feedback-suggestion-options. The naming is consistent per Angular style guidelines.
40-40: Confirm usage ofExercisePreliminaryFeedbackOptionsComponent.
It’s declared as a mocked component. Ensure that any interactions with it are tested or properly stubbed if needed.
Line range hint
140-142: Concise naming in test descriptions.
The test name accurately reflects toggling manual feedback requests. Good approach; keep it consistent with the new domain vocabulary.
383-394: Exam-mode check.
The test checks thatExerciseFeedbackSuggestionOptionsComponentis not rendered in exam mode. Straightforward logic. This part is fine.
396-411: Athena service mocking.
Line 402 mocksisEnabled(). The test verifies correct rendering ofExercisePreliminaryFeedbackOptionsComponent. Perfect usage of the observable pattern.
413-429: Expanded negative test coverage.
Here,isEnabled()returnsfalse, so the preliminary feedback options component is not rendered. Good coverage. The presence of the checkbox ensures fallback UI.src/test/java/de/tum/cit/aet/artemis/athena/AthenaResourceIntegrationTest.java (13)
3-3: No comment needed for import statements.
Following the retrieved learnings, import statements are automatically managed by formatters in the Artemis project.
5-5: No comment needed for import statements.
Following the retrieved learnings, import statements are automatically managed by formatters in the Artemis project.
7-7: No comment needed for import statements.
Following the retrieved learnings, import statements are automatically managed by formatters in the Artemis project.
256-256: Module assignment for text feedback suggestions is valid.
The code correctly sets the feedback suggestion module for text exercises.
259-259: Mock expectation correctly configured.
The second parameter set tofalsealigns with the new approach for preliminary feedback checks.
269-269: Programming feedback suggestion module assignment.
Assigning the programming suggestions module ensures the test covers relevant logic.
272-272: Appropriate mocking for programming feedback suggestions.
This mock configuration is consistent with the newly introduced parameter usage.
283-283: Modeling feedback suggestion module assignment is correct.
The logic for modeling exercises parallels the text and programming approaches.
286-286: Mocking modeling feedback suggestions.
This aligns with the updated approach where we passfalsefor preliminary checks.
302-302: Consistent usage of the updated mock in programming feedback suggestions.
Maintains uniform handling across different exercise types.
315-315: Text feedback suggestion mock verification for unauthorized users.
The approach to forbid feedback suggestions for students is correct.
322-322: Programming feedback suggestion mock verification for unauthorized users.
Mirrors the same security checks as text and modeling tests.
330-330: Modeling feedback suggestion mock verification for unauthorized users.
Properly validates unauthorized access for modeling exercises.src/test/javascript/spec/component/overview/exercise-details/exercise-details-student-actions.component.spec.ts (2)
36-36: No comment needed for import statements.
As per Artemis conventions, import statements are managed automatically.
79-79: Correct inclusion of the new component in TestBed.
RegisteringRequestFeedbackButtonComponentunderimportsensures that it can be properly tested.src/main/java/de/tum/cit/aet/artemis/programming/domain/ProgrammingExercise.java (2)
687-687: Manual feedback request logic looks consistent.
By switching togetAllowManualFeedbackRequests(), the code aligns with the new naming and usage.
801-801: Early return for manual feedback condition is clear.
This ensures no further checks occur if manual feedback requests are not permitted.src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseExportImportResource.java (4)
44-44: No comment needed for import statements.
As per the retrieved learnings, import statements should not be commented on for Artemis.
247-248: Properly documenting Athena module checks.
These comments clearly explain the new logic for validating module usage and disabling it if unavailable.
250-251: Access check for FEEDBACK_SUGGESTIONS module.
The approach ensures unauthorized modules are disabled, preventing unintended usage.
256-262: Access check for PRELIMINARY_FEEDBACK module.
This block parallels the feedback suggestion logic, helping maintain consistent security checks.src/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.java (5)
43-43: New import forModuleType
Good addition to reference the new enum.
229-232: Check returned exception handling forfeedbackSuggestionModule
The fallback logic here is correct. IfathenaModuleServiceis not present, it gracefully disables the feature.
538-539: Properly handle exceptions duringfeedbackSuggestionModuleimport
The catch block sets the module to null, ensuring the import doesn't fail the entire request.
544-545: Same fallback logic forpreliminaryFeedbackModule
Consistent fallback strategy is beneficial.
549-550: Graceful handling of exceptions
Nulling the module is a clean fallback approach but ensure logs are captured for debugging.src/main/webapp/app/exercises/shared/dashboards/tutor/exercise-assessment-dashboard.component.ts (1)
335-335: Use ofallowManualFeedbackRequests
ReplacingallowFeedbackRequestswithallowManualFeedbackRequestsaligns the naming consistently with the backend changes.src/main/java/de/tum/cit/aet/artemis/exercise/domain/Exercise.java (7)
104-104: Renaming toallowManualFeedbackRequests
This more specific name clearly distinguishes between automated vs. manual feedback.
148-150: IntroducingpreliminaryFeedbackModule
Storing the module name at the entity level looks consistent withfeedbackSuggestionModule.
253-254: Added gettergetAllowManualFeedbackRequests()
Straightforward accessor matching the renamed field.
257-258: Added settersetAllowManualFeedbackRequests(...)
Ensures consistent naming and function signature.
839-841: Getter forpreliminaryFeedbackModule
Supplies new property to the rest of the system.
843-845: Setter forpreliminaryFeedbackModule
Matches the established naming pattern.
851-853:isPreliminaryFeedbackEnabled()
Provides an immediate null check for the newly introducedpreliminaryFeedbackModule.src/main/java/de/tum/cit/aet/artemis/exercise/service/SubmissionService.java (1)
700-700: Refined condition usinggetAllowManualFeedbackRequests()
All consistency checks with the newly named property appear valid.src/main/webapp/app/exercises/programming/manage/programming-exercise-detail.component.ts (1)
591-599: Confirm boolean conversion logic.These lines coerce module properties into booleans using
!!. Ensure these fields (feedbackSuggestionModule,preliminaryFeedbackModule) are set to the expected type or can be reliably converted to boolean. Otherwise, this might cause unintended UI behavior if the properties are not properly initialized as booleans.src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingAssessmentIntegrationTest.java (1)
957-957: Renamed property appears correct.Switching from a generic feedback request to a manual feedback request aligns with the new naming convention across the codebase. No issues spotted.
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java (3)
44-44: Import statement update is straightforward.This import for
ModuleTypeis expected if you’re referencing the Athena modules. No issues here.
250-253: Ensuring module access check consistency.The
ifPresentOrElselogic properly defaults the module to null if access is disallowed. Looks consistent with the intended design.
351-354: Parallel module checks for updates.These lines mirror the creation logic to re-check access on updates. No concerns found; it maintains consistency.
src/test/javascript/spec/component/programming-exercise/programming-exercise-update.component.spec.ts (1)
1349-1352: Import verification of feedback modules.Verifying that
allowManualFeedbackRequests,feedbackSuggestionModule, andpreliminaryFeedbackModuleare reset upon import is correct for ensuring a fresh state. No issues found in these assertions.src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.html (2)
1-1: LGTM! Following new Angular syntax.The usage of
@ifinstead of*ngIffollows the new Angular syntax as specified in the coding guidelines.
25-27: LGTM! Proper usage of @for syntax.The new
@forsyntax with track expression is correctly implemented.src/main/webapp/app/exercises/shared/feedback-suggestion/exercise-feedback-suggestion-options.component.html (1)
8-8: LGTM! Consistent feedback handling.The changes to checkbox binding and translation keys align well with the PR's objective of improving feedback options.
Also applies to: 17-17, 19-19
src/main/webapp/i18n/en/textExercise.json (1)
Line range hint
1-48: Verify replacement translations for removed feedback keysThe removal of Athena-specific feedback keys aligns with the PR objective to generalize the feedback model selection. However, we should verify if these translations have been replaced elsewhere.
✅ Verification successful
Removal of Athena-specific translations is appropriate
The removed translations
allowPreliminaryAthenaFeedbackRequestsandallowPreliminaryAthenaFeedbackRequestsTooltipwere Athena-specific. The codebase already has generic feedback request translations inprogrammingExercise.jsonthat can be reused for the generalized feedback model selection. This aligns with the PR's objective to make the feedback system more generic.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for new feedback-related translations that might replace the removed ones rg -g '*.json' -l 'preliminaryFeedback|feedbackRequests' src/main/webapp/i18n/Length of output: 178
Script:
#!/bin/bash # Check content of programming exercise files for feedback-related translations rg -g '*.json' 'preliminaryFeedback|feedbackRequests' src/main/webapp/i18n/Length of output: 590
src/main/webapp/i18n/de/textExercise.json (1)
Line range hint
1-48: LGTM! German translations follow guidelinesThe German translations maintain consistency with the English file changes and correctly use informal language (dutzen) as required by the coding guidelines.
src/main/webapp/app/exercises/text/participate/text-editor.component.html (1)
Line range hint
27-36: LGTM! Template follows new Angular syntax guidelinesThe template correctly uses the new
@ifsyntax instead of*ngIfas required by the coding guidelines.src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise-update.component.html (1)
217-220: Well-structured addition of feedback options section!The new feedback section is logically placed after grading instructions and properly guarded against exam mode. This aligns well with the PR objective of allowing users to choose preliminary feedback models while maintaining exam mode functionality.
src/main/webapp/app/exercises/programming/shared/lifecycle/programming-exercise-lifecycle.component.html (1)
89-89: Property rename improves clarityThe rename from
allowFeedbackRequeststoallowManualFeedbackRequestsbetter describes the specific type of feedback being controlled.src/main/webapp/app/overview/exercise-details/exercise-details-student-actions.component.html (2)
140-142: Clean simplification of programming exercise feedback conditionThe simplified condition improves code readability while maintaining the required logic.
Line range hint
234-241: Verify exercise type handling for feedback requestsThe condition correctly handles text and modeling exercises, but let's verify the exhaustive handling of exercise types.
✅ Verification successful
Let me gather more information about feedback request functionality to ensure we have a complete picture.
Let me try one more search to find the request feedback component and its usage.
Exercise type handling for feedback requests is correctly implemented
Based on the code analysis, the handling of exercise types for feedback requests is properly implemented:
- The code correctly handles all exercise types defined in the
ExerciseTypeenum: PROGRAMMING, MODELING, QUIZ, TEXT, and FILE_UPLOAD.- The feedback request button's visibility and behavior is appropriately controlled based on:
- Exercise type (TEXT and MODELING exercises have specific handling)
- Athena enablement status
- Submission status
- Exercise deadline
- Manual feedback request settings
The implementation includes proper validation through
assureConditionsSatisfied()method with specific handling for TEXT/MODELING exercises viaassureTextModelingConditions().🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any exercise types that might need feedback support # Look for exercise type definitions and their usage in feedback contexts # Find exercise type enum definition ast-grep --pattern 'enum ExerciseType { $$$ }' # Search for feedback-related code with exercise types rg -A 5 'exercise.type.*feedback'Length of output: 3637
Script:
#!/bin/bash # Search for request-feedback component implementation and its usage ast-grep --pattern 'class RequestFeedbackButtonComponent { $$$ }' # Search for feedback request related conditions rg -A 3 'RequestFeedback.*exercise\.type'Length of output: 237
Script:
#!/bin/bash # Search for request-feedback component files and their content fd request-feedback -t f -x cat {} # Search for conditions related to exercise types and feedback rg -A 5 '@if.*exercise\.type.*feedback|ngIf.*exercise\.type.*feedback'Length of output: 18481
src/main/webapp/app/exercises/modeling/manage/modeling-exercise-update.component.html (1)
271-274: LGTM! Verify the preliminary feedback component.The feedback section is properly implemented using the new Angular
@ifsyntax and correctly positioned after the grading section.Let's verify the preliminary feedback component exists:
✅ Verification successful
Component integration verified successfully
The preliminary feedback component exists at the correct location and has all the required inputs properly defined:
- Located at:
src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.ts- Accepts the expected
@Input()properties:
exercise: ExercisedueDate?: dayjs.DayjsreadOnly: boolean(optional with default false)The component usage in the template is correct and matches the component's interface.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the preliminary feedback component exists and has the expected props ast-grep --pattern 'class ExercisePreliminaryFeedbackOptionsComponent { $$$ @Input() exercise: $_ @Input() dueDate: $_ $$$ }'Length of output: 259
Script:
#!/bin/bash # Let's try a different approach to find the component # First, let's find the component file fd -t f "exercise-preliminary-feedback-options.component" # Then let's check for Input decorators in TypeScript files rg "@Input\(\)" -A 2 -g "exercise-preliminary-feedback-options.component.ts"Length of output: 1061
src/main/webapp/app/exercises/modeling/participate/modeling-submission.component.html (1)
27-29: LGTM! Verify the Exercise class property.The condition for showing the feedback request button has been updated to use
preliminaryFeedbackModulewhile maintaining the same due date check logic.Let's verify the property exists in the Exercise class:
✅ Verification successful
Property exists and is correctly used in the codebase
The
preliminaryFeedbackModuleproperty is properly defined in the Exercise class and used consistently throughout the codebase:
- It's defined in
Exercise.javaas a string property with the column namepreliminary_feedback_module- The property exists in the TypeScript model (
exercise.model.ts)- It's used consistently in various components for feedback request functionality
- The property is properly tested in multiple test files
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the preliminaryFeedbackModule property exists in Exercise class ast-grep --pattern 'class Exercise { $$$ preliminaryFeedbackModule: $_ $$$ }'Length of output: 209
Script:
#!/bin/bash # Let's try a different approach to find the property in any class definition ast-grep --pattern 'preliminaryFeedbackModule: $_' # Also search for the property name in the codebase using ripgrep rg "preliminaryFeedbackModule" -A 2 -B 2Length of output: 23460
src/main/webapp/i18n/en/exercise.json (1)
176-179: LGTM! Translation keys are clear and well-documented.The new translation keys for Athena feedback functionality are properly structured and provide clear explanations through tooltips.
src/main/webapp/i18n/de/exercise.json (1)
176-179: LGTM! German translations follow guidelines.The translations correctly use informal language (dutzen) and maintain consistent terminology with other feedback-related translations in the file.
src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise.module.ts
Show resolved
Hide resolved
...ript/spec/component/exercises/shared/feedback/preliminary-feedback-options.component.spec.ts
Show resolved
Hide resolved
| final RequestDTO request = new RequestDTO(athenaDTOConverterService.ofExercise(exercise), athenaDTOConverterService.ofSubmission(exercise.getId(), submission), | ||
| isPreliminary); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Parameter naming adjustment.
Similar to RequestDTO, rename isGraded in the record’s third field to isPreliminary for clarity.
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.java
Outdated
Show resolved
Hide resolved
...javascript/spec/component/programming-exercise/programming-exercise-update.component.spec.ts
Show resolved
Hide resolved
...p/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.html
Show resolved
Hide resolved
...app/exercises/shared/feedback-suggestion/exercise-feedback-suggestion-options.component.html
Show resolved
Hide resolved
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 pmd (7.8.0)src/main/java/de/tum/cit/aet/artemis/athena/domain/ModuleType.javaThe following rules are missing or misspelled in your ruleset file category/vm/bestpractices.xml: BooleanInstantiation, DontImportJavaLang, DuplicateImports, EmptyFinallyBlock, EmptyIfStmt, EmptyInitializer, EmptyStatementBlock, EmptyStatementNotInLoop, EmptySwitchStatements, EmptySynchronizedBlock, EmptyTryBlock, EmptyWhileStmt, ExcessiveClassLength, ExcessiveMethodLength, ImportFromSamePackage, MissingBreakInSwitch, SimplifyBooleanAssertion. Please check your ruleset configuration. src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSendingService.javaThe following rules are missing or misspelled in your ruleset file category/vm/bestpractices.xml: BooleanInstantiation, DontImportJavaLang, DuplicateImports, EmptyFinallyBlock, EmptyIfStmt, EmptyInitializer, EmptyStatementBlock, EmptyStatementNotInLoop, EmptySwitchStatements, EmptySynchronizedBlock, EmptyTryBlock, EmptyWhileStmt, ExcessiveClassLength, ExcessiveMethodLength, ImportFromSamePackage, MissingBreakInSwitch, SimplifyBooleanAssertion. Please check your ruleset configuration. src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.javaThe following rules are missing or misspelled in your ruleset file category/vm/bestpractices.xml: BooleanInstantiation, DontImportJavaLang, DuplicateImports, EmptyFinallyBlock, EmptyIfStmt, EmptyInitializer, EmptyStatementBlock, EmptyStatementNotInLoop, EmptySwitchStatements, EmptySynchronizedBlock, EmptyTryBlock, EmptyWhileStmt, ExcessiveClassLength, ExcessiveMethodLength, ImportFromSamePackage, MissingBreakInSwitch, SimplifyBooleanAssertion. Please check your ruleset configuration.
WalkthroughThis pull request introduces a comprehensive refactoring of the feedback mechanism in the Artemis platform, focusing on distinguishing between manual feedback requests and preliminary AI-generated feedback suggestions. The changes span multiple components across backend and frontend, introducing a new Changes
Sequence DiagramsequenceDiagram
participant Student
participant Exercise
participant AthenaService
participant FeedbackProvider
alt Manual Feedback Request
Student->>Exercise: Request Manual Feedback
Exercise->>AthenaService: Check Manual Feedback Enabled
end
alt Preliminary AI Feedback
Student->>Exercise: Request Preliminary Feedback
Exercise->>AthenaService: Check Preliminary Feedback Module
AthenaService->>FeedbackProvider: Generate Preliminary Feedback
FeedbackProvider-->>Student: Return Preliminary Feedback
end
Possibly related PRs
Suggested Labels
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (39)
src/main/webapp/app/exercises/modeling/participate/modeling-submission.component.html (1)
27-29: Simplify template expression and remove unnecessarythis.prefix.The condition correctly uses the new
@ifsyntax and implements the preliminary feedback module check. However, the template expression can be simplified.Apply this diff to improve readability:
- @if ( - modelingExercise.preliminaryFeedbackModule && (!this.modelingExercise.dueDate || !hasExerciseDueDatePassed(this.modelingExercise, this.participation)) - ) { + @if (modelingExercise.preliminaryFeedbackModule && (!modelingExercise.dueDate || !hasExerciseDueDatePassed(modelingExercise, participation))) {The changes:
- Remove unnecessary line breaks for a single condition
- Remove redundant
this.prefix as it's not needed in Angular templates- Maintain the same logical checks while improving readability
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseExportImportResource.java (2)
250-251: Optional usage is concise but consider centralizing error handling.Utilizing
ifPresentOrElseis a neat way to handle the service presence. However, consider extracting repeated module checks (e.g., forModuleType.FEEDBACK_SUGGESTIONSandModuleType.PRELIMINARY_FEEDBACK) into a small helper method to reduce duplication and centralize any future modifications to the error-handling flow.
256-262: Avoid duplicating logic for the second Athena module check.The second try-catch block for
PRELIMINARY_FEEDBACKis nearly identical to the first block forFEEDBACK_SUGGESTIONS. Refactoring both checks into a single helper function that accepts theModuleTypeparameter would promote the Single Responsibility Principle and streamline this code.src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaModuleService.java (3)
115-126: Validate exercise types exhaustively.
The enhanced switch expression is clean and readable. However, consider a fallback for unexpected or future exercise types.+ default -> { + throw new IllegalArgumentException("Unrecognized or unsupported exercise type: " + exerciseType); + }
139-146: Improve error context for restricted modules.
This logic is straightforward. If you want clearer debugging, consider logging the restricted module and course details before throwing the exception.
150-156: Add default branch ingetModuleswitch statement.
Currently, the method only handles two module types. If more module types are added in the future, you might forget to extend this switch. A default or exhaustive check ensures coverage.switch (moduleType) { case ModuleType.FEEDBACK_SUGGESTIONS -> module = exercise.getFeedbackSuggestionModule(); case ModuleType.PRELIMINARY_FEEDBACK -> module = exercise.getPreliminaryFeedbackModule(); + default -> throw new IllegalArgumentException("Unsupported module type: " + moduleType); }src/main/webapp/app/exercises/programming/shared/lifecycle/programming-exercise-lifecycle.module.ts (1)
11-11: Consider lazy-loading or feature modules if sizing grows.
Including several standalone components directly in theimportsarray is valid. Should it expand further, you may want to group them in a dedicated feature module for maintainability.src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.ts (2)
39-47: Handle potential errors from Athena service calls.
Currently, there's no error handling for the subscription. Consider a fallback or user-facing message if the call fails.
74-81: Strongly type the event parameter intogglePreliminaryFeedback.
Usinganymight introduce unexpected behavior. Alternatively, specifyevent: MouseEventor the relevant type.- togglePreliminaryFeedback(event: any) { + togglePreliminaryFeedback(event: Event) { ... }src/main/webapp/app/exercises/shared/feedback-suggestion/exercise-feedback-suggestion-options.component.ts (1)
32-32: Ensure UI state is properly tracked.
showDropdownListis a clear property name. Track potential user toggles if you plan to store or restore state across sessions.src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSendingService.java (1)
98-102: Consider centralizing URL construction logic.
The approach to build the URL withexercise.getExerciseType()andexercise.getFeedbackSuggestionModule()appears duplicated in multiple services (e.g.,AthenaSubmissionSelectionService). Extracting this into a shared method might reduce maintenance overhead and ensure consistency.src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaSubmissionSelectionService.java (1)
86-88: Unify duplicated Athena URL construction with other services.
Similar logic appears inAthenaFeedbackSendingService. Consider refactoring to a shared helper method inAthenaModuleServiceor another utility class to avoid duplication.src/test/javascript/spec/component/exercises/shared/feedback/feedback-suggestion-option.component.spec.ts (2)
10-10: Confirm that ArtemisTestModule is needed.
If specific testing utilities fromArtemisTestModuleare not used, consider removing the unused import to reduce overhead. Otherwise, this looks fine.
101-113: Expand coverage for toggling feedback suggestions in various scenarios.
The updated test checks toggling for “any exercise”. While this is a good start, consider adding separate tests for each exercise type to ensure coverage of corner cases and different states (e.g., no modules available, due date passed, read-only).src/test/javascript/spec/component/exercises/shared/feedback/preliminary-feedback-options.component.spec.ts (6)
1-11: Use more granular mocks for dependencies [jest best practices].In test files under
spec/**/*.ts, try to avoid importing entire modules if you only need a few components or functions. Consider using isolated mocks or stubs for better test performance and reduced coupling.
12-29: Enforce meaningful suite-level description.Your test suite title matches the component's name, which is good, but consider adding a short detail to illustrate the scope of functionalities covered (e.g., "Exercise Preliminary Feedback Options Component – Basic Interactions and State"). This can help with test reporting clarity.
39-49: Assert more specific outcome regarding modules.Beyond checking
modulesAvailable, consider verifying that the dropdown or any UI element is updated properly in the DOM for a fully comprehensive test, if feasible.
74-79: Critical path coverage for due date checks.Your test verifies the due date condition. Good job. Also, ensure coverage if
dueDateis null or undefined.
89-96: Consistent style expectations across different exercise types.Similar to the previous test, ensure that style logic remains consistent for other exercise types (e.g., TEXT, MODELING). In a real scenario, a single style function might handle them differently.
97-114: Meaningful toggling logic test.This test effectively checks the toggling of the preliminary feedback module and the dropdown list logic. Consider also verifying conditions where multiple modules might be recommended (e.g., if you had more than two modules).
src/main/webapp/app/overview/exercise-details/request-feedback-button/request-feedback-button.component.ts (1)
52-56: Prioritize comprehensibility of conditional logic.Separating the two conditional branches (one for
athenaEnabledand one forallowManualFeedbackRequests) helps clarity. Further, consider logging or user feedback for cases when neither condition is met.src/test/javascript/spec/component/overview/exercise-details/request-feedback-button/request-feedback-button.component.spec.ts (3)
78-78: Clarify the comment about exam exercises.The inline comment mentions "course undefined means exam exercise," yet here
courseis an empty object. Consider aligning the code or comment for clearer maintainability.
176-176: Recommend verifying compatibility with manual feedback requests.Here, Athena is enabled, but the test scenario focuses on preliminary feedback. Confirm that any manual feedback paths also remain unaffected.
194-194: Maintain consistent property usage for consistent UI states.This code sets
preliminaryFeedbackModuleagain. Confirm that the UI state is correctly toggled or disabled in the presence of multiple feedback properties.src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.java (2)
133-134: Logged messages clarify the type of feedback requested.Using
"Graded"vs."Non Graded"is helpful, but consider standardizing these terms to match theisPreliminarynomenclature.
153-155: Method signature and logging rework for modeling also looks good.The “Graded” vs. “Non Graded” phrasing is repeated here—ensure consistency in logs.
src/main/webapp/app/exercises/programming/shared/lifecycle/programming-exercise-lifecycle.component.ts (1)
140-142: Consider documenting the reason for resetting dates when manual feedback requests are enabled.
WhenallowManualFeedbackRequestsis set totrue, the code sets bothassessmentDueDateandbuildAndTestStudentSubmissionsAfterDueDatetoundefined. A brief comment explaining the underlying rationale would improve understandability for future maintainers.src/main/java/de/tum/cit/aet/artemis/athena/web/AthenaResource.java (2)
108-114: Consider using an enum instead of a raw Boolean.
UsingBooleanto denote a “preliminary” vs. “graded” state can be less expressive. An enum may provide more clarity and reduce ambiguity.
133-134: Clarify the logic behind passingfalseasisPreliminary.
The comment suggests this endpoint is “only for graded feedback suggestions,” which impliesisPreliminary = falseis correct. Consider adding a short in-code explanation for future readers.src/test/java/de/tum/cit/aet/artemis/core/connector/AthenaRequestMockProvider.java (1)
112-120: MethodgetTestModuleNameis a useful abstraction.
Generating the name based onmoduleTypeandisPreliminaryis straightforward. If these patterns grow more complex, consider an enum or central configuration to avoid string concatenation pitfalls.src/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.java (3)
535-536: Cohesive handling of Athena imports in exercise imports.This block indicates a new check on imported exercise modules for Athena features. If missing or unauthorized, it sets them to null. Ensure the calling code gracefully handles these nulls, especially when merging exercise configurations.
Would you like a helper function or central point to unify these checks for all exercise types?
538-539: Catch and disable feedback suggestions upon import.The explicit
BadRequestAlertExceptioncatch disables suggestions. Confirm that partial import states do not linger. Maybe log an INFO-level message for better traceability.
544-550: Refine repeated error handling across modules.This block similarly guards
ModuleType.PRELIMINARY_FEEDBACK. Consolidating these repeated try-catch blocks into a single or iterative structure might improve readability.src/main/java/de/tum/cit/aet/artemis/exercise/domain/Exercise.java (2)
104-104: Use consistent naming convention for booleans.While this field is self-explanatory, consider renaming the field or its getter to match typical boolean naming patterns (e.g.,
isAllowManualFeedbackRequests) in a future refactor for clarity.
253-254: Getter naming alignment.The getter name
getAllowManualFeedbackRequests()is functionally correct but slightly departs from typical “isX” boolean naming. Updating to maintain a consistent approach could improve clarity.src/test/javascript/spec/component/programming-exercise/programming-exercise-update.component.spec.ts (1)
1376-1378: Inconsistent String Delimiter forpreliminaryFeedbackModuleNotice the ending quote in
'athena-module-2"'. This might be a minor typo, which can cause inconsistency or errors.- programmingExercise.preliminaryFeedbackModule = 'athena-module-2"'; + programmingExercise.preliminaryFeedbackModule = 'athena-module-2';src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.html (1)
1-14: Conditional Rendering for Preliminary Feedback CheckboxThis block is clearly gated by
isAthenaEnabled$andmodulesAvailable. Binding the checkbox toshowDropdownListis straightforward, though thename="allowFeedbackRequests"HTML attribute might benefit from aligning with theallowManualFeedbackRequestsnaming convention used in the TypeScript code. This would reinforce internal consistency across the codebase.src/main/webapp/app/assessment/assessment-warning/assessment-warning.component.ts (1)
Line range hint
19-33: Consider improving accessibility of the warning message.While the implementation is functionally correct, consider enhancing accessibility:
- Use
role="alert"for the warning message- Add
aria-labelto the icon- <div class="card-header"> + <div class="card-header" role="alert"> - <fa-icon [icon]="faExclamationTriangle" size="2x" class="text-warning" placement="bottom auto" /> + <fa-icon [icon]="faExclamationTriangle" size="2x" class="text-warning" placement="bottom auto" + aria-label="Warning" />src/main/webapp/app/exercises/programming/shared/lifecycle/programming-exercise-lifecycle.component.html (1)
203-224: Consider adding loading state for Athena availability check.The feedback section is well-structured with proper conditional rendering based on Athena availability. However, consider adding a loading state while checking Athena availability to improve user experience.
@if (isAthenaEnabled$ | async) { <jhi-exercise-preliminary-feedback-options [exercise]="exercise" [dueDate]="exercise.dueDate" [readOnly]="readOnly" /> } @else { + @if (isAthenaEnabled$ | async) === undefined) { + <div class="text-center"> + <div class="spinner-border" role="status"> + <span class="visually-hidden">Loading...</span> + </div> + </div> + } @else { <div class="form-check"> <input type="checkbox" class="form-check-input" name="allowManualFeedbackRequests" [checked]="exercise.allowManualFeedbackRequests" id="allowManualFeedbackRequests" (change)="toggleManualFeedbackRequests()" /> <label class="form-control-label" for="allowManualFeedbackRequests" jhiTranslate="artemisApp.programmingExercise.timeline.manualFeedbackRequests"></label> <jhi-help-icon placement="right auto" [text]="'artemisApp.programmingExercise.timeline.manualFeedbackRequestsTooltip'" /> </div> + } }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/main/resources/config/liquibase/changelog/20241212135412_changelog.xmlis excluded by!**/*.xmlsrc/main/resources/config/liquibase/master.xmlis excluded by!**/*.xml
📒 Files selected for processing (62)
src/main/java/de/tum/cit/aet/artemis/athena/domain/ModuleType.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSendingService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaModuleService.java(3 hunks)src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaRepositoryExportService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaSubmissionSelectionService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaSubmissionSendingService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/athena/web/AthenaResource.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/communication/dto/ChannelDTO.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exercise/domain/Exercise.java(4 hunks)src/main/java/de/tum/cit/aet/artemis/exercise/service/SubmissionService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/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/assessment-warning/assessment-warning.component.ts(1 hunks)src/main/webapp/app/entities/exercise.model.ts(4 hunks)src/main/webapp/app/exercises/modeling/manage/modeling-exercise-detail.component.ts(1 hunks)src/main/webapp/app/exercises/modeling/manage/modeling-exercise-update.component.html(1 hunks)src/main/webapp/app/exercises/modeling/manage/modeling-exercise-update.component.ts(1 hunks)src/main/webapp/app/exercises/modeling/manage/modeling-exercise.module.ts(2 hunks)src/main/webapp/app/exercises/modeling/participate/modeling-submission.component.html(1 hunks)src/main/webapp/app/exercises/programming/manage/programming-exercise-detail.component.ts(1 hunks)src/main/webapp/app/exercises/programming/manage/update/programming-exercise-update.helper.ts(2 hunks)src/main/webapp/app/exercises/programming/shared/lifecycle/programming-exercise-lifecycle.component.html(3 hunks)src/main/webapp/app/exercises/programming/shared/lifecycle/programming-exercise-lifecycle.component.ts(2 hunks)src/main/webapp/app/exercises/programming/shared/lifecycle/programming-exercise-lifecycle.module.ts(1 hunks)src/main/webapp/app/exercises/shared/dashboards/tutor/exercise-assessment-dashboard.component.ts(1 hunks)src/main/webapp/app/exercises/shared/feedback-suggestion/exercise-feedback-suggestion-options.component.html(2 hunks)src/main/webapp/app/exercises/shared/feedback-suggestion/exercise-feedback-suggestion-options.component.ts(3 hunks)src/main/webapp/app/exercises/shared/feedback-suggestion/exercise-feedback-suggestion-options.module.ts(0 hunks)src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.html(1 hunks)src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.ts(1 hunks)src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise-detail.component.ts(1 hunks)src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise-update.component.html(1 hunks)src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise.module.ts(2 hunks)src/main/webapp/app/exercises/text/participate/text-editor.component.html(1 hunks)src/main/webapp/app/overview/exercise-details/exercise-details-student-actions.component.html(2 hunks)src/main/webapp/app/overview/exercise-details/request-feedback-button/request-feedback-button.component.ts(1 hunks)src/main/webapp/i18n/de/exercise.json(1 hunks)src/main/webapp/i18n/de/programmingExercise.json(0 hunks)src/main/webapp/i18n/de/textExercise.json(1 hunks)src/main/webapp/i18n/en/exercise.json(1 hunks)src/main/webapp/i18n/en/programmingExercise.json(0 hunks)src/main/webapp/i18n/en/textExercise.json(1 hunks)src/test/java/de/tum/cit/aet/artemis/athena/AthenaResourceIntegrationTest.java(6 hunks)src/test/java/de/tum/cit/aet/artemis/athena/service/connectors/AthenaFeedbackSuggestionsServiceTest.java(6 hunks)src/test/java/de/tum/cit/aet/artemis/core/connector/AthenaRequestMockProvider.java(3 hunks)src/test/java/de/tum/cit/aet/artemis/exercise/participation/ParticipationIntegrationTest.java(4 hunks)src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingAssessmentIntegrationTest.java(1 hunks)src/test/javascript/spec/component/assessment-dashboard/exercise-assessment-dashboard.component.spec.ts(1 hunks)src/test/javascript/spec/component/exercises/shared/feedback/feedback-suggestion-option.component.spec.ts(3 hunks)src/test/javascript/spec/component/exercises/shared/feedback/preliminary-feedback-options.component.spec.ts(1 hunks)src/test/javascript/spec/component/modeling-exercise/modeling-exercise-update.component.spec.ts(0 hunks)src/test/javascript/spec/component/overview/exercise-details/exercise-details-student-actions.component.spec.ts(2 hunks)src/test/javascript/spec/component/overview/exercise-details/request-feedback-button/request-feedback-button.component.spec.ts(9 hunks)src/test/javascript/spec/component/programming-exercise/programming-exercise-lifecycle.component.spec.ts(6 hunks)src/test/javascript/spec/component/programming-exercise/programming-exercise-update.component.spec.ts(2 hunks)src/test/javascript/spec/component/text-exercise/text-exercise-update.component.spec.ts(0 hunks)
💤 Files with no reviewable changes (5)
- src/main/webapp/i18n/de/programmingExercise.json
- src/test/javascript/spec/component/modeling-exercise/modeling-exercise-update.component.spec.ts
- src/main/webapp/app/exercises/shared/feedback-suggestion/exercise-feedback-suggestion-options.module.ts
- src/test/javascript/spec/component/text-exercise/text-exercise-update.component.spec.ts
- src/main/webapp/i18n/en/programmingExercise.json
✅ Files skipped from review due to trivial changes (2)
- src/main/webapp/app/exercises/modeling/manage/modeling-exercise-update.component.ts
- src/main/java/de/tum/cit/aet/artemis/athena/domain/ModuleType.java
🧰 Additional context used
📓 Path-based instructions (53)
src/main/webapp/app/exercises/modeling/participate/modeling-submission.component.html (1)
Pattern src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaSubmissionSendingService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/webapp/app/assessment/assessment-warning/assessment-warning.component.ts (1)
src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise-update.component.html (1)
Pattern src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.
src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingAssessmentIntegrationTest.java (1)
Pattern 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/main/webapp/app/exercises/shared/dashboards/tutor/exercise-assessment-dashboard.component.ts (1)
src/main/webapp/app/exercises/text/participate/text-editor.component.html (1)
Pattern src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.
src/main/webapp/app/exercises/modeling/manage/modeling-exercise-detail.component.ts (1)
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaRepositoryExportService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/webapp/app/exercises/modeling/manage/modeling-exercise.module.ts (1)
src/main/webapp/i18n/de/textExercise.json (1)
Pattern src/main/webapp/i18n/de/**/*.json: German language translations should be informal (dutzen) and should never be formal (sietzen). So the user should always be addressed with "du/dein" and never with "sie/ihr".
src/test/javascript/spec/component/overview/exercise-details/exercise-details-student-actions.component.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise-detail.component.ts (1)
src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.html (1)
Pattern src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.
src/test/javascript/spec/component/overview/exercise-details/request-feedback-button/request-feedback-button.component.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/main/java/de/tum/cit/aet/artemis/programming/domain/ProgrammingExercise.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/webapp/app/exercises/programming/manage/update/programming-exercise-update.helper.ts (1)
src/main/java/de/tum/cit/aet/artemis/exercise/service/SubmissionService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/webapp/app/exercises/shared/feedback-suggestion/exercise-feedback-suggestion-options.component.html (1)
Pattern src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.
src/main/webapp/i18n/de/exercise.json (1)
Pattern src/main/webapp/i18n/de/**/*.json: German language translations should be informal (dutzen) and should never be formal (sietzen). So the user should always be addressed with "du/dein" and never with "sie/ihr".
src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise.module.ts (1)
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaSubmissionSelectionService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/test/javascript/spec/component/assessment-dashboard/exercise-assessment-dashboard.component.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseExportImportResource.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/webapp/app/exercises/modeling/manage/modeling-exercise-update.component.html (1)
Pattern src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.
src/test/javascript/spec/component/programming-exercise/programming-exercise-lifecycle.component.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/main/webapp/app/entities/exercise.model.ts (1)
src/main/webapp/app/exercises/programming/shared/lifecycle/programming-exercise-lifecycle.component.ts (1)
src/main/webapp/app/exercises/programming/shared/lifecycle/programming-exercise-lifecycle.module.ts (1)
src/main/webapp/app/overview/exercise-details/exercise-details-student-actions.component.html (1)
Pattern src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.
src/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/test/java/de/tum/cit/aet/artemis/athena/AthenaResourceIntegrationTest.java (1)
Pattern 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/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSendingService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingAssessmentService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/webapp/app/overview/exercise-details/request-feedback-button/request-feedback-button.component.ts (1)
src/test/java/de/tum/cit/aet/artemis/athena/service/connectors/AthenaFeedbackSuggestionsServiceTest.java (1)
Pattern 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/javascript/spec/component/programming-exercise/programming-exercise-update.component.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/main/webapp/app/exercises/programming/manage/programming-exercise-detail.component.ts (1)
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/test/java/de/tum/cit/aet/artemis/exercise/participation/ParticipationIntegrationTest.java (1)
Pattern 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/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.ts (1)
src/main/webapp/app/exercises/shared/feedback-suggestion/exercise-feedback-suggestion-options.component.ts (1)
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingSubmissionResource.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/webapp/app/exercises/programming/shared/lifecycle/programming-exercise-lifecycle.component.html (1)
Pattern src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.
src/main/java/de/tum/cit/aet/artemis/exercise/domain/Exercise.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/test/javascript/spec/component/exercises/shared/feedback/feedback-suggestion-option.component.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/test/javascript/spec/component/exercises/shared/feedback/preliminary-feedback-options.component.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/main/java/de/tum/cit/aet/artemis/communication/dto/ChannelDTO.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/test/java/de/tum/cit/aet/artemis/core/connector/AthenaRequestMockProvider.java (1)
Pattern 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/main/java/de/tum/cit/aet/artemis/athena/service/AthenaModuleService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/athena/web/AthenaResource.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
📓 Learnings (2)
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingAssessmentService.java (1)
Learnt from: undernagruzez
PR: ls1intum/Artemis#8498
File: src/main/java/de/tum/in/www1/artemis/service/programming/ProgrammingExerciseCodeReviewFeedbackService.java:236-236
Timestamp: 2024-11-12T12:51:40.391Z
Learning: Rate limit exceptions for AI feedback requests are handled in the client app.
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java (1)
Learnt from: Hialus
PR: ls1intum/Artemis#8607
File: src/main/java/de/tum/in/www1/artemis/web/rest/programming/ProgrammingExerciseResource.java:64-64
Timestamp: 2024-11-12T12:51:51.201Z
Learning: For the Artemis project, import statements are automatically managed by formatters and should not be commented on.
🔇 Additional comments (130)
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseExportImportResource.java (2)
44-44: Well-structured import usage.Good job on importing just the necessary enum
ModuleTyperather than using a wildcard import, adhering to the guideline to avoid star imports.
247-248: Clear and concise documentation.These new comments effectively clarify the purpose of the Athena modules check, informing future maintainers of the rationale behind disabling feedback functionalities if appropriate module access is not granted.
src/main/webapp/app/overview/exercise-details/exercise-details-student-actions.component.html (2)
140-142: LGTM! Feedback button implementation for programming exercisesThe implementation correctly uses the new
@ifsyntax and appropriately places the feedback request button within the IDE buttons section, making it available only for graded programming exercise participations.
Line range hint
234-241: Verify submission status handlingThe feedback button implementation for text/modeling exercises looks good, with appropriate conditions for Athena service and exercise types. However, let's verify the submission status handling:
- The optional chaining for submissions (
gradedParticipation?.submissions) is good, but accessinglast()might throw if the array is empty.- Consider adding a null check for the submissions array.
Consider this safer approach:
-[isSubmitted]="gradedParticipation?.submissions?.last()?.submitted || false" +[isSubmitted]="gradedParticipation?.submissions?.length ? gradedParticipation.submissions.last()?.submitted : false"Let's verify the submission handling in the component:
src/test/java/de/tum/cit/aet/artemis/exercise/participation/ParticipationIntegrationTest.java (4)
550-550: Use a dedicated test scenario name for clarity.
This line sets up a successful mock for "programming" feedback. It looks correct, but consider using a more descriptive test name to clarify the expected outcome of the mock (e.g., “feedbackSuggestionsProgrammingSuccess”).
594-594: Duplicate usage of the success mock setup.
This code segment duplicates the success mock for "programming" feedback. Ensure that each test scenario is distinct or consolidated if they share the same functionality.
638-638: Check coverage of the text feedback scenario.
This line mirrors the approach used for programming but now for text-based exercises. Confirm that the test also handles potential edge cases, like empty or short text feedback.
677-677: Modeling feedback success mock aligns with the new feature.
Invoking themockGetFeedbackSuggestionsAndExpect("modeling", true)call is consistent with the new preliminary feedback selection feature. Ensure the test properly covers modeling-specific edge cases, like diagrams with no elements.src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise.module.ts (2)
32-33: Ensure test coverage for newly introduced components
These imports bring in new components, and it’s important to confirm that adequate unit tests (and, if applicable, e2e tests) are in place to ensure that they function as intended.
61-62: Validate standalone components
If these are truly standalone components (Angular ≥14), importing them here is correct. Otherwise, they should be moved to thedeclarationsarray. Ensure that each component's metadata is properly configured so no runtime errors occur.src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaModuleService.java (1)
25-25: Ensure consistent usage of the newModuleTypeenum.
The new import is correct. Verify that all references toModuleTypeare covered throughout the codebase, including error handling for unrecognized enum values, if any.src/main/webapp/app/exercises/programming/shared/lifecycle/programming-exercise-lifecycle.module.ts (1)
7-8: Imports alignment looks good.
This aligns well with Angular style guidelines for standalone components. No immediate issues found.src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.ts (1)
49-55: Guard against null or undefined changes todueDate.
IfdueDateis unset or invalid, ensure this logic doesn't overwrite the module incorrectly.src/main/webapp/app/exercises/programming/manage/update/programming-exercise-update.helper.ts (2)
43-44: Enum additions align with the new feedback architecture.
These new enum fields match the introduced feedback functionality. Ensure that calling code checks for backward compatibility if older references assume a shorter enum.
93-101: Clearly separate the advanced feedback options inIS_DISPLAYED_IN_SIMPLE_MODE.
SettingfeedbackSuggestionsandpreliminaryFeedbackRequeststofalseis consistent with disallowing them in simple mode. The code is clear and maintainable.src/main/webapp/app/exercises/shared/feedback-suggestion/exercise-feedback-suggestion-options.component.ts (3)
8-9: Stand-alone imports are consistent with Angular style guide.
This approach ensures modularity. No issues noted here.
13-14: Module usage for a stand-alone component is good.
The approach helps modularize the component. Continue following the recommended folder structure for maintainability.
79-82: Disable manual feedback requests upon enabling suggestions.
This logic is correct, but confirm that toggling suggestions back off should re-check if manual feedback can be re-enabled.src/main/webapp/app/exercises/modeling/manage/modeling-exercise.module.ts (1)
30-30: Ensure the component is declared or imported as a standalone component.
IfExercisePreliminaryFeedbackOptionsComponentis not a standalone component (Angular 15+ feature), it typically needs to be included in thedeclarationsarray or exported by another module that you import here.src/test/javascript/spec/component/exercises/shared/feedback/feedback-suggestion-option.component.spec.ts (1)
21-21: Verify standalone approach for the component under test.
ListingExerciseFeedbackSuggestionOptionsComponentinimportsimplies it is a standalone component. Confirm that its metadata is annotated withstandalone: true; otherwise, it should be declared indeclarations.src/test/javascript/spec/component/exercises/shared/feedback/preliminary-feedback-options.component.spec.ts (5)
19-33: Ensure correctness of mock forisEnabled.You’re stubbing
isEnabledto returnof(true)by default. If other tests need variations, keep each test’s stubbing local. This prevents accidental overrides among multiple tests and ensures better test isolation.
35-38: Validate that the component is created properly.It’s a standard test to ensure the component is instantiated. Good job!
50-61: Verify the observable logic.You have a great check on the
isAthenaEnabled$observable. Also ensure that your test covers expected false states for completeness (i.e., whenisEnabledreturnsof(false)).
63-72: Expand test coverage for edge cases.When
inputControlsDisabled()returns true for certain conditions, add tests for borderline scenarios (e.g., due date exactly now, or assessment type changed). This helps confirm no off-by-one or timing boundary issues.
81-88: Improve label style tests with more specificity.Verifying the returned style object is correct is good. You might also consider future-proofing with additional style checks if the logic grows.
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaSubmissionSendingService.java (1)
129-131: Confirm URL correctness for preliminary feedback suggestions.You’ve updated the connector call with
getAthenaModuleUrl(exercise.getExerciseType(), exercise.getFeedbackSuggestionModule()). Ensure this logic is tested thoroughly to avoid malformed endpoints. For robust coverage, add unit tests to validate correct parameter usage (e.g., valid vs. invalid module names).src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise-detail.component.ts (1)
106-107: Align property naming with domain understanding.Renaming properties to clarify that one boolean is for "feedbackSuggestions" and the other is for "preliminaryFeedback" is a good change. Ensure the rest of the UI, translations, and docs consistently reflect the newly introduced property names (
!!exercise.feedbackSuggestionModule,!!exercise.preliminaryFeedbackModule).src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaRepositoryExportService.java (1)
69-69: Validate Exception Type for Unavailable Features
Review the usage ofServiceUnavailableExceptionhere. If the lack of enabled feedback suggestions should indicate a permission issue (for example, an unauthorized attempt to get suggestions), you might consider usingAccessForbiddenExceptionor a more appropriate status code/message. Otherwise, this logic appears consistent with the intended checks for enabling feedback suggestions or preliminary feedback.src/main/java/de/tum/cit/aet/artemis/communication/dto/ChannelDTO.java (1)
200-204: Consider Additional Property Handling in Conversion
The newtoChannel()method correctly initializes basic fields. Verify whether any extra fields inChannelDTO(e.g., tutorialGroupId, subType) should also be reflected in the constructedChannel. If not necessary, the current approach is fine.src/test/java/de/tum/cit/aet/artemis/athena/service/connectors/AthenaFeedbackSuggestionsServiceTest.java (12)
3-8: New Athena Module Constants for Modeling & Preliminary Feedback
Importing additional constants for modeling and preliminary feedback fosters code clarity and strongly typed usage. Ensure these constants align with the modules configured elsewhere to prevent mismatches.
27-29: Enabling Modeling Exercise Testing
Good addition of modeling exercise domain imports. This ensures the test file properly references modeling classes and extends coverage to modeling scenarios.
50-52: Injecting ModelingExerciseUtilService
The injection ofmodelingExerciseUtilServiceis consistent with the existing pattern (textExerciseUtilService,programmingExerciseUtilService, etc.). No issues found.
61-64: Introducing ModelingExercise & ModelingSubmission Fields
IntroducingmodelingExerciseandmodelingSubmissionfields in the test class ensures coverage for the new feedback flow in modeling exercises.
70-71: Setting Text Preliminary & Suggestion Modules
Assigning the text-based exercise modules aligns with the new preliminary feedback flow. No apparent issues found.
76-77: Configuring Programming Exercise for Preliminary & Suggestion Modules
Consistent approach for programming exercises, ensuring that both suggestion and preliminary modules are tested. Implementation looks solid.
81-88: ModelingExercise Setup for Preliminary & Suggestion Modules
This addition parallels the text and programming approach, giving modeling exercises the same facility for feedback suggestions and preliminary feedback. Good for uniform test coverage.
94-97: Testing Text Feedback Suggestions
The new or updated lines correctly mock and verify the suggestion retrieval. Assertions are meaningful, and the approach aligns with similar exercises.
106-114: Testing Programming Feedback Suggestions
The addition ensures that the programming exercise suggestions flow is tested under the new modules. This is essential for coverage consistency.
116-130: Testing Preliminary Feedback on Text Exercises
Valid assertions and mock usage confirm that the new preliminary feedback logic works for text exercises. Nicely done.
Line range hint
131-142: Testing Preliminary Feedback on Programming Exercises
Mirroring the text-based approach ensures that programming preliminary feedback is equally validated. Good coverage.
143-143: Conflict Handling for Mismatching Exercise IDs
This test appropriately verifies the conflict scenario when a submission’s exercise differs from the requested exercise. Good to see robust error handling.src/main/webapp/app/exercises/modeling/manage/modeling-exercise-detail.component.ts (1)
139-140: Display Feedback Modules in Exercise Details
DisplayingfeedbackSuggestionModuleandpreliminaryFeedbackModuleas boolean fields in the detail overview is a great way to maintain transparency around feedback options. Implementation follows best practices for Angular’s property binding with booleans.src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingAssessmentService.java (1)
155-155: Looks good: consistent naming withallowManualFeedbackRequests.This change aligns the property name with the rest of the codebase to distinguish between different request types.
src/test/javascript/spec/component/overview/exercise-details/request-feedback-button/request-feedback-button.component.spec.ts (6)
59-59: Ensure coverage for the newpreliminaryFeedbackModuleproperty.Including
preliminaryFeedbackModulein this test is appropriate, but ensure that you have sufficient coverage to confirm that the new property is honored in all relevant scenarios.
107-107: Double-check error-handling paths with undefined participations.When
studentParticipationsis undefined, ensure that the component logic and any related error or empty state handling is validated.
126-126: Good test coverage of the “preliminary” property.The scenario covers a submitted student participation. Make sure further tests verify different
coursestates (e.g., exam vs. non-exam) for thorough coverage.
149-149: Confirm that programming exercises behave similarly to text exercises regarding preliminary feedback.This scenario properly sets
preliminaryFeedbackModule. Ensure the logic for programming-specific checks (e.g., test run) is tested in additional tests if needed.
216-216: Validate usage withisGeneratingFeedbackandisSubmitted.You’ve correctly handled disabling the button. If additional properties or states are introduced (e.g., partial submissions), ensure those states are covered, too.
230-243: Thorough test for when Athena is disabled but manual feedback requests are allowed.Great job adding this scenario to confirm the correct display of the request feedback button.
src/main/webapp/app/entities/exercise.model.ts (4)
90-90: Renaming toallowManualFeedbackRequestsimproves clarity.This name more accurately reflects the intended functionality—manually-triggered requests rather than generic requests.
132-132:preliminaryFeedbackModuleproperty introduction aligns with the new feedback flow.Ensure that all referencing components handle undefined gracefully.
163-163: Default initialization tofalseforallowManualFeedbackRequests.This ensures a stable default, but confirm that any inherited logic sets this flag appropriately for exercises that do support manual requests by default.
283-285: Reset these properties upon import.Clearing
allowManualFeedbackRequestsand the module properties during cloning or import operations helps avoid unintended carry-over settings.src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.java (10)
98-100: Updated documentation clarifies theisPreliminaryparameter.Renaming “graded” to “preliminary” is more explicit in capturing the feedback state.
103-103: Method signature changes remain consistent with new naming convention.Matching the doc comment to the new parameter fosters better readability.
112-119: ConstructingRequestDTOwithisPreliminaryis clear.The invocation with retry ensures reliability. Just confirm that partial success or additional states (e.g., offline Athena) are still handled correctly.
126-128: Method documentation update matches the new logic for preliminary requests.This keeps usage consistent for consumers of the method.
131-131: Refactored method signature for programming feedback.Ensure calling code that depends on the “graded” parameter is updated accordingly.
135-141: Revised URL selection is appropriate.Choosing between
preliminaryFeedbackModuleandfeedbackSuggestionModulebased onisPreliminaryaligns with the new logic.
148-150: Modeling feedback doc comment mirrors changes in other methods.Maintaining parallel documentation ensures consistency across exercise types.
162-163: Accurate creation ofRequestDTO.This approach is consistent with the text and programming methods.
165-167: Conditional URL logic.Selecting the correct Athena module remains consistent with the previous changes.
168-169: Token usage is stored properly.Ensures that preliminary vs. final feedback usage is appropriately tracked, meeting the new logic.
src/main/webapp/app/exercises/programming/shared/lifecycle/programming-exercise-lifecycle.component.ts (1)
162-162: Verify forced disabling of manual feedback requests in SEMI_AUTOMATIC mode.
Explicitly settingallowManualFeedbackRequests = falseon switching toSEMI_AUTOMATICmay prevent a valid scenario in which both partial automation and manual feedback requests are desired together.src/main/java/de/tum/cit/aet/artemis/athena/web/AthenaResource.java (1)
118-118: Confirm correctness of updated generic signature.
Changing the provider’s signature to<FeedbackProvider<ExerciseT, SubmissionT, Boolean, OutputT>>looks consistent, but ensure all call sites correctly pass the expected parameters.src/test/java/de/tum/cit/aet/artemis/core/connector/AthenaRequestMockProvider.java (4)
58-61: Constants for text suggestions and prelminary are well-structured.
These new constants align with the naming conventions for the existing ones. Good job keeping them consistent.
66-69: Constants for programming suggestions and preliminary are clearly named.
No issues here. The naming is consistent and descriptive.
74-77: Constants for modeling suggestions and preliminary adhere to the same approach.
They follow the established naming pattern, which supports readability across the codebase.
212-214: Ensure alignment between the endpoint URL pattern and the new param.
The URL includesgetTestModuleName(moduleType, isPreliminary). Just confirm that all integration points expect this structure without mismatch.src/test/javascript/spec/component/programming-exercise/programming-exercise-lifecycle.component.spec.ts (9)
17-19: Imports for feedback-related components are correctly added.
These new components help test the synergy between exercise feedback options and the main lifecycle flow.
30-30: State injection ofAthenaServiceis appropriate.
Great job injecting the service to mock or spy on Athena-related calls.
40-40: Mocking theExercisePreliminaryFeedbackOptionsComponentensures holistic test coverage.
This mock complements existing tests for toggling preliminary feedback and is in line with Angular testing best practices.
77-78: InjectingAthenaServicewith TestBed for further checks is correct.
This approach allows flexible mocking and ensures robust test coverage of external dependencies.
157-163: Test verifies togglingallowManualFeedbackRequests.
The assertion is accurate and aligns with the functional intent of the toggle logic.
370-381: Correctly tests rendering ofExerciseFeedbackSuggestionOptionsComponent.
The scenario checks if suggestions are displayed when relevant flags are enabled. Looks good.
383-394: Ensures suggestion options are omitted in exam mode.
This test helps confirm that features not permitted in exam scenarios are properly hidden.
396-411: Validates presence of preliminary feedback options only if Athena is enabled.
MockingathenaService.isEnabled()totrueis a solid approach for verifying that the feature is conditionally displayed.
413-429: Confirms that preliminary feedback options are hidden if Athena is disabled.
It’s helpful that the test also checks the presence of the fallback checkbox. Looks complete.src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportService.java (1)
296-298: Disable feedback modules for exam exercises - verified implementation.These lines correctly disable the feedback suggestion and preliminary feedback modules for exam exercises, ensuring no unexpected usage. Looks consistent with the broader logic.
src/test/java/de/tum/cit/aet/artemis/athena/AthenaResourceIntegrationTest.java (13)
3-3: Imports for modeling feedback suggestions - looks fine.No issues with adding constants for modeling suggestion modules.
5-5: Imports for programming feedback suggestions - looks fine.No issues with adding constants for programming suggestion modules.
7-7: Imports for text feedback suggestions - looks fine.No issues with adding constants for text suggestion modules.
256-256: Enabling text feedback suggestion module for testing.Setting the text exercise’s feedback suggestion module is consistent with the new approach to text-based feedback.
259-259: Mocking text suggestions in tests - verified.Appropriate usage of the mock provider to test text-based feedback suggestions.
269-269: Enabling programming feedback suggestion module for testing.Similar to text; the new constant usage aligns with the refactored approach.
272-272: Mocking programming feedback suggestions - verified.Looks correct and consistent with the rest of the test suite.
283-283: Enabling modeling feedback suggestion module for testing.Similar to text/programming; usage of the modeling module constant is consistent.
286-286: Mocking modeling feedback suggestions - verified.No issues with the approach to testing modeling suggestions.
302-302: Mocking programming feedback suggestions (not found scenario).Ensures correct behavior if the exercise and submission are non-existent. Looks good.
315-315: Mocking text feedback suggestions for forbidden access test.Properly ensures user with insufficient permissions gets a 403 response.
322-322: Mocking programming feedback suggestions for forbidden access test.Proper test coverage for restricted roles.
330-330: Mocking modeling feedback suggestions for forbidden access test.This covers the modeling scenario with the forbidden role. Looks consistent.
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingSubmissionResource.java (1)
387-387: Refined condition for allowing manual feedback requests.Using
getAllowManualFeedbackRequests()is consistent with the new naming. The date check logic also looks correct.src/test/javascript/spec/component/overview/exercise-details/exercise-details-student-actions.component.spec.ts (2)
36-36: Importing RequestFeedbackButtonComponent for extended test coverage.This import is necessary for verifying the new feedback request logic in the component. Looks good.
79-79: Including RequestFeedbackButtonComponent in TestBed imports.Ensures the component and related functionalities are properly tested.
src/main/java/de/tum/cit/aet/artemis/programming/domain/ProgrammingExercise.java (2)
687-687: Reconfirm the inverted logic for manual feedback checks.Here, the check for
getAllowManualFeedbackRequests()is introduced. Ensure that toggling this property in the UI or config doesn't lead to any unintentional behavior, especially if, elsewhere, the logic for feedback requests was based on the old property.
801-801: Ensure consistent user flow for manual feedback.This condition early-returns if manual feedback requests are disabled, bypassing further validations. Verify that there are no edge cases where the user needs partial feedback or different flow if manual requests are off but other feedback features remain.
src/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.java (3)
43-43: Import statement clarity.The imported
ModuleTypealigns with new feature toggles for Athena modules. Keep imports minimal and relevant, and confirm no star-imports remain, per coding guidelines.
229-232: Verify Athena module checks for feedback suggestions.This snippet checks access for the
ModuleType.FEEDBACK_SUGGESTIONS. Ensure that all code paths correctly disable or allow the feedback suggestions on thetextExerciseobject if the check fails. This helps maintain clarity on feature toggles for suggestions.
286-289: Validate preliminary feedback module usage.Same logic as with feedback suggestions, but for
ModuleType.PRELIMINARY_FEEDBACK. Confirm consistent handling in upstream and downstream calls, so that no module-specific fields remain set if the user lacks permissions.src/test/javascript/spec/component/assessment-dashboard/exercise-assessment-dashboard.component.spec.ts (1)
595-595: Confirm property rename in specs.The spec sets
allowManualFeedbackRequests: false. Verify that all references to the old propertyallowFeedbackRequestsare removed from the test suite, ensuring consistency.src/main/webapp/app/exercises/shared/dashboards/tutor/exercise-assessment-dashboard.component.ts (1)
335-335: Double-check single vs. team mode logic.Switching from
allowFeedbackRequeststoallowManualFeedbackRequestsat line 335 might affect how we fetch submissions for solo/team exercises. Make sure the logic remains consistent regarding partial feature usage (e.g., if manual feedback is turned off while complaints or auto feedback are on).src/main/java/de/tum/cit/aet/artemis/exercise/domain/Exercise.java (5)
148-150: New field for preliminary feedback.Introducing the new
preliminary_feedback_modulecolumn enhances flexibility in controlling feedback modules. Ensure null-safety and default values in code handling it.
257-258: Setter logic is clear.The setter properly updates the renamed field. The implementation aligns well with the revised naming.
839-841: Return type is straightforward.This getter’s logic is straightforward. Make sure all references handle possible null values for
preliminaryFeedbackModule.
843-845: Setter for preliminaryFeedbackModule.The setter correctly covers the new field. No issues observed.
851-853: Boolean method for enabling preliminary feedback.A suitable naming pattern for boolean checks. The method is concise.
src/main/java/de/tum/cit/aet/artemis/exercise/service/SubmissionService.java (1)
700-700: Conditional check for manual feedback requests.This condition properly references the newly renamed getter. Ensure consistent usage of
getAllowManualFeedbackRequests()throughout to avoid confusion.src/main/webapp/app/exercises/programming/manage/programming-exercise-detail.component.ts (3)
591-591: Display setting for manual feedback requests.This addition to the UI properly reflects the new
allowManualFeedbackRequestsflag. No issues found.
598-598: Feedback suggestions display logic.Good use of a Boolean detail to handle the
feedbackSuggestionModule. Consider ensuring consistent labeling and usage throughout the UI.
599-599: Preliminary feedback display logic.Properly conveys the presence of
preliminaryFeedbackModulein the UI. Maintain consistency with the naming scheme in tooltips or labels.src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingAssessmentIntegrationTest.java (1)
957-957: Enable manual feedback requests in test.Changing to
programmingExercise.setAllowManualFeedbackRequests(true);aligns with the renamed property. This ensures the test accurately reflects current logic.src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java (3)
44-44: New Import forModuleTypeThis new import references the
ModuleTypeenum in theathena.domainpackage. It's a clean addition aligned with the new feedback modules feature, and it helps specify which Athena module is being checked.
250-253: Added Athena Access Check forfeedbackSuggestionModuleandpreliminaryFeedbackModuleThe invocation of
checkHasAccessToAthenaModulewithModuleType.FEEDBACK_SUGGESTIONSand thenModuleType.PRELIMINARY_FEEDBACKensures that only the designated modules can be attached to a programming exercise. The fallback to setting the module tonullwhen Athena is unavailable prevents invalid configurations.
351-354: Consistent Athena Module Check in Update MethodSimilarly, in the
updateProgrammingExercisemethod, the code confirms the user’s access to the respective Athena modules. This consistency between create and update operations is commendable.src/test/javascript/spec/component/programming-exercise/programming-exercise-update.component.spec.ts (1)
1349-1352: Ensuring Feedback Properties Reset on ImportThese lines confirm that manual feedback requests and associated Athena modules are cleared during import, preventing unexpected carryovers from the source exercise. This robust test coverage is helpful for preventing inadvertent data leaks between exercises.
src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.html (1)
15-31: Dropdown for Selecting the Preliminary Feedback ModuleRendering the Athena module dropdown within the same
@ifblock is clean. Binding with(ngModel)forexercise.preliminaryFeedbackModuleis consistent with Angular forms. Ensuring thatinputControlsDisabled()handles edge cases (e.g., disable past the due date, read-only mode) is a nice touch, keeping the logic user-friendly.src/main/webapp/app/exercises/shared/feedback-suggestion/exercise-feedback-suggestion-options.component.html (3)
8-8: Checkbox for Feedback SuggestionsThe checkbox now leverages
[checked]="showDropdownList", decoupling it from directly inspectingexercise.feedbackSuggestionModule. This is a more explicit approach, aligning the UI state withshowDropdownList.
17-17: Updated Label and TooltipChanging the translation key to
artemisApp.exercise.allowFeedbackSuggestionsharmonizes naming across the codebase. The help icon similarly referencesallowFeedbackSuggestionsTooltip, making the UI more self-explanatory.Also applies to: 19-19
23-23: Dropdown Label for Athena Feedback SuggestionsRenaming the label to "Athena Feedback Suggestions Module" provides clarity about which module is being selected. The usage of
[value]="this.exercise.feedbackSuggestionModule"is consistent with the prior changes.src/main/webapp/app/assessment/assessment-warning/assessment-warning.component.ts (1)
47-47: LGTM! The condition now correctly checks for manual feedback requests.The change from
allowFeedbackRequeststoallowManualFeedbackRequestsaligns with the PR's objective of distinguishing between manual and preliminary feedback mechanisms.src/main/webapp/i18n/en/textExercise.json (1)
Line range hint
1-48: Verify if new translations are needed for feedback model selection.While the removal of preliminary feedback translations is aligned with the changes, the new feature to "choose preliminary feedback model" might require additional translation entries.
✅ Verification successful
No new translations needed for feedback model selection
Based on the investigation, I found that the feedback model selection UI is already properly internationalized:
- The checkbox for enabling preliminary feedback uses the existing translation key
artemisApp.exercise.allowFeedbackRequests- The checkbox tooltip uses the existing translation key
artemisApp.exercise.allowFeedbackRequestsTooltip- The model selection dropdown uses a hardcoded label "Athena Preliminary Feedback Module" which is appropriate since the module names themselves are technical identifiers that shouldn't be translated
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any new translation keys in the codebase that might be missing here rg -g '*.{ts,html}' -A 3 "translate.*preliminary.*feedback.*model"Length of output: 68
Script:
#!/bin/bash # Let's try a broader search for any UI elements related to feedback model selection rg -g '*.{ts,html}' -A 3 'feedback.*model.*selection|model.*selection.*feedback'Length of output: 82
Script:
#!/bin/bash # Search for any new translation keys in the codebase that might be missing here rg -g '*.{ts,html}' -A 3 'translateService.*feedback|translate.*feedback'Length of output: 9168
Script:
#!/bin/bash # Let's check for any new feedback-related components or templates fd -e ts -e html . -x grep -l "feedback.*model"Length of output: 9352
Script:
#!/bin/bash # Let's check for any new UI components or templates that might need translations for feedback model selection rg -g '*.{ts,html}' -A 3 'select.*feedback|feedback.*select'Length of output: 24489
Script:
#!/bin/bash # Search for any new components or services that might handle feedback model selection fd -e ts -e html . -x grep -l "model.*selection"Length of output: 407
Script:
#!/bin/bash # Let's check for any new preliminary feedback related components fd -e ts -e html . -x grep -l "preliminary.*feedback"Length of output: 1075
Script:
#!/bin/bash # Let's check the exercise-preliminary-feedback-options component rg -g '*.{ts,html}' -A 10 'exercise-preliminary-feedback-options.component'Length of output: 9508
Script:
#!/bin/bash # Let's check the content of the preliminary feedback options component template cat src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.htmlLength of output: 1565
Script:
#!/bin/bash # Let's check for any translation keys related to preliminary feedback in the codebase rg -g '*.json' -A 3 'preliminary.*feedback|feedback.*model'Length of output: 731
src/main/webapp/i18n/de/textExercise.json (1)
Line range hint
1-48: LGTM! German translations are consistent with English changes.The removal of preliminary feedback translations matches the English file changes, and the informal language style is maintained throughout.
Note: If new translations are needed for the feedback model selection feature (as mentioned in the English file review), they should be added here as well.
src/main/webapp/app/exercises/text/participate/text-editor.component.html (1)
27-27: LGTM: Property name change reflects the specific feedback type.The condition change from
allowFeedbackRequeststoallowManualFeedbackRequestsaccurately represents the type of feedback being requested and maintains the due date check.src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise-update.component.html (1)
217-220: LGTM: Well-structured feedback section addition.The new feedback section is logically placed and properly handles exam mode restrictions. Good reuse of the
jhi-exercise-preliminary-feedback-optionscomponent.src/main/webapp/app/exercises/programming/shared/lifecycle/programming-exercise-lifecycle.component.html (2)
89-89: LGTM: Assessment date picker condition updated.The condition correctly checks for manual feedback requests in non-exam mode.
123-124: LGTM: Comprehensive feedback type conditions.The conditions properly handle all feedback types: suggestions, preliminary, and manual requests.
src/main/webapp/app/exercises/modeling/manage/modeling-exercise-update.component.html (1)
271-274: LGTM! Clean integration of the feedback section.The feedback section is well-structured and follows the established pattern:
- Section header with proper translation key
- Conditional rendering using modern Angular syntax
- Proper component integration with required inputs
Let's verify the component integration:
✅ Verification successful
Component integration is properly implemented
The feedback section is correctly integrated with the required inputs:
- The component accepts
exerciseanddueDateinputs as used in the template- The component is standalone with proper imports
- The component handles preliminary feedback options with proper validation and state management
Note: The component implementation shows that it properly handles the inputs passed from the modeling exercise update component, and includes additional features like:
- Athena service integration for feedback modules
- Due date validation
- Proper state management for feedback options
- Accessibility considerations (label styling)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the preliminary feedback component integration # Test: Check if the component exists and has the expected inputs ast-grep --pattern 'class $CLASS { $$$ @Input() exercise: $_ @Input() dueDate: $_ $$$ }'Length of output: 222
Script:
#!/bin/bash # Let's try a different approach to verify the component # First, let's find the component file fd "exercise-preliminary-feedback-options.component.ts" # Then let's check its content and inputs rg "@Input\(\)" -A 2 -B 2Length of output: 71778
Script:
#!/bin/bash # Let's check the actual component implementation rg -l "exercise-preliminary-feedback-options.component" --type ts # Then let's look at its content cat src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.tsLength of output: 3973
src/main/webapp/i18n/en/exercise.json (1)
176-179: LGTM! Clear and consistent translations.The new translation keys are well-structured and provide clear information about the feedback options:
- Clear distinction between feedback suggestions and requests
- Helpful tooltips explaining the functionality
- Consistent terminology with the rest of the application
src/main/webapp/i18n/de/exercise.json (1)
176-179: LGTM! Accurate and consistent German translations.The German translations are well-crafted:
- Maintains informal style (du/dein) as per guidelines
- Accurately conveys the meaning of the English text
- Uses consistent terminology with existing translations
src/main/webapp/app/exercises/modeling/manage/modeling-exercise.module.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.java (3)
115-119: Reduce code duplication with a helper method
This block (constructing request, calling Athena, logging, storing usage) appears repeatedly across different feedback suggestion methods. Consider a small helper method to avoid duplication and simplify maintenance.
136-141: Encourage a unified approach for repeated feedback-suggestion retrieval
Building the request, callinginvokeWithRetry, logging, and storing usage is repeated across multiple methods. Centralizing this logic into one well-typed method could reduce duplications, following DRY principles.
163-168: Additional pointer to reduce boilerplate
As in the other feedback methods, consider extracting the repeatedinvokeWithRetry + logging + token usagesteps into a shared utility method for improved readability.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.java(1 hunks)src/main/webapp/app/exercises/shared/feedback-suggestion/exercise-feedback-suggestion-options.component.html(2 hunks)src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.html(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.html
- src/main/webapp/app/exercises/shared/feedback-suggestion/exercise-feedback-suggestion-options.component.html
🧰 Additional context used
📓 Path-based instructions (1)
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
🔇 Additional comments (5)
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.java (5)
98-100: Document the parameter to avoid confusion
Currently, the doc states “should Athena generate grade suggestions or not,” but the parameter name isisPreliminary. Iftruemeans we generate partial, non-final feedback, then the method doc can be misconstrued. Clarify the intended meaning to avoid future confusion.
112-114: Parameter naming mismatch
Here,new RequestDTO(..., isPreliminary)is assigned toboolean isGraded. This was highlighted before in older review comments and still persists. Unifying the naming (e.g., consistently calling itisPreliminary) would enhance maintainability.
126-128: Confirm clarity of theisPreliminarydoc parameter
Similar to the first method, re-check that “should Athena generate grade suggestions or not” aligns with the boolean. Iftrueindicates partial or non-graded feedback, ensure the wording leaves no ambiguity.
148-150: Re-evaluate the Javadoc for parameter meaning
Just like in the text and programming feedback methods, make sure “isPreliminary” consistently conveys the idea of partial vs. final feedback generation.
161-162: Repeated naming inconsistency
Same mismatch: passingisPreliminaryto aboolean isGradedfield. Unify the naming to match previous recommendations.
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.java
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.java
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.java (1)
104-104: 🛠️ Refactor suggestionAlign logging messages with parameter semantics.
The current logging messages are counterintuitive as they show "Graded" when
isPreliminaryis true. This could confuse developers maintaining the code.Apply this pattern to all logging statements:
- log.debug("Start Athena {} Feedback Suggestions Service for Exercise '{}' (#{}).", - isPreliminary ? "Graded" : "Non Graded", exercise.getTitle(), exercise.getId()); + log.debug("Start Athena {} Feedback Suggestions Service for Exercise '{}' (#{}).", + isPreliminary ? "Preliminary" : "Graded", exercise.getTitle(), exercise.getId());Also applies to: 133-133, 154-154, 118-118, 140-140, 167-167
🧹 Nitpick comments (1)
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.java (1)
98-100: Enhance parameter documentation for clarity.The
isPreliminaryparameter documentation could be more descriptive about its purpose and impact. Consider clarifying what "preliminary" means in this context and how it affects the feedback generation.Consider updating the parameter documentation to:
- * @param isPreliminary the {@link Boolean} should Athena generate grade suggestions or not + * @param isPreliminary if true, uses the preliminary feedback module for generating initial, non-graded feedback; if false, uses the feedback suggestion module for generating graded feedbackAlso applies to: 126-128, 148-150
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.java(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
🔇 Additional comments (1)
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.java (1)
112-113: LGTM: Correct parameter mapping for Athena API.The inversion of
isPreliminarytoisGradedin the RequestDTO construction is correct as it maintains compatibility with Athena's API expectations.Also applies to: 134-135, 161-162
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.java
Show resolved
Hide resolved
Please test only on TS1
!!! Contains database migrations !!!
Checklist
General
Server
Client
authoritiesto all new routes and checked the course groups for displaying navigation elements (links, buttons).Changes affecting Programming Exercises
Motivation and Context
Currently, Artemis allows only one module to be set for Athena Feedback Requests, be it for preliminary or graded feedback. This PR addresses the limitation and enables setting separate modules for each type of feedback.
Description
This PR introduces functionality that allows instructors to configure different Athena Feedback Request modules for preliminary feedback and graded feedback. Preliminary feedback can now be assigned a dedicated module without affecting the graded feedback settings.
Steps for Testing
Disclaimer:
Currently, only LLM modules support preliminary feedback. You can easily test this functionality with non-LLM modules to verify if the correct module is being used (non-LLM modules will not work).
As an instructor:
2. Navigate to the course settings.
3. Create a Programming, Text, or Modeling exercise.
4. Select a module for preliminary feedback (no due date required). For graded feedback, enable Feedback Suggestions, set a due date, and select a module(text or programming).
5. Save the changes.
As a student:
6. Open the created exercise.
7. Submit something.
8. Submit an AI feedback request.
9. Verify that the appropriate module is used based on the type of feedback.
Testserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.
Review Progress
Performance Review
Code Review
Manual Tests
Exam Mode Test
Performance Tests
Screenshots
Summary by CodeRabbit
Summary by CodeRabbit
Based on the comprehensive review of the changes, here are the high-level release notes:
Release Notes
New Features
ModuleTypeenum to manage different Athena module types.Changes
allowFeedbackRequeststoallowManualFeedbackRequestsacross the application.Improvements
Removed
These release notes capture the key modifications while maintaining a high-level, user-friendly perspective on the changes implemented in this update.