Quiz exercises: Fix an issue with the evaluation when practice mode submissions are available
#9821
Conversation
github-actions
bot
added
server
WalkthroughThe changes in the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizResultService.java (3)
111-111: Remove unused variablequizDeadline.The variable
quizDeadlineis declared but not used in the code. Consider removing it to clean up the code.Apply this diff to remove the unused variable:
- ZonedDateTime quizDeadline = quizExercise.getDueDate();
158-184: Refactor entity save operations to simplify code and improve maintainability.Detaching and re-associating the submission with the result leads to multiple save operations and added complexity. Consider refactoring the save operations to leverage cascading persist operations or properly configured entity relationships to handle save order without manual detachment and reattachment.
Here's a possible refactor:
- // Detach submission to maintain proper save order - result.setSubmission(null); - // Save entities individually - submissionRepository.save(quizSubmission); - result = resultRepository.save(result); - // Update participation with new result - participation.addResult(result); - studentParticipationRepository.save(participation); - // Re-associate result with submission and save - result.setSubmission(quizSubmission); - quizSubmission.addResult(result); - submissionRepository.save(quizSubmission); + // Associate submission with result and add result to participation + result.setSubmission(quizSubmission); + participation.addResult(result); + // Save participation with cascading saves + studentParticipationRepository.save(participation); + // The result and submission will be saved through cascading + // Add result to the set of created results createdResults.add(result);By configuring the entity relationships to cascade persist operations appropriately, you can simplify the save operations and reduce the number of explicit saves, improving code readability and maintainability.
Line range hint
92-184: Consider breaking down theevaluateSubmissionsmethod for better readability.The
evaluateSubmissionsmethod is lengthy and contains nested logic, which can make it harder to understand and maintain. Refactoring it into smaller, well-named methods (e.g.,filterValidSubmissions,processParticipation,createOrUpdateResult) can improve code clarity and adhere to the single responsibility principle.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizResultService.java(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizResultService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
🔇 Additional comments (5)
src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizResultService.java (5)
9-9: Import statement for Optional is appropriate.
The import of java.util.Optional is necessary for the use of Optional instances in the code.
92-93: Javadoc updated to reflect new logic.
The added comments accurately document the new step of filtering out submissions not submitted before the quiz deadline, enhancing code readability and maintainability.
96-97: Clarification in Javadoc about existing rated results.
Including the logic about skipping evaluation if a rated result already exists helps to clearly communicate the method's functionality.
130-142: Filtering submissions based on quiz deadline is implemented correctly.
The code correctly filters out submissions made after the quiz deadline and selects the submission with the highest ID among the valid ones. This ensures that only eligible submissions are evaluated, aligning with the PR objectives.
150-155: Properly skip evaluation when a rated result already exists.
The logic to check for an existing rated result and skip evaluation if present prevents duplicate results and unnecessary processing.
krusche
dismissed
iyannsch’s stale review
the comment looks like an approval
…r-practice-submissions
krusche
changed the title
Quiz exercises: Fix evaluation logic after practice mode submissions were performedQuiz exercises: Fix an issue with the evaluation when practice mode submissions are available
krusche
deleted the
bugfix/quiz-exercises/fix-evaluation-after-practice-submissions
branch
Checklist
General
Server
Motivation and Context
In rare cases, the scheduled evaluation of a quiz may not automatically occur after the deadline has passed. If the course organizer subsequently opens the quiz for practice without first evaluating it, and users submit answers in practice mode, the evaluation process will no longer function correctly. This happens because the system's logic prioritizes the most recent submissions for each participant, which in this case are the practice submissions. As a result, the unevaluated original submissions are overlooked and skipped during the evaluation process.
Description
I updated the evaluation function to address this issue. For quizzes with a deadline, the function now selects only the submissions made before the deadline. For exam quizzes without a deadline, it retains the existing logic of selecting the submission with the highest ID. With this updated approach, even if submissions are made after the deadline, only the relevant ones are evaluated, ensuring accurate results.
Steps for Testing
Prerequisites:
Testserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.
Review Progress
Performance Review
Code Review
Manual Tests
Exam Mode Test
Summary by CodeRabbit
New Features
Bug Fixes
Chores