[$500] Profile - Address - Zip-code is saved when changing country but the city is cleared

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel

---

Version Number: 1.3.90-2

Reproducible in staging?: Yes

Reproducible in production?: Yes

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 or open App
2. Go to Settings > Profile > Personal Details > Address
3. Start to enter "Address Line 1" and select any address from suggested or enter all address manually
4. Change Country to any other

Expected Result:

When changing the country, fields uniquely related to the country could be cleared - state/province and zip code

Actual Result:

Zip-code is saved when changing country but the city is cleared

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

Android: Native

Android: mWeb Chrome

iOS: Native

Bug6249456_1698185032677.T3891508-Address-City-cleared-after-country-change.mp4

iOS: mWeb Safari

MacOS: Chrome / Safari

MacOS: Desktop

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01637e39759029f96e
• Upwork Job ID: 1716961481545691136
• Last Price Increase: 2023-10-24
• Automatic offers:
• DylanDylann | Contributor | 27490337

[melvin-bot]

Triggered auto assignment to @laurenreidexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

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

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[melvin-bot]

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

[neonbhai]

Proposal

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

Zip Code not cleared when country changes

What is the root cause of that problem?

On Address Change, when country is set, we reset the related fields here:

App/src/pages/settings/Profile/PersonalDetails/AddressPage.js

Lines 143 to 148 in dd87393

	if (key === 'country') {
	setCurrentCountry(value);
	setState('');
	setCity('');
	return;
	}

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

We should also clear the zip field, when country is changed. We will be add a zip state, and calling setZip('') here.

Branch here

Result:

Choosing address:

Screencast.from.27-10-23.09.24.10.PM.IST.webm

Changing Country:

Screencast.from.27-10-23.09.38.39.PM.IST.webm

Zipcode now reacts to address changes.

[yh-0218]

Proposal

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

Zip Code not cleared when country changes

What is the root cause of that problem?

When we change country, we didn't reset zipcode.

App/src/pages/settings/Profile/PersonalDetails/AddressPage.js

Lines 143 to 148 in f8fc21d

	if (key === 'country') {
	setCurrentCountry(value);
	setState('');
	setCity('');
	return;
	}

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

1. we need to add zip state variable like country and city
2. we need to reset zip before return

setZipe('')

App/src/pages/settings/Profile/PersonalDetails/AddressPage.js

Lines 146 to 148 in f8fc21d

	setCity('');
	return;
	}

3. I think we need to add these code too.

 if (key === 'state') {
            setCity('');
            setZip('');
            return;
 }
 if (key === 'city') {
            setZip('');
            return;
 }

What alternative solutions did you explore? (Optional)

[DylanDylann]

Proposal

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

Zipcode field is not clear when select another country

What is the root cause of that problem?

We only use useState for country and state, city fields because we need to re-set 3 fields when changing country

App/src/pages/settings/Profile/PersonalDetails/AddressPage.js

Lines 139 to 154 in a8482e2

	const handleAddressChange = useCallback((value, key) => {
	if (key !== 'country' && key !== 'state' && key !== 'city') {
	return;
	}
	if (key === 'country') {
	setCurrentCountry(value);
	setState('');
	setCity('');
	return;
	}
	if (key === 'state') {
	setState(value);
	return;
	}
	setCity(value);
	}, []);

I think the zipcode field also should be clear after changing country like state, city fields.

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

Firstly, we need to use useState for zipcode field

const [zipcode, setZipcode] = useState(address.zip);

When country change we need to re-set zipcode to empty like this

const handleAddressChange = (value, key) => {
        if (key !== ‘country’ && key !== ‘state’ && key !== ‘city’ && key !== ‘zipPostCode’) {          // UPDATE HERE
            return;
        }
        if (key === ‘country’) {
            setCurrentCountry(value);
            setState(‘’);
            setCity(‘’);         
            setZipcode('')
            return;
        }
        if (key === 'state') {
            setState(value);
            setCity('') // IF we need to clear city field when changing state
            setZipcode('') // IF we need to clear zip field when changing state
            return;
        }
        if (key === 'city') {
            setCity(value);
            setZipcode('') // IF we need to clear zip field when changing city
            return;
        }
        setZipcode(value);
    };

and then in here

App/src/pages/settings/Profile/PersonalDetails/AddressPage.js

Line 222 in 70e9fa0

defaultValue={address.zip || ''}

we should use value instead of default value

value={zipcode || ‘’}

One more thing, in here

App/src/pages/settings/Profile/PersonalDetails/AddressPage.js

Line 91 in a8482e2

setCity(address.city);

add this line setZipcode(address.zip); to avoid this bug #27814

[laurenreidexpensify]

@robertKozik bump for review on these proposals thanks

[robertKozik]

Hi all! After reviewing your proposals, I've noticed that they all suggest a similar approach to solving the problem. In this case, I believe we should acknowledge the contributor who first posted their proposal - @neonbhai.

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[DylanDylann]

I believe we should acknowledge the contributor who first posted their proposal - @neonbhai.

@robertKozik Could you help to check again? @neonbhai edited his proposal (and his branch )3 days ago after I posted the proposal. My proposal is first one that is fully and cover all edge cases

[DylanDylann]

@robertKozik This is the original proposal of @neonbhai

And I don't think It covers all cases

[neonbhai]

Adding zipcode to state logic is a simple change 🤔

Many times I've seen in app, the first contributor who presents the idea is assigned. I hope the right decision is made 🙇

[DylanDylann]

@neonbhai It is not a simple change. It so easily causes regression If we don't clear all cases (you can see this issue as an example #28007). In your original proposal, you only mentioned the new state and calling setZip, I agree that It maybe fix this issue but It will break other flows.

[robertKozik]

I see your point of view @DylanDylann. I got misled by the edited proposals. The explicit coverage of the regression-prone cases is a meaningful change and this proposal should be, in my opinion, considered as a separate one.

Also, as stated inside the CONTRIBUTING.md doc file, after making a change to an existing proposal, you should post a new comment to alert everyone about the change @neonbhai.

That's why I think my previous decision was not right, and @DylanDylann's proposal should be recommended by me. But as now we have CME assigned, let's wait for his input on that topic.

[lakchote]

Sometimes a simple change can make or break an app.

Even though @DylanDylann adds just one line of code, this line of code is important to prevent bugs. He has covered all edge cases.

@neonbhai you should have notified everyone you did change your proposal as stated in [CONTRIBUTING.MD] guidelines:(https://github.com/Expensify/App/blob/main/contributingGuides/CONTRIBUTING.md)

For these reasons, I'm assigning @DylanDylann.

[melvin-bot]

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

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[DylanDylann]

@robertKozik The PR is ready for review

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

[DylanDylann]

Proposal

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

Personal details - City and Postcode fields are not populated with suggested address

What is the root cause of that problem?

Currently in address page

• when we change the country we will reset state, city, zipcode to empty
• when we change the state we will reset city, zipcode to empty
• when we change the city we will reset zipcode to empty

When we update address line 1, all fields will be updated in order to: country, city, zipCode, state

zipcode and city fields is updated before state field, so that when state field is updated the App will reset zipcode and city

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

When we update address line 1, all fields should be updated in order to: country, state, city, zipCode
To do that in here

App/src/components/AddressSearch/index.js

Lines 231 to 239 in dee9b31

	country: '',
	// When locality is not returned, many countries return the city as postalTown (e.g. 5 New Street
	// Square, London), otherwise as sublocality (e.g. 384 Court Street Brooklyn). If postalTown is
	// returned, the sublocality will be a city subdivision so shouldn't take precedence (e.g.
	// Salagatan, Upssala, Sweden).
	city: locality || postalTown || sublocality || cityAutocompleteFallback,
	zipCode,
	state: state || stateAutoCompleteFallback,

Should move the state field to before city field like this

            country: '',
            state: state || stateAutoCompleteFallback,
            city: locality || postalTown || sublocality || cityAutocompleteFallback,
            zipCode,

What alternative solutions did you explore? (Optional)

[DylanDylann]

Proposal to fix regression
cc @robertKozik

[robertKozik]

I'm ok with your approach 🚀 If you could create the PR with it
CC. @lakchote

[laurenreidexpensify]

ah shucks this was deployed to prod weeks ago - #30831 - i'll work on payments today, the automation failed

[laurenreidexpensify]

Payment Summary:

• C+ @robertKozik expert contributor, no payment
• Contributor - @DylanDylann $500 payment issued in Upwork

[laurenreidexpensify]

@robertKozik can you confirm normal checklist items / whether we need a regression test thanks

[robertKozik]

I think we don't need new regression test here

• [@robertKozik] The PR that introduced the bug has been identified. Link to the PR: It's not a regression from the PR, it was a case which was not covered yet
• [@robertKozik] 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: N/A
• [@robertKozik] 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: N/A
• [@robertKozik] Determine if we should create a regression test for this bug. I think we don't need it here
• [@robertKozik] 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. N/A