Fix News & Announcements parity between web and Android#605
Conversation
kckagancan
left a comment
There was a problem hiding this comment.
The product direction of this PR looks correct: Android is now backend-driven for News & Announcements, the old hardcoded mockNews list is removed, web no longer silently falls back to demo announcements, and cached announcements are labeled as cached instead of live. This matches the intent of #598.
However, the PR is not merge-ready yet because the E2E suite is currently failing.
The failing test is web/e2e/news-fallback.spec.js. It still asserts the old behavior:
Using fallback news- demo announcement content such as
Know your nearest gathering area - fallback/demo content shown when the announcements API is unavailable
But this PR intentionally changes that behavior. When the API fails and there is no cache, the expected behavior should now be an explicit error/empty state, not demo announcements shown as fallback content.
Please update the E2E test to match the new product behavior instead of restoring the old fallback behavior.
Suggested update:
- Rename the test from “shows demo announcements...” to something like:
shows explicit error state when news API is unavailable and no cache exists - Keep the
/api/announcements503 route mock. - Assert:
Couldn't load announcementsis visible- the error copy indicates announcements could not be loaded/refreshed
Retry newsis visibleNo announcements could be loaded right now.is visible- old demo announcement titles are NOT visible
- Optionally add a separate test for the cached fallback path:
- seed
localStoragewith cached announcements - make
/api/announcementsfail - assert
Using cached announcements - assert cached content is shown with a warning
- seed
Do not reintroduce demo fallback announcements. The new behavior is correct for #598; the regression is only that the E2E test still expects the old behavior.
Minor non-blocking notes:
- Android
AnnouncementsRepository.fetchAnnouncements()clamps limit to200, while the backend validator allows max100. The current default is100, so this is not currently breaking, but aligning the clamp to100would be cleaner. AnnouncementSummary.hasMoreis currently unused in AndroidNewsScreen.kt; harmless, but it can be removed for cleanliness.
After updating the E2E expectations and getting CI green, I think this PR should be merge-ready.
- update news-fallback spec for no-cache error state - add cached-announcements fallback path e2e coverage - clamp android announcements limit to backend max (100) - remove unused hasMore from Android news summary helper
kckagancan
left a comment
There was a problem hiding this comment.
Thanks for the fixes. Looks good now. Merging.
Summary
This PR fixes cross-platform parity issues in News & Announcements by making both web and Android backend-driven and removing demo/hardcoded announcement content from runtime paths.
What Changed
Android
GET /announcements?limit=100GET /announcements/:announcementIdWeb
Formatting
en-US) for timestamp formatting to keep News dates consistent in English.Acceptance Criteria Mapping
Validation
./gradlew :app:compileDebugKotlin✅npx tsc --noEmit✅Issue
Closes #598