Merge pull request #1842 from OneSignal/fix/timezone_id_not_set

Fix timezone_id not being set
OneSignal · Jan 31, 2024 · 0d08052 · 0d08052
2 parents 20cd3bb + 43e4b90
commit 0d08052
Show file tree

Hide file tree

Showing 6 changed files with 46 additions and 35 deletions.
diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/TimeUtils.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/TimeUtils.kt
@@ -17,7 +17,7 @@ object TimeUtils {
         return offset / 1000
     }
 
-    fun getTimeZoneId(): String? {
+    fun getTimeZoneId(): String {
         return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
             ZoneId.systemDefault().id
         } else {

diff --git a/...K/onesignal/core/src/main/java/com/onesignal/user/internal/backend/IUserBackendService.kt b/...K/onesignal/core/src/main/java/com/onesignal/user/internal/backend/IUserBackendService.kt
@@ -14,10 +14,11 @@ interface IUserBackendService {
      * the application.
      * @param subscriptions The subscriptions that should also be created and associated with the user. If subscriptions are already owned by a different user
      * they will be transferred to this user.
+     * @param properties The properties for this user. For new users this should include the timezone_id property.
      *
      * @return The backend response
      */
-    suspend fun createUser(appId: String, identities: Map<String, String>, subscriptions: List<SubscriptionObject>): CreateUserResponse
+    suspend fun createUser(appId: String, identities: Map<String, String>, subscriptions: List<SubscriptionObject>, properties: Map<String, String>): CreateUserResponse
     // TODO: Change to send only the push subscription, optimally
 
     /**

diff --git a/...esignal/core/src/main/java/com/onesignal/user/internal/backend/impl/UserBackendService.kt b/...esignal/core/src/main/java/com/onesignal/user/internal/backend/impl/UserBackendService.kt
@@ -14,7 +14,7 @@ internal class UserBackendService(
     private val _httpClient: IHttpClient,
 ) : IUserBackendService {
 
-    override suspend fun createUser(appId: String, identities: Map<String, String>, subscriptions: List<SubscriptionObject>): CreateUserResponse {
+    override suspend fun createUser(appId: String, identities: Map<String, String>, subscriptions: List<SubscriptionObject>, properties: Map<String, String>): CreateUserResponse {
         val requestJSON = JSONObject()
 
         if (identities.isNotEmpty()) {
@@ -26,6 +26,10 @@ internal class UserBackendService(
                 .put("subscriptions", JSONConverter.convertToJSON(subscriptions))
         }
 
+        if (properties.isNotEmpty()) {
+            requestJSON.put("properties", JSONObject().putMap(properties))
+        }
+
         val response = _httpClient.post("apps/$appId/users", requestJSON)
 
         if (!response.isSuccess) {

diff --git a/.../java/com/onesignal/user/internal/operations/impl/executors/LoginUserOperationExecutor.kt b/.../java/com/onesignal/user/internal/operations/impl/executors/LoginUserOperationExecutor.kt
@@ -1,11 +1,7 @@
 package com.onesignal.user.internal.operations.impl.executors
 
 import android.os.Build
-import com.onesignal.common.AndroidUtils
-import com.onesignal.common.DeviceUtils
-import com.onesignal.common.NetworkUtils
-import com.onesignal.common.OneSignalUtils
-import com.onesignal.common.RootToolsInternalMethods
+import com.onesignal.common.*
 import com.onesignal.common.exceptions.BackendException
 import com.onesignal.common.modeling.ModelChangeTags
 import com.onesignal.core.internal.application.IApplicationService
@@ -16,10 +12,8 @@ import com.onesignal.core.internal.operations.ExecutionResult
 import com.onesignal.core.internal.operations.IOperationExecutor
 import com.onesignal.core.internal.operations.Operation
 import com.onesignal.debug.internal.logging.Logging
-import com.onesignal.user.internal.backend.IUserBackendService
+import com.onesignal.user.internal.backend.*
 import com.onesignal.user.internal.backend.IdentityConstants
-import com.onesignal.user.internal.backend.SubscriptionObject
-import com.onesignal.user.internal.backend.SubscriptionObjectType
 import com.onesignal.user.internal.identity.IdentityModelStore
 import com.onesignal.user.internal.operations.CreateSubscriptionOperation
 import com.onesignal.user.internal.operations.DeleteSubscriptionOperation
@@ -106,6 +100,8 @@ internal class LoginUserOperationExecutor(
     private suspend fun createUser(createUserOperation: LoginUserOperation, operations: List<Operation>): ExecutionResponse {
         var identities = mapOf<String, String>()
         var subscriptions = mapOf<String, SubscriptionObject>()
+        val properties = mutableMapOf<String, String>()
+        properties["timezone_id"] = TimeUtils.getTimeZoneId()
 
         if (createUserOperation.externalId != null) {
             val mutableIdentities = identities.toMutableMap()
@@ -125,7 +121,7 @@ internal class LoginUserOperationExecutor(
 
         try {
             val subscriptionList = subscriptions.toList()
-            val response = _userBackend.createUser(createUserOperation.appId, identities, subscriptionList.map { it.second })
+            val response = _userBackend.createUser(createUserOperation.appId, identities, subscriptionList.map { it.second }, properties)
             val idTranslations = mutableMapOf<String, String>()
             // Add the "local-to-backend" ID translation to the IdentifierTranslator for any operations that were
             // *not* executed but still reference the locally-generated IDs.

diff --git a/...esignal/core/src/test/java/com/onesignal/user/internal/backend/UserBackendServiceTests.kt b/...esignal/core/src/test/java/com/onesignal/user/internal/backend/UserBackendServiceTests.kt
@@ -28,11 +28,12 @@ class UserBackendServiceTests : FunSpec({
         coEvery { spyHttpClient.post(any(), any()) } returns HttpResponse(403, "FORBIDDEN")
         val userBackendService = UserBackendService(spyHttpClient)
         val identities = mapOf<String, String>()
+        val properties = mapOf<String, String>()
         val subscriptions = listOf<SubscriptionObject>()
 
         /* When */
         val exception = shouldThrowUnit<BackendException> {
-            userBackendService.createUser("appId", identities, subscriptions)
+            userBackendService.createUser("appId", identities, subscriptions, properties)
         }
 
         /* Then */
@@ -44,17 +45,19 @@ class UserBackendServiceTests : FunSpec({
         /* Given */
         val osId = "11111111-1111-1111-1111-111111111111"
         val spyHttpClient = mockk<IHttpClient>()
-        coEvery { spyHttpClient.post(any(), any()) } returns HttpResponse(202, "{identity:{onesignal_id: \"$osId\", aliasLabel1: \"aliasValue1\"}}")
+        coEvery { spyHttpClient.post(any(), any()) } returns HttpResponse(202, "{identity:{onesignal_id: \"$osId\", aliasLabel1: \"aliasValue1\"}, properties:{timezone_id: \"testTimeZone\"}}")
         val userBackendService = UserBackendService(spyHttpClient)
         val identities = mapOf("aliasLabel1" to "aliasValue1")
+        val properties = mapOf("timzone_id" to "testTimeZone")
         val subscriptions = listOf<SubscriptionObject>()
 
         /* When */
-        val response = userBackendService.createUser("appId", identities, subscriptions)
+        val response = userBackendService.createUser("appId", identities, subscriptions, properties)
 
         /* Then */
         response.identities["onesignal_id"] shouldBe osId
         response.identities["aliasLabel1"] shouldBe "aliasValue1"
+        response.properties.timezoneId shouldBe "testTimeZone"
         response.subscriptions.count() shouldBe 0
         coVerify {
             spyHttpClient.post(
@@ -63,7 +66,7 @@ class UserBackendServiceTests : FunSpec({
                     it.has("identity") shouldBe true
                     it.getJSONObject("identity").has("aliasLabel1") shouldBe true
                     it.getJSONObject("identity").getString("aliasLabel1") shouldBe "aliasValue1"
-                    it.has("properties") shouldBe false
+                    it.has("properties") shouldBe true
                     it.has("subscriptions") shouldBe false
                 },
             )
@@ -74,17 +77,19 @@ class UserBackendServiceTests : FunSpec({
         /* Given */
         val osId = "11111111-1111-1111-1111-111111111111"
         val spyHttpClient = mockk<IHttpClient>()
-        coEvery { spyHttpClient.post(any(), any()) } returns HttpResponse(202, "{identity:{onesignal_id: \"$osId\"}, subscriptions:[{id:\"subscriptionId1\", type:\"AndroidPush\"}]}")
+        coEvery { spyHttpClient.post(any(), any()) } returns HttpResponse(202, "{identity:{onesignal_id: \"$osId\"}, subscriptions:[{id:\"subscriptionId1\", type:\"AndroidPush\"}], properties:{timezone_id: \"testTimeZone\"}}")
         val userBackendService = UserBackendService(spyHttpClient)
         val identities = mapOf<String, String>()
         val subscriptions = mutableListOf<SubscriptionObject>()
+        val properties = mapOf("timzone_id" to "testTimeZone")
         subscriptions.add(SubscriptionObject("SHOULDNOTUSE", SubscriptionObjectType.ANDROID_PUSH))
 
         /* When */
-        val response = userBackendService.createUser("appId", identities, subscriptions)
+        val response = userBackendService.createUser("appId", identities, subscriptions, properties)
 
         /* Then */
         response.identities["onesignal_id"] shouldBe osId
+        response.properties.timezoneId shouldBe "testTimeZone"
         response.subscriptions.count() shouldBe 1
         response.subscriptions[0].id shouldBe "subscriptionId1"
         response.subscriptions[0].type shouldBe SubscriptionObjectType.ANDROID_PUSH
@@ -94,7 +99,7 @@ class UserBackendServiceTests : FunSpec({
                 "apps/appId/users",
                 withArg {
                     it.has("identity") shouldBe false
-                    it.has("properties") shouldBe false
+                    it.has("properties") shouldBe true
                     it.has("subscriptions") shouldBe true
                     it.getJSONArray("subscriptions").length() shouldBe 1
                     it.getJSONArray("subscriptions").getJSONObject(0).has("type") shouldBe true

diff --git a/...e/src/test/java/com/onesignal/user/internal/operations/LoginUserOperationExecutorTests.kt b/...e/src/test/java/com/onesignal/user/internal/operations/LoginUserOperationExecutorTests.kt
@@ -46,7 +46,7 @@ class LoginUserOperationExecutorTests : FunSpec({
     test("login anonymous user successfully creates user") {
         /* Given */
         val mockUserBackendService = mockk<IUserBackendService>()
-        coEvery { mockUserBackendService.createUser(any(), any(), any()) } returns
+        coEvery { mockUserBackendService.createUser(any(), any(), any(), any()) } returns
             CreateUserResponse(
                 mapOf(IdentityConstants.ONESIGNAL_ID to remoteOneSignalId),
                 PropertiesObject(),
@@ -80,14 +80,15 @@ class LoginUserOperationExecutorTests : FunSpec({
                 appId,
                 mapOf(),
                 any(),
+                any(),
             )
         }
     }
 
     test("login anonymous user fails with retry when network condition exists") {
         /* Given */
         val mockUserBackendService = mockk<IUserBackendService>()
-        coEvery { mockUserBackendService.createUser(any(), any(), any()) } throws BackendException(408, "TIMEOUT")
+        coEvery { mockUserBackendService.createUser(any(), any(), any(), any()) } throws BackendException(408, "TIMEOUT")
 
         val mockIdentityOperationExecutor = mockk<IdentityOperationExecutor>()
 
@@ -103,13 +104,13 @@ class LoginUserOperationExecutorTests : FunSpec({
 
         /* Then */
         response.result shouldBe ExecutionResult.FAIL_RETRY
-        coVerify(exactly = 1) { mockUserBackendService.createUser(appId, mapOf(), any()) }
+        coVerify(exactly = 1) { mockUserBackendService.createUser(appId, mapOf(), any(), any()) }
     }
 
     test("login anonymous user fails with no retry when backend error condition exists") {
         /* Given */
         val mockUserBackendService = mockk<IUserBackendService>()
-        coEvery { mockUserBackendService.createUser(any(), any(), any()) } throws BackendException(404, "NOT FOUND")
+        coEvery { mockUserBackendService.createUser(any(), any(), any(), any()) } throws BackendException(404, "NOT FOUND")
 
         val mockIdentityOperationExecutor = mockk<IdentityOperationExecutor>()
 
@@ -125,13 +126,13 @@ class LoginUserOperationExecutorTests : FunSpec({
 
         /* Then */
         response.result shouldBe ExecutionResult.FAIL_NORETRY
-        coVerify(exactly = 1) { mockUserBackendService.createUser(appId, mapOf(), any()) }
+        coVerify(exactly = 1) { mockUserBackendService.createUser(appId, mapOf(), any(), any()) }
     }
 
     test("login identified user without association successfully creates user") {
         /* Given */
         val mockUserBackendService = mockk<IUserBackendService>()
-        coEvery { mockUserBackendService.createUser(any(), any(), any()) } returns
+        coEvery { mockUserBackendService.createUser(any(), any(), any(), any()) } returns
             CreateUserResponse(mapOf(IdentityConstants.ONESIGNAL_ID to remoteOneSignalId), PropertiesObject(), listOf())
 
         val mockIdentityOperationExecutor = mockk<IdentityOperationExecutor>()
@@ -148,7 +149,7 @@ class LoginUserOperationExecutorTests : FunSpec({
 
         /* Then */
         response.result shouldBe ExecutionResult.SUCCESS
-        coVerify(exactly = 1) { mockUserBackendService.createUser(appId, mapOf(IdentityConstants.EXTERNAL_ID to "externalId"), any()) }
+        coVerify(exactly = 1) { mockUserBackendService.createUser(appId, mapOf(IdentityConstants.EXTERNAL_ID to "externalId"), any(), any()) }
     }
 
     test("login identified user with association succeeds when association is successful") {
@@ -187,7 +188,7 @@ class LoginUserOperationExecutorTests : FunSpec({
     test("login identified user with association fails with retry when association fails with retry") {
         /* Given */
         val mockUserBackendService = mockk<IUserBackendService>()
-        coEvery { mockUserBackendService.createUser(any(), any(), any()) } returns
+        coEvery { mockUserBackendService.createUser(any(), any(), any(), any()) } returns
             CreateUserResponse(mapOf(IdentityConstants.ONESIGNAL_ID to remoteOneSignalId), PropertiesObject(), listOf())
 
         val mockIdentityOperationExecutor = mockk<IdentityOperationExecutor>()
@@ -222,7 +223,7 @@ class LoginUserOperationExecutorTests : FunSpec({
     test("login identified user with association successfully creates user when association fails with no retry") {
         /* Given */
         val mockUserBackendService = mockk<IUserBackendService>()
-        coEvery { mockUserBackendService.createUser(any(), any(), any()) } returns
+        coEvery { mockUserBackendService.createUser(any(), any(), any(), any()) } returns
             CreateUserResponse(mapOf(IdentityConstants.ONESIGNAL_ID to remoteOneSignalId), PropertiesObject(), listOf())
 
         val mockIdentityOperationExecutor = mockk<IdentityOperationExecutor>()
@@ -252,13 +253,13 @@ class LoginUserOperationExecutorTests : FunSpec({
                 },
             )
         }
-        coVerify(exactly = 1) { mockUserBackendService.createUser(appId, mapOf(IdentityConstants.EXTERNAL_ID to "externalId"), any()) }
+        coVerify(exactly = 1) { mockUserBackendService.createUser(appId, mapOf(IdentityConstants.EXTERNAL_ID to "externalId"), any(), any()) }
     }
 
     test("login identified user with association fails with retry when association fails with no retry and network condition exists") {
         /* Given */
         val mockUserBackendService = mockk<IUserBackendService>()
-        coEvery { mockUserBackendService.createUser(any(), any(), any()) } throws BackendException(408, "TIMEOUT")
+        coEvery { mockUserBackendService.createUser(any(), any(), any(), any()) } throws BackendException(408, "TIMEOUT")
 
         val mockIdentityOperationExecutor = mockk<IdentityOperationExecutor>()
         coEvery { mockIdentityOperationExecutor.execute(any()) } returns ExecutionResponse(ExecutionResult.FAIL_NORETRY)
@@ -287,13 +288,13 @@ class LoginUserOperationExecutorTests : FunSpec({
                 },
             )
         }
-        coVerify(exactly = 1) { mockUserBackendService.createUser(appId, mapOf(IdentityConstants.EXTERNAL_ID to "externalId"), any()) }
+        coVerify(exactly = 1) { mockUserBackendService.createUser(appId, mapOf(IdentityConstants.EXTERNAL_ID to "externalId"), any(), any()) }
     }
 
     test("creating user will merge operations into one backend call") {
         /* Given */
         val mockUserBackendService = mockk<IUserBackendService>()
-        coEvery { mockUserBackendService.createUser(any(), any(), any()) } returns
+        coEvery { mockUserBackendService.createUser(any(), any(), any(), any()) } returns
             CreateUserResponse(
                 mapOf(IdentityConstants.ONESIGNAL_ID to remoteOneSignalId),
                 PropertiesObject(),
@@ -357,14 +358,15 @@ class LoginUserOperationExecutorTests : FunSpec({
                     it[0].token shouldBe "pushToken2"
                     it[0].notificationTypes shouldBe SubscriptionStatus.SUBSCRIBED
                 },
+                any(),
             )
         }
     }
 
     test("creating user will hydrate when the user hasn't changed") {
         /* Given */
         val mockUserBackendService = mockk<IUserBackendService>()
-        coEvery { mockUserBackendService.createUser(any(), any(), any()) } returns
+        coEvery { mockUserBackendService.createUser(any(), any(), any(), any()) } returns
             CreateUserResponse(
                 mapOf(IdentityConstants.ONESIGNAL_ID to remoteOneSignalId),
                 PropertiesObject(),
@@ -423,14 +425,15 @@ class LoginUserOperationExecutorTests : FunSpec({
                 appId,
                 mapOf(),
                 any(),
+                any(),
             )
         }
     }
 
     test("creating user will not hydrate when the user has changed") {
         /* Given */
         val mockUserBackendService = mockk<IUserBackendService>()
-        coEvery { mockUserBackendService.createUser(any(), any(), any()) } returns
+        coEvery { mockUserBackendService.createUser(any(), any(), any(), any()) } returns
             CreateUserResponse(
                 mapOf(IdentityConstants.ONESIGNAL_ID to remoteOneSignalId),
                 PropertiesObject(),
@@ -489,14 +492,15 @@ class LoginUserOperationExecutorTests : FunSpec({
                 appId,
                 mapOf(),
                 any(),
+                any(),
             )
         }
     }
 
     test("creating user will provide local to remote translations") {
         /* Given */
         val mockUserBackendService = mockk<IUserBackendService>()
-        coEvery { mockUserBackendService.createUser(any(), any(), any()) } returns
+        coEvery { mockUserBackendService.createUser(any(), any(), any(), any()) } returns
             CreateUserResponse(
                 mapOf(IdentityConstants.ONESIGNAL_ID to remoteOneSignalId),
                 PropertiesObject(),
@@ -536,6 +540,7 @@ class LoginUserOperationExecutorTests : FunSpec({
                 appId,
                 mapOf(),
                 any(),
+                any(),
             )
         }
     }