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.12k stars 2.62k forks source link

[$250] Expense - Tapping back button after deleting expense leads to not here page #44199

Open m-natarajan opened 2 weeks ago

m-natarajan commented 2 weeks 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.1-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. Launch New Expensify app.
  2. Submit two manual expenses to any user.
  3. Go to transaction thread of any expenses.
  4. Tap on the report header.
  5. Tap Delete expense.
  6. Delete the expense.
  7. Tap on the back button on the top left.

Expected Result:

App will return to the main page.

Actual Result:

App shows long skeleton loading and then not here page loads.

Workaround:

unknown

Platforms:

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

Screenshots/Videos

https://github.com/Expensify/App/assets/38435837/19324436-532e-4869-99f7-aee4e247de92

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0149255b2b8a3385dc
  • Upwork Job ID: 1805204830810844307
  • Last Price Increase: 2024-06-24
Issue OwnerCurrent Issue Owner: @allroundexperts
melvin-bot[bot] commented 2 weeks ago

Triggered auto assignment to @CortneyOfstad (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[bot] commented 2 weeks ago

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

github-actions[bot] commented 2 weeks ago

:wave: 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 commented 2 weeks ago

@CortneyOfstad 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 commented 2 weeks ago

We think that this bug might be related to #wave-collect - Release 1

melvin-bot[bot] commented 2 weeks ago

Job added to Upwork: https://www.upwork.com/jobs/~0149255b2b8a3385dc

melvin-bot[bot] commented 2 weeks ago

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

mountiny commented 1 week ago

@bernhardoj is your fix going to work here too?

mountiny commented 1 week ago

I think we can demote this issue at this point. Its not blocking the user from using the app further, its quite specific edge case too

mountiny commented 1 week ago

Asked @cdOut to look into this as he worked on the details revamp

bernhardoj commented 1 week ago

@mountiny sorry, which one?

yassinezaanouni commented 1 week ago

I fixed the issue. I came from Upwork. Should I submit the proposal, or is there no need to now?

melvin-bot[bot] commented 1 week ago

๐Ÿ“ฃ @yassinezaanouni! ๐Ÿ“ฃ Hey, it seems we donโ€™t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork. Please follow these steps:

  1. Make sure you've read and understood the contributing guidelines.
  2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details. Screen Shot 2022-11-16 at 4 42 54 PM Format:
    Contributor details
    Your Expensify account email: <REPLACE EMAIL HERE>
    Upwork Profile Link: <REPLACE LINK HERE>
yassinezaanouni commented 1 week ago

Contributor details Your Expensify account email: yassinezaanouni@gmail.com Upwork Profile Link: https://www.upwork.com/freelancers/~01b7c13603c181bfe7

melvin-bot[bot] commented 1 week ago

โœ… Contributor details stored successfully. Thank you for contributing to Expensify!

yassinezaanouni commented 1 week ago

@mountiny I fixed the issue. I came from Upwork. Should I submit the proposal, or is there no need to now?

kosmydel commented 1 week ago

The root cause might be the same as in this issue.

yassinezaanouni commented 1 week ago

I submitted a proposal on Upwork outlining how to fix this. I have already fixed it locally and included a video in the proposal showcasing the app working as expected.

kosmydel commented 1 week ago

Hey, I'm from SWM. I've found a possible solution. I need to consult it with navigation colleagues. I will update you tomorrow as they are already OOO.

yassinezaanouni commented 1 week ago

Proposal

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

When a user deletes a Transaction and subsequently presses the back button, the app shows "page is not here" error.

What is the root cause of that problem?

When the user deletes a Transaction, they are navigated to the page of the deleted item, resulting in a "page is not here" error.

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

We will remove the deleted Transaction's page from the routes stack list.

๐—œ๐˜๐—ฒ๐—บ ๐——๐—ฒ๐—น๐—ฒ๐˜๐—ถ๐—ผ๐—ป: Make the necessary changes on transaction deletion.

๐—”๐—ฐ๐—ฐ๐—ฒ๐˜€๐˜€ ๐—ก๐—ฎ๐˜ƒ๐—ถ๐—ด๐—ฎ๐˜๐—ถ๐—ผ๐—ป ๐—ฆ๐˜๐—ฎ๐—ฐ๐—ธ: Utilize the navigation library's API to access and manipulate the navigation stack.

๐—ฅ๐—ฒ๐—บ๐—ผ๐˜ƒ๐—ฒ ๐——๐—ฒ๐—น๐—ฒ๐˜๐—ฒ๐—ฑ ๐—ฃ๐—ฎ๐—ด๐—ฒ ๐—ณ๐—ฟ๐—ผ๐—บ ๐—ฆ๐˜๐—ฎ๐—ฐ๐—ธ: Identify and remove the page corresponding to the deleted item from the navigation stack.

๐—จ๐—ฝ๐—ฑ๐—ฎ๐˜๐—ฒ ๐—ก๐—ฎ๐˜ƒ๐—ถ๐—ด๐—ฎ๐˜๐—ถ๐—ผ๐—ป ๐—ฆ๐˜๐—ฎ๐˜๐—ฒ: Apply the updated navigation stack state using the navigation library's state management functions to ensure the deleted page is no longer accessible.

kosmydel commented 1 week ago

I've found a possible fix similar to this one. But after a discussion with @WojtekBoman we've concluded that it might require changes in the goBack function of Navigation.

@mountiny what is the priority of it? If needed, I can instantly prepare a fix; later, the navigation team will prepare a more generic solution.

mountiny commented 1 week ago

Depends on what other tasks you have lined up, I think it would be great to get a PR merged for this by EOW

yassinezaanouni commented 1 week ago

Depends on what other tasks you have lined up, I think it would be great to get a PR merged for this by EOW

Hey I'd like to know why my proposal is not getting considered?

kosmydel commented 1 week ago

the updated navigation stack state using the navigation library's state management functions to ensure the deleted page is no longer accessible.

Hey @yassinezaanouni, initially I with @cdOut was working on this change which caused this bug. That's why we are investigating it.

Moreover, your proposal is IMO too general and doesn't mention the detailed root cause and solution.

yassinezaanouni commented 1 week ago

@kosmydel Ok thank you, next time I'll try to be more specific.

kosmydel commented 1 week ago

@mountiny I've prepared a fix as the navigation team (Adam & Wojtek) is busy now, but they will probably need to fix it using a more general approach as a follow-up (fixing the goBack function), as I don't have enough context here.

mountiny commented 1 week ago

Nice, thanks @kosmydel! Can you please write more description for the fix and prepare the PR body/ checklist?

kosmydel commented 1 week ago

Nice, thanks @kosmydel! Can you please write more description for the fix and prepare the PR body/ checklist?

I've added more details in the PR, and filled in the checklist