Quiz exercises: Add quiz training settings to user settings
#11488
Conversation
github-actions
bot
added
server
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
WalkthroughIntroduces a user-facing “Quiz Training” settings feature with a leaderboard visibility toggle. Adds a backend GET/PUT API for leaderboard settings, a repository query to fetch the flag, renames a DTO field ( Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant FE as QuizTrainingSettingsComponent
participant Svc as QuizTrainingSettingsService
participant API as QuizTrainingResource
participant Repo as QuizTrainingLeaderboardRepository
User->>FE: open Quiz Training settings
FE->>Svc: getSettings()
Svc->>API: GET /api/quiz/leaderboard-settings
API->>Repo: getShowInLeaderboard(userId)
Repo-->>API: Optional<Boolean>
API-->>Svc: 200 { showInLeaderboard }
Svc-->>FE: HttpResponse<DTO>
FE->>FE: bind toggle to showInLeaderboard
rect rgba(219,237,255,0.3)
User->>FE: toggle visibility
FE->>Svc: updateSettings({ showInLeaderboard })
Svc->>API: PUT /api/quiz/leaderboard-settings { showInLeaderboard }
API->>Repo: persist change
API-->>Svc: 200 { showInLeaderboard }
Svc-->>FE: HttpResponse<DTO>
FE->>User: show success message
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/main/java/de/tum/cit/aet/artemis/quiz/repository/QuizTrainingLeaderboardRepository.java (1)
74-79: Consider removing DISTINCT for clarity.The
DISTINCTkeyword is likely unnecessary since there should be at most oneQuizTrainingLeaderboardentry peruserId. While not harmful, removing it would make the intent clearer and potentially improve query performance slightly.Apply this diff if there's truly a 1:1 relationship between user and showInLeaderboard:
@Query(""" - SELECT DISTINCT qtl.showInLeaderboard + SELECT qtl.showInLeaderboard FROM QuizTrainingLeaderboard qtl WHERE qtl.user.id = :userId """) Optional<Boolean> getShowInLeaderboard(@Param("userId") long userId);src/main/webapp/app/core/user/settings/quiz-training-settings/quiz-training-settings.component.ts (1)
31-37: Add error handling to loadSettings.The
loadSettings()method lacks error handling. If the API call fails,isVisibleInLeaderboardremainsundefinedwith no user feedback. While this triggers the info alert, it's unclear whether it's a loading error or intentional behavior.Apply this diff to add error handling:
private loadSettings(): void { - this.quizService.getSettings().subscribe((response) => { - if (response.body) { - this.isVisibleInLeaderboard = response.body.showInLeaderboard; - } - }); + this.quizService.getSettings().subscribe({ + next: (response) => { + if (response.body) { + this.isVisibleInLeaderboard = response.body.showInLeaderboard; + } + }, + error: (error) => { + onError(this.alertService, error); + }, + }); }src/main/java/de/tum/cit/aet/artemis/quiz/dto/LeaderboardSettingDTO.java (1)
8-8: All code references updated; noshownInLeaderboardoccurrences remain.
- Update Javadoc comments still referring to the old name.
src/main/webapp/app/core/user/settings/quiz-training-settings/quiz-training-settings-service.ts (1)
10-16: Prefer single quotes for simple strings without interpolation.Lines 11 and 15 use template literals (backticks) for simple URL strings without any interpolation. Consider using single quotes for consistency with the coding guidelines.
Apply this diff:
getSettings(): Observable<HttpResponse<LeaderboardSettingsDTO>> { - return this.http.get(`api/quiz/leaderboard-settings`, { observe: 'response' }); + return this.http.get('api/quiz/leaderboard-settings', { observe: 'response' }); } updateSettings(settings: LeaderboardSettingsDTO): Observable<HttpResponse<LeaderboardSettingsDTO>> { - return this.http.put(`api/quiz/leaderboard-settings`, settings, { observe: 'response' }); + return this.http.put('api/quiz/leaderboard-settings', settings, { observe: 'response' }); }As per coding guidelines
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/main/java/de/tum/cit/aet/artemis/quiz/dto/LeaderboardSettingDTO.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/quiz/repository/QuizTrainingLeaderboardRepository.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/quiz/web/QuizTrainingResource.java(5 hunks)src/main/webapp/app/core/user/settings/quiz-training-settings/leaderboard-settings-dto.ts(1 hunks)src/main/webapp/app/core/user/settings/quiz-training-settings/quiz-training-settings-service.spec.ts(1 hunks)src/main/webapp/app/core/user/settings/quiz-training-settings/quiz-training-settings-service.ts(1 hunks)src/main/webapp/app/core/user/settings/quiz-training-settings/quiz-training-settings.component.html(1 hunks)src/main/webapp/app/core/user/settings/quiz-training-settings/quiz-training-settings.component.spec.ts(1 hunks)src/main/webapp/app/core/user/settings/quiz-training-settings/quiz-training-settings.component.ts(1 hunks)src/main/webapp/app/core/user/settings/user-settings-container/user-settings-container.component.html(1 hunks)src/main/webapp/app/core/user/settings/user-settings.route.ts(1 hunks)src/main/webapp/i18n/de/userSettings.json(1 hunks)src/main/webapp/i18n/en/userSettings.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
src/main/java/**/*.java
⚙️ CodeRabbit configuration file
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
Files:
src/main/java/de/tum/cit/aet/artemis/quiz/repository/QuizTrainingLeaderboardRepository.javasrc/main/java/de/tum/cit/aet/artemis/quiz/dto/LeaderboardSettingDTO.javasrc/main/java/de/tum/cit/aet/artemis/quiz/web/QuizTrainingResource.java
src/main/webapp/**/*.ts
⚙️ CodeRabbit configuration file
Files:
src/main/webapp/app/core/user/settings/quiz-training-settings/leaderboard-settings-dto.tssrc/main/webapp/app/core/user/settings/quiz-training-settings/quiz-training-settings.component.tssrc/main/webapp/app/core/user/settings/quiz-training-settings/quiz-training-settings-service.spec.tssrc/main/webapp/app/core/user/settings/quiz-training-settings/quiz-training-settings-service.tssrc/main/webapp/app/core/user/settings/user-settings.route.tssrc/main/webapp/app/core/user/settings/quiz-training-settings/quiz-training-settings.component.spec.ts
src/main/webapp/**/*.html
⚙️ CodeRabbit configuration file
@if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.
Files:
src/main/webapp/app/core/user/settings/quiz-training-settings/quiz-training-settings.component.htmlsrc/main/webapp/app/core/user/settings/user-settings-container/user-settings-container.component.html
src/main/webapp/i18n/de/**/*.json
⚙️ CodeRabbit configuration file
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".
Files:
src/main/webapp/i18n/de/userSettings.json
🧠 Learnings (3)
📓 Common learnings
Learnt from: MoritzSpengler
PR: ls1intum/Artemis#11345
File: src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizTrainingLeaderboardService.java:128-141
Timestamp: 2025-09-21T11:51:17.945Z
Learning: In QuizTrainingLeaderboardService, prefilling leaderboardName with user.getFirstName() during leaderboard entry creation is a deliberate design choice. Privacy protection is handled at the display level rather than initialization level.
📚 Learning: 2025-09-05T15:13:32.171Z
Learnt from: MoritzSpengler
PR: ls1intum/Artemis#11345
File: src/main/java/de/tum/cit/aet/artemis/quiz/repository/QuizTrainingLeaderboardRepository.java:22-23
Timestamp: 2025-09-05T15:13:32.171Z
Learning: The derived query method name `findByLeagueAndCourseIdOrderByScoreDescUserAscId` in QuizTrainingLeaderboardRepository correctly references the `user` entity field directly in the OrderBy clause, not requiring `userId`. Spring Data JPA supports sorting by entity references directly in derived query method names.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/quiz/repository/QuizTrainingLeaderboardRepository.java
📚 Learning: 2025-09-05T15:11:31.588Z
Learnt from: MoritzSpengler
PR: ls1intum/Artemis#11345
File: src/main/java/de/tum/cit/aet/artemis/quiz/domain/QuizTrainingLeaderboard.java:10-11
Timestamp: 2025-09-05T15:11:31.588Z
Learning: In the QuizTrainingLeaderboard domain class, validation is handled by the service class logic rather than using bean validation annotations at the entity level.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/quiz/web/QuizTrainingResource.java
🧬 Code graph analysis (4)
src/main/webapp/app/core/user/settings/quiz-training-settings/quiz-training-settings.component.ts (1)
src/main/webapp/app/core/user/settings/quiz-training-settings/leaderboard-settings-dto.ts (1)
LeaderboardSettingsDTO(1-3)
src/main/webapp/app/core/user/settings/quiz-training-settings/quiz-training-settings-service.spec.ts (1)
src/main/webapp/app/core/user/settings/quiz-training-settings/leaderboard-settings-dto.ts (1)
LeaderboardSettingsDTO(1-3)
src/main/webapp/app/core/user/settings/quiz-training-settings/quiz-training-settings-service.ts (1)
src/main/webapp/app/core/user/settings/quiz-training-settings/leaderboard-settings-dto.ts (1)
LeaderboardSettingsDTO(1-3)
src/main/webapp/app/core/user/settings/quiz-training-settings/quiz-training-settings.component.spec.ts (1)
src/main/webapp/app/core/user/settings/quiz-training-settings/leaderboard-settings-dto.ts (1)
LeaderboardSettingsDTO(1-3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: server-tests
- GitHub Check: client-tests
- GitHub Check: Build and Push Docker Image / Build linux/arm64 Docker Image for ls1intum/artemis
- GitHub Check: Build and Push Docker Image / Build linux/amd64 Docker Image for ls1intum/artemis
- GitHub Check: Analyse
🔇 Additional comments (9)
src/main/webapp/app/core/user/settings/quiz-training-settings/leaderboard-settings-dto.ts (1)
1-3: LGTM!The DTO structure is simple and correct. The
showInLeaderboardproperty name aligns with the backendLeaderboardSettingDTOfield.src/main/webapp/app/core/user/settings/quiz-training-settings/quiz-training-settings.component.html (1)
1-32: LGTM!The template correctly uses modern Angular control flow syntax (
@if/@else), properly implements two-way binding withngModel, and consistently applies i18n directives for all user-facing text. The conditional rendering logic for showing the toggle vs. the info alert is well structured.src/main/webapp/i18n/en/userSettings.json (1)
230-238: LGTM!The i18n keys for Quiz Training settings are well-structured, follow existing naming conventions, and provide clear, informative text for users. The key hierarchy is logical and consistent with other settings sections.
src/main/webapp/i18n/de/userSettings.json (1)
230-238: LGTM! German translations follow the informal tone guideline.All strings correctly use the informal address (du/dein) as required by the coding guidelines for German translations.
src/main/webapp/app/core/user/settings/quiz-training-settings/quiz-training-settings-service.spec.ts (1)
1-48: LGTM! Well-structured test suite with proper HTTP testing.The test suite follows Angular testing best practices:
- Uses modern
provideHttpClientandprovideHttpClientTesting- Includes proper cleanup with
httpMock.verify()inafterEach- Tests both GET and PUT operations with appropriate assertions
src/main/webapp/app/core/user/settings/quiz-training-settings/quiz-training-settings.component.spec.ts (1)
1-139: LGTM! Comprehensive test coverage with proper async handling.The test suite thoroughly covers all component functionality:
- Loading and displaying settings
- Toggling leaderboard visibility
- Error handling with
onErrorutility- DOM interactions and state synchronization
- Edge case: no settings available
All tests use appropriate Angular testing patterns with
fakeAsyncand proper fixture management.src/main/java/de/tum/cit/aet/artemis/quiz/web/QuizTrainingResource.java (3)
7-7: LGTM! Proper dependency injection.The new repository dependency is correctly injected via constructor following the coding guidelines.
Also applies to: 43-43, 68-80
166-166: LGTM! Field name updated consistently.The DTO method call is correctly updated from
shownInLeaderboard()toshowInLeaderboard(), aligning with the DTO field rename across the codebase.Also applies to: 191-191
196-216: LGTM! New GET endpoint follows REST best practices.The implementation is correct:
- Properly documented with Javadoc
- Uses appropriate security annotation (
@EnforceAtLeastStudent)- Returns a DTO with nullable Boolean to indicate "not set" state
- Consistent with the existing PUT endpoint pattern at line 160
...webapp/app/core/user/settings/user-settings-container/user-settings-container.component.html
Show resolved
Hide resolved
github-project-automation
bot
moved this from Work In Progress
to Ready For Review
in Artemis Development
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/webapp/i18n/en/userSettings.json (1)
231-239: Minor copy polish and i18n parity
- Add missing period to the end of
leaderboardVisibilityTooltip:- "leaderboardVisibilityTooltip": "Determines if your name and results are shown in the Quiz Training leaderboard", + "leaderboardVisibilityTooltip": "Determines if your name and results are shown in the Quiz Training leaderboard.",
- Optional: align label/tooltip wording (label mentions “name” only vs. tooltip “name and results”).
- Confirm that “name” refers to the displayed identifier to avoid confusion.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/webapp/app/core/user/settings/user-settings-container/user-settings-container.component.html(1 hunks)src/main/webapp/app/core/user/settings/user-settings.route.ts(1 hunks)src/main/webapp/i18n/de/userSettings.json(2 hunks)src/main/webapp/i18n/en/userSettings.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/webapp/i18n/de/userSettings.json
- src/main/webapp/app/core/user/settings/user-settings-container/user-settings-container.component.html
🧰 Additional context used
📓 Path-based instructions (1)
src/main/webapp/**/*.ts
⚙️ CodeRabbit configuration file
Files:
src/main/webapp/app/core/user/settings/user-settings.route.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: MoritzSpengler
PR: ls1intum/Artemis#11345
File: src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizTrainingLeaderboardService.java:128-141
Timestamp: 2025-09-21T11:51:17.945Z
Learning: In QuizTrainingLeaderboardService, prefilling leaderboardName with user.getFirstName() during leaderboard entry creation is a deliberate design choice. Privacy protection is handled at the display level rather than initialization level.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Build and Push Docker Image / Build linux/arm64 Docker Image for ls1intum/artemis
- GitHub Check: Build and Push Docker Image / Build linux/amd64 Docker Image for ls1intum/artemis
- GitHub Check: Build .war artifact
- GitHub Check: client-tests
- GitHub Check: bean-instantiation-check
- GitHub Check: Analyse
- GitHub Check: client-style
- GitHub Check: server-style
- GitHub Check: server-tests
🔇 Additional comments (2)
src/main/webapp/app/core/user/settings/user-settings.route.ts (1)
115-121: LGTM! Route configuration follows best practices.The quiz-training route is properly configured with:
- Lazy loading for optimal performance
- Translation key for i18n compliance
- Consistent structure with other user settings routes
- Appropriate authorization (inherits USER authority from parent route)
The previous review concern about using a hardcoded pageTitle has been addressed—the code now correctly uses the translation key
artemisApp.userSettings.categories.QUIZ_TRAINING_SETTINGS.As per coding guidelines.
src/main/webapp/i18n/en/userSettings.json (1)
147-149: Categories added: looks good; verify i18n parity and usage.GLOBAL_NOTIFICATIONS and QUIZ_TRAINING_SETTINGS entries look consistent with existing naming. Please ensure:
- The same keys exist in the German locale with translations.
- The UI routes/menus use these exact category keys (no divergence from NOTIFICATION_SETTINGS).
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||||||||
| log.debug("REST request to get leaderboard settings"); | ||
| User user = userRepository.getUserWithGroupsAndAuthorities(); | ||
| Optional<Boolean> showInLeaderboardOptional = quizTrainingLeaderboardRepository.getShowInLeaderboard(user.getId()); | ||
| Boolean showInLeaderboard = showInLeaderboardOptional.orElse(null); |
There was a problem hiding this comment.
why is the value not false when nothing is found. I assume it's a reasonable default to not show them if they have not opted in.
| @Component({ | ||
| selector: 'jhi-quiz-training-settings', | ||
| templateUrl: './quiz-training-settings.component.html', | ||
| standalone: true, |
There was a problem hiding this comment.
standalone is the default from angular 19 onwards
| selector: 'jhi-quiz-training-settings', | ||
| templateUrl: './quiz-training-settings.component.html', | ||
| standalone: true, | ||
| imports: [TranslateDirective, FormsModule, NgbTooltipModule, FontAwesomeModule], |
There was a problem hiding this comment.
Do we really need the entire font awesome module and the entire tooltip module?
3 participants
Checklist
General
Server
Client
authoritiesto all new routes and checked the course groups for displaying navigation elements (links, buttons).Motivation and Context
Students can choose the settings for the quiz training mode when they first enter the mode. We want students to be able to change the settings for the quiz training mode afterwards in the user settings.
Description
I added the quiz training settings component and a service class to display the quiz training settings in the user settings. I also added a REST API and repository method that fetches the current settings from the database.
Steps for Testing
Prerequisites:
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
Summary by CodeRabbit
New Features
Tests