General: Fix an issue with course deletion summary entries
#9856
Conversation
github-actions
bot
added
server
ole-ve
changed the title
Courses: Load course deletion summary entries from server that are not present in the clientUsability: Load course deletion summary entries from server that are not present in the client
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 pmd (7.8.0)src/main/java/de/tum/cit/aet/artemis/core/service/CourseService.javaThe following rules are missing or misspelled in your ruleset file category/vm/bestpractices.xml: BooleanInstantiation, DontImportJavaLang, DuplicateImports, EmptyFinallyBlock, EmptyIfStmt, EmptyInitializer, EmptyStatementBlock, EmptyStatementNotInLoop, EmptySwitchStatements, EmptySynchronizedBlock, EmptyTryBlock, EmptyWhileStmt, ExcessiveClassLength, ExcessiveMethodLength, ImportFromSamePackage, MissingBreakInSwitch, SimplifyBooleanAssertion. Please check your ruleset configuration. src/main/java/de/tum/cit/aet/artemis/exercise/dto/ExerciseTypeCountDTO.javaThe following rules are missing or misspelled in your ruleset file category/vm/bestpractices.xml: BooleanInstantiation, DontImportJavaLang, DuplicateImports, EmptyFinallyBlock, EmptyIfStmt, EmptyInitializer, EmptyStatementBlock, EmptyStatementNotInLoop, EmptySwitchStatements, EmptySynchronizedBlock, EmptyTryBlock, EmptyWhileStmt, ExcessiveClassLength, ExcessiveMethodLength, ImportFromSamePackage, MissingBreakInSwitch, SimplifyBooleanAssertion. Please check your ruleset configuration. src/main/java/de/tum/cit/aet/artemis/exercise/repository/ExerciseRepository.javaThe following rules are missing or misspelled in your ruleset file category/vm/bestpractices.xml: BooleanInstantiation, DontImportJavaLang, DuplicateImports, EmptyFinallyBlock, EmptyIfStmt, EmptyInitializer, EmptyStatementBlock, EmptyStatementNotInLoop, EmptySwitchStatements, EmptySynchronizedBlock, EmptyTryBlock, EmptyWhileStmt, ExcessiveClassLength, ExcessiveMethodLength, ImportFromSamePackage, MissingBreakInSwitch, SimplifyBooleanAssertion. Please check your ruleset configuration.
WalkthroughThis pull request enhances the course deletion summary functionality across multiple components of the Artemis application. The changes introduce more comprehensive metrics for course deletion, including detailed counts for lectures, exams, and various exercise types. The modifications span both backend (Java) and frontend (TypeScript) files, updating data transfer objects, repositories, services, and components to provide a more accurate representation of course components during the deletion process. Changes
Assessment against linked issues
Suggested labels
Suggested reviewers
Possibly related PRs
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
🧰 Additional context used📓 Path-based instructions (1)src/main/java/de/tum/cit/aet/artemis/exercise/dto/ExerciseTypeCountDTO.java (1)Pattern 🔇 Additional comments (2)
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: 4
🧹 Outside diff range and nitpick comments (11)
src/main/webapp/app/entities/course-deletion-summary.model.ts (1)
5-11: Consider adding JSDoc documentation for the new properties.The new properties are well-named and correctly typed. However, adding JSDoc comments would improve maintainability by documenting the purpose and expected values of each property.
export interface CourseDeletionSummaryDTO { numberOfBuilds: number; numberOfCommunicationPosts: number; numberOfAnswerPosts: number; + /** Number of programming exercises in the course */ numberProgrammingExercises: number; + /** Number of text exercises in the course */ numberTextExercises: number; + /** Number of file upload exercises in the course */ numberFileUploadExercises: number; + /** Number of quiz exercises in the course */ numberQuizExercises: number; + /** Number of modeling exercises in the course */ numberModelingExercises: number; + /** Number of exams in the course */ numberExams: number; + /** Number of lectures in the course */ numberLectures: number; }src/main/java/de/tum/cit/aet/artemis/core/dto/CourseDeletionSummaryDTO.java (1)
6-7: Consider improving organization and documentationWhile the implementation is functionally correct, consider these improvements:
- Group related fields into nested records for better organization
- Add JavaDoc documentation explaining the purpose of each field
Example refactor:
-public record CourseDeletionSummaryDTO(long numberOfBuilds, long numberOfCommunicationPosts, long numberOfAnswerPosts, long numberProgrammingExercises, long numberTextExercises, - long numberFileUploadExercises, long numberModelingExercises, long numberQuizExercises, long numberExams, long numberLectures) { +/** + * Data Transfer Object for course deletion summary information. + * Contains counts of various course components that will be affected by deletion. + */ +public record CourseDeletionSummaryDTO( + // Build and communication metrics + long numberOfBuilds, + long numberOfCommunicationPosts, + long numberOfAnswerPosts, + + // Exercise counts by type + ExerciseCountsDTO exerciseCounts, + + // Course structure counts + long numberExams, + long numberLectures +) { + /** + * Nested record for exercise-related counts + */ + public record ExerciseCountsDTO( + long numberProgrammingExercises, + long numberTextExercises, + long numberFileUploadExercises, + long numberModelingExercises, + long numberQuizExercises + ) {} +}src/main/java/de/tum/cit/aet/artemis/exercise/dto/ExerciseTypeCount.java (3)
8-12: Add JavaDoc documentation for better maintainability.Consider adding JavaDoc documentation to describe:
- The purpose and usage of this DTO class
- The meaning of each field
- Any constraints or invariants
+/** + * Data Transfer Object for representing the count of exercises grouped by their type. + * Used in course deletion summary to provide exercise statistics. + */ public class ExerciseTypeCount { + /** The type of the exercise */ private final ExerciseType exerciseType; + /** The number of exercises of this type */ private final long count;
19-25: Add JavaDoc to getter methods.While the getters are correctly implemented, adding documentation would improve code maintainability.
+/** + * @return the type of the exercise + */ public ExerciseType getExerciseType() { return exerciseType; } +/** + * @return the number of exercises of this type + */ public long getCount() { return count; }
8-26: Consider converting to a Java Record.Since this class is a simple DTO with immutable fields and basic getters, it's a perfect candidate for a Java Record. This would:
- Reduce boilerplate code
- Automatically provide equals(), hashCode(), and toString()
- Better express the intent of the class as a data carrier
-public class ExerciseTypeCount { - private final ExerciseType exerciseType; - private final long count; - - public ExerciseTypeCount(Class<?> exerciseType, Long count) { - this.exerciseType = ExerciseType.getExerciseTypeFromClass(exerciseType.asSubclass(Exercise.class)); - this.count = Objects.requireNonNullElse(count, 0L); - } - - public ExerciseType getExerciseType() { - return exerciseType; - } - - public long getCount() { - return count; - } -} +/** + * Data Transfer Object for representing the count of exercises grouped by their type. + * Used in course deletion summary to provide exercise statistics. + * + * @param exerciseType the type of the exercise + * @param count the number of exercises of this type + */ +public record ExerciseTypeCount(ExerciseType exerciseType, long count) { + /** + * Creates a new ExerciseTypeCount instance. + * + * @param exerciseType the class of the exercise type, must be a subclass of Exercise + * @param count the number of exercises of this type, defaults to 0 if null + * @throws NullPointerException if exerciseType is null + * @throws ClassCastException if exerciseType is not a subclass of Exercise + */ + public ExerciseTypeCount(Class<?> exerciseType, Long count) { + this( + Objects.requireNonNull(exerciseType, "exerciseType must not be null"), + ExerciseType.getExerciseTypeFromClass(exerciseType.asSubclass(Exercise.class)), + Objects.requireNonNullElse(count, 0L) + ); + } +}src/main/java/de/tum/cit/aet/artemis/lecture/repository/LectureRepository.java (1)
199-200: LGTM with suggestions for improvement.The method follows Spring Data JPA conventions and aligns with the repository's purpose. Consider these enhancements:
+ /** + * Counts the number of lectures associated with a specific course. + * Used in course deletion summary to track the number of lectures that will be removed. + * + * @param courseId the ID of the course + * @return the number of lectures in the course + */ + @Cacheable(cacheNames = "lectureCounts", key = "#courseId") long countByCourse_Id(Long courseId);The suggested improvements:
- Added JavaDoc documentation to align with the existing codebase style
- Added @Cacheable annotation to optimize performance for repeated calls, following the pattern used in
getLectureTitlesrc/main/webapp/app/course/manage/course-management-tab-bar/course-management-tab-bar.component.ts (1)
246-252: Add JSDoc comments for the new summary fieldsTo maintain consistency with the PR's documentation requirements, consider adding JSDoc comments explaining these new fields.
fetchCourseDeletionSummary(): Observable<EntitySummary> { + /** + * Fetches a comprehensive summary of course elements for deletion confirmation. + * Includes counts of: + * - Exams and lectures + * - Different types of exercises (programming, text, modeling, etc.) + * - Related entities (builds, communication posts, etc.) + * @returns Observable<EntitySummary> containing all course element counts + */src/main/java/de/tum/cit/aet/artemis/exam/repository/ExamRepository.java (1)
388-389: Consider using primitive long for parameter type and add documentation.While the implementation follows Spring Data JPA conventions, consider these improvements:
- Use primitive
longinstead ofLongto prevent null values and maintain consistency with other similar methods in the repository.- Add JavaDoc documentation to describe the method's purpose and parameters.
+ /** + * Count the number of exams associated with a course. + * + * @param courseId the id of the course + * @return the number of exams in the course + */ - long countByCourse_Id(Long courseId); + long countByCourse_Id(long courseId);The overall implementation is correct and aligns well with the repository's purpose of providing exam-related data access.
src/main/java/de/tum/cit/aet/artemis/exercise/repository/ExerciseRepository.java (1)
632-639: Consider enhancing the method documentation.While the current documentation is clear, it would be helpful to add:
- @throws clause for potential exceptions
- Example of the returned data structure
/** * Returns a mapping from exercise type to count for a given course id. Note that there are way fewer courses * than exercise, so loading the course and joining the course is way faster than vice versa. * * @param courseId the courseId to get the exerciseType->count mapping for * @return a list of mappings from exercise type to count + * @throws IllegalArgumentException if courseId is invalid + * Example return value: [(MODELING, 5), (PROGRAMMING, 3), (QUIZ, 2)] */src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseService.java (1)
835-845: Good implementation, but could be enhanced with validation and optimization.The implementation is clean and handles all exercise types correctly. However, consider these improvements:
- Add parameter validation for courseId
- Add @NotNull annotation for the parameter
- Optimize the stream operations
Here's a suggested improvement:
- public Map<ExerciseType, Long> countByCourseIdGroupByType(Long courseId) { + public Map<ExerciseType, Long> countByCourseIdGroupByType(@NotNull Long courseId) { + if (courseId == null) { + throw new IllegalArgumentException("courseId must not be null"); + } + Map<ExerciseType, Long> exerciseTypeCountMap = exerciseRepository.countByCourseIdGroupedByType(courseId).stream() .collect(Collectors.toMap(ExerciseTypeCount::getExerciseType, ExerciseTypeCount::getCount)); - - return Arrays.stream(ExerciseType.values()).collect(Collectors.toMap(type -> type, type -> exerciseTypeCountMap.getOrDefault(type, 0L))); + return Arrays.stream(ExerciseType.values()) + .collect(Collectors.toMap( + Function.identity(), + type -> exerciseTypeCountMap.getOrDefault(type, 0L) + )); }src/main/java/de/tum/cit/aet/artemis/core/service/CourseService.java (1)
479-480: Ensure consistent naming conventions in repository methodsThe method names
countByCourse_Iddo not follow standard CamelCase naming conventions. Consider renaming them to adhere to Java conventions.Apply this diff to rename the methods:
- long numberLectures = lectureRepository.countByCourse_Id(courseId); - long numberExams = examRepository.countByCourse_Id(courseId); + long numberLectures = lectureRepository.countByCourseId(courseId); + long numberExams = examRepository.countByCourseId(courseId);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (9)
src/main/java/de/tum/cit/aet/artemis/core/dto/CourseDeletionSummaryDTO.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/core/service/CourseService.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/exam/repository/ExamRepository.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exercise/dto/ExerciseTypeCount.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exercise/repository/ExerciseRepository.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseService.java(3 hunks)src/main/java/de/tum/cit/aet/artemis/lecture/repository/LectureRepository.java(1 hunks)src/main/webapp/app/course/manage/course-management-tab-bar/course-management-tab-bar.component.ts(1 hunks)src/main/webapp/app/entities/course-deletion-summary.model.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
src/main/java/de/tum/cit/aet/artemis/core/dto/CourseDeletionSummaryDTO.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
src/main/java/de/tum/cit/aet/artemis/core/service/CourseService.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
src/main/java/de/tum/cit/aet/artemis/exam/repository/ExamRepository.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
src/main/java/de/tum/cit/aet/artemis/exercise/dto/ExerciseTypeCount.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
src/main/java/de/tum/cit/aet/artemis/exercise/repository/ExerciseRepository.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
src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseService.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
src/main/java/de/tum/cit/aet/artemis/lecture/repository/LectureRepository.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
src/main/webapp/app/course/manage/course-management-tab-bar/course-management-tab-bar.component.ts (1)
src/main/webapp/app/entities/course-deletion-summary.model.ts (1)
🔇 Additional comments (10)
src/main/webapp/app/entities/course-deletion-summary.model.ts (1)
5-11: Verify null handling in consuming components.
Since these properties are loaded from the server, ensure that components using this DTO properly handle cases where these values might be undefined or null.
src/main/java/de/tum/cit/aet/artemis/core/dto/CourseDeletionSummaryDTO.java (1)
6-7: Implementation follows DTO best practices
The record implementation correctly follows the coding guidelines by:
- Using Java records for DTOs
- Using primitive types
- Following naming conventions
- Maintaining single responsibility
src/main/java/de/tum/cit/aet/artemis/exercise/dto/ExerciseTypeCount.java (1)
1-7: LGTM! Clean and well-organized imports.
The package structure and imports follow best practices with specific imports and no wildcards.
src/main/java/de/tum/cit/aet/artemis/lecture/repository/LectureRepository.java (1)
199-200: Verify integration with related components.
Let's ensure consistent implementation across the codebase:
✅ Verification successful
Method naming and integration are consistent with related components
The verification shows:
- The method
countByCourse_Idfollows a consistent naming pattern used in other repositories (ExamRepository) - It's properly integrated in CourseService where it's used alongside
examRepository.countByCourse_Idfor course deletion metrics - The implementation aligns with the PR's objective of enhancing course deletion summary
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for similar count methods in related repositories and CourseService usage
# Check for similar count methods in other repositories
echo "Checking for similar count methods in repositories..."
rg -l "countByCourse_Id|countByCourseId" --type java
# Check CourseService implementation
echo "Checking CourseService implementation..."
rg "countByCourse_Id|getDeletionSummary.*Lecture" --type java
# Check for other counting patterns in repositories
echo "Checking counting patterns in repositories..."
ast-grep --pattern 'interface $REPO extends $_ {
$$$
count$_($_) $_
$$$
}'
Length of output: 1708
src/main/webapp/app/course/manage/course-management-tab-bar/course-management-tab-bar.component.ts (1)
246-252: Verify performance impact for large courses
The PR objectives mention testing with courses having over 2000 students. Let's ensure the server-side summary calculation performs well at scale.
✅ Verification successful
Let me gather more information about the implementation to verify the performance aspects.
Let me search for more specific information about the database queries and their optimization.
Let me try one final search to check the query implementations.
Based on the gathered information, I can now provide a final response:
Performance optimizations are already in place for large-scale course operations
The implementation is well-optimized for handling large courses:
- Uses efficient count queries instead of loading full entities
- Leverages Hibernate's batch processing (
hibernate.jdbc.batch_size: 25) - Implements second-level caching with Hazelcast
- Uses optimized query configurations:
hibernate.query.in_clause_parameter_padding: truehibernate.order_inserts/updates: true- Proper fetch depth control (
hibernate.max_fetch_depth: 2)
The deletion summary calculation uses:
- Single database round-trips for counting entities
- Efficient group-by operations for exercise types
- Optimized collection handling with streaming operations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for performance-related configurations or optimizations in the backend
rg -g '*.java' -A 5 'CourseDeletionSummary|getDeletionSummary'
# Look for any existing performance tests
fd -e java -e ts test.*performance
Length of output: 6400
Script:
#!/bin/bash
# Check the implementation details of the deletion summary calculation
rg -g '*.java' -A 10 'buildJobRepository.countBuildJobsByExerciseIds|communicationPostRepository.countByCourseId|answerPostRepository.countByCourseId'
# Look for any database query optimizations or batch processing
ast-grep --pattern 'class BuildJobRepository {
$$$
countBuildJobsByExerciseIds($$$) {
$$$
}
$$$
}'
# Check if there are any pagination or limit configurations
rg -g '*.java' -B 3 -A 3 '@Query.*countBy.*CourseId|@Query.*countBy.*ExerciseIds'
Length of output: 3483
Script:
#!/bin/bash
# Check repository interfaces for query implementations
rg -g '*.java' -B 2 -A 2 'interface.*Repository.*{|@Query.*count'
# Look for any batch processing or pagination in exercise service
ast-grep --pattern 'class ExerciseService {
$$$
countByCourseIdGroupByType($$$) {
$$$
}
$$$
}'
# Check for any existing database indexes or performance configurations
fd -e java -e xml -e properties 'repository|database|jpa' --exec rg -l 'Index|@Index|hibernate.jdbc.batch|fetch'
Length of output: 3782
Script:
#!/bin/bash
# Check specific repository implementations
rg -g '*.java' -A 5 'countBuildJobsByExerciseIds|countAnswerPostsByPostIdIn|countByCourse_Id'
# Look for any JPA configurations
rg -g '*.properties' -g '*.yml' 'spring.jpa|hibernate'
# Check exercise service implementation
rg -g '*.java' -A 10 'countByCourseIdGroupByType.*\{'
Length of output: 10824
src/main/java/de/tum/cit/aet/artemis/exercise/repository/ExerciseRepository.java (1)
632-647: Well-implemented query with good performance considerations!
The implementation efficiently retrieves exercise counts by type through a direct join from the course side, which is optimal given that there are fewer courses than exercises. The explicit type filtering and use of a strongly-typed DTO also ensures type safety.
src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseService.java (1)
9-9: LGTM: Import statements are appropriate.
The added imports are necessary for the new functionality and follow the coding guidelines.
Also applies to: 67-67, 71-71
src/main/java/de/tum/cit/aet/artemis/core/service/CourseService.java (3)
99-99: Import statement added for ExerciseType
The import of ExerciseType is necessary for the new code additions and is appropriately included.
471-472: Extracted courseId for clarity and reuse
Defining courseId as a local variable improves readability and avoids multiple calls to course.getId().
489-490: Verified CourseDeletionSummaryDTO constructor parameters
The updated constructor parameters match the added counts, ensuring the deletion summary includes all necessary information.
src/main/java/de/tum/cit/aet/artemis/exercise/dto/ExerciseTypeCount.java
Outdated
Show resolved
Hide resolved
...in/webapp/app/course/manage/course-management-tab-bar/course-management-tab-bar.component.ts
Show resolved
Hide resolved
github-actions
bot
added
the
deployment-error
sachmii
added
deploy:artemis-test1
and removed
deployment-error
github-actions
bot
added
the
deployment-error
sachmii
removed
the
deployment-error
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseService.java (1)
836-847: Consider adding parameter validation and @NotNull annotation.The implementation is clean and efficient, making good use of streams to transform the data. It properly handles cases where some exercise types have no exercises by defaulting to 0.
Consider these improvements:
- Add
@NotNullannotation for thecourseIdparameter- Add parameter validation to handle null courseId gracefully
- public Map<ExerciseType, Long> countByCourseIdGroupByType(Long courseId) { + public Map<ExerciseType, Long> countByCourseIdGroupByType(@NotNull Long courseId) { + if (courseId == null) { + throw new IllegalArgumentException("courseId must not be null"); + } Map<ExerciseType, Long> exerciseTypeCountMap = exerciseRepository.countByCourseIdGroupedByType(courseId).stream() .collect(Collectors.toMap(ExerciseTypeCount::getExerciseType, ExerciseTypeCount::getCount)); return Arrays.stream(ExerciseType.values()).collect(Collectors.toMap(type -> type, type -> exerciseTypeCountMap.getOrDefault(type, 0L))); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseService.java(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseService.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 (1)
src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseService.java (1)
9-9: LGTM: Import statements are appropriate and well-organized.
The added imports are necessary for the new functionality and follow the project's conventions.
Also applies to: 67-67, 71-71
|
There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions. |
ole-ve
added
ready to merge
and removed
lock:artemis-test3
lock:artemis-test6
ready for review
labels
ole-ve
dismissed stale reviews from muradium, coolchock, SimonEntholzer, and coderabbitai[bot]
via
444cd80
Feras797
temporarily deployed
to
artemis-test2.artemis.cit.tum.de
GitHub Actions
Inactive
Checklist
General
Server
Client
Motivation and Context
Fixes #9722. On the course DTO, there are some fields that have no assigned values on the course management page.
These changes load the required values from the server.
Steps for Testing
Create a course with exercises for each type, lectures, and exams. Verify that all of these values are correct.
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
Code Review
Manual Tests
Summary by CodeRabbit
New Features
Technical Improvements
The changes provide a more comprehensive overview of course content before deletion, giving users more insight into the course's components.