square/SuperDelegate

RemoteNotification should search for values in "aps" dictionary instead of top level.

NinoScript opened this issue · 9 comments

I'm not really sure about this, as I'm new to using remote notifications.
But reading the documentation gives me the impression that the userInfo dictionary has an aps dictionary inside with the values.

So the RemoteNotification's init should actually look more like this:

public init?(remoteNotification: [NSObject : AnyObject]?) {
    guard let remoteNotification = remoteNotification as? [String : AnyObject] else {
        return nil
    }

    let apsKey = "aps"
    let aps = remoteNotification[apsKey] as? [String : AnyObject]

    alert = aps.flatMap(Alert.init(remoteNotification:))
    badge = aps?[badgeKey] as? Int
    sound = aps?[soundKey] as? String
    contentAvailable = aps?[contentAvailableKey] != nil
    categoryIdentifier = aps?[categoryKey] as? String
    remoteNotificationDictionary = remoteNotification

    var customFields = remoteNotification
    customFields.removeValueForKey(apsKey)

    userInfo = customFields
}

At least that seems to work for me, what do you think?

dfed commented

LaunchItem creates a RemoteNotification with the launchOptions[UIApplicationLaunchOptionsRemoteNotificationKey], which creates a valid remote notification object. When a notification is created via didReceiveRemoteNotification, the notification is not wrapped in an aps key. I believe this works as expected — SuperDelegate is used by Square Cash, and our notification processing works.

Are notifications not working for you with SuperDelegate? If not, can you provide specific reproduction steps and a sample remote notification dictionary payload?

You are supposed to wrap the badge, alert, sound and content-available keys in an aps dictionary when you create the payload in your backend.

Unless you're doing something manually, your notification shouldn't sound or show a badge if those keys aren't inside the aps dictionary.

(At least that's what I understand from the documentation, I also asked in a slack channel and people agree, so I'm not totally crazy.)

dfed commented

I think you're right re how the server should format the APNS dictionary (I've only done client work, not server work). That said, I'm pretty darned sure we're right on how we're processing RemoteNotifications.

@NinoScript, are you not seeing your RemoteNotifications parsed correctly? Or do you think there's a theoretical bug after reading the code? If the former, can you paste in an NSLog (or print from Swift or po from LLDB) of the remote notification we're having trouble parsing?

Square Cash works just fine with badges, notification sounds, etc. So I'm reasonably convinced that we're doing the right thing on our side. But I'm curious what your experience is!

I'm not seeing RemoteNotifications parsed correctly, this is not theoretical, and the code I posted makes it work.

Here's an example of a working remote notification, where the RemoteNotification object has sound and contentAvailable incorrectly set to nil and false respectively.

(this was printed inside RemoteNotification.init)

(lldb) po remoteNotification
▿ 5 elements
  ▿ [0] : 2 elements
    - .0 : "type"
    - .1 : messages.create
  ▿ [1] : 2 elements
    - .0 : "user_id"
  ▿ [2] : 2 elements
    - .0 : "organization_id"
  ▿ [3] : 2 elements
    - .0 : "aps"
    ▿ .1 : 2 elements
      ▿ [0] : 2 elements
        - .0 : sound
        - .1 : 
      ▿ [1] : 2 elements
        - .0 : content-available
  ▿ [4] : 2 elements
    - .0 : "conversation_id"

(lldb) po remoteNotification.debugDescription
"[\"type\": messages.create, \"user_id\": 30, \"organization_id\": 1, \"aps\": {\n    \"content-available\" = 1;\n    sound = \"\";\n}, \"conversation_id\": 1426]"

I think in your case you're probably re-firing a local notification with that data.

Look, here's some random people printing the contents of userInfo inside application(application:, didReceiveRemoteNotification userInfo:)
http://stackoverflow.com/q/28596295/1904287
http://stackoverflow.com/q/34399848/1904287

You'll notice in both cases they access those properties inside userInfo["aps"]

dfed commented

I'm not firing a local notification (we rarely ever do that). Looking into this. It's possible we missed something since we don't tend to use alert, badge or sound keys in Cash.

dfed commented

Fix coming for the 0.9 branch shortly!

dfed commented

0.9.1 has the fix. Waiting for review on 0.8.1.

dfed commented

0.8.1 has been pushed to cocoapods!

That's great! Now I can remove my fix and pull the latest version. :)