Add ability to start live activities #196

nakajima · 2024-04-03T21:29:05Z

I believe this PR brings things in line with the JSON payload described in the docs.

This makes the APNSLiveActivityNotificationEvent type a bit more clunky to deal with, so I'd love to hear if anyone has any ideas on how to improve that.

I also ran swift-format --configuration .swift-format on my changes which seems to have brought in some unrelated format changes. If this is a PR worth pursuing I'm happy to clean those up.

See also #191.

nakajima · 2024-04-03T21:55:26Z

~~Ok actually it looks like I need to include an alert as well. On it.~~

Done in 93565bd.

nakajima · 2024-04-03T22:41:10Z

OK, I've verified that this is working in a test app so it's ready for review!

FranzBusch

Thanks for working on this. I left a few comments on the PR. Mostly this is a breaking API change so we have to do it slightly differently to avoid that breakage.

FranzBusch · 2024-04-04T07:33:47Z

Sources/APNSCore/LiveActivity/APNSLiveActivityNotificationEvent.swift

-public struct APNSLiveActivityNotificationEvent: Hashable {
-
-    /// The underlying raw value that is send to APNs.
-    @usableFromInline
-    internal let rawValue: String
-
-    /// Specifies that live activity should be updated
-    public static let update = Self(rawValue: "update")
-
-    /// Specifies that live activity should be ended
-    public static let end = Self(rawValue: "end")
+public protocol APNSLiveActivityNotificationEvent: Hashable, Encodable {


This is a breaking change and we can't just change from a struct to a protocol. We should try and continue to model it with the current struct type.

FranzBusch · 2024-04-04T07:34:48Z

Sources/APNSCore/LiveActivity/APNSLiveActivityNotification.swift

@@ -17,7 +17,9 @@ import struct Foundation.UUID
 /// A live activity notification.
 ///
 /// It is **important** that you do not encode anything with the key `aps`.
-public struct APNSLiveActivityNotification<ContentState: Encodable>: APNSMessage {
+public struct APNSLiveActivityNotification<ContentState: Encodable & Hashable & Sendable>:


We can't just add these requirements to the generic type. This is a breaking API change otherwise. We can make the conformance to those protocols conditional though and that should work.

FranzBusch · 2024-04-04T07:35:12Z

Sources/APNSCore/LiveActivity/APNSLiveActivityNotification.swift

+                return APNSLiveActivityNotificationEventEnd()
+            case "update":
+                return APNSLiveActivityNotificationEventUpdate()
+            default:


We should be exhaustive here and avoid using default cases if possible.

nakajima · 2024-04-04T16:31:14Z

@FranzBusch Thanks so much for the review! I've moved things around to just introduce a separate object for the additional data a start event needs. I think the one downside to this approach is that if you pass a start event without the startOptions, the push just won't get sent. I don't think that's the end of the world though since that's also what APNS does afaik.

We can't just add these requirements to the generic type. This is a breaking API change otherwise. We can make the conformance to those protocols conditional though and that should work.

Fixed in 5af6d92 and fffc9ea.

This is a breaking change and we can't just change from a struct to a protocol. We should try and continue to model it with the current struct type.

Fixed in 4671d0a.

nakajima · 2024-04-04T19:38:03Z

Actually this PR is wrong. I think we need to pass the ActivityAttributes as the attributes field, not the content state.

nakajima · 2024-04-04T20:15:30Z

Ok I've reworked things to support sending the correct attributes value. I added a new type for start notifications because otherwise the existing types would have to know about an Attributes generic value, which I believe would be a breaking change?

alex-taffe · 2024-04-10T15:04:58Z

Sources/APNSCore/LiveActivity/APNSLiveActivityNotification.swift

@@ -152,7 +151,7 @@ public struct APNSLiveActivityNotification<ContentState: Encodable>: APNSMessage
    ) {
        self.aps = APNSLiveActivityNotificationAPSStorage(
            timestamp: timestamp,
-            event: event.rawValue,
+						event: event.rawValue,


Just before this gets merged, funky indentation here

Fixed in 2d41546.

FranzBusch · 2024-05-03T17:57:45Z

Sources/APNSCore/LiveActivity/APNSStartLiveActivityNotification.swift

+    ///   - attributes: The ActivityAttributes of the live activity to start
+    ///   - attributesString: The type name of the ActivityAttributes you want to send
+    ///   - alert: An alert that will be sent along with the notification
+    public init(


Do we need two inits?

No I don't think we do, removed in a142abe.

FranzBusch

Looks good. Just one question

chrisschlitt · 2024-05-11T15:27:31Z

Can confirm this works, was able to start a live activity from a push notification with:

let notification = APNSStartLiveActivityNotification(
    expiration: .immediately,
    priority: .immediately,
    appID: "com.example.example",
    contentState: TestContentState(),
    timestamp: 1_672_680_658,
    attributes: TestAttributes(),
    attributesType: "TestAttributes",
    alert: .init(title: .raw("Test"), body: .raw("Test"))
)
do {
    let response = try await apnsClient.send(
        APNSRequest(
            message: notification,
            deviceToken: kit.token,
            pushType: .liveactivity,
            expiration: notification.expiration,
            priority: notification.priority,
            apnsID: notification.apnsID,
            topic: notification.topic,
            collapseID: nil
        )
    )
    // Success
} catch {
    // Failure
}

alex-taffe · 2024-05-13T14:15:42Z

I know this is somewhat off topic, but while everyone is here and I can solicit advice, Apple's docs state:

When the system receives the ActivityKit push notification on a device, it starts a new Live Activity, wakes up your app, and grants it background run time to allow you to download assets that the Live Activity needs.

Does anyone know what method is invoked or what happens when the start notification goes out? I'm severely doubting it's just application:didFinishLaunchingWithOptions: but I could be wrong. I'm going to play around with it because I need it to download image assets, but just figured I'd just check to see if anyone has already figured it out

chrisschlitt · 2024-05-13T14:38:03Z

I know this is somewhat off topic, but while everyone is here and I can solicit advice, Apple's docs state:

When the system receives the ActivityKit push notification on a device, it starts a new Live Activity, wakes up your app, and grants it background run time to allow you to download assets that the Live Activity needs.

Does anyone know what method is invoked or what happens when the start notification goes out? I'm severely doubting it's just application:didFinishLaunchingWithOptions: but I could be wrong. I'm going to play around with it because I need it to download image assets, but just figured I'd just check to see if anyone has already figured it out

I only recently added an app delegate (I’m in a SwiftUI app), so I’m not sure which handler gets called there yet, but in SwiftUI I noticed that the onChange handler for scenePhase was triggered with the .background value, where I’m iterating over ActivityKit’s static activities property to grab the push tokens and sending them to my backend.

nakajima added 4 commits April 3, 2024 12:35

Add start case

3540be5

Add ability to start live activities

0c2bb05

Run swift-format

d39bd0b

Remove unnecessary imports

e20b90a

Add alert to live activity start notifications

93565bd

FranzBusch reviewed Apr 4, 2024

View reviewed changes

nakajima added 5 commits April 4, 2024 09:09

Remove additional conformances

5af6d92

Remove another conformance

fffc9ea

formatting tweak

3d78f52

Move APNSLiveActivityNotificationEvent back to a struct.

4671d0a

format

4fb1daf

Oops missed this one

d2cdfb8

nakajima requested a review from FranzBusch April 4, 2024 19:19

nakajima added 3 commits April 4, 2024 13:11

format

2b43198

remove unrelated change

3818be8

remove unnecessary event from struct

f5087ba

alex-taffe reviewed Apr 10, 2024

View reviewed changes

Fix indentation

2d41546

FranzBusch reviewed May 3, 2024

View reviewed changes

Remove unnecessary init

a142abe

nakajima requested a review from FranzBusch May 7, 2024 15:59

kylebrowning merged commit 7cfeec8 into swift-server-community:main Jul 4, 2024
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ability to start live activities #196

Add ability to start live activities #196

Add ability to start live activities #196

Add ability to start live activities #196

Conversation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment