gdelataillade / alarm

A Flutter plugin to easily manage alarms on iOS and Android
https://pub.dev/packages/alarm
MIT License
132 stars 86 forks source link

Is it possible to handle permissions in a unified way for iOS & Android? #181

Closed younesouhbi closed 1 month ago

younesouhbi commented 7 months ago

Again, thank you for this plugin, it's really helpful.

Right now, for iOS, the notification permission seems to be requested internally by the plugin. However, for Android, the example you provide uses the Permission Handler plugin to check and request for permissions. This makes a bit inconvenient and asymmetric.

Since the permission check/request is done after Alarm.init, I think it would be better to just pass the desired config to it, and have it do the permission checks/requests, or, if it's too much, just let the devs handle permissions themselves.

Thank you

gdelataillade commented 7 months ago

Hi @younesouhbi

Thanks for your feedback ! You're right, notification permission handling is currently asymmetric. I think I will ask for Android notification when Alarm.set is called, like in iOS. In my opinion it's better this way because:

What do you think ?

younesouhbi commented 7 months ago

Thank you for your reply!

I would say that there are advantages & disadvantages to this. Let me start with the advantages:

Advantage: making the permission request contextual can help the user better understand why the permission is being requested, however, I also think that users are now accustomed to permissions being requested from apps, especially an app that schedules alarms. What is suspicious and confusing is when a calculator app wants to access the user's contacts for example :) This means that while contextualizing the permission request is good, I don't think it's needed in this case.

Disadvantage 1: when the permission is requested at the moment of doing something, it interrupts the flow of what is being done. Personally, as a user, when I set out to perform an action, I don't like being distracted by "side quests". Imagine the user is trying to set an alarm for a specific date & time, then the permission request pops up, now they forgot which date or time they were trying to set an alarm for.

Disadvantage 2: if there are multiple permissions and the plugin asks for each permission as it is requested, it might become annoying as it interrupts the user multiple times.

Disadvantage 3: what if the app that uses the plugin cannot function without the permission? In this case, it might be better that the user doesn't waste their time if they are not willing to grant the permission. If said permission is only requested as it is needed, then the user might invest time in an app only to stumble upon the permission request and realizing the app doesn't work for them. It might be frustrating and it happened to me several times with various apps.

Disadvantage 4: from a developer standpoint, it might be better to let devs manage the permissions themselves and concentrate on what the purpose of the plugin is, which if I understand correctly is: setting alarms and letting you know when one is triggered. In my app for example, I would prefer to have an initial screen that informs the user of all the required permissions and why they are needed before they even get to the home screen. Other developers might prefer to do it as needed. Still, removing permission handling from the plugin might be better. This way, all devs know that before calling the plugin APIs, they need to have the needed permissions granted or things will fail. How they manage that is their responsibility.

So, all in all, I kind of lean towards another direction ;)

gdelataillade commented 7 months ago

@younesouhbi Thank you so much for your feedback !

It looks like it would be better to remove the permission request from the plugin then !

There is one thing you might need to know about notifications: for the android part I use a foreground service for triggering the alarm. It allows me to make the alarm work even if the app is in the background or killed. In order to use this foreground service, it is required by android to have a notification linked to it. Documentation here. It's not an issue but I just thought you may want to know that.

I'll add to my to do list: remove iOS notification permission request.

Sounds good ?

gdelataillade commented 7 months ago

@younesouhbi I realized I will have to wait to release version 4.0.0 to remove the iOS notification permission prompt because it's a breaking change because the apps in production that use the plugin may rely on this.

younesouhbi commented 7 months ago

@gdelataillade Thank you for your reply.

I have managed to remove the notification permission request from iOS. Here is the modified code for the NotificationManager.swift, in case someone wants to do the same thing:

import Foundation
import UserNotifications

/// Custom error to handle notification scheduling
enum NotificationSchedulingError: LocalizedError {
    case permissionNotGranted

    public var errorDescription: String? {
        switch self {
            case .permissionNotGranted:
                return "Could not schedule a notification because the notification permission is not currently granted!"
        }
    }
}

/// Singleton used to manage OS notifications
class NotificationManager {

    static let shared = NotificationManager()

    private init() {} // Private initializer to ensure singleton usage

    /// Schedules a notification
    /// - Parameters:
    ///   - id: the id of the alarm, which is a unique integer passed in as a string
    ///   - delayInSeconds: the number of seconds until the datetime of the notification
    ///   - title: the title of the notification
    ///   - body: the body of the notification
    ///   - completion: the callback to call once the scheduling attempt completes, either succeeds or fails
    func scheduleNotification(id: String, delayInSeconds: Int, title: String, body: String, completion: @escaping (Error?) -> Void) {

        // Check whether the notification permission is granted before going ahead and attempting to schedule one
        isNotificationPermissionGranted(completion: { granted in

            // Make sure the permission is currently granted
            guard granted else {
                // The notification permission is not currently granted so call the completion callback with an error
               completion(NotificationSchedulingError.permissionNotGranted)
               return
            }

            // Construct the content of the notification
            let content = UNMutableNotificationContent()
            content.title = title
            content.body = body
            content.sound = nil

            // Make the trigger of the notification (time based)
            let trigger = UNTimeIntervalNotificationTrigger(timeInterval: TimeInterval(delayInSeconds), repeats: false)

            // Make and attempt to schedule the notification request
            let request = UNNotificationRequest(identifier: buildNotificationIdentifier(id: id), content: content, trigger: trigger)
            UNUserNotificationCenter.current().add(request, withCompletionHandler: completion)
        })

    }

    /// Cancels a previously scheduled but not yet delivered notification
    /// - Parameter id: the id of the notification to cancel
    func cancelNotification(id: String) {
        UNUserNotificationCenter.current().removePendingNotificationRequests(withIdentifiers: [buildNotificationIdentifier(id: id)])
    }

    /// Checks whether the notification permission is currently granted or not
    /// - Parameter completion: the callback to call with the result after the check
    func isNotificationPermissionGranted(completion: @escaping (Bool) -> Void) {
        UNUserNotificationCenter.current().getNotificationSettings(completionHandler: { settings in
            switch settings.authorizationStatus {
                case .authorized, .provisional:
                    completion(true)

                default:
                    completion(false)
            }
        })
    }

    /// Builds the notification identifier
    /// - Parameter id: the base id from which to build a unique notification id
    /// - Returns: the final id to use for the notification that is guaranteed not to collide with other apps
    func buildNotificationIdentifier(id: String) -> String {
        return "<APP_BUNDLE_ID>.notifications.\(id)"
    }
}

This works in iOS. For Android, I haven't modified anything yet, and to be honest, I don't know where to begin. I read some of the Android native code, but couldn't yet identify the code that asks for the notification permission. I thought it was handled by Permission Handler, like it is in your example, but somehow, maybe it's the foreground service you mentioned in your comment above. I'll investigate more, but if you can point me in the right direction, that would be great!. There is no need to modify anything for Android. It was just my old code that was requesting the notification permission. The Andoid plugin does not require it by default, it seems.

Thank you

gdelataillade commented 1 month ago

Enhancement added to alarm version 4.x.x.