Skip to content

Commit e5ba296

Browse files
AR Abdul Azeezcursoragent
andcommitted
fix: [SDK-5606] omit subscription status on composite login email and SMS
Create User applies enabled/notification_types to an existing address, which would re-subscribe a user who unsubscribed. Device fields are still sent. Also moves the test assertions out of withArg so they don't call a mocked AndroidUtils during verification, and suppresses TooManyFunctions. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent c63f4fc commit e5ba296

2 files changed

Lines changed: 25 additions & 24 deletions

File tree

‎OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/LoginUserOperationExecutor.kt‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,9 @@ import com.onesignal.user.internal.properties.PropertiesModel
4444
import com.onesignal.user.internal.properties.PropertiesModelStore
4545
import com.onesignal.user.internal.subscriptions.SubscriptionModel
4646
import com.onesignal.user.internal.subscriptions.SubscriptionModelStore
47-
import com.onesignal.user.internal.subscriptions.SubscriptionStatus
4847
import com.onesignal.user.internal.subscriptions.SubscriptionType
4948

49+
@Suppress("TooManyFunctions")
5050
internal class LoginUserOperationExecutor(
5151
private val _identityOperationExecutor: IdentityOperationExecutor,
5252
private val _application: IApplicationService,
@@ -209,9 +209,11 @@ internal class LoginUserOperationExecutor(
209209
else -> throw Exception("Unrecognized operation: $operation")
210210
}
211211
}
212+
// No enabled/notification_types: Create User applies them to an address that already
213+
// exists, which would re-subscribe a user who unsubscribed.
212214
subscriptions =
213215
LoginProfileApplier.addSubscriptions(createUserOperation, subscriptions) { type, token ->
214-
subscriptionWithDevice(type, token, enabled = true, notificationTypes = SubscriptionStatus.SUBSCRIBED.value)
216+
subscriptionWithDevice(type, token)
215217
}
216218

217219
try {
@@ -387,8 +389,8 @@ internal class LoginUserOperationExecutor(
387389
private fun subscriptionWithDevice(
388390
type: SubscriptionObjectType,
389391
token: String,
390-
enabled: Boolean,
391-
notificationTypes: Int,
392+
enabled: Boolean? = null,
393+
notificationTypes: Int? = null,
392394
id: String? = null,
393395
) = SubscriptionObject(
394396
id = id,

‎OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/operations/LoginUserOperationExecutorTests.kt‎

Lines changed: 19 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,13 @@ import com.onesignal.user.internal.subscriptions.SubscriptionType
3939
import io.kotest.core.spec.style.FunSpec
4040
import io.kotest.matchers.collections.shouldBeOneOf
4141
import io.kotest.matchers.shouldBe
42+
import io.kotest.matchers.shouldNotBe
4243
import io.kotest.matchers.types.shouldBeInstanceOf
4344
import io.mockk.coEvery
4445
import io.mockk.coVerify
4546
import io.mockk.every
4647
import io.mockk.mockk
48+
import io.mockk.slot
4749

4850
@RobolectricTest
4951
class LoginUserOperationExecutorTests : FunSpec({
@@ -1088,14 +1090,21 @@ class LoginUserOperationExecutorTests : FunSpec({
10881090
) {
10891091
val context = ApplicationProvider.getApplicationContext<android.content.Context>()
10901092
sub.token shouldBe token
1091-
sub.enabled shouldBe true
1092-
sub.notificationTypes shouldBe SubscriptionStatus.SUBSCRIBED.value
1093+
sub.enabled shouldBe null
1094+
sub.notificationTypes shouldBe null
1095+
sub.sdk shouldNotBe null
10931096
sub.sdk shouldBe OneSignalUtils.sdkVersion
1097+
sub.deviceModel shouldNotBe null
10941098
sub.deviceModel shouldBe Build.MODEL
1099+
sub.deviceOS shouldNotBe null
10951100
sub.deviceOS shouldBe Build.VERSION.RELEASE
1101+
sub.rooted shouldNotBe null
10961102
sub.rooted shouldBe RootToolsInternalMethods.isRooted
1103+
sub.netType shouldNotBe null
10971104
sub.netType shouldBe DeviceUtils.getNetType(context)
1105+
// Robolectric has no carrier, so only equality is checked.
10981106
sub.carrier shouldBe DeviceUtils.getCarrierName(context)
1107+
sub.appVersion shouldNotBe null
10991108
sub.appVersion shouldBe AndroidUtils.getAppVersion(context)
11001109
}
11011110

@@ -1227,17 +1236,12 @@ class LoginUserOperationExecutorTests : FunSpec({
12271236

12281237
response.result shouldBe ExecutionResult.SUCCESS
12291238
response.metadata shouldBe LoginWaitMetadata(remoteOneSignalId, "email-sub")
1239+
val subs = slot<List<SubscriptionObject>>()
12301240
coVerify(exactly = 1) {
1231-
mockUserBackendService.createUser(
1232-
appId,
1233-
mapOf(IdentityConstants.EXTERNAL_ID to "externalId"),
1234-
withArg { subs ->
1235-
assertProfileSubscription(subs.single { it.type == SubscriptionObjectType.EMAIL }, "a@b.com")
1236-
subs.none { it.type == SubscriptionObjectType.SMS } shouldBe true
1237-
},
1238-
any(),
1239-
)
1241+
mockUserBackendService.createUser(appId, mapOf(IdentityConstants.EXTERNAL_ID to "externalId"), capture(subs), any())
12401242
}
1243+
assertProfileSubscription(subs.captured.single { it.type == SubscriptionObjectType.EMAIL }, "a@b.com")
1244+
subs.captured.none { it.type == SubscriptionObjectType.SMS } shouldBe true
12411245
}
12421246

12431247
test("composite login with phone only sends an SMS subscription") {
@@ -1255,17 +1259,12 @@ class LoginUserOperationExecutorTests : FunSpec({
12551259
)
12561260

12571261
response.result shouldBe ExecutionResult.SUCCESS
1262+
val subs = slot<List<SubscriptionObject>>()
12581263
coVerify(exactly = 1) {
1259-
mockUserBackendService.createUser(
1260-
appId,
1261-
mapOf(IdentityConstants.EXTERNAL_ID to "externalId"),
1262-
withArg { subs ->
1263-
assertProfileSubscription(subs.single { it.type == SubscriptionObjectType.SMS }, "+15555550100")
1264-
subs.none { it.type == SubscriptionObjectType.EMAIL } shouldBe true
1265-
},
1266-
any(),
1267-
)
1264+
mockUserBackendService.createUser(appId, mapOf(IdentityConstants.EXTERNAL_ID to "externalId"), capture(subs), any())
12681265
}
1266+
assertProfileSubscription(subs.captured.single { it.type == SubscriptionObjectType.SMS }, "+15555550100")
1267+
subs.captured.none { it.type == SubscriptionObjectType.EMAIL } shouldBe true
12691268
}
12701269

12711270
test("composite login with tags only forwards them on createUser") {

0 commit comments

Comments
 (0)