Assessment: Show grade suggestion based on assessment results
#608
Conversation
…ate props handling
…mat in StudentScoreBadge
…rop and adjust rendering logic
WalkthroughThe changes introduce a dedicated Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AssessmentCompletion
participant GradeSuggestion
participant validateGrade
User->>AssessmentCompletion: Interacts with grade suggestion UI
AssessmentCompletion->>GradeSuggestion: Renders with props (gradeSuggestion, studentScore, etc.)
User->>GradeSuggestion: Selects a grade
GradeSuggestion->>AssessmentCompletion: Calls onGradeSuggestionChange(newGrade)
AssessmentCompletion->>validateGrade: validateGrade(newGrade)
validateGrade-->>AssessmentCompletion: Returns validation result
AssessmentCompletion->>AssessmentCompletion: Updates state, triggers save if valid
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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
CodeRabbit Configuration File (
|
![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 (3)
clients/assessment_component/src/assessment/pages/AssessmentPage/components/AssessmentCompletion/utils/validateGrade.tsx (1)
1-33: Well-structured grade validation utility.The implementation correctly handles edge cases including empty inputs, invalid numbers, range validation, and floating-point precision issues. The use of tolerance (0.01) for floating-point comparison is a good practice.
Consider documenting the default value behavior (5.0 for empty input) more explicitly, as this might not be immediately obvious to users of this utility.
+// Returns default grade of 5.0 (fail) when input is empty or whitespace-only export const validateGrade = ( gradeString: string, ): { isValid: boolean; value?: number; error?: string } => {clients/assessment_component/src/assessment/pages/components/StudentScoreBadge.tsx (2)
23-29: Simplify the config determination logic.The ternary chain can be simplified for better readability since you already have an early return when both props are undefined.
- const config = getLevelConfig( - scoreLevel - ? scoreLevel - : scoreNumeric - ? mapNumberToScoreLevel(scoreNumeric) - : ScoreLevel.VeryBad, - ) + const config = getLevelConfig( + scoreLevel ?? mapNumberToScoreLevel(scoreNumeric!) + )Since you've already handled the case where both are undefined with an early return, the fallback to
ScoreLevel.VeryBadis unreachable.
43-45: Simplify the badge content display logic.The conditional logic for displaying badge content can be simplified for better readability.
- {scoreLevel ? config.title : ''} - {scoreLevel && scoreNumeric ? ` (${scoreNumeric.toFixed(1)})` : ''} - {!scoreLevel && scoreNumeric ? `${scoreNumeric.toFixed(1)}` : ''} + {scoreLevel && config.title} + {scoreNumeric && (scoreLevel ? ` (${scoreNumeric.toFixed(1)})` : scoreNumeric.toFixed(1))}This reduces duplication and makes the logic clearer.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
clients/assessment_component/src/assessment/pages/AssessmentPage/components/AssessmentCompletion/AssessmentCompletion.tsx(3 hunks)clients/assessment_component/src/assessment/pages/AssessmentPage/components/AssessmentCompletion/components/GradeSuggestion.tsx(1 hunks)clients/assessment_component/src/assessment/pages/AssessmentPage/components/AssessmentCompletion/utils/validateGrade.tsx(1 hunks)clients/assessment_component/src/assessment/pages/AssessmentPage/components/AssessmentProfile.tsx(2 hunks)clients/assessment_component/src/assessment/pages/AssessmentPage/components/AssessmentStatusBadge.tsx(1 hunks)clients/assessment_component/src/assessment/pages/AssessmentPage/components/CategoryAssessment.tsx(1 hunks)clients/assessment_component/src/assessment/pages/components/GradeSuggestionBadge.tsx(1 hunks)clients/assessment_component/src/assessment/pages/components/StudentScoreBadge.tsx(1 hunks)servers/assessment/assessments/scoreLevel/scoreLevelDTO/studentScore.go(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: test-servers / test (assessment)
- GitHub Check: build-clients / clients-base / Build Docker Image for ghcr.io/ls1intum/prompt2/prompt-clients-base
- GitHub Check: build-clients / clients-base / Build Docker Image for ghcr.io/ls1intum/prompt2/prompt-clients-base
- GitHub Check: Analyze (go)
🔇 Additional comments (28)
clients/assessment_component/src/assessment/pages/AssessmentPage/components/AssessmentStatusBadge.tsx (1)
10-14: Export style updated to named export
The component has been changed from a default export to a named export, which improves consistency and tree-shaking. Ensure no leftover default imports for this component remain.To verify, run:
#!/bin/bash # Verify no default imports of AssessmentStatusBadge remain rg -n "import\s+AssessmentStatusBadge\s+from"clients/assessment_component/src/assessment/pages/AssessmentPage/components/CategoryAssessment.tsx (1)
8-8: Import adjusted to named export
The import ofAssessmentStatusBadgehas been updated to a named import to match the component’s export. This aligns usage and avoids unresolved imports.servers/assessment/assessments/scoreLevel/scoreLevelDTO/studentScore.go (1)
9-9: LGTM: JSON field naming consistency improvement.The change from
"numericScore"to"scoreNumeric"aligns the backend JSON serialization with frontend component prop naming, ensuring consistent data contracts between backend and frontend layers.clients/assessment_component/src/assessment/pages/AssessmentPage/components/AssessmentProfile.tsx (2)
3-12: Good import organization and component integration.The import reordering follows TypeScript conventions by placing type imports first, and the addition of component imports is well-structured. The integration with the new GradeSuggestionBadge component is clean.
46-49: Correct prop addition for enhanced badge functionality.The addition of the
scoreNumericprop toStudentScoreBadgecorrectly utilizes the updated data structure and enables the enhanced tooltip functionality described in the PR objectives.clients/assessment_component/src/assessment/pages/AssessmentPage/components/AssessmentCompletion/components/GradeSuggestion.tsx (3)
17-29: Well-structured component interface.The props interface is clean and the component signature follows React best practices with proper typing and default values.
34-42: Smart conditional rendering for platform recommendations.The condition
studentScore.scoreNumeric > 0appropriately determines when to show platform recommendations, providing contextual guidance to users.
45-62: Verify grade values consistency with validation utility.The dropdown options should match the valid grade values defined in the validation utility to ensure consistency across the application.
#!/bin/bash # Verify that grade values in GradeSuggestion component match validateGrade utility echo "Grade values in validateGrade utility:" rg -A 1 "validGradeValues.*=" clients/assessment_component/src/assessment/pages/AssessmentPage/components/AssessmentCompletion/utils/validateGrade.tsx echo -e "\nGrade values in GradeSuggestion SelectItems:" rg "SelectItem value=" clients/assessment_component/src/assessment/pages/AssessmentPage/components/AssessmentCompletion/components/GradeSuggestion.tsxclients/assessment_component/src/assessment/pages/components/GradeSuggestionBadge.tsx (3)
14-14: Good prop type simplification.Changing from
number | nulltonumber?makes the interface cleaner and more idiomatic for optional props.
18-22: Clean logic simplification.The early return pattern and use of
mapNumberToScoreLevelutility improves code readability and maintainability compared to nested ternary operations.
26-40: Excellent tooltip implementation for better UX.The tooltip provides clear context about the grade suggestion's purpose, and the
cursor-helpclass gives users a visual cue that additional information is available. The 250ms delay duration provides a good balance between responsiveness and avoiding accidental triggers.clients/assessment_component/src/assessment/pages/components/StudentScoreBadge.tsx (6)
14-16: Props interface looks good with optional typing.The interface correctly makes both props optional, allowing for flexible usage patterns. The naming is consistent with the backend changes mentioned in the AI summary.
18-21: Early return prevents unnecessary rendering.Good defensive programming practice to return
undefinedwhen no score data is provided, preventing empty badge rendering.
40-46: Badge content logic handles multiple display scenarios correctly.The conditional rendering logic appropriately handles:
- Score level with optional numeric value
- Numeric-only display when no score level is provided
The formatting with
.toFixed(1)ensures consistent numeric display.
40-40: Consider improving hover cursor semantics.The
cursor-helpclass correctly indicates that help information is available via tooltip, which is good for accessibility.
23-29: ```shell
#!/bin/bashDisplay the ScoreLevel enum and mapNumberToScoreLevel implementation
sed -n '1,100p' clients/assessment_component/src/assessment/interfaces/scoreLevel.ts
--- `36-52`: **LGTM! Well-implemented tooltip functionality.** The tooltip implementation follows React patterns correctly with appropriate delay, positioning, and accessible content. The styling enhancements with `cursor-help` provide good UX. </details> <details> <summary>clients/assessment_component/src/assessment/pages/AssessmentPage/components/AssessmentCompletion/AssessmentCompletion.tsx (11)</summary> `35-39`: **Function conversion and prop destructuring look good.** The conversion to arrow function maintains the same functionality while following modern React patterns. Removing the default value for `deadline` is appropriate if it's handled elsewhere. --- `75-79`: **Grade validation integration is well-implemented.** The validation logic correctly: - Uses the extracted utility function - Handles validation errors appropriately - Prevents proceeding with invalid grades Good separation of concerns. --- `104-108`: **Consistent validation pattern in save handler.** The same validation pattern is correctly applied in the save handler, maintaining consistency across both code paths. --- `110-117`: **Inline object creation simplifies the code.** Removing the intermediate `completionData` variable reduces unnecessary complexity while maintaining readability. --- `148-156`: **New GradeSuggestion component integration looks well-structured.** The component receives appropriate props: - `studentScore` for display context - `gradeSuggestion` for current state - `onGradeSuggestionChange` with proper state update and save logic - `disabled` state for completion handling The callback correctly triggers both state update and save operation. --- `151-154`: ```shell #!/bin/bash # Inspect the GradeSuggestion component to verify its props interface sed -n '1,200p' clients/assessment_component/src/assessment/pages/AssessmentPage/components/AssessmentCompletion/components/GradeSuggestion.tsx
27-27: LGTM! Good refactoring to use centralized validation.Extracting grade validation into a utility function improves code organization and reusability.
21-21: LGTM! Component extraction improves modularity.Using the
GradeSuggestioncomponent to encapsulate grade selection UI is a good architectural decision that improves code organization.
35-39: Function converted to arrow function.The conversion from function declaration to arrow function is consistent, though note that the
deadlineprop no longer has a default value.
110-117: LGTM! Simplified mutation call.Removing the intermediate
completionDataobject and passing the data inline simplifies the code while maintaining the same functionality.
148-156: LGTM! Clean integration of GradeSuggestion component.The integration passes the necessary props correctly and maintains the existing save behavior through the change handler.
|
LGTM |
📝 Show grading suggestion based on assessment results
Warning
Depends on #607 due to db migrations
✨ What is the change?
📌 Reason for the change / Link to issue
closes #605
🧪 How to Test
GradeSuggestionBadgeand theStudentScoreBadge🖼️ Screenshots (if UI changes are included)
✅ PR Checklist
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Refactor