Skip to content

Conversation

@nan-li
Copy link
Contributor

@nan-li nan-li commented Jul 30, 2025

Description

One Line Summary

When multiple subscriptions are in the create user payload, parse the subscriptions correctly, and this also corrects the handling of deleted push subscriptions.

Details

The SDK was relying on subscription order in the sent request and response to match subscriptions, but the order is not maintained. We fix this by deprecating transfer-subscription in favor of always using create-subscription. Create-subscription includes more properties that we can use to parse and match the subscriptions. By using create-subscription, this also fixes problems related to push subscriptions deleted and losing the subscription data, now that we always send all the push data. See #2327 for an example.

Motivation

  • Fix bug with wrong subscription ID being used for push
  • The fix also improves situations where the push subscription is deleted

Scope

  1. Reverted changes in fix: logout incorrectly uses the old subscription ID #2327
  2. Change all transfer subscription calls to create subscription calls, which will include all push sub properties instead of just the ID.
  3. No longer rely on the ordering of subscriptions in responses. Instead, parse the subscription data to match the local subscription with the remote subscription.
  4. If push subscription ID returned by the server is different, the SDK updates its push subscription ID (this is existing behavior).

Testing

Unit testing

None added

Manual testing

Android emulator on API 35

New install, add email immediately, delete the anon user, login

  1. SDK calls create user for the anon user with push sub and email sub in payload
  2. Subscription responses are parsed correctly
  3. Delete the user via the dashboard
  4. Call login(external_id)
  5. SDK calls identify user and gets 404
  6. SDK recovers by calling create user with the whole push sub in payload
  7. SDK parses subscription response correctly, push sub state is correct

Current user is anon and has an email sub, delete the push sub, call login to existing user

  1. Current user is anon and has an email sub (prevents the user from being deleted)
  2. Delete the push sub via the dashboard
  3. Call login(existing_external_id)
  4. Identify user returns 409
  5. SDK calls create user with the external ID and the whole push sub in the payload
  6. The response returns a different push subscription ID and the SDK replaces the existing ID with the new ID correctly
  7. SDK state is correct, push sub state is correct

Current user is anon and has an email sub, delete the push sub, call login to new user

  1. Current user is anon and has an email sub (prevents the user from being deleted)
  2. Delete the push sub via the dashboard
  3. Call login(brand_new_external_id)
  4. Identify user returns success
  5. SDK calls create subscription and includes the whole push sub in the payload
  6. The response returns a different push subscription ID and the SDK replaces the existing ID with the new ID correctly
  7. SDK state is correct, push sub state is correct

Current user is identified, delete the push sub, then login + add email

  1. SDK has an identified user
  2. Delete only the push subscription via the dashboard
  3. Login(external_id) and immediately add email
  4. SDK calls create user with the whole push sub and email sub in payload
  5. The response returns a different push subscription ID and the SDK replaces the existing ID with the new ID correctly
  6. SDK state is correct, push sub state is correct

Current user is identified, delete the push sub, then logout

  1. SDK has an identified user
  2. Delete only the push subscription via the dashboard
  3. Logout
  4. SDK calls create user for the anon user with the whole push sub in the payload
  5. The response returns a different push subscription ID and the SDK replaces the existing ID with the new ID correctly
  6. SDK state is correct, push sub state is correct

Delete the push subscription, call optIn/optOut to trigger sub update

  1. Delete the push subscription via the dashboard
  2. Call optIn or optOut
  3. SDK sends an update subscription call and receives a 404
  4. SDK recovers by calling create subscription
  5. Receive a new push subscription ID and the SDK replaces the existing ID with the new ID correctly
  6. SDK state is correct, push sub state is correct

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 added 2 commits July 30, 2025 12:43
* The transfer subscription operation only contains the subscription ID. It is possible for a push subscription to have been deleted on the server. In this case, only using transfer subscription will lose accurate push subscription data when the server creates a new push subscription. Missing data can include no push token, and inaccurate opted in or permission status.
* Instead, we will use Create Subscription instead which will include accurate push subscription data so the server can create a new push subscription accurately.
* When logging into new users, generate a create-subscription operation for the push subscription instead of enqueueing a transfer subscription operation. The Create Subscription operation has the added benefit of including the whole push subscription's data.
@nan-li nan-li added the WIP Work In Progress label Jul 30, 2025
* The subscriptions in the create user response are not returned in the order that they are sent. We cannot rely on ordering to match sent and returned subscriptions. So, we do our best to use data to match them.
* Fixes a bug where a push subscription and email/sms subscription are in the same request and the SDK extracts the wrong subscription ID for push.
@nan-li nan-li force-pushed the fix/android_push_subscription_issues branch from 333ab3f to e266904 Compare July 30, 2025 20:25
@nan-li nan-li changed the title fix(subscriptions): Fix parsing subscriptions from server, and improve handling of deleted push subscriptions fix(subs): Fix parsing subscriptions from server, and improve handling of deleted push subscriptions Jul 30, 2025
@nan-li nan-li removed the WIP Work In Progress label Aug 4, 2025
Copy link
Member

@jkasten2 jkasten2 left a comment

Choose a reason for hiding this comment

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

The code changes themselves look good, but can we add some tests?

Copy link
Contributor

@jinliu9508 jinliu9508 left a comment

Choose a reason for hiding this comment

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

I also verified various scenarios by performing identity and subscription operations after the subscription was deleted. All behaviors aligned with expectations.

@jinliu9508
Copy link
Contributor

@jkasten2 I have added two test cases for this change, verifying the mapping of subscriptions is correct and can handle the situation when push sub is deleted.

@jinliu9508 jinliu9508 requested a review from jkasten2 August 11, 2025 19:05
@jinliu9508 jinliu9508 merged commit c324f43 into main Aug 11, 2025
2 checks passed
@jinliu9508 jinliu9508 deleted the fix/android_push_subscription_issues branch August 11, 2025 20:30
@jinliu9508 jinliu9508 mentioned this pull request Aug 14, 2025
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.

4 participants