diff --git a/OneSignalSDK/onesignal/core/api/core.api b/OneSignalSDK/onesignal/core/api/core.api index 0a26d1da1..ed1e590f4 100644 --- a/OneSignalSDK/onesignal/core/api/core.api +++ b/OneSignalSDK/onesignal/core/api/core.api @@ -194,6 +194,7 @@ public final class com/onesignal/OneSignalUserProfile { public fun equals (Ljava/lang/Object;)Z public final fun getAliases ()Ljava/util/Map; public final fun getEmail ()Ljava/lang/String; + public final fun getEmailEnabled ()Ljava/lang/Boolean; public final fun getSms ()Ljava/lang/String; public final fun getTags ()Ljava/util/Map; public fun hashCode ()I @@ -205,6 +206,7 @@ public final class com/onesignal/OneSignalUserProfile$Builder { public final fun build ()Lcom/onesignal/OneSignalUserProfile; public final fun setAliases (Ljava/util/Map;)Lcom/onesignal/OneSignalUserProfile$Builder; public final fun setEmail (Ljava/lang/String;)Lcom/onesignal/OneSignalUserProfile$Builder; + public final fun setEmailEnabled (Z)Lcom/onesignal/OneSignalUserProfile$Builder; public final fun setSms (Ljava/lang/String;)Lcom/onesignal/OneSignalUserProfile$Builder; public final fun setTags (Ljava/util/Map;)Lcom/onesignal/OneSignalUserProfile$Builder; } diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/OneSignalUserProfile.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/OneSignalUserProfile.kt index f35b058a7..f773f9892 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/OneSignalUserProfile.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/OneSignalUserProfile.kt @@ -12,6 +12,9 @@ import java.util.Collections class OneSignalUserProfile private constructor(builder: Builder) { val email: String? = builder.email?.takeIf { it.isNotBlank() } val sms: String? = builder.sms?.takeIf { it.isNotBlank() } + + /** False stores the email unsubscribed. True is not sent. Null omits status. */ + val emailEnabled: Boolean? = builder.emailEnabled val tags: Map = Collections.unmodifiableMap(builder.tags.toMap()) val aliases: Map = Collections.unmodifiableMap(builder.aliases.toMap()) @@ -39,6 +42,7 @@ class OneSignalUserProfile private constructor(builder: Builder) { if (other !is OneSignalUserProfile) return false return email == other.email && sms == other.sms && + emailEnabled == other.emailEnabled && tags == other.tags && aliases == other.aliases } @@ -46,6 +50,7 @@ class OneSignalUserProfile private constructor(builder: Builder) { override fun hashCode(): Int { var result = email?.hashCode() ?: 0 result = 31 * result + (sms?.hashCode() ?: 0) + result = 31 * result + (emailEnabled?.hashCode() ?: 0) result = 31 * result + tags.hashCode() result = 31 * result + aliases.hashCode() return result @@ -60,6 +65,8 @@ class OneSignalUserProfile private constructor(builder: Builder) { private set internal var sms: String? = null private set + internal var emailEnabled: Boolean? = null + private set internal var tags: Map = emptyMap() private set internal var aliases: Map = emptyMap() @@ -68,6 +75,9 @@ class OneSignalUserProfile private constructor(builder: Builder) { /** Email to create or transfer onto this user at login. */ fun setEmail(email: String?) = apply { this.email = email } + /** Store the email unsubscribed when false. True is not sent on Create User. */ + fun setEmailEnabled(enabled: Boolean) = apply { this.emailEnabled = enabled } + /** SMS to create or transfer onto this user at login. Must be E.164 (e.g. +14155552671). */ fun setSms(sms: String?) = apply { this.sms = sms } @@ -92,7 +102,10 @@ class OneSignalUserProfile private constructor(builder: Builder) { sms: String? = null, tags: Map = emptyMap(), aliases: Map = emptyMap(), - ) = Builder().setEmail(email).setSms(sms).setTags(tags).setAliases(aliases).build() + emailEnabled: Boolean? = null, + ) = Builder().setEmail(email).setSms(sms).setTags(tags).setAliases(aliases).apply { + if (emailEnabled != null) setEmailEnabled(emailEnabled) + }.build() private fun redact(value: String?): String = if (value == null) "null" else "" } diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/LoginUserOperation.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/LoginUserOperation.kt index 08822cc9f..a2086d0a2 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/LoginUserOperation.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/LoginUserOperation.kt @@ -61,6 +61,12 @@ class LoginUserOperation() : Operation(LoginUserOperationExecutor.LOGIN_USER) { setOptStringProperty(PROFILE_SMS, value) } + internal var emailEnabled: Boolean? + get() = getOptBooleanProperty(PROFILE_EMAIL_ENABLED) + set(value) { + setOptBooleanProperty(PROFILE_EMAIL_ENABLED, value) + } + internal var tags: Map get() = decodeMap(getOptStringProperty(PROFILE_TAGS)) set(value) { @@ -93,6 +99,7 @@ class LoginUserOperation() : Operation(LoginUserOperationExecutor.LOGIN_USER) { if (profile != null) { this.email = profile.email?.takeIf { it.isNotBlank() } this.sms = profile.sms?.takeIf { it.isNotBlank() } + this.emailEnabled = profile.emailEnabled this.tags = profile.tags this.aliases = profile.aliases } @@ -102,7 +109,10 @@ class LoginUserOperation() : Operation(LoginUserOperationExecutor.LOGIN_USER) { !email.isNullOrBlank() || !sms.isNullOrBlank() || tags.isNotEmpty() || aliases.isNotEmpty() internal fun mergeProfileFrom(other: LoginUserOperation) { - if (!other.email.isNullOrBlank()) email = other.email + if (!other.email.isNullOrBlank()) { + email = other.email + emailEnabled = other.emailEnabled + } if (!other.sms.isNullOrBlank()) sms = other.sms if (other.tags.isNotEmpty()) tags = tags + other.tags if (other.aliases.isNotEmpty()) aliases = aliases + other.aliases @@ -128,6 +138,7 @@ internal fun reservedLoginAliasLabel(label: String): Boolean = label.isBlank() || label == IdentityConstants.ONESIGNAL_ID || label == IdentityConstants.EXTERNAL_ID private const val PROFILE_EMAIL = "profileEmail" +private const val PROFILE_EMAIL_ENABLED = "profileEmailEnabled" private const val PROFILE_SMS = "profileSms" private const val PROFILE_TAGS = "profileTags" private const val PROFILE_ALIASES = "profileAliases" diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/LoginProfileApplier.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/LoginProfileApplier.kt index e668ba6ff..db68bdec9 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/LoginProfileApplier.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/LoginProfileApplier.kt @@ -24,15 +24,16 @@ internal object LoginProfileApplier { fun addSubscriptions( op: LoginUserOperation, subscriptions: Map, + buildProfileSubscription: (SubscriptionObjectType, String) -> SubscriptionObject, ): Map { val mutable = subscriptions.toMutableMap() val email = op.email?.takeIf { it.isNotBlank() } if (email != null && mutable.values.none { it.type == SubscriptionObjectType.EMAIL && it.token == email }) { - mutable[PROFILE_EMAIL_KEY] = SubscriptionObject(type = SubscriptionObjectType.EMAIL, token = email) + mutable[PROFILE_EMAIL_KEY] = buildProfileSubscription(SubscriptionObjectType.EMAIL, email) } val sms = op.sms?.takeIf { it.isNotBlank() } if (sms != null && mutable.values.none { it.type == SubscriptionObjectType.SMS && it.token == sms }) { - mutable[PROFILE_SMS_KEY] = SubscriptionObject(type = SubscriptionObjectType.SMS, token = sms) + mutable[PROFILE_SMS_KEY] = buildProfileSubscription(SubscriptionObjectType.SMS, sms) } return mutable } @@ -73,6 +74,7 @@ internal object LoginProfileApplier { backend: SubscriptionObject, fallbackToken: String?, subscriptionsModelStore: SubscriptionModelStore, + optedIn: Boolean = true, ) { val id = backend.id val token = backend.token ?: fallbackToken @@ -93,8 +95,8 @@ internal object LoginProfileApplier { model.id = id model.type = type model.address = token - model.status = SubscriptionStatus.SUBSCRIBED - model.optedIn = true + model.status = if (optedIn) SubscriptionStatus.SUBSCRIBED else SubscriptionStatus.UNSUBSCRIBE + model.optedIn = optedIn subscriptionsModelStore.add(model, ModelChangeTags.HYDRATE) } diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/LoginUserOperationExecutor.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/LoginUserOperationExecutor.kt index 200baaf3d..a325fd2a5 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/LoginUserOperationExecutor.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/operations/impl/executors/LoginUserOperationExecutor.kt @@ -44,8 +44,10 @@ import com.onesignal.user.internal.properties.PropertiesModel import com.onesignal.user.internal.properties.PropertiesModelStore import com.onesignal.user.internal.subscriptions.SubscriptionModel import com.onesignal.user.internal.subscriptions.SubscriptionModelStore +import com.onesignal.user.internal.subscriptions.SubscriptionStatus import com.onesignal.user.internal.subscriptions.SubscriptionType +@Suppress("TooManyFunctions") internal class LoginUserOperationExecutor( private val _identityOperationExecutor: IdentityOperationExecutor, private val _application: IApplicationService, @@ -208,7 +210,10 @@ internal class LoginUserOperationExecutor( else -> throw Exception("Unrecognized operation: $operation") } } - subscriptions = LoginProfileApplier.addSubscriptions(createUserOperation, subscriptions) + subscriptions = + LoginProfileApplier.addSubscriptions(createUserOperation, subscriptions) { type, token -> + profileSubscription(type, token, createUserOperation.emailEnabled) + } try { val subscriptionList = subscriptions.toList() @@ -259,6 +264,7 @@ internal class LoginUserOperationExecutor( backendSubscription, LoginProfileApplier.fallbackToken(backendSubscription.type, createUserOperation), _subscriptionsModelStore, + optedIn = !emailStoredUnsubscribed(backendSubscription.type, createUserOperation), ) } when (LoginProfileApplier.subscriptionType(backendSubscription.type)) { @@ -293,14 +299,6 @@ internal class LoginUserOperationExecutor( Logging.warn("LoginUserOperationExecutor: skipped profile hydration because the current identity is not the created user") } - if (!_identityVerificationService.ivBehaviorActive && - (!createUserOperation.email.isNullOrBlank() || !createUserOperation.sms.isNullOrBlank()) - ) { - Logging.warn( - "LoginUserOperationExecutor: email or SMS sent without identity verification. If that address belonged to another user it was transferred.", - ) - } - val wasPossiblyAnUpsert = identities.isNotEmpty() val followUpOperations = if (wasPossiblyAnUpsert) { @@ -377,24 +375,60 @@ internal class LoginUserOperationExecutor( } val subId = if (!IDManager.isLocalId(operation.subscriptionId)) operation.subscriptionId else null mutableSubscriptions[operation.subscriptionId] = - SubscriptionObject( - id = subId, + subscriptionWithDevice( subscriptionType, operation.address, operation.enabled, operation.status.value, - OneSignalUtils.sdkVersion, - Build.MODEL, - Build.VERSION.RELEASE, - RootToolsInternalMethods.isRooted, - DeviceUtils.getNetType(_application.appContext), - DeviceUtils.getCarrierName(_application.appContext), - AndroidUtils.getAppVersion(_application.appContext), + subId, ) return mutableSubscriptions } + // Create User applies a subscribed status to an address that already opted out. + // Send enabled false only when the customer asked, so they can collect the email + // without marketing consent. Anything else stays omitted. + private fun profileSubscription( + type: SubscriptionObjectType, + token: String, + emailEnabled: Boolean?, + ): SubscriptionObject { + val unsubscribed = type == SubscriptionObjectType.EMAIL && emailEnabled == false + return subscriptionWithDevice( + type, + token, + enabled = if (unsubscribed) false else null, + notificationTypes = if (unsubscribed) SubscriptionStatus.UNSUBSCRIBE.value else null, + ) + } + + private fun emailStoredUnsubscribed( + type: SubscriptionObjectType?, + operation: LoginUserOperation, + ): Boolean = LoginProfileApplier.subscriptionType(type) == SubscriptionType.EMAIL && operation.emailEnabled == false + + private fun subscriptionWithDevice( + type: SubscriptionObjectType, + token: String, + enabled: Boolean? = null, + notificationTypes: Int? = null, + id: String? = null, + ) = SubscriptionObject( + id = id, + type = type, + token = token, + enabled = enabled, + notificationTypes = notificationTypes, + sdk = OneSignalUtils.sdkVersion, + deviceModel = Build.MODEL, + deviceOS = Build.VERSION.RELEASE, + rooted = RootToolsInternalMethods.isRooted, + netType = DeviceUtils.getNetType(_application.appContext), + carrier = DeviceUtils.getCarrierName(_application.appContext), + appVersion = AndroidUtils.getAppVersion(_application.appContext), + ) + private fun createSubscriptionsFromOperation( operation: UpdateSubscriptionOperation, subscriptions: Map, diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/OneSignalUserProfileTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/OneSignalUserProfileTests.kt index 9c6805f10..271976832 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/OneSignalUserProfileTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/OneSignalUserProfileTests.kt @@ -12,11 +12,36 @@ class OneSignalUserProfileTests : FunSpec({ profile.email.shouldBeNull() profile.sms.shouldBeNull() + profile.emailEnabled.shouldBeNull() profile.tags shouldBe emptyMap() profile.aliases shouldBe emptyMap() profile.hasFields shouldBe false } + test("emailEnabled false is kept and true is distinct from unset") { + val unsubscribed = OneSignalUserProfile(email = "bob@example.com", emailEnabled = false) + val subscribed = OneSignalUserProfile(email = "bob@example.com", emailEnabled = true) + val unset = OneSignalUserProfile(email = "bob@example.com") + + unsubscribed.emailEnabled shouldBe false + subscribed.emailEnabled shouldBe true + unset.emailEnabled.shouldBeNull() + unsubscribed shouldBe OneSignalUserProfile(email = "bob@example.com", emailEnabled = false) + (unsubscribed == unset) shouldBe false + (unsubscribed == subscribed) shouldBe false + } + + test("Java builder can store an email unsubscribed") { + val profile = + OneSignalUserProfile.builder() + .setEmail("bob@example.com") + .setEmailEnabled(false) + .build() + + profile.email shouldBe "bob@example.com" + profile.emailEnabled shouldBe false + } + test("Kotlin named args set only the fields that were passed") { val profile = OneSignalUserProfile( diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/operations/LoginUserOperationExecutorTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/operations/LoginUserOperationExecutorTests.kt index 908c39bc4..63bf4ed71 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/operations/LoginUserOperationExecutorTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/operations/LoginUserOperationExecutorTests.kt @@ -1,8 +1,14 @@ package com.onesignal.user.internal.operations +import android.os.Build +import androidx.test.core.app.ApplicationProvider import br.com.colman.kotest.android.extensions.robolectric.RobolectricTest import com.onesignal.OneSignalUserProfile +import com.onesignal.common.AndroidUtils +import com.onesignal.common.DeviceUtils +import com.onesignal.common.OneSignalUtils import com.onesignal.common.PIIHasher +import com.onesignal.common.RootToolsInternalMethods import com.onesignal.common.exceptions.BackendException import com.onesignal.core.internal.operations.ExecutionResponse import com.onesignal.core.internal.operations.ExecutionResult @@ -33,11 +39,13 @@ import com.onesignal.user.internal.subscriptions.SubscriptionType import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.collections.shouldBeOneOf import io.kotest.matchers.shouldBe +import io.kotest.matchers.shouldNotBe import io.kotest.matchers.types.shouldBeInstanceOf import io.mockk.coEvery import io.mockk.coVerify import io.mockk.every import io.mockk.mockk +import io.mockk.slot @RobolectricTest class LoginUserOperationExecutorTests : FunSpec({ @@ -1076,6 +1084,32 @@ class LoginUserOperationExecutorTests : FunSpec({ mockk(relaxed = true), ) + fun assertProfileSubscription( + sub: SubscriptionObject, + token: String, + enabled: Boolean? = null, + notificationTypes: Int? = null, + ) { + val context = ApplicationProvider.getApplicationContext() + sub.token shouldBe token + sub.enabled shouldBe enabled + sub.notificationTypes shouldBe notificationTypes + sub.sdk shouldNotBe null + sub.sdk shouldBe OneSignalUtils.sdkVersion + sub.deviceModel shouldNotBe null + sub.deviceModel shouldBe Build.MODEL + sub.deviceOS shouldNotBe null + sub.deviceOS shouldBe Build.VERSION.RELEASE + sub.rooted shouldNotBe null + sub.rooted shouldBe RootToolsInternalMethods.isRooted + sub.netType shouldNotBe null + sub.netType shouldBe DeviceUtils.getNetType(context) + // Robolectric has no carrier, so only equality is checked. + sub.carrier shouldBe DeviceUtils.getCarrierName(context) + sub.appVersion shouldNotBe null + sub.appVersion shouldBe AndroidUtils.getAppVersion(context) + } + test("matchingBackendSubscription matches by id even when another sub has the token") { val remaining = setOf( @@ -1204,17 +1238,81 @@ class LoginUserOperationExecutorTests : FunSpec({ response.result shouldBe ExecutionResult.SUCCESS response.metadata shouldBe LoginWaitMetadata(remoteOneSignalId, "email-sub") + val subs = slot>() coVerify(exactly = 1) { - mockUserBackendService.createUser( - appId, - mapOf(IdentityConstants.EXTERNAL_ID to "externalId"), - withArg { subs -> - subs.single { it.type == SubscriptionObjectType.EMAIL }.token shouldBe "a@b.com" - subs.none { it.type == SubscriptionObjectType.SMS } shouldBe true - }, - any(), + mockUserBackendService.createUser(appId, mapOf(IdentityConstants.EXTERNAL_ID to "externalId"), capture(subs), any()) + } + assertProfileSubscription(subs.captured.single { it.type == SubscriptionObjectType.EMAIL }, "a@b.com") + subs.captured.none { it.type == SubscriptionObjectType.SMS } shouldBe true + } + + test("composite login stores an email unsubscribed when enabled is false") { + val mockUserBackendService = mockk() + coEvery { mockUserBackendService.createUser(any(), any(), any(), any()) } returns + CreateUserResponse( + mapOf(IdentityConstants.ONESIGNAL_ID to remoteOneSignalId), + PropertiesObject(), + listOf(SubscriptionObject(id = "email-sub", type = SubscriptionObjectType.EMAIL, token = "a@b.com")), + ) + val identityStore = MockHelper.identityModelStore { it.onesignalId = localOneSignalId } + val subscriptions = SubscriptionModelStore(MockPreferencesService()) + + val response = + profileExecutor(mockUserBackendService, subscriptions, identityStore).execute( + listOf( + LoginUserOperation( + appId, + localOneSignalId, + "externalId", + null, + OneSignalUserProfile(email = "a@b.com", emailEnabled = false), + ), + ), + ) + + response.result shouldBe ExecutionResult.SUCCESS + val subs = slot>() + coVerify(exactly = 1) { + mockUserBackendService.createUser(appId, mapOf(IdentityConstants.EXTERNAL_ID to "externalId"), capture(subs), any()) + } + assertProfileSubscription( + subs.captured.single { it.type == SubscriptionObjectType.EMAIL }, + "a@b.com", + enabled = false, + notificationTypes = SubscriptionStatus.UNSUBSCRIBE.value, + ) + subscriptions.list().single { it.type == SubscriptionType.EMAIL }.let { + it.optedIn shouldBe false + it.status shouldBe SubscriptionStatus.UNSUBSCRIBE + } + } + + test("composite login omits email status when enabled is true") { + val mockUserBackendService = mockk() + coEvery { mockUserBackendService.createUser(any(), any(), any(), any()) } returns + CreateUserResponse( + mapOf(IdentityConstants.ONESIGNAL_ID to remoteOneSignalId), + PropertiesObject(), + listOf(SubscriptionObject(id = "email-sub", type = SubscriptionObjectType.EMAIL, token = "a@b.com")), ) + + profileExecutor(mockUserBackendService).execute( + listOf( + LoginUserOperation( + appId, + localOneSignalId, + "externalId", + null, + OneSignalUserProfile(email = "a@b.com", emailEnabled = true), + ), + ), + ) + + val subs = slot>() + coVerify(exactly = 1) { + mockUserBackendService.createUser(appId, mapOf(IdentityConstants.EXTERNAL_ID to "externalId"), capture(subs), any()) } + assertProfileSubscription(subs.captured.single { it.type == SubscriptionObjectType.EMAIL }, "a@b.com") } test("composite login with phone only sends an SMS subscription") { @@ -1232,17 +1330,12 @@ class LoginUserOperationExecutorTests : FunSpec({ ) response.result shouldBe ExecutionResult.SUCCESS + val subs = slot>() coVerify(exactly = 1) { - mockUserBackendService.createUser( - appId, - mapOf(IdentityConstants.EXTERNAL_ID to "externalId"), - withArg { subs -> - subs.single { it.type == SubscriptionObjectType.SMS }.token shouldBe "+15555550100" - subs.none { it.type == SubscriptionObjectType.EMAIL } shouldBe true - }, - any(), - ) + mockUserBackendService.createUser(appId, mapOf(IdentityConstants.EXTERNAL_ID to "externalId"), capture(subs), any()) } + assertProfileSubscription(subs.captured.single { it.type == SubscriptionObjectType.SMS }, "+15555550100") + subs.captured.none { it.type == SubscriptionObjectType.EMAIL } shouldBe true } test("composite login with tags only forwards them on createUser") { @@ -1584,6 +1677,7 @@ class LoginUserOperationExecutorTests : FunSpec({ sms = "+15555550100", tags = mapOf("plan" to "pro"), aliases = mapOf("facebook" to "bob"), + emailEnabled = false, ), ) @@ -1591,6 +1685,7 @@ class LoginUserOperationExecutorTests : FunSpec({ restored.initializeFromJson(original.toJSON()) restored.email shouldBe "a@b.com" + restored.emailEnabled shouldBe false restored.sms shouldBe "+15555550100" restored.tags shouldBe mapOf("plan" to "pro") restored.aliases shouldBe mapOf("facebook" to "bob")