ibm-bluemix-mobile-services / bms-clientsdk-swift-push

iOS Push notifications SDK for IBM Cloud Mobile Services
Apache License 2.0
10 stars 12 forks source link

Need an error handler if the user doesn't permit notifications #57

Closed rajishere closed 6 years ago

rajishere commented 7 years ago

Hi Team,

Earlier in 2.0 version we were calling the UIApplication.shared.registerForRemoteNotifications() to ask user push permissions, but now BMSPushClient is handling this. However in 3.2.3, we don't have a call back in case user doesn't permit notifications, we need this for us to set the required flags and handle the permission failure gracefully. We see the pod is logging the failure but I am afraid we will need a error handler.

AnanthaKrish commented 7 years ago

@rajishere You can try checking the permission status after the initializeWithAppGUID method,

let push =  BMSPushClient.sharedInstance
push.initializeWithAppGUID(appGUID: "xxxxxx", clientSecret: "xxxxxx")
 let center = UNUserNotificationCenter.current()
center.getNotificationSettings { (settings) in
            if(settings.authorizationStatus == .authorized) {
                print("Push authorized")
            }else{
                print("Push not authorized")
            }
}
rajishere commented 7 years ago

@AnanthaKrish As we are calling an 'async' UNUserNotificationCenter.current().requestAuthorization in the BMSPushClient, UNUserNotificationCenter.current().getNotificationSettings is returning as unauthorized even before user has approved/disapproved the push. I am afraid we will need some kind of callback/delegate/notification from BMSPushClient, that user has approved/disapproved the request.

rajishere commented 7 years ago

@AnanthaKrish Also when user tries to turn off notifications in 2.0, we call BMSPushClient.sharedInstance.initializeWithAppGUID to initialize the BMSPushClient and then BMSPushClient.sharedInstance.unregisterDevice to unregister device, but in 3.2.3, calling BMSPushClient's initializeWithAppGUID, calls UNUserNotificationCenter.current().requestAuthorization which again triggers a device registration.

In a gist, we need a way to initialize the BMSPushClient during unregistration without triggering a call to appdelegate's didRegisterForRemoteNotificationsWithDeviceToken

AnanthaKrish commented 7 years ago

@rajishere

  1. I am working on it . Will get back to you soon.
  2. For unregistering the device you don't have to call initializeWithAppGUID . You can directly call unregisterDevice
  let push =  BMSPushClient.sharedInstance
  push.unregisterDevice(completionHandler: { (response, statusCode, error) -> Void 

           /.../

  })
rajishere commented 7 years ago

@AnanthaKrish We are seeing a crash if we call unregister without initialising. It seems to happen when the library tries to force unwrap the client id or secret.

AnanthaKrish commented 7 years ago

@rajishere How you are checking when the user turn off notifications ?

rajishere commented 7 years ago

@AnanthaKrish user can toggle the notifications in settings and we are looking to unregister him, if he toggles it to off. Is there some detail you are looking for? The issue is when the user toggles it off during subsequent app sessions (not the one he registered with)

AnanthaKrish commented 7 years ago

@rajishere Is that toggle button in your app or the notification button inside general-> notification ??

Yeah I need Some details . How you are trying to un-register . I want to see the code.

AnanthaKrish commented 7 years ago

@rajishere If the user disables the notifications from Settings->Notifications-> app name, the didRegisterForRemoteNotificationsWithDeviceToken method will not get invoked even if you do initializeWithAppGUID.. You can use the let center = UNUserNotificationCenter.current() center.getNotificationSettings { (settings) in...} method for unregistering the device from push.

AnanthaKrish commented 7 years ago

@rajishere Please try this.

       let center = UNUserNotificationCenter.current()
        center.getNotificationSettings { (settings) in
            if(settings.authorizationStatus == .authorized)
            {
                print("Granted access")
            }
            else
            {
                let myBMSClient = BMSClient.sharedInstance
                myBMSClient.initialize(bluemixRegion: BMSClient.Region.usSouth)
                let push =  BMSPushClient.sharedInstance
                push.initializeWithAppGUID(appGUID: "xx-xxxxxx-xxxx", clientSecret: "xxx-xxxxx-xxxxx")
                if (UIApplication.shared.isRegisteredForRemoteNotifications) {
                BMSPushClient.sharedInstance.unregisterDevice(completionHandler: { (response, statusCode, error) -> Void in
                    if error.isEmpty {
                        print( "Response during unregistering device : \(response)")
                        print( "status code during unregistering device : \(statusCode)")                        
                        UIApplication.shared.unregisterForRemoteNotifications()
                    }
                    else{
                        print( "Error during unregistering device \(error) ")
                    }
                })}
            }
        }
AnanthaKrish commented 7 years ago

@rajishere can you join this slack channel --https://openwhisk-team.slack.com/ ??

rajishere commented 7 years ago

Hi @AnanthaKrish Looks like I'll need a @us.ibm.com, @uk.ibm.com, @adobe.com, @de.ibm.com, @il.ibm.com or @apache.org email address to join that channel

rajishere commented 7 years ago

@AnanthaKrish Attaching the sample flow document and issue points 1 & 2 we are facing

bluemix flow

rajishere commented 7 years ago

The issue being discussed in this ticket are "1". i.e We don't have a callback if user doesn't allow permissions, to handle that negative scenario and "2". i.e. calling bluemix initialize is triggering Apple's didRegisterDevice in AppDelegate

So for issue 2, when I call push.initializeWithAppGUID(appGUID: "xx-xxxxxx-xxxx", clientSecret: "xxx-xxxxx-xxxxx") Internally it calls UIApplication.shared.registerForRemoteNotifications(). Below is the code in initializeWithAppGUID

if validateString(object: clientSecret) {

            self.clientSecret = clientSecret
            self.applicationId = appGUID
            isInitialized = true;
            self.bluemixDeviceId = ""

            if #available(iOS 10.0, *) {

                UNUserNotificationCenter.current().requestAuthorization(options: [.alert, .badge, .sound], completionHandler: { (granted, error) in
                    if(granted) {
                        UIApplication.shared.registerForRemoteNotifications()
                    } else {
                        print("Error while registering with APNS server :  \(error?.localizedDescription)")
                    }
                })
            } else {
                // Fallback on earlier versions
                let settings = UIUserNotificationSettings(types: [.alert, .badge, .sound], categories: nil)
                UIApplication.shared.registerUserNotificationSettings(settings)
                UIApplication.shared.registerForRemoteNotifications()
            }
        }
        else{
            self.sendAnalyticsData(logType: LogLevel.error, logStringData: "Error while registration - Client secret is not valid")
            print("Error while registration - Client secret is not valid")
        }
AnanthaKrish commented 7 years ago

@rajishere

  1. There is no way you can check for doesn't allow permissions in push

  2. If the user disables the notifications from Settings->Notifications-> app name, the didRegisterForRemoteNotificationsWithDeviceToken method will not get invoked even if you do initializeWithAppGUID.

    After disabling the notification when your app opens again you can try this code,

       let center = UNUserNotificationCenter.current()
        center.getNotificationSettings { (settings) in
            if(settings.authorizationStatus == .authorized)
            {
                print("Granted access")
            }
            else
            {
                let myBMSClient = BMSClient.sharedInstance
                myBMSClient.initialize(bluemixRegion: BMSClient.Region.usSouth)
                let push =  BMSPushClient.sharedInstance
                push.initializeWithAppGUID(appGUID: "xx-xxxxxx-xxxx", clientSecret: "xxx-xxxxx-xxxxx")
                if (UIApplication.shared.isRegisteredForRemoteNotifications) {
                BMSPushClient.sharedInstance.unregisterDevice(completionHandler: { (response, statusCode, error) -> Void in
                    if error.isEmpty {
                        print( "Response during unregistering device : \(response)")
                        print( "status code during unregistering device : \(statusCode)")                        
                        UIApplication.shared.unregisterForRemoteNotifications()
                    }
                    else{
                        print( "Error during unregistering device \(error) ")
                    }
                })}
            }
        }

Since the the notification is disabled it will come to the else part of if(settings.authorizationStatus == .authorized).. There you can add the unregister call..

Note : - I will try to solve this issues soon. The second one I have a solution. But for the 1st its little bit tricky ...

AnanthaKrish commented 7 years ago

@rajishere Hi, I have released a new version of sdk 3.2.4

  1. for init callback , follow this - https://github.com/ibm-bluemix-mobile-services/bms-clientsdk-swift-push#enabling-ios-applications-to-receive-push-notifications

  2. for unregistering from app you don't have to make initialise call now,

      BMSPushClient.sharedInstance.unregisterDevice(.....)..
rajishere commented 7 years ago

@AnanthaKrish Thank you so much, will test