Quiz exercises: Add isRated/isFree Mode Check to Quiz Training
#11382
+580
−180
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
tests
server
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
WalkthroughRemoves Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client (Web)
participant R as QuizTrainingResource
participant S as QuizQuestionProgressService
participant QQRepo as QuizQuestionRepository
participant QPRepo as QuizQuestionProgressRepository
Note over C,R: Load training questions (POST) with pageable, isNewSession, questionIds
C->>R: POST /api/quiz/courses/{courseId}/training-questions\nBody: questionIds\nQuery: page,size,isNewSession
R->>S: getQuestionsForSession(courseId, userId, pageable, questionIds, isNewSession)
alt isNewSession
S->>QPRepo: findAllByUserIdAndCourseId(userId, courseId)
Note over S: derive due question IDs from progress
end
S->>QQRepo: areQuestionsDue? (count/find)
alt Due questions available
S->>QQRepo: findAllDueQuestions(ids, courseId, pageable)
QQRepo-->>S: Slice<QuizQuestion>
Note over S: map to QuizQuestionTrainingDTO (isRated=true, include ids, isNewSession)
else Practice
S->>QQRepo: findAllPracticeQuizQuestionsByCourseId(courseId, pageable)
QQRepo-->>S: Slice<QuizQuestion>
Note over S: map to QuizQuestionTrainingDTO (isRated=false, isNewSession)
end
S-->>R: Slice<QuizQuestionTrainingDTO>
R-->>C: 200 OK\nBody: DTO[]\nHeader: X-Has-Next
sequenceDiagram
autonumber
participant C as Client (Web)
participant R as QuizTrainingResource
participant T as QuizTrainingService
participant QPS as QuizQuestionProgressService
Note over C,R: Submit training answer
C->>R: POST /api/quiz/courses/{courseId}/training-questions/{questionId}/submit?isRated=...
R->>T: submitForTraining(quizQuestionId, userId, courseId, submittedAnswer, isRated, answeredAt)
Note over T: evaluate answer, set score on SubmittedAnswer and link to question
alt isRated == true
T->>QPS: saveProgressFromTraining(question, userId, courseId, submittedAnswer, answeredAt)
end
T-->>R: SubmittedAnswerAfterEvaluationDTO
R-->>C: 200 OK + DTO
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/webapp/app/quiz/overview/service/course-training-quiz.service.spec.ts (1)
31-39: Align test with new wrapper type returned by the service.Service now returns training wrappers; this test still mocks plain
QuizQuestion[], risking type/behavior drift.Apply:
-import { QuizQuestion } from 'app/quiz/shared/entities/quiz-question.model'; +import { QuizQuestion } from 'app/quiz/shared/entities/quiz-question.model'; +import { QuizQuestionTraining } from '../course-training-quiz/quiz-question-training.model'; @@ - it('should fetch an Array of quiz questions', () => { - const mockQuestions: QuizQuestion[] = [{ id: 1 } as QuizQuestion]; + it('should fetch an Array of training questions', () => { + const mockQuestions: QuizQuestionTraining[] = [ + { quizQuestionWithSolutionDTO: { id: 1 } as QuizQuestion, isRated: true }, + ]; service.getQuizQuestions(courseId).subscribe((questions) => { expect(questions).toEqual(mockQuestions); }); const req = httpMock.expectOne(`api/quiz/courses/${courseId}/training-questions`); expect(req.request.method).toBe('GET'); req.flush(mockQuestions); });src/test/java/de/tum/cit/aet/artemis/quiz/QuizQuestionProgressIntegrationTest.java (1)
322-327: Update endpoint test to new DTO shapeThis still expects domain QuizQuestion from /training-questions, but the resource now returns QuizQuestionTrainingDTO. This will deserialize incorrectly.
- List<QuizQuestion> quizQuestions = request.getList("/api/quiz/courses/" + course.getId() + "/training-questions", OK, QuizQuestion.class); - - Assertions.assertThat(quizQuestions).isNotNull(); - Assertions.assertThat(quizQuestions).hasSameSizeAs(quizExercise.getQuizQuestions()); - Assertions.assertThat(quizQuestions).containsAll(quizExercise.getQuizQuestions()); + List<QuizQuestionTrainingDTO> dtos = request.getList("/api/quiz/courses/" + course.getId() + "/training-questions", OK, QuizQuestionTrainingDTO.class); + Assertions.assertThat(dtos).isNotNull(); + Assertions.assertThat(dtos).hasSameSizeAs(quizExercise.getQuizQuestions()); + // Compare IDs to avoid object equality mismatches + var dtoIds = dtos.stream().map(QuizQuestionTrainingDTO::getId).toList(); + var exerciseIds = quizExercise.getQuizQuestions().stream().map(QuizQuestion::getId).toList(); + Assertions.assertThat(dtoIds).containsExactlyInAnyOrderElementsOf(exerciseIds);
🧹 Nitpick comments (23)
src/main/webapp/i18n/en/quizQuestion.json (1)
36-37: Minor copy tweak for natural phrasing.Consider “in unrated mode” instead of “in the unrated mode.”
- "unratedConfirmation": "There are no questions due. Do you still want to train in the unrated mode?", + "unratedConfirmation": "There are no questions due. Do you still want to train in unrated mode?",src/main/webapp/app/quiz/overview/service/course-training-quiz.service.spec.ts (1)
41-54: Explicitly set and assertisRatedon submit to cover rated path.Without setting it, JSON will default to
false, skipping progress save server‑side.- const mockTrainingAnswer = new QuizTrainingAnswer(); + const mockTrainingAnswer = new QuizTrainingAnswer(); + mockTrainingAnswer.isRated = true; @@ - const req = httpMock.expectOne(`api/quiz/courses/${courseId}/training-questions/${questionId}/submit`); + const req = httpMock.expectOne(`api/quiz/courses/${courseId}/training-questions/${questionId}/submit`); expect(req.request.method).toBe('POST'); + expect(req.request.body.isRated).toBe(true);src/main/webapp/app/quiz/overview/course-training-quiz/quiz-training-answer.model.ts (1)
3-6: DefaultisRatedto true to avoid accidental unrated submissions.Rated is the common path; defaulting prevents silent non-persistence when the UI forgets to set it.
export class QuizTrainingAnswer { public submittedAnswer?: SubmittedAnswer; - public isRated?: boolean; + public isRated?: boolean = true;src/main/webapp/app/quiz/overview/course-training-quiz/quiz-question-training.model.ts (1)
3-6: Optional: prefer UI‑friendly naming.Consider
quizQuestioninstead ofquizQuestionWithSolutionDTOto keep frontend models DTO‑agnostic. Rename when convenient across usages.-export class QuizQuestionTraining { - public quizQuestionWithSolutionDTO?: QuizQuestion; +export class QuizQuestionTraining { + public quizQuestion?: QuizQuestion; public isRated?: boolean; }src/main/java/de/tum/cit/aet/artemis/quiz/dto/QuizTrainingAnswerDTO.java (1)
7-11: Decouple DTOs from entities.DTO currently embeds
de.tum...SubmittedAnswer(entity). Per guidelines, prefer a minimal submitted‑answer DTO to avoid exposing entity graphs and to control serialization.If you want, I can sketch a
SubmittedAnswerDTOand mapper.src/main/webapp/app/quiz/overview/course-training-quiz/course-training-quiz.component.spec.ts (2)
145-171: AssertisRatedpropagation on submit.Currently only success flow is asserted. Also verify the payload carries
isRatedfrom the wrapper for each question index.-const submitSpy = jest.spyOn(TestBed.inject(CourseTrainingQuizService), 'submitForTraining').mockReturnValue(of(new HttpResponse({ body: answer }))); +const submitSpy = jest.spyOn(TestBed.inject(CourseTrainingQuizService), 'submitForTraining') + .mockReturnValue(of(new HttpResponse({ body: answer }))); @@ component.onSubmit(); expect(submitSpy).toHaveBeenCalledOnce(); +expect(submitSpy.mock.calls[0][0].isRated).toBe(mockQuestions[0].isRated); @@ component.onSubmit(); expect(submitSpy).toHaveBeenCalledOnce(); +expect(submitSpy.mock.calls[0][0].isRated).toBe(mockQuestions[1].isRated); @@ component.onSubmit(); expect(submitSpy).toHaveBeenCalledOnce(); +expect(submitSpy.mock.calls[0][0].isRated).toBe(mockQuestions[2].isRated);
92-116: Edge-case test reads well; minor naming nit.Consider renaming “questions is empty” -> “questions are empty” for clarity.
src/main/webapp/app/quiz/overview/service/course-training-quiz.service.ts (2)
4-6: Align frontend model nullability with backend DTOBackend declares quizQuestionWithSolutionDTO @NotNull and isRated primitive. Make these required on the client to avoid optional chaining in callers and runtime surprises.
Apply in the model file:
-// src/main/webapp/app/quiz/overview/course-training-quiz/quiz-question-training.model.ts -export class QuizQuestionTraining { - public quizQuestionWithSolutionDTO?: QuizQuestion; - public isRated?: boolean; -} +export interface QuizQuestionTraining { + quizQuestionWithSolutionDTO: QuizQuestion; + isRated: boolean; +}
12-12: Mark injected HttpClient as readonlySmall immutability nit.
- private http = inject(HttpClient); + private readonly http = inject(HttpClient);src/main/java/de/tum/cit/aet/artemis/quiz/web/QuizTrainingResource.java (2)
62-67: Fix outdated Javadoc (“10 quiz questions”)Behavior now returns all due questions, otherwise a randomized set; not a fixed 10.
- /** - * Retrieves 10 quiz questions for the training session for the given course. The questions are selected based on the spaced repetition algorithm. + /** + * Retrieves quiz questions for the training session for the given course. + * If there are due questions, returns all due questions sorted by due date; otherwise returns a randomized set for free training.
97-101: Optional: inject Clock for time determinismUsing ZonedDateTime.now() hampers deterministic tests.
- ZonedDateTime answeredAt = ZonedDateTime.now(); + ZonedDateTime answeredAt = clock.instant().atZone(ZoneOffset.UTC);Wire a Clock bean and constructor‑inject it.
src/main/java/de/tum/cit/aet/artemis/quiz/dto/question/QuizQuestionTrainingDTO.java (1)
7-17: Enforce non-null in factory; tiny guardRecord already annotates @NotNull; enforce at construction.
+import java.util.Objects; @@ public static QuizQuestionTrainingDTO of(QuizQuestionWithSolutionDTO quizQuestionWithSolutionDTO, boolean isRated) { - return new QuizQuestionTrainingDTO(quizQuestionWithSolutionDTO, isRated); + return new QuizQuestionTrainingDTO(Objects.requireNonNull(quizQuestionWithSolutionDTO, "quizQuestionWithSolutionDTO"), isRated); }src/test/java/de/tum/cit/aet/artemis/quiz/QuizQuestionProgressIntegrationTest.java (2)
149-154: Assert DTO payload completeness as wellIn addition to size and isRated, assert the wrapped question is present to catch mapping regressions.
for (QuizQuestionTrainingDTO dto : result) { assertThat(dto.isRated()).isTrue(); + assertThat(dto.quizQuestionWithSolutionDTO()).isNotNull(); }
165-201: Good coverage for “no due date” branchAssertions correctly validate the unrated path. Consider also asserting quizQuestionWithSolutionDTO non-null.
src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizQuestionProgressService.java (2)
92-99: Javadoc no longer matches behaviorThe method returns due‑sorted questions or randomized when none are due; also wraps results and marks isRated accordingly.
- /** - * Get the sorted List of quiz questions based on their due date + /** + * Get quiz questions for a training session: + * - If there are due questions, return them sorted by due date (rated). + * - Otherwise, return a randomized set (unrated/free training). @@ - * @return A list of quiz questions sorted by due date + * @return A list of training DTOs indicating rated/unrated
110-114: Avoid O(n²): precompute due IDs for isRated checkcontains(q) on a List inside the mapping is O(n) per element. Use a Set of IDs.
- List<QuizQuestion> dueQuestions = allQuestions.stream().filter(q -> { + List<QuizQuestion> dueQuestions = allQuestions.stream().filter(q -> { ZonedDateTime dueDate = dueDateMap.getOrDefault(q.getId(), now); return !dueDate.toLocalDate().isAfter(now.toLocalDate()); }).sorted(Comparator.comparing(q -> dueDateMap.getOrDefault(q.getId(), now))).toList(); - List<QuizQuestion> questionsForSession; - boolean hasDueQuestions = !dueQuestions.isEmpty(); + List<QuizQuestion> questionsForSession; + boolean hasDueQuestions = !dueQuestions.isEmpty(); + var dueIds = hasDueQuestions ? dueQuestions.stream().map(QuizQuestion::getId).collect(Collectors.toSet()) : Set.<Long>of(); @@ if (hasDueQuestions) { questionsForSession = dueQuestions; } else { questionsForSession = allQuestions.stream().collect(Collectors.collectingAndThen(Collectors.toCollection(ArrayList::new), list -> { Collections.shuffle(list); return list; })); } - return questionsForSession.stream().map(q -> { + return questionsForSession.stream().map(q -> { QuizQuestionWithSolutionDTO dto = QuizQuestionWithSolutionDTO.of(q); - boolean isRated = hasDueQuestions && dueQuestions.contains(q); + boolean isRated = hasDueQuestions && dueIds.contains(q.getId()); return QuizQuestionTrainingDTO.of(dto, isRated); }).toList();Also applies to: 118-133
src/main/webapp/app/quiz/overview/course-training-quiz/course-training-quiz.component.html (2)
2-20: Consider using Angular CDK for modal implementation.While the current modal implementation works, consider using Angular CDK's overlay/dialog for better accessibility, focus management, and consistent behavior across the application.
Benefits of Angular CDK:
- Automatic focus trap and restoration
- Keyboard navigation (ESC key handling)
- ARIA attributes management
- Backdrop click handling
- Animations
Example migration approach:
import { Dialog } from '@angular/cdk/dialog'; // In component: constructor(private dialog: Dialog) {} showUnratedDialog() { const dialogRef = this.dialog.open(UnratedConfirmationDialogComponent, { hasBackdrop: true, backdropClass: 'modal-backdrop', panelClass: 'modal-dialog-centered' }); dialogRef.closed.subscribe(result => { if (result) { this.confirmUnratedPractice(); } else { this.cancelUnratedPractice(); } }); }
55-64: Consider extracting navigation button logic to a computed signal.The button display logic could be simplified by using a computed signal in the component.
Add to component:
navigationButton = computed(() => { if (!this.submitted) { return { action: () => this.onSubmit(), title: 'artemisApp.exercise.submit' }; } if (this.isLastQuestion()) { return { action: () => this.navigateToTraining(), title: 'artemisApp.exercise.endTraining' }; } return { action: () => this.nextQuestion(), title: 'artemisApp.exercise.nextQuestion' }; });Simplify template:
- @if (!submitted) { - <jhi-button id="submit-question" class="ms-2" (onClick)="onSubmit()" [title]="'artemisApp.exercise.submit'" /> - } - @if (submitted) { - @if (isLastQuestion()) { - <jhi-button class="ms-2" (onClick)="navigateToTraining()" [title]="'artemisApp.exercise.endTraining'" /> - } @else { - <jhi-button class="ms-2" (onClick)="nextQuestion()" [title]="'artemisApp.exercise.nextQuestion'" /> - } - } + @if (navigationButton(); as button) { + <jhi-button class="ms-2" (onClick)="button.action()" [title]="button.title" /> + }src/main/webapp/app/quiz/overview/course-training-quiz/course-training-quiz.component.ts (5)
41-41: Consider usingreadonlyfor compile-time immutability.The
INITIAL_QUESTIONSconstant should use TypeScript'sreadonlymodifier for better type safety.- private static readonly INITIAL_QUESTIONS: QuizQuestionTraining[] = []; + private static readonly INITIAL_QUESTIONS: readonly QuizQuestionTraining[] = [];
78-79: InitializepreviousRatedStatusconsistently.The
previousRatedStatusis initialized totruebut should match the initial state logic to avoid potential edge cases.- previousRatedStatus: boolean | undefined = true; + previousRatedStatus: boolean | undefined = undefined;
108-118: Replace German comment with English.Line 109 contains a German comment that should be in English for consistency.
- // Überwache das Laden von Fragen + // Monitor question loading
133-141: Add null safety check for edge cases.The method should handle the case where
currentIsRatedisundefinedmore explicitly.checkRatingStatusChange(): void { const currentIsRated = this.isRated(); - if (this.previousRatedStatus === true && currentIsRated === false) { + // Only show confirmation when transitioning from rated to unrated + if (this.previousRatedStatus === true && currentIsRated === false) { this.showUnratedConfirmation = true; } - this.previousRatedStatus = currentIsRated; + // Update previous status only if current status is defined + if (currentIsRated !== undefined) { + this.previousRatedStatus = currentIsRated; + } }
278-285: Consider preventing multiple dialog interactions.Add protection against rapid clicks that could cause unexpected behavior.
confirmUnratedPractice(): void { + if (!this.showUnratedConfirmation) return; this.showUnratedConfirmation = false; } cancelUnratedPractice(): void { + if (!this.showUnratedConfirmation) return; this.showUnratedConfirmation = false; this.navigateToTraining(); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
src/main/java/de/tum/cit/aet/artemis/quiz/dto/QuizTrainingAnswerDTO.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/quiz/dto/question/QuizQuestionTrainingDTO.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizQuestionProgressService.java(4 hunks)src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizTrainingService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/quiz/web/QuizTrainingResource.java(2 hunks)src/main/webapp/app/quiz/overview/course-training-quiz/course-training-quiz.component.html(1 hunks)src/main/webapp/app/quiz/overview/course-training-quiz/course-training-quiz.component.spec.ts(4 hunks)src/main/webapp/app/quiz/overview/course-training-quiz/course-training-quiz.component.ts(9 hunks)src/main/webapp/app/quiz/overview/course-training-quiz/quiz-question-training.model.ts(1 hunks)src/main/webapp/app/quiz/overview/course-training-quiz/quiz-training-answer.model.ts(1 hunks)src/main/webapp/app/quiz/overview/service/course-training-quiz.service.spec.ts(1 hunks)src/main/webapp/app/quiz/overview/service/course-training-quiz.service.ts(2 hunks)src/main/webapp/i18n/de/quizQuestion.json(1 hunks)src/main/webapp/i18n/en/quizQuestion.json(1 hunks)src/test/java/de/tum/cit/aet/artemis/quiz/QuizQuestionProgressIntegrationTest.java(4 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
src/main/webapp/**/*.ts
⚙️ CodeRabbit configuration file
Files:
src/main/webapp/app/quiz/overview/course-training-quiz/quiz-training-answer.model.tssrc/main/webapp/app/quiz/overview/service/course-training-quiz.service.spec.tssrc/main/webapp/app/quiz/overview/course-training-quiz/quiz-question-training.model.tssrc/main/webapp/app/quiz/overview/course-training-quiz/course-training-quiz.component.spec.tssrc/main/webapp/app/quiz/overview/service/course-training-quiz.service.tssrc/main/webapp/app/quiz/overview/course-training-quiz/course-training-quiz.component.ts
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/dto/QuizTrainingAnswerDTO.javasrc/main/java/de/tum/cit/aet/artemis/quiz/service/QuizQuestionProgressService.javasrc/main/java/de/tum/cit/aet/artemis/quiz/service/QuizTrainingService.javasrc/main/java/de/tum/cit/aet/artemis/quiz/web/QuizTrainingResource.javasrc/main/java/de/tum/cit/aet/artemis/quiz/dto/question/QuizQuestionTrainingDTO.java
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/quizQuestion.json
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/quiz/overview/course-training-quiz/course-training-quiz.component.html
src/test/java/**/*.java
⚙️ CodeRabbit configuration file
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
Files:
src/test/java/de/tum/cit/aet/artemis/quiz/QuizQuestionProgressIntegrationTest.java
🧠 Learnings (14)
📓 Common learnings
Learnt from: MoritzSpengler
PR: ls1intum/Artemis#11309
File: src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizQuestionProgressService.java:0-0
Timestamp: 2025-08-27T10:46:50.237Z
Learning: In QuizQuestionProgressService.saveProgressFromTraining, the dueDate-based gating allows same-day retries for incorrectly answered questions (where dueDate doesn't advance) while preventing same-day retries for correctly answered questions (where dueDate advances). This implements a nuanced binge-learning constraint that only applies to correctly answered questions.
📚 Learning: 2024-07-07T13:57:07.670Z
Learnt from: JohannesStoehr
PR: ls1intum/Artemis#8976
File: src/main/webapp/app/entities/quiz/quiz-exercise.model.ts:0-0
Timestamp: 2024-07-07T13:57:07.670Z
Learning: When defining the `resetQuizForImport` function in `src/main/webapp/app/entities/quiz/quiz-exercise.model.ts`, ensure to directly refer to the `exercise` parameter instead of using `this`.
Applied to files:
src/main/webapp/app/quiz/overview/service/course-training-quiz.service.spec.tssrc/main/webapp/app/quiz/overview/course-training-quiz/quiz-question-training.model.tssrc/main/webapp/app/quiz/overview/course-training-quiz/course-training-quiz.component.spec.tssrc/main/webapp/app/quiz/overview/service/course-training-quiz.service.tssrc/main/webapp/app/quiz/overview/course-training-quiz/course-training-quiz.component.ts
📚 Learning: 2025-08-21T17:30:20.530Z
Learnt from: MoritzSpengler
PR: ls1intum/Artemis#11297
File: src/main/webapp/app/quiz/shared/questions/drag-and-drop-question/drag-and-drop-question.component.spec.ts:34-34
Timestamp: 2025-08-21T17:30:20.530Z
Learning: FitTextDirective in src/main/webapp/app/quiz/shared/fit-text/fit-text.directive.ts is a standalone directive marked with standalone: true, so it should be imported in TestBed imports array, not declarations array.
Applied to files:
src/main/webapp/app/quiz/overview/service/course-training-quiz.service.spec.ts
📚 Learning: 2025-07-11T15:43:56.099Z
Learnt from: MoritzSpengler
PR: ls1intum/Artemis#11055
File: src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizQuestionProgressService.java:47-61
Timestamp: 2025-07-11T15:43:56.099Z
Learning: In QuizQuestionProgressService.retrieveProgressFromResultAndSubmission method, input validation is not necessary because getSubmittedAnswers() always returns a non-null set and the method receives non-null objects by contract when called from QuizSubmissionService.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizQuestionProgressService.javasrc/main/java/de/tum/cit/aet/artemis/quiz/service/QuizTrainingService.javasrc/main/java/de/tum/cit/aet/artemis/quiz/web/QuizTrainingResource.javasrc/test/java/de/tum/cit/aet/artemis/quiz/QuizQuestionProgressIntegrationTest.java
📚 Learning: 2025-09-01T10:20:40.706Z
Learnt from: Michael-Breu-UIbk
PR: ls1intum/Artemis#10989
File: src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.with-sharing.spec.ts:132-149
Timestamp: 2025-09-01T10:20:40.706Z
Learning: In the Artemis codebase, Angular component test files for ProgrammingExerciseDetailComponent follow a pattern where the component is imported but not explicitly declared in TestBed.configureTestingModule(), yet TestBed.createComponent() still works successfully. This pattern is consistently used across test files like programming-exercise-detail.component.spec.ts and programming-exercise-detail.component.with-sharing.spec.ts.
Applied to files:
src/main/webapp/app/quiz/overview/course-training-quiz/course-training-quiz.component.spec.ts
📚 Learning: 2025-09-01T13:47:02.624Z
Learnt from: Michael-Breu-UIbk
PR: ls1intum/Artemis#10989
File: src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.with-sharing.spec.ts:167-169
Timestamp: 2025-09-01T13:47:02.624Z
Learning: In Jest tests, prefer using jest.spyOn() over direct method assignment for mocking service methods, as spies are automatically restored by Jest's cleanup mechanisms (jest.restoreAllMocks()), while direct assignment bypasses this system and can lead to test pollution where mocked methods affect subsequent tests.
Applied to files:
src/main/webapp/app/quiz/overview/course-training-quiz/course-training-quiz.component.spec.ts
📚 Learning: 2025-08-27T10:46:50.237Z
Learnt from: MoritzSpengler
PR: ls1intum/Artemis#11309
File: src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizQuestionProgressService.java:0-0
Timestamp: 2025-08-27T10:46:50.237Z
Learning: In QuizQuestionProgressService.saveProgressFromTraining, the dueDate-based gating allows same-day retries for incorrectly answered questions (where dueDate doesn't advance) while preventing same-day retries for correctly answered questions (where dueDate advances). This implements a nuanced binge-learning constraint that only applies to correctly answered questions.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizTrainingService.javasrc/test/java/de/tum/cit/aet/artemis/quiz/QuizQuestionProgressIntegrationTest.java
📚 Learning: 2024-06-10T19:44:09.116Z
Learnt from: julian-christl
PR: ls1intum/Artemis#8052
File: src/test/java/de/tum/in/www1/artemis/lecture/CompetencyIntegrationTest.java:310-310
Timestamp: 2024-06-10T19:44:09.116Z
Learning: Modifications to parameters in `competencyProgressUtilService.createCompetencyProgress` for debugging purposes are considered irrelevant to the test outcomes but helpful for clarity during debugging.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizTrainingService.java
📚 Learning: 2024-10-14T10:27:58.500Z
Learnt from: florian-glombik
PR: ls1intum/Artemis#8858
File: src/main/webapp/i18n/de/student-dashboard.json:36-37
Timestamp: 2024-10-14T10:27:58.500Z
Learning: Avoid suggesting the translation change for "noExercisesFoundWithAppliedFilter" in the PR ls1intum/Artemis#8858. The preferred translation is "Keine Aufgaben passen zu den gewählten Filtern."
Applied to files:
src/main/webapp/i18n/de/quizQuestion.json
📚 Learning: 2024-10-08T15:35:52.595Z
Learnt from: florian-glombik
PR: ls1intum/Artemis#8858
File: src/main/webapp/i18n/de/student-dashboard.json:0-0
Timestamp: 2024-10-08T15:35:52.595Z
Learning: Avoid suggesting the phrase "Keine Aufgaben passen zu den gewählten Filtern" for the translation key `noExercisesFoundWithAppliedFilter` in the PR ls1intum/Artemis#8858.
Applied to files:
src/main/webapp/i18n/de/quizQuestion.json
📚 Learning: 2024-10-08T15:35:52.595Z
Learnt from: florian-glombik
PR: ls1intum/Artemis#8597
File: src/main/webapp/app/exercises/programming/shared/build-details/programming-exercise-repository-and-build-plan-details.component.ts:52-59
Timestamp: 2024-10-08T15:35:52.595Z
Learning: When handling errors in Angular components, ensure to integrate with the alert service for consistent error display across the application.
Applied to files:
src/main/webapp/app/quiz/overview/course-training-quiz/course-training-quiz.component.ts
📚 Learning: 2024-10-04T15:18:01.419Z
Learnt from: pzdr7
PR: ls1intum/Artemis#9407
File: src/main/webapp/app/shared/monaco-editor/monaco-diff-editor.component.ts:48-52
Timestamp: 2024-10-04T15:18:01.419Z
Learning: In Angular components, effects created using the `effect()` function are automatically disposed when the component is destroyed, so manual cleanup in `ngOnDestroy` is not necessary.
Applied to files:
src/main/webapp/app/quiz/overview/course-training-quiz/course-training-quiz.component.ts
📚 Learning: 2025-01-18T08:49:30.693Z
Learnt from: SimonEntholzer
PR: ls1intum/Artemis#10147
File: src/main/webapp/app/shared/components/code-button/code-button.component.ts:1-1
Timestamp: 2025-01-18T08:49:30.693Z
Learning: In Angular 19+ projects, component inputs should use the new input() function from angular/core instead of the Input() decorator, as it's part of the new signals architecture that improves performance and type safety.
Applied to files:
src/main/webapp/app/quiz/overview/course-training-quiz/course-training-quiz.component.ts
📚 Learning: 2025-08-19T20:07:18.114Z
Learnt from: MoritzSpengler
PR: ls1intum/Artemis#11297
File: src/main/webapp/app/quiz/shared/questions/short-answer-question/short-answer-question.component.ts:76-78
Timestamp: 2025-08-19T20:07:18.114Z
Learning: In ShortAnswerQuestion components (src/main/webapp/app/quiz/shared/questions/short-answer-question/short-answer-question.component.ts), the text, hint, and explanation properties are required fields for short answer questions and do not need null safety checks when accessing them from shortAnswerQuestion().
Applied to files:
src/main/webapp/app/quiz/overview/course-training-quiz/course-training-quiz.component.ts
🧬 Code graph analysis (2)
src/main/webapp/app/quiz/overview/service/course-training-quiz.service.ts (1)
src/main/webapp/app/quiz/overview/course-training-quiz/quiz-question-training.model.ts (1)
QuizQuestionTraining(3-6)
src/main/webapp/app/quiz/overview/course-training-quiz/course-training-quiz.component.ts (1)
src/main/webapp/app/quiz/overview/course-training-quiz/quiz-question-training.model.ts (1)
QuizQuestionTraining(3-6)
⏰ 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: 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
- GitHub Check: server-tests
- GitHub Check: client-tests
🔇 Additional comments (10)
src/main/webapp/app/quiz/overview/service/course-training-quiz.service.spec.ts (1)
8-8: Import path update LGTM.src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizTrainingService.java (1)
43-55: Gating persistence byisRatedis correct; add/verify test coverage.Logic is sound and respects the binge‑learning constraints via
QuizQuestionProgressService. Please add a test asserting no save on unrated and save on rated.Suggested Mockito checks:
// unrated verify(quizQuestionProgressService, times(0)).saveProgressFromTraining(any(), anyLong(), any(), any()); // rated verify(quizQuestionProgressService, times(1)).saveProgressFromTraining(eq(quizQuestion), eq(userId), any(), eq(answeredAt));src/main/java/de/tum/cit/aet/artemis/quiz/dto/QuizTrainingAnswerDTO.java (1)
10-11: @NotNull onsubmittedAnswerand addingisRatedlook good.src/main/webapp/app/quiz/overview/course-training-quiz/course-training-quiz.component.spec.ts (1)
56-60: Wrapper test data LGTM. Add a case for all‑rated to cover persistence path.Add one scenario where every item has
isRated: trueand assertsubmitForTrainingreceivesisRated: true.src/main/webapp/i18n/de/quizQuestion.json (1)
36-37: Remove remaining references to quizQuestion.noQuestionDueGerman phrasing and tone are correct (Du‑Form). Automated search skipped files; verify the old key is unused by running:
rg -n -uu --hidden -S "quizQuestion\.noQuestionDue\b" || trueRemove any matches found.
src/main/webapp/app/quiz/overview/service/course-training-quiz.service.ts (1)
18-20: API type change looks good; path matches backendReturn type and URL align with QuizTrainingResource.get endpoint.
src/test/java/de/tum/cit/aet/artemis/quiz/QuizQuestionProgressIntegrationTest.java (1)
284-284: Rated submission path exercised — LGTMConstructor change to include isRated=true matches the new contract.
src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizQuestionProgressService.java (1)
103-107: Semantics: questions without progress treated as “due”Defaulting missing dueDate to now marks brand‑new questions as rated/due. If the intent is “free training when no questions are due for this user,” consider defaulting to a future timestamp or requiring existing progress for due classification.
Would you like this behavior? If not, we can change the default:
- return (data != null && data.getDueDate() != null) ? data.getDueDate() : ZonedDateTime.now(); + return (data != null && data.getDueDate() != null) ? data.getDueDate() : ZonedDateTime.now().plusDays(1);Or gate “due” by presence of progress: only entries found in progressList can be due.
src/main/webapp/app/quiz/overview/course-training-quiz/course-training-quiz.component.html (1)
23-54: LGTM! Clean implementation of the conditional question rendering.The refactored template properly handles all question types within a single
currentQuestion()block, which improves maintainability and reduces duplication.src/main/webapp/app/quiz/overview/course-training-quiz/course-training-quiz.component.ts (1)
218-218: LGTM! Properly sets isRated flag before submission.The implementation correctly propagates the rating status to the training answer.
src/main/java/de/tum/cit/aet/artemis/quiz/web/QuizTrainingResource.java
Outdated
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
|
||||||||||||||||||||||||||||||
MoritzSpengler
temporarily deployed
to
artemis-test1.artemis.cit.tum.de
GitHub Actions
Inactive
Yhmidi
reviewed
Sep 15, 2025
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||||||||||||||
KonstiAnon
previously approved these changes
Sep 15, 2025
src/main/java/de/tum/cit/aet/artemis/quiz/dto/question/QuizQuestionTrainingDTO.java
Outdated
Show resolved
Hide resolved
src/main/webapp/app/quiz/overview/course-training-quiz/quiz-question-training.model.ts
Show resolved
Hide resolved
src/main/webapp/app/quiz/overview/course-training-quiz/quiz-training-answer.model.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/test/java/de/tum/cit/aet/artemis/quiz/QuizQuestionProgressIntegrationTest.java (3)
88-101: Use test util to create Course (aligns with util_service_factory_pattern).Prefer
courseUtilService.createCourse()over constructing/saving a bareCourseto ensure required defaults and consistency across tests.Apply:
- Course course = new Course(); - courseTestRepository.save(course); + Course course = courseUtilService.createCourse();
163-203: Free-mode coverage looks good; assert actual content length.Keep the
isRated == falsecheck. Prefer assertinggetNumberOfElements()orgetContent().size()overgetSize().Apply:
- assertThat(result.getSize()).isEqualTo(10); + assertThat(result.getNumberOfElements()).isEqualTo(10);
287-290: Add a complementary unrated-submission test (regression guard).Add a test where
isRated=falseto verify progress is not advanced (e.g., dueDate and session counters unchanged), per the conditional persistence added in this PR.Example new test (add below current submit test):
@Test @WithMockUser(username = TEST_PREFIX + "student1", roles = "USER") void testSubmitForTraining_unrated_doesNotAdvanceProgress() throws Exception { Course course = courseUtilService.createCourse(); MultipleChoiceQuestion mcQuestion = new MultipleChoiceQuestion(); mcQuestion.setTitle("Unrated Question"); mcQuestion.setPoints(1.0); mcQuestion.setScoringType(ScoringType.ALL_OR_NOTHING); mcQuestion = quizQuestionRepository.save(mcQuestion); // Existing progress with a future due date QuizQuestionProgress progress = new QuizQuestionProgress(); QuizQuestionProgressData dataExisting = new QuizQuestionProgressData(); dataExisting.setEasinessFactor(2.5); dataExisting.setInterval(1); dataExisting.setSessionCount(1); dataExisting.setDueDate(ZonedDateTime.now().plusDays(1)); progress.setProgressJson(dataExisting); progress.setQuizQuestionId(mcQuestion.getId()); progress.setUserId(userId); progress.setCourseId(course.getId()); quizQuestionProgressRepository.save(progress); MultipleChoiceSubmittedAnswer submittedAnswer = new MultipleChoiceSubmittedAnswer(); submittedAnswer.setQuizQuestion(mcQuestion); submittedAnswer.setSelectedOptions(Set.of()); // Unrated submission SubmittedAnswerAfterEvaluationDTO response = request.postWithResponseBody( "/api/quiz/courses/" + course.getId() + "/training-questions/" + mcQuestion.getId() + "/submit?isRated=false", submittedAnswer, SubmittedAnswerAfterEvaluationDTO.class, HttpStatus.OK); assertThat(response).isNotNull(); QuizQuestionProgress after = quizQuestionProgressRepository.findByUserIdAndQuizQuestionId(userId, mcQuestion.getId()).orElseThrow(); QuizQuestionProgressData data = after.getProgressJson(); // Due date and session should not advance in unrated mode assertThat(data.getDueDate()).isEqualTo(dataExisting.getDueDate()); assertThat(data.getSessionCount()).isEqualTo(1); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/test/java/de/tum/cit/aet/artemis/quiz/QuizQuestionProgressIntegrationTest.java(9 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/test/java/**/*.java
⚙️ CodeRabbit configuration file
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
Files:
src/test/java/de/tum/cit/aet/artemis/quiz/QuizQuestionProgressIntegrationTest.java
🧠 Learnings (12)
📓 Common learnings
Learnt from: MoritzSpengler
PR: ls1intum/Artemis#11382
File: src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizQuestionProgressService.java:100-109
Timestamp: 2025-09-23T10:03:34.064Z
Learning: In QuizQuestionProgressService.getQuestionsForSession, the isNewSession parameter is a request-level processing flag that indicates whether this is the first request of a training session. When true, it triggers initial progress data filtering to determine which questions are not yet due, then gets set to false before creating DTOs because the subsequent processing is no longer "new session" initialization. The client manages session state and sends isNewSession = false on subsequent paginated requests.
Learnt from: MoritzSpengler
PR: ls1intum/Artemis#11309
File: src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizQuestionProgressService.java:0-0
Timestamp: 2025-08-27T10:46:50.237Z
Learning: In QuizQuestionProgressService.saveProgressFromTraining, the dueDate-based gating allows same-day retries for incorrectly answered questions (where dueDate doesn't advance) while preventing same-day retries for correctly answered questions (where dueDate advances). This implements a nuanced binge-learning constraint that only applies to correctly answered questions.
Learnt from: MoritzSpengler
PR: ls1intum/Artemis#11382
File: src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizTrainingService.java:43-54
Timestamp: 2025-09-20T16:47:54.380Z
Learning: In QuizTrainingService.submitForTraining, cross-course mismatch protection is handled through PreAuthorize("hasAccessToCourse(#courseId)") authorization at the REST layer, ensuring users can only submit for courses they have access to, rather than through explicit courseId validation in the service method.
📚 Learning: 2025-09-20T16:43:32.823Z
Learnt from: MoritzSpengler
PR: ls1intum/Artemis#11382
File: src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizQuestionProgressService.java:130-137
Timestamp: 2025-09-20T16:43:32.823Z
Learning: The findAllDueQuestions method exists in QuizQuestionRepository and accepts Set<Long> ids, Long courseId, and Pageable parameters, returning Page<QuizQuestion>. This method is properly implemented and available for use in QuizQuestionProgressService.
Applied to files:
src/test/java/de/tum/cit/aet/artemis/quiz/QuizQuestionProgressIntegrationTest.java
📚 Learning: 2025-09-20T16:12:57.735Z
Learnt from: MoritzSpengler
PR: ls1intum/Artemis#11382
File: src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizQuestionProgressService.java:0-0
Timestamp: 2025-09-20T16:12:57.735Z
Learning: In QuizQuestionProgressService.areQuestionsDue, MoritzSpengler confirmed that defensive null/empty guards are unnecessary because the method is only called from getQuestionsForSession, which already handles null/empty questionIds by adding a sentinel value (-1L) before the call.
Applied to files:
src/test/java/de/tum/cit/aet/artemis/quiz/QuizQuestionProgressIntegrationTest.java
📚 Learning: 2025-09-20T12:21:38.321Z
Learnt from: MoritzSpengler
PR: ls1intum/Artemis#11382
File: src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizQuestionProgressService.java:121-125
Timestamp: 2025-09-20T12:21:38.321Z
Learning: In QuizQuestionProgressService.getQuestionsForSession, the questionIds set contains questions that are NOT due yet (filtered by dueDate.isAfter(now)). The areQuestionsDue method correctly uses notDueCount < totalQuestionsCount to determine if there are questions due for practice.
Applied to files:
src/test/java/de/tum/cit/aet/artemis/quiz/QuizQuestionProgressIntegrationTest.java
📚 Learning: 2025-08-30T20:20:17.236Z
Learnt from: SamuelRoettgermann
PR: ls1intum/Artemis#11111
File: src/test/java/de/tum/cit/aet/artemis/exam/ExamRoomIntegrationTest.java:0-0
Timestamp: 2025-08-30T20:20:17.236Z
Learning: In ExamRoomIntegrationTest.validateAdminOverview(), the first assertion should use subset containment (contains) rather than exact equality because the admin overview shows all newest unique exam rooms in the system including those from previous uploads, not just rooms from the current upload being tested. The test only needs to verify that the expected rooms from the current upload are present in the response.
Applied to files:
src/test/java/de/tum/cit/aet/artemis/quiz/QuizQuestionProgressIntegrationTest.java
📚 Learning: 2025-07-11T15:43:56.099Z
Learnt from: MoritzSpengler
PR: ls1intum/Artemis#11055
File: src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizQuestionProgressService.java:47-61
Timestamp: 2025-07-11T15:43:56.099Z
Learning: In QuizQuestionProgressService.retrieveProgressFromResultAndSubmission method, input validation is not necessary because getSubmittedAnswers() always returns a non-null set and the method receives non-null objects by contract when called from QuizSubmissionService.
Applied to files:
src/test/java/de/tum/cit/aet/artemis/quiz/QuizQuestionProgressIntegrationTest.java
📚 Learning: 2025-09-20T16:45:43.715Z
Learnt from: MoritzSpengler
PR: ls1intum/Artemis#11382
File: src/main/java/de/tum/cit/aet/artemis/quiz/repository/QuizQuestionRepository.java:77-77
Timestamp: 2025-09-20T16:45:43.715Z
Learning: In QuizQuestionProgressService.areQuestionsDue method, the implementation calls quizQuestionRepository.countAllPracticeQuizQuestionsByCourseId(courseId) to get the total count of practice questions and compares it with notDueCount parameter to determine if questions are due for practice.
Applied to files:
src/test/java/de/tum/cit/aet/artemis/quiz/QuizQuestionProgressIntegrationTest.java
📚 Learning: 2024-10-08T15:35:42.972Z
Learnt from: jakubriegel
PR: ls1intum/Artemis#8050
File: src/test/java/de/tum/in/www1/artemis/plagiarism/PlagiarismUtilService.java:62-66
Timestamp: 2024-10-08T15:35:42.972Z
Learning: The `createCourseWithUsers` method in `PlagiarismUtilService.java` uses fixed inputs as it is designed to be a test helper method for simplifying the setup of courses and users in tests.
Applied to files:
src/test/java/de/tum/cit/aet/artemis/quiz/QuizQuestionProgressIntegrationTest.java
📚 Learning: 2025-08-27T10:46:50.237Z
Learnt from: MoritzSpengler
PR: ls1intum/Artemis#11309
File: src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizQuestionProgressService.java:0-0
Timestamp: 2025-08-27T10:46:50.237Z
Learning: In QuizQuestionProgressService.saveProgressFromTraining, the dueDate-based gating allows same-day retries for incorrectly answered questions (where dueDate doesn't advance) while preventing same-day retries for correctly answered questions (where dueDate advances). This implements a nuanced binge-learning constraint that only applies to correctly answered questions.
Applied to files:
src/test/java/de/tum/cit/aet/artemis/quiz/QuizQuestionProgressIntegrationTest.java
📚 Learning: 2025-09-20T16:43:32.823Z
Learnt from: MoritzSpengler
PR: ls1intum/Artemis#11382
File: src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizQuestionProgressService.java:130-137
Timestamp: 2025-09-20T16:43:32.823Z
Learning: The findAllDueQuestions method exists in QuizQuestionRepository and accepts Set<Long> ids, Long courseId, and Pageable parameters, returning Page<QuizQuestion>. The method has a Query annotation that filters by courseId, isOpenForPractice = TRUE, and excludes questions with IDs in the provided set using NOT IN.
Applied to files:
src/test/java/de/tum/cit/aet/artemis/quiz/QuizQuestionProgressIntegrationTest.java
📚 Learning: 2025-09-20T16:47:54.380Z
Learnt from: MoritzSpengler
PR: ls1intum/Artemis#11382
File: src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizTrainingService.java:43-54
Timestamp: 2025-09-20T16:47:54.380Z
Learning: In QuizTrainingService.submitForTraining, cross-course mismatch protection is handled through PreAuthorize("hasAccessToCourse(#courseId)") authorization at the REST layer, ensuring users can only submit for courses they have access to, rather than through explicit courseId validation in the service method.
Applied to files:
src/test/java/de/tum/cit/aet/artemis/quiz/QuizQuestionProgressIntegrationTest.java
📚 Learning: 2025-09-20T16:47:54.380Z
Learnt from: MoritzSpengler
PR: ls1intum/Artemis#11382
File: src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizTrainingService.java:43-54
Timestamp: 2025-09-20T16:47:54.380Z
Learning: In QuizTrainingService.submitForTraining, cross-course validation is handled by the REST layer through authCheckService.checkHasAtLeastRoleInCourseElseThrow() which validates user access to the course before the service method is called, eliminating the need for additional courseId validation in the service layer.
Applied to files:
src/test/java/de/tum/cit/aet/artemis/quiz/QuizQuestionProgressIntegrationTest.java
🧬 Code graph analysis (1)
src/test/java/de/tum/cit/aet/artemis/quiz/QuizQuestionProgressIntegrationTest.java (1)
src/main/webapp/app/quiz/overview/service/course-training-quiz.service.ts (1)
getQuizQuestions(21-27)
⏰ 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). (10)
- GitHub Check: Codacy Static Code Analysis
- 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: server-style
- GitHub Check: client-style
- GitHub Check: client-tests
- GitHub Check: server-tests
- GitHub Check: Analyse
- GitHub Check: bean-instantiation-check
🔇 Additional comments (2)
src/test/java/de/tum/cit/aet/artemis/quiz/QuizQuestionProgressIntegrationTest.java (2)
325-332: Assert rated mode for practice endpoint and add 1-question edge-case test.This test does not validate
isRated. With an empty body andisNewSession=trueand no existing progress, the first page should be rated mode. Also add a focused single-question edge case to catch sentinel/decision regressions.Apply:
- Assertions.assertThat(quizQuestions).isNotNull(); - Assertions.assertThat(quizQuestions).hasSameSizeAs(quizExercise.getQuizQuestions()); - Assertions.assertThat(quizQuestions.stream().map(QuizQuestionTrainingDTO::getId).toList()) - .containsAll(quizExercise.getQuizQuestions().stream().map(QuizQuestion::getId).toList()); + Assertions.assertThat(quizQuestions).isNotNull(); + Assertions.assertThat(quizQuestions).hasSameSizeAs(quizExercise.getQuizQuestions()); + Assertions.assertThat(quizQuestions.stream().map(QuizQuestionTrainingDTO::getId).toList()) + .containsAll(quizExercise.getQuizQuestions().stream().map(QuizQuestion::getId).toList()); + // With no existing progress, first page should be rated mode + Assertions.assertThat(quizQuestions.stream().allMatch(QuizQuestionTrainingDTO::isRated)).isTrue();Additionally, consider adding a new test creating a course with exactly 1 practice question and empty questionIds, asserting the single returned DTO has
isRated == true. This guards the empty-set/sentinel path.
153-161: Strengthen assertions; confirm Java target before replacing List.getFirst().
- I couldn't find maven compiler/source/target settings — repo uses getFirst() in many places (rg found many occurrences). Confirm whether the project targets Java 21; if not, replace questions.getFirst() with questions.get(0).
- For src/test/java/de/tum/cit/aet/artemis/quiz/QuizQuestionProgressIntegrationTest.java (lines ~153–161): assert actual element count (result.getNumberOfElements() or result.getContent().size()) instead of getSize(), assert the excludedId is not present in result.getContent(), and assert all returned DTOs are rated.
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
tobias-lippert
previously approved these changes
Sep 23, 2025
src/main/java/de/tum/cit/aet/artemis/quiz/repository/QuizQuestionRepository.java
Outdated
Show resolved
Hide resolved
helios-aet
bot
temporarily deployed
to
artemis-test1.artemis.cit.tum.de
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||||||||||||||
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
Yhmidi
previously approved these changes
Sep 23, 2025
ekayandan
temporarily deployed
to
artemis-test1.artemis.cit.tum.de
GitHub Actions
Inactive
ekayandan
previously approved these changes
Sep 24, 2025
KonstiAnon
previously approved these changes
Sep 24, 2025
MoritzSpengler
dismissed stale reviews from KonstiAnon, ekayandan, HawKhiem, Yhmidi, tobias-lippert, and coderabbitai[bot]
via
0eef4c4
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
src/main/java/de/tum/cit/aet/artemis/quiz/repository/QuizQuestionRepository.java (1)
45-56: Align the Javadoc with the new Slice return type.The documentation still promises a “set” even though the method now returns a
Slice. Please update the wording to avoid misleading callers.- * @return a set of quiz questions + * @return a slice of quiz questionssrc/main/webapp/app/quiz/overview/course-training-quiz/course-training-quiz.component.ts (5)
202-206: Avoid double rating-status checkscheckRatingStatusChange() is already driven by the isRated() effect; calling it again in initQuestion() can re-open the dialog unexpectedly.
initQuestion(question: QuizQuestion): void { this.showingResult = false; this.submitted = false; - this.checkRatingStatusChange(); if (question) {Also applies to: 118-125
261-269: Localize the warning; use a translation key instead of plain textFollow the project’s i18n guideline; don’t rely on translating a literal sentence.
- this.alertService.addAlert({ - type: AlertType.WARNING, - message: 'No questionId found', - }); + this.alertService.addAlert({ + type: AlertType.WARNING, + message: 'artemisApp.quiz.noQuestionIdFound', + });
55-61: Ensure courseId is numericActivatedRoute params are strings; casting with
as numberdoesn’t coerce at runtime. Consider normalizing:Example:
courseId = computed(() => { const id = this.paramsSignal()?.['courseId']; return id !== undefined ? Number(id) : undefined; });And in courseSignal:
map((p) => Number(p['courseId'])).
49-49: Remove or populate totalItemstotalItems is never set/used. Either remove it or populate it from a header (e.g., X-Total-Count) if provided.
Is the backend exposing a total-count header for this endpoint? If not, we can drop totalItems here.
188-196: Confirm initial dialog behavior on first unrated questionpreviousRatedStatus defaults to true; if the first question is unrated, the confirmation opens immediately. If you only want the prompt on transitions within a session, initialize previousRatedStatus lazily (first value only sets baseline).
Option:
// Change type: previousRatedStatus: boolean | undefined = undefined; // In checkRatingStatusChange(): if (this.previousRatedStatus === undefined) { this.previousRatedStatus = currentIsRated; return; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/de/tum/cit/aet/artemis/quiz/repository/QuizQuestionRepository.java(3 hunks)src/main/webapp/app/quiz/overview/course-training-quiz/course-training-quiz.component.ts(7 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
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/QuizQuestionRepository.java
src/main/webapp/**/*.ts
⚙️ CodeRabbit configuration file
Files:
src/main/webapp/app/quiz/overview/course-training-quiz/course-training-quiz.component.ts
🧠 Learnings (15)
📓 Common learnings
Learnt from: MoritzSpengler
PR: ls1intum/Artemis#11382
File: src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizQuestionProgressService.java:100-109
Timestamp: 2025-09-23T10:03:34.091Z
Learning: In QuizQuestionProgressService.getQuestionsForSession, the isNewSession parameter is a request-level processing flag that indicates whether this is the first request of a training session. When true, it triggers initial progress data filtering to determine which questions are not yet due, then gets set to false before creating DTOs because the subsequent processing is no longer "new session" initialization. The client manages session state and sends isNewSession = false on subsequent paginated requests.
Learnt from: MoritzSpengler
PR: ls1intum/Artemis#11382
File: src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizTrainingService.java:43-54
Timestamp: 2025-09-20T16:47:54.380Z
Learning: In QuizTrainingService.submitForTraining, cross-course mismatch protection is handled through PreAuthorize("hasAccessToCourse(#courseId)") authorization at the REST layer, ensuring users can only submit for courses they have access to, rather than through explicit courseId validation in the service method.
Learnt from: MoritzSpengler
PR: ls1intum/Artemis#11309
File: src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizQuestionProgressService.java:0-0
Timestamp: 2025-08-27T10:46:50.237Z
Learning: In QuizQuestionProgressService.saveProgressFromTraining, the dueDate-based gating allows same-day retries for incorrectly answered questions (where dueDate doesn't advance) while preventing same-day retries for correctly answered questions (where dueDate advances). This implements a nuanced binge-learning constraint that only applies to correctly answered questions.
📚 Learning: 2025-09-20T16:43:32.823Z
Learnt from: MoritzSpengler
PR: ls1intum/Artemis#11382
File: src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizQuestionProgressService.java:130-137
Timestamp: 2025-09-20T16:43:32.823Z
Learning: The findAllDueQuestions method exists in QuizQuestionRepository and accepts Set<Long> ids, Long courseId, and Pageable parameters, returning Page<QuizQuestion>. The method has a Query annotation that filters by courseId, isOpenForPractice = TRUE, and excludes questions with IDs in the provided set using NOT IN.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/quiz/repository/QuizQuestionRepository.java
📚 Learning: 2025-09-20T16:43:32.823Z
Learnt from: MoritzSpengler
PR: ls1intum/Artemis#11382
File: src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizQuestionProgressService.java:130-137
Timestamp: 2025-09-20T16:43:32.823Z
Learning: The findAllDueQuestions method exists in QuizQuestionRepository and accepts Set<Long> ids, Long courseId, and Pageable parameters, returning Page<QuizQuestion>. This method is properly implemented and available for use in QuizQuestionProgressService.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/quiz/repository/QuizQuestionRepository.java
📚 Learning: 2025-09-20T16:45:43.715Z
Learnt from: MoritzSpengler
PR: ls1intum/Artemis#11382
File: src/main/java/de/tum/cit/aet/artemis/quiz/repository/QuizQuestionRepository.java:77-77
Timestamp: 2025-09-20T16:45:43.715Z
Learning: In QuizQuestionProgressService.areQuestionsDue method, the implementation calls quizQuestionRepository.countAllPracticeQuizQuestionsByCourseId(courseId) to get the total count of practice questions and compares it with notDueCount parameter to determine if questions are due for practice.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/quiz/repository/QuizQuestionRepository.java
📚 Learning: 2025-09-20T12:21:38.321Z
Learnt from: MoritzSpengler
PR: ls1intum/Artemis#11382
File: src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizQuestionProgressService.java:121-125
Timestamp: 2025-09-20T12:21:38.321Z
Learning: In QuizQuestionProgressService.getQuestionsForSession, the questionIds set contains questions that are NOT due yet (filtered by dueDate.isAfter(now)). The areQuestionsDue method correctly uses notDueCount < totalQuestionsCount to determine if there are questions due for practice.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/quiz/repository/QuizQuestionRepository.java
📚 Learning: 2025-09-20T16:12:57.735Z
Learnt from: MoritzSpengler
PR: ls1intum/Artemis#11382
File: src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizQuestionProgressService.java:0-0
Timestamp: 2025-09-20T16:12:57.735Z
Learning: In QuizQuestionProgressService.areQuestionsDue, MoritzSpengler confirmed that defensive null/empty guards are unnecessary because the method is only called from getQuestionsForSession, which already handles null/empty questionIds by adding a sentinel value (-1L) before the call.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/quiz/repository/QuizQuestionRepository.java
📚 Learning: 2024-08-01T09:26:26.610Z
Learnt from: rabeatwork
PR: ls1intum/Artemis#9112
File: src/main/webapp/app/exam/participate/exam-bar/exam-bar.component.ts:45-50
Timestamp: 2024-08-01T09:26:26.610Z
Learning: In the context of PR ls1intum/Artemis#9112, avoid using non-null assertions and instead use nullish coalescing operators to provide default values.
Applied to files:
src/main/webapp/app/quiz/overview/course-training-quiz/course-training-quiz.component.ts
📚 Learning: 2024-07-09T19:06:18.665Z
Learnt from: florian-glombik
PR: ls1intum/Artemis#8858
File: src/main/webapp/app/shared/exercise-filter/exercise-filter-modal.component.ts:97-111
Timestamp: 2024-07-09T19:06:18.665Z
Learning: In the context of PR ls1intum/Artemis#8858, non-null assertions should not be flagged or suggested for removal.
Applied to files:
src/main/webapp/app/quiz/overview/course-training-quiz/course-training-quiz.component.ts
📚 Learning: 2024-10-08T15:35:52.595Z
Learnt from: florian-glombik
PR: ls1intum/Artemis#8858
File: src/main/webapp/app/shared/sidebar/sidebar.component.ts:0-0
Timestamp: 2024-10-08T15:35:52.595Z
Learning: In the context of PR ls1intum/Artemis#8858, avoid suggesting the removal of non-null assertions.
Applied to files:
src/main/webapp/app/quiz/overview/course-training-quiz/course-training-quiz.component.ts
📚 Learning: 2024-10-08T15:35:52.595Z
Learnt from: florian-glombik
PR: ls1intum/Artemis#8597
File: src/main/webapp/app/exercises/programming/shared/build-details/programming-exercise-repository-and-build-plan-details.component.ts:52-59
Timestamp: 2024-10-08T15:35:52.595Z
Learning: When handling errors in Angular components, ensure to integrate with the alert service for consistent error display across the application.
Applied to files:
src/main/webapp/app/quiz/overview/course-training-quiz/course-training-quiz.component.ts
📚 Learning: 2024-10-04T15:18:01.419Z
Learnt from: pzdr7
PR: ls1intum/Artemis#9407
File: src/main/webapp/app/shared/monaco-editor/monaco-diff-editor.component.ts:48-52
Timestamp: 2024-10-04T15:18:01.419Z
Learning: In Angular components, effects created using the `effect()` function are automatically disposed when the component is destroyed, so manual cleanup in `ngOnDestroy` is not necessary.
Applied to files:
src/main/webapp/app/quiz/overview/course-training-quiz/course-training-quiz.component.ts
📚 Learning: 2025-01-18T08:49:30.693Z
Learnt from: SimonEntholzer
PR: ls1intum/Artemis#10147
File: src/main/webapp/app/shared/components/code-button/code-button.component.ts:1-1
Timestamp: 2025-01-18T08:49:30.693Z
Learning: In Angular 19+ projects, component inputs should use the new input() function from angular/core instead of the Input() decorator, as it's part of the new signals architecture that improves performance and type safety.
Applied to files:
src/main/webapp/app/quiz/overview/course-training-quiz/course-training-quiz.component.ts
📚 Learning: 2025-08-19T20:07:18.114Z
Learnt from: MoritzSpengler
PR: ls1intum/Artemis#11297
File: src/main/webapp/app/quiz/shared/questions/short-answer-question/short-answer-question.component.ts:76-78
Timestamp: 2025-08-19T20:07:18.114Z
Learning: In ShortAnswerQuestion components (src/main/webapp/app/quiz/shared/questions/short-answer-question/short-answer-question.component.ts), the text, hint, and explanation properties are required fields for short answer questions and do not need null safety checks when accessing them from shortAnswerQuestion().
Applied to files:
src/main/webapp/app/quiz/overview/course-training-quiz/course-training-quiz.component.ts
📚 Learning: 2024-10-08T15:35:42.972Z
Learnt from: JohannesStoehr
PR: ls1intum/Artemis#8976
File: src/main/webapp/app/entities/quiz/quiz-exercise.model.ts:0-0
Timestamp: 2024-10-08T15:35:42.972Z
Learning: When defining the `resetQuizForImport` function in `src/main/webapp/app/entities/quiz/quiz-exercise.model.ts`, ensure to directly refer to the `exercise` parameter instead of using `this`.
Applied to files:
src/main/webapp/app/quiz/overview/course-training-quiz/course-training-quiz.component.ts
📚 Learning: 2025-08-19T19:56:07.618Z
Learnt from: MoritzSpengler
PR: ls1intum/Artemis#11297
File: src/main/webapp/app/quiz/shared/questions/multiple-choice-question/visual-question/multiple-choice-visual-question.component.ts:84-86
Timestamp: 2025-08-19T19:56:07.618Z
Learning: In Angular signals migration for MultipleChoiceVisualQuestionComponent, when adding new answer options, it's preferred to create a copy of the answerOptions array rather than directly mutating it, maintaining immutability principles while working with signal-returned objects.
Applied to files:
src/main/webapp/app/quiz/overview/course-training-quiz/course-training-quiz.component.ts
🧬 Code graph analysis (1)
src/main/webapp/app/quiz/overview/course-training-quiz/course-training-quiz.component.ts (7)
src/main/webapp/app/core/course/overview/course-training/course-training.component.ts (1)
Component(7-22)src/main/webapp/app/quiz/overview/course-training-quiz/quiz-question-training.model.ts (1)
QuizQuestionTraining(34-39)src/main/webapp/app/quiz/shared/entities/answer-option.model.ts (1)
AnswerOption(6-19)src/main/webapp/app/quiz/shared/entities/drag-and-drop-mapping.model.ts (1)
DragAndDropMapping(7-21)src/main/webapp/app/quiz/shared/entities/short-answer-submitted-text.model.ts (1)
ShortAnswerSubmittedText(5-11)src/main/webapp/app/quiz/shared/entities/drag-and-drop-submitted-answer.model.ts (1)
DragAndDropSubmittedAnswer(5-11)src/main/webapp/app/quiz/shared/entities/short-answer-submitted-answer.model.ts (1)
ShortAnswerSubmittedAnswer(5-11)
⏰ 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). (10)
- GitHub Check: Codacy Static Code Analysis
- 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: server-style
- GitHub Check: client-style
- GitHub Check: client-tests
- GitHub Check: server-tests
- GitHub Check: bean-instantiation-check
- GitHub Check: Analyse
🔇 Additional comments (2)
src/main/webapp/app/quiz/overview/course-training-quiz/course-training-quiz.component.ts (2)
46-49: Page size likely too small; consider 50–100 or making it configurablePer prior discussion, 20 yields extra roundtrips; 50–100 is safe and reduces DB/HTTP overhead.
Would you prefer bumping
sizeto 50 or reading a default from configuration?
271-271: LGTM: submitForTraining includes isRated flagPassing isRated resolves the rated vs. free training distinction end-to-end.
src/main/webapp/app/quiz/overview/course-training-quiz/course-training-quiz.component.ts
Show resolved
Hide resolved
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||||||||
ekayandan
approved these changes
Sep 25, 2025
Yhmidi
approved these changes
Sep 25, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Only Deploy to TS1
Checklist
General
Server
Client
authoritiesto all new routes and checked the course groups for displaying navigation elements (links, buttons).Motivation and Context
Currently, when students use training mode, they can only answer questions that are due according to the spaced repetition algorithm. We want to introduce a free training mode for highly motivated students, allowing them to continue practicing after completing all due questions. In this mode, they can work on any available questions without affecting their spaced repetition progress. Additionally, to handle situations where there are hundreds of questions, we have improved performance by introducing paging.
Description
We added a flag to determine whether a training session is rated or unrated, making it possible to distinguish between Rated and Free Training Mode. Quiz questions for each training session are now fetched using paging, and we refactored the service logic that selects questions for training. In addition, we introduced a new column in the quiz_question_progress table, which enables course-specific filtering when loading progress objects for a training session from the database.
Steps for Testing
Only Deploy to TS1
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
Improvements
Localization
Tests