feat: in-app announcements feed for cross-version user communication#498
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a complete Announcements feature: domain models, DTOs, mappers, repository + cache, tweaks persistence, network fetch, view-model, UI (list, critical modal, mute sheet), Profile navigation/wiring, DI and platform datastores, and backend endpoint documentation. ChangesAnnouncements Feature
Sequence DiagramsequenceDiagram
participant User
participant App as App/HomeScreen
participant VM as AnnouncementsViewModel
participant Repo as AnnouncementsRepository
participant Tweaks as TweaksRepository
participant Backend as BackendApiClient
participant UI as AnnouncementsUI
rect rgba(100,150,255,0.5)
Note over User,App: Initial load / refresh
User->>App: launch/navigate
App->>VM: collect feed / observe pendingCriticalAcknowledgment
VM->>Repo: observeFeed() / refresh()
Repo->>Backend: getAnnouncements()
Backend-->>Repo: AnnouncementsResponseDto
Repo->>Repo: cache payload via AnnouncementsCacheStore
Repo->>Tweaks: setAnnouncementsLastFetchedAt()
Repo-->>VM: AnnouncementsFeedSnapshot
VM-->>App: pendingCriticalAcknowledgment emitted
App->>UI: show CriticalAnnouncementModal (if gating passes)
end
rect rgba(150,200,100,0.5)
Note over User,UI: User interactions
User->>UI: acknowledge/dismiss/mute/open CTA
UI->>VM: acknowledge/dismiss/setMuted/openCta
VM->>Repo: acknowledge/dismiss/setMuted
Repo->>Tweaks: persist changes
Tweaks-->>Repo: updated flows
Repo-->>VM: updated snapshot
VM-->>UI: UI updates
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (6)
core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/repository/TweaksRepository.kt (1)
123-125: ⚡ Quick winPrefer
AnnouncementCategoryoverStringin the muted-categories API.The domain contract leaks the DataStore serialization detail (
Set<String>) instead of the typed domain value. IfAnnouncementCategoryentries are ever renamed, persisted strings silently diverge from the enum with no compiler warning.The implementation can handle the
name/valueOfmapping internally:♻️ Proposed refactor
- fun getAnnouncementsMutedCategories(): Flow<Set<String>> + fun getAnnouncementsMutedCategories(): Flow<Set<AnnouncementCategory>> - suspend fun setAnnouncementCategoryMuted(categoryName: String, muted: Boolean) + suspend fun setAnnouncementCategoryMuted(category: AnnouncementCategory, muted: Boolean)The
TweaksRepositoryImplconverts viaAnnouncementCategory.valueOf(name)/category.namebefore reading/writing DataStore, keeping the serialization concern out of the interface.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/repository/TweaksRepository.kt` around lines 123 - 125, Change the API to use the domain type instead of raw strings: update TweaksRepository's method signatures to return Flow<Set<AnnouncementCategory>> from getAnnouncementsMutedCategories() and accept category: AnnouncementCategory in setAnnouncementCategoryMuted(category, muted). Keep DataStore string serialization inside TweaksRepositoryImpl by converting persisted names via AnnouncementCategory.valueOf(name) when reading and using category.name when writing so callers never see raw strings; update any callers/tests to use AnnouncementCategory accordingly.core/presentation/src/commonMain/composeResources/values/strings.xml (1)
1030-1030: 💤 Low value
announcements_dismissis an exact duplicate of the existingdismissstring (line 394).Both resolve to
"Dismiss". Reusing the shared key avoids drift if the copy ever changes.♻️ Proposed fix
- <string name="announcements_dismiss">Dismiss</string>Reference
@string/dismissdirectly in the composable instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/presentation/src/commonMain/composeResources/values/strings.xml` at line 1030, The string resource announcements_dismiss duplicates the existing dismiss string ("Dismiss"); remove the duplicate <string name="announcements_dismiss"> and update any usage sites (the composable(s) that used announcements_dismiss) to reference the shared key dismiss instead (e.g., use stringResource(R.string.dismiss) or `@string/dismiss` as appropriate) so the code uses the single canonical resource name dismiss and avoids drift.core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/model/Announcement.kt (1)
5-6: Consider usingkotlinx.datetime.InstantforexpiresAtrather thanString.
expiresAt: String?defers date parsing to filter time inisPastIso(), where parsing failures silently allow expired announcements to remain visible (line 117 and 131-134 ofAnnouncementsRepositoryImpl.kt). Usingkotlinx.datetime.Instant(orLongepoch millis for consistency withgetAnnouncementsLastFetchedAt) makes the format contract explicit at the type level and moves parse errors to the mapper boundary where they can be handled deliberately.kotlinx-datetimeis already a project dependency (v0.7.1).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/model/Announcement.kt` around lines 5 - 6, The Announcement data model currently declares expiresAt: String? which defers parsing and silently permits invalid dates to pass through; change the type to kotlinx.datetime.Instant? (or Long epoch millis to match getAnnouncementsLastFetchedAt) on the Announcement class (publishedAt/expiresAt fields) and update the mapper that constructs Announcement instances so it parses/raw-converts the incoming string into Instant (handling parse errors explicitly). Then update consumers such as isPastIso() in AnnouncementsRepositoryImpl to accept Instant? (or Long) instead of parsing strings, ensuring parse failures are handled at the mapping boundary rather than inside isPastIso().core/presentation/src/commonMain/kotlin/zed/rainxch/core/presentation/components/announcements/AnnouncementCard.kt (1)
259-265: ⚡ Quick win
categoryLabelis duplicated verbatim inMuteSettingsBottomSheet.kt.Both files define an identical private
categoryLabel(AnnouncementCategory): StringResourcewhen-expression. If a newAnnouncementCategoryentry is added, both copies need updating independently. Extract it to a shared internal file in the package (e.g.,AnnouncementUiHelpers.kt).♻️ Proposed extraction
Create
core/presentation/src/commonMain/kotlin/zed/rainxch/core/presentation/components/announcements/AnnouncementUiHelpers.kt:package zed.rainxch.core.presentation.components.announcements import org.jetbrains.compose.resources.StringResource import zed.rainxch.core.domain.model.AnnouncementCategory import zed.rainxch.githubstore.core.presentation.res.Res import zed.rainxch.githubstore.core.presentation.res.announcements_category_news import zed.rainxch.githubstore.core.presentation.res.announcements_category_privacy import zed.rainxch.githubstore.core.presentation.res.announcements_category_security import zed.rainxch.githubstore.core.presentation.res.announcements_category_status import zed.rainxch.githubstore.core.presentation.res.announcements_category_survey internal fun categoryLabel(category: AnnouncementCategory): StringResource = when (category) { AnnouncementCategory.NEWS -> Res.string.announcements_category_news AnnouncementCategory.PRIVACY -> Res.string.announcements_category_privacy AnnouncementCategory.SURVEY -> Res.string.announcements_category_survey AnnouncementCategory.SECURITY -> Res.string.announcements_category_security AnnouncementCategory.STATUS -> Res.string.announcements_category_status }Then delete the private copies from both
AnnouncementCard.ktandMuteSettingsBottomSheet.kt.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/presentation/src/commonMain/kotlin/zed/rainxch/core/presentation/components/announcements/AnnouncementCard.kt` around lines 259 - 265, Duplicate private categoryLabel(category: AnnouncementCategory) exists in both AnnouncementCard and MuteSettingsBottomSheet; extract it into a single internal helper function in a new shared file (e.g., AnnouncementUiHelpers.kt) inside the same package, implement internal fun categoryLabel(category: AnnouncementCategory): StringResource = when(...) using the existing mappings (AnnouncementCategory.NEWS/PRIVACY/SURVEY/SECURITY/STATUS -> Res.string.announcements_category_*), then replace the private copies in AnnouncementCard and MuteSettingsBottomSheet with imports/usage of this shared categoryLabel and remove the duplicate functions; ensure you import AnnouncementCategory, StringResource and Res.string identifiers in the new helper.composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/Main.kt (1)
174-174: 💤 Low value
canShowCriticalis an exact duplicate ofcanShowWhatsNewLines 147 and 174 compute the same boolean expression. Reusing the existing variable avoids silent drift if the condition ever changes.
♻️ Proposed refactor
- val canShowCritical = onHomeScreen && authSettled && rateLimitCleared + val canShowCritical = canShowWhatsNew val criticalToShow = pendingCritical🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/Main.kt` at line 174, canShowCritical duplicates the exact boolean expression used for canShowWhatsNew (onHomeScreen && authSettled && rateLimitCleared); replace the duplicated expression by reusing canShowWhatsNew (i.e., assign canShowCritical = canShowWhatsNew or remove canShowCritical and use canShowWhatsNew where needed) so the logic isn't repeated and can't drift; update references to canShowCritical accordingly if you choose to remove the variable.core/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/AnnouncementsRepositoryImpl.kt (1)
37-57: 💤 Low value
lastRefreshFailedis alwaysfalse— consider tracking actual failure state.The
lastRefreshFailedproperty inAnnouncementsFeedSnapshotis always set tofalse, even whenrefresh()fails. While the PR objectives mention "silent refresh failure," the snapshot never exposes this state to the UI layer.If you want the UI to optionally show a subtle indicator (e.g., stale-data badge), consider adding a separate flow or state variable to track refresh failures. Otherwise, if silent failure is intentional and the UI should never react to it, you could document this design decision or remove the property from the snapshot.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/AnnouncementsRepositoryImpl.kt` around lines 37 - 57, The snapshot's lastRefreshFailed is hard-coded to false; update observeFeed to combine in a real "refresh failed" boolean instead of a literal false by adding a source that reflects refresh() failure (either a new tweaksRepository.getAnnouncementsLastRefreshFailed() Flow<Boolean> or a MutableStateFlow<Boolean> updated by the refresh() implementation) and include that value when constructing AnnouncementsFeedSnapshot in observeFeed; locate observeFeed and AnnouncementsFeedSnapshot usage and ensure refresh() sets the failure state on the chosen flow (or resets it to false on success) so the UI can read the real lastRefreshFailed value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@core/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/TweaksRepositoryImpl.kt`:
- Around line 343-356: getAnnouncementsCachedPayload and
setAnnouncementsCachedPayload currently store a large JSON blob under
ANNOUNCEMENTS_CACHED_PAYLOAD_KEY in the shared preferences DataStore
(preferences.data / preferences.edit), which forces full-file rewrites and
deserializations on every preference change; refactor by moving the
announcements cache out of the shared Preferences DataStore into a dedicated
storage (either a separate DataStore<Preferences> instance scoped to
announcements or a file-based cache using okio/FileSystem), update
getAnnouncementsCachedPayload to read from that new store and
setAnnouncementsCachedPayload to write/remove from it, and remove usage of
ANNOUNCEMENTS_CACHED_PAYLOAD_KEY from the global preferences to avoid bloating
preferences.data emissions.
In
`@core/presentation/src/commonMain/kotlin/zed/rainxch/core/presentation/components/announcements/AnnouncementCard.kt`:
- Around line 77-88: The severity bar is invisible because the first Box is a
no-op and the second Box collapses to zero height; in the AnnouncementCard
composable remove the dead first Box (the spacer with .height(0.dp)) and make
the colored bar Box span the row's height by replacing its measurement modifiers
with a fixed width (4.dp) and fillMaxHeight() (or matchParentSize equivalent) so
it renders as a vertical bar; ensure the colored Box sits at the start of the
Row and remove any .fillMaxWidth()/.height(0.dp) that cause collapse.
- Around line 176-196: The ExpandableBody composable currently uses
BODY_COLLAPSED_LINES and a character-length check to decide when to show the
"read more" button, causing silent clipping or unnecessary buttons; change
ExpandableBody to track actual visual overflow via Text's onTextLayout (e.g.,
remember a boolean isOverflowing and update it from
layoutResult.hasVisualOverflow), then set Text's maxLines to if (expanded)
Int.MAX_VALUE else BODY_COLLAPSED_LINES and only render the TextButton when
!expanded && isOverflowing; ensure the overflow state is reset/remembered based
on the body parameter so it recalculates when body changes.
In
`@core/presentation/src/commonMain/kotlin/zed/rainxch/core/presentation/components/announcements/AnnouncementsRoot.kt`:
- Around line 63-77: The two IconButton usages in AnnouncementsRoot.kt currently
set contentDescription = null which makes them inaccessible; update the
IconButton for back-navigation (the one invoking onNavigateBack with
Icons.AutoMirrored.Filled.ArrowBack) and the IconButton that sets showMuteSheet
= true (with Icons.Filled.Tune) to provide meaningful contentDescription strings
(e.g., use string resources like "back_button" and "open_mute_settings") and
wire them via stringResource(...) so screen readers can announce them; ensure
the corresponding string keys are added to the localization files if missing.
- Around line 82-94: The empty-state branch currently only checks
items.isEmpty() and shows "No announcements", which swallows refreshFailed;
update the AnnouncementsRoot composable so that when items.isEmpty() &&
refreshFailed is true it shows an error UI (e.g., replace or augment the Box
content with an error Text/message and optional retry affordance) instead of the
silent empty message. Locate the items and refreshFailed usage in
AnnouncementsRoot.kt (the Box/ Text block using innerPadding,
stringResource(Res.string.announcements_empty), MaterialTheme) and add a
conditional to render a failure message (use an appropriate stringResource or
pass through the existing error state) and a retry trigger tied to the existing
refresh logic.
In `@docs/backend/announcements-endpoint.md`:
- Around line 17-19: The fenced code blocks containing the endpoint path "GET
/v1/announcements" and the directory tree starting with "announcements/" lack a
language specifier; update each triple-backtick fence to declare a language (use
"text") so they become ```text and ```text respectively; locate the blocks by
the exact block contents ("GET /v1/announcements" and the "announcements/"
listing) and add the language token to satisfy markdownlint MD040.
- Around line 94-108: Add a new cross-field validation rule that enforces
"severity = CRITICAL → requiresAcknowledgment = true" so critical announcements
always trigger the modal: update the server-side announcements validation (the
same place that enforces "category = SECURITY" and "requiresAcknowledgment =
true → dismissible = false") to reject any announcement where severity ==
CRITICAL and requiresAcknowledgment != true; also update the documentation list
of validation rules to include the new line. Reference the client check in
AnnouncementsRepository.kt (pendingCriticalAcknowledgment) to ensure the
validation matches the modal trigger logic.
---
Nitpick comments:
In `@composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/Main.kt`:
- Line 174: canShowCritical duplicates the exact boolean expression used for
canShowWhatsNew (onHomeScreen && authSettled && rateLimitCleared); replace the
duplicated expression by reusing canShowWhatsNew (i.e., assign canShowCritical =
canShowWhatsNew or remove canShowCritical and use canShowWhatsNew where needed)
so the logic isn't repeated and can't drift; update references to
canShowCritical accordingly if you choose to remove the variable.
In
`@core/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/AnnouncementsRepositoryImpl.kt`:
- Around line 37-57: The snapshot's lastRefreshFailed is hard-coded to false;
update observeFeed to combine in a real "refresh failed" boolean instead of a
literal false by adding a source that reflects refresh() failure (either a new
tweaksRepository.getAnnouncementsLastRefreshFailed() Flow<Boolean> or a
MutableStateFlow<Boolean> updated by the refresh() implementation) and include
that value when constructing AnnouncementsFeedSnapshot in observeFeed; locate
observeFeed and AnnouncementsFeedSnapshot usage and ensure refresh() sets the
failure state on the chosen flow (or resets it to false on success) so the UI
can read the real lastRefreshFailed value.
In
`@core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/model/Announcement.kt`:
- Around line 5-6: The Announcement data model currently declares expiresAt:
String? which defers parsing and silently permits invalid dates to pass through;
change the type to kotlinx.datetime.Instant? (or Long epoch millis to match
getAnnouncementsLastFetchedAt) on the Announcement class (publishedAt/expiresAt
fields) and update the mapper that constructs Announcement instances so it
parses/raw-converts the incoming string into Instant (handling parse errors
explicitly). Then update consumers such as isPastIso() in
AnnouncementsRepositoryImpl to accept Instant? (or Long) instead of parsing
strings, ensuring parse failures are handled at the mapping boundary rather than
inside isPastIso().
In
`@core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/repository/TweaksRepository.kt`:
- Around line 123-125: Change the API to use the domain type instead of raw
strings: update TweaksRepository's method signatures to return
Flow<Set<AnnouncementCategory>> from getAnnouncementsMutedCategories() and
accept category: AnnouncementCategory in setAnnouncementCategoryMuted(category,
muted). Keep DataStore string serialization inside TweaksRepositoryImpl by
converting persisted names via AnnouncementCategory.valueOf(name) when reading
and using category.name when writing so callers never see raw strings; update
any callers/tests to use AnnouncementCategory accordingly.
In `@core/presentation/src/commonMain/composeResources/values/strings.xml`:
- Line 1030: The string resource announcements_dismiss duplicates the existing
dismiss string ("Dismiss"); remove the duplicate <string
name="announcements_dismiss"> and update any usage sites (the composable(s) that
used announcements_dismiss) to reference the shared key dismiss instead (e.g.,
use stringResource(R.string.dismiss) or `@string/dismiss` as appropriate) so the
code uses the single canonical resource name dismiss and avoids drift.
In
`@core/presentation/src/commonMain/kotlin/zed/rainxch/core/presentation/components/announcements/AnnouncementCard.kt`:
- Around line 259-265: Duplicate private categoryLabel(category:
AnnouncementCategory) exists in both AnnouncementCard and
MuteSettingsBottomSheet; extract it into a single internal helper function in a
new shared file (e.g., AnnouncementUiHelpers.kt) inside the same package,
implement internal fun categoryLabel(category: AnnouncementCategory):
StringResource = when(...) using the existing mappings
(AnnouncementCategory.NEWS/PRIVACY/SURVEY/SECURITY/STATUS ->
Res.string.announcements_category_*), then replace the private copies in
AnnouncementCard and MuteSettingsBottomSheet with imports/usage of this shared
categoryLabel and remove the duplicate functions; ensure you import
AnnouncementCategory, StringResource and Res.string identifiers in the new
helper.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2248e81a-85e8-4d50-b5b4-70b8eeb70920
📒 Files selected for processing (27)
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/Main.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/app/announcements/AnnouncementsViewModel.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/app/di/ViewModelsModule.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/app/navigation/AppNavigation.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/app/navigation/GithubStoreGraph.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/app/navigation/NavigationUtils.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/di/SharedModule.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/dto/AnnouncementDto.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/mappers/AnnouncementMapper.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/network/BackendApiClient.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/AnnouncementsRepositoryImpl.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/TweaksRepositoryImpl.ktcore/domain/src/commonMain/kotlin/zed/rainxch/core/domain/model/Announcement.ktcore/domain/src/commonMain/kotlin/zed/rainxch/core/domain/model/AnnouncementCategory.ktcore/domain/src/commonMain/kotlin/zed/rainxch/core/domain/model/AnnouncementSeverity.ktcore/domain/src/commonMain/kotlin/zed/rainxch/core/domain/repository/AnnouncementsRepository.ktcore/domain/src/commonMain/kotlin/zed/rainxch/core/domain/repository/TweaksRepository.ktcore/presentation/src/commonMain/composeResources/values/strings.xmlcore/presentation/src/commonMain/kotlin/zed/rainxch/core/presentation/components/announcements/AnnouncementCard.ktcore/presentation/src/commonMain/kotlin/zed/rainxch/core/presentation/components/announcements/AnnouncementsRoot.ktcore/presentation/src/commonMain/kotlin/zed/rainxch/core/presentation/components/announcements/CriticalAnnouncementModal.ktcore/presentation/src/commonMain/kotlin/zed/rainxch/core/presentation/components/announcements/MuteSettingsBottomSheet.ktdocs/backend/announcements-endpoint.mdfeature/profile/presentation/src/commonMain/kotlin/zed/rainxch/profile/presentation/ProfileAction.ktfeature/profile/presentation/src/commonMain/kotlin/zed/rainxch/profile/presentation/ProfileRoot.ktfeature/profile/presentation/src/commonMain/kotlin/zed/rainxch/profile/presentation/ProfileViewModel.ktfeature/profile/presentation/src/commonMain/kotlin/zed/rainxch/profile/presentation/components/sections/Options.kt
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/app/announcements/AnnouncementsViewModel.kt`:
- Around line 117-119: The preview data in AnnouncementsViewModel
(previewSampleAnnouncements(), _previewItems, SamplePreviewItems) is never
cleared and can leak into real sessions; fix by scoping or clearing the state:
either move SamplePreviewItems out of the shared AnnouncementsViewModel into a
destination-scoped view model, or add a clear method (e.g., clearPreviewItems())
that sets _previewItems back to an empty/neutral value and ensure
clearPreviewItems() is invoked when the preview destination exits (call from the
destination's onLeave/unregister lifecycle or navigation exit handler) so
previewSampleAnnouncements() no longer affects later sessions.
In
`@core/presentation/src/commonMain/kotlin/zed/rainxch/core/presentation/components/announcements/AnnouncementsRoot.kt`:
- Around line 88-97: The refresh indicator is being hidden after a fixed 800ms
delay instead of when the refresh work completes; change the logic in the
PullToRefreshBox onRefresh handler (where isRefreshing is set) to await the
actual refresh completion: make or treat onRefresh as a suspend function or
return a Job/Deferred, launch a coroutine that sets isRefreshing=true,
calls/awaits onRefresh(), and then sets isRefreshing=false in a finally block to
ensure the spinner stops even on errors; update the signature or call sites of
onRefresh if needed so the handler can await completion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6143063b-87a8-4d9d-9b8f-76cd29517eee
📒 Files selected for processing (7)
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/app/announcements/AnnouncementsViewModel.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/app/navigation/AppNavigation.ktcore/presentation/src/commonMain/kotlin/zed/rainxch/core/presentation/components/announcements/AnnouncementsRoot.ktfeature/profile/presentation/src/commonMain/kotlin/zed/rainxch/profile/presentation/ProfileAction.ktfeature/profile/presentation/src/commonMain/kotlin/zed/rainxch/profile/presentation/ProfileRoot.ktfeature/profile/presentation/src/commonMain/kotlin/zed/rainxch/profile/presentation/ProfileViewModel.ktfeature/profile/presentation/src/commonMain/kotlin/zed/rainxch/profile/presentation/components/sections/Options.kt
🚧 Files skipped from review as they are similar to previous changes (4)
- feature/profile/presentation/src/commonMain/kotlin/zed/rainxch/profile/presentation/ProfileViewModel.kt
- feature/profile/presentation/src/commonMain/kotlin/zed/rainxch/profile/presentation/components/sections/Options.kt
- feature/profile/presentation/src/commonMain/kotlin/zed/rainxch/profile/presentation/ProfileRoot.kt
- feature/profile/presentation/src/commonMain/kotlin/zed/rainxch/profile/presentation/ProfileAction.kt
Comment on lines
+117
to
+119
| fun previewSampleAnnouncements() { | ||
| _previewItems.value = SamplePreviewItems | ||
| } |
Contributor
There was a problem hiding this comment.
Reset preview data when the preview flow ends.
_previewItems lives in a shared ViewModel and is never cleared, so the sample announcements can bleed into later real sessions and keep affecting the global critical-modal path. Scope this state to the destination or clear it on exit.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/app/announcements/AnnouncementsViewModel.kt`
around lines 117 - 119, The preview data in AnnouncementsViewModel
(previewSampleAnnouncements(), _previewItems, SamplePreviewItems) is never
cleared and can leak into real sessions; fix by scoping or clearing the state:
either move SamplePreviewItems out of the shared AnnouncementsViewModel into a
destination-scoped view model, or add a clear method (e.g., clearPreviewItems())
that sets _previewItems back to an empty/neutral value and ensure
clearPreviewItems() is invoked when the preview destination exits (call from the
destination's onLeave/unregister lifecycle or navigation exit handler) so
previewSampleAnnouncements() no longer affects later sessions.
The cached payload used to live next to every other tweak in the shared preferences DataStore, which meant every refresh rewrote the whole file and woke every other collector. Move it behind a small AnnouncementsCacheStore backed by its own DataStore so feature-scoped writes stay scoped, and drop the cache plumbing from TweaksRepository.
…ndary Previously expiresAt and publishedAt were String? on the domain model and parsed lazily inside isPastIso, so an unparsable timestamp would silently fall through and an item with a malformed expiresAt looked permanently active. Move parsing into the mapper, drop items whose publishedAt does not parse, and let the comparison work on Instant values directly.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@core/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/AnnouncementsCacheStoreImpl.kt`:
- Around line 14-15: getCachedPayload() currently forwards preferences.data
directly so any DataStore I/O exception will break the flow; wrap
preferences.data with a catch that intercepts IOException and recovers by
emitting an empty state (e.g., emit emptyPreferences() or emit a null payload)
so the returned Flow<String?> yields null instead of failing; update the
implementation of getCachedPayload() to use preferences.data.catch { if (it is
IOException) emit(emptyPreferences()) else throw it } and then map { prefs ->
prefs[CACHED_PAYLOAD_KEY] } so observeFeed() degrades gracefully on read errors.
In
`@core/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/AnnouncementsRepositoryImpl.kt`:
- Around line 72-88: In AnnouncementsRepositoryImpl.refresh() the onSuccess
branch can still throw during json.encodeToString, cacheStore.setCachedPayload
or tweaksRepository.setAnnouncementsLastFetchedAt which currently escapes the
function; wrap the serialization and persistence steps inside a try/catch within
the onSuccess lambda (cover
json.encodeToString(AnnouncementsResponseDto.serializer(), dto),
cacheStore.setCachedPayload(raw) and
tweaksRepository.setAnnouncementsLastFetchedAt(nowMillis())), on exception log
the error, set lastRefreshFailed.value = true and return
Result.failure(exception), otherwise proceed to set lastRefreshFailed.value =
false and return Result.success(Unit).
- Around line 41-62: The feed currently parses item.installerTypes but never
uses it; update observeFeed to include the current installer type from
tweaksRepository in the combined PersistedFeedState (add a field like
currentInstallerType and include
tweaksRepository.getCurrentInstallerType()/getInstallerType() in the combine
call), then update parseAndFilter(…) to accept the installer type from
PersistedFeedState and reject any announcement whose installerTypes does not
contain the current installer type (or whose installerTypes is non-empty and
does not match DEFAULT/SHIZUKU accordingly). Ensure you update the
PersistedFeedState data class, the combine call in observeFeed, and
parseAndFilter signature/logic to perform this filtering.
- Around line 90-113: The catch blocks in dismiss, acknowledge, and setMuted are
catching Throwable and thus swallowing CancellationException; update each catch
to rethrow CancellationException (or check and throw if t is
CancellationException) before logging and handling persistence errors for
tweaksRepository.addAnnouncementDismissedId,
tweaksRepository.addAnnouncementAcknowledgedId, and
tweaksRepository.setAnnouncementCategoryMuted so cancelled coroutines unwind
normally; keep the existing logging for non-cancellation throwables.
In
`@core/presentation/src/commonMain/kotlin/zed/rainxch/core/presentation/components/announcements/AnnouncementCard.kt`:
- Around line 174-187: The isOverflowing flag only flips true currently; in the
Text composable's onTextLayout callback (for the Text that uses body, expanded
and BODY_COLLAPSED_LINES) update the logic so that on every collapsed layout
pass you assign isOverflowing = layout.hasVisualOverflow (rather than only
setting true). Locate the variables expanded and isOverflowing and the Text's
onTextLayout and change the callback to reassign isOverflowing based on
layout.hasVisualOverflow whenever !expanded so resize/font-scale changes
correctly hide or show the “read more” state.
In
`@core/presentation/src/commonMain/kotlin/zed/rainxch/core/presentation/components/announcements/MuteSettingsBottomSheet.kt`:
- Around line 74-107: CategoryRow currently renders the label as a separate Text
so screen readers can reach the Switch without its label; wrap/convert the whole
row into a single labeled affordance by making the Row merge semantics and be
toggleable: add Modifier.semantics(mergeDescendants = true) and
Modifier.toggleable(checked = enabled, onValueChange = { if (!locked)
onToggle(it) }, role = Role.Switch) to the Row's modifier (keep the visual
Switch but set its onCheckedChange to null or disable its own interaction via
enabled = !locked), so the row and switch are announced together with the
category label; reference: CategoryRow, the Row modifier and the Switch
parameters.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e81167b8-0493-4aae-b07e-927a3ea2decb
📒 Files selected for processing (22)
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/Main.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/app/announcements/AnnouncementsViewModel.ktcomposeApp/src/commonMain/kotlin/zed/rainxch/githubstore/app/navigation/AppNavigation.ktcore/data/src/androidMain/kotlin/zed/rainxch/core/data/di/PlatformModule.android.ktcore/data/src/androidMain/kotlin/zed/rainxch/core/data/local/data_store/createAnnouncementsDataStore.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/di/SharedModule.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/local/data_store/announcementsDataStoreFile.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/mappers/AnnouncementMapper.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/AnnouncementsCacheStoreImpl.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/AnnouncementsRepositoryImpl.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/TweaksRepositoryImpl.ktcore/data/src/jvmMain/kotlin/zed/rainxch/core/data/di/PlatformModule.jvm.ktcore/data/src/jvmMain/kotlin/zed/rainxch/core/data/local/data_store/createAnnouncementsDataStore.ktcore/domain/src/commonMain/kotlin/zed/rainxch/core/domain/model/Announcement.ktcore/domain/src/commonMain/kotlin/zed/rainxch/core/domain/repository/AnnouncementsCacheStore.ktcore/domain/src/commonMain/kotlin/zed/rainxch/core/domain/repository/TweaksRepository.ktcore/presentation/src/commonMain/composeResources/values/strings.xmlcore/presentation/src/commonMain/kotlin/zed/rainxch/core/presentation/components/announcements/AnnouncementCard.ktcore/presentation/src/commonMain/kotlin/zed/rainxch/core/presentation/components/announcements/AnnouncementUiHelpers.ktcore/presentation/src/commonMain/kotlin/zed/rainxch/core/presentation/components/announcements/AnnouncementsRoot.ktcore/presentation/src/commonMain/kotlin/zed/rainxch/core/presentation/components/announcements/MuteSettingsBottomSheet.ktdocs/backend/announcements-endpoint.md
✅ Files skipped from review due to trivial changes (3)
- core/data/src/commonMain/kotlin/zed/rainxch/core/data/local/data_store/announcementsDataStoreFile.kt
- composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/Main.kt
- core/presentation/src/commonMain/composeResources/values/strings.xml
🚧 Files skipped from review as they are similar to previous changes (1)
- core/data/src/commonMain/kotlin/zed/rainxch/core/data/di/SharedModule.kt
- Wrap onSuccess persistence in try/catch so a cache or DataStore write failure flips lastRefreshFailed and surfaces a Result.failure instead of leaking out of refresh(). - Rethrow CancellationException in dismiss / acknowledge / setMuted so cancelled coroutines unwind structured concurrency correctly while persistence errors stay logged. - Filter announcements by the current InstallerType so installer- scoped advisories (e.g. Shizuku-only) only reach matching users.
- ExpandableBody now reassigns isOverflowing every collapsed layout pass instead of latching true once, so resizes and font-scale changes hide the read-more button when the body fits again. - Mute-settings rows are now toggleable as a single labelled affordance via Modifier.toggleable + mergeDescendants, so screen readers announce the category and switch state together.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@core/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/AnnouncementsRepositoryImpl.kt`:
- Around line 42-57: The feed combo currently ignores locale changes because
observeFeed() only combines persistedFlow, lastRefreshFailed and installer type;
include a locale signal so parseAndFilter() runs on language switches. Modify
observeFeed() to also combine a locale Flow from localizationManager (e.g.
localizationManager.localeFlow / observeLocale()) or pass locale down into the
downstream mapping and call parseAndFilter(persisted.payload,
installerType.name, locale) (or re-run parseAndFilter inside a map keyed by
locale) so snapshots are re-resolved immediately when the app language changes;
update parseAndFilter signature if necessary to accept the locale or move
localization resolution downstream where localeFlow is observed.
- Around line 146-154: The filter predicate in AnnouncementsRepositoryImpl that
currently checks expiry, version, platform and installer (the lambda using
item.expiresAt, item.minVersionCode, item.maxVersionCode, item.platforms,
item.installerTypes) is missing a check for item.publishedAt, so announcements
with a future publishedAt still pass; update the predicate to compute a
publishedOk boolean (e.g., publishedOk = item.publishedAt?.let { it <= now } ?:
true) and include publishedOk in the final conjunction alongside !expired,
versionFloorOk, versionCeilingOk, platformOk and installerOk so items scheduled
for the future are excluded.
In
`@core/presentation/src/commonMain/kotlin/zed/rainxch/core/presentation/components/announcements/AnnouncementCard.kt`:
- Around line 141-150: The Icon in AnnouncementCard is currently using the same
stringResource as the Text which causes duplicate announcement in
TalkBack/VoiceOver; update the Icon call (the Icon(...) inside AnnouncementCard)
to be decorative by setting its contentDescription to null (leave the Text(...)
with text = stringResource(Res.string.announcements_acknowledged) unchanged) so
only the textual label is announced.
- Around line 213-222: The TextButton currently uses announcement.ctaLabel
directly which renders an empty label when ctaLabel is blank; update the label
resolution in AnnouncementCard so it uses the same null-or-blank fallback as the
CTA URL (e.g., treat announcement.ctaLabel with an isNullOrBlank check) and pass
stringResource(Res.string.announcements_read_more) when
announcement.ctaLabel.isNullOrBlank(), leaving the surrounding
TextButton(onClick = onCtaClick) and Icon/Spacer structure unchanged.
- Around line 132-135: The Icon currently sets contentDescription = null which
hides the announcement severity from accessibility services; update the Icon in
AnnouncementCard.kt to provide a meaningful contentDescription derived from the
announcement severity (e.g., use severity.name or a localized display
label/stringResource for severity) and/or include iconHint context so screen
readers convey urgency (for example "Severity: High"). Ensure the description is
used instead of null so TalkBack/VoiceOver can announce severity along with the
visual icon.
In
`@core/presentation/src/commonMain/kotlin/zed/rainxch/core/presentation/components/announcements/MuteSettingsBottomSheet.kt`:
- Around line 66-69: The toggle row currently sets enabled = category !in
mutedCategories even for locked categories; change the enabled expression so
locked categories (where category.isMutable is false) always have enabled = true
and only use category !in mutedCategories for mutable ones (e.g., enabled = if
(!category.isMutable) true else category !in mutedCategories) while keeping
locked = !category.isMutable and leaving onToggle unchanged or no-op when
locked.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: abc161ce-1f84-4a9d-9ace-c79f0016b135
📒 Files selected for processing (4)
core/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/AnnouncementsCacheStoreImpl.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/AnnouncementsRepositoryImpl.ktcore/presentation/src/commonMain/kotlin/zed/rainxch/core/presentation/components/announcements/AnnouncementCard.ktcore/presentation/src/commonMain/kotlin/zed/rainxch/core/presentation/components/announcements/MuteSettingsBottomSheet.kt
…tems Add tweaksRepository.getAppLanguage() to the observeFeed combine so parseAndFilter re-runs when the in-app language switches; otherwise the cached payload kept its first-resolved locale strings until something else triggered a re-emission. Also reject items whose publishedAt is in the future so a scheduled announcement does not surface before its release date.
- Severity icon now carries a localized severityLabel content description so screen readers announce urgency alongside the visual color/shape cue. - The acknowledged badge icon goes back to decorative since the Text right next to it already conveys the same word; TalkBack no longer reads 'Acknowledged' twice. - TextButton CTA falls back to 'Read more' when ctaLabel is blank, not just null, so an empty string from the backend never renders an empty button.
Items that require an explicit acknowledgment (privacy, security) still wait for the user to tap their button, so the badge stays until those are confirmed. Routine items get auto-acknowledged when the inbox screen mounts, which is what gives the dot a natural way to clear without forcing the user to dismiss every card.
Mirrors the bottom-nav badge so users who tap into Profile from a flagged tab can see exactly which row needs attention instead of hunting. Same error-color dot, same dataset, same auto-clear when the inbox marks routine items as seen.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Adds an in-app announcements channel for messages that don't fit a release: privacy-policy changes, surveys, initiative endorsements, security advisories, backend-status notices.
Built privacy-first: no per-user identifier sent, single anonymous endpoint, dismissal/acknowledgment recorded only on device. Same payload returned to every caller.
Client (this PR)
Announcement,AnnouncementSeverity(INFO / IMPORTANT / CRITICAL),AnnouncementCategory(NEWS / PRIVACY / SURVEY / SECURITY / STATUS),AnnouncementsRepositoryinterface,AnnouncementsFeedSnapshotvalue type.AnnouncementDto+AnnouncementsResponseDto+ locale variants (AnnouncementLocaleDto). Mapper handles locale resolution chain (full → primary → English fallback).AnnouncementsRepositoryImplfetches from backend, caches in DataStore, applies client-side filtering (expired items, version range, platform). Backend endpoint added to existingBackendApiClientasgetAnnouncements().announcements_dismissed_ids,announcements_acknowledged_ids,announcements_muted_categories,announcements_cached_payload,announcements_last_fetched_at. Acknowledged-ID set kept locally only, never transmitted.AnnouncementsRoot— inbox screen reachable from Profile. LazyColumn of cards, mute-settings overflow.AnnouncementCard— severity-coded border bar, category chip, expandable body, CTA + dismiss/acknowledge actions.CriticalAnnouncementModal—AlertDialogfor severity = CRITICAL + requiresAcknowledgment, non-dismissible until tap.MuteSettingsBottomSheet— toggle categories on/off; PRIVACY and SECURITY locked on.AnnouncementsScreenroute inGithubStoreGraph. Critical modal hosted inMain.ktnext to existing what's-new sheet, using the same gating (only on Home, AuthState settled, no rate-limit dialog active, 600ms debounce). Profile gets a new "Announcements" entry below "What's new".i18n: Map<locale, variant>per item — single fetch covers all locales; switching app language re-resolves without refetch. English is mandatory fallback.Backend (separate work — implementation spec attached)
docs/backend/announcements-endpoint.mdis the full contract for the backend agent: endpoint shape, item schema, validation rules, storage recommendation, privacy guardrails, example payloads, and a per-announcement authoring rubric. Self-contained; no need to read the rest of this PR to implement.Until the backend endpoint exists, the client gracefully no-ops: refresh fails silently → cached payload (empty initially) renders → inbox shows the empty state.
What's deliberately NOT in this PR (deferred to phase 2)
Test plan
Without the backend live:
Once backend ships an empty
items: []payload:Once backend ships at least one INFO item:
Once backend ships an IMPORTANT item with
requiresAcknowledgment: true:Once backend ships a CRITICAL item with
requiresAcknowledgment: true:Files of note
docs/backend/announcements-endpoint.md— backend contract.core/domain/.../model/Announcement.kt,AnnouncementSeverity.kt,AnnouncementCategory.kt— domain.core/domain/.../repository/AnnouncementsRepository.kt— interface +AnnouncementsFeedSnapshot.core/data/.../dto/AnnouncementDto.kt,mappers/AnnouncementMapper.kt— wire format + locale resolution.core/data/.../repository/AnnouncementsRepositoryImpl.kt— fetch, cache, filter.core/data/.../network/BackendApiClient.kt— addedgetAnnouncements().core/presentation/.../components/announcements/— all composables.composeApp/.../app/announcements/AnnouncementsViewModel.kt— state + actions.composeApp/.../Main.kt— critical modal host.feature/profile/.../components/sections/Options.kt— Profile entry.Summary by CodeRabbit
New Features
Documentation