Live Activity Support #1152

fhboswell · 2022-11-15T18:32:37Z

Description

One Line Summary

Adds Live Activity support for the SDK

Details

Motivation

Adding 2 sdk methods that allow a customer to register their temporary live activity token with an activity ID on OneSignal's backend

Scope

Adding two SDK public methods and two corresponding requests

Testing

Unit testing

Added several tests to test the requests and the public SDK method

Manual testing

The backend piece is not available yet so only testing with mock data.

Affected code checklist

Checklist

Overview

[ x] I have filled out all REQUIRED sections above
PR does one thing
- If it is hard to explain how any codes changes are related to each other then it most likely needs to be more than one PR
Any Public API changes are explained in the PR details and conform to existing APIs

Testing

I have included test coverage for these changes, or explained why they are not needed
All automated tests pass, or I explained why that is not possible
I have personally tested this on my device, or explained why that is not possible

Final pass

Code is as readable as possible.
- Simplify with less code, followed by splitting up code into well named functions and variables, followed by adding comments to the code.
I have reviewed this PR myself, ensuring it meets each checklist item
- WIP (Work In Progress) is ok, but explain what is still in progress and what you would like feedback on. Start the PR title with "WIP" to indicate this.

This change is

emawby

Reviewable status: 0 of 6 files reviewed, 5 unresolved discussions (waiting on @fhboswell, @jennantilla, @jkasten2, and @nan-li)

iOS_SDK/OneSignalSDK/OneSignalCore/Source/API/OSRequests.m line 500 at r1 (raw file):

                     appId:(NSString * _Nonnull)appId
                activityId:(NSString * _Nonnull)activityId
                     token:(NSString * _Nullable)token {

Design question - Are we sure the token should be nullable? Is this so that folks can track a live activity with OneSignal even if they aren't updating it via push?

iOS_SDK/OneSignalSDK/Source/OneSignal.h line 298 at r1 (raw file):

#pragma mark Live Activity
typedef void (^OSLiveActivitySuccessBlock)();

Design question - Is there any server response info we want to pass as a parameter to this? My gut reaction is that there isn't, but thought I would ask.

iOS_SDK/OneSignalSDK/Source/OneSignal.m line 671 at r1 (raw file):

}
+ (void)enterLiveActivity:(NSString * _Nonnull)activityId withToken:(NSString * _Nullable)token withSuccess:(OSLiveActivitySuccessBlock _Nullable)successBlock withFailure:(OSLiveActivityFailureBlock _Nullable)failureBlock{

We probably want to check privacy consent here (see sendTags for an example)

iOS_SDK/OneSignalSDK/Source/OneSignal.m line 672 at r1 (raw file):

+ (void)enterLiveActivity:(NSString * _Nonnull)activityId withToken:(NSString * _Nullable)token withSuccess:(OSLiveActivitySuccessBlock _Nullable)successBlock withFailure:(OSLiveActivityFailureBlock _Nullable)failureBlock{
    
    [OneSignalClient.sharedClient executeRequest:[OSRequestLiveActivityEnter withUserId:self.currentSubscriptionState.userId appId:appId activityId:activityId token:token]

What happens if we don't have the userId yet? We should probably queue it up as a pending call to make once register user has finished successfully. See tags as an example.

iOS_SDK/OneSignalSDK/Source/OneSignal.m line 685 at r1 (raw file):

+ (void)exitLiveActivity:(NSString * _Nonnull)activityId withSuccess:(OSLiveActivitySuccessBlock _Nullable)successBlock withFailure:(OSLiveActivityFailureBlock _Nullable)failureBlock{

same comments for Exit as in Enter

iOS_SDK/OneSignalSDK/Source/OneSignal.h

iOS_SDK/OneSignalSDK/UnitTests/RequestTests.m

iOS_SDK/OneSignalSDK/OneSignalCore/Source/API/OSRequests.m

fhboswell

Reviewable status: 0 of 6 files reviewed, 7 unresolved discussions (waiting on @emawby, @fhboswell, @jennantilla, @jkasten2, and @nan-li)

iOS_SDK/OneSignalSDK/OneSignalCore/Source/API/OSRequests.m line 500 at r1 (raw file):

Previously, emawby (Elliot Mawby) wrote…

Design question - Are we sure the token should be nullable? Is this so that folks can track a live activity with OneSignal even if they aren't updating it via push?

It should not be nullable

iOS_SDK/OneSignalSDK/Source/OneSignal.h line 298 at r1 (raw file):

Previously, emawby (Elliot Mawby) wrote…

Design question - Is there any server response info we want to pass as a parameter to this? My gut reaction is that there isn't, but thought I would ask.

Neither API endpoint has anything data in the response, see tech spec.

iOS_SDK/OneSignalSDK/Source/OneSignal.h line 301 at r1 (raw file):

Previously, jkasten2 (Josh Kasten) wrote…

token should be _Nonnull, as it is required.

Done.

iOS_SDK/OneSignalSDK/Source/OneSignal.h line 302 at r1 (raw file):

Previously, jkasten2 (Josh Kasten) wrote…

same _Nonnull as noted above.

Done.

iOS_SDK/OneSignalSDK/Source/OneSignal.m line 671 at r1 (raw file):

Previously, emawby (Elliot Mawby) wrote…

We probably want to check privacy consent here (see sendTags for an example)

Done.

fhboswell

Reviewable status: 0 of 6 files reviewed, 7 unresolved discussions (waiting on @emawby, @jennantilla, @jkasten2, and @nan-li)

iOS_SDK/OneSignalSDK/Source/OneSignal.m line 685 at r1 (raw file):

Previously, emawby (Elliot Mawby) wrote…

same comments for Exit as in Enter

Done.

emawby

Reviewable status: 0 of 6 files reviewed, 2 unresolved discussions (waiting on @jennantilla, @jkasten2, and @nan-li)

jkasten2

Reviewed 1 of 6 files at r1, 2 of 4 files at r2, 3 of 3 files at r3, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @fhboswell, @jennantilla, and @nan-li)

iOS_SDK/OneSignalSDK/OneSignalCore/Source/API/OSRequests.m line 500 at r3 (raw file):

                     appId:(NSString * _Nonnull)appId
                activityId:(NSString * _Nonnull)activityId
                     token:(NSString * _Nullable)token {

token here should be _Nonnull.

iOS_SDK/OneSignalSDK/Source/OneSignal.m line 722 at r3 (raw file):

+ (void)executePendingLiveActivityUpdates{
    if(pendingLiveActivityUpdates.count <= 0){

nit, missing spaces which doesn't match the rest of the formatting. Here as well as a few other places.

fhboswell

Reviewable status: 3 of 6 files reviewed, 1 unresolved discussion (waiting on @jennantilla, @jkasten2, and @nan-li)

iOS_SDK/OneSignalSDK/OneSignalCore/Source/API/OSRequests.m line 500 at r3 (raw file):

Previously, jkasten2 (Josh Kasten) wrote…

token here should be _Nonnull.

Done

fhboswell

Dismissed @jkasten2 from a discussion.
Reviewable status: 3 of 6 files reviewed, all discussions resolved (waiting on @jennantilla, @jkasten2, and @nan-li)

jkasten2

Reviewed 3 of 3 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @jennantilla and @nan-li)

fhboswell added 6 commits November 3, 2022 14:43

Update OneSignal.h to include headers for new customer facing methods.

6c6760c

Adding interfaces to OSRequest.h

e8808b8

Adding implementation for the new OSRequest subclasses in OSRequests.m

a0d8656

Adding impl for new SDK API in OneSignal.m

55c87d8

updating request path

3e76f6f

Adding tests

9451da5

fhboswell changed the title ~~Live Activity Support WiP~~ Live Activity Support Nov 15, 2022

fhboswell changed the title ~~Live Activity Support~~ Live Activity Support WiP Nov 15, 2022

Adding further tests

e3a4435

fhboswell requested review from jkasten2, emawby and nan-li November 15, 2022 22:56

fhboswell changed the title ~~Live Activity Support WiP~~ Live Activity Support Nov 15, 2022

fhboswell requested a review from jennantilla November 15, 2022 23:03

emawby suggested changes Nov 15, 2022

View reviewed changes

jkasten2 requested changes Nov 15, 2022

View reviewed changes

PR feedback NONULL, Privacy consent, nits

a617887

fhboswell commented Nov 15, 2022

View reviewed changes

Add pending live activity update support

63a1b9c

emawby approved these changes Nov 16, 2022

View reviewed changes

fhboswell requested a review from jkasten2 November 16, 2022 01:29

jkasten2 requested changes Nov 16, 2022

View reviewed changes

token should be _Nonull in OSRequest

720d1f4

fhboswell commented Nov 16, 2022

View reviewed changes

fhboswell requested a review from jkasten2 November 16, 2022 16:15

fhboswell commented Nov 16, 2022

View reviewed changes

jkasten2 approved these changes Nov 16, 2022

View reviewed changes

fhboswell merged commit 32de71b into main Nov 16, 2022

fhboswell deleted the live-activity-support branch November 16, 2022 18:26

This was referenced Nov 16, 2022

Release 3.12.0 #1154

Merged

Release 3.12.0 OneSignal/OneSignal-XCFramework#47

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Live Activity Support #1152

Live Activity Support #1152

fhboswell commented Nov 15, 2022 •

edited

Loading

emawby left a comment

fhboswell left a comment

fhboswell left a comment

emawby left a comment

jkasten2 left a comment

fhboswell left a comment

fhboswell left a comment

jkasten2 left a comment

Live Activity Support #1152

Live Activity Support #1152

Conversation

fhboswell commented Nov 15, 2022 • edited Loading

Description

One Line Summary

Details

Motivation

Scope

Testing

Unit testing

Manual testing

Affected code checklist

Checklist

Overview

Testing

Final pass

emawby left a comment

Choose a reason for hiding this comment

fhboswell left a comment

Choose a reason for hiding this comment

fhboswell left a comment

Choose a reason for hiding this comment

emawby left a comment

Choose a reason for hiding this comment

jkasten2 left a comment

Choose a reason for hiding this comment

fhboswell left a comment

Choose a reason for hiding this comment

fhboswell left a comment

Choose a reason for hiding this comment

jkasten2 left a comment

Choose a reason for hiding this comment

fhboswell commented Nov 15, 2022 •

edited

Loading