Development: Remove deprecated Participation#getResults()
#10983
Conversation
github-actions
bot
added
tests
server
| @@ -834,7 +834,6 @@ public void deleteResultsAndSubmissionsOfParticipation(Long participationId, boo | |||
| Set<Submission> submissions = participation.getSubmissions(); | |||
| // Delete all results for this participation | |||
| Set<Result> resultsToBeDeleted = submissions.stream().flatMap(submission -> submission.getResults().stream()).collect(Collectors.toSet()); | |||
| resultsToBeDeleted.addAll(participation.getResults()); | |||
There was a problem hiding this comment.
already initialized resultsToBeDeleted with the results of all submissions
jfr2102
changed the title
jfr2102
changed the title
Development: Remove deprecated Participation->getResults()
jfr2102
changed the title
Development: Remove deprecated Participation->getResults()Development: Remove deprecated Participation#getResults()
| participation.setSubmissions(null); | ||
| participation.setParticipant(null); | ||
| if (participation.getResults() != null && !participation.getResults().isEmpty()) { |
There was a problem hiding this comment.
this whole block doesnt make sense if we set submissions to null before
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||
|
Warning Rate limit exceeded@jfr2102 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 14 minutes and 30 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis change removes the deprecated Changes
Sequence Diagram(s)sequenceDiagram
participant Controller/Service
participant Participation
participant Submission
participant Result
Controller/Service->>Participation: getSubmissions()
loop for each Submission
Participation->>Submission: getResults()
loop for each Result
Submission->>Result: (collect if not null)
end
end
Controller/Service-->>Controller/Service: Aggregate all Results from all Submissions
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
🧪 Generate Unit Tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
src/main/java/de/tum/cit/aet/artemis/assessment/web/ResultResource.java (1)
278-279: Correct implementation but consider performance implications.The streaming approach to aggregate results from all submissions is correct and aligns with the removal of the deprecated
getResults()method. The logic properly checks for any existing results by flattening submissions and their result collections.However, this approach may be less performant than direct access to a participation's results collection, especially for participations with many submissions. Consider if this endpoint is performance-critical and whether the result checking could be optimized if needed.
src/test/java/de/tum/cit/aet/artemis/exercise/participation/util/ParticipationUtilService.java (1)
983-987: Missing@paramin Javadoc
The Javadoc block forgetResultsForParticipationdocuments the behavior and return value but omits the@param participation. Please add a parameter description for clarity.Proposed diff:
/** * Gets all Results of all submissions for the given Participation. + * @param participation the Participation whose submission results are aggregated * @return A Set of Results that belong to the Participation's submissions */src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingAssessmentIntegrationTest.java (1)
1016-1017: Consider using the utility service for consistency.While this approach works, it's inconsistent with the pattern used elsewhere in this PR. For consistency with other changes, consider using the utility service method:
-Result result = programmingExerciseStudentParticipation.getSubmissions().stream().findFirst().orElseThrow().getFirstResult(); -assertThat(result).isNotNull(); +Result result = participationUtilService.getResultsForParticipation(programmingExerciseStudentParticipation).stream().findFirst().orElse(null); +assertThat(result).isNotNull();
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
src/main/java/de/tum/cit/aet/artemis/assessment/service/CourseScoreCalculationService.java(3 hunks)src/main/java/de/tum/cit/aet/artemis/assessment/web/ResultResource.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/exam/service/ExamService.java(4 hunks)src/main/java/de/tum/cit/aet/artemis/exercise/domain/participation/Participation.java(0 hunks)src/main/java/de/tum/cit/aet/artemis/exercise/repository/StudentParticipationRepository.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exercise/web/ExerciseResource.java(3 hunks)src/main/java/de/tum/cit/aet/artemis/exercise/web/ParticipationResource.java(4 hunks)src/main/java/de/tum/cit/aet/artemis/modeling/web/ModelingSubmissionResource.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/programming/domain/ProgrammingExerciseParticipation.java(0 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseCodeReviewFeedbackService.java(3 hunks)src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingAssessmentResource.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseParticipationResource.java(3 hunks)src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizResultService.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizSubmissionService.java(0 hunks)src/test/java/de/tum/cit/aet/artemis/assessment/service/CourseScoreCalculationServiceTest.java(2 hunks)src/test/java/de/tum/cit/aet/artemis/exam/StudentExamIntegrationTest.java(5 hunks)src/test/java/de/tum/cit/aet/artemis/exercise/ExerciseIntegrationTest.java(4 hunks)src/test/java/de/tum/cit/aet/artemis/exercise/participation/ParticipationIntegrationTest.java(6 hunks)src/test/java/de/tum/cit/aet/artemis/exercise/participation/util/ParticipationUtilService.java(2 hunks)src/test/java/de/tum/cit/aet/artemis/exercise/team/TeamIntegrationTest.java(2 hunks)src/test/java/de/tum/cit/aet/artemis/fileupload/FileUploadAssessmentIntegrationTest.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/fileupload/FileUploadSubmissionIntegrationTest.java(4 hunks)src/test/java/de/tum/cit/aet/artemis/modeling/ModelingAssessmentIntegrationTest.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/modeling/ModelingSubmissionIntegrationTest.java(0 hunks)src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingAssessmentIntegrationTest.java(2 hunks)src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseGradingServiceTest.java(10 hunks)src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseParticipationIntegrationTest.java(10 hunks)src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalCIIntegrationTest.java(2 hunks)src/test/java/de/tum/cit/aet/artemis/text/TextSubmissionIntegrationTest.java(4 hunks)
💤 Files with no reviewable changes (4)
- src/test/java/de/tum/cit/aet/artemis/modeling/ModelingSubmissionIntegrationTest.java
- src/main/java/de/tum/cit/aet/artemis/exercise/domain/participation/Participation.java
- src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizSubmissionService.java
- src/main/java/de/tum/cit/aet/artemis/programming/domain/ProgrammingExerciseParticipation.java
🧰 Additional context used
📓 Path-based instructions (2)
`src/test/java/**/*.java`: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_...
src/test/java/**/*.java: 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
src/test/java/de/tum/cit/aet/artemis/assessment/service/CourseScoreCalculationServiceTest.javasrc/test/java/de/tum/cit/aet/artemis/programming/icl/LocalCIIntegrationTest.javasrc/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseGradingServiceTest.javasrc/test/java/de/tum/cit/aet/artemis/fileupload/FileUploadSubmissionIntegrationTest.javasrc/test/java/de/tum/cit/aet/artemis/fileupload/FileUploadAssessmentIntegrationTest.javasrc/test/java/de/tum/cit/aet/artemis/exercise/participation/util/ParticipationUtilService.javasrc/test/java/de/tum/cit/aet/artemis/exercise/participation/ParticipationIntegrationTest.javasrc/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseParticipationIntegrationTest.javasrc/test/java/de/tum/cit/aet/artemis/text/TextSubmissionIntegrationTest.javasrc/test/java/de/tum/cit/aet/artemis/exercise/team/TeamIntegrationTest.javasrc/test/java/de/tum/cit/aet/artemis/exercise/ExerciseIntegrationTest.javasrc/test/java/de/tum/cit/aet/artemis/programming/ProgrammingAssessmentIntegrationTest.javasrc/test/java/de/tum/cit/aet/artemis/modeling/ModelingAssessmentIntegrationTest.javasrc/test/java/de/tum/cit/aet/artemis/exam/StudentExamIntegrationTest.java
`src/main/java/**/*.java`: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,de...
src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizResultService.javasrc/main/java/de/tum/cit/aet/artemis/exercise/web/ParticipationResource.javasrc/main/java/de/tum/cit/aet/artemis/assessment/web/ResultResource.javasrc/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingAssessmentResource.javasrc/main/java/de/tum/cit/aet/artemis/modeling/web/ModelingSubmissionResource.javasrc/main/java/de/tum/cit/aet/artemis/exercise/web/ExerciseResource.javasrc/main/java/de/tum/cit/aet/artemis/assessment/service/CourseScoreCalculationService.javasrc/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseCodeReviewFeedbackService.javasrc/main/java/de/tum/cit/aet/artemis/exercise/repository/StudentParticipationRepository.javasrc/main/java/de/tum/cit/aet/artemis/exam/service/ExamService.javasrc/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.javasrc/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseParticipationResource.java
🧠 Learnings (4)
src/main/java/de/tum/cit/aet/artemis/exercise/web/ExerciseResource.java (1)
Learnt from: janthoXO
PR: ls1intum/Artemis#9406
File: src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseParticipationResource.java:209-209
Timestamp: 2025-02-11T12:05:49.151Z
Learning: In ProgrammingExerciseParticipationResource, exam-related authorization checks and sensitive information filtering for results and feedbacks are handled by resultService.filterSensitiveInformationIfNecessary().
src/test/java/de/tum/cit/aet/artemis/exercise/ExerciseIntegrationTest.java (1)
Learnt from: janthoXO
PR: ls1intum/Artemis#9406
File: src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseParticipationResource.java:209-209
Timestamp: 2025-02-11T12:05:49.151Z
Learning: In ProgrammingExerciseParticipationResource, exam-related authorization checks and sensitive information filtering for results and feedbacks are handled by resultService.filterSensitiveInformationIfNecessary().
src/test/java/de/tum/cit/aet/artemis/exam/StudentExamIntegrationTest.java (1)
undefined
<retrieved_learning>
Learnt from: janthoXO
PR: #9406
File: src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseParticipationResource.java:209-209
Timestamp: 2025-02-11T12:05:49.151Z
Learning: In ProgrammingExerciseParticipationResource, exam-related authorization checks and sensitive information filtering for results and feedbacks are handled by resultService.filterSensitiveInformationIfNecessary().
</retrieved_learning>
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseParticipationResource.java (1)
Learnt from: janthoXO
PR: ls1intum/Artemis#9406
File: src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseParticipationResource.java:209-209
Timestamp: 2025-02-11T12:05:49.151Z
Learning: In ProgrammingExerciseParticipationResource, exam-related authorization checks and sensitive information filtering for results and feedbacks are handled by resultService.filterSensitiveInformationIfNecessary().
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Build and Push Docker Image / Build Docker Image for ls1intum/artemis
- GitHub Check: Build and Push Docker Image / Build Docker Image for ls1intum/artemis
- GitHub Check: Build .war artifact
- GitHub Check: client-tests
- GitHub Check: server-tests
- GitHub Check: Analyse
🔇 Additional comments (64)
src/test/java/de/tum/cit/aet/artemis/assessment/service/CourseScoreCalculationServiceTest.java (1)
147-147: LGTM! Proper replacement of deprecated getResults() method.The test correctly uses
participationUtilService.getResultsForParticipation()to aggregate results from all submissions instead of directly accessing the deprecatedgetResults()method. This aligns with the refactoring objectives and maintains the same test behavior.Also applies to: 157-157, 165-165
src/main/java/de/tum/cit/aet/artemis/assessment/web/ResultResource.java (1)
8-8: Appropriate import addition.The
java.util.Collectionimport is correctly added to support the new streaming logic for result aggregation.src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalCIIntegrationTest.java (1)
492-492: Consistent refactoring applied correctly.Both test methods properly use
participationUtilService.getResultsForParticipation()instead of the deprecatedgetResults()method. The pattern of using.iterator().next()to get the first result is safe in these test contexts where results are guaranteed to exist.Also applies to: 545-545
src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseGradingServiceTest.java (7)
578-584: LGTM! Correctly updated to use utility service method.The change from
participation.getResults()toparticipationUtilService.getResultsForParticipation(participation)aligns with the refactoring to remove the deprecated method while maintaining the same test behavior.
588-594: LGTM! Consistent refactoring pattern applied.The solution participation result retrieval has been correctly updated to use the utility service method, maintaining consistency with the refactoring approach.
641-647: LGTM! Refactoring consistently applied across test scenarios.The utility service method is correctly used for template participation result retrieval in the invisible test case scenario, maintaining test consistency.
651-657: LGTM! Solution participation handling updated correctly.The solution participation result retrieval has been properly updated to use the utility service method in the invisible test case scenario.
826-835: LGTM! Critical helper method updated correctly.The
verifyStudentScoreCalculationhelper method has been properly updated to use the utility service, ensuring consistent result retrieval across all test scenarios that depend on this method.
1061-1067: LGTM! SCA test updated correctly.The Static Code Analysis test correctly uses the utility service method for result retrieval, maintaining the test's ability to verify penalty calculations.
1069-1075: LGTM! All remaining SCA and result verification tests updated consistently.All remaining instances of result retrieval have been correctly updated to use the utility service method. The changes maintain:
- Consistent refactoring pattern across all test scenarios
- Same test logic and assertions
- Proper handling of various SCA penalty scenarios
- Correct verification of student score calculations
The refactoring successfully removes the deprecated
getResults()method usage while preserving all test functionality.Also applies to: 1104-1110, 1112-1118, 1144-1150, 1176-1182, 1226-1232
src/test/java/de/tum/cit/aet/artemis/fileupload/FileUploadSubmissionIntegrationTest.java (1)
437-437: LGTM! Consistent adoption of the new utility method.The test assertions have been correctly updated to use
participationUtilService.getResultsForParticipation()instead of accessingsubmission.getParticipation().getResults()directly. This aligns perfectly with the broader refactoring to remove the deprecatedParticipation#getResults()method and ensures null-safe access to results aggregated from all submissions.Also applies to: 452-452, 467-467, 639-639
src/main/java/de/tum/cit/aet/artemis/exercise/web/ParticipationResource.java (3)
10-10: Good addition of required imports.The new imports for
CollectionandStreamsupport the updated result counting logic that streams over submissions and their results.Also applies to: 20-20
680-680: Appropriate type change for result counting.Changing from
inttolongforresultCountis sensible since we're now aggregating results across potentially many submissions, which could exceed integer limits in large courses.
709-711: Excellent implementation of the new result aggregation pattern.The code correctly replaces direct access to
participation.getResults()with streaming over all submissions and their results. The use ofStream.of()andflatMap()safely handles the collections and aggregates results comprehensively across all submissions of each participation, which aligns perfectly with the refactoring goals.src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizResultService.java (2)
6-6: Appropriate import additions for the new logic.The new imports for
Collection,Objects, andStreamsupport the enhanced null-safe result aggregation logic.Also applies to: 10-10, 13-13
148-150: Excellent null-safe implementation of result aggregation.The new logic properly replaces the deprecated
participation.getResults()by streaming over all submissions and their results. The use ofStream.ofNullable()andObjects::nonNullfilter provides robust null-safety, ensuring the code handles cases where submissions or results collections might be null. The filtering for rated results maintains the original business logic while adopting the new access pattern.src/test/java/de/tum/cit/aet/artemis/exercise/participation/util/ParticipationUtilService.java (2)
16-22: Imports for new utility method
The added imports (Collection,Objects,Set,Collectors,Stream) are necessary and correctly support the implementation ofgetResultsForParticipation().
988-991: Null-safe aggregation of results
The method implementation correctly handles potential null collections from bothparticipation.getSubmissions()andsubmission.getResults(), filters out null entries, and aggregates all results into aSet.src/test/java/de/tum/cit/aet/artemis/text/TextSubmissionIntegrationTest.java (4)
22-22: Added import forResult
The new import ofde.tum.cit.aet.artemis.assessment.domain.Resultis required for the test assertions that reference theResulttype.
315-316: Replaced directgetResults()with utility call
UsingparticipationUtilService.getResultsForParticipation(studentParticipation)correctly replaces the deprecated direct access and ensures the test checks the assessor field as expected.
328-330: Consistent use of new utility in test
Retrieving and asserting onresultsviaparticipationUtilService.getResultsForParticipation(participation)aligns with the updated pattern and verifies the single expected result.
490-490: Helper assertion updated to use utility method
The privatecheckDetailsHiddenhelper now correctly verifies an empty result set by callinggetResultsForParticipation, replacing the old direct collection access.src/test/java/de/tum/cit/aet/artemis/modeling/ModelingAssessmentIntegrationTest.java (1)
742-743: LGTM! Correctly refactored to use utility service method.The change properly replaces the deprecated
Participation#getResults()method withparticipationUtilService.getResultsForParticipation(), which aggregates results from all submissions. This aligns with the PR objectives and maintains the same functional behavior.src/test/java/de/tum/cit/aet/artemis/exercise/team/TeamIntegrationTest.java (2)
25-25: Import addition is appropriate.The
Submissionimport is correctly added to support the variable declaration in the test method.
553-555: Good refactoring: extracted submission for better readability.Extracting the submission into a variable improves code readability and avoids redundant calls to
participation.getSubmissions().iterator().next(). This is a clean improvement that makes the test assertions more maintainable.src/main/java/de/tum/cit/aet/artemis/exercise/repository/StudentParticipationRepository.java (1)
973-974: LGTM! Correct implementation of result aggregation from submissions.The code correctly replaces the deprecated
participation.getResults()pattern by aggregating results from all submissions. The null-safe approach usingStream.ofNullable(submission.getResults())and the proper flattening of nested streams ensures robust result collection.src/test/java/de/tum/cit/aet/artemis/fileupload/FileUploadAssessmentIntegrationTest.java (1)
502-503: LGTM! Proper usage of the new utility method for result retrieval.The test correctly adopts the new
participationUtilService.getResultsForParticipation()method to replace direct access to participation results. This maintains the test's intent while using the recommended approach for result aggregation.src/main/java/de/tum/cit/aet/artemis/exercise/web/ExerciseResource.java (2)
6-6: Approve import additions for stream-based refactoring.The new imports (
Collection,Objects,Stream) are necessary and appropriate for the stream-based result aggregation that replaces the deprecatedgetResults()method.Also applies to: 10-10, 14-14
338-340: Excellent implementation of null-safe result aggregation.The stream-based approach properly replaces the deprecated
participation.getResults()method by:
- Safely navigating through submissions using
Stream.ofNullable()- Extracting results from each submission with proper null handling
- Filtering out null results with
filter(Objects::nonNull)- Applying sensitive information filtering to each result
This implementation is more comprehensive than the previous approach as it ensures all results from all submissions are processed, improving both null safety and completeness.
The code follows the coding guidelines for:
- ✅ Small methods principle (concise stream operations)
- ✅ Null safety with defensive programming
- ✅ Proper use of Java streams for collection processing
- ✅ Static member reference for
Result::filterSensitiveInformationsrc/test/java/de/tum/cit/aet/artemis/exam/StudentExamIntegrationTest.java (5)
1658-1658: LGTM: Consistent migration from deprecated method.The replacement of direct participation results access with the utility service call is consistent with the broader refactoring effort described in the PR.
1698-1698: LGTM: Proper use of utility service.The migration maintains the same test logic while using the new
participationUtilService.getResultsForParticipation()API.
1717-1717: LGTM: Consistent API migration.The test assertion logic remains intact while successfully migrating to the utility service method.
1757-1759: LGTM: Well-structured result retrieval and processing.The refactored code properly retrieves results using the utility service and maintains clear, readable test assertions. The variable extraction improves code readability.
2143-2143: LGTM: Final migration to utility service.This completes the consistent migration pattern throughout the test file, replacing the deprecated direct access with the appropriate utility service call.
src/test/java/de/tum/cit/aet/artemis/exercise/participation/ParticipationIntegrationTest.java (7)
1030-1030: LGTM! Proper replacement of deprecated method.The use of
participationUtilService.getResultsForParticipation()correctly replaces the deprecatedgetResults()method while maintaining the same test logic.
1034-1034: LGTM! Correct access pattern through submissions.Accessing results through submissions (
getSubmissions().iterator().next().getResults()) aligns with the new approach of aggregating results from submissions rather than direct participation access.
1063-1063: LGTM! Consistent use of utility method.The replacement with
participationUtilService.getResultsForParticipation()maintains the same assertion logic while using the new access pattern.
1091-1093: LGTM! Consistent refactoring across multiple assertions.All three lines properly use the utility method to access participation results, maintaining consistent test patterns while replacing the deprecated method.
1448-1449: LGTM! Proper separation of concerns in test assertions.Line 1448 correctly accesses submissions directly from the participation, while line 1449 uses the utility method for results access, following the new pattern of separating submission and result access.
1702-1702: LGTM! Clean variable assignment using utility method.Extracting results into a variable using the utility method improves readability while properly replacing the deprecated method access.
1900-1900: LGTM! Consistent pattern in isolated test.The use of
participationUtilService.getResultsForParticipation()in the isolated test class maintains consistency with the refactoring throughout the file.src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseParticipationIntegrationTest.java (6)
17-17: LGTM: Import addition aligns with refactoring.The
Setimport is correctly added to support the new return type fromparticipationUtilService.getResultsForParticipation().
117-119: LGTM: Correct refactoring pattern implementation.The change properly replaces the deprecated
getResults()method with the utility service call, maintaining equivalent functionality while using the new approach to aggregate results from submissions.
143-145: LGTM: Consistent refactoring pattern.The implementation correctly follows the same pattern established in the previous change.
211-211: LGTM: Clean assertion updates.The direct assertion calls using the utility service method are correct and maintain the same test logic while following the refactoring pattern.
Also applies to: 221-221, 231-231
256-277: LGTM: Complex test scenario correctly refactored.The changes properly handle the multi-result test case, maintaining all assertion logic while using the new utility service approach. The conditional size checks and result containment assertions are correctly implemented.
358-358: LGTM: Consistent single-line refactoring.These changes correctly apply the established refactoring pattern for simple assertion cases.
Also applies to: 369-369, 382-382
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseParticipationResource.java (1)
7-7: LGTM: Necessary imports for streaming refactoring.The added imports support the new stream-based approach for collecting results from submissions, including null safety with
Objects.nonNull().Also applies to: 10-10, 12-12, 14-14
src/main/java/de/tum/cit/aet/artemis/modeling/web/ModelingSubmissionResource.java (2)
6-6: LGTM! Appropriate imports added for the refactoring.The new imports for
Collection,Objects, andStreamsupport the updated result collection logic.Also applies to: 9-9, 12-12
410-412: Excellent null-safe result collection implementation.The refactoring successfully replaces the deprecated
participation.getResults()with a comprehensive approach that:
- Safely handles null submissions collection with
Stream.ofNullable- Streams over all submissions to ensure completeness
- Safely handles null results within each submission
- Maintains the same filtering logic for
AUTOMATIC_ATHENAassessment typeThis approach is more robust and ensures all results across all submissions are considered.
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.java (2)
5-5: LGTM! Required imports added for stream operations.The imports support the updated result collection pattern.
Also applies to: 8-8
206-208: Consistent and correct implementation of the refactoring pattern.The rate limit check now properly collects Athena results from all submissions using the same null-safe streaming pattern as other files in this refactoring. This ensures comprehensive coverage of all results across all submissions for accurate rate limiting.
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingAssessmentResource.java (2)
5-5: LGTM! Necessary imports added for the refactoring.The imports support the updated result retrieval logic.
Also applies to: 7-8
138-141: Correct implementation of manual result retrieval.The refactoring properly replaces the deprecated
participation.getResults()with comprehensive result collection from all submissions. The use ofmax(Comparator.comparing(Result::getId))to find the most recent manual result is appropriate, and the null-safe stream operations ensure robustness.src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseCodeReviewFeedbackService.java (2)
8-8: LGTM! Required imports added for the refactoring.The new imports support the updated result collection and processing logic.
Also applies to: 13-13, 15-16, 26-26
203-204: Correct implementation for result invalidation.The refactoring properly collects all results from all submissions using the consistent null-safe streaming pattern. Using
Collectors.toSet()is appropriate here to prevent potential duplicates when invalidating results, and the approach ensures comprehensive coverage of all results across all submissions.src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingAssessmentIntegrationTest.java (1)
879-880: LGTM! Consistent with the refactoring pattern.The change correctly uses the utility service method to retrieve results from participation, which aligns with the PR objective of removing the deprecated
getResults()method.src/test/java/de/tum/cit/aet/artemis/exercise/ExerciseIntegrationTest.java (4)
439-447: LGTM! Consistent refactoring of result access.The changes correctly replace the deprecated
getResults()method with the utility service approach, maintaining the same test logic while following the new pattern.
467-477: LGTM! Proper use of utility service method.The refactoring consistently applies the new pattern for retrieving results from participations across all assertions in this test method.
514-525: LGTM! Consistent with the overall refactoring approach.The changes properly implement the utility service pattern while preserving the existing test assertions and logic.
540-545: LGTM! Final consistent implementation of the refactoring.All changes in this method follow the established pattern of using the utility service to access participation results.
src/main/java/de/tum/cit/aet/artemis/assessment/service/CourseScoreCalculationService.java (1)
14-14: LGTM on import additions.The new imports for
ObjectsandStreamare appropriately added to support the result aggregation logic.Also applies to: 18-18
src/main/java/de/tum/cit/aet/artemis/exam/service/ExamService.java (2)
23-23: LGTM on Stream import addition.The import is appropriately added to support the result aggregation logic.
186-189: LGTM on dependency cleanup.Removing the unused
GroupNotificationServicedependency from the constructor is appropriate and aligns with clean code practices.
| Set<Result> results = Stream.ofNullable(participation.getSubmissions()).flatMap(Collection::stream) | ||
| .flatMap(submission -> Stream.ofNullable(submission.getResults()).flatMap(Collection::stream)).filter(Objects::nonNull).collect(Collectors.toSet()); | ||
| // hide details that should not be shown to the students | ||
| resultService.filterSensitiveInformationIfNecessary(participation, participation.getResults(), Optional.empty()); | ||
| resultService.filterSensitiveInformationIfNecessary(participation, results, Optional.empty()); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Duplicate streaming logic should be extracted.
This method contains the exact same complex streaming logic as the previous method, confirming the need for extracting this to a shared utility method as suggested above.
Apply the same refactoring pattern here:
- Set<Result> results = Stream.ofNullable(participation.getSubmissions()).flatMap(Collection::stream)
- .flatMap(submission -> Stream.ofNullable(submission.getResults()).flatMap(Collection::stream)).filter(Objects::nonNull).collect(Collectors.toSet());
+ Set<Result> results = getResultsFromParticipationSubmissions(participation);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseParticipationResource.java
around lines 181 to 184, the streaming logic to collect results from submissions
is duplicated from a previous method. Extract this complex streaming logic into
a shared private utility method that takes a participation object and returns
the set of results. Replace the duplicated code here with a call to that new
utility method to improve code reuse and maintainability.
...in/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseParticipationResource.java
Outdated
Show resolved
Hide resolved
| Set<Result> resultsSet = Stream.ofNullable(participation.getSubmissions()).flatMap(Collection::stream) | ||
| .flatMap(submission -> Stream.ofNullable(submission.getResults()).flatMap(Collection::stream)).filter(Objects::nonNull).collect(Collectors.toSet()); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consistent implementation but same refactoring opportunity.
The result aggregation logic is correctly implemented and consistent with the pattern used elsewhere. However, this reinforces the need for the helper method suggested above.
This location would also benefit from using the proposed getResultsFromParticipation() helper method to eliminate code duplication.
🤖 Prompt for AI Agents
In
src/main/java/de/tum/cit/aet/artemis/assessment/service/CourseScoreCalculationService.java
around lines 413 to 414, the code manually aggregates results from participation
submissions, duplicating logic found elsewhere. Refactor this by replacing the
current stream operations with a call to the existing or newly created helper
method getResultsFromParticipation(participation) to centralize and reuse the
result extraction logic, thereby eliminating code duplication and improving
maintainability.
| Set<Result> results = Stream.ofNullable(studentParticipation.getSubmissions()).flatMap(Collection::stream) | ||
| .flatMap(submission -> Stream.ofNullable(submission.getResults()).flatMap(Collection::stream)).filter(Objects::nonNull).collect(Collectors.toSet()); | ||
| if (!results.isEmpty()) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider extracting the result aggregation pattern into a helper method.
The streaming logic correctly replaces the deprecated getResults() method by aggregating results from all submissions. However, this verbose pattern is repeated multiple times throughout the codebase.
Consider extracting this into a reusable helper method:
+ private Set<Result> getResultsFromParticipation(StudentParticipation participation) {
+ return Stream.ofNullable(participation.getSubmissions())
+ .flatMap(Collection::stream)
+ .flatMap(submission -> Stream.ofNullable(submission.getResults()).flatMap(Collection::stream))
+ .filter(Objects::nonNull)
+ .collect(Collectors.toSet());
+ }
+
for (StudentParticipation studentParticipation : gradedStudentParticipations) {
- Set<Result> results = Stream.ofNullable(studentParticipation.getSubmissions()).flatMap(Collection::stream)
- .flatMap(submission -> Stream.ofNullable(submission.getResults()).flatMap(Collection::stream)).filter(Objects::nonNull).collect(Collectors.toSet());
+ Set<Result> results = getResultsFromParticipation(studentParticipation);
if (!results.isEmpty()) {This would improve code reusability and reduce duplication as per the coding guidelines.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Set<Result> results = Stream.ofNullable(studentParticipation.getSubmissions()).flatMap(Collection::stream) | |
| .flatMap(submission -> Stream.ofNullable(submission.getResults()).flatMap(Collection::stream)).filter(Objects::nonNull).collect(Collectors.toSet()); | |
| if (!results.isEmpty()) { | |
| private Set<Result> getResultsFromParticipation(StudentParticipation participation) { | |
| return Stream.ofNullable(participation.getSubmissions()) | |
| .flatMap(Collection::stream) | |
| .flatMap(submission -> Stream.ofNullable(submission.getResults()).flatMap(Collection::stream)) | |
| .filter(Objects::nonNull) | |
| .collect(Collectors.toSet()); | |
| } | |
| for (StudentParticipation studentParticipation : gradedStudentParticipations) { | |
| Set<Result> results = getResultsFromParticipation(studentParticipation); | |
| if (!results.isEmpty()) { | |
| // existing processing logic | |
| } | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/de/tum/cit/aet/artemis/assessment/service/CourseScoreCalculationService.java
around lines 269 to 271, the code uses a verbose stream pattern to aggregate
results from submissions, which is repeated multiple times. Extract this
streaming logic into a private helper method that takes a studentParticipation
object and returns a Set of Results by aggregating all results from its
submissions. Replace all occurrences of the original stream pattern with calls
to this new helper method to improve code reusability and reduce duplication.
| Result result = participation.getSubmissions().stream().findFirst().map(Submission::getFirstResult).orElse(null); | ||
| return calculateAchievedPoints(participation.getExercise(), result, course, plagiarismPointDeductionPercentage); | ||
| })); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consistent pattern but reinforces need for refactoring.
The implementation correctly aggregates results from submissions and maintains the expected behavior. However, this is the same verbose pattern repeated throughout the codebase.
Consider using a shared utility method like ParticipationUtil.getResultsFromParticipation(participation) to eliminate this duplication and improve maintainability.
🤖 Prompt for AI Agents
In src/main/java/de/tum/cit/aet/artemis/exam/service/ExamService.java around
lines 852 to 854, the code repetitively extracts the first result from
participation submissions using a verbose stream pattern. Refactor this by
replacing the stream logic with a call to a shared utility method like
ParticipationUtil.getResultsFromParticipation(participation) to retrieve the
results, thereby reducing duplication and improving maintainability.
github-project-automation
bot
moved this from Work In Progress
to Ready For Review
in Artemis Development
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
| @@ -964,14 +963,9 @@ private List<StudentParticipation> filterParticipationsWithRelevantResults(List< | |||
| .peek(participation -> { | |||
There was a problem hiding this comment.
todo check if we can remove this peek completely, it seems to have no side effects so it doesnt have any effect at all?
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||
github-actions
bot
added
the
iris
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
|||||||||||||||||||||||||||||||||||||||||||||
Checklist
General
Server
Motivation and Context
Participation#getResults() is deprecated and should be removed.
Description
Removing Participation#getResults() method and introducing a similar helper in test utils for simplicity.
Callers should control themself how they get results for a participation. For now most calls are replaced by flattening over submissions to collect all results of all submissions similar to what getResults() did.
Steps for Testing
code review
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
Summary by CodeRabbit
Bug Fixes
Refactor
Tests
Chores