Athena: Extend the learner profile with feedback preferences
#10916
Conversation
github-actions
bot
added
server
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
WalkthroughThis change introduces a feedback-specific learner profile feature, adding new fields to the backend Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as FeedbackLearnerProfileComponent
participant API as LearnerProfileApiService
participant Server as LearnerProfileResource
participant DB as LearnerProfileRepository
User->>UI: Open learner profile settings
UI->>API: getLearnerProfileForCurrentUser()
API->>Server: GET /api/atlas/learner-profiles
Server->>DB: findByUser()
DB-->>Server: LearnerProfile
Server-->>API: LearnerProfileDTO
API-->>UI: LearnerProfileDTO
UI->>User: Display feedback profile options
User->>UI: Change feedback preference
UI->>API: putUpdatedLearnerProfile(updatedDTO)
API->>Server: PUT /api/atlas/learner-profiles/{id}
Server->>DB: findById(id)
DB-->>Server: LearnerProfile
Server->>DB: save(updated LearnerProfile)
DB-->>Server: LearnerProfile
Server-->>API: LearnerProfileDTO
API-->>UI: LearnerProfileDTO
UI->>User: Show success message
sequenceDiagram
participant AthenaService
participant AthenaFeedbackSuggestionsService
participant LearnerProfileApi
participant LearnerProfileRepository
AthenaService->>AthenaFeedbackSuggestionsService: getTextFeedbackSuggestions(submission)
AthenaFeedbackSuggestionsService->>LearnerProfileApi: findByIdInitialized(profileId)
LearnerProfileApi->>LearnerProfileRepository: findByIdInitialized(profileId)
LearnerProfileRepository-->>LearnerProfileApi: LearnerProfile
LearnerProfileApi-->>AthenaFeedbackSuggestionsService: LearnerProfile
AthenaFeedbackSuggestionsService-->>AthenaService: RequestDTO (with LearnerProfileDTO)
Possibly related PRs
Suggested labels
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (6)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (1)
src/main/webapp/app/core/user/settings/learner-profile/feedback-learner-profile/feedback-learner-profile.component.spec.ts (1)
1-23: 🛠️ Refactor suggestionExpand test coverage to include meaningful test cases.
The current test only verifies component creation, but according to the coding guidelines, tests should be meaningful. Based on the component's purpose (managing learner profile feedback settings), consider adding tests for:
- Loading and displaying feedback preferences
- Handling user interactions with segmented toggles
- API service interactions (with mocked services)
- Error handling scenarios
- Form validation and saving
Additionally, the TestBed configuration likely needs additional dependencies that the component requires, such as:
HttpClientTestingModulefor API callsTranslateModule.forRoot()for translations- Mock providers for
AlertServiceandLearnerProfileApiServiceimport { ComponentFixture, TestBed } from '@angular/core/testing'; +import { HttpClientTestingModule } from '@angular/common/http/testing'; +import { TranslateModule } from '@ngx-translate/core'; +import { AlertService } from 'app/shared/service/alert.service'; +import { LearnerProfileApiService } from '../learner-profile-api.service'; import { FeedbackLearnerProfileComponent } from './feedback-learner-profile.component'; describe('FeedbackLearnerProfileComponent', () => { let component: FeedbackLearnerProfileComponent; let fixture: ComponentFixture<FeedbackLearnerProfileComponent>; + let mockAlertService: jasmine.SpyObj<AlertService>; + let mockApiService: jasmine.SpyObj<LearnerProfileApiService>; beforeEach(async () => { + mockAlertService = jasmine.createSpyObj('AlertService', ['addAlert', 'closeAll']); + mockApiService = jasmine.createSpyObj('LearnerProfileApiService', ['getLearnerProfileForCurrentUser', 'putUpdatedLearnerProfile']); + await TestBed.configureTestingModule({ imports: [ FeedbackLearnerProfileComponent, + HttpClientTestingModule, + TranslateModule.forRoot() ], + providers: [ + { provide: AlertService, useValue: mockAlertService }, + { provide: LearnerProfileApiService, useValue: mockApiService } + ] }).compileComponents(); fixture = TestBed.createComponent(FeedbackLearnerProfileComponent); component = fixture.componentInstance; fixture.detectChanges(); }); it('should create', () => { expect(component).toBeTruthy(); }); + + // Add meaningful test cases here for component functionality });
🧹 Nitpick comments (6)
src/main/webapp/app/core/user/settings/learner-profile/course-learner-profile/course-learner-profile.component.ts (1)
181-195: Consider improving error message localization consistency.While the success and validation messages now use translation keys, the error handling still contains hardcoded English fallback messages ('An error occurred', 'An unexpected error occurred'). For complete internationalization consistency, consider using translation keys for these error messages as well.
- errorMessage = error.error?.title || error.headers?.get('x-artemisapp-alert') || 'An error occurred'; + errorMessage = error.error?.title || error.headers?.get('x-artemisapp-alert') || this.translateService.instant('artemisApp.error.general'); } else { - errorMessage = 'An unexpected error occurred'; + errorMessage = this.translateService.instant('artemisApp.error.unexpected');src/main/webapp/app/core/user/settings/learner-profile/dto/learner-profile-dto.model.ts (1)
25-27: Consider including id validation in isValid method.The validation only checks feedback fields but doesn't validate the id field, which could be problematic if the id is invalid.
Consider adding id validation:
public isValid(): boolean { - return this.isValueInRange(this.feedbackAlternativeStandard) && this.isValueInRange(this.feedbackFollowupSummary) && this.isValueInRange(this.feedbackBriefDetailed); + return this.id > 0 && + this.isValueInRange(this.feedbackAlternativeStandard) && + this.isValueInRange(this.feedbackFollowupSummary) && + this.isValueInRange(this.feedbackBriefDetailed); }src/main/java/de/tum/cit/aet/artemis/atlas/dto/LearnerProfileDTO.java (1)
33-35: Consider adding range validation documentation.The clamp method effectively ensures data integrity, but adding JavaDoc with examples would improve clarity.
/** * Clamps the given value to be within the range of {@link LearnerProfile#MIN_PROFILE_VALUE} and {@link LearnerProfile#MAX_PROFILE_VALUE}. * * @param value The value to clamp * @return The clamped value + * @example clamp(-1) returns 1, clamp(5) returns 3, clamp(2) returns 2 */ private static int clamp(int value) {src/main/webapp/app/core/user/settings/learner-profile/feedback-learner-profile/feedback-learner-profile.component.ts (1)
36-47: Consider dynamic translation updates for language switching.The translated labels are computed once during component initialization. If the application supports dynamic language switching, these labels won't update when the language changes.
Consider using a computed signal or getter to ensure labels update when the language changes:
protected get feedbackAlternativeStandardOptions() { return ALTERNATIVE_STANDARD_OPTIONS.map((option) => ({ label: this.translateService.instant(option.translationKey), value: option.level, })); }Alternatively, if performance is a concern, you could subscribe to language change events and update the options accordingly.
src/main/java/de/tum/cit/aet/artemis/atlas/web/CourseLearnerProfileResource.java (2)
113-113: Fix typo in error message.There's a typo in the error message: "courseLEarnerProfileId" should be "courseLearnerProfileId".
- throw new BadRequestAlertException("Provided courseLEarnerProfileId does not match CourseLearnerProfile.", CourseLearnerProfile.ENTITY_NAME, "objectDoesNotMatchId", + throw new BadRequestAlertException("Provided courseLearnerProfileId does not match CourseLearnerProfile.", CourseLearnerProfile.ENTITY_NAME, "objectDoesNotMatchId",
68-69: Consider optimizing database query filtering.The current implementation first queries for profiles by login and then filters by user group membership in Java. This could potentially be optimized by incorporating the group filtering into the database query itself, especially if the dataset is large.
Consider adding a repository method that filters by both login and group membership at the database level:
// In CourseLearnerProfileRepository Set<CourseLearnerProfile> findAllByLoginAndCourseActiveAndUserInGroup(String login, ZonedDateTime now, Set<String> userGroups);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/main/resources/config/liquibase/changelog/20250525101905_changelog.xmlis excluded by!**/*.xmlsrc/main/resources/config/liquibase/master.xmlis excluded by!**/*.xml
📒 Files selected for processing (28)
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.java(7 hunks)src/main/java/de/tum/cit/aet/artemis/atlas/api/LearnerProfileApi.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/atlas/domain/profile/LearnerProfile.java(4 hunks)src/main/java/de/tum/cit/aet/artemis/atlas/dto/CourseLearnerProfileDTO.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/atlas/dto/LearnerProfileDTO.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/atlas/repository/LearnerProfileRepository.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/atlas/service/profile/LearnerProfileService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/atlas/web/CourseLearnerProfileResource.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/atlas/web/LearnerProfileResource.java(3 hunks)src/main/webapp/app/core/learner-profile/shared/entities/learner-profile-options.model.ts(0 hunks)src/main/webapp/app/core/user/settings/learner-profile/course-learner-profile/course-learner-profile.component.spec.ts(1 hunks)src/main/webapp/app/core/user/settings/learner-profile/course-learner-profile/course-learner-profile.component.ts(3 hunks)src/main/webapp/app/core/user/settings/learner-profile/dto/learner-profile-dto.model.ts(1 hunks)src/main/webapp/app/core/user/settings/learner-profile/entities/course-learner-profile-options.model.ts(1 hunks)src/main/webapp/app/core/user/settings/learner-profile/entities/learner-profile-options.model.ts(1 hunks)src/main/webapp/app/core/user/settings/learner-profile/feedback-learner-profile/feedback-learner-profile.component.html(1 hunks)src/main/webapp/app/core/user/settings/learner-profile/feedback-learner-profile/feedback-learner-profile.component.scss(1 hunks)src/main/webapp/app/core/user/settings/learner-profile/feedback-learner-profile/feedback-learner-profile.component.spec.ts(1 hunks)src/main/webapp/app/core/user/settings/learner-profile/feedback-learner-profile/feedback-learner-profile.component.ts(1 hunks)src/main/webapp/app/core/user/settings/learner-profile/learner-profile-api.service.ts(2 hunks)src/main/webapp/app/core/user/settings/learner-profile/learner-profile.component.html(1 hunks)src/main/webapp/app/core/user/settings/learner-profile/learner-profile.component.ts(1 hunks)src/main/webapp/app/shared/segmented-toggle/segmented-toggle.component.html(1 hunks)src/main/webapp/app/shared/segmented-toggle/segmented-toggle.component.scss(1 hunks)src/main/webapp/app/shared/segmented-toggle/segmented-toggle.component.spec.ts(3 hunks)src/main/webapp/app/shared/segmented-toggle/segmented-toggle.component.ts(1 hunks)src/main/webapp/i18n/de/learnerProfile.json(2 hunks)src/main/webapp/i18n/en/learnerProfile.json(2 hunks)
💤 Files with no reviewable changes (1)
- src/main/webapp/app/core/learner-profile/shared/entities/learner-profile-options.model.ts
🧰 Additional context used
📓 Path-based instructions (4)
`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/user/settings/learner-profile/course-learner-profile/course-learner-profile.component.tssrc/main/webapp/app/core/user/settings/learner-profile/course-learner-profile/course-learner-profile.component.spec.tssrc/main/webapp/app/core/user/settings/learner-profile/learner-profile.component.tssrc/main/webapp/app/shared/segmented-toggle/segmented-toggle.component.spec.tssrc/main/webapp/app/core/user/settings/learner-profile/entities/course-learner-profile-options.model.tssrc/main/webapp/app/core/user/settings/learner-profile/feedback-learner-profile/feedback-learner-profile.component.spec.tssrc/main/webapp/app/core/user/settings/learner-profile/learner-profile-api.service.tssrc/main/webapp/app/shared/segmented-toggle/segmented-toggle.component.tssrc/main/webapp/app/core/user/settings/learner-profile/feedback-learner-profile/feedback-learner-profile.component.tssrc/main/webapp/app/core/user/settings/learner-profile/dto/learner-profile-dto.model.tssrc/main/webapp/app/core/user/settings/learner-profile/entities/learner-profile-options.model.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/user/settings/learner-profile/learner-profile.component.htmlsrc/main/webapp/app/shared/segmented-toggle/segmented-toggle.component.htmlsrc/main/webapp/app/core/user/settings/learner-profile/feedback-learner-profile/feedback-learner-profile.component.html
`src/main/webapp/i18n/de/**/*.json`: German language translations should be informal (dutzen) and should never be formal (sietzen). So the user should always be addressed with "du/...
src/main/webapp/i18n/de/**/*.json: German language translations should be informal (dutzen) and should never be formal (sietzen). So the user should always be addressed with "du/dein" and never with "sie/ihr".
src/main/webapp/i18n/de/learnerProfile.json
`src/main/java/**/*.java`: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,de...
src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/atlas/dto/CourseLearnerProfileDTO.javasrc/main/java/de/tum/cit/aet/artemis/atlas/repository/LearnerProfileRepository.javasrc/main/java/de/tum/cit/aet/artemis/atlas/api/LearnerProfileApi.javasrc/main/java/de/tum/cit/aet/artemis/atlas/dto/LearnerProfileDTO.javasrc/main/java/de/tum/cit/aet/artemis/atlas/domain/profile/LearnerProfile.javasrc/main/java/de/tum/cit/aet/artemis/atlas/service/profile/LearnerProfileService.javasrc/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.javasrc/main/java/de/tum/cit/aet/artemis/atlas/web/LearnerProfileResource.javasrc/main/java/de/tum/cit/aet/artemis/atlas/web/CourseLearnerProfileResource.java
🧬 Code Graph Analysis (3)
src/main/java/de/tum/cit/aet/artemis/atlas/dto/LearnerProfileDTO.java (1)
src/main/webapp/app/core/user/settings/learner-profile/dto/learner-profile-dto.model.ts (1)
LearnerProfileDTO(1-32)
src/main/java/de/tum/cit/aet/artemis/atlas/web/LearnerProfileResource.java (1)
src/main/webapp/app/core/user/settings/learner-profile/dto/learner-profile-dto.model.ts (1)
LearnerProfileDTO(1-32)
src/main/java/de/tum/cit/aet/artemis/atlas/web/CourseLearnerProfileResource.java (3)
src/main/java/de/tum/cit/aet/artemis/atlas/web/LearnerProfileResource.java (1)
Conditional(26-94)src/main/webapp/app/core/user/settings/learner-profile/dto/course-learner-profile-dto.model.ts (1)
CourseLearnerProfileDTO(1-27)src/main/webapp/app/core/user/user.model.ts (1)
User(5-62)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Build and Push Docker Image / Build Docker Image for ls1intum/artemis
- GitHub Check: Build and Push Docker Image / Build Docker Image for ls1intum/artemis
- GitHub Check: server-tests
- GitHub Check: Analyse
🔇 Additional comments (39)
src/main/webapp/app/shared/segmented-toggle/segmented-toggle.component.scss (1)
1-9: LGTM! Clean and responsive CSS implementation.The SCSS styling is well-structured and follows best practices:
- Uses semantic class names with kebab-case convention
- Implements responsive full-width layout with flexbox
- Simple and maintainable CSS rules
src/main/webapp/app/core/user/settings/learner-profile/feedback-learner-profile/feedback-learner-profile.component.scss (1)
1-11: LGTM! Accessible and scalable typography styling.The SCSS implementation follows good practices:
- Uses
remunits for better accessibility and scalability- Semantic class names for
.titleand.description- Appropriate font sizes and spacing for clear visual hierarchy
- Clean and maintainable structure
src/main/webapp/app/core/user/settings/learner-profile/course-learner-profile/course-learner-profile.component.ts (2)
5-6: LGTM! Import paths properly updated for new directory structure.The import path updates correctly reflect the reorganized directory structure and maintain consistency across the codebase.
Also applies to: 10-10
152-152: LGTM! Improved internationalization by using translation keys only.The removal of hardcoded strings and reliance on translation keys improves the localization consistency of the application.
Also applies to: 169-169
src/main/webapp/app/core/user/settings/learner-profile/learner-profile.component.html (1)
1-2: LGTM! Clean integration of feedback learner profile component.The addition of the feedback learner profile component with proper visual separation using the horizontal rule is well-structured and follows Angular conventions.
src/main/java/de/tum/cit/aet/artemis/atlas/dto/CourseLearnerProfileDTO.java (1)
28-28: Good documentation improvement!The explicit Javadoc references to the constants improve clarity and maintainability by providing direct links to the actual constraint values.
src/main/webapp/app/shared/segmented-toggle/segmented-toggle.component.html (1)
1-9: Well-structured component improvements!The changes correctly use the new
@forsyntax as recommended in the coding guidelines and add proper container structure with Bootstrap classes for better styling control.src/main/webapp/app/core/user/settings/learner-profile/course-learner-profile/course-learner-profile.component.spec.ts (1)
2-2: Import path updates look correct.The import path changes properly reflect the new directory structure and maintain consistency with the refactored learner profile organization.
Also applies to: 4-4, 8-8
src/main/java/de/tum/cit/aet/artemis/atlas/api/LearnerProfileApi.java (2)
9-9: LGTM! Import follows Java conventions.The import for
LearnerProfileis correctly added to support the new API method's return type.
52-60: LGTM! Well-implemented API method following best practices.The new
findByIdInitializedmethod properly:
- Follows camelCase naming convention as per guidelines
- Includes comprehensive JavaDoc documentation
- Delegates to the service layer as required by REST guidelines
- Uses single responsibility principle
- Maintains stateless API design
src/main/webapp/app/core/user/settings/learner-profile/learner-profile.component.ts (2)
2-3: LGTM! Import statements follow Angular conventions.The import paths are correctly structured and the new
FeedbackLearnerProfileComponentimport supports the feedback preferences functionality.
9-9: LGTM! Component integration follows Angular best practices.The
FeedbackLearnerProfileComponentis properly added to the imports array, enabling the new feedback learner profile functionality in the UI.src/main/webapp/app/shared/segmented-toggle/segmented-toggle.component.spec.ts (5)
5-6: LGTM! Proper generic type usage for component testing.The component and fixture declarations correctly use the generic type parameter
<number>, aligning with the generalizedSegmentedToggleComponentdesign.
9-12: LGTM! Test data properly updated for numeric values.The mock options array correctly uses numeric literals instead of enum values, maintaining consistency with the component's new generic design.
19-19: LGTM! Test setup follows TypeScript generic conventions.The component creation properly specifies the generic type parameter, ensuring type safety in the test environment.
31-43: LGTM! Test assertions updated consistently.All test values and assertions have been properly updated to use numeric literals, maintaining test coverage while adapting to the generic component interface.
70-70: LGTM! Selected value test maintains consistency.The selected value test correctly uses numeric literal, ensuring consistent testing approach throughout the test suite.
src/main/java/de/tum/cit/aet/artemis/atlas/service/profile/LearnerProfileService.java (1)
46-54: LGTM! Service method follows best practices.The
findByIdInitializedmethod is well-implemented with:
- Comprehensive JavaDoc documentation
- Proper delegation to repository layer
- Appropriate Optional handling with
orElse(null)- Single responsibility principle
- CamelCase naming convention as per guidelines
src/main/webapp/app/core/user/settings/learner-profile/feedback-learner-profile/feedback-learner-profile.component.html (1)
1-37: LGTM! Well-structured Angular template following best practices.The template demonstrates excellent Angular practices:
- Proper use of
jhiTranslatefor internationalization- Correct two-way binding syntax with
[(selected)]- Appropriate event handling with
(selectedChange)- Clean Bootstrap layout with responsive grid system
- Semantic HTML structure with accessible labels and descriptions
The conditional visibility pattern using
[ngClass]="{ 'd-none': disabled }"is a good approach for managing component state.src/main/webapp/app/core/user/settings/learner-profile/entities/course-learner-profile-options.model.ts (1)
1-12: LGTM! Clean TypeScript model following conventions.The interface and constant array are well-designed:
- Clear, descriptive property names using camelCase convention
- Consistent translation key naming pattern
- Logical level progression from 1-5
- Good separation of concerns with dedicated model file
src/main/webapp/i18n/en/learnerProfile.json (2)
22-22: Good specificity improvement in success message.Updating the message from generic "Learner profile saved" to "Course learner profile saved" provides better user feedback and clarity about which profile was actually saved.
32-65: Comprehensive and well-structured feedback profile localization.The new feedback learner profile section is excellently organized:
- Clear hierarchical structure matching the UI components
- Descriptive labels and explanations for each preference category
- Consistent three-level options (brief/balanced/detailed, etc.)
- Proper validation message indicating the 1-3 range
- Complete coverage including success, validation, and error scenarios
The translation keys align perfectly with the HTML template usage.
src/main/webapp/app/core/user/settings/learner-profile/entities/learner-profile-options.model.ts (1)
1-22: LGTM! Consistent model structure for feedback preferences.The implementation follows the established pattern from the course options model:
- Same interface structure with
levelandtranslationKeyproperties- Consistent three-level progression (1-3) for each preference type
- Well-organized constant arrays for each feedback dimension
- Translation keys properly reference the feedback learner profile section
- camelCase naming convention adhered to
The three-level system aligns well with the typical preference scales and backend constraints.
src/main/webapp/app/core/user/settings/learner-profile/learner-profile-api.service.ts (2)
2-3: Good import organization for the new DTOs.The imports are properly organized to include both the existing
CourseLearnerProfileDTOand the newLearnerProfileDTOfrom the dto subdirectory, maintaining clean separation of concerns.
16-22: Well-designed API methods following established patterns.The new learner profile methods demonstrate excellent consistency:
getLearnerProfileForCurrentUser()follows the same async pattern as the course profile methodputUpdatedLearnerProfile()maintains the same signature pattern with proper typing- RESTful endpoint naming (
atlas/learner-profilesandatlas/learner-profiles/{id})- Proper use of generics for type safety
- Consistent with the existing service architecture
src/main/webapp/app/core/user/settings/learner-profile/dto/learner-profile-dto.model.ts (1)
6-11:✅ Verification successful
Verify constant synchronization with server-side values.
The comments mention these constants must match the server-side LearnerProfile.java, but there's no automated verification of this synchronization.
🏁 Script executed:
#!/bin/bash # Description: Verify that MIN_VALUE and MAX_VALUE constants match between frontend and backend. # Expected: Frontend values (1,3) should match backend MIN_PROFILE_VALUE and MAX_PROFILE_VALUE. echo "Frontend constants:" rg -A 1 "MIN_VALUE|MAX_VALUE" src/main/webapp/app/core/user/settings/learner-profile/dto/learner-profile-dto.model.ts echo -e "\nBackend constants:" rg -A 1 "MIN_PROFILE_VALUE|MAX_PROFILE_VALUE" src/main/java/de/tum/cit/aet/artemis/atlas/domain/profile/LearnerProfile.javaLength of output: 966
Constants synchronization confirmed
- src/main/webapp/app/core/user/settings/learner-profile/dto/learner-profile-dto.model.ts:
MIN_VALUE = 1, MAX_VALUE = 3- src/main/java/de/tum/cit/aet/artemis/atlas/domain/profile/LearnerProfile.java:
MIN_PROFILE_VALUE = 1, MAX_PROFILE_VALUE = 3No discrepancies found—constants are in sync.
src/main/java/de/tum/cit/aet/artemis/atlas/dto/LearnerProfileDTO.java (1)
19-25: Excellent implementation with proper null handling and data integrity.The static factory method correctly handles null inputs and the clamping ensures data integrity. The use of a Java record is appropriate for a DTO.
src/main/webapp/app/shared/segmented-toggle/segmented-toggle.component.ts (2)
11-20: Excellent refactor to make component generic and reusable.The conversion to a generic component with type parameter
Tsignificantly improves reusability while maintaining type safety. The implementation correctly handles the generic types in inputs, outputs, and methods.
9-9: Good addition of component-specific stylesheet.Adding the SCSS stylesheet follows Angular best practices for component-specific styling and maintains better encapsulation.
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.java (2)
94-94: Good extension of RequestDTO to include learner profile.The addition of
LearnerProfileDTO learnerProfileto the RequestDTO record properly supports the new functionality while maintaining backward compatibility with the@JsonInclude(JsonInclude.Include.NON_EMPTY)annotation.
158-159:❓ Verification inconclusive
Consider consistency in learner profile usage across exercise types.
Currently, only text feedback includes learner profile data while programming and modeling feedback explicitly set it to
null. This inconsistency might limit the personalization benefits.Is this intentional design or should learner profiles be included for all exercise types?
Also applies to: 182-183
🏁 Script executed:
#!/bin/bash # Description: Check if there are plans to include learner profiles for programming and modeling feedback # Expected: Find any TODOs or comments about extending learner profile support rg -i "todo|fixme|learner.?profile" src/main/java/de/tum/cit/aet/artemis/athena/ -A 2 -B 2Length of output: 7739
Confirm learner profile inclusion for all exercise types?
I didn’t find any TODOs or design notes indicating support for learner profiles in programming and modeling feedback. Currently:
- In
requestTextFeedbackSuggestions(Lines 158–159), you passLearnerProfileDTO.of(extractLearnerProfile(submission))- But in
requestProgrammingFeedbackSuggestions(Lines 182–183) and in the analogous modeling method, the profile is hard-coded asnull.If you want consistent personalization, consider changing those calls to:
final RequestDTO request = new RequestDTO( athenaDTOConverterService.ofExercise(exercise), athenaDTOConverterService.ofSubmission(exercise.getId(), submission), LearnerProfileDTO.of(extractLearnerProfile(submission)), isGraded );Otherwise, please confirm that omitting learner profiles for programming and modeling is an intentional design decision.
src/main/webapp/i18n/de/learnerProfile.json (1)
22-22: LGTM!The change to "Kurs-Lernprofil gespeichert" appropriately distinguishes between course-specific and feedback-specific profile messages.
src/main/java/de/tum/cit/aet/artemis/atlas/domain/profile/LearnerProfile.java (2)
32-65: Well-structured implementation of feedback preferences!The implementation correctly:
- Defines clear constants for validation bounds
- Uses appropriate JPA column mapping with snake_case naming
- Applies proper validation constraints with
@Minand@Max- Sets sensible default values (2) as the middle of the allowed range
95-117: LGTM!The getter and setter methods follow Java conventions and provide clean access to the feedback preference fields.
src/main/webapp/app/core/user/settings/learner-profile/feedback-learner-profile/feedback-learner-profile.component.ts (1)
60-77: Clean implementation with proper error handling!The component demonstrates good practices:
- Proper async/await error handling
- Clear separation of concerns with private helper methods
- Appropriate use of Angular signals for state management
- Proper alert management with
closeAll()before showing successAlso applies to: 93-117
src/main/java/de/tum/cit/aet/artemis/atlas/web/LearnerProfileResource.java (2)
55-62: LGTM!The GET endpoint correctly fetches the learner profile for the authenticated user using proper security annotations.
71-93: Secure implementation of profile updates!The implementation correctly:
- Validates that the DTO ID matches the path parameter for API consistency
- Fetches the profile using the current authenticated user (line 81), ensuring users can only update their own profile
- Validates all feedback fields are within allowed bounds before updating
- Returns the updated profile in the response
src/main/java/de/tum/cit/aet/artemis/atlas/web/CourseLearnerProfileResource.java (2)
41-55: Good implementation of constructor injection and immutable fields.The constructor properly implements dependency injection following the coding guidelines, and all injected dependencies are declared as final fields. This ensures thread safety and follows best practices.
91-96: Excellent validation method implementation.The
validateProfileFieldmethod is well-designed with:
- Clear parameter validation
- Descriptive error messages
- Proper exception handling
- Reusable across different profile fields
This follows the DRY principle and provides consistent validation behavior.
src/main/java/de/tum/cit/aet/artemis/atlas/repository/LearnerProfileRepository.java
Outdated
Show resolved
Hide resolved
src/main/webapp/app/core/user/settings/learner-profile/dto/learner-profile-dto.model.ts
Outdated
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.java
Outdated
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/atlas/web/CourseLearnerProfileResource.java
Show resolved
Hide resolved
github-project-automation
bot
moved this from Work In Progress
to Ready For Review
in Artemis Development
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||
|
@coderabbitai review |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/webapp/app/core/user/settings/learner-profile/feedback-learner-profile/feedback-learner-profile.component.spec.ts (1)
17-17: Remove unused variable.The
alertServicevariable is declared and injected but never used in any test cases.- let alertService: AlertService;Also remove the corresponding injection on line 41:
- alertService = TestBed.inject(AlertService);🧰 Tools
🪛 GitHub Check: client-tests-selected
[failure] 17-17:
'alertService' is declared but its value is never read.🪛 GitHub Check: client-tests
[failure] 17-17:
'alertService' is declared but its value is never read.
📜 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/service/AthenaFeedbackSuggestionsService.java(7 hunks)src/main/java/de/tum/cit/aet/artemis/atlas/api/LearnerProfileApi.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/atlas/service/profile/LearnerProfileService.java(1 hunks)src/main/webapp/app/core/user/settings/learner-profile/dto/learner-profile-dto.model.ts(1 hunks)src/main/webapp/app/core/user/settings/learner-profile/feedback-learner-profile/feedback-learner-profile.component.spec.ts(1 hunks)src/main/webapp/i18n/de/learnerProfile.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/de/tum/cit/aet/artemis/atlas/service/profile/LearnerProfileService.java
- src/main/java/de/tum/cit/aet/artemis/atlas/api/LearnerProfileApi.java
- src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.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/user/settings/learner-profile/feedback-learner-profile/feedback-learner-profile.component.spec.tssrc/main/webapp/app/core/user/settings/learner-profile/dto/learner-profile-dto.model.ts
`src/main/webapp/i18n/de/**/*.json`: German language translations should be informal (dutzen) and should never be formal (sietzen). So the user should always be addressed with "du/...
src/main/webapp/i18n/de/**/*.json: German language translations should be informal (dutzen) and should never be formal (sietzen). So the user should always be addressed with "du/dein" and never with "sie/ihr".
src/main/webapp/i18n/de/learnerProfile.json
🧬 Code Graph Analysis (1)
src/main/webapp/app/core/user/settings/learner-profile/feedback-learner-profile/feedback-learner-profile.component.spec.ts (2)
src/main/webapp/app/core/user/settings/learner-profile/dto/learner-profile-dto.model.ts (1)
LearnerProfileDTO(8-56)src/test/javascript/spec/helpers/mocks/service/mock-alert.service.ts (1)
MockAlertService(3-8)
🪛 GitHub Check: client-tests-selected
src/main/webapp/app/core/user/settings/learner-profile/feedback-learner-profile/feedback-learner-profile.component.spec.ts
[failure] 17-17:
'alertService' is declared but its value is never read.
🪛 GitHub Check: client-tests
src/main/webapp/app/core/user/settings/learner-profile/feedback-learner-profile/feedback-learner-profile.component.spec.ts
[failure] 17-17:
'alertService' is declared but its value is never read.
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: server-tests
- 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: Analyse
🔇 Additional comments (3)
src/main/webapp/app/core/user/settings/learner-profile/feedback-learner-profile/feedback-learner-profile.component.spec.ts (1)
26-133: Excellent test coverage and structure.The test suite demonstrates comprehensive coverage with proper:
- Component lifecycle testing
- Success and error scenario handling
- Async operation testing with appropriate
fixture.whenStable()calls- Mock setup using Jest spies
- Edge case testing (no profile exists)
The testing approach follows Angular best practices and provides good confidence in the component behavior.
src/main/webapp/app/core/user/settings/learner-profile/dto/learner-profile-dto.model.ts (1)
1-56: Well-implemented DTO with robust validation.The implementation demonstrates excellent improvements over previous versions:
- Type Safety: Constructor uses proper
LearnerProfileDatainterface instead ofany- Validation: Comprehensive validation with
validateAndGetValue()andisValueInRange()methods- Error Handling: Proper null checks and meaningful error messages
- Constants: Well-defined Likert scale bounds that align with backend constraints
- Default Values: Sensible defaults using null coalescing operators
The validation logic ensuring values stay within the 1-3 range is particularly important for the Likert scale feedback preferences.
src/main/webapp/i18n/de/learnerProfile.json (1)
32-65: German translations are grammatically correct and well-structured.The new feedback learner profile section demonstrates:
- Proper Grammar: Previous grammatical issues have been resolved (correct articles and declensions)
- Informal Addressing: Correctly uses "du/dein" throughout as required by coding guidelines
- Clear Descriptions: Each feedback preference category has clear, understandable descriptions
- Consistent Structure: Follows the same localization pattern as the existing course learner profile
- Appropriate Validation Messages: Error messages align with the 1-3 range validation in the DTO
The translations effectively communicate the feedback preference options to German-speaking users.
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||
☣️ Don't deploy - contains migration
Checklist
General
Server
Client
authoritiesto all new routes and checked the course groups for displaying navigation elements (links, buttons).Motivation and Context
We want to further personalize the feedback students get from the exercise submissions. A naive way is to let students choose their preferences about the feedback style.
Description
Three attributes setting the feedback's style have been added to the table
Learner Profile:These attributes can now be set in the learner profile tab under the user settings.
Steps for Testing
Prerequisites:
Learner Profile Setup
Learner Profilefrom the left sections and see the attributes related to feedback styleTesting Athena Functionalities (To test if adding learner profile breaks anything)
Testserver States
You can manage test servers using Helios. Check environment statuses in the environment list. To deploy to a test server, go to the CI/CD page, find your PR or branch, and trigger the deployment.
Review Progress
Code Review
Manual Tests
Test Coverage
Screenshots
Screen.Recording.2025-05-25.at.16.50.10.mov
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Refactor
Tests
Style