braze-inc / braze-cordova-sdk

Public repo for the Braze Cordova SDK
https://www.braze.com
Other
21 stars 61 forks source link

Can this plugin use with https://capacitor.ionicframework.com/ ? #49

Closed asangadev closed 4 years ago

asangadev commented 4 years ago

Hi @radixdev

I have successfully added the Appboy plugin to a capacitor project.

  1. AppboyPlugin.changeUser worked and the user is registered in Braze.
  2. User can receive in-app messages
  3. Most recent location worked (map)
  4. Profile update worked (such as name, email and gender)
  5. logCustomEvent worked
  6. logPurchase worked

ISSUE:

User device is not Opted In for PUSH notifications.

  1. App asked for the PUSH notification permissions in iOS. Then got the NSLog(@"Permission granted.");

  2. But [[Appboy sharedInstance] registerDeviceToken:deviceToken]; never been called.

Is this something that we can fix? Please advise. Do you think Appboy conflicting with https://capacitor.ionicframework.com/docs/apis/push-notifications ?

I can share the app if you want.

Bucimis commented 4 years ago

@asangadev Did you disable automatic iOS push registration using com.appboy.ios_disable_automatic_push_registration?

asangadev commented 4 years ago

Hi @Bucimis ,

Yes, I did. Also, I used the same settings and same plugin version with a blank PhoneGap App. PhoneGap app works without any issues.

The difference that I noticed between PhoneGap app and the capacitor app is below:

PhoneGap app always trigger below (but not the capacitor app):

- (void)appboy_swizzled_no_application:(UIApplication *)application didRegisterForRemoteNotificationsWithDeviceToken:(NSData *)deviceToken {
    // If the delegate is not implemented, swizzle the method but don't call the original (or we'd get in a loop)
    [[Appboy sharedInstance] registerDeviceToken:deviceToken];
}
Bucimis commented 4 years ago

@asangadev what happens if you disable the push notifications plugin per https://capacitor.ionicframework.com/docs/apis/push-notifications#disabling-push-notifications-plugin

asangadev commented 4 years ago

@Bucimis thanks for the quick reply! Yes I removed that variable values also I commented below lines in their code for testings. But still no luck.

#if USE_PUSH

  func application(_ application: UIApplication, didRegisterForRemoteNotificationsWithDeviceToken deviceToken: Data) {
    NotificationCenter.default.post(name: Notification.Name(CAPNotifications.DidRegisterForRemoteNotificationsWithDeviceToken.name()), object: deviceToken)
  }

  func application(_ application: UIApplication, didFailToRegisterForRemoteNotificationsWithError error: Error) {
    NotificationCenter.default.post(name: Notification.Name(CAPNotifications.DidFailToRegisterForRemoteNotificationsWithError.name()), object: error)
  }

#endif

Is that possible to call below within their SWFT: func application(_ application: UIApplication, didRegisterForRemoteNotificationsWithDeviceToken deviceToken: Data) {

Appboy.sharedInstance()?.registerDeviceToken(deviceToken)

Would that work? I wanted to try this but I was getting below error and not sure how to pass Appboy to swift?

Screen Shot 2020-02-28 at 6 21 46 am
Bucimis commented 4 years ago

@asangadev we should find out if the Capacitor plugin does anything that could interfere with the push delegate method swizzling we do here https://github.com/Appboy/appboy-cordova-sdk/blob/3f9262f736205d8893b183f1afd79d3b86f68550/src/ios/AppDelegate%2BAppboy.m#L6 (via https://github.com/Appboy/appboy-cordova-sdk/blob/3f9262f736205d8893b183f1afd79d3b86f68550/src/ios/AppboyPlugin.m#L21)

Given you can connect to the Braze backend, it does seem like pluginInitialize is getting called successfully

asangadev commented 4 years ago

@Bucimis I did further tests. You can find the amended code here (this works with PhoneGap): https://codeshare.io/GkRK8p

In Xcode you get:

Screen Shot 2020-02-28 at 11 38 57 am

But it's blocking the: appboy_swizzled_no_application

Do you think you can set this up on a blank capacitor project and give it a go? I tried my best with different things but no luck yet. :(

You are right the capacitor push plugin is doing something here. I could not find a way to remove it from the core.

Not sure this helps? https://github.com/ionic-team/capacitor/issues/2441#issuecomment-591238438

Bucimis commented 4 years ago

@asangadev do you have an AppDelegate file that you can edit directly in your project? If so, it may be better for you to disable automatic push integration and integrate the delegates manually. That may be a more direct route than trying to understand why the swizzling isn't working

asangadev commented 4 years ago

@Bucimis I need your help with this. I have set up the project here: https://github.com/asangadev/temp-app

could you please have a look or can you please guide me a little here? AppDelegate is setup in AppDelegate.swift (from capacitor).

https://github.com/asangadev/temp-app/blob/master/ios/App/App/AppDelegate.swift#L5

Bucimis commented 4 years ago

@asangadev you should be able to integrate push natively inside of your https://github.com/asangadev/temp-app/blob/master/ios/App/App/AppDelegate.swift#L5 using delegate methods described in https://www.braze.com/docs/developer_guide/platform_integration_guides/ios/push_notifications/integration/ instead of the swizzled delegates

asangadev commented 4 years ago

@Bucimis Okay I've fixed it.

For anyone who is planning to use AppBoy Cordova plugin with a capacitor project, follow below steps.

1. in your capacitor.config.json file add below

{
  "appId": "com.yourapp.app",
  "appName": "Sample App",
  "bundledWebRuntime": false,
  "npmClient": "npm",
  "webDir": "www",
  "plugins": {
    "PushNotifications": {
      "presentationOptions": ["badge", "sound", "alert"]
    }
  },
  "cordova": {
    "preferences": {
      "com.appboy.api_key": "**CHANGE-THIS:**XXXXX-XXXXXX-XXXXXX-XXXXXX",
      "com.appboy.ios_api_endpoint": "**CHANGE-THIS:**YOUR-END-POINT(see below link)",
      "com.appboy.ios_disable_automatic_push_registration": "YES"
    }
  }
}

Available endpoints (com.appboy.ios_api_endpoint): https://www.braze.com/docs/api/basics/#endpoints

2. In your app JS do below to trigger the push permission popup

import {Plugins} from '@capacitor/core'
const { PushNotifications } = Plugins

setTimeout(function(){
  //register user
  AppboyPlugin.changeUser("ABC001")

  PushNotifications.register()

  PushNotifications.addListener('registration',
        (PushNotificationToken) => {
          alert('Push registration success, token: ' + PushNotificationToken.value)
      )
}, 4000)

3. Modify your AppDelegate.swift in Xcode (I did this manually: still need to find a better way to implement this with CLI)

add import Appboy_iOS_SDK here: https://github.com/asangadev/temp-app/blob/master/ios/App/App/AppDelegate.swift#L3

add Appboy.sharedInstance()?.registerDeviceToken(deviceToken) here: https://github.com/asangadev/temp-app/blob/master/ios/App/App/AppDelegate.swift#L66

This is it guys!

Bucimis commented 4 years ago

@asangadev nice! did push notification analytics work as well? The appboy SDK needs its delegate methods called (whether the swizzled ones or called directly) in order to log opens.

asangadev commented 4 years ago

@Bucimis I think all good. I've setup a push campaign and open the push in one device. Below are the results. Is this mean we all set and good to go?

Screen Shot 2020-02-29 at 10 36 58 am
asangadev commented 4 years ago

@Bucimis before I close this, could you please confirm above?

Bucimis commented 4 years ago

@asangadev looks good to me! the relevant stat here is "Direct Opens" - there should be a 1:1 relationship between clicking a push and that stat incrementing

asangadev commented 4 years ago

@Bucimis thanks mate! closing this now.

katie-propel commented 8 months ago

This workaround used to work for us, but now in the latest version of the SDK, Braze.sharedInstance() is deprecated and we're supposed to create and keep a reference to the braze object. That's entirely managed by the plugin though. Has anyone had any luck propagating that up to AppDelegate.swift or come up with another workaround?

jerielng commented 8 months ago

Hi @katie-propel,

The Braze plugin should be registered to the list of Cordova plugins, so you should be able to access the instance of BrazePlugin using the getCommandInstance method of CDVViewController, and then from there access that braze instance.

So using our sample app as a reference point, our MainViewController conforms to CDVViewController, and you can do something like this:

- (void)application:(UIApplication *)application didRegisterForRemoteNotificationsWithDeviceToken:(NSData *)deviceToken {
  MainViewController *cordovaViewController = (MainViewController *)self.viewController;
  BrazePlugin *plugin = [cordovaViewController getCommandInstance:@"BrazePlugin"];
  [plugin.braze.notifications registerDeviceToken:deviceToken];
}

In Swift, this would be:

func application(
    _ application: UIApplication, didRegisterForRemoteNotificationsWithDeviceToken deviceToken: Data
) {
  if let cordovaViewController = self.viewController as? MainViewController {
    let plugin = cordovaViewController.getCommandInstance(pluginName: "BrazePlugin")
    plugin.braze?.notifications.register(deviceToken: deviceToken)
  }
}

Let me know if that works for you! We'll look into getting some examples added to our sample app.

ckuang commented 8 months ago

image (3)

Thanks for the speedy response! It looks like we're getting a Value of type 'AppDelegate' has no member 'viewController'

jerielng commented 8 months ago

Hey @ckuang, I should clarify that MainViewController is specific to our example app, but it's an implementation of CDVViewController, which is provided by the Cordova internals. Specific to your Xcode project, do you happen to have any class or view controller that implements CDVViewController? You'll need to reference that class.

katie-propel commented 8 months ago

@jerielng from the main AppDelegate.swift, I'm not seeing anything accessible that implements CDVViewController. The only place in the project I see that is deep in the AppDelegate.m for the CapacitorCordova Development Pod. Don't think that's something I should go changing, but even trying to add your code snippet, it's still not registering the device token with Braze.

jerielng commented 8 months ago

@katie-propel Yep, modifying that AppDelegate.m in CapacitorCordova wouldn't be appropriate since that's owned by the Capacitor internals, and it would also be overridden anytime you run pod install in either case.

The snippet provided would only succeed if it's attached to your app's AppDelegate, not any other that belongs to an imported pod.

That being said, we may need some additional details about your AppDelegate.swift implementation to provide further guidance. Would you be willing to share what your file looks like by writing in to support@braze.com and referencing this conversation for context?

jerielng commented 8 months ago

@katie-propel As a quick update update, is your AppDelegate implementing CDVCommandDelegateImpl? Could you try adding an implementation to CDVCommandDelegateImpl to your AppDelegate like this:

class AppDelegate: CDVCommandDelegateImpl { // This should get you access to the `getCommandInstance` function

  // From there, you can implement 
  func application(
      _ application: UIApplication, didRegisterForRemoteNotificationsWithDeviceToken deviceToken: Data
  ) {
      let plugin = self.getCommandInstance(pluginName: "BrazePlugin")
      plugin.braze?.notifications.register(deviceToken: deviceToken)
  }
}
katie-propel commented 8 months ago

Our AppDelegate is almost identical to the one automatically generated by Capacitor: https://github.com/ionic-team/capacitor/blob/main/ios-template/App/App/AppDelegate.swift aside from the fix above to override didRegisterForRemoteNotificationsWithDeviceToken. I had to replace UIResponder with CDVCommandDelegateImpl because of an error, but plugin is coming up as nil.

jerielng commented 8 months ago

Hi @katie-propel, we've just released 7.0.0 which exposes the static braze instance as a convenience property for iOS.

katie-propel commented 2 months ago

Thank you @jerielng! We finally got around to updating to 8.0.0 and were able to get it to work by adding the Braze plugin as a static plugin in the Capacitor config and updating AppDelegate.swift like so:

import CordovaPluginsStatic

@UIApplicationMain
class AppDelegate: UIResponder, UIApplicationDelegate {
...

  func application(_ application: UIApplication, didRegisterForRemoteNotificationsWithDeviceToken deviceToken: Data) {
      BrazePlugin.braze().notifications.register(deviceToken: deviceToken)
  }
}
jerielng commented 2 months ago

@katie-propel Awesome, that's great to hear! Thanks again as always for your thoughtful feedback!

benmarsh commented 3 weeks ago

Hi, I'm having this issue too now we've upgraded our app use 8.1.0...

@katie-propel are you able to share what the static plugin reference looks like in your Capacitor config file please? I'm getting a nil response when trying to interact with the plugin from AppDelegate.swift.

@jerielng Do you have any documentation on how to implement this?

Thank you!

katie-propel commented 3 weeks ago

@benmarsh In our Capacitor config we have this:

cordova: {
  preferences: { ... },
  staticPlugins: [
    'cordova-plugin-braze',
    ...
  ]
}

Is that what you're looking for?

benmarsh commented 2 weeks ago

@katie-propel exactly what I needed - thanks for this 👍🏼