[abdulraqeeb33]

Description

One Line Summary

Composite login lets the caller set subscription status per channel, so the SDK stops deciding it for them.

Details

Fixes SDK-5623. Follow-up to #2815 and #2816, both merged.

Motivation

The SDK gives customers no way to set email or SMS status. Today the outcome depends on whether addEmail() happened to batch with login(), which depends on network timing and queue state. I reproduced both sides on an emulator with two addresses seeded identically unsubscribed and the same addEmail() payload:

	Batched with login	Separate call
Endpoint	POST /users	POST /users/by/onesignal_id/.../subscriptions
SDK sent	enabled: true, notification_types: 1	enabled: true, notification_types: 1
Result	true / 1, resubscribed	false / -99, unchanged

Same method, same body, two outcomes. Jon F sized the exposure at 92.4M unsubscribed email addresses and 12.5M unsubscribed phone numbers across apps using mobile push with external IDs. Rather than pick whichever side of that race wins more often, the caller states intent. Decision thread.

Scope

OneSignalUserProfile gains smsEnabled alongside emailEnabled, both on the Kotlin properties and the Java builder. The demo app gains matching inputs on the Login With Profile dialog. Each flag maps straight to the wire for its own channel:

Flag	enabled	notification_types
unset	omitted	omitted
true	true	1
false	false	-2

Both flags are persisted on LoginUserOperation and carried through mergeProfileFrom, so a merged login does not lose them. A status is scoped to the address it was stated for: when two composite logins merge, a null status on the later one keeps the earlier status for the same address, and never carries it onto a different address. Local optedIn hydration now reads the flag for whichever channel the backend returned instead of email only, and is applied to a local record that already exists rather than only to a new one.

A batched addEmail or addSms for the same address already carries a subscribed status, and addSubscriptions dedupes on type plus token, so the profile entry was skipped entirely in that case. An explicit profile status now overrides the grouped operation's status for that address, on the wire and on the local record, matched by address so a sibling address of the same channel is untouched. An unset profile status leaves the grouped operation untouched.

Status is written to local models with ModelChangeTags.HYDRATE. It records what Create User already applied, so a NORMAL write on an attached model would make ModelStoreListener enqueue a redundant Update Subscription.

Address matching goes through one shared sameSubscriptionAddress, case-insensitive for email and exact for SMS, used by the grouped lookup, the merge, and hydration. Before that the grouped lookup compared exactly, so addEmail("Bob@Example.com") batched with a profile for bob@example.com put two objects for one mailbox in the same request with opposite statuses.

Identity-only login(externalId) and the granular addEmail / addSms methods are unchanged.

The detekt baseline gains one LongParameterList entry, because the internal Kotlin shorthand for building a profile in tests now takes six parameters. I added only that entry rather than regenerating the baseline, which would have pulled in unrelated drift.

Why -2 and not -99

-2 is UNSUBSCRIBE. The subscriptions docs define it as user opted out, set by optOut() in the SDK or by enabled: false via the API. -99 is reserved for never asked, and it is also what a brand-new subscribed address gets, so it cannot express opt-out intent.

Omitting notification_types on an enabled: false is worse than wrong: turbine then assigns -31 (DISABLED_FROM_REST_API) itself, which misattributes an SDK opt-out to the REST API. Measured:

Starting address	Sent	Stored
Brand new	enabled: false only	false / -99
Subscribed	enabled: false only	false / -31
Subscribed	enabled: false, notification_types: -2	false / -2

Requested behavior vs. what is coded

Every row was confirmed against the API on a test app. The SDK sends the payload; the backend produces the outcome.

The address already exists in the app

#	Requested	SDK sends	Confirmed outcome
1.1	Unsubscribed, enabled not set, stays unsubscribed	nothing	stayed false / -99	✅
1.2	Unsubscribed, enabled: true, becomes subscribed	true, 1	became true / 1	✅
1.3	Unsubscribed, enabled: false, stays unsubscribed	false, -2	stayed false / -2	✅
1.4	Subscribed, enabled not set, stays subscribed	nothing	stayed true / -99	✅
1.5	Subscribed, enabled: true, stays subscribed	true, 1	stayed true / 1	✅
1.6	Subscribed, enabled: false, becomes unsubscribed	false, -2	became false / -2	✅

The address is created inside the login call

#	Requested	SDK sends	Confirmed outcome
2.1	enabled omitted, subscribed	nothing	true / -99, subscribed	✅
2.2	enabled: true, subscribed	true, 1	true / 1	✅
2.3	enabled: false, unsubscribed	false, -2	false / -2	✅

Before this PR, 1.2 was the broken case: true was dropped on the wire, so an opted-out address could not be resubscribed. 1.5 and 2.2 landed on the right end state by accident. None of the SMS cases existed, because there was no smsEnabled.

One caveat on 2.1. The end state is subscribed as requested, but notification_types is -99 rather than 1. Perseus treats those differently, since -99 is never asked and 1 emits optin.first_accepts. That needs the turbine change to default a new address with no status to subscribed, and is tracked separately.

Testing

Unit testing

LoginUserOperationExecutorTests pins the six payload shapes, three flag states across two channels:

• Unset sends no enabled and no notification_types, for email and SMS together.
• true sends enabled: true with notification_types: 1, for email and for SMS.
• false sends enabled: false with notification_types: -2, for email and for SMS, and hydrates the local subscription as opted out.
• Both flags survive a LoginUserOperation JSON round trip.
• An explicit status overrides a grouped CreateSubscriptionOperation for the same address.
• mergeProfileFrom keeps an earlier status when the later login states none, and takes the later one when it does.
• mergeProfileFrom drops the earlier status when the address changes, for email and SMS, and still takes an explicit status for a new address.
• An unsubscribe applies to an email that is already in the local store.
• A profile status reaches the local record of a grouped subscription, and a grouped subscription for a different address of the same channel is left alone.
• A grouped email differing only by case is overridden rather than duplicated.
• A real SubscriptionModelStoreListener over a mock operation repo asserts that hydrating a status enqueues nothing.
• An unset flag leaves a local record's status alone, an explicit true resubscribes an unsubscribed local record, and an SMS status with no SMS address is ignored.

OneSignalUserProfileTests covers smsEnabled being independent of emailEnabled and the Java builder setting each channel.

Unit tests can only assert what the SDK sends. The nine outcomes above are backend behavior and were confirmed by API call, not by unit test.

I reverted each production change and confirmed the matching tests go red: both "enabled is true" tests, the grouped-subscription override, the mergeProfileFrom preservation, and the existing-local-record unsubscribe.

Manual testing

The nine rows above were run against a test app through the API.

The demo app now has Email status and SMS status inputs on the Login With Profile dialog, where blank means no intent. All nine cases were then run end to end on an emulator against this build (onesignal/android/051101), each paired with the matching SMS case, so 18 outcomes, nine per channel. Every row matched, and email and SMS behaved identically.

For group 1 the address was seeded first and transferred in by the login. The seed users were confirmed to hold nothing afterwards, so those were genuinely existing-address cases.

#	Flag	Before	Sent	After
1.1	unset	false / -2	nothing	false / -2
1.2	true	false / -2	true, 1	true / 1
1.3	false	false / -2	false, -2	false / -2
1.4	unset	true / 1	nothing	true / 1
1.5	true	true / 1	true, 1	true / 1
1.6	false	true / 1	false, -2	false / -2
2.1	unset	new	nothing	true / -99
2.2	true	new	true, 1	true / 1
2.3	false	new	false, -2	false / -2

External IDs dev11 through dev16 and dev21 through dev23 on app 77e32082. Logs in the retest comment on this PR.

The one row worth noting is 2.1: subscribed as specified, but at notification_types: -99 rather than 1, since nothing is sent and the backend defaults a new address to "never asked". That needs the turbine default change and cannot be fixed by the SDK sending a status it was not given.

The batched versus separate reproduction in the Motivation section was run on an emulator against a local build of the merged #2815 and #2816.

Affected code checklist

• Notifications
• 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

[github-actions]

📊 Diff Coverage Report

Diff Coverage Report (Changed Lines Only)

Gate: aggregate coverage on changed executable lines must be ≥ 80% (JaCoCo line data for lines touched in the diff).

Changed Files Coverage

• OneSignalUserProfile.kt: 6/7 touched executable lines (85.7%) (26 touched lines in diff)
• LoginUserOperation.kt: 10/10 touched executable lines (100.0%) (30 touched lines in diff)
• LoginProfileApplier.kt: 31/31 touched executable lines (100.0%) (56 touched lines in diff)
• LoginUserOperationExecutor.kt: 26/28 touched executable lines (92.9%) (50 touched lines in diff)

Overall (aggregate gate)

73/76 touched executable lines covered (96.1% — requires ≥ 80%)

📥 View workflow run

[fadi-george]

Title it a fix? I'd imagine we need to make 5.11.1 with this.

[abdulraqeeb33]

hmm, thats a good point. i thought the release merge was pending. also its not a bug fix, its more like a new minor feature we are allowing the user to pass the flag value.

[fadi-george]

Or will need to go back to old pr, update pr notes.

[abdulraqeeb33]

Or will need to go back to old pr, update pr notes.

that would be ideal. for now, please review the PR. i will work on merging and updating the notes etc

[fadi-george]

I feel we should just make loginWithProfile private if we keep finding stuff but could do that if this keeps going till Friday. Still reviewing.

[fadi-george]

Thanks, one more finding:

• Non-blocking, LoginUserOperation.kt:120-126: a null flag no longer overwrites, even when the address changes. If login(email = a, emailEnabled = false) is still queued and login(email = b) with no flag merges into it, the result is email = b, emailEnabled = false. That sends enabled: false, -2 and unsubscribes b, which the caller never asked for. SMS has the same problem. The new test only covers the same address. A fix would be to keep the earlier flag only when the address is the same:

  if (other.email != email) emailEnabled = other.emailEnabled else other.emailEnabled?.let { emailEnabled = it }

  I'd call it Non-blocking: it needs two composite logins for the same user queued before the first one sends, with different addresses. But the effect is an unrequested unsubscribe, the exact outcome this PR is meant to prevent.

[abdulraqeeb33]

Good catch, and that one was caused by my fix for your earlier merge finding. Reproduced it as a failing test for both channels before changing anything.

Fixed in 51c27ac, using the shape you suggested, pulled into a helper since both channels need it:

private fun mergedStatus(sameAddress: Boolean, current: Boolean?, incoming: Boolean?): Boolean? =
    if (sameAddress) incoming ?: current else incoming

The status comparison runs before the address is reassigned, otherwise sameAddress would always be true.

Five merge tests now: same address keeps the earlier status, same address takes an explicit later one, a changed address drops the earlier status for email and for SMS, and a changed address still takes an explicit status.

One note, the address compare is exact. A merge from a@b.com to A@B.com reads as a change and clears the status rather than carrying it, which is the safe direction.

[fadi-george]

Changes look good.

We should retest:

1. The email or SMS subscription already exists in the app

Currently unsubscribed, enabled not set → stays unsubscribed
Currently unsubscribed, enabled: true → becomes subscribed
Currently unsubscribed, enabled: false → stays unsubscribed
Currently subscribed, enabled not set → stays subscribed
Currently subscribed, enabled: true → stays subscribed
Currently subscribed, enabled: false → becomes unsubscribed

2. The email or SMS subscription is being created inside the login method

enabled omitted → subscribed
enabled: true → subscribed
enabled: false → unsubscribed

[fadi-george]

Non-blocking: this matches email case-insensitively, but addSubscriptions (LoginProfileApplier.kt:35) still matches the grouped entry with ==. With addEmail("A@b.com") plus profile "a@b.com" and emailEnabled = false, both entries go out (one true, one false) and then this marks A@b.com's local row unsubscribed even though the wire sent true for it. Could addSubscriptions use the same ignoreCase rule for email?

[abdulraqeeb33]

Retested all nine on 1f5d1fde4, on device through the SDK this time rather than the API, using the demo's new status inputs. Build onesignal/android/051101, app 77e32082.

I paired each case with the matching SMS case, so this covers 18 outcomes, nine per channel. That also closes the "SMS never exercised on device" gap.

1. The address already exists in the app

Seeded before each login, then transferred in by composite login. Verified the seed users end up holding nothing, so these really were existing-address cases and not new ones.

#	Flag	Before	Sent	After
1.1	unset	false / -2	nothing	false / -2	✅ stays unsubscribed
1.2	true	false / -2	enabled: true, 1	true / 1	✅ becomes subscribed
1.3	false	false / -2	enabled: false, -2	false / -2	✅ stays unsubscribed
1.4	unset	true / 1	nothing	true / 1	✅ stays subscribed
1.5	true	true / 1	enabled: true, 1	true / 1	✅ stays subscribed
1.6	false	true / 1	enabled: false, -2	false / -2	✅ becomes unsubscribed

2. The address is created inside the login method

#	Flag	Sent	After
2.1	unset	nothing	true / -99	✅ subscribed
2.2	true	enabled: true, 1	true / 1	✅ subscribed
2.3	false	enabled: false, -2	false / -2	✅ unsubscribed

Every row is identical for email and SMS.

One thing to flag on 2.1. The end state is subscribed as specified, but notification_types is -99 rather than 1, because nothing is sent and the backend defaults a new address to "never asked". Perseus counts -99 and 1 differently, and 1 is what emits optin.first_accepts. That is the turbine default change Nan raised, not something the SDK can fix by sending a status it was not given.

External IDs are dev11 through dev16 and dev21 through dev23 if you want to look at any of them.

[fadi-george]

Thanks I also ran a test matrix on an emulator against this build (1f5d1fd), driving the Login With Profile dialog with Maestro. For each row I checked the email/SMS entry in the POST /users payload from logcat and the stored state from the API.

• Email: all 9 rows (1.1 to 1.6 with an unsubscribed -99 seed, 2.1 to 2.3) passed. Existing addresses kept their subscription id.
• SMS: same 9 rows, all passed.
• Combo: email true + SMS false in one login, and subscribed email false + unsubscribed SMS unset. Each status stayed on its own channel, and the unset SMS sent no status and stayed -99.