[MBL-19605][Student] Fix quiz submission attempt display order #3465
Conversation
🧪 Unit Test Results✅ 📱 Student App
✅ 🌅 Horizon
✅ 📦 Submodules
📊 Summary
Last updated: Mon, 12 Jan 2026 11:22:12 GMT |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary
This PR improves how submission attempts are numbered by using the actual attempt field from submission objects instead of calculating from array indices. This is a good change that should make attempt numbering more reliable and accurate.
Positive Feedback
- ✅ Good fix to use actual attempt numbers instead of index-based calculation
- ✅ Consistent implementation across both
SubmissionDetailsViewandAssignmentDetailsViewModel - ✅ Test coverage updated to include attempt numbers in test data
- ✅ The
sortedByDescendingensures proper ordering by attempt number
Issues Found
- Type clarity concern in
SubmissionDetailsView.kt:236- ThePair<Long, String>structure makes it unclear what.firstrepresents. Consider using a named data class for better code readability. - Null safety consideration in
AssignmentDetailsViewModel.kt:450- Thesubmission.attemptfield should be validated to handle potential null or zero values that could cause display issues.
Additional Observations
Code Quality: The change is straightforward and follows Kotlin idioms. The shift from mapIndexed to map and from reversed() to sortedByDescending are both appropriate for this use case.
Test Coverage: Good that test data was updated to include attempt numbers. Consider adding an edge case test for submissions with null or zero attempt numbers.
Performance: The sorting operation is minimal impact given typical submission history sizes.
Security: No security concerns identified.
Overall, this is a solid improvement with minor suggestions for enhancement around type safety and null handling.
...m/instructure/student/mobius/assignmentDetails/submissionDetails/ui/SubmissionDetailsView.kt
Show resolved
Hide resolved
...n/java/com/instructure/pandautils/features/assignments/details/AssignmentDetailsViewModel.kt
Outdated
Show resolved
Hide resolved
tamaskozmer
changed the title
📊 Code Coverage Report
|
|
Student Install Page |
There was a problem hiding this comment.
Code Review - Attempt Numbering Fix
This PR fixes the submission attempt numbering by using actual attempt values from the API instead of calculating from array indices. The changes look good overall.
Summary of Changes:
- ✅ Fixed submission attempt display to use actual attempt numbers
- ✅ Updated mock canvas to generate sequential attempt numbers
- ✅ Consistent changes across ViewModel and View layers
- ✅ Added IPv4 preference to GitHub workflows (unrelated but good)
- ✅ Updated tests with explicit attempt numbers
Minor Suggestions:
I've left inline comments on potential edge cases around null handling and robustness improvements, but these are optional enhancements. The core logic is sound.
Testing:
The test updates look good and verify the new behavior correctly.
.../src/main/kotlin/com/instructure/canvas/espresso/mockcanvas/endpoints/SubmissionEndpoints.kt
Show resolved
Hide resolved
...n/java/com/instructure/pandautils/features/assignments/details/AssignmentDetailsViewModel.kt
Outdated
Show resolved
Hide resolved
...m/instructure/student/mobius/assignmentDetails/submissionDetails/ui/SubmissionDetailsView.kt
Show resolved
Hide resolved
Test plan:
refs: MBL-19605
affects: Student
release note: Fixed quiz submission attempts displaying in reverse order
Checklist
🤖 Generated with Claude Code
Co-Authored-By: Claude [email protected]