[HOLD for payment 2023-08-17] [$1000] Web - Country suggestion does not work with head or trail space

[kbecciv]

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

---

Action Performed:

1. Go to Settings > Profile > Personal details > Home address > Country
2. Type any name with head or trail space

Expected Result:

Should show country suggestion if there is head or trail space

Actual Result:

Country suggestion does not work with head or trail space

Workaround:

Unknown

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

**Version Number:**1.3.45-8
Reproducible in staging?: y
Reproducible in production?: new feature
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
Notes/Photos/Videos: Any additional supporting documentation

Screen.Recording.2023-07-26.at.7.25.29.PM.1.mov

Recording.3931.mp4

Expensify/Expensify Issue URL:
Issue reported by: @gadhiyamanan
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1690379902974339

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01918263d8bfff6b0e
• Upwork Job ID: 1684585153238552576
• Last Price Increase: 2023-08-03
• Automatic offers:
• gadhiyamanan | Reporter | 25935145

[huzaifa-99]

Proposal

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

We want the country selection to work if there is a head or trail space

What is the root cause of that problem?

Here, we are not using the trimmed value of search

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

There are 2 solutions to this

1. Just dealing with extra spaces: Remove the leading/trailing space from search text when filtering result and show the filtered result, this can be done using the trimmed value of searchValue here and it will fix the bug.

But the search still feels broken as it won't work for some other search text, like when a text has extra spaces in the middle ex: United Arab Emirates. I suggest we just remove any spaces in the search text with empty string when filtering the results. This can be done with a simple regex or str.replaceAll()

2. Deal with extra spaces and other chars: We can remove extra spaces by the above method but some countries have

• brackets ex: Myanmar (Burma), Cocos (Keeling) Islands
• dots ex: St. Lucia, U.S Virgin Islands
• dashes ex: Congo - Kinshasa, Congo - Brazzaville, Timor-Leste
• ampersand ex: St. Kitts & Nevis, St. Vincent & Grenadines
• single quote ex: Côte d'Ivoire

so search values like these will fail

Keeling Islands
US Virgin Islands
Timor Leste
Kitts N
Côte dIvoire

I think we should also remove these by using some regex like /[-'().&\s]/g and when filtering we should remove these chars from the searchValue and the countries text. This will give more accurate results.

Optional:
1- we can omit/add any other chars when filtering
2- the leading/trailing and middle spaces issue is also with the <StateSelectorModal/>, we can also replace leading/trailing and middle spaces from the suggestion when searching in the same manner.
3- to optimize results, we can store the updated text of countries (replacing the chars discussed above) in state and use that when filtering results

What alternative solutions did you explore? (Optional)

N/A

[OSBotify]

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

[melvin-bot]

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

[danieldoglas]

This doesn't need to be a deploy blocker, so removing the hourly/deployblockercash labels and adding bug

[melvin-bot]

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

[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

[danieldoglas]

Also adding External for this.

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[danieldoglas]

Removing myself from the issue for now until there are proposals.

[b4s36t4]

Proposal

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

Web - Country suggestion does not work with head or trail space

What is the root cause of that problem?

App/src/components/CountryPicker/CountrySelectorModal.js

Lines 36 to 44 in ac74ab9

	function filterOptions(searchValue, data) {
	const trimmedSearchValue = searchValue.trim();
	if (trimmedSearchValue.length === 0) {
	return [];
	}
	return _.filter(data, (country) => country.text.toLowerCase().includes(searchValue.toLowerCase()));
	}

Here in this function snippet we're doing filtering the countries based on the searchValue.

We're trimming the searchValue but not doing the filter based on the trimmedSearchValue even though we're having and checking for the trimmed value.

Because of the not trimmed value we're not getting any results

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

Changing this

App/src/components/CountryPicker/CountrySelectorModal.js

Line 42 in ac74ab9

return _.filter(data, (country) => country.text.toLowerCase().includes(searchValue.toLowerCase()));

line to like this

return _.filter(data, (country) => country.text.toLowerCase().includes(trimmedSearchValue.toLowerCase()));

should work perfect

What alternative solutions did you explore? (Optional)

NA

[huzaifa-99]

@danieldoglas I added a proposal here

[melvin-bot]

📣 @eVoloshchak Please request via NewDot manual requests for the Reviewer role ($1000)

[melvin-bot]

📣 @huzaifa-99 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 📖

[melvin-bot]

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

Offer link
Upwork job

[huzaifa-99]

Thanks for the assignment, pr will be ready for review shortly

[huzaifa-99]

PR is ready for review @eVoloshchak.

[marcaaron]

Looks like @eVoloshchak will be reviewing today.

[melvin-bot]

🎯 ⚡️ Woah @eVoloshchak / @huzaifa-99, great job pushing this forwards! ⚡️

The pull request got merged within 3 working days of assignment, so this job is eligible for a 50% #urgency bonus 🎉

• when @huzaifa-99 got assigned: 2023-08-03 21:03:26 Z
• when the PR got merged: 2023-08-07 22:26:18 UTC

On to the next one 🚀

[marcaaron]

This was merged ahead of schedule. Not overdue 😄

[melvin-bot]

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

• Updated country and state searches to fix extra spaces + support searching with/without non alphabet chars in country name #24140

If no regressions arise, payment will be issued on 2023-08-17. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

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

• @eVoloshchak does not require payment (Eligable for Manual Requests)
• @huzaifa-99 requires payment

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

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

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

[eVoloshchak]

• The PR that introduced the bug has been identified. Link to the PR: That's how it was implemented initially in 17548 Personal details push to page #21049, I wouldn't call it a regression, more of a feature/polish that wasn't implemented at the time
• 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
• 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: https://expensify.slack.com/archives/C049HHMV9SM/p1692272320271749

[eVoloshchak]

Regression Test Proposal

1. Go to settings > profile > personal details > home address
2. Go to the country selector
3. Try out different searches with/without 1 or more than 1 spaces in the start, middle or end of the search term -- Verify that it works
4. Try out different searches with a search term with/without brackets ex: Myanmar (Burma), Cocos (Keeling) Islands -- Verify that it works
5. Try out different searches with a search term with/without dots ex: St. Lucia, U.S Virgin Islands -- Verify that it works
6. Try out different searches with a search term with/without dashes ex: Congo - Kinshasa, Congo - Brazzaville, Timor-Leste -- Verify that it works
7. Try out different searches with a search term with/without ampersand ex: St. Kitts & Nevis, St. Vincent & Grenadines -- Verify that it works
8. Try out different searches with a search term with/without single quote ex: Côte d'Ivoire -- Verify that it works
9. Try out different searches with country code ex: AF -> Afganistan, US -> United States, DZ -> Argeliz -- Verify that it works

Do we agree 👍 or 👎

[JmillsExpensify]

Payment summary for this issue:

• Issue reporter: @gadhiyamanan $250
• Contributor: @huzaifa-99 $1,500 (incl. urgency bonus)
• Contributor+: @eVoloshchak $1,500 (incl. urgency bonus)

Upwork is here: https://www.upwork.com/jobs/~01918263d8bfff6b0e. I've extended contracts to issue reporter and contributor.

@eVoloshchak You payment has been reviewed and approved for NewDot.

[huzaifa-99]

Accepted the offer @JmillsExpensify. Thanks

[JmillsExpensify]

Paid out both issue reporter and contributor. Will close this issue out once I add the regression issue and process payment in NewDot.

[marcaaron]

I think one can be closed? But feel free to reopen if there's anything left to do here.