Skip to content

Update push subscription model properties between sessions#1922

Merged
nan-li merged 6 commits intouser-model/mainfrom
update_between_sessions
Dec 1, 2023
Merged

Update push subscription model properties between sessions#1922
nan-li merged 6 commits intouser-model/mainfrom
update_between_sessions

Conversation

@nan-li
Copy link
Copy Markdown
Contributor

@nan-li nan-li commented Nov 30, 2023

Description

One Line Summary

On new sessions, we will update the server with changes to app version, sdk version, carrier, and device OS.

Details

Motivation

If app version or sdk version is updated, these updates should be sent to the server.

Scope

1. Add these properties to Subscription Model
First, these properties should exist on Subscription Model. It now has sdk, deviceOS, carrier, and appVersion as properties so that they can persist. When push subscription models are initialized, these properties are now also set on the model.

2. On new sessions, check for subscription property updates
Let the SubscriptionManager refresh the push subscription model on new sessions.

3. Don't hydrate this info for a push subscription model
When hydrating an existing push subscription model, use existing local data for these properties.
This information should always come from the local device. The reason for this change is that on a new session, we may detect a change to one of these properties and we do not want to overwrite it with old remote data from the get_user response.

There are other ways to implement this PR, and open to discussing another approach.

Testing

Unit testing

None, should consider adding a test that can test this.

Manual testing

Android 33 Emulator

  • Tested new app installs
  • Tested with logging in
  • Tested an existing app installation that does not have this data saved, and then running a new session with the changes in this PR. It will enqueue extraneous update subscription operations because it had no previous data to compare, but this is acceptable as the SDK is on a new version anyway. Eventually, there will be a single update request sent with a snapshot of the push subscription model.

Affected code checklist

  • Notifications
    • Display
    • Open
    • Push Processing
    • Confirm Deliveries
  • Outcomes
  • Sessions
  • In-App Messaging
  • REST API requests
  • Public API changes

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 Reviewable

@nan-li nan-li changed the title Update between sessions Update push subscription model properties between sessions Nov 30, 2023
@emawby emawby self-requested a review November 30, 2023 20:47
Copy link
Copy Markdown
Contributor

@emawby emawby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 8 of 8 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @jennantilla, @jinliu9508, @jkasten2, @nan-li, and @shepherd-l)

nan-li and others added 6 commits December 1, 2023 11:30
Subscription Manager contains a method previously called `addOrUpdatePushSubscription` that is meant to be used in relation to updating push tokens.

This method is renamed to `addOrUpdatePushSubscriptionToken` so as to not confuse with other updates on the push subscription such as detecting app version changes between app opens, which is a newly added feature
Subscription Model now has sdk, deviceOS, carrier, and appVersion as properties so that they can persist.
The getter is defaulted to "" (the empty string) as these properties do not exist prior to v5.0.5. They are only read when detecting changes so it is ok that they default to "" as the empty string will not be sent.
Additionally set these properties when a push subscription model is created:
- sdk, deviceOS, carrier, appVersion
Let the SubscriptionManager refresh the push subscription model on new sessions.
When hydrating an existing push subscription model, use existing local device-scoped information instead of remote information for:
- sdk
- deviceOS
- carrier
- appVersion
This information should always come from the local device. The reason for this change is that on a new session, we may detect a change to one of these properties and we do not want to overwrite it with old remote data from the get_user response.
@nan-li nan-li force-pushed the update_between_sessions branch from 6328a5d to 7c8167d Compare December 1, 2023 19:42
@nan-li nan-li merged commit fef3aad into user-model/main Dec 1, 2023
@nan-li nan-li deleted the update_between_sessions branch December 1, 2023 20:11
@jennantilla jennantilla mentioned this pull request Dec 1, 2023
jinliu9508 pushed a commit that referenced this pull request Jan 31, 2024
Update push subscription model properties between sessions
jinliu9508 pushed a commit that referenced this pull request Jan 31, 2024
Update push subscription model properties between sessions
jinliu9508 pushed a commit that referenced this pull request Feb 6, 2024
Update push subscription model properties between sessions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants