Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HOLD for payment 2023-10-31] Address - 'Hmm it's not here' page shows up when selecting a country #30128

Closed
6 tasks done
lanitochka17 opened this issue Oct 21, 2023 · 34 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering Monthly KSv2

Comments

@lanitochka17
Copy link

lanitochka17 commented Oct 21, 2023

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: 1.3.88-3

Reproducible in staging?: Yes

Reproducible in production?: No

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 Settings > Profile > Personal details > Address > Country
  3. Select a country

Expected Result:

Country is selected successfully

Actual Result:

'Hmm it's not here' page shows up

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
iOS: mWeb Safari
MacOS: Chrome / Safari
Bug6245644_1697905786399.bandicam_2023-10-21_20-14-31-107.mp4
MacOS: Desktop

View all open jobs on GitHub

@lanitochka17 lanitochka17 added DeployBlockerCash This issue or pull request should block deployment Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Oct 21, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 21, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Oct 21, 2023

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

@OSBotify
Copy link
Contributor

👋 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
Copy link

melvin-bot bot commented Oct 21, 2023

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

@ahmedGaber93
Copy link
Contributor

ahmedGaber93 commented Oct 21, 2023

Proposal

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

Address - 'Hmm it's not here' page shows up when selecting a country

What is the root cause of that problem?

After select country we backTo using route.params.backTo

Navigation.navigate(`${route.params.backTo}?country=${option.value}`);

the issue here is the URL is not correct, so the route.params.backTo is return undifined
the url has 2 "?" ?country=US?backTo=
http://localhost:8082/settings/profile/personal-details/address/country?country=US?backTo=%2Fsettings%2Fprofile%2Fpersonal-details%2Faddress

this because getBackToParam is start with "?" regardless the main URL has "?" or not

App/src/ROUTES.ts

Lines 14 to 16 in 6031f3e

function getBackToParam(backTo?: string): string {
return backTo ? `?backTo=${encodeURIComponent(backTo)}` : '';
}

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

We need to change getBackToParam to start with "?" if is first params and else start with "&".

function getBackToParam(backTo?: string, isFirstParam: boolean = true): string {
    return backTo ? `${isFirstParam ? "?" : "&"}backTo=${encodeURIComponent(backTo)}` : '';
}

What alternative solutions did you explore? (Optional)

or replace getBackToParam with new method getUrlWithBackToParam

function getUrlWithBackToParam(url: string, backTo?: string): string {
    const backToParam = backTo ? `${url.includes("?") ? "&" : "?"}backTo=${encodeURIComponent(backTo)}` : '';
    return url + backToParam;
}

// use it like this
// getRoute: (accountID: string | number, backTo?: string) => `a/${accountID}${getBackToParam(backTo)}`
getRoute: (accountID: string | number, backTo?: string) => getUrlWithBackToParam(`a/${accountID}`)

@s-alves10
Copy link
Contributor

s-alves10 commented Oct 21, 2023

This is a regression from the PR #29813

cc @tgolen

@thienlnam
Copy link
Contributor

thienlnam commented Oct 22, 2023

Hmm I tried the revert of that PR but still running into a different problem when trying the reproduction steps
http://localhost:8082/settings/profile/personal-details/address

Screenshot 2023-10-21 at 5 10 12 PM

Uncaught Error: allCountries.a was not found in the default language

@thienlnam
Copy link
Contributor

Ah nevermind, looks like it is fixed after NPM install - going with a straight revert here

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Hourly KSv2 labels Oct 22, 2023
@luacmartins
Copy link
Contributor

I think @ahmedGaber93's solution with getUrlWithBackToParam looks good. I created a PR with it since we might CP this weekend to fix the issue before Money2020. We should compensate @ahmedGaber93 accordingly.

@mountiny
Copy link
Contributor

@JmillsExpensify What I have usually applied was the 25% rule so that would be $125 to @ahmedGaber93

@mountiny mountiny removed the DeployBlockerCash This issue or pull request should block deployment label Oct 22, 2023
@mountiny
Copy link
Contributor

Fixed in staging

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Oct 23, 2023
@melvin-bot melvin-bot bot added the Overdue label Nov 1, 2023
@JmillsExpensify
Copy link

@luacmartins Quick check on the regression test. Do you think we need one for this case?

@melvin-bot melvin-bot bot removed the Overdue label Nov 1, 2023
@luacmartins luacmartins changed the title [HOLD for payment 2023-10-31] [HOLD for payment 2023-10-30] Address - 'Hmm it's not here' page shows up when selecting a country [HOLD for payment 2023-10-31] Address - 'Hmm it's not here' page shows up when selecting a country Nov 1, 2023
@luacmartins
Copy link
Contributor

No need for a regression test. Applause caught this during it already.

@melvin-bot melvin-bot bot added the Overdue label Nov 6, 2023
@luacmartins
Copy link
Contributor

Just missing payment here

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Nov 6, 2023
@luacmartins
Copy link
Contributor

Same

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Nov 9, 2023
Copy link

melvin-bot bot commented Nov 13, 2023

@JmillsExpensify, @luacmartins Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

1 similar comment
Copy link

melvin-bot bot commented Nov 13, 2023

@JmillsExpensify, @luacmartins Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@JmillsExpensify
Copy link

Ok great, circling back for payment then.

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Nov 15, 2023
@luacmartins
Copy link
Contributor

Just missing payment

@melvin-bot melvin-bot bot removed the Overdue label Nov 20, 2023
Copy link

melvin-bot bot commented Nov 24, 2023

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

@melvin-bot melvin-bot bot added the Overdue label Nov 24, 2023
@JmillsExpensify
Copy link

@ahmedGaber93 Do you mind confirming that you haven't been paid for this issue? I'm unable to find a linked Upwork job, so I just want to be sure before creating another job.

@melvin-bot melvin-bot bot removed the Overdue label Nov 24, 2023
@ahmedGaber93
Copy link
Contributor

@JmillsExpensify no, I haven't been paid for this issue.

@JmillsExpensify
Copy link

Thanks I'll create an Upwork job now.

@JmillsExpensify
Copy link

Mind applying here: https://www.upwork.com/jobs/~0172d8e78897a88014

@ahmedGaber93
Copy link
Contributor

@JmillsExpensify Applied.

@JmillsExpensify
Copy link

Offer sent! Thank you

@JmillsExpensify
Copy link

Offer paid via Upwork. Thanks everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering Monthly KSv2
Projects
None yet
Development

No branches or pull requests

8 participants