[5.0.0] Push subscription related fixes (3) #1289

nan-li · 2023-08-08T12:01:17Z

Description

One Line Summary

Three fixes that are push subscription related:

Send push subscription changes that were not captured before such as app_version or sdk
Initialize push subscriptions with a notification type
Send create push subscription request if we detect it has been deleted

Details

Motivation

1. Check for push subscription updates on new sessions

Properties on the push subscription can change between app opens such as app_version, sdk version, or device_os
These properties aren't driven by changes in the SDK and previously wouldn't be detected as changed
Now, on each new session, we will ask the push subscription model to check itself for updates and enqueue those changes, if any.
app_version particularly is important to keep updated for app developers

2. Initialize the push subscription model with a notification_type

Create the push sub model with the notification_types at time of creation
This is so that on the first app open of a new install, we can receive push prompt IAMs (the server expects to have an appropriate notification_type to determine eligibility)
Typically, the notification_type sent up on User Create is ERROR_PUSH_DELEGATE_NEVER_FIRED or -14

3. Recreate a push subscription to the server when we detect it is deleted

On new sessions, we fetch the SDK's user to update our local models of any changes to the user. If we detect our push subscription missing from the fetched user (indicating the push subscription may be deleted), we will send a create subscription request for our device's push subscription.

4. On the user manager, add a convenience accessor to the push subscription model

We access the push subscription model via the model store instead of via user.pushSubscriptionModel
If privacy consent is set in a wrong order, we may have sent requests, but by the time the request returns, we don't want to hydrate on a mock user's push subscription.
So, we want to set important information such as tokens and subscription ID on the actual push subscription model that the SDK will rely on, but user.pushSubscriptionModel may operate on a mock push subscription.

Testing

Unit testing

No

Manual testing

Tested on physical iPhone 13 on iOS 16.6

Scenario 1: Send push subscription changes that were not captured before such as app_version or sdk

Install app with an app_version and sdk version
Update app_version and sdk version and reopen app
See that the new app_version and sdk version is sent to server

Scenario 2: Initialize push subscriptions with a notification type

Prior to this PR, on the first app open, we will not receive the push prompt IAM. We will receive it on a second session.
Now, we will receive the push prompt IAM on the first app open.

Scenario 3: Send create push subscription request if we detect it has been deleted

In between 2 sessions, delete the push subscription via REST API
Reopen app and see a new create subscription is sent
Receive a new subscription_id

Affected code checklist

Checklist

Overview

I have filled out all REQUIRED sections above
PR does one thing
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.
I have reviewed this PR myself, ensuring it meets each checklist item

This change is

* Properties on the push subscription can change between app opens such as app version, SDK version, or device OS * These changes aren't driven by changes in the SDK and previously wouldn't be detected as changed * Now, on each new session, we will ask the push subscription model to check itself for updates and enqueue those changes.

- Create the push sub model with the notification_types at time of creation - This is so that on the first app open, we can receive push prompt IAMs (server expects to have an appropriate notification_type to determine eligibility)

* On a new session, we fetch the user. If we detect our push subscription missing from the user (indicating the push subscription may be deleted), we send a create subscription request for the device's push sub.

* On the user manager * We access the push subscription model via the model store instead of via `user.pushSubscriptionModel`. * If privacy consent is set in a wrong order, we may have sent requests, but hydrate on a mock user. * However, we want to set tokens and subscription ID on the actual push subscription model.

* If a createUser fails, let's pause the operation repo. We will still poll, but we won't flush it is paused. * After a create user request is uncached, update the `originalPushToken` property to be the push token associated with this request. Because, we update the push subscription before sending this request.

* On a new session, we will trigger the user executor to execute any pending requests, in case there is a failed create user request that should be retried. We can't rely on the operation repo flushing to trigger the user executor as it bypasses the operation repo.

* Use the existing `isCurrentUser` method * remove extraneous logging that make the logs messy

Cherry pick #1276

The JS dismiss event is being sent to the SDK twice. Make sure we don't dismiss twice or the second IAM in the queue will be dismissed before it is displayed

* Each executor manages its own background task * For attributed sessions, define another background task to send the session_time to update user (in addition to the existing request to the outcomes endpoint)

* When app is backgrounded, flush the operation repo and let each executor and request manage its background task * don't end background tasks until requests return * Processing the queues will take in another flag called `inBackground`

* Add callbacks before ending the background task to send attributed session time, and save unsent time as zero only after a success callback * This waits 30 seconds in the background

* When app is backgrounded, create a background task and send the session_time to the user endpoint * Have callbacks, bypassing the operation repo * This is done both for unattributed sessions that send the request right away and for attributed sessions that wait 30 seconds before sending

* Remove messy logging * Add informational log for background task expiring

* When we set tags and it goes over limit, we don't delete them locally. * Let's do the same for added aliases that can't be added to the user due to conflict * The SDK will refresh itself on new sessions * We will need better systematic way to address these types of failure cases

[5.0.0] Fix background tasks

[User model] Fix in app display

[5.0.0] Allow the SDK the chance to recover from failed Create User

* Rename `OSBackgroundTaskManagerDelegate` -> `OSBackgroundTaskHandler` because it is not truly a delegate, but is a concrete implementation that is injected

[5.0.0] Push subscription related fixes (3)

nan-li added 8 commits August 8, 2023 04:53

Create the push sub model with a notification type

2fe55b3

- Create the push sub model with the notification_types at time of creation - This is so that on the first app open, we can receive push prompt IAMs (server expects to have an appropriate notification_type to determine eligibility)

Recreate a push subscription when we detect it is removed

ea6c30c

* On a new session, we fetch the user. If we detect our push subscription missing from the user (indicating the push subscription may be deleted), we send a create subscription request for the device's push sub.

Add timezone_id to property model

af02758

[nits]

0291cfa

* Use the existing `isCurrentUser` method * remove extraneous logging that make the logs messy

nan-li changed the title ~~5.0.0/push sub fixes~~ [5.0.0] Push subscription related fixes (3) Aug 9, 2023

nan-li requested review from emawby, jkasten2, shepherd-l and jennantilla August 9, 2023 18:52

emawby approved these changes Aug 9, 2023

View reviewed changes

emawby and others added 13 commits August 9, 2023 16:50

Always remove the window when an IAM is dismissed

9641884

Cherry pick #1276

Ignore the second JS dismiss event

c34fc63

The JS dismiss event is being sent to the SDK twice. Make sure we don't dismiss twice or the second IAM in the queue will be dismissed before it is displayed

Define more background tasks

f9ac55d

* Each executor manages its own background task * For attributed sessions, define another background task to send the session_time to update user (in addition to the existing request to the outcomes endpoint)

Background flush of operation repo

ff6c5fa

* When app is backgrounded, flush the operation repo and let each executor and request manage its background task * don't end background tasks until requests return * Processing the queues will take in another flag called `inBackground`

Background sending of session ending outcomes

5a309b1

* Add callbacks before ending the background task to send attributed session time, and save unsent time as zero only after a success callback * This waits 30 seconds in the background

[nits] logging

2202fe7

* Remove messy logging * Add informational log for background task expiring

Merge pull request #1300 from OneSignal/5.0.0/rehaul_background_tasks

cc22dee

[5.0.0] Fix background tasks

Merge pull request #1294 from OneSignal/user_model/fix_in_app_display

c2a9be3

[User model] Fix in app display

Merge pull request #1290 from OneSignal/5.0.0/create_user_error

be65c89

[5.0.0] Allow the SDK the chance to recover from failed Create User

[nits] rename Background Task Delegate to Handler

1c8f185

* Rename `OSBackgroundTaskManagerDelegate` -> `OSBackgroundTaskHandler` because it is not truly a delegate, but is a concrete implementation that is injected

Merge branch '5.0.0/fix_session_influence' into 5.0.0/push_sub_fixes

bef92f7

Base automatically changed from 5.0.0/fix_session_influence to major_release_5.0.0 August 10, 2023 17:46

nan-li merged commit eeb94a5 into major_release_5.0.0 Aug 10, 2023
1 check failed

nan-li deleted the 5.0.0/push_sub_fixes branch August 10, 2023 17:52

This was referenced Aug 10, 2023

Release 5.0.0 #1301

Merged

Update package.swift for release 5.0.0 OneSignal/OneSignal-XCFramework#66

Merged

nan-li added a commit that referenced this pull request Oct 30, 2023

Merge pull request #1289 from OneSignal/5.0.0/push_sub_fixes

37e7637

[5.0.0] Push subscription related fixes (3)

nan-li added a commit that referenced this pull request Oct 30, 2023

Merge pull request #1289 from OneSignal/5.0.0/push_sub_fixes

9dae545

[5.0.0] Push subscription related fixes (3)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[5.0.0] Push subscription related fixes (3) #1289

[5.0.0] Push subscription related fixes (3) #1289

nan-li commented Aug 8, 2023 •

edited

Loading

[5.0.0] Push subscription related fixes (3) #1289

[5.0.0] Push subscription related fixes (3) #1289

Conversation

nan-li commented Aug 8, 2023 • edited Loading

Description

One Line Summary

Details

Motivation

Testing

Unit testing

Manual testing

Affected code checklist

Checklist

Overview

Testing

Final pass

nan-li commented Aug 8, 2023 •

edited

Loading