Desktop - Automatically-set time zones don't update when you change time zones (pay on 9/8)

[isagoico]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

Easy mode 🟢

1. Open desktop app
2. Check the time zone is set automatically in settings
3. Go to device settings
4. Change the time zone of the device
5. Check the time zone settings in New Expensify app

Very hard and expensive mode 🔴

1. Open desktop app in location A
2. Verify the time zone settings in app. Set the device to sleep without closing the app.
3. Buy a plane ticket to location B that is in another time zone
4. Travel ✈️
5. Open the device in location B and check the New Expensify app time zone settings.

Expected Result:

Time zone should update automatically without the need of closing and reopening the app.

Actual Result:

Time zone is not updated automatically. User has to close and reopen for the time zone to update.

Workaround:

User has to close and reopen for the time zone to update.

Platform:

Where is this issue occurring?

• Web
• iOS
• Android
• Desktop App ✔️
• Mobile Web

Version Number: 1.0.83-0

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos:

Expensify/Expensify Issue URL:

View all open jobs on Upwork

---

From @roryabraham https://expensify.slack.com/archives/C01GTK53T8Q/p1628522723109900

Automatically-set timezones don't update when you change timezones until you close and reopen the app.

[MelvinBot]

Triggered auto assignment to @TomatoToaster (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[TomatoToaster]

Looks good, I think this could easily be categorized as external.

I think one possible way to solve this could be similar to the BaseUpdateAppModal which asks the user to essentially reopen the app when we notice there's a newer version of the app out. This could reopen the app for them in the same way or ask them to quit and reopen to update the timezone. I'm not sure how BaseUpdateAppModal works so if it's not helpful ignore the suggestion 😄 .

[MelvinBot]

Triggered auto assignment to @bfitzexpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[dklymenk]

Hello,

Currently the code responsible for automatic time zone updates is being executed only once during route initialization. (and on actual setting change).

To make it so it is updated automatically without restart, normally I would create a setInterval that would run that code every 10 or 20 minutes, however, since I know setInterval is a big no-no around here, I propose a solution similar to one we had here #3272.

A tl;dr is to create an event listener for user activity ('mousemove', 'touchstart', 'keydown', 'scroll') and run some new checkCurrentTimezone function throttled by, for example, 3 hours.

I think if we introduce that we shouldn't need the old implementation.

[MelvinBot]

Triggered auto assignment to @thienlnam (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[thienlnam]

@dklymenk

The proposal sounds solid, I took a look at your PR https://github.com/Expensify/App/pull/3573/files and agree a similar solution here could work for that as well. As for removing the old implementation, we might want to keep that for a timezone change when the app is initialized

[thienlnam]

@bfitzexpensify Could you please hire @dklymenk in UpWork?

[dklymenk]

As for removing the old implementation, we might want to keep that for a timezone change when the app is initialized

I meant that since the newly implemented check would happen immediately on the first interaction with the app anyway, why should we keep the old redundant implementation.

Anyway, I will need to get my hands dirty first and make some tests to make sure removing it doesn't break anything.

[thienlnam]

@dklymenk 👍 sounds good, had a brain freeze for a bit but yeah I agree

[MelvinBot]

@thienlnam, @bfitzexpensify, @dklymenk Huh... This is 4 days overdue. Who can take care of this?

[dklymenk]

Sorry, waiting for contract to start.

[bfitzexpensify]

My bad @dklymenk! Thought I had hired you in Upwork. That should be done now.

[dklymenk]

Yep, got the offer. Thanks.

[dklymenk]

Hello @thienlnam . I have a few questions.

1. Do we need this only on desktop as stated in the original post, or should the same automatic timezone change behavior apply to web (+ mweb) and native platforms?
2. If we are to do it on native, we won't be able to use the document object to track events. We will instead need to rely on some new lib or try to wrap some view with onPress around the entire app and hope it works. I tried to do the View with onPress, and app either crashes or just shows a blank screen. Before I go into researching those issues, I'd like to know if we even need this feature on native.
3. If we are not doing it on native, we will need to keep the old code that updates the timezone on app start and make sure we add {leading: false} option to underscore's throttle to make it so we don't check for timezone updates twice.
4. In my current implementation of the feature, I am trying to reuse the value from this listener , however I am unsure if my current approach around avoiding the mess surrounding the _.isObject call is valid and I'm wondering if we even need it. To me, that value will always be an object. Even the code on ProfilePage always treats it as an object. Do you know anything about it?

[thienlnam]

1. All changes should be cross platform, and especially since it is most likely that the user would experience a time change on a mobile device, it makes sense to include it there as well

2/3. Let's do an investigation of what this feature would look like on native devices and see where to go from there

4. Not sure about the context on that, but cc @NikkiWines, do you happen to have the context for question 4?

In my current implementation of the feature, I am trying to reuse the value from this listener , however I am unsure if my current approach around avoiding the mess surrounding the _.isObject call is valid and I'm wondering if we even need it. To me, that value will always be an object. Even the code on ProfilePage always treats it as an object. Do you know anything about it?

[NikkiWines]

Ah yeah, I have context. Back in January when we were first building E.cash/New Expensify we only passed the timezone name (e.g. America/Los_Angeles) and not whether or not it was automatically selected (because we hadn't yet built out the feature that allowed users to modify that in product)

We even had a migration for it, though it ended up causing some issues and we reverted it - opting instead to implement that check to determine if it was an object or not. It's been months since we moved to passing the timezone as an object, so I'd say we're probably ok to remove that check.

[dklymenk]

@thienlnam

All changes should be cross platform, and especially since it is most likely that the user would experience a time change on a mobile device, it makes sense to include it there as well

There aren't that many options when it comes to truly cross-platform solutions, especially if we are not using a setInterval to check for timezone updates every N minutes.

Sadly I couldn't find anything worthwhile when looking for some kind of global user activity event listeners for react native. The obvious approach is to try and wrap the app in TouachableOpacity or TouchableWithoutFeedback and run the throttled timezone check function on onPress, but if an onPress of any child element is triggered this parent one is not triggered, so it's not a valid way to determine user activity, unless we add that throttled function call to every onPress handler of the app.

However, I believe there are much better and cleaner solutions than using global user activity here. On native, for example, we can use AppState.addEventListener('change', () => {}) and check timezone when app becomes active. User will definitely trigger this more often than every 3 hours (a throttle value for web and desktop), and when boarding off the plane even if you don't close the app you still put your phone to sleep to get the bags or papers or whatever. One case where this will not work is during bus travel where you basically can drive through timezone while consistently chatting on the app, not sure if this would be a big concern.

Another potential trick we can do is run the throttled function on some events like message send or profile settings open, since that's where you will see your currently selected timezone. This one is actually cross platform too.

So to summarize, here are the options:

1. on app start (current implementation) - cross-platform
2. global activity - only web and desktop
3. app state - only native
4. setInterval - cross-platform
5. on message send and on profile settings open - cross-platform

While none of these are great, we are not forced to rely on just one. I think a good overall solution would be to use the already implemented 1 in combination with 5. It should cover all the potential issues and potential user discomforts especially if we don't throttle the function on profile settings open to make sure that if you open them you are guaranteed to see an up-to-date timezone there.

[thienlnam]

@dklymenk Thanks for the great write-up! I agree with you having a combination of 1 and 5 here will cover enough cases and is the closest thing the cross-platform we can get here. Let's go ahead with that solution

[dklymenk]

@thienlnam

Hello. the PR is ready. #4799

One potential point of contention is the throttle time I use for the "on message send" trigger. I put it at 5 minutes because I don't want it to be checked on every other message, but also want to make sure it's as up-to-date as possible.

[MelvinBot]

@thienlnam, @bfitzexpensify, @dklymenk Whoops! This issue is 2 days overdue. Let's get this updated quick!

[thienlnam]

Been focused on internal projects but just got around to reviewing this - changes look good!

[bfitzexpensify]

Sitting with Tyler for review.

[botify]

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

[botify]

🚀 Deployed to staging by @tylerkaraszewski in version: 1.0.89-0 🚀

platform	result
🤖 android 🤖	skipped 🚫
🖥 desktop 🖥	skipped 🚫
🍎 iOS 🍎	skipped 🚫
🕸 web 🕸	skipped 🚫

[botify]

🚀 Deployed to production by @roryabraham in version: 1.0.90-2 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	failure ❌
🕸 web 🕸	success ✅

[MelvinBot]

@thienlnam, @bfitzexpensify, @dklymenk Huh... This is 4 days overdue. Who can take care of this?

[bfitzexpensify]

Deployed 7 days ago, no issues, paid out and ended the contract. Thanks @dklymenk!