[HOLD for payment 2024-10-25] [$250] Android - Timezone-Instead of Asia/Kolkata, America/Los Angeles time zone displayed

[IuliiaHerets]

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

---

Version Number: 9.0.32-1
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4949281
Issue reported by: Applause Internal Team

Action Performed:

Keep location enabled in your device

1. Login with new account
2. Tap profile icon - profile -- timezone

Expected Result:

Incorrect time zone displayed.

Actual Result:

Instead of Asia/Kolkata, America/Los Angeles time zone displayed.

Workaround:

Unknown

Platforms:

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6599795_1726053852288.yg.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021835786081971713587
• Upwork Job ID: 1835786081971713587
• Last Price Increase: 2024-10-07
• Automatic offers:
• akinwale | Reviewer | 104308342

Issue Owner

Current Issue Owner: @trjExpensify

[melvin-bot]

Triggered auto assignment to @trjExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[IuliiaHerets]

@trjExpensify FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[trjExpensify]

Hm, I can't repro this on the browserstack. Asking someone with an Android device to give it a try.

[slafortune]

I can reproduce this in CST -

[melvin-bot]

@trjExpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~021835786081971713587

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @akinwale (External)

[trjExpensify]

Thanks, @slafortune! Moving it on. 👍

[huult]

Edited by proposal-police: This proposal was edited at 2024-10-03 10:30:26 UTC.

Proposal

Please re-state the problem that we are trying to solve in this issue.

Timezone-Instead of Asia/Kolkata, America/Los Angeles time zone displayed

What is the root cause of that problem?

When executing the Onyx connection with key: ONYXKEYS.PERSONAL_DETAILS_LIST, the data for ONYXKEYS.PERSONAL_DETAILS_LIST has not been added yet Sovalue returns empty object, causing timezone to return an empty object, and the personal details do not update the timezone

App/src/libs/Navigation/AppNavigator/AuthScreens.tsx

Line 149 in 539327d

timezone = value?.[currentAccountID]?.timezone ?? {};

App/src/libs/Navigation/AppNavigator/AuthScreens.tsx

Lines 154 to 160 in 539327d

	if (!isEmptyObject(currentTimezone) && timezone?.automatic && timezone?.selected !== currentTimezone) {
	timezone.selected = currentTimezone;
	PersonalDetails.updateAutomaticTimezone({
	automatic: true,
	selected: currentTimezone,
	});
	}

What changes do you think we should make in order to solve the problem?

This issue only occurs with newly created accounts where the default value automatically determines the location. To resolve it, we can simply skip checking selected and currentTimezone, as shown in the following code:
diff~~ ~~// .src/libs/Navigation/AppNavigator/AuthScreens.tsx#L154~~ ~~- if (!isEmptyObject(currentTimezone) && timezone?.automatic && timezone?.selected !== currentTimezone) {~~ ~~+. if (!isEmptyObject(currentTimezone) && (isEmptyObject(timezone) || (timezone.automatic && timezone.selected~~ ~~!== currentTimezone))) {~~ ~~

This issue only occurs with newly created accounts, where the default value automatically determines the location. To resolve it, we can wait for PERSONAL_DETAILS_LIST to finish updating, validate that it has a timezone value, and then update the personal data.

// .src/libs/Navigation/AppNavigator/AuthScreens.tsx#L131
- let timezone: Timezone | null;

// .src/libs/Navigation/AppNavigator/AuthScreens.tsx#L142
-            timezone = null;

// .src/libs/Navigation/AppNavigator/AuthScreens.tsx#L158
-        if (!value || timezone) {
-            return;
-        }

-        timezone = value?.[currentAccountID]?.timezone ?? {};
+      const timezone = value?.[currentAccountID]?.timezone ?? {};
        const currentTimezone = Intl.DateTimeFormat().resolvedOptions().timeZone as SelectedTimezone;


        if (!isEmptyObject(currentTimezone) && timezone?.automatic && timezone?.selected !== currentTimezone) {
        // We don't need to set `selected` here because the timezone will be updated after completing `updateAutomaticTimezone`.
-            timezone.selected = currentTimezone;
            PersonalDetails.updateAutomaticTimezone({
                automatic: true,
                selected: currentTimezone,
            });
        }

POC

Screen.Recording.2024-10-03.at.17.09.49.mp4

[mkzie2]

Proposal

Please re-state the problem that we are trying to solve in this issue.

• Instead of Asia/Kolkata, America/Los Angeles time zone displayed.

What is the root cause of that problem?

• This issue occurs when we create a new account. After creating a new account, this callback
  

  App/src/libs/Navigation/AppNavigator/AuthScreens.tsx

  Line 144 in 539327d

  callback: (value) => {

  
  is called a few times.

1. 1st time, value: {}, timezone: undefined, it set timezone to {} in:
   

   App/src/libs/Navigation/AppNavigator/AuthScreens.tsx

   Line 149 in 539327d

   timezone = value?.[currentAccountID]?.timezone ?? {};

   
   and then return.

2. 2nd time,

    "1234567": {
        "accountID": 1234567,
        "timezone": {
            "automatic": true,
            "selected": "Asia/Kolkata"
        },
    }
}

and

timezone: {} so it will early return in:

App/src/libs/Navigation/AppNavigator/AuthScreens.tsx

Line 146 in 539327d

return;

and there is no API to update the timezone is triggered.

What changes do you think we should make in order to solve the problem?

• This this condition:
  

  App/src/libs/Navigation/AppNavigator/AuthScreens.tsx

  Line 145 in 539327d

  if (!value || timezone) {

  
  we should check !isEmptyObject(timezone) instead of timezone.

What alternative solutions did you explore? (Optional)

[melvin-bot]

@akinwale, @trjExpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

[akinwale]

This is fairly straightforward so we can move forward with @huult's proposal.

🎀👀🎀 C+ reviewed.

[melvin-bot]

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

[mkzie2]

@akinwale I tested your selected solution but the bug is still appeared. Did I miss something? With that solution, the code is early returned in:

App/src/libs/Navigation/AppNavigator/AuthScreens.tsx

Line 146 in 539327d

return;

if timezone is {}.

[akinwale]

@akinwale I tested your selected solution but the bug is still appeared. Did I miss something? With that solution, the code is early returned in:

App/src/libs/Navigation/AppNavigator/AuthScreens.tsx

Line 146 in 539327d

return;

if timezone is {}.

I'll have another look at this.

[huult]

@akinwale, Thank you for reviewing my proposal.

The solution to this issue is to check whether the timeZone object is empty or not. I will test more during the PR phase, optimize the code as necessary.

[mkzie2]

@akinwale Is this still your final decision, right?

[akinwale]

@akinwale Is this still your final decision, right?

No, I will run some tests and then post an update afterwards. Thanks for bringing it up.

[melvin-bot]

📣 @akinwale 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @mkzie2 You have been assigned to this job!
Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs!
Keep in mind: Code of Conduct | Contributing 📖

[dangrous]

How are we looking on the pr @mkzie2 ?

[melvin-bot]

@akinwale @dangrous @trjExpensify @mkzie2 this issue is now 4 weeks old, please consider:

• Finding a contributor to fix the bug
• Closing the issue if BZ has been unable to add the issue to a VIP or Wave project
• If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

[dangrous]

@akinwale do you think you'll have time to review today?

[akinwale]

@akinwale do you think you'll have time to review today?

Yes, I'll get this later today.

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.50-8 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix: incorrect timezone when log in with a new account #50552

If no regressions arise, payment will be issued on 2024-10-25. 🎊

For reference, here are some details about the assignees on this issue:

• @akinwale requires payment automatic offer (Reviewer)
• @mkzie2 requires payment (Needs manual offer from BZ)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@akinwale] The PR that introduced the bug has been identified. Link to the PR:
• [@akinwale] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@akinwale] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@akinwale] Determine if we should create a regression test for this bug.
• [@akinwale] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@trjExpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[akinwale]

• [@akinwale] The PR that introduced the bug has been identified. Link to the PR:
• [@akinwale] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@akinwale] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:

Not a regression.

• [@akinwale] Determine if we should create a regression test for this bug.
• [@akinwale] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Regression Test Steps

1. Launch Expensify
2. Log in with a new account
3. Finish the onboarding steps
4. Navigate to Settings > Profile
5. Verify that the correct timezone based on the device's settings is displayed

Do we agree 👍 or 👎?

[trjExpensify]

Not a regression.

Can you expand on that a bit, please? It's certainly not expected that the timezone of the app is PST on sign-up. We have a timeZone account NVP that's set, and has been for years.

[akinwale]

Not a regression.

Can you expand on that a bit, please? It's certainly not expected that the timezone of the app is PST on sign-up. We have a timeZone account NVP that's set, and has been for years.

The timezone being an empty object was never accounted for in the original implementation, so that led to this issue where it falls back to the default which is PST.

App/src/CONST.ts

Line 1268 in 9f97734

DEFAULT_TIME_ZONE: {automatic: true, selected: 'America/Los_Angeles'},

[trjExpensify]

Hm, okay. I'm not wholly convinced this isn't a regression from somewhere later than the implementation of the timeZone profile setting (i.e some "personal details" onyx updates or something), but probably not worth investing in further.

Payment summary as follows:

• $250 to @akinwale for the C+ review (paid!)
• $250 to @mkzie2 for the fix (offer sent!)

[trjExpensify]

Paid, closing!