[MBL-19894][Student] Fix dashboard course widget not showing all unread announcements#3614
Conversation
![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 correctly identifies that the Canvas API can return readState in varying cases (e.g. "unread" vs "UNREAD") and adds case-insensitive comparison to handle this. The added test case in LoadCourseAnnouncementsUseCaseTest is a good demonstration of the intent.
Issues Found
-
Null safety bug (
LoadCourseAnnouncementsUseCase.kt:38):announcement.readState.equals(...)is called on a nullableString?field, which will throwNullPointerExceptionat runtime ifreadStateisnull. The fix inDiscussionListHolder.ktcorrectly uses?.equals() == true— the same pattern should be applied here. (See inline comment) -
Indentation regression (
LoadCourseAnnouncementsUseCase.kt:38): The filter lambda body was dedented from 12 to 8 spaces, breaking the surrounding code style. (See inline comment) -
statusproperty not used (both files):DiscussionTopicHeaderalready exposes a typedstatus: ReadStatecomputed property that maps the rawreadStatestring to the enum. Usingannouncement.status == DiscussionTopicHeader.ReadState.UNREADin both places would be cleaner, type-safe, and avoid the raw string comparison and null handling entirely. (See inline comments) -
Missing newline at end of file: Both
LoadCourseAnnouncementsUseCase.ktandLoadCourseAnnouncementsUseCaseTest.ktare missing a trailing newline (shown as\ No newline at end of filein the diff).
Positive Feedback
The test added in LoadCourseAnnouncementsUseCaseTest is well-structured — it exercises both "unread" and "Unread" variants and correctly asserts that the "read" item is excluded. The null-safe ?.equals() == true pattern used in DiscussionListHolder.kt is the right approach for nullable strings.
| // Filter for unread announcements only | ||
| return announcements.filter { announcement -> | ||
| announcement.unreadCount > 0 || announcement.readState == DiscussionTopicHeader.ReadState.UNREAD.name | ||
| announcement.unreadCount > 0 || announcement.readState.equals(DiscussionTopicHeader.ReadState.UNREAD.name, ignoreCase = true) |
There was a problem hiding this comment.
Null safety bug: readState is declared as String? (nullable), so calling .equals() directly will throw a NullPointerException at runtime if the API returns null for this field.
Use the safe-call pattern (consistent with how DiscussionListHolder.kt handles this in the same PR):
announcement.unreadCount > 0 || announcement.readState?.equals(DiscussionTopicHeader.ReadState.UNREAD.name, ignoreCase = true) == trueAlternatively, DiscussionTopicHeader already exposes a typed status: ReadState computed property (lines 147–157 of the model) that maps the raw string — using it would eliminate the null concern and the string comparison entirely:
announcement.unreadCount > 0 || announcement.status == DiscussionTopicHeader.ReadState.UNREAD
🧪 Unit Test Results✅ 📱 Student App
✅ 🌅 Horizon
✅ 📦 Submodules
📊 Summary
Last updated: Thu, 02 Apr 2026 11:46:53 GMT |
📊 Code Coverage Report✅ Student
✅ Teacher
✅ Pandautils
📈 Overall Average
|
|
Student Install Page |
tamaskozmer
deleted the
MBL-19894-Dashboard-course-widget-does-not-shows-all-unread-announcements
branch
4 participants
Summary
readStatefieldLoadCourseAnnouncementsUseCaseto properly detect unread announcements when the API returns lowercase "unread" statusDiscussionListHolderto show the unread indicator whenreadStateis "unread" (case-insensitive)Test plan
refs: MBL-19894
affects: Student
release note: Fixed an issue where the dashboard course widget was not showing all unread announcements
🤖 Generated with Claude Code
Co-Authored-By: Claude [email protected]