[MBL-19553][Student] Android Student ToDo list QA fixes and test coverage #3408
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
Fixed issue #1: Empty state only appeared after manual refresh when all ToDo items were marked as done via checkboxes. Changes: - Added applyFiltersLocally() call in startCheckboxDebounceTimer() after items are added to removingItemIds for animation - This ensures itemsByDate is updated to reflect filtered completed items - Empty state now appears automatically when last item is completed Test plan: - Mark all visible ToDo items as done using checkboxes - Verify empty state appears automatically after animation (~1 second) - Test with various filter combinations (Show Completed on/off) - Verify both swipe-to-done and checkbox trigger empty state correctly 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Fixed issue #11: Account-level calendar events were clickable when they shouldn't be. Changes: - Added isClickable field to ToDoItemUiState (defaults to true) - In ToDoListViewModel.mapToUiState(), detect account-level events by checking if contextType equals "Account" (case-insensitive) - Set isClickable to false for account-level calendar events - In ToDoListScreen.ToDoItemContent(), conditionally apply clickable modifier based on isClickable flag using Modifier.then() Account-level calendar events now appear in the ToDo list but are not clickable, preventing navigation to invalid URLs. Test plan: - Create account-level calendar events - Verify they appear in ToDo list - Verify clicking on them does nothing (no navigation) - Verify other items (course/user events, assignments, etc.) remain clickable - Verify checkbox and swipe-to-done still work on account events 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Fixed multiple issues where ToDo list didn't automatically refresh after changes in various parts of the app: Changes: - ToDoListViewModel: Added CalendarSharedEvents listener to refresh on RefreshToDoList events - EditDashboardViewModel: Emit RefreshToDoList when favoriting/ unfavoriting courses to update filtered ToDo items - SubmissionWorker: Emit RefreshToDoList after successful assignment submissions to remove completed items This fixes the following QA findings from MBL-19553: - Issue #2: Badge updates but page doesn't refresh after creating events/todos - Issue #4: Favorite courses filter doesn't auto-update - Issue #5: Assignment submission doesn't refresh ToDo page - Issue #9: ToDo created from widget appears on widget but not page - Issue #10: Deleted event/ToDo doesn't disappear from page Test plan: - Create/update/delete calendar events and verify ToDo list refreshes - Favorite/unfavorite courses and verify filtered ToDos update - Submit assignments and verify they disappear from ToDo list - Create ToDos from widget and verify they appear in list - Delete events/ToDos and verify they disappear immediately 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Fixed issue where panda illustrations incorrectly appeared at the bottom of ToDo list after rotating from landscape to portrait mode. Root cause: - itemPositions and itemSizes maps survived configuration changes with stale values from previous orientation - listContentHeight calculation used these stale values, making it smaller than actual content height in new orientation - This caused availableSpacePx to be larger than reality, making showPandas condition incorrectly evaluate to true Solution: - Added LocalConfiguration.current as key to remember() blocks for itemPositions and itemSizes maps - Maps now reset on any configuration change (orientation, etc.) - All items remeasured with correct values for new layout - Panda visibility logic uses accurate data immediately This fixes Issue #3 from MBL-19553: - Panda avatars appearing incorrectly after orientation change Test plan: - Open ToDo list with some items (not enough to fill entire screen) - Rotate device from portrait to landscape - Rotate back to portrait - Verify pandas don't appear unless scrolling reveals actual space - Verify pandas appear correctly when there is genuine space at bottom 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Added comprehensive unit tests for the QUIZ type in isComplete() function to verify Issue #7 fix (classic quiz doesn't disappear after submission). Tests added: - isComplete returns true for QUIZ when submitted - isComplete returns false for QUIZ when not submitted - isComplete returns false for QUIZ with null submission state - isComplete returns true for QUIZ with plannerOverride marked complete These tests ensure that quizzes behave the same as assignments when determining completion status, preventing regression of the fix where completed quizzes now properly disappear from the ToDo list. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Fixed all test compilation errors caused by adding CalendarSharedEvents parameter to EditDashboardViewModel constructor. Changes: - Added CalendarSharedEvents import - Added calendarSharedEvents mock instance - Updated all 31 EditDashboardViewModel constructor calls to include the calendarSharedEvents parameter This ensures all existing tests continue to pass after adding the favorite courses refresh functionality (Issue #4). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…ture Added unit tests and Compose UI tests to verify CalendarSharedEvents integration and empty state behavior with removingItemIds filtering. Changes: - Added ToDoListScreenTest.kt with 18 Compose UI tests covering all states (loading, error, empty, with items), filtering, and interactions - Made ToDoListContent internal with testTags for testability - Added tests in ToDoListViewModelTest.kt for: * Account-level calendar events clickability * RefreshToDoList event triggering data reload * Empty state when completing last item via swipe/checkbox - Added tests in EditDashboardViewModelTest.kt for: * RefreshToDoList event when adding/removing courses from favorites Test plan: - Run unit tests: ./gradle/gradlew :libs:pandautils:test - Run UI tests: ./gradle/gradlew :libs:pandautils:connectedDebugAndroidTest - Verify all tests pass 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull Request Review Summary
This PR implements a To-Do List feature with calendar integration for the Canvas Android app. The implementation includes comprehensive test coverage and follows the project's architectural patterns well.
✅ Positive Feedback
- Excellent test coverage: The new
ToDoListScreenTest.ktincludes 20+ comprehensive UI tests covering loading states, error states, empty states, item interactions, and edge cases - Good architectural patterns: Follows MVVM pattern with proper separation of concerns (ViewModel, Repository, UI State)
- Proper dependency injection: Uses Hilt throughout for dependency management
- Comprehensive feature integration: Integrates with calendar events, submission workflows, and dashboard editing
- Good use of Jetpack Compose: Modern UI implementation with proper state management
- Test maintenance: Updates to existing test files properly mock the new
CalendarSharedEventsdependency
🔍 Issues Found
- Memory leak concern in
SubmissionWorker.kt:665- Creating a newCoroutineScopeunnecessarily - Configuration change handling in
ToDoListScreen.kt:331- Overly broad state reset on configuration changes - Missing error handling in
ToDoListViewModel.kt:126- Event collection lacks error handling - Logic simplification in
ToDoListViewModel.kt:196- Account-level event logic could be cleaner
📋 Additional Observations
Code Quality:
- Import organization follows project conventions
- Proper use of test tags for UI testing
- Good handling of edge cases (empty states, filtering, etc.)
- Quiz submission status now properly tracked in
PlannerItemExtensions.kt
Architecture:
- Shared events pattern used appropriately for cross-feature communication
- Filtering logic properly implemented with
removingItemIdsset - Account-level calendar events correctly marked as non-clickable
Testing:
- Unit tests cover new functionality comprehensively
- Test helpers are well-designed and reusable
- Mock setup is clean and follows existing patterns
🎯 Recommendations
- Address the memory leak concern in
SubmissionWorker - Review the configuration change handling to avoid unnecessary state resets
- Add error handling to the calendar event observer
- Consider simplifying the account-level event clickability logic
Overall, this is a solid implementation with good test coverage. The issues identified are relatively minor and should be straightforward to address.
apps/student/src/main/java/com/instructure/student/mobius/common/ui/SubmissionWorker.kt
Outdated
Show resolved
Hide resolved
libs/pandautils/src/main/java/com/instructure/pandautils/features/todolist/ToDoListScreen.kt
Show resolved
Hide resolved
libs/pandautils/src/main/java/com/instructure/pandautils/features/todolist/ToDoListViewModel.kt
Show resolved
Hide resolved
libs/pandautils/src/main/java/com/instructure/pandautils/features/todolist/ToDoListViewModel.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
This PR addresses several ToDo list bugs in the Android Student app and adds comprehensive test coverage. The changes focus on fixing UI state management issues, improving user interaction behavior, and implementing auto-refresh functionality across related features.
Key Changes:
- Fixed empty state display logic by filtering items before checking if the list is empty
- Made account-level calendar events non-clickable since they have no detail page
- Integrated CalendarSharedEvents to auto-refresh ToDo list when courses, calendar events, or submissions change
- Fixed orientation-related bug with panda avatars by using configuration-aware remember keys
- Added quiz completion support to isComplete() logic
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| PlannerItemExtensions.kt | Added QUIZ type to isComplete() logic for checking submission status |
| PlannerItemExtensionsTest.kt | Added comprehensive test coverage for quiz completion scenarios |
| ToDoListViewModel.kt | Integrated CalendarSharedEvents observer and added account-level event clickability logic |
| ToDoListViewModelTest.kt | Added tests for clickability behavior, refresh events, and empty state scenarios |
| ToDoListUiState.kt | Added isClickable boolean property to control item interaction |
| ToDoListScreen.kt | Moved item filtering logic to parent composable for proper empty state detection and fixed orientation bug |
| ToDoListScreenTest.kt | Created comprehensive UI test suite with 18 tests covering all major scenarios |
| EditDashboardViewModel.kt | Integrated CalendarSharedEvents to emit RefreshToDoList when favorites change |
| EditDashboardViewModelTest.kt | Added tests to verify RefreshToDoList event emission on favorite changes |
| ToDoFragment.kt | Added RefreshToDoList event emission on todo create/edit/delete |
| EventFragment.kt | Added RefreshToDoList event emission on calendar event operations |
| SubmissionWorker.kt | Added RefreshToDoList event emission after successful submission upload |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
📊 Code Coverage Report✅ Student
✅ Teacher
✅ Pandautils
📈 Overall Average
|
|
Student Install Page |
…call Fixed potential memory leak by using coroutineScope instead of creating an unmanaged CoroutineScope when sending CalendarSharedEvents. The coroutineScope function properly inherits the Worker's coroutine context and ensures the scope is tied to the Worker's lifecycle. Addresses PR review feedback. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
adamNagy56
requested changes
Nov 24, 2025
Contributor
adamNagy56
left a comment
adamNagy56
left a comment
There was a problem hiding this comment.
QA Findings in Slack thread.
Extended the favorite courses filter logic to properly handle PLANNER_NOTE items that have courseId stored in plannable.courseId when item.courseId is null. Changes: - Updated filterByToDoFilters to check plannable.courseId as fallback for PLANNER_NOTE items when item.courseId is null - item.courseId always takes precedence when set - Added 2 unit tests to verify the fallback logic and priority This ensures PLANNER_NOTE items associated with courses are properly filtered when "favorite courses only" filter is enabled. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Fixed sticky header not hiding and push-up animation not working after rotation to landscape mode. The issue was that rememberStickyHeaderState wasn't tracking changes to itemPositions in its remember key. After rotation, itemPositions gets reset but the derivedStateOf wasn't being recreated, causing it to use stale position data. Added itemPositions to the remember dependencies so the sticky header state recalculates properly when positions are remeasured after rotation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Fixed issue where ToDo badge count showed incorrect number when favorite courses filter was enabled. Root cause: Badge was passing only favorite courses to filterByToDoFilters, causing items from non-favorite courses to not be filtered out (course lookup returned null, so filter couldn't check isFavorite property). Solution: Fetch ALL courses when favorite filter is enabled, then let filterByToDoFilters properly check isFavorite property on each course. This ensures items from non-favorite courses are correctly filtered out. Changes: - Use courseApi.getFirstPageCourses() to fetch all courses - Only fetch courses when favoriteCourses filter is enabled (optimization) - Filter out access restricted and invited courses (matches ViewModel logic) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Fixed issue where panda illustrations were not showing/hiding correctly after filter changes. Root cause: listContentHeight calculation used remember(dateGroups) but depended on itemPositions which is a mutable state map. When filters changed, dateGroups changed but itemPositions for new items weren't measured yet, causing stale values and incorrect showPandas calculation. Solution: Added itemPositions to remember dependencies, ensuring listContentHeight recalculates when item positions are updated after filter changes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Extended the calendar ToDo list refresh to trigger after successful media and file submissions, not just text/URL/annotation submissions. Previously only text, URL, and student annotation submissions refreshed the calendar via handleSubmissionResult(). Media and file submissions had their own success paths that didn't trigger the refresh. Changes: - Added coroutineScope calendar refresh call after media submission success - Added coroutineScope calendar refresh call after file submission success - Ensures consistent ToDo list refresh across all submission types 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
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.
Summary
Changes
Bug Fixes
Test Coverage
ToDoListScreenTest.kt (new) - 18 Compose UI tests:
ToDoListViewModelTest.kt - Added 6 tests:
EditDashboardViewModelTest.kt - Added 2 tests:
QuizSubmissionViewModelTest.kt - Added 2 tests:
Test plan
./gradle/gradlew :libs:pandautils:test./gradle/gradlew :libs:pandautils:connectedDebugAndroidTestAffects
Student
🤖 Generated with Claude Code