-
Notifications
You must be signed in to change notification settings - Fork 313
Programming exercises
: Allow to choose preliminary feedback model
#10441
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Programming exercises
: Allow to choose preliminary feedback model
#10441
Conversation
…odules: client side
…g-exercises/choose-preliminary-feedback-model
…iminary-feedback-model
…oose-preliminary-feedback-model' into feature/programming-exercises/choose-preliminary-feedback-model
…iminary-feedback-model
…g-exercises/choose-preliminary-feedback-model # Conflicts: # src/main/webapp/app/exercises/modeling/manage/modeling-exercise.module.ts # src/main/webapp/app/exercises/programming/shared/lifecycle/programming-exercise-lifecycle.module.ts # src/main/webapp/app/exercises/shared/feedback-suggestion/exercise-feedback-suggestion-options.component.ts # src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise.module.ts # src/main/webapp/app/overview/exercise-details/exercise-details-student-actions.component.html # src/main/webapp/i18n/de/exercise.json # src/test/javascript/spec/component/exercises/shared/feedback/feedback-suggestion-option.component.spec.ts # src/test/javascript/spec/component/modeling-exercise/modeling-exercise-update.component.spec.ts # src/test/javascript/spec/component/overview/exercise-details/exercise-details-student-actions.component.spec.ts # src/test/javascript/spec/component/overview/exercise-details/request-feedback-button/request-feedback-button.component.spec.ts # src/test/javascript/spec/component/programming-exercise/programming-exercise-lifecycle.component.spec.ts # src/test/javascript/spec/component/text-exercise/text-exercise-update.component.spec.ts
End-to-End (E2E) Test Results Summary
|
…iminary-feedback-model
…g-exercises/choose-preliminary-feedback-model # Conflicts: # src/main/webapp/app/core/course/overview/exercise-details/student-actions/exercise-details-student-actions.component.spec.ts # src/main/webapp/app/programming/shared/lifecycle/programming-exercise-lifecycle.component.spec.ts
End-to-End (E2E) Test Results Summary
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.ts (2)
77-84
: Consider initializing dropdown state based on existing moduleThe dropdown state is set based only on the checkbox event, but isn't initialized based on whether the exercise already has a preliminary feedback module set.
ngOnInit(): void { const courseId = Number(this.activatedRoute.snapshot.paramMap.get('courseId')); this.athenaService.getAvailableModules(courseId, this.exercise()).subscribe((modules) => { this.availableAthenaModules = modules; this.modulesAvailable = modules.length > 0; this.cdr.detectChanges(); }); this.isAthenaEnabled$ = this.athenaService.isEnabled(); this.initialAthenaModule = this.exercise().preliminaryFeedbackModule; + this.showDropdownList = !!this.exercise().preliminaryFeedbackModule; }
86-88
: Consider timezone handling in date comparisonThe
hasDueDatePassed
method compares dates using the local timezone, which could lead to inconsistent behavior across different regions.private hasDueDatePassed() { - return dayjs(this.exercise().dueDate).isBefore(dayjs()); + return dayjs(this.exercise().dueDate).utc().isBefore(dayjs().utc()); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.ts
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`src/main/webapp/**/*.ts`: angular_style:https://angular.io/guide/styleguide;methods_in_html:false;lazy_loading:true;code_reuse:true;tests:meaningful;types:PascalCase;enums:PascalC...
src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.ts
🧠 Learnings (1)
src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.ts (3)
Learnt from: dmytropolityka
PR: ls1intum/Artemis#10441
File: src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.ts:76-83
Timestamp: 2025-04-07T13:18:39.389Z
Learning: In the Artemis client code, the Array prototype is extended with `first()` and `last()` methods, allowing for calls like `array.first()` and `array.last()` instead of using standard array indexing like `array[0]` or `array[array.length-1]`.
Learnt from: dmytropolityka
PR: ls1intum/Artemis#10441
File: src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.ts:76-83
Timestamp: 2025-04-07T13:18:35.405Z
Learning: In the Artemis client code, the Array prototype is extended with `first()` and `last()` methods (defined in src/main/webapp/app/shared/util/array.extension.ts), allowing for calls like `array.first()` and `array.last()` instead of using standard array indexing like `array[0]` or `array[array.length-1]`.
Learnt from: dmytropolityka
PR: ls1intum/Artemis#10441
File: src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.ts:76-83
Timestamp: 2025-04-07T13:18:32.368Z
Learning: In the Artemis client code, the Array prototype is extended with `first()`, `last()`, and `shuffle()` methods (defined in src/main/webapp/app/shared/util/array.extension.ts). The `first()` method returns the first element of an array or undefined if the array is empty, while `last()` returns the last element or undefined if empty. These extensions allow for calls like `array.first()` and `array.last()` instead of using standard array indexing like `array[0]` or `array[array.length-1]`.
🧬 Code Definitions (1)
src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.ts (1)
src/main/webapp/app/exercise/feedback-suggestion/exercise-feedback-suggestion-options.component.ts (1)
Component
(13-98)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Build and Push Docker Image / Build Docker Image for ls1intum/artemis
- GitHub Check: Build and Push Docker Image / Build Docker Image for ls1intum/artemis
- GitHub Check: Build .war artifact
- GitHub Check: server-tests
- GitHub Check: Analyse
🔇 Additional comments (3)
src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.ts (3)
1-18
: Component setup looks good with signal-based inputsThe component is correctly set up using the new
input()
signal approach and proper imports. This aligns with the PR objective of allowing configuration of preliminary feedback modules.
80-80
: The use offirst()
on array is consistent with Artemis patternsUsing
this.availableAthenaModules.first()
is appropriate since the Artemis codebase extends the Array prototype withfirst()
andlast()
methods.
52-58
:❓ Verification inconclusive
Verify ngOnChanges behavior with signal inputs
When using the new signal-based inputs, the SimpleChanges object might not contain the properties as expected. Consider updating the change detection approach to use signal features instead.
🏁 Script executed:
#!/bin/bash # Search for patterns of handling changes with signals in the codebase rg "ngOnChanges.*input\(" --type ts -A 10Length of output: 43
Attention: Verify Change Detection for Signal Inputs
The current implementation in
src/main/webapp/app/exercises/shared/preliminary-feedback/exercise-preliminary-feedback-options.component.ts
(lines 52–58) relies on standard Angular change detection viangOnChanges
and theSimpleChanges
object. Since new signal-based inputs may not propagate changes in the expected way (i.e. the expected change properties can be missing), please verify whether signal-based inputs are being used in this component (or elsewhere in the codebase). If they are, consider refactoring the change detection logic to leverage the appropriate signal APIs or lifecycle mechanisms.
isAthenaEnabled$: Observable<boolean>; | ||
modulesAvailable: boolean; | ||
availableAthenaModules: string[]; | ||
initialAthenaModule?: string; | ||
showDropdownList: boolean = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add memory leak prevention by unsubscribing observables
The component subscribes to observables but doesn't implement OnDestroy to clean them up, which could lead to memory leaks.
import { ChangeDetectorRef, Component, OnChanges, OnDestroy, OnInit, SimpleChanges, input } from '@angular/core';
import { AssessmentType } from 'app/assessment/shared/entities/assessment-type.model';
import { Exercise, ExerciseType } from 'app/exercise/shared/entities/exercise/exercise.model';
-import { Observable } from 'rxjs';
+import { Observable, Subject } from 'rxjs';
+import { takeUntil } from 'rxjs/operators';
import { ActivatedRoute } from '@angular/router';
import dayjs from 'dayjs/esm';
// ...rest of imports
@Component({
selector: 'jhi-exercise-preliminary-feedback-options',
imports: [TranslateDirective, NgStyle, HelpIconComponent, FormsModule, AsyncPipe],
templateUrl: './exercise-preliminary-feedback-options.component.html',
})
-export class ExercisePreliminaryFeedbackOptionsComponent implements OnInit, OnChanges {
+export class ExercisePreliminaryFeedbackOptionsComponent implements OnInit, OnChanges, OnDestroy {
// ...existing properties
+ private destroy$ = new Subject<void>();
ngOnInit(): void {
const courseId = Number(this.activatedRoute.snapshot.paramMap.get('courseId'));
this.athenaService.getAvailableModules(courseId, this.exercise()).subscribe((modules) => {
- this.availableAthenaModules = modules;
+ this.availableAthenaModules = modules;
this.modulesAvailable = modules.length > 0;
this.cdr.detectChanges();
- });
+ }, undefined, () => {}, takeUntil(this.destroy$));
- this.isAthenaEnabled$ = this.athenaService.isEnabled();
+ this.isAthenaEnabled$ = this.athenaService.isEnabled().pipe(takeUntil(this.destroy$));
this.initialAthenaModule = this.exercise().preliminaryFeedbackModule;
}
+ ngOnDestroy(): void {
+ this.destroy$.next();
+ this.destroy$.complete();
+ }
}
Committable suggestion skipped: line range outside the PR's diff.
End-to-End (E2E) Test Results Summary
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/webapp/app/exercise/feedback-suggestion/feedback-suggestion-option.component.spec.ts (1)
121-160
: Comprehensive test case for assessment type switching.This test case thoroughly validates component behavior when switching between assessment types, ensuring:
- Controls are disabled and the dropdown is hidden when switching to automatic assessment
- Controls are re-enabled when switching back to semi-automatic assessment
- Checkbox state and properties are correctly reset
The test follows good practices with clear setup, actions, and expectations.
Consider adding comment separators between the test's logical sections to improve readability:
// prepare data const modules = ['Module1', 'Module2']; jest.spyOn(athenaService, 'getAvailableModules').mockReturnValue(of(modules)); component.exercise = { type: ExerciseType.PROGRAMMING, dueDate: futureDueDate, assessmentType: AssessmentType.SEMI_AUTOMATIC } as Exercise; fixture.detectChanges(); +// SECTION 1: Initial state with active module // assume a module is chosen, hence, the controls are active const event = { target: { checked: true } }; component.toggleFeedbackSuggestions(event); expect(component.inputControlsDisabled()).toBeFalse(); expect(component.showDropdownList).toBeTrue(); +// SECTION 2: Switch to automatic assessment // change assessment type component.exercise.assessmentType = AssessmentType.AUTOMATIC; fixture.detectChanges(); // now, the input is unchecked and the controls are disabled expect(component.inputControlsDisabled()).toBeTrue(); expect(component.showDropdownList).toBeFalse(); let checkbox = fixture.debugElement.query(By.css('#feedbackSuggestionsEnabledCheck')); expect(checkbox.nativeElement.disabled).toBeTrue(); +// SECTION 3: Switch back to semi-automatic assessment // Now, change the assessment type back to semi automatic component.exercise.assessmentType = AssessmentType.SEMI_AUTOMATIC; component.exercise.feedbackSuggestionModule = undefined; // will be reset by parent component
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/webapp/app/exercise/feedback-suggestion/feedback-suggestion-option.component.spec.ts
(3 hunks)src/main/webapp/app/programming/shared/lifecycle/programming-exercise-lifecycle.component.spec.ts
(4 hunks)src/test/javascript/spec/component/exercises/shared/feedback/preliminary-feedback-options.component.spec.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/test/javascript/spec/component/exercises/shared/feedback/preliminary-feedback-options.component.spec.ts
🧰 Additional context used
📓 Path-based instructions (1)
`src/main/webapp/**/*.ts`: angular_style:https://angular.io/guide/styleguide;methods_in_html:false;lazy_loading:true;code_reuse:true;tests:meaningful;types:PascalCase;enums:PascalC...
src/main/webapp/app/exercise/feedback-suggestion/feedback-suggestion-option.component.spec.ts
src/main/webapp/app/programming/shared/lifecycle/programming-exercise-lifecycle.component.spec.ts
🧬 Code Definitions (1)
src/main/webapp/app/exercise/feedback-suggestion/feedback-suggestion-option.component.spec.ts (1)
src/test/javascript/spec/helpers/mocks/service/mock-translate.service.ts (1)
MockTranslateService
(9-56)
🪛 GitHub Check: client-tests
src/main/webapp/app/exercise/feedback-suggestion/feedback-suggestion-option.component.spec.ts
[failure] 11-11:
Cannot find module '../../../../../../test/javascript/spec/helpers/mocks/service/mock-translate.service' or its corresponding type declarations.
src/main/webapp/app/programming/shared/lifecycle/programming-exercise-lifecycle.component.spec.ts
[failure] 76-76:
Cannot find name 'athenaService'. Did you mean 'AthenaService'?
[failure] 416-416:
Cannot find name 'athenaService'. Did you mean 'AthenaService'?
[failure] 399-399:
Cannot find name 'athenaService'. Did you mean 'AthenaService'?
🪛 GitHub Check: client-tests-selected
src/main/webapp/app/exercise/feedback-suggestion/feedback-suggestion-option.component.spec.ts
[failure] 11-11:
Cannot find module '../../../../../../test/javascript/spec/helpers/mocks/service/mock-translate.service' or its corresponding type declarations.
src/main/webapp/app/programming/shared/lifecycle/programming-exercise-lifecycle.component.spec.ts
[failure] 76-76:
Cannot find name 'athenaService'. Did you mean 'AthenaService'?
[failure] 416-416:
Cannot find name 'athenaService'. Did you mean 'AthenaService'?
[failure] 399-399:
Cannot find name 'athenaService'. Did you mean 'AthenaService'?
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Build and Push Docker Image / Build Docker Image for ls1intum/artemis
- GitHub Check: Build and Push Docker Image / Build Docker Image for ls1intum/artemis
- GitHub Check: Build .war artifact
- GitHub Check: server-tests
- GitHub Check: Analyse
🔇 Additional comments (10)
src/main/webapp/app/programming/shared/lifecycle/programming-exercise-lifecycle.component.spec.ts (6)
22-22
: Import of AthenaService is appropriate.The AthenaService import is necessary for the new tests related to preliminary feedback functionality. This aligns with the PR objectives of configuring separate modules for preliminary feedback.
154-161
: Rename from allowFeedbackRequests to allowManualFeedbackRequests is aligned with PR objectives.The test has been properly updated to use
allowManualFeedbackRequests
andtoggleManualFeedbackRequests
instead of the previous naming. This change is consistent with the PR's objective of distinguishing between preliminary and manual feedback requests.
367-378
: Test for ExerciseFeedbackSuggestionOptionsComponent is well structured.This test properly verifies that the feedback suggestion component renders correctly when feedback suggestions are enabled, which is important for the new feedback configuration functionality.
380-391
: Test correctly verifies component doesn't render in exam mode.This test properly checks that the ExerciseFeedbackSuggestionOptionsComponent doesn't render in exam mode, which is an important constraint to verify.
393-408
: Use of fakeAsync is appropriate for testing asynchronous behavior.This test correctly uses fakeAsync and tick() to handle the asynchronous nature of the athenaService.isEnabled() observable. The test properly checks that the ExercisePreliminaryFeedbackOptionsComponent renders when Athena is enabled and preliminary feedback is configured.
🧰 Tools
🪛 GitHub Check: client-tests
[failure] 399-399:
Cannot find name 'athenaService'. Did you mean 'AthenaService'?🪛 GitHub Check: client-tests-selected
[failure] 399-399:
Cannot find name 'athenaService'. Did you mean 'AthenaService'?
410-426
: Test correctly handles the disabled state case.This test properly verifies that the ExercisePreliminaryFeedbackOptionsComponent doesn't render when Athena is disabled, while still showing the manual feedback requests checkbox. This is an important edge case to verify for the preliminary feedback implementation.
🧰 Tools
🪛 GitHub Check: client-tests
[failure] 416-416:
Cannot find name 'athenaService'. Did you mean 'AthenaService'?🪛 GitHub Check: client-tests-selected
[failure] 416-416:
Cannot find name 'athenaService'. Did you mean 'AthenaService'?src/main/webapp/app/exercise/feedback-suggestion/feedback-suggestion-option.component.spec.ts (4)
4-4
: Import statements correctly added to support test functionality.The imports for
MockDirective
,TranslateDirective
,MockTranslateService
,TranslateService
, andBy
are well-structured and necessary for the enhanced test cases.Also applies to: 10-13
29-30
: Test providers correctly configured.The addition of
MockDirective(TranslateDirective)
and the mock translation service provider are appropriate for the testing requirements.
101-119
: Test case correctly generalized for any exercise type.The test has been updated to check feedback suggestions functionality for any exercise type rather than just programming exercises, which aligns with the PR objective of enhancing flexibility in the feedback system.
The additional expectations for
showDropdownList
andallowManualFeedbackRequests
properties correctly reflect the system changes from general to manual feedback requests.
11-11
:✅ Verification successful
Fix import path for the MockTranslateService.
The import path for the
MockTranslateService
is causing static analysis errors. The path with multiple../
segments suggests it might be incorrectly referenced.-import { MockTranslateService } from '../../../../../../test/javascript/spec/helpers/mocks/service/mock-translate.service'; +import { MockTranslateService } from 'test/javascript/spec/helpers/mocks/service/mock-translate.service';Consider using a more direct path or creating an alias in the TypeScript configuration to simplify imports from test helpers.
🏁 Script executed:
#!/bin/bash # Check for the correct location of the MockTranslateService file fd --type f "mock-translate.service.ts" --exec echo {}Length of output: 131
Correct Import Path for MockTranslateService
The file was confirmed to exist at
./src/test/javascript/spec/helpers/mocks/service/mock-translate.service.ts
. With a properly configured TypeScript base URL (set tosrc
), using the absolute path:-import { MockTranslateService } from '../../../../../../test/javascript/spec/helpers/mocks/service/mock-translate.service'; +import { MockTranslateService } from 'test/javascript/spec/helpers/mocks/service/mock-translate.service';ensures static analysis errors are resolved and eliminates the need for deeply nested relative imports. Please update the import accordingly.
🧰 Tools
🪛 GitHub Check: client-tests
[failure] 11-11:
Cannot find module '../../../../../../test/javascript/spec/helpers/mocks/service/mock-translate.service' or its corresponding type declarations.🪛 GitHub Check: client-tests-selected
[failure] 11-11:
Cannot find module '../../../../../../test/javascript/spec/helpers/mocks/service/mock-translate.service' or its corresponding type declarations.
...ain/webapp/app/programming/shared/lifecycle/programming-exercise-lifecycle.component.spec.ts
Show resolved
Hide resolved
End-to-End (E2E) Test Results Summary
|
End-to-End (E2E) Test Results Summary
|
…iminary-feedback-model
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/webapp/app/core/course/overview/exercise-details/student-actions/exercise-details-student-actions.component.spec.ts (1)
547-563
: Add Athena profile check for programming exercise testUnlike the previous tests, this test doesn't mock the profileService to include the Athena profile. Consider adding this check to maintain consistency with the other tests, or include a comment explaining why it's intentionally different.
+ getProfileInfoSub = jest.spyOn(profileService, 'getProfileInfo'); + getProfileInfoSub.mockReturnValue( + of({ + activeProfiles: [PROFILE_ATHENA], + } as ProfileInfo), + ); + const exercise = { type: ExerciseType.PROGRAMMING } as ProgrammingExercise; exercise.studentParticipations = [ {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/de/tum/cit/aet/artemis/athena/web/AthenaResource.java
(2 hunks)src/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.java
(4 hunks)src/main/webapp/app/core/course/overview/exercise-details/student-actions/exercise-details-student-actions.component.html
(2 hunks)src/main/webapp/app/core/course/overview/exercise-details/student-actions/exercise-details-student-actions.component.spec.ts
(2 hunks)src/main/webapp/i18n/de/exercise.json
(1 hunks)src/main/webapp/i18n/en/exercise.json
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/main/webapp/i18n/de/exercise.json
- src/main/webapp/i18n/en/exercise.json
- src/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.java
- src/main/java/de/tum/cit/aet/artemis/athena/web/AthenaResource.java
🧰 Additional context used
📓 Path-based instructions (2)
`src/main/webapp/**/*.ts`: angular_style:https://angular.io/guide/styleguide;methods_in_html:false;lazy_loading:true;code_reuse:true;tests:meaningful;types:PascalCase;enums:PascalC...
src/main/webapp/app/core/course/overview/exercise-details/student-actions/exercise-details-student-actions.component.spec.ts
`src/main/webapp/**/*.html`: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.
src/main/webapp/**/*.html
: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.
src/main/webapp/app/core/course/overview/exercise-details/student-actions/exercise-details-student-actions.component.html
🧬 Code Definitions (1)
src/main/webapp/app/core/course/overview/exercise-details/student-actions/exercise-details-student-actions.component.spec.ts (2)
src/main/webapp/app/app.constants.ts (1)
PROFILE_ATHENA
(50-50)src/main/webapp/app/core/layouts/profiles/profile-info.model.ts (1)
ProfileInfo
(6-63)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Build and Push Docker Image / Build Docker Image for ls1intum/artemis
- GitHub Check: Build and Push Docker Image / Build Docker Image for ls1intum/artemis
- GitHub Check: Build .war artifact
- GitHub Check: Analyse
- GitHub Check: server-tests
- GitHub Check: client-tests-selected
- GitHub Check: client-tests
🔇 Additional comments (5)
src/main/webapp/app/core/course/overview/exercise-details/student-actions/exercise-details-student-actions.component.spec.ts (3)
32-32
: LGTM: Good import of constantUsing the constant
PROFILE_ATHENA
is a good practice instead of hardcoding the profile name.
505-524
: Well-structured test for feedback button display in text exercisesThis test correctly verifies that the feedback button is displayed for text exercises when the Athena profile is enabled and the exercise is before its due date, which aligns with the PR objectives of configuring separate modules for preliminary feedback.
526-545
: Well-structured test for feedback button display in modeling exercisesThe test follows the same pattern as the text exercise test, ensuring consistent verification of the feedback button visibility logic across different exercise types.
src/main/webapp/app/core/course/overview/exercise-details/student-actions/exercise-details-student-actions.component.html (2)
133-135
: Simplified condition for programming exercises feedback buttonThe condition for displaying the feedback button for programming exercises has been appropriately simplified to only check for the existence of gradedParticipation and exercise type.
225-225
: Updated condition for text/modeling exercises feedback buttonThe condition has been correctly updated to check if Athena is enabled and if the exercise is a text or modeling type before its due date, aligning with the PR objective of allowing instructors to configure separate modules for preliminary feedback.
End-to-End (E2E) Test Results Summary
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code
athenaApi.ifPresentOrElse(api -> api.checkHasAccessToAthenaModule(programmingExercise, course, ModuleType.FEEDBACK_SUGGESTIONS, ENTITY_NAME), | ||
() -> programmingExercise.setFeedbackSuggestionModule(null)); | ||
athenaApi.ifPresentOrElse(api -> api.checkHasAccessToAthenaModule(programmingExercise, course, ModuleType.PRELIMINARY_FEEDBACK, ENTITY_NAME), | ||
() -> programmingExercise.setPreliminaryFeedbackModule(null)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this snippet not wrapped by try/catch? I saw you wrapped the same snippet in src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseExportImportResource.java
…iminary-feedback-model
End-to-End (E2E) Test Results Summary
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please resolve the merge conflicts and get the PR finished asap.
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. |
Please test only on TS3
!!! Contains database migrations !!!
Checklist
General
Server
Client
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
Performance Tests
Screenshots
Summary by CodeRabbit
New Features
FEEDBACK_SUGGESTIONS
andPRELIMINARY_FEEDBACK
.UI/Text Updates
Enhancements