Assessment: Fix an issue where instructors accidentally override presentation scores
#10143
Conversation
github-actions
bot
added
server
|
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/web/CourseResource.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. WalkthroughThe pull request involves modifications to the course management system across multiple files. Changes include removing getter and setter methods for posts in the Changes
Suggested Labels
Suggested Reviewers
Finishing Touches
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
🧹 Nitpick comments (2)
src/main/webapp/app/course/manage/course-update.component.ts (2)
298-301: Good type safety improvement, but consider addressing the technical debt.The explicit type casting and preservation of
presentationScoreeffectively prevents accidental overrides. However, the TODO comment suggests a better long-term solution.Consider creating a follow-up task to move
presentationScoretogradingScalefor a more maintainable solution. This would eliminate the need for special handling in the save method.
304-309: LGTM! Clear and consistent configuration handling.The logic for setting
courseInformationSharingConfigurationis well-structured and handles all states appropriately.Consider using an early return pattern to reduce nesting:
-if (this.communicationEnabled && this.messagingEnabled) { - course.courseInformationSharingConfiguration = CourseInformationSharingConfiguration.COMMUNICATION_AND_MESSAGING; -} else if (this.communicationEnabled && !this.messagingEnabled) { - course.courseInformationSharingConfiguration = CourseInformationSharingConfiguration.COMMUNICATION_ONLY; -} else { - this.communicationEnabled = false; - course.courseInformationSharingConfiguration = CourseInformationSharingConfiguration.DISABLED; -} +if (this.communicationEnabled && this.messagingEnabled) { + course.courseInformationSharingConfiguration = CourseInformationSharingConfiguration.COMMUNICATION_AND_MESSAGING; + return; +} + +if (this.communicationEnabled && !this.messagingEnabled) { + course.courseInformationSharingConfiguration = CourseInformationSharingConfiguration.COMMUNICATION_ONLY; + return; +} + +this.communicationEnabled = false; +course.courseInformationSharingConfiguration = CourseInformationSharingConfiguration.DISABLED;
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/de/tum/cit/aet/artemis/core/domain/Course.java(0 hunks)src/main/java/de/tum/cit/aet/artemis/core/web/CourseResource.java(2 hunks)src/main/webapp/app/course/manage/course-update.component.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/de/tum/cit/aet/artemis/core/domain/Course.java
🧰 Additional context used
📓 Path-based instructions (2)
src/main/webapp/app/course/manage/course-update.component.ts (1)
src/main/java/de/tum/cit/aet/artemis/core/web/CourseResource.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
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: client-tests
- GitHub Check: server-tests
- GitHub Check: Build and Push Docker Image
- GitHub Check: Analyse
🔇 Additional comments (2)
src/main/java/de/tum/cit/aet/artemis/core/web/CourseResource.java (2)
192-194: LGTM!The constructor parameter reordering and reformatting improves readability while maintaining the same functionality.
1334-1334: Verify the direct repository usage impact.The change from using
gradingScaleServiceto directly usinggradingScaleRepositorysimplifies the code but bypasses the service layer. While this works, it's worth verifying that we're not losing any important business logic that was previously handled by the service.Run this script to check for any business logic in the removed service that we might be missing:
Checklist
General
Server
Client
authoritiesto all new routes and checked the course groups for displaying navigation elements (links, buttons).Changes affecting Programming Exercises
Motivation and Context
Description
Steps for Testing
Prerequisites:
Exam Mode 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
Performance Tests
Test Coverage
Screenshots
Summary by CodeRabbit
Code Refactoring
Technical Improvements