Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Umbrella issue for the Apple location push related functionality across different repositaries #1763

Open
maratal opened this issue Jun 13, 2023 · 16 comments
Labels
code-quality Affects the developer experience when working in our codebase. documentation Improvements or additions to public interface documentation (API reference or readme). enhancement New feature or improved functionality.

Comments

@maratal
Copy link
Collaborator

maratal commented Jun 13, 2023

Apple article on how to implement location pushes:
https://developer.apple.com/documentation/corelocation/creating_a_location_push_service_extension

ably-cocoa: #1762

Server:

  1. Handle additional dictionary parameter in POST "/push/deviceRegistrations".
    a) In addition to "push.recipient.deviceToken", "push.recipient.locationDeviceToken" should be read from client's request and saved.
    b) Implement sending location push according to https://developer.apple.com/documentation/corelocation/creating_a_location_push_service_extension#3875733 and https://developer.apple.com/documentation/usernotifications/setting_up_a_remote_notification_server/sending_notification_requests_to_apns

Docs:

  1. Update APNs tutorial at https://ably.com/tutorials/ios-location-push-notifications on how to receive and save location push token.

Specs:

  1. Update RSH8i (or create additional) spec point for location pushes. Make a note that it's an iOS and iPadOS only.

┆Issue is synchronized with this Jira Task by Unito

@sync-by-unito
Copy link

sync-by-unito bot commented Jun 13, 2023

➤ Automation for Jira commented:

The link to the corresponding Jira issue is https://ably.atlassian.net/browse/SDK-3643

@maratal
Copy link
Collaborator Author

maratal commented Jun 14, 2023

https://ably.com/docs/api/rest-api#post-device-registration

curl -X POST https://rest.ably.io/push/deviceRegistrations \
 -u "******"
 -H "Content-Type: application/json" \
 --data \
'{
  "id": "*****",
  "platform": "ios",
  "formFactor": "phone",
  "push": {
    "recipient": {
      "transportType": "apns",
      "deviceToken": "***"
    }
  }
}

I would suggest instead of "transportType" to use "apns" (or "fcm") dictionary. It will help to eliminate potential problems if you need to change something on one platform, and not to mess up with the other.

"push": {
   "recipient": {
      "apns": {
         "deviceTokens": {
            "default": "***",
            "location": "****",
            "voip": "*****",
            "pushtotalk": "******"
         }
      }
    }
  }

Device tokens are different, as well as API used for their obtain. Keys in the "deviceTokens" (plural) are values from Apple's documentation for "apns-push-type" header field: https://developer.apple.com/documentation/usernotifications/setting_up_a_remote_notification_server/sending_notification_requests_to_apns
WDYT @lawrence-forooghian

@lawrence-forooghian
Copy link
Collaborator

I think the idea of being able to specify a separate token to be used for a specific push type is a sensible one.

It will help to eliminate potential problems if you need to change something on one platform, and not to mess up with the other.

Could you please elaborate on what scenario you're suggesting this helps us avoid, @maratal? A device registration applies to a single device and hence a single platform (note the top-level platform key).

@maratal
Copy link
Collaborator Author

maratal commented Jun 14, 2023

I've meant having keys for different platforms in one dictionary is probably not the best recipe to handle on the server, but someone from a server side team can have a different opinion on it @lawrence-forooghian

@Peter-Kubik
Copy link

So just to clarify, for each different type of apns-push-type, the iOS app needs to generate a separate token, where the API to generate this token is different of each type?

Certainly changing the API completely might be quite difficult at the backend, especially because we would need to handle both types for ongoing support.

@maratal
Copy link
Collaborator Author

maratal commented Jun 15, 2023

where the API to generate this token is different of each type

Correct. All those tokens come from different iOS frameworks.

might be quite difficult at the backend

Agree, what about adding just one extra token to speed up process and then think of more neat solution? @Peter-Kubik @lawrence-forooghian

@Peter-Kubik
Copy link

Peter-Kubik commented Jun 15, 2023

@maratal Yeah, I think adding deviceLocationToken should be sufficient for now at least. Just to clarify, for the the location push notifications, our server should send the notification to /3/device/<locationDeviceToken> instead of /3/device/<deviceToken>?

@maratal
Copy link
Collaborator Author

maratal commented Jun 15, 2023

At least it's what Apple says here @Peter-Kubik

@maratal
Copy link
Collaborator Author

maratal commented Jun 15, 2023

@stmoreau @lawrence-forooghian WDYT of adding just one additional token for simplicity?

@maratal
Copy link
Collaborator Author

maratal commented Jun 15, 2023

@Peter-Kubik Some more information on how to handle device tokens here:
https://developer.apple.com/documentation/pushkit/supporting_pushkit_notifications_in_your_app#3377584

Because users can run your app on multiple devices, be prepared to handle multiple tokens for each notification type your app supports.

Also see https://developer.apple.com/documentation/pushkit/pkpushtype-swift.struct

@Peter-Kubik
Copy link

@maratal I have talked to Paddy, and he said that for now, we should do the bare minimum to support the new feature of the GM app, which will require location push notification, so I think for now just sending the location token is fine.

Also, thanks for the docs!

@maratal
Copy link
Collaborator Author

maratal commented Jun 15, 2023

Would you be able to read this dictionary? Where "deviceToken" is an old token, and "deviceTokens" (plural) - is the new one? You just read it like recipient["deviceTokens"]["location"]. Or it's important to keep it as a simple additional variable?

"push": {
   "recipient": {
       "transportType": "apns",
       "deviceToken": "***",
       "deviceTokens": {
           "default": "***",
           "location": "****"
        }
    }
}

I mean it's two different dictionaries - one without "deviceTokens" is sent by old clients and with "deviceTokens" (but without "deviceToken" (singular)) by the new ones. @Peter-Kubik

@Peter-Kubik
Copy link

@maratal I don't think it would be an issue. I am just cautious of the very small difference in the name deviceToken and deviceTokens. Would deviceTokens.default be the same as deviceToken ?

@maratal
Copy link
Collaborator Author

maratal commented Jun 15, 2023

Would deviceTokens.default be the same as deviceToken

Yes, but as I've mentioned above, not in the same request.

@Peter-Kubik
Copy link

Ah, I got you. I think if we were to do it this way, we should consider maybe apnsDeviceTokens instead of deviceTokens to make it clearly distinguished from deviceToken. What do you think?

@maratal
Copy link
Collaborator Author

maratal commented Jun 15, 2023

Yeah, it's less confusing this way, will do.

@umair-ably umair-ably added enhancement New feature or improved functionality. documentation Improvements or additions to public interface documentation (API reference or readme). code-quality Affects the developer experience when working in our codebase. labels May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code-quality Affects the developer experience when working in our codebase. documentation Improvements or additions to public interface documentation (API reference or readme). enhancement New feature or improved functionality.
Development

No branches or pull requests

4 participants