[MBL-19627][Teacher] Fix quiz filtering to include New Quizzes #3461
+27
−7
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
This PR refactors quiz detection logic and enhances test coverage for assignment grouping. The changes look good overall with some points to verify.
Positive Feedback
✅ Good refactoring: Using isQuiz() method instead of direct string comparison improves encapsulation and maintainability
✅ Excellent test coverage: Added test case for New Quizzes (LTI-based quizzes) which is important for modern Canvas quiz handling
✅ Test data improvements: Added position fields to AssignmentGroup test fixtures for better test accuracy
Issues to Address
- Test sorting behavior change (line 567): Verify the removal of
.reversed()is intentional and not masking a regression in assignment group ordering - Position field consistency: Consider whether
positionfields should be added to other test fixtures in the file for consistency - New Quizzes detection verification: Confirm that
isQuiz()correctly identifies external tool quizzes by URL pattern
Code Quality Assessment
- Best Practices: ✅ Good - Proper encapsulation with method extraction
- Test Coverage: ✅ Good - New test case added for expanded functionality
- Potential Bugs:
⚠️ Low risk - Verify sorting behavior change is correct - Performance: ✅ No concerns - No performance impact
- Security: ✅ No concerns identified
Recommendations
- Add a comment in the test explaining that
assignment4represents "New Quizzes" for clarity - Verify the
isQuiz()implementation handles both classic quizzes and LTI-based New Quizzes correctly - Confirm the test assertion changes reflect actual behavior and not test fixes
Great work on improving code quality and test coverage!
...est/java/com/instructure/pandautils/features/assignments/list/AssignmentListViewModelTest.kt
Show resolved
Hide resolved
...est/java/com/instructure/pandautils/features/assignments/list/AssignmentListViewModelTest.kt
Show resolved
Hide resolved
...est/java/com/instructure/pandautils/features/assignments/list/AssignmentListViewModelTest.kt
Show resolved
Hide resolved
tamaskozmer
changed the title
📊 Code Coverage Report✅ Student
✅ Teacher
|
🧪 Unit Test Results✅ 📱 Student App
✅ 📱 Teacher App
✅ 🌅 Horizon
✅ 📦 Submodules
📊 Summary
Last updated: Mon, 05 Jan 2026 10:53:28 GMT |
|
Teacher Install Page |
|
Student Install Page |
kristofnemere
approved these changes
Jan 6, 2026
tamaskozmer
deleted the
MBL-19627-correct-quiz-filtering-on-assignment-list
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Test plan
refs: MBL-19627
affects: Teacher
release note: Fixed quiz filtering to properly include New Quizzes in the Quizzes group
Checklist
🤖 Generated with Claude Code
Co-Authored-By: Claude noreply@anthropic.com