Skip to content

Feat: Add overlay permission guidance and persistence#19

Merged
meticha-admin merged 7 commits into
meticha:mainfrom
bhavikmaradiya:feature/manual-permission-handlation
Jul 2, 2025
Merged

Feat: Add overlay permission guidance and persistence#19
meticha-admin merged 7 commits into
meticha:mainfrom
bhavikmaradiya:feature/manual-permission-handlation

Conversation

@bhavikmaradiya

@bhavikmaradiya bhavikmaradiya commented Jul 1, 2025

Copy link
Copy Markdown
Contributor

This commit introduces a dialog to guide users on enabling "display pop-up windows while running in the background" permission, particularly for devices like Xiaomi, Oppo, and Vivo. The user's acknowledgment of this dialog is now persisted using DataStore.

Key changes:

  • triggerx/src/main/java/com/meticha/triggerx/permission/TriggerXPermissionComposable.kt:
    • Added PermissionType.OVERLAY and PermissionType.OVERLAY_WHILE_BACKGROUND to the default permissions list.
    • Implemented ShowPermissionGuidanceDialog to display specific instructions for OVERLAY_WHILE_BACKGROUND.
    • Integrated TriggerXPermissionFlagManager to save and check the acknowledgment state of the guidance dialog.
    • Permission requests and dialog showings are now launched within a CoroutineScope.
    • Added ShowManualPermissionDialog composable for generic manual permission guidance.
  • triggerx/src/main/java/com/meticha/triggerx/permission/TriggerXPermissionManager.kt:
    • Added OVERLAY_WHILE_BACKGROUND to PermissionType enum, marking it as a isManualPermissionType.
    • Implemented isOverlayBackgroundPermissionEnabled to check acknowledgment status via TriggerXPermissionFlagManager.
    • isGranted now handles OVERLAY_WHILE_BACKGROUND by checking its acknowledgment status.
    • createPermissionIntent now returns null for PermissionTypes that are manually configured by the user (like OVERLAY_WHILE_BACKGROUND).
    • Added OverlayPermissionGuidanceDialog composable (though currently unused directly, parts of its logic are adapted elsewhere).
    • PermissionState.requestPermission now checks for isManualPermissionType to show showPermissionGuidanceDialog instead of launching an intent.
    • PermissionState.allRequiredGranted and PermissionState.isGranted are now suspend functions.
    • PermissionState.next is now a suspend function.
  • app/src/main/java/com/meticha/triggerxexample/home/HomeScreen.kt:
    • Permission requests (permissionState.requestPermission()) and checks (permissionState.allRequiredGranted()) are now called within a CoroutineScope.
  • triggerx/src/main/java/com/meticha/triggerx/permission/PermissionLifeCycleCheckEffect.kt:
    • permissionState.requestPermission() is now launched within a CoroutineScope in the lifecycle observer.
  • app/src/main/java/com/meticha/triggerxexample/home/HomeViewModel.kt:
    • scheduleOneMinuteAlarm and cancelCurrentAlarm now launch their operations within viewModelScope.
  • app/src/main/AndroidManifest.xml:
    • Added android.permission.SYSTEM_ALERT_WINDOW permission.
  • triggerx/src/main/java/com/meticha/triggerx/preference/TriggerXAlarmIdManager.kt:
    • Added TriggerXPermissionFlagManager object to save and retrieve boolean flags for permission dialog acknowledgments using DataStore.
    • Implemented savePermissionDialogResponse and isPermissionDialogAcknowledged.

name: Pull Request
about: Propose changes to TriggerX
title: ''
labels: ''
assignees: ''

Description

Please include a summary of the change and which issue is fixed (if any).
e.g., Fixes # (issue)

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce.
Please also list any relevant details for your test configuration.

  • Test A
  • Test B

Test Configuration:

  • Android Version(s):
  • Device(s)/Emulator(s):
  • TriggerXExample app variant:

Screenshots (if applicable)

Before After

Summary by CodeRabbit

  • New Features

    • Added support for new manual permission types, including overlay permissions that require user guidance.
    • Introduced user-facing dialogs with detailed instructions for enabling permissions that cannot be requested via standard system prompts.
    • Enhanced permission handling UI to better guide users through manual permission processes.
  • Improvements

    • Permission checks and requests are now handled asynchronously, improving app responsiveness.
    • Permission state and dialog acknowledgment are now persisted for a smoother user experience.
    • Alarm scheduling and cancellation operations are now performed asynchronously to enhance performance.
  • Chores

    • Updated app permissions to include overlay window support.

This commit introduces a dialog to guide users on enabling "display pop-up windows while running in the background" permission, particularly for devices like Xiaomi, Oppo, and Vivo. The user's acknowledgment of this dialog is now persisted using DataStore.

Key changes:
- **triggerx/src/main/java/com/meticha/triggerx/permission/TriggerXPermissionComposable.kt**:
    - Added `PermissionType.OVERLAY` and `PermissionType.OVERLAY_WHILE_BACKGROUND` to the default `permissions` list.
    - Implemented `ShowPermissionGuidanceDialog` to display specific instructions for `OVERLAY_WHILE_BACKGROUND`.
    - Integrated `TriggerXPermissionFlagManager` to save and check the acknowledgment state of the guidance dialog.
    - Permission requests and dialog showings are now launched within a `CoroutineScope`.
    - Added `ShowManualPermissionDialog` composable for generic manual permission guidance.
- **triggerx/src/main/java/com/meticha/triggerx/permission/TriggerXPermissionManager.kt**:
    - Added `OVERLAY_WHILE_BACKGROUND` to `PermissionType` enum, marking it as a `isManualPermissionType`.
    - Implemented `isOverlayBackgroundPermissionEnabled` to check acknowledgment status via `TriggerXPermissionFlagManager`.
    - `isGranted` now handles `OVERLAY_WHILE_BACKGROUND` by checking its acknowledgment status.
    - `createPermissionIntent` now returns `null` for `PermissionType`s that are manually configured by the user (like `OVERLAY_WHILE_BACKGROUND`).
    - Added `OverlayPermissionGuidanceDialog` composable (though currently unused directly, parts of its logic are adapted elsewhere).
    - `PermissionState.requestPermission` now checks for `isManualPermissionType` to show `showPermissionGuidanceDialog` instead of launching an intent.
    - `PermissionState.allRequiredGranted` and `PermissionState.isGranted` are now suspend functions.
    - `PermissionState.next` is now a suspend function.
- **app/src/main/java/com/meticha/triggerxexample/home/HomeScreen.kt**:
    - Permission requests (`permissionState.requestPermission()`) and checks (`permissionState.allRequiredGranted()`) are now called within a `CoroutineScope`.
- **triggerx/src/main/java/com/meticha/triggerx/permission/PermissionLifeCycleCheckEffect.kt**:
    - `permissionState.requestPermission()` is now launched within a `CoroutineScope` in the lifecycle observer.
- **app/src/main/java/com/meticha/triggerxexample/home/HomeViewModel.kt**:
    - `scheduleOneMinuteAlarm` and `cancelCurrentAlarm` now launch their operations within `viewModelScope`.
- **app/src/main/AndroidManifest.xml**:
    - Added `android.permission.SYSTEM_ALERT_WINDOW` permission.
- **triggerx/src/main/java/com/meticha/triggerx/preference/TriggerXAlarmIdManager.kt**:
    - Added `TriggerXPermissionFlagManager` object to save and retrieve boolean flags for permission dialog acknowledgments using DataStore.
    - Implemented `savePermissionDialogResponse` and `isPermissionDialogAcknowledged`.
@coderabbitai

coderabbitai Bot commented Jul 1, 2025

Copy link
Copy Markdown

Walkthrough

This update introduces asynchronous coroutine-based permission handling throughout the app, adds support for new manual overlay permission types, and implements user guidance dialogs for permissions that require manual user action. The Android manifest is updated to declare the SYSTEM_ALERT_WINDOW permission, and new state management for permission dialog acknowledgments is introduced.

Changes

File(s) Change Summary
app/src/main/AndroidManifest.xml Added android.permission.SYSTEM_ALERT_WINDOW permission declaration.
app/src/main/java/com/meticha/triggerxexample/home/HomeScreen.kt Button click handler logic now runs inside a coroutine scope for asynchronous permission and alarm actions.
app/src/main/java/com/meticha/triggerxexample/home/HomeViewModel.kt Alarm scheduling/cancellation methods now launch coroutines; removed Boolean return from scheduling method.
triggerx/src/main/java/com/meticha/triggerx/permission/PermissionLifeCycleCheckEffect.kt Permission request now launched asynchronously inside a coroutine scope within the composable.
triggerx/src/main/java/com/meticha/triggerx/permission/TriggerXPermissionComposable.kt Added support/UI for manual permission types, overlay/background overlay permissions, and guidance dialogs.
triggerx/src/main/java/com/meticha/triggerx/permission/TriggerXPermissionManager.kt Added async/manual permission handling, new permission type, guidance dialog, and suspend functions.
triggerx/src/main/java/com/meticha/triggerx/preference/TriggerXManualPermissionStatusManager.kt Added new manager for storing manual permission dialog acknowledgment state in DataStore.
triggerx/src/main/java/com/meticha/triggerx/preference/TriggerXAlarmIdManager.kt Removed trailing newline; no functional changes.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant HomeScreen
    participant HomeViewModel
    participant PermissionState
    participant TriggerXPermissionFlagManager

    User->>HomeScreen: Clicks button
    HomeScreen->>HomeScreen: Launch coroutine
    HomeScreen->>PermissionState: requestPermission() (suspend)
    alt Manual permission type
        PermissionState->>HomeScreen: Show guidance dialog
        User->>HomeScreen: Acknowledge dialog
        HomeScreen->>TriggerXPermissionFlagManager: savePermissionDialogResponse()
    else Standard permission
        PermissionState->>PermissionState: Launch system intent
    end
    HomeScreen->>HomeViewModel: scheduleOneMinuteAlarm() (coroutine)
    HomeViewModel->>HomeViewModel: Launch coroutine
    HomeViewModel->>...: Schedule alarm asynchronously
Loading

Possibly related PRs

  • meticha/triggerx#14: This PR is directly related as it removes the SYSTEM_ALERT_WINDOW permission and overlay permission from the default list, which is the opposite of the changes in this PR.

Suggested labels

enhancement, documentation

Poem

A bunny hops through code anew,
Overlay windows come into view.
With coroutines, permissions flow—
Async checks, dialogs aglow.
Guidance pops up, alarms now chime,
As rabbits celebrate in rhyme!
🐇✨


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f03c9d9 and dcd9a04.

📒 Files selected for processing (1)
  • .github/workflows/release.yaml (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • .github/workflows/release.yaml
✨ Finishing Touches
🧪 Generate Unit Tests
  • Create PR with Unit Tests
  • Post Copyable Unit Tests in a Comment

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai auto-generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

…nePlus devices

This commit modifies the `TriggerXPermissionComposable` to conditionally add the `OVERLAY_WHILE_BACKGROUND` permission only for OnePlus devices. Previously, this permission was added for all devices.

Key changes:

- **triggerx/src/main/java/com/meticha/triggerx/permission/TriggerXPermissionComposable.kt**:
    - Wrapped the addition of `PermissionType.OVERLAY_WHILE_BACKGROUND` in a conditional check for `Build.MANUFACTURER.equals("oneplus", true)`.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (3)
triggerx/src/main/java/com/meticha/triggerx/permission/TriggerXPermissionComposable.kt (2)

71-73: Clean up the commented OnePlus-specific code.

The commented code suggests that OVERLAY_WHILE_BACKGROUND was initially intended only for OnePlus devices, but it's now being added unconditionally. Please remove the commented code to avoid confusion about the implementation intent.

-//        if (Build.MANUFACTURER.equals("oneplus", true)) {
         add(PermissionType.OVERLAY_WHILE_BACKGROUND)
-//        }

192-212: Consider adding an explicit dismiss button.

The ShowManualPermissionDialog only shows an "Acknowledge" button without an explicit dismiss/cancel option. Users can only dismiss via the back button or tapping outside, which might not be intuitive.

         confirmButton = {
             Button(onClick = onConfirm) {
                 Text("Acknowledge")
             }
+        },
+        dismissButton = {
+            Button(onClick = onDismiss) {
+                Text("Later")
+            }
         }
triggerx/src/main/java/com/meticha/triggerx/permission/TriggerXPermissionManager.kt (1)

394-406: Handle the case when createPermissionIntent returns null for non-manual permissions.

When createPermissionIntent returns null for a non-manual permission type, the code silently moves to the next permission. Consider logging a warning or showing an error to help with debugging.

                     } else {
                         val intent = AlarmPermissionManager.createPermissionIntent(
                             context,
                             permission
                         )
                         if (intent != null) {
                             launcher?.launch(intent)
                         } else {
+                            // Log warning for debugging
+                            android.util.Log.w("PermissionState", "No intent available for permission: $permission")
                             next()
                         }
                     }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 528e7e6 and 868bd28.

📒 Files selected for processing (7)
  • app/src/main/AndroidManifest.xml (1 hunks)
  • app/src/main/java/com/meticha/triggerxexample/home/HomeScreen.kt (2 hunks)
  • app/src/main/java/com/meticha/triggerxexample/home/HomeViewModel.kt (2 hunks)
  • triggerx/src/main/java/com/meticha/triggerx/permission/PermissionLifeCycleCheckEffect.kt (2 hunks)
  • triggerx/src/main/java/com/meticha/triggerx/permission/TriggerXPermissionComposable.kt (6 hunks)
  • triggerx/src/main/java/com/meticha/triggerx/permission/TriggerXPermissionManager.kt (12 hunks)
  • triggerx/src/main/java/com/meticha/triggerx/preference/TriggerXAlarmIdManager.kt (2 hunks)
🔇 Additional comments (11)
app/src/main/AndroidManifest.xml (1)

24-24: LGTM! Correctly adds overlay permission.

The SYSTEM_ALERT_WINDOW permission is properly declared and necessary for the overlay functionality described in the PR objectives.

triggerx/src/main/java/com/meticha/triggerx/permission/PermissionLifeCycleCheckEffect.kt (1)

20-20: LGTM! Proper coroutine integration for async permission requests.

The addition of rememberCoroutineScope() and launching the permission request within a coroutine is appropriate for non-blocking permission handling in Compose.

Also applies to: 25-25, 45-45, 50-50

app/src/main/java/com/meticha/triggerxexample/home/HomeScreen.kt (1)

27-27: LGTM! Proper async handling for UI interactions.

Wrapping the permission check and alarm scheduling logic in a coroutine scope ensures non-blocking UI interactions and proper handling of suspend functions.

Also applies to: 34-34, 42-42, 56-69

app/src/main/java/com/meticha/triggerxexample/home/HomeViewModel.kt (2)

21-21: LGTM! Proper async alarm scheduling using viewModelScope.

The conversion to coroutine-based alarm scheduling is appropriate for potentially long-running operations and follows proper ViewModel coroutine patterns.

Also applies to: 24-24, 43-50, 55-55


37-37: No return-value dependencies for scheduleOneMinuteAlarm

A search across the codebase found only two occurrences:

  • Definition in HomeViewModel.kt (line 37)
  • Invocation in HomeScreen.kt (line 63) as a standalone call

There are no assignments, conditional checks, or initializations relying on a Boolean return. The signature change is safe and requires no further updates.

triggerx/src/main/java/com/meticha/triggerx/preference/TriggerXAlarmIdManager.kt (1)

67-92: LGTM! Well-designed permission flag manager with proper DataStore usage.

The TriggerXPermissionFlagManager follows the same established patterns as TriggerXAlarmIdManager and provides:

  • Proper async persistence with suspend functions
  • Reactive observation via Flow
  • Appropriate key generation based on permission type
  • Sensible default behavior (false for unacknowledged)

The shared DataStore usage is acceptable and keeps related preferences together.

triggerx/src/main/java/com/meticha/triggerx/permission/TriggerXPermissionComposable.kt (2)

79-79: LGTM! Proper coroutine handling for async permission checks.

The introduction of rememberCoroutineScope() and wrapping the launcher callback in coroutineScope.launch is the correct approach for handling the new suspend functions in the permission system.

Also applies to: 93-104


139-183: Well-implemented permission guidance dialog.

The ShowPermissionGuidanceDialog composable properly handles the manual permission flow with clear user instructions and persistence of user acknowledgment via TriggerXPermissionFlagManager.

triggerx/src/main/java/com/meticha/triggerx/permission/TriggerXPermissionManager.kt (3)

142-146: Correct implementation of manual permission acknowledgment check.

The isOverlayBackgroundPermissionEnabled function properly checks the acknowledgment status from the DataStore-backed flag manager.


175-175: Appropriate nullable return type for manual permissions.

The change to return Intent? correctly handles manual permission types that don't have associated system intents.

Also applies to: 216-219


357-361: Proper async conversion for permission handling.

The conversion of permission methods to suspend functions correctly supports the new asynchronous permission checks while maintaining backward compatibility for synchronous permission types.

Also applies to: 372-376, 385-412, 421-428

This commit relocates `TriggerXPermissionFlagManager` to a new file `TriggerXManualPermissionStatusManager.kt` and renames it accordingly.

Additionally, it enhances the permission guidance dialog in `TriggerXPermissionComposable.kt`:
- The dialog is no longer dismissible by clicking outside.
- A "Later" button has been added to the dismiss action.

Key changes:

- **triggerx/src/main/java/com/meticha/triggerx/permission/TriggerXPermissionComposable.kt**:
    - Replaced `TriggerXPermissionFlagManager` with `TriggerXManualPermissionStatusManager`.
    - Set `dismissOnClickOutside` to `false` in `DialogProperties` for the permission guidance dialog.
    - Added a `dismissButton` with "Later" text to the permission guidance dialog.
- **triggerx/src/main/java/com/meticha/triggerx/permission/TriggerXPermissionManager.kt**:
    - Replaced `TriggerXPermissionFlagManager` with `TriggerXManualPermissionStatusManager`.
    - Removed unused `OverlayPermissionGuidanceDialog` composable.
- **triggerx/src/main/java/com/meticha/triggerx/preference/TriggerXAlarmIdManager.kt**:
    - Removed the `TriggerXPermissionFlagManager` object.
- **triggerx/src/main/java/com/meticha/triggerx/preference/TriggerXManualPermissionStatusManager.kt**:
    - New file created.
    - Contains the relocated and renamed `TriggerXPermissionFlagManager` (now `TriggerXManualPermissionStatusManager`).

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
triggerx/src/main/java/com/meticha/triggerx/permission/TriggerXPermissionManager.kt (1)

295-299: Async permission checking in batch operations.

Converting allRequiredGranted to suspend is necessary for manual permission checking, but this could impact performance when checking multiple permissions.

Consider parallel processing for better performance:

 suspend fun allRequiredGranted(): Boolean {
-    isRequiredPermissionGranted = allPermissions
-        .all { isGranted(it) }
+    isRequiredPermissionGranted = allPermissions
+        .map { async { isGranted(it) } }
+        .awaitAll()
+        .all { it }
     return isRequiredPermissionGranted
 }

Don't forget to import kotlinx.coroutines.async and kotlinx.coroutines.awaitAll.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 356ae06 and 5e5b423.

📒 Files selected for processing (4)
  • triggerx/src/main/java/com/meticha/triggerx/permission/TriggerXPermissionComposable.kt (6 hunks)
  • triggerx/src/main/java/com/meticha/triggerx/permission/TriggerXPermissionManager.kt (12 hunks)
  • triggerx/src/main/java/com/meticha/triggerx/preference/TriggerXAlarmIdManager.kt (1 hunks)
  • triggerx/src/main/java/com/meticha/triggerx/preference/TriggerXManualPermissionStatusManager.kt (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • triggerx/src/main/java/com/meticha/triggerx/preference/TriggerXAlarmIdManager.kt
🚧 Files skipped from review as they are similar to previous changes (1)
  • triggerx/src/main/java/com/meticha/triggerx/permission/TriggerXPermissionComposable.kt
🔇 Additional comments (11)
triggerx/src/main/java/com/meticha/triggerx/preference/TriggerXManualPermissionStatusManager.kt (3)

13-13: DataStore initialization follows best practices.

The use of preferencesDataStore delegate with a descriptive name is appropriate for managing permission dialog acknowledgment states.


28-37: Flow-based data access is well implemented.

The function correctly returns a Flow<Boolean> with appropriate default value handling. The use of map to transform preferences data is idiomatic.


39-41: Preference key generation is robust.

Using the permission type name ensures unique keys per permission type, which prevents conflicts between different manual permissions.

triggerx/src/main/java/com/meticha/triggerx/permission/TriggerXPermissionManager.kt (8)

36-37: Import additions support the new manual permission functionality.

The imports for TriggerXManualPermissionStatusManager and kotlinx.coroutines.flow.first are necessary for the manual permission status checking implementation.


126-130: Manual permission status checking is correctly implemented.

The suspend function properly uses the manual permission status manager to check acknowledgment via DataStore. Using .first() to get the current value from the Flow is appropriate for this use case.


159-204: Intent creation properly handles manual permissions.

The function correctly returns null for manual permission types since they don't have system intents. The logic properly distinguishes between system permissions that have intents and manual permissions that require user guidance.


212-231: Permission type architecture supports extensibility.

The addition of isManualPermissionType parameter to the enum constructor provides a clean way to distinguish between system and manual permissions. The new OVERLAY_WHILE_BACKGROUND permission type is properly flagged as manual.


265-265: State management for guidance dialog is appropriately added.

The new showPermissionGuidanceDialog state variable follows the existing pattern and is necessary for controlling the display of manual permission guidance dialogs.


323-350: Permission request flow correctly handles manual permissions.

The updated logic properly distinguishes between manual and system permissions, showing guidance dialogs for manual permissions and launching system intents for others. The error handling for null intents is appropriate.


359-366: Async next() function maintains proper flow control.

Converting next() to suspend and calling requestPermission() maintains the permission flow while supporting async operations. The logic correctly processes the queue and handles the async permission checking.


138-147: Verify suspend overhead of isGranted

Our search (rg '\.isGranted\(') only uncovered the internal call to AlarmPermissionManager.isGranted(…) within TriggerXPermissionManager.kt and no external consumers of TriggerXPermissionManager.isGranted. Please confirm whether this suspend function ends up in performance-sensitive paths (e.g., UI loops, scrolling adapters). If it does, you may want to:

  • Introduce a non-suspending API for system permissions:
    fun isGrantedSync(context: Context, permission: PermissionType): Boolean
  • Reserve the suspend fun isGranted for manual/DataStore-backed permission checks only.

This hybrid approach avoids unnecessary coroutine overhead for simple system checks.

This commit introduces several improvements to permission management and logging within the `TriggerXPermissionManager`.

Key changes:

- **triggerx/src/main/java/com/meticha/triggerx/permission/TriggerXPermissionManager.kt**:
    - `createPermissionIntent` for `PermissionType.ALARM` now returns `null` for Android versions below S, as `ACTION_REQUEST_SCHEDULE_EXACT_ALARM` is only available from S onwards.
    - `allRequiredGranted` now uses `coroutineScope` and `async/awaitAll` for concurrent permission checks, potentially improving performance.
    - Added error logging in `checkAndRequestNextPermission` if an intent for a permission cannot be created.
    - Minor formatting adjustment.
@bhavikmaradiya

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jul 2, 2025

Copy link
Copy Markdown
✅ Actions performed

Full review triggered.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (1)
triggerx/src/main/java/com/meticha/triggerx/preference/TriggerXManualPermissionStatusManager.kt (1)

17-26: Add error handling for DataStore operations.

The function correctly uses suspend and DataStore's edit operation, but missing error handling could lead to unhandled exceptions during persistence operations.

Consider wrapping the DataStore operation in a try-catch block:

 suspend fun savePermissionDialogResponse(
     context: Context,
     permissionType: PermissionType,
     acknowledged: Boolean,
 ) {
-    val key = getPreferenceKeyForType(permissionType)
-    context.dataStore.edit { settings ->
-        settings[key] = acknowledged
-    }
+    try {
+        val key = getPreferenceKeyForType(permissionType)
+        context.dataStore.edit { settings ->
+            settings[key] = acknowledged
+        }
+    } catch (e: Exception) {
+        // Log error or handle gracefully based on your logging strategy
+        // Consider whether to rethrow or provide default behavior
+        throw e
+    }
 }
🧹 Nitpick comments (4)
app/src/main/java/com/meticha/triggerxexample/home/HomeViewModel.kt (1)

37-51: Good async conversion, but consider error handling implications.

The conversion to viewModelScope.launch properly handles alarm scheduling asynchronously. However, removing the Boolean return value eliminates the ability for callers to determine if the operation succeeded.

Consider whether you need to:

  • Add error handling within the coroutine
  • Provide alternative mechanisms (like StateFlow) to communicate operation results
  • Log failures appropriately for debugging
triggerx/src/main/java/com/meticha/triggerx/permission/TriggerXPermissionComposable.kt (2)

72-72: Consider using consistent manufacturer name casing.

For consistency with the Xiaomi check above, consider using the proper casing for OnePlus.

-        if (Build.MANUFACTURER.equals("oneplus", true)) {
+        if (Build.MANUFACTURER.equals("OnePlus", true)) {

152-157: Consider making the guidance message more generic and maintainable.

The hardcoded settings navigation path may not be accurate across all device models, Android versions, and custom ROMs. Consider:

  1. Making the instructions more generic
  2. Moving the message to string resources for easier maintenance
  3. Using device-specific instructions if available

Example of a more generic message:

message = "For alarms to reliably appear when the app is in the background, " +
          "'$appName' needs an additional permission.\n\n" +
          "Please navigate to your device's app settings and ensure that background " +
          "pop-up or overlay permissions are enabled for '$appName'.\n\n" +
          "The exact location varies by device manufacturer."
triggerx/src/main/java/com/meticha/triggerx/permission/TriggerXPermissionManager.kt (1)

309-309: Remove redundant variable assignment.

The isRequiredPermissionGranted value is already assigned on line 306.

-        isRequiredPermissionGranted
+        return isRequiredPermissionGranted
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 528e7e6 and b696044.

📒 Files selected for processing (8)
  • app/src/main/AndroidManifest.xml (1 hunks)
  • app/src/main/java/com/meticha/triggerxexample/home/HomeScreen.kt (2 hunks)
  • app/src/main/java/com/meticha/triggerxexample/home/HomeViewModel.kt (2 hunks)
  • triggerx/src/main/java/com/meticha/triggerx/permission/PermissionLifeCycleCheckEffect.kt (2 hunks)
  • triggerx/src/main/java/com/meticha/triggerx/permission/TriggerXPermissionComposable.kt (6 hunks)
  • triggerx/src/main/java/com/meticha/triggerx/permission/TriggerXPermissionManager.kt (12 hunks)
  • triggerx/src/main/java/com/meticha/triggerx/preference/TriggerXAlarmIdManager.kt (1 hunks)
  • triggerx/src/main/java/com/meticha/triggerx/preference/TriggerXManualPermissionStatusManager.kt (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
app/src/main/java/com/meticha/triggerxexample/home/HomeScreen.kt (1)
triggerx/src/main/java/com/meticha/triggerx/permission/TriggerXPermissionComposable.kt (1)
  • rememberAppPermissionState (56-138)
triggerx/src/main/java/com/meticha/triggerx/permission/TriggerXPermissionComposable.kt (1)
triggerx/src/main/java/com/meticha/triggerx/permission/TriggerXPermissionManager.kt (2)
  • isGranted (143-152)
  • isGranted (321-325)
🔇 Additional comments (9)
triggerx/src/main/java/com/meticha/triggerx/preference/TriggerXAlarmIdManager.kt (1)

62-62: LGTM! Minor formatting cleanup.

Removing the trailing newline is a trivial formatting improvement with no functional impact.

triggerx/src/main/java/com/meticha/triggerx/preference/TriggerXManualPermissionStatusManager.kt (2)

28-37: LGTM! Good reactive pattern for permission state.

The Flow-based approach enables reactive UI updates when permission acknowledgment status changes. The default false value appropriately represents unacknowledged permissions.


39-41: LGTM! Proper key generation for unique permission types.

Using the permission type name ensures unique DataStore keys for different permission types, preventing conflicts.

app/src/main/AndroidManifest.xml (1)

24-24: LGTM! Necessary permission for overlay functionality.

Adding SYSTEM_ALERT_WINDOW permission aligns with the PR objectives to support overlay permissions and "display pop-up windows while running in the background" functionality.

app/src/main/java/com/meticha/triggerxexample/home/HomeScreen.kt (2)

27-27: LGTM! Proper coroutine imports for async handling.

Added necessary imports for coroutine scope management in the Composable context.

Also applies to: 34-34, 42-42


56-69: LGTM! Correct async conversion for permission handling.

Wrapping the permission check and alarm scheduling logic in coroutines.launch properly handles the asynchronous nature of the updated permission system. This ensures UI interactions don't block the main thread while maintaining the same control flow.

app/src/main/java/com/meticha/triggerxexample/home/HomeViewModel.kt (2)

21-21: LGTM! Proper coroutine imports for ViewModel scope.

Added necessary imports for launching coroutines within the ViewModel's lifecycle-aware scope.

Also applies to: 24-24


55-55: LGTM! Consistent async pattern for alarm cancellation.

Converting to viewModelScope.launch maintains consistency with the alarm scheduling method and properly handles the asynchronous nature of alarm cancellation.

triggerx/src/main/java/com/meticha/triggerx/permission/PermissionLifeCycleCheckEffect.kt (1)

20-20: LGTM! Correct coroutine integration for suspend permission requests.

The changes properly integrate coroutine support to handle the new suspend requestPermission() function. The coroutine scope is correctly created using rememberCoroutineScope() and the permission request is launched within it, maintaining proper lifecycle awareness.

Also applies to: 25-25, 45-45, 50-50

This commit updates the text in the permission guidance dialog.

Key changes:

- **triggerx/src/main/java/com/meticha/triggerx/permission/TriggerXPermissionComposable.kt**:
    - Changed "Understood" to "Acknowledge" in the permission guidance dialog message.
This commit adds the Apache License, Version 2.0 to the `TriggerXManualPermissionStatusManager.kt` file.

Key changes:

- **triggerx/src/main/java/com/meticha/triggerx/preference/TriggerXManualPermissionStatusManager.kt**:
    - Added the Apache License, Version 2.0 header.
This commit updates the `VERSION_NAME` environment variable in the release workflow.

Key changes:

- **.github/workflows/release.yaml**:
    - Updated `VERSION_NAME` from `0.0.7` to `0.0.8`.
@meticha-admin meticha-admin merged commit 5689817 into meticha:main Jul 2, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants