Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.32k stars 2.75k forks source link

[$250] Book travel - Report screen changes to profile view when clicking Country field #47069

Open izarutskaya opened 1 month ago

izarutskaya commented 1 month ago

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: 9.0.18.1 Reproducible in staging?: Y Reproducible in production?: Y Email or phone of affected tester (no customers): applausetester+kh050806@applause.expensifail.com Logs: https://stackoverflow.com/c/expensify/questions/4856 Issue reported by: Applause-Internal team

Action Performed:

  1. Go to staging.new.expensify.com
  2. Go to FAB > Book travel.
  3. Click Book travel.
  4. Click Country.

Expected Result:

The report screen in the background will not change to profile page.

Actual Result:

The report screen in the background changes to profile page while LHN remains the same after opening Country RHP.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

https://github.com/user-attachments/assets/e82cb612-2a8a-44c4-8885-c7244c90ebd4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~011952935e51b38f07
  • Upwork Job ID: 1826610853854953560
  • Last Price Increase: 2024-08-22
  • Automatic offers:
    • rojiphil | Reviewer | 103684843
Issue OwnerCurrent Issue Owner: @rojiphil
melvin-bot[bot] commented 1 month ago

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

izarutskaya commented 1 month ago

We think this issue might be related to the #vip-travel

abekkala commented 4 weeks ago

@izarutskaya is it only when clicking Country that the pages changes or is it any of them?

melvin-bot[bot] commented 3 weeks ago

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

abekkala commented 3 weeks ago

@izarutskaya bumping my question above from last week

bernhardoj commented 3 weeks ago

Proposal

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

The background screen changes to workspace profile view when open address page from travel page.

What is the root cause of that problem?

The behavior is a bit different now than the video. When we press Book travel, it will open the profile address page and based on the mapping, the workspace address RHP maps to the workspace profile full screen page. https://github.com/Expensify/App/blob/3559aed60e43c9c179a66fbd5c9dd178cac73324/src/libs/Navigation/linkingConfig/FULL_SCREEN_TO_RHP_MAPPING.ts#L5

So, the workspace profile page is shown. It's the same case with the country selection page where the backTo param is the workspace address page which we recursively find the matching root for the backTo params page. https://github.com/Expensify/App/blob/3559aed60e43c9c179a66fbd5c9dd178cac73324/src/libs/Navigation/linkingConfig/getAdaptedStateFromPath.ts#L112-L135

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

We need to add a backTo params to the workspace address page that points to the travel page so getMatchingRootRouteForRHPRoute will return the matching root for the travel page, which is nothing.

https://github.com/Expensify/App/blob/3559aed60e43c9c179a66fbd5c9dd178cac73324/src/ROUTES.ts#L546-L549

getRoute: (policyID: string, backTo?: string) => getUrlWithBackToParam(`settings/workspaces/${policyID}/profile/address` as const, backTo),

Then, pass the active route as the backTo. https://github.com/Expensify/App/blob/3559aed60e43c9c179a66fbd5c9dd178cac73324/src/pages/Travel/ManageTrips.tsx#L64

Navigation.navigate(ROUTES.WORKSPACE_PROFILE_ADDRESS.getRoute(activePolicyID ?? '-1', Navigation.getActiveRoute()));

Next, we need to do the same thing for the address sub-page, that is country and state selection page. Both pages already have a backTo, but for the country selection page, we pass the active route without params, so the nested backTo is removed. https://github.com/Expensify/App/blob/3559aed60e43c9c179a66fbd5c9dd178cac73324/src/components/CountrySelector.tsx#L78-L80

So, the matching root will still be the workspace profile page. We need to replace it with Navigation.getActiveRoute().

Last, we need to update the route here so it appends the param correctly because the backTo contains a nested backTo (?backTo={...?backTo=...}). https://github.com/Expensify/App/blob/3559aed60e43c9c179a66fbd5c9dd178cac73324/src/pages/settings/Profile/PersonalDetails/CountrySelectionPage.tsx#L50-L56

} else if (!!backTo && navigation.getState().routes.length === 1) {
    // If "backTo" is not empty and there is only one route, go back to the specific route defined in "backTo" with a country parameter
    Navigation.goBack(appendParam(backTo, 'country', option.value) as Route);
} else {
    // Otherwise, navigate to the specific route defined in "backTo" with a country parameter
    Navigation.navigate(appendParam(backTo, 'country', option.value) as Route);
}
melvin-bot[bot] commented 2 weeks ago

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

melvin-bot[bot] commented 2 weeks ago

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

melvin-bot[bot] commented 2 weeks ago

@rojiphil @abekkala 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!

rojiphil commented 2 weeks ago

Thanks for the proposal. The RCA is correct i.e. RHP pages of Address and Country selection map to the workspace profile page in central pane due to which the central pane switches to profile page. And it makes sense to fix this using backTo @bernhardoj proposal LGTM ๐ŸŽ€๐Ÿ‘€๐ŸŽ€ C+ reviewed

melvin-bot[bot] commented 2 weeks ago

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

melvin-bot[bot] commented 2 weeks ago

๐Ÿ“ฃ @rojiphil ๐ŸŽ‰ An offer has been automatically sent to your Upwork account for the Reviewer role ๐ŸŽ‰ Thanks for contributing to the Expensify app!

Offer link Upwork job

bernhardoj commented 2 weeks ago

PR is ready

cc: @rojiphil

abekkala commented 1 week ago

note: not yet merged

rojiphil commented 6 days ago

Oh! The automation did not get triggered here. Note: Reached production on 06 Sep