Conversation
This commit enhances user preferences by introducing sorting options for launcher apps and a toggle for displaying system apps. - UserPreferencesDataSource now supports updating and retrieving sorting preferences for launcher apps by name, update time, and install time, as well as the sort order (ascending/descending). - The UserData model has been updated to include these new sorting and visibility preferences. - The GetLauncherAppsActivityInfosUseCase now filters apps based on the showSystem preference and sorts them according to user-defined criteria. - PackageManagerWrapper has been extended with methods to get the last install time of an app and to check if an app is a system app. - Protobuf definitions for sorting preferences have been added. - Mappers have been implemented to facilitate the conversion between domain models and proto representations for sorting.
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds persisted launcher-app sort and visibility preferences, maps them through datastore and repository layers, enriches activity metadata from package information, and adds configurable filtering, ordering, and sorting controls to the apps UI. ChangesLauncher app sorting
Shared settings resources
Build and IDE configuration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant AppsScreen
participant AppsViewModel
participant UserDataRepository
participant GetLauncherAppsActivityInfosUseCase
AppsScreen->>AppsViewModel: submit sorting and visibility settings
AppsViewModel->>UserDataRepository: persist updated preferences
UserDataRepository->>GetLauncherAppsActivityInfosUseCase: emit updated preferences
GetLauncherAppsActivityInfosUseCase->>AppsScreen: return filtered and sorted activity data
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
data/datastore/src/main/kotlin/com/android/geto/data/datastore/UserPreferencesDataSource.kt (1)
77-80: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUpdate function call to match the suggested rename.
If you accept the suggestion to rename
asSortOrderLauncherAppsActivityInfotoasSortOrderLauncherAppsActivityInfoProtoinDataStoreMapper.kt, ensure the call site is updated here as well.♻️ Proposed fix
this.sortOrderLauncherAppsActivityInfo = - sortOrderLauncherAppsActivityInfo.asSortOrderLauncherAppsActivityInfo() + sortOrderLauncherAppsActivityInfo.asSortOrderLauncherAppsActivityInfoProto() }🤖 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 `@data/datastore/src/main/kotlin/com/android/geto/data/datastore/UserPreferencesDataSource.kt` around lines 77 - 80, Update the conversion call in the UserPreferencesDataSource assignment to use the renamed asSortOrderLauncherAppsActivityInfoProto function from DataStoreMapper.kt, preserving the existing assignment behavior.data/datastore/src/main/kotlin/com/android/geto/data/datastore/mapper/DataStoreMapper.kt (1)
57-61: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename function to clarify the return type.
The function converts a domain model to a proto model, but its name doesn't include
Protoat the end, unlikeasSortLauncherAppsActivityInfoProtoandasThemeProto. Renaming it toasSortOrderLauncherAppsActivityInfoProtomakes the naming convention consistent and prevents confusion with the function that maps from proto to domain.♻️ Proposed fix
-internal fun SortOrderLauncherAppsActivityInfo.asSortOrderLauncherAppsActivityInfo(): SortOrderLauncherAppsActivityInfoProto = when (this) { +internal fun SortOrderLauncherAppsActivityInfo.asSortOrderLauncherAppsActivityInfoProto(): SortOrderLauncherAppsActivityInfoProto = when (this) { SortOrderLauncherAppsActivityInfo.Ascending -> SortOrderLauncherAppsActivityInfoProto.SortOrderAscending SortOrderLauncherAppsActivityInfo.Descending -> SortOrderLauncherAppsActivityInfoProto.SortOrderDescending }🤖 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 `@data/datastore/src/main/kotlin/com/android/geto/data/datastore/mapper/DataStoreMapper.kt` around lines 57 - 61, Rename the domain-to-proto mapper function from asSortOrderLauncherAppsActivityInfo to asSortOrderLauncherAppsActivityInfoProto, and update all call sites to use the new name consistently with the existing mapper naming convention.domain/use-case/src/main/kotlin/com/android/geto/domain/usecase/GetLauncherAppsActivityInfosUseCase.kt (1)
36-78: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueUse case-insensitive sorting for app names.
The current
compareByforNamewill sort alphabetically with case sensitivity (so "Z" sorts before "a"), which may result in unexpected app grouping in the launcher. Additionally, adding a secondary sort by name for the time-based comparators ensures deterministic ordering when multiple apps are updated or installed at exactly the same time.♻️ Proposed refactor for sorting logic
val comparator = when (userData.sortLauncherAppsActivityInfo) { SortLauncherAppsActivityInfo.Name -> - compareBy<LauncherAppsActivityInfo> { it.activityLabel } + compareBy<LauncherAppsActivityInfo>(String.CASE_INSENSITIVE_ORDER) { it.activityLabel } SortLauncherAppsActivityInfo.UpdateTime -> - compareBy { it.lastUpdateTime } + compareBy<LauncherAppsActivityInfo> { it.lastUpdateTime } + .thenBy(String.CASE_INSENSITIVE_ORDER) { it.activityLabel } SortLauncherAppsActivityInfo.InstallTime -> - compareBy { it.firstInstallTime } + compareBy<LauncherAppsActivityInfo> { it.firstInstallTime } + .thenBy(String.CASE_INSENSITIVE_ORDER) { it.activityLabel } }🤖 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 `@domain/use-case/src/main/kotlin/com/android/geto/domain/usecase/GetLauncherAppsActivityInfosUseCase.kt` around lines 36 - 78, Update the comparator selection in invoke so Name sorting compares activityLabel case-insensitively. For UpdateTime and InstallTime, add a secondary activityLabel sort using the same case-insensitive comparison to make equal timestamps deterministic, while preserving the existing sort-order reversal and filtering behavior.data/datastore-proto/src/main/proto/com/android/geto/data/datastore/proto/sort_order_launcher_apps_activity_info.proto (1)
19-22: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDefine a package to prevent name conflicts.
The file is missing a protobuf
packagedeclaration, which is recommended to avoid naming collisions.♻️ Proposed refactor
syntax = "proto3"; +package com.android.geto.data.datastore.proto; + option java_package = "com.android.geto.data.datastore.proto"; option java_multiple_files = true;🤖 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 `@data/datastore-proto/src/main/proto/com/android/geto/data/datastore/proto/sort_order_launcher_apps_activity_info.proto` around lines 19 - 22, Add a protobuf package declaration to the proto definition alongside the existing syntax and Java options, using the project's established package namespace to prevent message name conflicts while preserving the current Java package configuration.Source: Linters/SAST tools
🤖 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
`@data/datastore-proto/src/main/proto/com/android/geto/data/datastore/proto/sort_order_launcher_apps_activity_info.proto`:
- Around line 19-22: Add a protobuf package declaration to the proto definition
alongside the existing syntax and Java options, using the project's established
package namespace to prevent message name conflicts while preserving the current
Java package configuration.
In
`@data/datastore/src/main/kotlin/com/android/geto/data/datastore/mapper/DataStoreMapper.kt`:
- Around line 57-61: Rename the domain-to-proto mapper function from
asSortOrderLauncherAppsActivityInfo to asSortOrderLauncherAppsActivityInfoProto,
and update all call sites to use the new name consistently with the existing
mapper naming convention.
In
`@data/datastore/src/main/kotlin/com/android/geto/data/datastore/UserPreferencesDataSource.kt`:
- Around line 77-80: Update the conversion call in the UserPreferencesDataSource
assignment to use the renamed asSortOrderLauncherAppsActivityInfoProto function
from DataStoreMapper.kt, preserving the existing assignment behavior.
In
`@domain/use-case/src/main/kotlin/com/android/geto/domain/usecase/GetLauncherAppsActivityInfosUseCase.kt`:
- Around line 36-78: Update the comparator selection in invoke so Name sorting
compares activityLabel case-insensitively. For UpdateTime and InstallTime, add a
secondary activityLabel sort using the same case-insensitive comparison to make
equal timestamps deterministic, while preserving the existing sort-order
reversal and filtering behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ed0dfbf1-ccfd-4821-95b3-fa56ea5c2db4
📒 Files selected for processing (17)
.idea/deploymentTargetSelector.xmldata/datastore-proto/src/main/proto/com/android/geto/data/datastore/proto/sort_launcher_apps_activity_info.protodata/datastore-proto/src/main/proto/com/android/geto/data/datastore/proto/sort_order_launcher_apps_activity_info.protodata/datastore-proto/src/main/proto/com/android/geto/data/datastore/proto/user_preferences.protodata/datastore/src/main/kotlin/com/android/geto/data/datastore/UserPreferencesDataSource.ktdata/datastore/src/main/kotlin/com/android/geto/data/datastore/mapper/DataStoreMapper.ktdata/repository/src/main/kotlin/com/android/geto/data/repository/DefaultUserDataRepository.ktdomain/framework/src/main/kotlin/com/android/geto/domain/framework/PackageManagerWrapper.ktdomain/model/src/main/kotlin/com/android/geto/domain/model/LauncherAppsActivityInfo.ktdomain/model/src/main/kotlin/com/android/geto/domain/model/SortLauncherAppsActivityInfo.ktdomain/model/src/main/kotlin/com/android/geto/domain/model/SortOrderLauncherAppsActivityInfo.ktdomain/model/src/main/kotlin/com/android/geto/domain/model/UserData.ktdomain/repository/src/main/kotlin/com/android/geto/domain/repository/UserDataRepository.ktdomain/use-case/src/main/kotlin/com/android/geto/domain/usecase/GetLauncherAppsActivityInfosUseCase.ktframework/launcher-apps/src/main/kotlin/com/android/geto/framework/launcherapps/DefaultLauncherAppsWrapper.ktframework/package-manager/src/main/kotlin/com/android/geto/framework/packagemanager/DefaultPackageManagerWrapper.ktgradle/gradle-daemon-jvm.properties
This commit refines the app sorting logic and improves the mapping between domain models and protobuf representations. - The `UserPreferencesDataSource` now correctly maps `SortOrderLauncherAppsActivityInfo` to its proto representation. - The `GetLauncherAppsActivityInfosUseCase` now includes case-insensitive sorting by activity label for all sorting criteria and uses `thenBy` for secondary sorting. - Mappers in `DataStoreMapper.kt` have been updated to use explicit `when` branches for clarity and to correctly map `SortOrderLauncherAppsActivityInfo` to its proto counterpart.
Enable parallel sync for Gradle versions 9.4 and above. This setting can improve build performance by allowing Gradle to perform more tasks concurrently.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@gradle.properties`:
- Around line 41-42: Update the Gradle wrapper configuration from version 9.3.1
to 9.4.0 or newer so the org.gradle.tooling.parallel=true setting takes effect,
preserving the existing parallel-sync property.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Comment on lines
+41
to
+42
| # Enabled parallel sync for Gradle 9.4+ | ||
| org.gradle.tooling.parallel=true |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -a 'gradle-wrapper.properties' -x sh -c 'echo "--- $1"; rg -n "distributionUrl" "$1"' sh {}Repository: JackEblan/Geto
Length of output: 299
Bump the Gradle wrapper to 9.4.0+ org.gradle.tooling.parallel=true won’t take effect until the wrapper is updated; the current wrapper is 9.3.1.
🤖 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 `@gradle.properties` around lines 41 - 42, Update the Gradle wrapper
configuration from version 9.3.1 to 9.4.0 or newer so the
org.gradle.tooling.parallel=true setting takes effect, preserving the existing
parallel-sync property.
This commit introduces app sorting functionality to the AppsScreen and refactors the data handling for app lists. - The `GetLauncherAppsActivityInfosUseCase` now encapsulates the filtered and sorted app list along with user data into a `LauncherAppsActivityInfoData` object. - The `AppsScreen` now displays this `LauncherAppsActivityInfoData` and exposes functions to update sorting preferences. - A new `SortLauncherAppsActivityInfoDialog` has been created to allow users to select sorting criteria (name, update time, install time) and order (ascending, descending). - The `AppsViewModel` now includes functions to update these sorting preferences via the `UserDataRepository`. - The `GetoIcons` object has been updated to include a `Sort` icon. - New string resources for sorting options have been added to `feature/apps/src/main/res/values/strings.xml`. - Dependency updates in `feature/apps/build.gradle.kts` ensure necessary modules are included. - `AppsUiState` has been updated to accommodate the new `LauncherAppsActivityInfoData`. - Minor UI adjustments were made in `ThemeDialog.kt` and `ShortcutDialog.kt` for consistent padding. - Redundant string resources for "add", "update", and "cancel" have been removed from `feature/app-settings/src/main/res/values/strings.xml` as they are now centralized in `common/src/main/res/values/strings.xml`.
This commit integrates the "Show System Apps" toggle into the `SortLauncherAppsActivityInfoDialog`. - The `AppsScreen` now passes the `showSystem` preference to the dialog. - The `SortLauncherAppsActivityInfoDialog` displays a `Switch` for toggling system app visibility. - A new `ShowSystemSetting` composable is introduced for this functionality. - The `AppsViewModel` now includes an `updateShowSystem` function to handle preference updates. - Minor UI adjustments in `SortLauncherAppsActivityInfoDialog` for better layout.
This commit refactors the `SortLauncherAppsActivityInfoDialog` to correctly manage and update the "Show System Apps" state. - A `selectedShowSystem` state variable is introduced to locally track the toggle's state within the dialog. - The `ShowSystemSetting` composable now uses this local state for its `showSystem` parameter and updates it via its `onUpdateShowSystem` lambda. - The `SortLauncherAppsActivityInfoDialogButtons` now receives `selectedShowSystem` and passes it to `onUpdateShowSystem` when the confirm button is clicked. - This ensures that the "Show System Apps" preference is accurately updated and reflected when the user confirms their selections in the dialog.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
feature/apps/src/main/kotlin/com/android/geto/feature/apps/dialog/SortLauncherAppsActivityInfoDialog.kt (1)
198-207: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFix the typo in the variable name.
The variable name
selectedSortOrdeLauncherAppsActivityInfois missing anrinOrder.♻️ Proposed refactor
- val selectedSortOrdeLauncherAppsActivityInfo = + val selectedSortOrderLauncherAppsActivityInfo = SortOrderLauncherAppsActivityInfo.entries.getOrNull( selectedSortOrderLauncherAppsActivityInfoIndex, ) selectedSortLauncherAppsActivityInfo?.let(onUpdateSortLauncherAppsActivityInfo) - selectedSortOrdeLauncherAppsActivityInfo?.let( + selectedSortOrderLauncherAppsActivityInfo?.let( onUpdateSortOrderLauncherAppsActivityInfo, )🤖 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/src/main/kotlin/com/android/geto/feature/apps/dialog/SortLauncherAppsActivityInfoDialog.kt` around lines 198 - 207, Rename the local variable selectedSortOrdeLauncherAppsActivityInfo to selectedSortOrderLauncherAppsActivityInfo and update its usage in the corresponding let call, preserving the existing behavior.feature/apps/src/main/kotlin/com/android/geto/feature/apps/AppsScreen.kt (1)
205-210: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUse a unique
keyfor items inLazyVerticalGrid.When sorting, filtering, or modifying the list, providing a stable, unique
keyhelps Compose optimize recompositions and correctly maintain scroll position and item state, rather than falling back to index-based keys.♻️ Proposed refactor
- items(items = launcherAppsActivityInfoData.launcherAppsActivityInfos) { launcherAppsActivityInfo -> + items( + items = launcherAppsActivityInfoData.launcherAppsActivityInfos, + key = { it.componentName }, + ) { launcherAppsActivityInfo -> AppItem( launcherAppsActivityInfo = launcherAppsActivityInfo, onClickApp = onClickApp, )🤖 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/src/main/kotlin/com/android/geto/feature/apps/AppsScreen.kt` around lines 205 - 210, Update the LazyVerticalGrid items call around launcherAppsActivityInfoData.launcherAppsActivityInfos to provide a stable, unique key for each launcherAppsActivityInfo, using its existing unique application/activity identifier rather than the list index. Keep the AppItem rendering and onClickApp behavior unchanged.
🤖 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
`@feature/apps/src/main/kotlin/com/android/geto/feature/apps/dialog/SortLauncherAppsActivityInfoDialog.kt`:
- Around line 236-248: Replace the hardcoded “Show System” and “Show system
applications” values in the dialog’s Column with stringResource() lookups, and
add corresponding show_system and show_system_description entries to strings.xml
using the current text as defaults.
---
Nitpick comments:
In `@feature/apps/src/main/kotlin/com/android/geto/feature/apps/AppsScreen.kt`:
- Around line 205-210: Update the LazyVerticalGrid items call around
launcherAppsActivityInfoData.launcherAppsActivityInfos to provide a stable,
unique key for each launcherAppsActivityInfo, using its existing unique
application/activity identifier rather than the list index. Keep the AppItem
rendering and onClickApp behavior unchanged.
In
`@feature/apps/src/main/kotlin/com/android/geto/feature/apps/dialog/SortLauncherAppsActivityInfoDialog.kt`:
- Around line 198-207: Rename the local variable
selectedSortOrdeLauncherAppsActivityInfo to
selectedSortOrderLauncherAppsActivityInfo and update its usage in the
corresponding let call, preserving the existing behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: bb612dfe-b88f-40f1-a839-ee9597186865
📒 Files selected for processing (14)
common/src/main/res/values/strings.xmldesign-system/src/main/kotlin/com/android/geto/designsystem/icon/GetoIcons.ktdomain/model/src/main/kotlin/com/android/geto/domain/model/LauncherAppsActivityInfoData.ktdomain/use-case/src/main/kotlin/com/android/geto/domain/usecase/GetLauncherAppsActivityInfosUseCase.ktfeature/app-settings/src/main/kotlin/com/android/geto/feature/appsettings/dialog/AppSettingDialog.ktfeature/app-settings/src/main/kotlin/com/android/geto/feature/appsettings/dialog/ShortcutDialog.ktfeature/app-settings/src/main/res/values/strings.xmlfeature/apps/build.gradle.ktsfeature/apps/src/main/kotlin/com/android/geto/feature/apps/AppsScreen.ktfeature/apps/src/main/kotlin/com/android/geto/feature/apps/AppsUiState.ktfeature/apps/src/main/kotlin/com/android/geto/feature/apps/AppsViewModel.ktfeature/apps/src/main/kotlin/com/android/geto/feature/apps/dialog/SortLauncherAppsActivityInfoDialog.ktfeature/apps/src/main/res/values/strings.xmlfeature/settings/src/main/kotlin/com/android/geto/feature/settings/dialog/ThemeDialog.kt
💤 Files with no reviewable changes (1)
- feature/app-settings/src/main/res/values/strings.xml
🚧 Files skipped from review as they are similar to previous changes (1)
- domain/use-case/src/main/kotlin/com/android/geto/domain/usecase/GetLauncherAppsActivityInfosUseCase.kt
This commit replaces hardcoded strings for "Show System" and "Show system applications" with references to string resources. - The `SortLauncherAppsActivityInfoDialog.kt` file is updated to use `stringResource(R.string.show_system)` and `stringResource(R.string.show_system_applications)`. - New string resources `show_system` and `show_system_applications` are added to `feature/apps/src/main/res/values/strings.xml`. This change improves localization and maintainability of the UI strings.
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 enhances user preferences by introducing sorting options for launcher apps and a toggle for displaying system apps.
Closes #379
Summary by CodeRabbit