Fix: update timezone onSessionStart instead

Author: jinliu9508

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/session/internal/session/impl/SessionListener.kt

@@ -1,5 +1,6 @@
 package com.onesignal.session.internal.session.impl
 
+import com.onesignal.common.TimeUtils
 import com.onesignal.common.threading.suspendifyOnIO
 import com.onesignal.core.internal.config.ConfigModelStore
 import com.onesignal.core.internal.operations.IOperationRepo
@@ -11,6 +12,7 @@ import com.onesignal.session.internal.session.ISessionService
 import com.onesignal.user.internal.identity.IdentityModelStore
 import com.onesignal.user.internal.operations.TrackSessionEndOperation
 import com.onesignal.user.internal.operations.TrackSessionStartOperation
+import com.onesignal.user.internal.properties.PropertiesModelStore
 
 /**
  * The [SessionListener] is responsible for subscribing itself as an [ISessionLifecycleHandler]
@@ -33,17 +35,20 @@ internal class SessionListener(
     private val _sessionService: ISessionService,
     private val _configModelStore: ConfigModelStore,
     private val _identityModelStore: IdentityModelStore,
+    private val _propertiesModelStore: PropertiesModelStore,
     private val _outcomeEventsController: IOutcomeEventsController,
 ) : IStartableService, ISessionLifecycleHandler {
     override fun start() {
         _sessionService.subscribe(this)
     }
 
     override fun onSessionStarted() {
+        _propertiesModelStore.model.timezone = TimeUtils.getTimeZoneId()
         _operationRepo.enqueue(TrackSessionStartOperation(_configModelStore.model.appId, _identityModelStore.model.onesignalId), true)
     }
 
     override fun onSessionActive() {
+        _propertiesModelStore.model.timezone = TimeUtils.getTimeZoneId()
     }
 
     override fun onSessionEnded(duration: Long) {

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/UserManager.kt

@@ -2,7 +2,6 @@ package com.onesignal.user.internal
 
 import com.onesignal.common.IDManager
 import com.onesignal.common.OneSignalUtils
-import com.onesignal.common.TimeUtils
 import com.onesignal.common.events.EventProducer
 import com.onesignal.common.modeling.ISingletonModelStoreChangeHandler
 import com.onesignal.common.modeling.ModelChangedArgs
@@ -266,13 +265,7 @@ internal open class UserManager(
         }
     }
 
-    override fun onFocus(firedOnSubscribe: Boolean) {
-        // Detect any user properties updates that changed
-        // Skip updating timezone when onesignalId is not set in the property model
-        if (_propertiesModel.hasProperty(PropertiesModel::onesignalId.name)) {
-            _propertiesModel.timezone = TimeUtils.getTimeZoneId()
-        }
-    }
+    override fun onFocus(firedOnSubscribe: Boolean) { }
 
     override fun onUnfocused() { }
 }

OneSignalSDK/onesignal/core/src/test/java/com/onesignal/session/internal/session/impl/SessionListenerTests.kt

@@ -1,25 +1,38 @@
 package com.onesignal.session.internal.session.impl
 
+import com.onesignal.common.TimeUtils
 import com.onesignal.mocks.MockHelper
 import com.onesignal.user.internal.operations.TrackSessionStartOperation
 import io.kotest.core.spec.style.FunSpec
+import io.mockk.every
 import io.mockk.mockk
+import io.mockk.mockkObject
+import io.mockk.unmockkAll
 import io.mockk.verify
 
+private class Mocks {
+    val mockOperationRepo = mockk<com.onesignal.core.internal.operations.IOperationRepo>(relaxed = true)
+
+    val propertiesModelStore = MockHelper.propertiesModelStore()
+
+    val sessionListener =
+        SessionListener(
+            mockOperationRepo,
+            mockk<com.onesignal.session.internal.session.ISessionService>(relaxed = true),
+            MockHelper.configModelStore(),
+            MockHelper.identityModelStore(),
+            propertiesModelStore,
+            mockk<com.onesignal.session.internal.outcomes.IOutcomeEventsController>(relaxed = true),
+        )
+}
+
 class SessionListenerTests : FunSpec({
 
     test("onSessionStarted enqueues TrackSessionStartOperation") {
         // Given
-        val mockOperationRepo = mockk<com.onesignal.core.internal.operations.IOperationRepo>(relaxed = true)
-
-        val sessionListener =
-            SessionListener(
-                mockOperationRepo,
-                mockk<com.onesignal.session.internal.session.ISessionService>(relaxed = true),
-                MockHelper.configModelStore(),
-                MockHelper.identityModelStore(),
-                mockk<com.onesignal.session.internal.outcomes.IOutcomeEventsController>(relaxed = true),
-            )
+        val mocks = Mocks()
+        val mockOperationRepo = mocks.mockOperationRepo
+        val sessionListener = mocks.sessionListener
 
         // When
         sessionListener.onSessionStarted()
@@ -29,4 +42,47 @@ class SessionListenerTests : FunSpec({
             mockOperationRepo.enqueue(any<TrackSessionStartOperation>(), true)
         }
     }
+
+    test("onSessionStarted updates timezone") {
+        // Given
+        val mocks = Mocks()
+        val sessionListener = mocks.sessionListener
+        val propertiesModel = mocks.propertiesModelStore.model
+        var timezoneId = "1"
+        mockkObject(TimeUtils)
+        every { TimeUtils.getTimeZoneId() } returns timezoneId
+
+        // When
+        sessionListener.onSessionStarted()
+
+        // Then
+        propertiesModel.timezone = timezoneId
+
+        // Once timezone is changed, repetitive onSessionStarted call will update timezone
+        timezoneId = "2"
+        sessionListener.onSessionStarted()
+        propertiesModel.timezone = timezoneId
+
+        unmockkAll()
+    }
+
+    test("onSessionActive updates timezone") {
+        // Given
+        val mocks = Mocks()
+        val sessionListener = mocks.sessionListener
+        val propertiesModel = mocks.propertiesModelStore.model
+        var timezoneId = "1"
+        mockkObject(TimeUtils)
+        every { TimeUtils.getTimeZoneId() } returns timezoneId
+
+        // When
+        sessionListener.onSessionStarted()
+
+        // Once timezone is changed, onSessionActive will update timezone
+        timezoneId = "2"
+        sessionListener.onSessionActive()
+        propertiesModel.timezone = timezoneId
+
+        unmockkAll()
+    }
 })

OneSignalSDK/onesignal/core/src/test/java/com/onesignal/user/internal/UserManagerTests.kt

@@ -3,7 +3,6 @@ package com.onesignal.user.internal
 import com.onesignal.common.TimeUtils
 import com.onesignal.core.internal.language.ILanguageContext
 import com.onesignal.mocks.MockHelper
-import com.onesignal.user.internal.properties.PropertiesModel
 import com.onesignal.user.internal.subscriptions.ISubscriptionManager
 import com.onesignal.user.internal.subscriptions.SubscriptionList
 import io.kotest.core.spec.style.FunSpec
@@ -197,7 +196,7 @@ class UserManagerTests : FunSpec({
         verify(exactly = 1) { mockSubscriptionManager.removeSmsSubscription("+15558675309") }
     }
 
-    test("onFocus updates timezone when onesignalId exists in property model") {
+    test("onFocus updates timezone") {
         // Given
         val mockTimeZone = "Europe/Foo"
         mockkObject(TimeUtils)
@@ -228,36 +227,4 @@ class UserManagerTests : FunSpec({
             unmockkObject(TimeUtils)
         }
     }
-
-    test("onFocus does not update timezone when onesignalId is missing in property model") {
-        // Given
-        val mockTimeZone = "Europe/Foo"
-        mockkObject(TimeUtils)
-        every { TimeUtils.getTimeZoneId() } returns mockTimeZone
-
-        val mockPropertiesModelStore = MockHelper.propertiesModelStore()
-        every { mockPropertiesModelStore.model.hasProperty(PropertiesModel::onesignalId.name) } returns false
-
-        val userManager =
-            UserManager(
-                mockk<ISubscriptionManager>(),
-                MockHelper.identityModelStore(),
-                mockPropertiesModelStore,
-                MockHelper.languageContext(),
-                MockHelper.applicationService(),
-            )
-
-        val propertiesModel = mockPropertiesModelStore.model
-
-        try {
-            // When
-            userManager.onFocus(firedOnSubscribe = false)
-
-            // Then
-            verify(exactly = 0) { propertiesModel.timezone = mockTimeZone }
-        } finally {
-            // Clean up the mock
-            unmockkObject(TimeUtils)
-        }
-    }
 })