feat(details): Implement release picker and fetch all releases#251
Merged
rainxchzed merged 8 commits intomainfrom Feb 19, 2026
Merged
feat(details): Implement release picker and fetch all releases#251rainxchzed merged 8 commits intomainfrom
rainxchzed merged 8 commits intomainfrom
Conversation
This commit introduces a version picker on the details screen, allowing users to select between stable, pre-release, and all available releases for an application. Previously, only the latest published release was fetched and displayed. Now, the app fetches a list of all releases, automatically selects the latest stable version by default, and provides UI for the user to switch to other versions. - **feat(details)**: Fetched all releases instead of just the latest. The view now defaults to the latest stable release, falling back to the most recent pre-release if no stable version is available. - **feat(details)**: Added a version picker component (`VersionPicker.kt`) to the details screen, which allows users to filter and select from stable releases, pre-releases, or all available versions. - **refactor(details)**: Updated `DetailsViewModel` and `DetailsState` to manage the list of all releases, the selected release, and the state of the version picker. - **refactor(data)**: Implemented `getAllReleases` in `DetailsRepository` to retrieve up to 30 recent releases from the GitHub API. - **chore(i18n)**: Added new string resources for the version picker UI (e.g., "Stable", "Pre-release", "Select version").
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 version-picker flow: domain model gains Changes
Sequence DiagramsequenceDiagram
participant UI as UI (VersionPicker/Header)
participant VM as DetailsViewModel
participant State as DetailsState
participant Repo as DetailsRepository
participant API as GitHub API
UI->>VM: ToggleVersionPicker()
VM->>State: isVersionPickerVisible = true/false
UI->>VM: SelectReleaseCategory(category)
VM->>State: selectedReleaseCategory = category
State-->>State: compute filteredReleases
UI->>VM: SelectRelease(release)
VM->>VM: recomputeAssetsForRelease(release)
VM->>State: selectedRelease, installableAssets, primaryAsset
Note over VM,Repo: initial/refresh load
VM->>Repo: getAllReleases(owner, repo, branch)
Repo->>API: fetch releases
API-->>Repo: releases payload
Repo-->>VM: List<GithubRelease> (isPrerelease set)
VM->>State: allReleases = list
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/SmartInstallButton.kt (1)
79-92:⚠️ Potential issue | 🟡 MinorBehavioral change when
selectedReleaseis null: button may misleadingly show "Update app".When
selectedReleaseisnull(e.g., no releases fetched yet),state.selectedRelease?.tagNameevaluates tonull, making the condition on line 81 true for any installed app. This would display "Update app" even though there's no target release to update to.This could be guarded by checking
selectedRelease != nullfirst:Proposed fix
val buttonText = when { !enabled && primaryAsset == null -> stringResource(Res.string.not_available) - installedApp != null && installedApp.installedVersion != state.selectedRelease?.tagName -> stringResource( + installedApp != null && state.selectedRelease != null && installedApp.installedVersion != state.selectedRelease.tagName -> stringResource( Res.string.update_app )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/SmartInstallButton.kt` around lines 79 - 92, In SmartInstallButton.kt the buttonText computation incorrectly treats installedApp as needing an "Update app" when state.selectedRelease is null because installedApp.installedVersion != state.selectedRelease?.tagName evaluates true; update the when branch that returns "Update app" to also require state.selectedRelease != null (e.g., change the condition checking installedApp.installedVersion != state.selectedRelease?.tagName to require state.selectedRelease != null && installedApp.installedVersion != state.selectedRelease.tagName) so the label only shows when an explicit selectedRelease exists.feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt (2)
244-261:⚠️ Potential issue | 🟠 MajorBug: Inconsistent state when all releases are pre-releases.
When no stable releases exist,
selectedReleasefalls back to the first pre-release (line 246), butselectedReleaseCategoryis hardcoded toReleaseCategory.STABLE(line 261). This meansfilteredReleases(which filters by category) will return an empty list, whileselectedReleaseis a pre-release that doesn't appear in that filtered list — creating an inconsistent UI state where the version picker shows "Stable" with no versions, yet a pre-release is displayed in "What's New."🐛 Proposed fix: set category based on whether stable releases exist
// Auto-select latest stable, fall back to first release if no stable exists -val selectedRelease = allReleases.firstOrNull { !it.isPrerelease } - ?: allReleases.firstOrNull() +val latestStable = allReleases.firstOrNull { !it.isPrerelease } +val selectedRelease = latestStable ?: allReleases.firstOrNull() +val initialCategory = if (latestStable != null) ReleaseCategory.STABLE else ReleaseCategory.ALL val (installable, primary) = recomputeAssetsForRelease(selectedRelease) // ... in the state copy: - selectedReleaseCategory = ReleaseCategory.STABLE, + selectedReleaseCategory = initialCategory,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt` around lines 244 - 261, selectedRelease is chosen to be a prerelease when no stable exists, but selectedReleaseCategory is always set to ReleaseCategory.STABLE, causing the filteredReleases list to be empty; change the assignment of selectedReleaseCategory in DetailsViewModel so it reflects the chosen release: set selectedReleaseCategory = if (selectedRelease?.isPrerelease == true) ReleaseCategory.PRE_RELEASE else ReleaseCategory.STABLE (or determine by allReleases.any { !it.isPrerelease }) so the UI’s filteredReleases and the displayed selectedRelease stay consistent.
357-368:⚠️ Potential issue | 🟡 Minor
latestVersionin FavoriteRepo may store a non-latest version.When the user manually selects an older release via the version picker and then toggles favorite,
latestVersionandlatestReleaseUrlwill reflect the selected release rather than the actual latest. Consider usingallReleases.firstOrNull()(the most recent release regardless of category) instead ofselectedReleasefor these fields to keep the semantics accurate.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt` around lines 357 - 368, The FavoriteRepo creation currently uses _state.value.selectedRelease for latestVersion/latestReleaseUrl so favorites can record a user-selected older release; change these to use the actual most-recent release (e.g., _state.value.allReleases.firstOrNull()) so latestVersion/latestReleaseUrl reflect the true latest release. Update the FavoriteRepo construction in DetailsViewModel (replace selectedRelease?.tagName and selectedRelease?.htmlUrl with a variable like newestRelease = _state.value.allReleases.firstOrNull() and use newestRelease?.tagName / newestRelease?.htmlUrl) and keep selectedRelease for the UI-selected fields only.
🧹 Nitpick comments (2)
feature/details/data/src/commonMain/kotlin/zed/rainxch/details/data/repository/DetailsRepositoryImpl.kt (1)
99-119: Processing all release bodies eagerly may be unnecessarily expensive.
getAllReleasesrunsprocessReleaseBody(markdown preprocessing) on every release, but the user likely only views one release's notes at a time via the version picker. For repos with many releases, this does redundant work upfront.Consider deferring body processing — either lazily process on selection, or only process the initially selected release here and process others on demand in the ViewModel.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/details/data/src/commonMain/kotlin/zed/rainxch/details/data/repository/DetailsRepositoryImpl.kt` around lines 99 - 119, getAllReleases eagerly calls processReleaseBody for every release causing unnecessary work; change the behavior to return releases without preprocessing and only process the body for the initially selected release or on-demand in the ViewModel. Specifically, in DetailsRepositoryImpl::getAllReleases stop calling processReleaseBody inside the map (leave ReleaseNetwork/GithubRelease body raw or add a flag), and instead expose the raw body via GithubRelease (or add a new rawBody property) so the ViewModel can call processReleaseBody when a release is selected (or add an optional parameter to getAllReleases like selectedTag to process only that release's body). Ensure function names referenced: getAllReleases, processReleaseBody, DetailsRepositoryImpl, and ViewModel selection logic are updated accordingly.feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsState.kt (1)
54-61: Duplicated filtering logic —filteredReleasesis recomputed in the ViewModel instead of being reused.The
filteredReleasescomputed property defined here filtersallReleasesbyselectedReleaseCategory. However, inDetailsViewModel.SelectReleaseCategoryhandler (lines 566-570), the exact same filtering is performed manually. If the filtering criteria ever change, you'd need to update both places.Consider having the ViewModel rely on the state's
filteredReleasesafter updating the category:♻️ Suggested refactor in DetailsViewModel.kt (SelectReleaseCategory handler)
is DetailsAction.SelectReleaseCategory -> { val newCategory = action.category - val filtered = when (newCategory) { - ReleaseCategory.STABLE -> _state.value.allReleases.filter { !it.isPrerelease } - ReleaseCategory.PRE_RELEASE -> _state.value.allReleases.filter { it.isPrerelease } - ReleaseCategory.ALL -> _state.value.allReleases - } - val newSelected = filtered.firstOrNull() + // Update category first, then use the derived filteredReleases + _state.update { it.copy(selectedReleaseCategory = newCategory) } + val newSelected = _state.value.filteredReleases.firstOrNull() val (installable, primary) = recomputeAssetsForRelease(newSelected) _state.update { it.copy( - selectedReleaseCategory = newCategory, selectedRelease = newSelected, installableAssets = installable, primaryAsset = primary ) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsState.kt` around lines 54 - 61, The duplicated filtering logic lives in the DetailsState.filteredReleases computed property and is also manually re-implemented in the DetailsViewModel.SelectReleaseCategory handler; update the handler to stop re-filtering and instead set the selectedReleaseCategory on the state and rely on state.filteredReleases for the derived list. Specifically, in the SelectReleaseCategory handling code in DetailsViewModel, remove the manual allReleases.filter(...) branches and after updating state.selectedReleaseCategory use state.filteredReleases (or pass it to where the ViewModel currently uses the duplicated result) so all filtering logic is centralized in the filteredReleases property.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/SmartInstallButton.kt`:
- Around line 79-92: In SmartInstallButton.kt the buttonText computation
incorrectly treats installedApp as needing an "Update app" when
state.selectedRelease is null because installedApp.installedVersion !=
state.selectedRelease?.tagName evaluates true; update the when branch that
returns "Update app" to also require state.selectedRelease != null (e.g., change
the condition checking installedApp.installedVersion !=
state.selectedRelease?.tagName to require state.selectedRelease != null &&
installedApp.installedVersion != state.selectedRelease.tagName) so the label
only shows when an explicit selectedRelease exists.
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt`:
- Around line 244-261: selectedRelease is chosen to be a prerelease when no
stable exists, but selectedReleaseCategory is always set to
ReleaseCategory.STABLE, causing the filteredReleases list to be empty; change
the assignment of selectedReleaseCategory in DetailsViewModel so it reflects the
chosen release: set selectedReleaseCategory = if (selectedRelease?.isPrerelease
== true) ReleaseCategory.PRE_RELEASE else ReleaseCategory.STABLE (or determine
by allReleases.any { !it.isPrerelease }) so the UI’s filteredReleases and the
displayed selectedRelease stay consistent.
- Around line 357-368: The FavoriteRepo creation currently uses
_state.value.selectedRelease for latestVersion/latestReleaseUrl so favorites can
record a user-selected older release; change these to use the actual most-recent
release (e.g., _state.value.allReleases.firstOrNull()) so
latestVersion/latestReleaseUrl reflect the true latest release. Update the
FavoriteRepo construction in DetailsViewModel (replace selectedRelease?.tagName
and selectedRelease?.htmlUrl with a variable like newestRelease =
_state.value.allReleases.firstOrNull() and use newestRelease?.tagName /
newestRelease?.htmlUrl) and keep selectedRelease for the UI-selected fields
only.
---
Nitpick comments:
In
`@feature/details/data/src/commonMain/kotlin/zed/rainxch/details/data/repository/DetailsRepositoryImpl.kt`:
- Around line 99-119: getAllReleases eagerly calls processReleaseBody for every
release causing unnecessary work; change the behavior to return releases without
preprocessing and only process the body for the initially selected release or
on-demand in the ViewModel. Specifically, in
DetailsRepositoryImpl::getAllReleases stop calling processReleaseBody inside the
map (leave ReleaseNetwork/GithubRelease body raw or add a flag), and instead
expose the raw body via GithubRelease (or add a new rawBody property) so the
ViewModel can call processReleaseBody when a release is selected (or add an
optional parameter to getAllReleases like selectedTag to process only that
release's body). Ensure function names referenced: getAllReleases,
processReleaseBody, DetailsRepositoryImpl, and ViewModel selection logic are
updated accordingly.
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsState.kt`:
- Around line 54-61: The duplicated filtering logic lives in the
DetailsState.filteredReleases computed property and is also manually
re-implemented in the DetailsViewModel.SelectReleaseCategory handler; update the
handler to stop re-filtering and instead set the selectedReleaseCategory on the
state and rely on state.filteredReleases for the derived list. Specifically, in
the SelectReleaseCategory handling code in DetailsViewModel, remove the manual
allReleases.filter(...) branches and after updating
state.selectedReleaseCategory use state.filteredReleases (or pass it to where
the ViewModel currently uses the duplicated result) so all filtering logic is
centralized in the filteredReleases property.
This commit introduces new string translations across multiple languages for UI elements related to the version picker and repository status.
- **feat(i18n)**: Added translations for the version picker, including categories ("Stable", "Pre-release", "All"), titles ("Versions", "Select version"), and states ("No version selected").
- **feat(i18n)**: Added a "Pre-release" badge label.
- **feat(i18n)**: Added a translation for the "Forked repository" label.
This commit introduces a new `VersionPicker` composable in the details screen, allowing users to select a specific release from a list. It also adds a `ReleaseCategory` enum to filter releases by type (Stable, Pre-release, All). - **feat(details)!** Added `VersionPicker.kt`, a new composable that provides: - Filter chips for `Stable`, `Pre-release`, and `All` release categories. - A card that displays the currently selected version and opens a modal bottom sheet on click. - A bottom sheet that lists available versions, showing their tag, name, and publication date. It also highlights the selected version and indicates pre-releases. - **feat(details)!** Created `ReleaseCategory.kt` to define the filtering options for GitHub releases.
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 `@core/presentation/src/commonMain/composeResources/values-es/strings-es.xml`:
- Line 286: The string resource forked_repository currently uses the infinitive
"Bifurcar"; change its value to a noun or adjective to match label semantics
(e.g., "Bifurcación" or "Bifurcado") in the file where string
name="forked_repository" is defined so the translation aligns with other locales
and serves as a repository badge/label rather than an action.
In `@core/presentation/src/commonMain/composeResources/values-tr/strings-tr.xml`:
- Line 336: The translation for the string resource named "forked_repository" is
using the imperative verb "Çatalla" but should be a noun/label; update the value
of the "forked_repository" string to a noun form such as "Çatal" or
"Çatallanmış" to match other locales' label semantics so the resource key
forked_repository becomes a label rather than an action.
| <string name="bottom_nav_apps_title">Aplicaciones</string> | ||
| <string name="bottom_nav_profile_title">Perfil</string> | ||
|
|
||
| <string name="forked_repository">Bifurcar</string> |
Contributor
There was a problem hiding this comment.
Minor translation concern: forked_repository uses verb infinitive.
"Bifurcar" is the infinitive ("To fork"), while this key appears to be used as a label/badge for forked repositories. Consider "Bifurcación" (noun) or "Bifurcado" (adjective) to match the label semantics used in other locales.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@core/presentation/src/commonMain/composeResources/values-es/strings-es.xml`
at line 286, The string resource forked_repository currently uses the infinitive
"Bifurcar"; change its value to a noun or adjective to match label semantics
(e.g., "Bifurcación" or "Bifurcado") in the file where string
name="forked_repository" is defined so the translation aligns with other locales
and serves as a repository badge/label rather than an action.
| <string name="bottom_nav_apps_title">Uygulamalar</string> | ||
| <string name="bottom_nav_profile_title">Profil</string> | ||
|
|
||
| <string name="forked_repository">Çatalla</string> |
Contributor
There was a problem hiding this comment.
Minor translation concern: forked_repository uses verb form.
"Çatalla" is the imperative form ("Fork it"), while this key appears to be used as a label/badge for forked repositories. Other locales use noun forms (e.g., "フォーク", "Форк", "Fork"). Consider using "Çatal" or "Çatallanmış" to match the label semantics.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@core/presentation/src/commonMain/composeResources/values-tr/strings-tr.xml`
at line 336, The translation for the string resource named "forked_repository"
is using the imperative verb "Çatalla" but should be a noun/label; update the
value of the "forked_repository" string to a noun form such as "Çatal" or
"Çatallanmış" to match other locales' label semantics so the resource key
forked_repository becomes a label rather than an action.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/VersionPicker.kt (3)
169-241: Well-structured list item composable.Good visual differentiation for selected state and pre-release badge. One minor accessibility note: the
clickablemodifier on Line 178 doesn't specify anonClickLabel, which means TalkBack/screen readers won't announce the action. Consider adding semantic info for better accessibility.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/VersionPicker.kt` around lines 169 - 241, The clickable modifier in VersionListItem lacks an accessibility label; update the Modifier.clickable(onClick = onClick) call inside VersionListItem to supply an onClickLabel (e.g., stringResource for a new string like "select_version_action") or add appropriate semantics (Modifier.semantics { this.onClick(label = ...) }/role = Role.Button) so screen readers announce the action and the selected state; ensure the label uses the release.tagName (or a formatted string resource) to provide context to TalkBack/VoiceOver.
225-229: Fragile date extraction viatake(10).
release.publishedAt.take(10)assumes a fixed ISO 8601 prefix (YYYY-MM-DD). This works for the GitHub API's current format but is brittle — consider parsing the date properly (e.g., viakotlinx-datetime) and formatting it according to the user's locale for a more robust and localized display.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/VersionPicker.kt` around lines 225 - 229, The code in VersionPicker.kt uses release.publishedAt.take(10), which is brittle; parse the publishedAt string as a date/time (e.g., use kotlinx.datetime.Instant.parse or kotlinx.datetime.LocalDateTime parsing), convert to the appropriate local date/time (with TimeZone.currentSystemDefault() or platform-specific zone), and format it with a proper date formatter (user-locale aware) instead of substringing; replace the Text(...) usage that references release.publishedAt.take(10) with the formatted date string and ensure parsing errors are handled (fallback to the original string) so the UI remains stable.
118-166: Bottom sheet dismiss animation is cut short.When the user swipes to dismiss,
onDismissRequestimmediately setsisPickerVisible = false, removing the sheet from composition before its slide-down animation completes. The recommended M3 pattern is to animate the sheet to hidden state first, then update the visibility boolean.♻️ Suggested approach
Hoist a
CoroutineScopeand usesheetState.hide()before toggling visibility:+ val scope = rememberCoroutineScope() + ModalBottomSheet( - onDismissRequest = { onAction(DetailsAction.ToggleVersionPicker) }, + onDismissRequest = { + scope.launch { + sheetState.hide() + }.invokeOnCompletion { + onAction(DetailsAction.ToggleVersionPicker) + } + }, sheetState = sheetState ) {You'll also need to add
import kotlinx.coroutines.launchandimport androidx.compose.runtime.rememberCoroutineScope.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/VersionPicker.kt` around lines 118 - 166, The bottom sheet is removed from composition immediately in the onDismissRequest which cuts off the dismiss animation; fix by hoisting a CoroutineScope via rememberCoroutineScope(), obtain the existing ModalBottomSheetState (sheetState from rememberModalBottomSheetState) and in onDismissRequest launch a coroutine that calls sheetState.hide() and only after hide() completes dispatch DetailsAction.ToggleVersionPicker to update isPickerVisible; also add imports for rememberCoroutineScope and kotlinx.coroutines.launch so the coroutine call compiles.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/VersionPicker.kt`:
- Around line 169-241: The clickable modifier in VersionListItem lacks an
accessibility label; update the Modifier.clickable(onClick = onClick) call
inside VersionListItem to supply an onClickLabel (e.g., stringResource for a new
string like "select_version_action") or add appropriate semantics
(Modifier.semantics { this.onClick(label = ...) }/role = Role.Button) so screen
readers announce the action and the selected state; ensure the label uses the
release.tagName (or a formatted string resource) to provide context to
TalkBack/VoiceOver.
- Around line 225-229: The code in VersionPicker.kt uses
release.publishedAt.take(10), which is brittle; parse the publishedAt string as
a date/time (e.g., use kotlinx.datetime.Instant.parse or
kotlinx.datetime.LocalDateTime parsing), convert to the appropriate local
date/time (with TimeZone.currentSystemDefault() or platform-specific zone), and
format it with a proper date formatter (user-locale aware) instead of
substringing; replace the Text(...) usage that references
release.publishedAt.take(10) with the formatted date string and ensure parsing
errors are handled (fallback to the original string) so the UI remains stable.
- Around line 118-166: The bottom sheet is removed from composition immediately
in the onDismissRequest which cuts off the dismiss animation; fix by hoisting a
CoroutineScope via rememberCoroutineScope(), obtain the existing
ModalBottomSheetState (sheetState from rememberModalBottomSheetState) and in
onDismissRequest launch a coroutine that calls sheetState.hide() and only after
hide() completes dispatch DetailsAction.ToggleVersionPicker to update
isPickerVisible; also add imports for rememberCoroutineScope and
kotlinx.coroutines.launch so the coroutine call compiles.
This commit introduces a "pending install" state to more accurately reflect the app status between a successful download/install trigger and the system confirming the installation. This prevents users from interacting with an app that is about to be updated and improves the overall robustness of the app synchronization and update process. - **feat(apps)**: Added a `isPendingInstall` flag to `InstalledApp`. This state is now displayed in the app list, and the "Open" button is disabled for apps in this state. - **feat(apps)**: The `SyncInstalledAppsUseCase` now resolves pending installs. When a sync runs, it checks if a pending app is now present on the system and updates its version information accordingly, clearing the pending flag. - **refactor(update)**: The update process now marks an app as `pending` before initiating the installation. The database record is updated with the new version information only *after* the installation intent has been sent. - **fix(update)**: The update availability check is now more resilient. If downloading a temporary APK to check its version fails, it gracefully falls back to using the release tag name for comparison, preventing check failures due to network errors. - **feat(details)**: The version picker in the app details screen now displays a "Latest" badge next to the most recent release, improving clarity for users.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/use_cases/SyncInstalledAppsUseCase.kt (1)
14-19:⚠️ Potential issue | 🟡 MinorUpdate KDoc to document the new pending-install resolution responsibility.
The class-level comment still lists only two responsibilities and no longer accurately describes the use case.
📝 Proposed update
* Responsibilities: * 1. Remove apps from DB that are no longer installed on the system * 2. Migrate legacy apps missing versionName/versionCode fields + * 3. Resolve pending installs once they appear in the system package manager🤖 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/use_cases/SyncInstalledAppsUseCase.kt` around lines 14 - 19, The class KDoc for SyncInstalledAppsUseCase is out of date — update the top-level comment above the SyncInstalledAppsUseCase declaration to include the new responsibility that the use case also resolves "pending-install" app entries (e.g., detect and reconcile entries marked pending-install with the current PackageManager state, updating install flags and versionName/versionCode as needed), alongside the existing responsibilities to remove uninstalled apps and migrate legacy version fields; keep the style consistent with the existing two-point list and mention that this resolution runs before loading/refreshing app data for consistency.
🧹 Nitpick comments (3)
core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/use_cases/SyncInstalledAppsUseCase.kt (1)
163-169:executeInTransactionprovides no transactional semantics — the name is misleading.The function is a plain call-through; the try-catch re-throws unconditionally, so it adds no value over calling the lambda directly. A reader seeing
executeInTransaction { … }reasonably expects atomicity/rollback guarantees that do not exist. If true atomicity is not supported, either rename this to something likeexecuteSequentiallyor remove the wrapper entirely to avoid false confidence.🤖 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/use_cases/SyncInstalledAppsUseCase.kt` around lines 163 - 169, The wrapper function executeInTransaction currently only invokes the lambda and rethrows exceptions, providing no transactional semantics; either implement real transaction handling using your persistence/transaction manager (e.g., wrap block in a transaction/rollback mechanism and commit on success) or remove/rename the wrapper to avoid misleading callers (rename to executeSequentially or inline callers and delete executeInTransaction). Update all call sites that use executeInTransaction to reflect the chosen approach (use the real transaction API or call the lambda directly/renamed method) and remove the noop try-catch.feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/VersionPicker.kt (2)
119-120: PreferskipPartiallyExpanded = truefor a selection picker.With
skipPartiallyExpanded = false(the default), the sheet opens in a half-height state, making the user drag it up before they can see the full release list. For a picker with up to 30 items this is a noticeable UX friction point; the sheet should expand fully on open.♻️ Proposed change
- val sheetState = rememberModalBottomSheetState(skipPartiallyExpanded = false) + val sheetState = rememberModalBottomSheetState(skipPartiallyExpanded = true)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/VersionPicker.kt` around lines 119 - 120, Change the modal sheet to open fully by setting skipPartiallyExpanded = true when creating the sheet state; locate the picker visibility block where isPickerVisible is checked and rememberModalBottomSheetState(...) is called (the variable sheetState) and change its parameter from skipPartiallyExpanded = false to skipPartiallyExpanded = true so the version picker expands to full height on open.
242-246:publishedAt.take(10)is a fragile raw string slice.This silently assumes the field always starts with
YYYY-MM-DD(10+ chars). A malformed or shorter value produces a truncated/empty string with no indication of failure, and there's no locale-aware date formatting.Consider parsing the date through a
kotlinx-datetimeInstant/LocalDateand formatting it with the user's locale rather than relying on a raw substring.- Text( - text = release.publishedAt.take(10), + val displayDate = remember(release.publishedAt) { + runCatching { + Instant.parse(release.publishedAt) + .toLocalDateTime(TimeZone.currentSystemDefault()) + .date + .toString() // or format via a DateTimeFormatter + }.getOrElse { release.publishedAt.take(10) } + } + Text( + text = displayDate,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/VersionPicker.kt` around lines 242 - 246, The code in VersionPicker.kt currently uses a fragile raw slice release.publishedAt.take(10) inside the Text composable; instead, parse the string using kotlinx-datetime (e.g., Instant.parse(release.publishedAt) or a safe parse function), convert to LocalDate/LocalDateTime using TimeZone.currentSystemDefault(), then format it with a locale-aware formatter for display; wrap parsing/formatting in a try/catch or a safe-parse helper and fall back to a sensible placeholder (e.g., the original string or "-") if parsing fails so the Text composable always receives a safe, user-formatted date.
🤖 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/InstalledAppsRepositoryImpl.kt`:
- Around line 173-198: The temp APK file isn't deleted if
installer.getApkInfoExtractor().extractPackageInfo(tempPath) throws; wrap the
extraction in a try/finally inside the block after
downloader.getDownloadedFilePath(tempAssetName) so File(tempPath).delete() is
executed in finally, and also in the outer catch (surrounding
downloader.download/collect and extraction) attempt to delete any non-null
tempPath (use downloader.getDownloadedFilePath or the local tempPath variable)
to ensure cleanup of partially downloaded files; update the code around
downloader.download, downloader.getDownloadedFilePath, and
installer.getApkInfoExtractor().extractPackageInfo to implement these
guaranteed-deletion steps.
In
`@core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/use_cases/SyncInstalledAppsUseCase.kt`:
- Around line 76-84: In SyncInstalledAppsUseCase, stop overwriting the
app.latestVersionName/latestVersionCode with systemInfo values when calling
installedAppsRepository.updateApp via app.copy; instead only update
installedVersionName, installedVersionCode, isPendingInstall (set false) and
compute isUpdateAvailable based on comparing the preserved latestVersion* (or by
re-fetching the repo latest release before updating) so the latest-release
pointer isn't silently discarded—locate the app.copy call inside the
pending-install branch and remove assignment of
latestVersionName/latestVersionCode (or replace with values read from the
current app/latest-release source) and leave latest fields unchanged unless an
explicit refresh is performed.
In
`@feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsRoot.kt`:
- Around line 519-521: The Update button isn't checking app.isPendingInstall,
allowing re-triggering installs while a pending install is persisted; update the
Update button's enabled guard (the same place where Open button uses
app.isPendingInstall) to include !app.isPendingInstall in addition to the
existing checks (e.g., appItem.updateState == Idle and app.isUpdateAvailable),
so the Update button is disabled whenever app.isPendingInstall is true and thus
mirrors the Open button behavior.
In
`@feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsViewModel.kt`:
- Around line 473-482: The current markPendingUpdate function swallows
exceptions from installedAppsRepository.updatePendingStatus (in
markPendingUpdate), which prevents updateSingleApp from aborting when the
pending flag fails to be set; change markPendingUpdate to let the exception
propagate instead of catching and discarding it—either remove the try/catch or
rethrow the caught exception after logging (keep logger.debug/error usage), so
callers like updateSingleApp can catch the failure and abort
installer.install()/updateAppVersion as intended.
- Around line 270-282: Make markPendingUpdate(packageName) failures non-silent
and ensure pending flag is cleaned on any install error: propagate the exception
from markPendingUpdate so the flow stops if
installedAppsRepository.updatePendingStatus fails, and in the error/catch paths
around installer.install(...) (and in any finally) call
installedAppsRepository.updatePendingStatus(packageName, false) to clear
isPendingInstall if install() throws or is canceled; also avoid calling
updateAppVersion(...) until the pending flag was successfully set (i.e., after
markPendingUpdate returns) so the DB never advances the version when
markPendingUpdate failed—reference markPendingUpdate, installer.install,
updateAppVersion, and installedAppsRepository.updatePendingStatus (and consider
SyncInstalledAppsUseCase if you opt to add reconciliation logic later).
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/VersionPicker.kt`:
- Around line 179-183: The Row with Modifier.clickable in VersionPicker.kt
currently uses .clickable(onClick = onClick) which provides no accessibility
label; update the clickable call to include an onClickLabel (e.g.
.clickable(onClickLabel = "Select version", onClick = onClick)) so
TalkBack/screen readers announce the action; locate the Row in VersionPicker.kt
and change the Modifier.clickable invocation to pass a descriptive onClickLabel
(or use a localized string resource) alongside the existing onClick parameter.
---
Outside diff comments:
In
`@core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/use_cases/SyncInstalledAppsUseCase.kt`:
- Around line 14-19: The class KDoc for SyncInstalledAppsUseCase is out of date
— update the top-level comment above the SyncInstalledAppsUseCase declaration to
include the new responsibility that the use case also resolves "pending-install"
app entries (e.g., detect and reconcile entries marked pending-install with the
current PackageManager state, updating install flags and versionName/versionCode
as needed), alongside the existing responsibilities to remove uninstalled apps
and migrate legacy version fields; keep the style consistent with the existing
two-point list and mention that this resolution runs before loading/refreshing
app data for consistency.
---
Nitpick comments:
In
`@core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/use_cases/SyncInstalledAppsUseCase.kt`:
- Around line 163-169: The wrapper function executeInTransaction currently only
invokes the lambda and rethrows exceptions, providing no transactional
semantics; either implement real transaction handling using your
persistence/transaction manager (e.g., wrap block in a transaction/rollback
mechanism and commit on success) or remove/rename the wrapper to avoid
misleading callers (rename to executeSequentially or inline callers and delete
executeInTransaction). Update all call sites that use executeInTransaction to
reflect the chosen approach (use the real transaction API or call the lambda
directly/renamed method) and remove the noop try-catch.
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/VersionPicker.kt`:
- Around line 119-120: Change the modal sheet to open fully by setting
skipPartiallyExpanded = true when creating the sheet state; locate the picker
visibility block where isPickerVisible is checked and
rememberModalBottomSheetState(...) is called (the variable sheetState) and
change its parameter from skipPartiallyExpanded = false to skipPartiallyExpanded
= true so the version picker expands to full height on open.
- Around line 242-246: The code in VersionPicker.kt currently uses a fragile raw
slice release.publishedAt.take(10) inside the Text composable; instead, parse
the string using kotlinx-datetime (e.g., Instant.parse(release.publishedAt) or a
safe parse function), convert to LocalDate/LocalDateTime using
TimeZone.currentSystemDefault(), then format it with a locale-aware formatter
for display; wrap parsing/formatting in a try/catch or a safe-parse helper and
fall back to a sensible placeholder (e.g., the original string or "-") if
parsing fails so the Text composable always receives a safe, user-formatted
date.
core/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/InstalledAppsRepositoryImpl.kt
Outdated
Show resolved
Hide resolved
core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/use_cases/SyncInstalledAppsUseCase.kt
Show resolved
Hide resolved
feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsRoot.kt
Show resolved
Hide resolved
feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsViewModel.kt
Show resolved
Hide resolved
feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsViewModel.kt
Outdated
Show resolved
Hide resolved
...sentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/VersionPicker.kt
Show resolved
Hide resolved
This commit introduces several refinements to the app update process, data handling, and general code quality. The app update mechanism now more robustly handles pending installs. When an update is initiated, it is marked as "pending." `SyncInstalledAppsUseCase` will then resolve this status once the new version is detected by the system's package manager, correctly updating the version info and `isUpdateAvailable` flag. The "Update" button is now disabled for apps with a pending install to prevent redundant actions. Error handling during the update process has also been improved to ensure the pending status is cleared on failures like cancellations or rate-limiting. Additionally, this commit includes code cleanup across multiple modules by removing redundant comments and KDoc. - **refactor(apps)**: Implemented a more robust pending install flow. `SyncInstalledAppsUseCase` now resolves pending installs by checking against the system's package manager. - **fix(apps)**: The "Update" button is now hidden for apps that have a pending install. - **fix(apps)**: Ensured the `isPendingInstall` flag is cleared on update cancellation, rate limit errors, or other failures. - **refactor(data)**: Encapsulated APK info extraction within a `try...finally` block to guarantee the temporary APK file is deleted, even if parsing fails. - **refactor(search)**: Removed automatic phrase quoting from search queries to allow for more flexible keyword matching. - **chore**: Removed redundant comments and KDoc from various files, including `DetailsViewModel`, `SearchRepositoryImpl`, `DesktopDeepLink`, and UI components.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/use_cases/SyncInstalledAppsUseCase.kt (1)
42-60:⚠️ Potential issue | 🟠 MajorPending installs that never complete (cancelled/failed) become orphaned DB entries.
When
isPendingInstall == truebut the app is not on the system (user cancelled the OS install dialog, or the install failed after the flag was set), the firstwhenbranch matches but does nothing. The!isOnSystemdelete branch is never reached becauseisPendingInstallis matched first. This leaves a ghost record withisPendingInstall = trueforever — it's never deleted and never resolved.🐛 Proposed fix — clear or delete orphaned pending installs
app.isPendingInstall -> { if (isOnSystem) { toResolvePending.add(app) + } else { + // App was never actually installed (user cancelled or install failed) + toDelete.add(app.packageName) } }Alternatively, if you want to keep the DB record but just clear the pending flag, replace
toDelete.add(...)with a call toupdatePendingStatus(app.packageName, false)and add it to a separate list for resolution.🤖 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/use_cases/SyncInstalledAppsUseCase.kt` around lines 42 - 60, The loop over apps treats any app with app.isPendingInstall as matched and does nothing when the package is no longer on the device, leaving orphaned DB rows; modify the logic in SyncInstalledAppsUseCase (the block using isPendingInstall, installedPackageNames, toResolvePending, toDelete, determineMigrationData, toMigrate) so that when app.isPendingInstall && !isOnSystem you either add the package to toDelete or call updatePendingStatus(app.packageName, false) and add to a separate list for pending-resolution; ensure the new branch runs before or replaces the current empty isPendingInstall branch so orphaned pending installs are either removed or have their pending flag cleared.feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt (2)
244-260:⚠️ Potential issue | 🟡 MinorCategory defaults to STABLE even when the selected release is a pre-release — UI inconsistency.
When all releases are pre-releases, line 245 falls back to
allReleases.firstOrNull()(a pre-release), but line 260 hardcodesselectedReleaseCategory = ReleaseCategory.STABLE. The derivedfilteredReleasesinDetailsStatewill filter for stable releases only → empty list. The version picker would show a selected release that doesn't appear in the filtered list.🐛 Proposed fix
- val selectedRelease = allReleases.firstOrNull { !it.isPrerelease } - ?: allReleases.firstOrNull() + val firstStable = allReleases.firstOrNull { !it.isPrerelease } + val selectedRelease = firstStable ?: allReleases.firstOrNull() + val initialCategory = if (firstStable != null) ReleaseCategory.STABLE else ReleaseCategory.ALL // ... later in state copy ... - selectedReleaseCategory = ReleaseCategory.STABLE, + selectedReleaseCategory = initialCategory,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt` around lines 244 - 260, In DetailsViewModel (around where selectedRelease is chosen and state updated), the code always sets selectedReleaseCategory = ReleaseCategory.STABLE causing the UI to filter out the chosen pre-release; instead compute the category from the selectedRelease (e.g., if selectedRelease.isPrerelease then ReleaseCategory.PRERELEASE else ReleaseCategory.STABLE) and assign that to selectedReleaseCategory before updating _state so filteredReleases and the version picker remain consistent; update any related logic that assumes a hardcoded STABLE value (references: selectedRelease, selectedReleaseCategory, DetailsViewModel, DetailsState, ReleaseCategory, recomputeAssetsForRelease).
352-370:⚠️ Potential issue | 🟡 MinorFavoriting with
selectedReleaserecords the user-selected version as "latest", not the actual latest.
latestVersionandlatestReleaseUrlin theFavoriteRepoare sourced fromselectedRelease, which could be any version the user picked. If the user has selected v1.0 while v2.0 exists, the favorites list will show "v1.0" as the latest version. Consider usingallReleases.firstOrNull()(or the first stable release) instead.🐛 Proposed fix
+ val actualLatest = _state.value.allReleases.firstOrNull { !it.isPrerelease } + ?: _state.value.allReleases.firstOrNull() + val favoriteRepo = FavoriteRepo( // ... - latestVersion = selectedRelease?.tagName, - latestReleaseUrl = selectedRelease?.htmlUrl, + latestVersion = actualLatest?.tagName, + latestReleaseUrl = actualLatest?.htmlUrl, // ... )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt` around lines 352 - 370, The favoriting logic in DetailsAction.OnToggleFavorite incorrectly sets FavoriteRepo.latestVersion and latestReleaseUrl from _state.value.selectedRelease (which may be a user-picked older release); change it to derive the "latest" release from the releases collection (e.g., use allReleases.firstOrNull() or the first stable release) and populate latestVersion/latestReleaseUrl from that release instead, while keeping selectedRelease preserved separately; update the FavoriteRepo construction in the OnToggleFavorite handler to use the chosen latestRelease (allReleases.firstOrNull() or stable selection) for latestVersion/latestReleaseUrl and retain repo fields from _state.value.repository.
🧹 Nitpick comments (5)
core/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/InstalledAppsRepositoryImpl.kt (1)
296-299: Add a targeted DAO query forupdatePendingStatusto eliminate the read-modify-write race condition.The current pattern reads the entire entity, modifies
isPendingInstall, and writes the whole object back. A concurrent update between the read and write could silently discard changes to other fields. The DAO already uses targeted queries elsewhere (e.g.,updateVersionInfo,updateLastChecked); this method should follow the same pattern.♻️ Proposed refactor
Add to
InstalledAppDao:`@Query`("UPDATE installed_apps SET isPendingInstall = :isPending WHERE packageName = :packageName") suspend fun updatePendingStatus(packageName: String, isPending: Boolean)Then simplify
updatePendingStatusinInstalledAppsRepositoryImpl:override suspend fun updatePendingStatus(packageName: String, isPending: Boolean) { - val app = installedAppsDao.getAppByPackage(packageName) ?: return - installedAppsDao.updateApp(app.copy(isPendingInstall = isPending)) + installedAppsDao.updatePendingStatus(packageName, isPending) }🤖 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/InstalledAppsRepositoryImpl.kt` around lines 296 - 299, Replace the read-modify-write in InstalledAppsRepositoryImpl.updatePendingStatus with a targeted DAO update to avoid race conditions: add a new suspend DAO method InstalledAppDao.updatePendingStatus(packageName: String, isPending: Boolean) annotated with a SQL UPDATE that sets isPendingInstall for the given packageName, then change InstalledAppsRepositoryImpl.updatePendingStatus to call that DAO method directly (remove the installedAppsDao.getAppByPackage/read and app.copy/write flow) so only the isPendingInstall column is updated atomically.core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/use_cases/SyncInstalledAppsUseCase.kt (1)
163-168:executeInTransactionis not a transaction — the name is misleading.This function simply calls
block()and re-throws on error; there is no actual transactional rollback or atomicity guarantee. If a deletion succeeds but a subsequent migration fails, the operations are not rolled back together. Consider renaming it (e.g.,executeSyncOperations) or removing the wrapper entirely, since it adds no behavior.🤖 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/use_cases/SyncInstalledAppsUseCase.kt` around lines 163 - 168, The wrapper function executeInTransaction in SyncInstalledAppsUseCase is misleading because it only calls block() and re-throws exceptions without providing transactional semantics; either implement real transactionality via the backing persistence layer (e.g., using a database transaction API inside executeInTransaction) or remove/rename the wrapper and its pointless try/catch (for example rename to executeSyncOperations or inline the block call where used). Locate executeInTransaction in SyncInstalledAppsUseCase, then either (A) replace the body with calls into your repository/DB transaction methods to ensure rollback on failure, or (B) delete the wrapper and call the block directly (or rename it) and remove the redundant try/catch to avoid the misleading name.feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/SmartInstallButton.kt (1)
78-91: Button text now reflects the user-selected release rather than the latest — verify this is the intended UX for downgrades.With
selectedReleasereplacinglatestRelease, if a user picks an older version via the version picker, the button will read "Update" even though it's technically a downgrade. This is fine if intentional, but consider whether the label should differ for downgrades (e.g., "Install" or "Downgrade") to avoid confusing users who expect "Update" to mean a newer version.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/SmartInstallButton.kt` around lines 78 - 91, The buttonText computation in SmartInstallButton.kt currently uses state.selectedRelease?.tagName to decide "Update" which makes a downgrade show as an update; change the logic in the buttonText block (the when that references installedApp, state.selectedRelease?.tagName, isUpdateAvailable, isInstalled) to detect whether the selectedRelease is newer or older than the installedApp (compare selectedRelease version vs installedApp.installedVersion/installedApp.latestVersion using your semver/Version comparator) and pick different strings for upgrade vs downgrade (e.g., Res.string.update_app for newer, Res.string.downgrade_app or Res.string.install_latest for older) so the label reflects an actual upgrade vs a downgrade. Ensure you update any isUpdateAvailable usage if it assumes selectedRelease is always newer.feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsViewModel.kt (1)
233-233: Unused variablelatestAssetSize.
latestAssetSizeis assigned fromprimaryAsset.sizebut never referenced in the rest ofupdateSingleApp.♻️ Proposed fix
- val latestAssetSize = primaryAsset.size🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsViewModel.kt` at line 233, In updateSingleApp remove the unused local val latestAssetSize (assigned from primaryAsset.size) or, if intended for use, reference it where needed (e.g., in comparison/logging or when updating asset size fields); locate the assignment to latestAssetSize in the updateSingleApp function and either delete that line or replace its usage into the appropriate logic that consumes primaryAsset.size (e.g., size checks, state updates, or debug logs).feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/VersionPicker.kt (1)
242-246:publishedAt.take(10)is brittle date formatting.This relies on
publishedAtalways being in ISO 8601 format ("2026-02-17T...") so that the first 10 characters yield"2026-02-17". If the field is ever empty or in a different format, this will silently display garbage. Consider usingkotlinx.datetimeto parse and format the date properly, or at minimum add a safe fallback.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/VersionPicker.kt` around lines 242 - 246, The current use of release.publishedAt.take(10) in VersionPicker.kt is brittle; replace it with safe parsing/formatting using kotlinx.datetime (e.g., Instant.parse or LocalDateTime.parse as appropriate) and format to a date string (or use DateTimeFormatter) to render a consistent "yyyy-MM-dd" (or localized) value, and fall back to a safe placeholder like "-" when parsing fails or the string is blank; locate the Text usage that references release.publishedAt in the VersionPicker composable and implement the parse+format+fallback logic there.
🤖 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/InstalledAppsRepositoryImpl.kt`:
- Around line 194-196: The catch in InstalledAppsRepositoryImpl currently logs
"Failed to download APK..." even when extractPackageInfo throws; change the
logging to reflect download vs extraction by either (a) broadening the message
to "Failed to download or extract APK for version check of ${app.packageName}:
${e.message}" in the Logger.w call, or (b) inspect
downloader.getDownloadedFilePath(tempAssetName) inside the catch and log "Failed
to download..." if null and "Failed to extract APK..." if a downloaded file
exists; ensure you still call
downloader.getDownloadedFilePath(tempAssetName)?.let { File(it).delete() } to
cleanup.
In
`@core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/use_cases/SyncInstalledAppsUseCase.kt`:
- Around line 76-86: The logger claims pending installs are resolved even when
app.latestVersionCode is null because the updateApp call is inside a let that
skips on null; update SyncInstalledAppsUseCase so
installedAppsRepository.updateApp is always called to clear isPendingInstall
(set isPendingInstall = false and update
installedVersionName/installedVersionCode from systemInfo), and derive
isUpdateAvailable only when app.latestVersionCode != null (e.g.,
isUpdateAvailable = app.latestVersionCode?.let { it > systemInfo.versionCode }
?: false); then keep the logger.info("Resolved pending install:
${app.packageName}...") after that update so the DB state matches the log.
---
Outside diff comments:
In
`@core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/use_cases/SyncInstalledAppsUseCase.kt`:
- Around line 42-60: The loop over apps treats any app with app.isPendingInstall
as matched and does nothing when the package is no longer on the device, leaving
orphaned DB rows; modify the logic in SyncInstalledAppsUseCase (the block using
isPendingInstall, installedPackageNames, toResolvePending, toDelete,
determineMigrationData, toMigrate) so that when app.isPendingInstall &&
!isOnSystem you either add the package to toDelete or call
updatePendingStatus(app.packageName, false) and add to a separate list for
pending-resolution; ensure the new branch runs before or replaces the current
empty isPendingInstall branch so orphaned pending installs are either removed or
have their pending flag cleared.
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt`:
- Around line 244-260: In DetailsViewModel (around where selectedRelease is
chosen and state updated), the code always sets selectedReleaseCategory =
ReleaseCategory.STABLE causing the UI to filter out the chosen pre-release;
instead compute the category from the selectedRelease (e.g., if
selectedRelease.isPrerelease then ReleaseCategory.PRERELEASE else
ReleaseCategory.STABLE) and assign that to selectedReleaseCategory before
updating _state so filteredReleases and the version picker remain consistent;
update any related logic that assumes a hardcoded STABLE value (references:
selectedRelease, selectedReleaseCategory, DetailsViewModel, DetailsState,
ReleaseCategory, recomputeAssetsForRelease).
- Around line 352-370: The favoriting logic in DetailsAction.OnToggleFavorite
incorrectly sets FavoriteRepo.latestVersion and latestReleaseUrl from
_state.value.selectedRelease (which may be a user-picked older release); change
it to derive the "latest" release from the releases collection (e.g., use
allReleases.firstOrNull() or the first stable release) and populate
latestVersion/latestReleaseUrl from that release instead, while keeping
selectedRelease preserved separately; update the FavoriteRepo construction in
the OnToggleFavorite handler to use the chosen latestRelease
(allReleases.firstOrNull() or stable selection) for
latestVersion/latestReleaseUrl and retain repo fields from
_state.value.repository.
---
Duplicate comments:
In
`@core/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/InstalledAppsRepositoryImpl.kt`:
- Around line 178-189: No change required: the inner try-finally correctly
ensures File(tempPath).delete() regardless of extractPackageInfo throwing, and
the null-latestInfo branch falls back to latestRelease.tagName; keep
extractPackageInfo(...) and File(tempPath).delete() as-is and ensure the outer
catch that logs "Failed to download APK" remains to handle exceptions escaping
the inner block.
In
`@feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsViewModel.kt`:
- Around line 494-497: The markPendingUpdate function must propagate errors
instead of swallowing them so install flows abort on failure; remove any silent
try/catch around installedAppsRepository.updatePendingStatus and rely on the
exception to bubble up from updatePendingStatus(packageName, true), while
retaining the logger.debug("Marked ${app.packageName} as pending install") for
success reporting in markPendingUpdate.
- Around line 270-324: Pending-install flag must be cleared on all error paths:
ensure markPendingUpdate(app) remains at start, wrap installer.install(filePath,
ext) so any thrown exception triggers
installedAppsRepository.updatePendingStatus(app.packageName, false), and verify
each catch block (CancellationException, RateLimitException, generic Exception)
calls installedAppsRepository.updatePendingStatus(...) and
cleanupUpdate(app.packageName, app.latestAssetName) as appropriate and then
resets state via updateAppState(...). Refer to markPendingUpdate,
installer.install, installedAppsRepository.updatePendingStatus, cleanupUpdate,
and updateAppState to locate and apply the fixes.
---
Nitpick comments:
In
`@core/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/InstalledAppsRepositoryImpl.kt`:
- Around line 296-299: Replace the read-modify-write in
InstalledAppsRepositoryImpl.updatePendingStatus with a targeted DAO update to
avoid race conditions: add a new suspend DAO method
InstalledAppDao.updatePendingStatus(packageName: String, isPending: Boolean)
annotated with a SQL UPDATE that sets isPendingInstall for the given
packageName, then change InstalledAppsRepositoryImpl.updatePendingStatus to call
that DAO method directly (remove the installedAppsDao.getAppByPackage/read and
app.copy/write flow) so only the isPendingInstall column is updated atomically.
In
`@core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/use_cases/SyncInstalledAppsUseCase.kt`:
- Around line 163-168: The wrapper function executeInTransaction in
SyncInstalledAppsUseCase is misleading because it only calls block() and
re-throws exceptions without providing transactional semantics; either implement
real transactionality via the backing persistence layer (e.g., using a database
transaction API inside executeInTransaction) or remove/rename the wrapper and
its pointless try/catch (for example rename to executeSyncOperations or inline
the block call where used). Locate executeInTransaction in
SyncInstalledAppsUseCase, then either (A) replace the body with calls into your
repository/DB transaction methods to ensure rollback on failure, or (B) delete
the wrapper and call the block directly (or rename it) and remove the redundant
try/catch to avoid the misleading name.
In
`@feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsViewModel.kt`:
- Line 233: In updateSingleApp remove the unused local val latestAssetSize
(assigned from primaryAsset.size) or, if intended for use, reference it where
needed (e.g., in comparison/logging or when updating asset size fields); locate
the assignment to latestAssetSize in the updateSingleApp function and either
delete that line or replace its usage into the appropriate logic that consumes
primaryAsset.size (e.g., size checks, state updates, or debug logs).
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/SmartInstallButton.kt`:
- Around line 78-91: The buttonText computation in SmartInstallButton.kt
currently uses state.selectedRelease?.tagName to decide "Update" which makes a
downgrade show as an update; change the logic in the buttonText block (the when
that references installedApp, state.selectedRelease?.tagName, isUpdateAvailable,
isInstalled) to detect whether the selectedRelease is newer or older than the
installedApp (compare selectedRelease version vs
installedApp.installedVersion/installedApp.latestVersion using your
semver/Version comparator) and pick different strings for upgrade vs downgrade
(e.g., Res.string.update_app for newer, Res.string.downgrade_app or
Res.string.install_latest for older) so the label reflects an actual upgrade vs
a downgrade. Ensure you update any isUpdateAvailable usage if it assumes
selectedRelease is always newer.
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/VersionPicker.kt`:
- Around line 242-246: The current use of release.publishedAt.take(10) in
VersionPicker.kt is brittle; replace it with safe parsing/formatting using
kotlinx.datetime (e.g., Instant.parse or LocalDateTime.parse as appropriate) and
format to a date string (or use DateTimeFormatter) to render a consistent
"yyyy-MM-dd" (or localized) value, and fall back to a safe placeholder like "-"
when parsing fails or the string is blank; locate the Text usage that references
release.publishedAt in the VersionPicker composable and implement the
parse+format+fallback logic there.
Comment on lines
194
to
196
| } catch (e: Exception) { | ||
| Logger.w { "Failed to download APK for version check of ${app.packageName}: ${e.message}" } | ||
| downloader.getDownloadedFilePath(tempAssetName)?.let { File(it).delete() } |
Contributor
There was a problem hiding this comment.
Log message is misleading when extraction, not download, throws.
Since extractPackageInfo's exception now propagates through the try-finally to this catch, the "Failed to download APK" message will appear even when the download itself succeeded. Consider broadening the message or distinguishing the two failure cases:
🔧 Proposed fix
- } catch (e: Exception) {
- Logger.w { "Failed to download APK for version check of ${app.packageName}: ${e.message}" }
- downloader.getDownloadedFilePath(tempAssetName)?.let { File(it).delete() }
- latestVersionName = latestRelease.tagName
- }
+ } catch (e: Exception) {
+ Logger.w { "Failed to download or extract APK for version check of ${app.packageName}: ${e.message}" }
+ downloader.getDownloadedFilePath(tempAssetName)?.let { File(it).delete() }
+ latestVersionName = latestRelease.tagName
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| } catch (e: Exception) { | |
| Logger.w { "Failed to download APK for version check of ${app.packageName}: ${e.message}" } | |
| downloader.getDownloadedFilePath(tempAssetName)?.let { File(it).delete() } | |
| } catch (e: Exception) { | |
| Logger.w { "Failed to download or extract APK for version check of ${app.packageName}: ${e.message}" } | |
| downloader.getDownloadedFilePath(tempAssetName)?.let { File(it).delete() } | |
| latestVersionName = latestRelease.tagName | |
| } |
🤖 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/InstalledAppsRepositoryImpl.kt`
around lines 194 - 196, The catch in InstalledAppsRepositoryImpl currently logs
"Failed to download APK..." even when extractPackageInfo throws; change the
logging to reflect download vs extraction by either (a) broadening the message
to "Failed to download or extract APK for version check of ${app.packageName}:
${e.message}" in the Logger.w call, or (b) inspect
downloader.getDownloadedFilePath(tempAssetName) inside the catch and log "Failed
to download..." if null and "Failed to extract APK..." if a downloaded file
exists; ensure you still call
downloader.getDownloadedFilePath(tempAssetName)?.let { File(it).delete() } to
cleanup.
Comment on lines
76
to
86
| app.latestVersionCode?.let { latestVersionCode -> | ||
| installedAppsRepository.updateApp( | ||
| app.copy( | ||
| isPendingInstall = false, | ||
| installedVersionName = systemInfo.versionName, | ||
| installedVersionCode = systemInfo.versionCode, | ||
| isUpdateAvailable = latestVersionCode > systemInfo.versionCode | ||
| ) | ||
| ) | ||
| } | ||
| logger.info("Resolved pending install: ${app.packageName} (v${systemInfo.versionName}, code=${systemInfo.versionCode})") |
Contributor
There was a problem hiding this comment.
When latestVersionCode is null, the pending flag is never cleared but the log claims it was resolved.
If app.latestVersionCode is null, the let block on line 76 is skipped entirely — no DB update occurs — yet logger.info("Resolved pending install: ...") on line 86 still runs unconditionally. The app remains stuck with isPendingInstall = true.
🐛 Proposed fix — handle the null case
if (systemInfo != null) {
- app.latestVersionCode?.let { latestVersionCode ->
- installedAppsRepository.updateApp(
- app.copy(
- isPendingInstall = false,
- installedVersionName = systemInfo.versionName,
- installedVersionCode = systemInfo.versionCode,
- isUpdateAvailable = latestVersionCode > systemInfo.versionCode
- )
- )
- }
- logger.info("Resolved pending install: ${app.packageName} (v${systemInfo.versionName}, code=${systemInfo.versionCode})")
+ val latestVersionCode = app.latestVersionCode ?: 0L
+ installedAppsRepository.updateApp(
+ app.copy(
+ isPendingInstall = false,
+ installedVersionName = systemInfo.versionName,
+ installedVersionCode = systemInfo.versionCode,
+ isUpdateAvailable = latestVersionCode > systemInfo.versionCode
+ )
+ )
+ logger.info("Resolved pending install: ${app.packageName} (v${systemInfo.versionName}, code=${systemInfo.versionCode})")🤖 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/use_cases/SyncInstalledAppsUseCase.kt`
around lines 76 - 86, The logger claims pending installs are resolved even when
app.latestVersionCode is null because the updateApp call is inside a let that
skips on null; update SyncInstalledAppsUseCase so
installedAppsRepository.updateApp is always called to clear isPendingInstall
(set isPendingInstall = false and update
installedVersionName/installedVersionCode from systemInfo), and derive
isUpdateAvailable only when app.latestVersionCode != null (e.g.,
isUpdateAvailable = app.latestVersionCode?.let { it > systemInfo.versionCode }
?: false); then keep the logger.info("Resolved pending install:
${app.packageName}...") after that update so the DB state matches the log.
This commit introduces several refinements to the app update process and the release selection UI. The app update check in `SyncInstalledAppsUseCase` now gracefully handles cases where `latestVersionCode` is null by defaulting it to `0L`, preventing potential null pointer issues. In the repository implementation, a log message has been clarified to indicate that failures during version checks can occur during either download or extraction of the APK. Additionally, the release category filter chips in the `VersionPicker` component are now wrapped in a `LazyRow` to ensure they scroll horizontally on smaller screens, preventing UI overflow. - **refactor(domain)**: Handled null `latestVersionCode` in `SyncInstalledAppsUseCase` by coalescing to `0L` for safer update comparisons. - **fix(details)**: Replaced `Row` with `LazyRow` for the release category `FilterChip`s to improve horizontal scrolling on small displays. - **chore(data)**: Clarified a warning log message in `InstalledAppsRepositoryImpl` to include both download and extraction failures for APK version checks.
This commit refactors the update-checking mechanism to be faster and more reliable by removing the pre-download and APK parsing steps. It now compares version tags directly. It also enhances the user experience on the "Apps" screen with pull-to-refresh, better status indicators, and improved handling of pending and stale app installations. - **refactor(data)**: Simplified `checkForUpdate` logic in `InstalledAppsRepositoryImpl`. The check for updates now directly compares the normalized installed version tag with the latest release tag, removing the need to download the APK for version code comparison. This makes the process significantly faster and less error-prone. - **feat(apps)**: Added pull-to-refresh functionality on the Apps screen to manually trigger a sync and check for updates. - **feat(apps)**: Implemented a "last checked" timestamp on the Apps screen to inform the user when updates were last fetched. An automatic check now runs on a 30-minute cooldown. - **feat(domain)**: Added logic to `SyncInstalledAppsUseCase` to automatically clean up pending installs that have not been completed within 24 hours. - **feat(ui)**: Introduced a "Pending Install" badge on the app details screen for apps that are awaiting installation. The install buttons are now correctly disabled for pending installs. - **chore(android)**: Registered a `PackageEventReceiver` in the main `Application` class to respond to app install/uninstall events, ensuring the database is kept in sync with the system's state.
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.
This commit introduces a version picker on the details screen, allowing users to select between stable, pre-release, and all available releases for an application.
Previously, only the latest published release was fetched and displayed. Now, the app fetches a list of all releases, automatically selects the latest stable version by default, and provides UI for the user to switch to other versions.
VersionPicker.kt) to the details screen, which allows users to filter and select from stable releases, pre-releases, or all available versions.DetailsViewModelandDetailsStateto manage the list of all releases, the selected release, and the state of the version picker.getAllReleasesinDetailsRepositoryto retrieve up to 30 recent releases from the GitHub API.Summary by CodeRabbit
New Features
UX Improvements
Localization
Bug Fixes