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

Current assignee @puneetlath is eligible for the Bug assigner, not assigning anyone new.

Proposal

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

Don't set the user's timezone if it's empty

What is the root cause of that problem?

We updateAutomaticTimezone if there is a difference between the local timezone and user preferences if automatic is set without checking if the local timezone is not empty

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

We need to add a check that currentTimezone is not empty

 if(timezone?.automatic && timezone?.selected !== currentTimezone && !!currentTimezone)

What alternative solutions did you explore? (Optional)

Elliot-Alderson504 Your proposal will be dismissed because you did not follow the proposal template.

Edited by proposal-police: This proposal was edited at 2024-08-15 02:04:16 UTC.

Proposal

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

What is the root cause of that problem?

• As mentioned in the OP, Intl.DateTimeFormat().resolvedOptions().timeZone can be empty. That leads to if the user has "automatic" set for their timezone preferences, we will call the API with the empty timezone in:
  

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

  Line 144 in 8b947e3

  PersonalDetails.updateAutomaticTimezone({

  
  and
  

  App/src/pages/settings/Profile/TimezoneInitialPage.tsx

  Line 31 in 8b947e3

  PersonalDetails.updateAutomaticTimezone({

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

• We need to make sure we don't call the server to update the timezone if Intl.DateTimeFormat().resolvedOptions().timeZone is empty and instead leave it as-is.

• So we need to update:
  

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

  Line 142 in 8b947e3

  if (timezone?.automatic && timezone?.selected !== currentTimezone) {

  
  to:

        if (!isEmpty(currentTimezone) && timezone?.automatic && timezone?.selected !== currentTimezone) {

• And
  

  App/src/pages/settings/Profile/TimezoneInitialPage.tsx

  Line 33 in 8b947e3

  selected: isAutomatic ? (Intl.DateTimeFormat().resolvedOptions().timeZone as SelectedTimezone) : timezone.selected,

  
  to:

            selected: isAutomatic && !isEmpty(currentTimezone) ? currentTimezone : timezone.selected,

with:

        const currentTimezone = Intl.DateTimeFormat().resolvedOptions().timeZone as SelectedTimezone;

What alternative solutions did you explore? (Optional)

• We can early return in here:
  

  App/src/libs/actions/PersonalDetails.ts

  Line 207 in 8b947e3

  const formatedTimezone = DateUtils.formatToSupportedTimezone(timezone);

  
  if formatedTimezone.automatic && isEmpty(formatedTimezone.selected)

Proposal Updated

Proposal

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

Do not assign a timezone to the user if it is empty.

What is the root cause of that problem?

Intl.DateTimeFormat().resolvedOptions().timeZone can return empty timezone depending on each platform implementation, it might also specific browser bug e.g. this chrome issue.

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

In updateAutomaticTimezone function, early return.

src/libs/actions/PersonalDetails.ts

function updateAutomaticTimezone(timezone: Timezone) {
    if (!currentUserAccountID) {
        return;
    }

    if(isEmpty(timezone) || isEmpty(timezone.selected)) {
        return;
    }

What alternative solutions did you explore? (Optional)

N/A

@puneetlath, @chiragsalian, @abdulrahuman5196 Huh... This is 4 days overdue. Who can take care of this?

@abdulrahuman5196 thoughts on the proposals?

Checking now

Contributors, I don't see solid information on the first point in all the proposals.

understand what the scenarios are when the currentTimezone would be empty

Furthermore currentTimezone is not optional value and doesn't have empty options - https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/DateTimeFormat/resolvedOptions#timezone.

Could you folks kindly check and update the proposal on
Then why are we getting empty value?
Or is there some other issue?

And @puneetlath Do we have any way to repro this issue normally?

The person who experienced it was on airplane wifi when they experienced it. I don't know a way to reliably reproduce it.

@puneetlath, @chiragsalian, @abdulrahuman5196 Huh... This is 4 days overdue. Who can take care of this?

@puneetlath @chiragsalian @abdulrahuman5196 this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

@puneetlath, @chiragsalian, @abdulrahuman5196 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

Not overdue, waiting for proposals. Demoting to weekly.

@abdulrahuman5196 PR is ready.

This issue has not been updated in over 15 days. @puneetlath, @Beamanator, @chiragsalian, @abdulrahuman5196, @daledah eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

PR has been on prod since 18/9/2024

@chiragsalian / @puneetlath Seems the automation had failed here. The PR has been in production for more than a week.

The issue should be due for payment.

The PR that introduced the bug has been identified. Link to the PR:
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:
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

Determine if we should create a regression test for this bug.
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.

Not a consistently reproducible bug, so regression test wont be useful here.

Triggered auto assignment to @strepanier03 (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.

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

Current assignee @abdulrahuman5196 is eligible for the External assigner, not assigning anyone new.

Contributor: @daledah owed $250 via Upwork
Contributor : @abdulrahuman5196 due $250 via NewDot

@daledah , can you please accept the job and reply here once you have?
https://www.upwork.com/jobs/~021841911357137490129

Not a consistently reproducible bug,

@abdulrahuman5196 , can you expand on this? We have an option to test monthly for edgecase bugs, which we might want to do here.

can you expand on this? We have an option to test monthly for edgecase bugs, which we might want to do here.

@mallenexpensify This would be fine. I think the test case would already be in regression test. Anyways added the test steps if required.

1. Go to Profile > Timezone
2. Switch to manual timezone
3. Select another timezone
4. Switch back to automatic timezone
5. Verify that: The timezone is updated, no error appears

$250 approved for @abdulrahuman5196

@daledah , can you please accept the job and reply here once you have?

@mallenexpensify I accepted, thanks 🙏

Thanks for stepping in here @mallenexpensify. I've gone ahead and paid @daledah so we're good to go here. Thanks everyone!