Feat: Detect for timezone changes and update the user #2378

nan-li · 2025-10-10T00:10:02Z

Description

One Line Summary

Start detecting timezone changes and sending it up when it changes.

Details

Unless users switch via logging in and out, we never update timezone (as it is only sent in the Create User request).
Behavior aligns with Detect for timezone changes and update the user OneSignal-iOS-SDK#1542

Motivation

This was a request of clients so that when users enter new timezones, they can be properly segmented without needing a Create User call to happen.

Scope

No longer hydrate timezone from remote (this aligns with existing iOS behavior)
When timezone changes on the device, the new one will be sent
We don't need to address the migration and legacy player issue that had to be addressed by iOS: Detect for timezone changes and update the user OneSignal-iOS-SDK#1542. These PR changes automatically address them because missing timezone cases will get the timezone sent on the next session (since locally it is null which will trigger a change detection).

Testing

Unit testing

Added

Manual testing

Change timezone on emulator and see the next session, that timezone is sent with the track session start call.

Affected code checklist

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

This change is

Tack onto existing SessionListener's TrackSessionStartOperation call so this update would be combined. Otherwise, because the TrackSessionStartOperation has flush = true, the timing may mean 2 user update requests are made when it could be combined. Cons: the Session Listener is encroaching beyond what its scope should be.

After the implementation to start updating timezone changes on new sessions, hydrating timezone from the user refresh call will conflict if the timing between the update and refresh requests is not perfect. For example, imagine this scenario if we still hydrate this property: 1. On a new session, timezone detects as changed from US to UK. A property update is enqueued. 2. This request is sent and the refresh user call returns (which still contains timezone as US) 3. The SDK hydrates timezone as US 4. On the next session, timezone is detected as changed from US to UK.

abdulraqeeb33

Must write unit tests

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

abdulraqeeb33 · 2025-10-15T18:16:54Z

...n/java/com/onesignal/user/internal/operations/impl/executors/RefreshUserOperationExecutor.kt

-            if (response.properties.timezoneId != null) {
-                propertiesModel.timezone = response.properties.timezoneId
-            }
+            // No longer hydrate timezone from remote, set locally


same with this.

abdulraqeeb33 · 2025-10-15T18:18:16Z

...alSDK/onesignal/core/src/main/java/com/onesignal/user/internal/properties/PropertiesModel.kt

    }
+
+    fun update() {
+        timezone = TimeUtils.getTimeZoneId()


does this TimeUtils.getTimeZoneId() match with what the backend really expects?
Can we write a test to make sure that?

Yeah, it does. Currently, the only place the SDK ever sends timezone is in a create user call here:

// the non-null assert is actually extraneous properties["timezone_id"] = TimeUtils.getTimeZoneId()!!

does this TimeUtils.getTimeZoneId() match with what the backend really expects? Can we write a test to make sure that?

I don't know how we could write a test for what the server expects. We could write a test that ensures timezone is always sent as a certain format, but if the backend ever changed its expectation, we wouldn't know.

You are right, we cant test server expectations directly. But we should write tests to catch internal breaking changes.

I think the model classes should be very simple and all setting should done outside of it. So we shouldn't add a helper like this that is set a specific implementation of getting timezone.

abdulraqeeb33 · 2025-10-15T18:20:25Z

...n/java/com/onesignal/user/internal/operations/impl/executors/RefreshUserOperationExecutor.kt

-                propertiesModel.timezone = response.properties.timezoneId
-            }
+            // No longer hydrate timezone from remote, set locally
+            propertiesModel.update()


Should this be done on a background thread?

Same comment as before, propertiesModel.update() just sets propertiesModel.timezone = TimeUtils.getTimeZoneId(). If we look at the lines above this, we are setting various properties on the propertiesModel like:

propertiesModel.country = response.properties.country propertiesModel.tags[tagKVP.key] = tagKVP.value!! propertiesModel.language = response.properties.language

Setting timezone shouldn't run separately while this object is being set up. Also, during this time while the model is being set up, no updates will fire an operation to enqueue.

sounds good around the background thread. but we definitely need to write a test if the properties.update() is called and if the timezone is actually set.
this helps in possible regression defects and helps us gain confidence when we are making changes.

nan-li · 2025-10-15T19:12:53Z

Must write unit tests

Yeah, I mentioned in PR that I will add if implementation is fine. I put the logic in the SessionListener, but for true separation of concerns, it should probably be in UserManager or something similar. However, putting in SessionListener can batch this operation with the track session start operation so we don't send 2 requests. But I'll make sure to write tests for the cases you pointed out.

jkasten2 · 2025-10-15T22:05:33Z

...alSDK/onesignal/core/src/main/java/com/onesignal/user/internal/properties/PropertiesModel.kt

    }
+
+    fun update() {
+        timezone = TimeUtils.getTimeZoneId()


I think the model classes should be very simple and all setting should done outside of it. So we shouldn't add a helper like this that is set a specific implementation of getting timezone.

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

abdulraqeeb33 · 2025-10-16T15:11:08Z

#2378 (comment)

that makes sense to me as well @jkasten2 . Though I wonder if timezone would really changes between every app session. And i am guessing you would prefer to be more careful so better to do it onFocus?

Would it be more optimized if we update the timezone in the properties only if different from the current one. That way we dont update everytime on onFocus.

jkasten2 · 2025-10-16T18:35:04Z

#2378 (comment)

that makes sense to me as well @jkasten2 . Though I wonder if timezone would really changes between every app session. And i am guessing you would prefer to be more careful so better to do it onFocus?

I suggested using onFocus as this would result in picking up changes more often than a new session and decouples it from any session logic that could change in the future. Not concern about being careful of checking too often.

The best option would be if our SDK listens to timezone changes from the system (Android APIs) and can send that to our server. That way we always have the latest correct value without the user needing to open the app. Example code of how that can be done:
https://stackoverflow.com/questions/31398324/is-there-an-android-event-listener-for-change-in-the-timezone-of-the-device

However it's pretty complex to setup the Android APIs. Using something like onFocus gets us 90% of the benefits, is something that we can be happy with long term, and matches closer to iOS's behavior.

If we really want to update timezone as soon as it changes maybe we can see if iOS supports this as well.

Would it be more optimized if we update the timezone in the properties only if different from the current one. That way we dont update everytime on onFocus.

I believe our models already takes care of not doing anything if the value didn't change.

Let the UserManager refresh the changes to properties model, instead of SessionListener. The SubscriptionManager already refreshes the push subscription.

nan-li · 2025-10-20T17:10:01Z

@jkasten2 @abdulraqeeb33 - I pushed a commit to have UserManager instead detect changes in onFocus.
By calling propertiesModel.timezone = TimeUtils.getTimeZoneId() within onFocus, this always happens before onSessionStarted so any timezone updates get enqueued before TrackSessionStart in SessionListener. The issue prior was that TrackSessionStart had flush=true so it does not wait 5 seconds on a new session for other updates.

AR started adding tests in #2382 🙏

abdulraqeeb33

thank you creating a new pr for tests

abdulraqeeb33 · 2025-10-16T20:56:48Z

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

+
+    override fun onFocus(firedOnSubscribe: Boolean) {
+        // Detect any user properties updates that changed
+        _propertiesModel.timezone = TimeUtils.getTimeZoneId()


do you want to pass TimeUtils into this class? so that you can mock it?

also please make sure to write tests

abdulraqeeb33

As the tests are in the other PR

nan-li · 2025-10-20T18:49:59Z

As the tests are in the other PR

Let's not merge this until then, we can pick those commits over when done

nan-li · 2025-10-23T22:36:18Z

Hi @abdulraqeeb33 I used the changes you made in #2382 and modified to add the tests here. Let me know if a4a6d5e looks good to you.

* Add UserManagerTests that onFocus will update timezone * Update an existing test in RefreshUserOperationExecutorTests that tests a successful getUser request and hydration already. Let's add on the timezone component to this hydration, which is that timezone is no longer hydrated from the server but set locally. * Add TimeUtilsTest class testing getTimeZoneId

It's already non-null

nan-li force-pushed the feat/update_timezone branch 2 times, most recently from dbb8bde to f5dc3c9 Compare October 10, 2025 00:29

nan-li force-pushed the feat/update_timezone branch from f5dc3c9 to 6018f4d Compare October 10, 2025 00:30

nan-li requested review from a team, brismithers and jkasten2 October 13, 2025 20:46

abdulraqeeb33 requested changes Oct 15, 2025

View reviewed changes

abdulraqeeb33 reviewed Oct 15, 2025

View reviewed changes

jkasten2 requested changes Oct 15, 2025

View reviewed changes

Let UserManager refresh timezone changes on focus

e433a4f

Let the UserManager refresh the changes to properties model, instead of SessionListener. The SubscriptionManager already refreshes the push subscription.

nan-li force-pushed the feat/update_timezone branch from 2b8d221 to e433a4f Compare October 16, 2025 19:23

nan-li requested review from abdulraqeeb33 and jkasten2 October 20, 2025 17:10

jkasten2 approved these changes Oct 20, 2025

View reviewed changes

abdulraqeeb33 requested changes Oct 20, 2025

View reviewed changes

abdulraqeeb33 approved these changes Oct 20, 2025

View reviewed changes

tests: pass application service

f225c32

nan-li added 2 commits October 23, 2025 15:40

nit: remove extraneous non-null assert

14b5d76

It's already non-null

nan-li force-pushed the feat/update_timezone branch from bf179a3 to 14b5d76 Compare October 23, 2025 22:40

nan-li mentioned this pull request Oct 23, 2025

chore(tests): add SessionListenerTests class #2389

Open

18 tasks

Feat: Detect for timezone changes and update the user #2378

Are you sure you want to change the base?

Feat: Detect for timezone changes and update the user #2378

Uh oh!

Conversation

nan-li commented Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

One Line Summary

Details

Motivation

Scope

Testing

Unit testing

Manual testing

Affected code checklist

Checklist

Overview

Testing

Final pass

Uh oh!

abdulraqeeb33 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nan-li Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nan-li commented Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

abdulraqeeb33 commented Oct 16, 2025

Uh oh!

jkasten2 commented Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nan-li commented Oct 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

abdulraqeeb33 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abdulraqeeb33 left a comment

Choose a reason for hiding this comment

Uh oh!

nan-li commented Oct 20, 2025

Uh oh!

nan-li commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

nan-li commented Oct 10, 2025 •

edited

Loading

nan-li Oct 15, 2025 •

edited

Loading

nan-li commented Oct 15, 2025 •

edited

Loading

jkasten2 commented Oct 16, 2025 •

edited

Loading

nan-li commented Oct 20, 2025 •

edited

Loading

abdulraqeeb33 left a comment •

edited

Loading

nan-li commented Oct 23, 2025 •

edited

Loading