[HOLD for payment 2024-07-22] [HOLD for payment 2024-07-17] [$250] Profile - Unable to select a different country when there is existing address

[m-natarajan]

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

---

Issue found when validating #41970
Version Number: 1.4.76-0
Reproducible in staging?: y
Reproducible in production?: n
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause internal team
Slack conversation:

Action Performed:

1. Go to staging.new.expensify.com
2. Go to Account settings.
3. Go to Profile > Address.
4. Enter an address and save it.
5. Click Address.
6. Click Country.
7. Select a different country.

Expected Result:

The new country is selected.

Actual Result:

Unable to select a new country.

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

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

Screenshots/Videos

Add any screenshot/video evidence

Bug6492993_1716820730437.20240527_221415.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~018874b9ee9ce02e91
• Upwork Job ID: 1795126238019223552
• Last Price Increase: 2024-05-27
• Automatic offers:
• getusha | Contributor | 102495373

Issue Owner

Current Issue Owner: @puneetlath

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @carlosmiceli (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[m-natarajan]

@carlosmiceli 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

[m-natarajan]

We think that this bug might be related to #vip-vsb

[melvin-bot]

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

[melvin-bot]

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

[carlosmiceli]

I think the issue was caused by this PR.

[carlosmiceli]

@puneetlath @roryabraham Quick help understanding the proper way to handle this: I can see that it can be an External issue, but it's also hourly deploy blocker. Should I not add the External label in that case and fix it myself or assign it to whoever worked on that PR? In this case, since the issue was also caused by a Contributor's PR, should I assign it instead to whomever reviewed/approved the PR?

[nkdengineer]

@carlosmiceli I believe deploy blockers are high priority so if a contributor found the cause and can raise the PR immediately to fix it, they can be assigned

[nkdengineer]

@carlosmiceli I can raise a PR asap if assigned.

Proposal

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

Unable to select a new country.

What is the root cause of that problem?

The dependency here is an object, so even though address does not change, it still trigger rerenders because 2 objects are considered different objects even though their values are the same, causing the address.country to override the recently selected country.

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

We need to make the dependencies explicit

}, [address?.state, address?.country, address?.city, address?.zip]);

So when none of those values change, the useEffect will not be run again.

[Optional] This should be updated so we check all fields individually, so there'll be no dependency on address as an object.

What alternative solutions did you explore? (Optional)

Store the prevAddress using usePrevious, and use deepEqual to compare it with address, if there's no change, early return. We can use deepEqual or can manually compare all fields individually to see if anything change

Another solution, we can split those to smaller useEffect that will only run if one of the address fields change, like country, and set only that address field.

useEffect(() => {
    // optional, if not then currentCountry will be set to empty if address?.country becomes empty
    if (!address?.country) {
        return;
    }
    setCurrentCountry(address.country);
}, [address?.country]);

Another solution: Memoize the AddressPage/PersonalAddressPage so it doesn't rerender if the address individual fields do not change (along other fields)

[hungvu193]

I think the issue was caused by this PR.

This PR is still in 7 days regression period, so if it's urgent you can just handle it by your own. Otherwise, you can assign the original contributor and reviewer who worked on offending PR.

[mountiny]

I have created a straight revert PR #42697

@smelaa Can you please comment here, and I will assign you? You can raise the PR again and let's make sure we fix and test this case thoroughly. Thank you!

[mountiny]

Reverted, but reopening so we can handle the fix next

[blazejkustra]

Someone from SWM will comment on this issue early next week (Ola is no longer working at SWM 😢)

[puneetlath]

@mountiny do we still need this issue? Will we handle the fix here or in #41965?

[mountiny]

yeah we can handle it there, I will cross link it

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[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.5-13 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• [Travel] Bring back refactored AddressPage #43348

If no regressions arise, payment will be issued on 2024-07-17. 🎊

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

• @getusha requires payment automatic offer (Contributor)

[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:

• [@getusha] The PR that introduced the bug has been identified. Link to the PR:
• [@getusha] 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:
• [@getusha] 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:
• [@getusha] Determine if we should create a regression test for this bug.
• [@getusha] 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.
• [@puneetlath] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[melvin-bot]

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

• [Travel] Bring back refactored AddressPage #43348

If no regressions arise, payment will be issued on 2024-07-22. 🎊

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

• @getusha requires payment automatic offer (Contributor)

[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:

• [@getusha] The PR that introduced the bug has been identified. Link to the PR:
• [@getusha] 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:
• [@getusha] 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:
• [@getusha] Determine if we should create a regression test for this bug.
• [@getusha] 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.
• [@puneetlath] Link the GH issue for creating/updating the regression test once above steps have been agreed upon: