[MBL-19895][Student] Fix announcement unread badge not clearing after viewing#3621
[MBL-19895][Student] Fix announcement unread badge not clearing after viewing#3621kristofnemere merged 1 commit intomasterfrom
Conversation
… viewing The new WebView-based discussion/announcement details screen loaded with embed=true, which prevents Canvas from automatically marking entries as read server-side. Added an explicit call to PUT read_all after loading the discussion to mark the topic and all its entries as read. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
![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 adds a markAllDiscussionTopicEntriesRead API endpoint and calls it automatically when a discussion is loaded in the WebView, which is a clean and useful improvement.
Positive aspects:
- The new Retrofit endpoint in
DiscussionAPIfollows the existing suspend/DataResultconventions correctly. - The API is placed logically next to the related
markDiscussionTopicReadandmarkDiscussionTopicEntryReadendpoints. - The test scaffolding was updated to include the new mock dependency.
Issues found:
-
Silent failure:
DataResultreturn value discarded (DiscussionDetailsWebViewViewModel.kt:72) — The result ofmarkAllDiscussionTopicEntriesReadis not checked. ADataResult.Failresponse won't throw and won't be caught by the surroundingtry/catch, so failures are silently ignored. At minimum, log failures or make the fire-and-forget intent explicit. -
Pattern inconsistency: raw API interface injected into ViewModel (
DiscussionDetailsWebViewViewModel.kt:44) — All other network calls in this ViewModel go through Manager classes (DiscussionManager,OAuthManager). InjectingDiscussionAPI.DiscussionInterfacedirectly bypasses that abstraction layer. Consider adding the method toDiscussionManagerinstead. -
Missing test assertion for the new behavior (
DiscussionDetailsWebViewViewModelTest.kt:113) — The success test doesn't verify thatmarkAllDiscussionTopicEntriesReadis called. AcoVerifyshould be added to confirm the call happens with the correct arguments, and a test for the failure path would be valuable. -
Nit:
isForceReadFromNetwork = truehas no effect on a PUT (DiscussionDetailsWebViewViewModel.kt:71) — This flag is relevant for caching GET responses; it's a no-op for mutating requests. Consider using plainRestParams()to avoid confusion.
🧪 Unit Test Results✅ 📦 Submodules
📊 Summary
Last updated: Tue, 07 Apr 2026 09:11:56 GMT |
📊 Code Coverage Report✅ Student
✅ Teacher
✅ Pandautils
📈 Overall Average
|
|
Student Install Page |
4 participants
Test plan:
refs: MBL-19895
affects: Student
release note: Fixed an issue where the unread badge on announcements would remain visible even after reading the announcement and its replies.
What changed
The new WebView-based discussion/announcement detail screen (
DiscussionDetailsWebViewFragment) loads the page withembed=true, which prevents Canvas from automatically marking the discussion topic and its entries as read server-side. Added an explicit API call toPUT .../discussion_topics/{topicId}/read_allwhen the detail screen loads, which marks both the topic header and all reply entries as read.Checklist