feat(apps): ignore updates per app (#536)#537
Merged
Conversation
β¦et across 13 locales
Contributor
WalkthroughThis PR adds per-app update check toggling: users can suppress update notifications per app via an overflow menu; the flag is persisted, repository checks respect it (early-return), the UI shows a bar-shaped ignored indicator, and localized whatsnew/string entries were added. ChangesPer-App Update Check Toggle Feature
Estimated code review effortπ― 3 (Moderate) | β±οΈ ~25 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 docstrings
π§ͺ 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: 4
Caution
Some comments are outside the diff and canβt be posted inline due to platform limitations.
β οΈ Outside diff range comments (1)
feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsRoot.kt (1)
551-553:β οΈ Potential issue | π Major | β‘ Quick winExclude ignored apps from the βUpdate allβ trigger condition.
Line 551 currently counts updates even when
updateCheckEnabledisfalse, so ignored apps can still surface the global update CTA. Gate this withupdateCheckEnabledto keep UI behavior aligned with ignore-updates semantics.Proposed fix
- val hasUpdates = state.apps.any { it.installedApp.isUpdateAvailable } + val hasUpdates = + state.apps.any { + it.installedApp.isUpdateAvailable && it.installedApp.updateCheckEnabled + }π€ Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsRoot.kt` around lines 551 - 553, The current hasUpdates calculation uses state.apps.any { it.installedApp.isUpdateAvailable } and thus includes apps that have updateCheckEnabled == false; change the predicate to also require the app's updateCheckEnabled flag (e.g., state.apps.any { it.installedApp.updateCheckEnabled && it.installedApp.isUpdateAvailable }) so the βUpdate allβ CTA only appears for apps that are not ignored; update the hasUpdates usage so the gate for Button remains consistent.
π§Ή Nitpick comments (1)
core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/repository/InstalledAppsRepository.kt (1)
76-79: π€ Low valueConsider documenting the non-trivial side effects in KDoc.
setUpdateCheckEnabledhas meaningful side-effect semantics β disabling clears cached update metadata, enabling immediately re-triggerscheckForUpdatesβ that are not obvious from the signature alone. Several other methods in this interface (setAssetFilter,setPreferredVariant,setPendingInstallFilePath) carry KDoc explaining exactly these kinds of implicit behaviors. Callers and future implementors benefit from the same clarity here.π Suggested KDoc
+ /** + * Enables or disables the periodic update check for [packageName]. + * + * When [enabled] is `false` the implementation should clear any + * cached update metadata (badge + latest-release fields) so the app + * moves out of the "Updates available" group immediately. + * + * When [enabled] is `true` the implementation should call + * [checkForUpdates] for [packageName] right away so the badge is + * restored without waiting for the next periodic worker run. + */ suspend fun setUpdateCheckEnabled( packageName: String, enabled: Boolean, )π€ Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/repository/InstalledAppsRepository.kt` around lines 76 - 79, The method setUpdateCheckEnabled in InstalledAppsRepository.kt has non-obvious side effects (disabling should clear cached update metadata; enabling should immediately re-trigger checkForUpdates) that need KDoc similar to other interface methods; add a KDoc comment (/** ... */) directly above the suspend fun setUpdateCheckEnabled(packageName: String, enabled: Boolean) declaration describing these behaviors, the expected semantics for implementors and callers, and any relevant concurrency or timing guarantees (e.g., that enabling triggers an immediate checkForUpdates call and disabling clears cached update metadata).
π€ Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/presentation/src/commonMain/composeResources/files/whatsnew/pl/17.json`:
- Line 10: Replace the non-idiomatic phrase "per aplikacja" in the JSON string
"Ignorowanie aktualizacji per aplikacja β wycisz powiadomienie o aktualizacji
dla dowolnej aplikacji, ktΓ³rej nie chcesz aktualizowaΔ." with an idiomatic
Polish alternative such as "dla kaΕΌdej aplikacji" or "dla poszczegΓ³lnych
aplikacji" (e.g., "Ignorowanie aktualizacji dla kaΕΌdej aplikacji β wycisz
powiadomienie o aktualizacji dla dowolnej aplikacji, ktΓ³rej nie chcesz
aktualizowaΔ.") to match natural Polish phrasing used elsewhere.
In `@core/presentation/src/commonMain/composeResources/values-hi/strings-hi.xml`:
- Line 1010: Update the Hindi string value for the resource key
apps_compact_status_updates_ignored to use correct grammatical agreement:
replace the current text "ΰ€
ΰ€ͺΰ€‘ΰ₯ΰ€ ΰ€
ΰ€¨ΰ€¦ΰ₯ΰ€ΰ€Ύ ΰ€ΰ€Ώΰ€ ΰ€ΰ€" with "ΰ€
ΰ€ͺΰ€‘ΰ₯ΰ€ ΰ€
ΰ€¨ΰ€¦ΰ₯ΰ€ΰ₯ ΰ€ΰ€Ώΰ€ ΰ€ΰ€" so the
phrase reads naturally and agrees in number/gender with the noun.
In `@core/presentation/src/commonMain/composeResources/values-it/strings-it.xml`:
- Line 1014: Update the Italian string value for the resource named
apps_ignore_updates to use the more natural phrasing "Ignora gli aggiornamenti"
instead of "Ignora aggiornamenti"; locate the <string
name="apps_ignore_updates"> entry and replace its text content accordingly so
the UI shows the idiomatic action label.
In
`@feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsViewModel.kt`:
- Around line 593-600: The toggleUpdateCheck coroutine currently catches all
Exception types, which will also swallow CancellationException and break
coroutine cancellation; update the error handling in toggleUpdateCheck (the
viewModelScope.launch block calling
installedAppsRepository.setUpdateCheckEnabled) so that CancellationException is
not suppressed β detect and rethrow CancellationException (or handle it
separately) and only log other exceptions via logger.error; keep the existing
try/catch structure but ensure CancellationException is propagated.
---
Outside diff comments:
In
`@feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsRoot.kt`:
- Around line 551-553: The current hasUpdates calculation uses state.apps.any {
it.installedApp.isUpdateAvailable } and thus includes apps that have
updateCheckEnabled == false; change the predicate to also require the app's
updateCheckEnabled flag (e.g., state.apps.any {
it.installedApp.updateCheckEnabled && it.installedApp.isUpdateAvailable }) so
the βUpdate allβ CTA only appears for apps that are not ignored; update the
hasUpdates usage so the gate for Button remains consistent.
---
Nitpick comments:
In
`@core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/repository/InstalledAppsRepository.kt`:
- Around line 76-79: The method setUpdateCheckEnabled in
InstalledAppsRepository.kt has non-obvious side effects (disabling should clear
cached update metadata; enabling should immediately re-trigger checkForUpdates)
that need KDoc similar to other interface methods; add a KDoc comment (/** ...
*/) directly above the suspend fun setUpdateCheckEnabled(packageName: String,
enabled: Boolean) declaration describing these behaviors, the expected semantics
for implementors and callers, and any relevant concurrency or timing guarantees
(e.g., that enabling triggers an immediate checkForUpdates call and disabling
clears cached update metadata).
πͺ 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: 92074511-2e19-44e2-b0f1-0d7604f97f93
π Files selected for processing (34)
core/data/src/commonMain/kotlin/zed/rainxch/core/data/local/db/dao/InstalledAppDao.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/InstalledAppsRepositoryImpl.ktcore/domain/src/commonMain/kotlin/zed/rainxch/core/domain/repository/InstalledAppsRepository.ktcore/presentation/src/commonMain/composeResources/files/whatsnew/17.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/ar/17.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/bn/17.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/es/17.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/fr/17.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/hi/17.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/it/17.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/ja/17.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/ko/17.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/pl/17.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/ru/17.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/tr/17.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/zh-CN/17.jsoncore/presentation/src/commonMain/composeResources/values-ar/strings-ar.xmlcore/presentation/src/commonMain/composeResources/values-bn/strings-bn.xmlcore/presentation/src/commonMain/composeResources/values-es/strings-es.xmlcore/presentation/src/commonMain/composeResources/values-fr/strings-fr.xmlcore/presentation/src/commonMain/composeResources/values-hi/strings-hi.xmlcore/presentation/src/commonMain/composeResources/values-it/strings-it.xmlcore/presentation/src/commonMain/composeResources/values-ja/strings-ja.xmlcore/presentation/src/commonMain/composeResources/values-ko/strings-ko.xmlcore/presentation/src/commonMain/composeResources/values-pl/strings-pl.xmlcore/presentation/src/commonMain/composeResources/values-ru/strings-ru.xmlcore/presentation/src/commonMain/composeResources/values-tr/strings-tr.xmlcore/presentation/src/commonMain/composeResources/values-zh-rCN/strings-zh-rCN.xmlcore/presentation/src/commonMain/composeResources/values/strings.xmlfeature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsAction.ktfeature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsRoot.ktfeature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsViewModel.ktfeature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/components/CompactAppRow.ktfeature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/components/StatusDotCluster.kt
β¦xception, gate Update All on updateCheckEnabled
Contributor
There was a problem hiding this comment.
π§Ή Nitpick comments (1)
feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsRoot.kt (1)
1122-1123: β‘ Quick winUse
leadingIconinstead of a hardcoded β glyph for the checked-state indicator.Appending
" β"to the label string is not idiomatic Material3, and TTS engines have inconsistent narration for the Unicode β character (some say "checkmark", some say "tick mark", some skip it).DropdownMenuItemexposes aleadingIconslot specifically for this purpose; conditionally placing a check icon there provides both a consistent visual and a properly described semantics node.β»οΈ Proposed refactor β replace β text with leadingIcon
DropdownMenuItem( - text = { - val baseLabel = stringResource(Res.string.apps_ignore_updates) - Text(text = if (!app.updateCheckEnabled) "$baseLabel β" else baseLabel) - }, + text = { Text(text = stringResource(Res.string.apps_ignore_updates)) }, onClick = { showRowOverflow = false onToggleUpdateCheck(!app.updateCheckEnabled) }, + leadingIcon = { + if (!app.updateCheckEnabled) { + Icon( + imageVector = Icons.Default.CheckCircle, + contentDescription = null, + ) + } else { + // Reserve space so the text column stays aligned + Spacer(Modifier.size(24.dp)) + } + }, )π€ Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsRoot.kt` around lines 1122 - 1123, Replace the hardcoded " β" glyph appended to the label in AppsRoot.kt: stop mutating baseLabel/Text and instead supply the check indicator via the DropdownMenuItem leadingIcon slot; conditionally render an Icon (e.g., check icon) when !app.updateCheckEnabled and leave Text(text = baseLabel) unmodified, and provide an appropriate contentDescription or use semantics for the Icon so screen readers get a consistent description.
π€ Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsRoot.kt`:
- Around line 1122-1123: Replace the hardcoded " β" glyph appended to the label
in AppsRoot.kt: stop mutating baseLabel/Text and instead supply the check
indicator via the DropdownMenuItem leadingIcon slot; conditionally render an
Icon (e.g., check icon) when !app.updateCheckEnabled and leave Text(text =
baseLabel) unmodified, and provide an appropriate contentDescription or use
semantics for the Icon so screen readers get a consistent description.
βΉοΈ Review info
βοΈ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 348e76d5-6f46-49cb-9e78-65d16806dd76
π Files selected for processing (5)
core/presentation/src/commonMain/composeResources/files/whatsnew/pl/17.jsoncore/presentation/src/commonMain/composeResources/values-hi/strings-hi.xmlcore/presentation/src/commonMain/composeResources/values-it/strings-it.xmlfeature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsRoot.ktfeature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsViewModel.kt
β Files skipped from review due to trivial changes (1)
- core/presentation/src/commonMain/composeResources/files/whatsnew/pl/17.json
π§ Files skipped from review as they are similar to previous changes (2)
- feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsViewModel.kt
- core/presentation/src/commonMain/composeResources/values-it/strings-it.xml
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.
Closes #536.
Summary
checkForUpdatesimmediately so the badge comes back without waiting for the next periodic check.Implementation
InstalledAppDao.updateUpdateCheckEnabledwrites the existingupdateCheckEnabledcolumn.InstalledAppsRepository.setUpdateCheckEnabledwrites the column, clears stale update metadata on disable, and re-checks on enable.checkForUpdates(packageName)is now gated on the column so single-package callers (e.g. PackageEventReceiver post-install refresh) honour the user's choice.AppsRootsection split now also routes!updateCheckEnabledrows to the idle group as a defensive filter.AppsAction.OnToggleUpdateCheck+AppsViewModel.toggleUpdateCheckthread the toggle through.CompactAppRowoverflow gets a new "Ignore updates" item;AppItemCardgains a trailing overflow with the same item.CompactStatusFlags.Strings
apps_ignore_updatesandapps_compact_status_updates_ignoredadded to all 13 locales (ar/bn/es/fr/hi/it/ja/ko/pl/ru/tr/zh-rCN + default).What's-new
Test plan
checkAllForUpdatesskips ignored apps (was already gated; verify no regression).Summary by CodeRabbit
New Features
Documentation