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 #26498][$1000] Chat - It's not here" displayed when click on back bin after all request money was deleted #24490

Closed
4 of 6 tasks
lanitochka17 opened this issue Aug 12, 2023 · 68 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Monthly KSv2

Comments

@lanitochka17
Copy link

lanitochka17 commented Aug 12, 2023

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Action Performed:

Precondition: user should be logged in

  1. Open app
  2. Tap on any 1:1 conversation with user
  3. Make a two request money in a row
  4. Tap on the requested money fiend in chat
  5. Tap on the first requested money
  6. Tap on 3 dots button > delete
  7. Tap on back button
  8. Tap on the remaining requested money
  9. Tap on 3 dots button > delete
  10. Tap on back button

Expected Result:

User should be navigated back to the DM

Actual Result:

Step 8: has navigated to the same page
Step 10: user has navigated to the "It's not here" page

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

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

Notes/Photos/Videos: Any additional supporting documentation

Bug6162479_screen-20230812-175610.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01c14c4b05f0985c77
  • Upwork Job ID: 1694085407589380096
  • Last Price Increase: 2023-08-29
@lanitochka17 lanitochka17 added the DeployBlockerCash This issue or pull request should block deployment label Aug 12, 2023
@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.

@github-actions github-actions bot added the Hourly KSv2 label Aug 12, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 12, 2023

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

@tymatein
Copy link

Proposal

Please re-state the problem that we are trying to solve in this issue.
When deleting the requested money, the navigation is set incorrectly.

What is the root cause of that problem?
The first problem with the duplicate screen is caused by redundant navigation to the report route with reportID.
The second problem with the error screen is caused by the omission of the second call to the goBack function. This leaves an invalid ID in the navigation stack.

What changes do you think we should make in order to solve the problem?
Make additional call of Navigation.goBack if needed and avoid useless navigation to avoid redundancy in the navigation stack.

diff --git a/src/libs/actions/IOU.js b/src/libs/actions/IOU.js
index 5660bc31a8..65357e22a8 100644
--- a/src/libs/actions/IOU.js
+++ b/src/libs/actions/IOU.js
@@ -965,14 +965,21 @@ function deleteMoneyRequest(transactionID, reportAction, isSingleTransactionView
     if (isSingleTransactionView && shouldDeleteTransactionThread && !shouldDeleteIOUReport) {
         // Pop the deleted report screen before navigating. This prevents navigating to the Concierge chat due to the missing report.
         Navigation.goBack();
-        Navigation.navigate(ROUTES.getReportRoute(iouReport.reportID));
+        if (!Navigation.isActiveRoute(ROUTES.getReportRoute(iouReport.reportID))) {
+            Navigation.navigate(ROUTES.getReportRoute(iouReport.reportID));
+        }
         return;
     }
 
     if (shouldDeleteIOUReport) {
         // Pop the deleted report screen before navigating. This prevents navigating to the Concierge chat due to the missing report.
         Navigation.goBack();
-        Navigation.navigate(ROUTES.getReportRoute(iouReport.chatReportID));
+        if (Navigation.isActiveRoute(ROUTES.getReportRoute(iouReport.reportID))) {
+            Navigation.goBack();
+        }
+        if (!Navigation.isActiveRoute(ROUTES.getReportRoute(iouReport.chatReportID))) {
+            Navigation.navigate(ROUTES.getReportRoute(iouReport.chatReportID));
+        }
     }
 }

What alternative solutions did you explore? (Optional)
No one.
Introduced by commit dc343dd. As that commit is revert of the PR 22787 it would be safer to just fix this issue than do revert.

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

@melvin-bot melvin-bot bot added Daily KSv2 and removed Hourly KSv2 labels Aug 13, 2023
@luacmartins
Copy link
Contributor

This is not a blocker. It's been the case since we introduced the feature.

@ShogunFire
Copy link
Contributor

ShogunFire commented Aug 14, 2023

Proposal

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

The navigation is wrong when we delete iou transaction

What is the root cause of that problem?

When we delete a transaction in single transaction view we navigate to the iou report and return but we should not return if the iou is also deleted because we need to navigate to the chat report

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

When we delete a transaction from the transaction view we should remove the test on shouldDeleteIOUReport because we have to goBack in every cases, and we need to remove the return so that it goes back to the chat report if we delete the iou report.

if (isSingleTransactionView && shouldDeleteTransactionThread) {

    Navigation.goBack(ROUTES.getReportRoute(iouReport.reportID));

}
if (shouldDeleteIOUReport) {
    Navigation.goBack(ROUTES.getReportRoute(iouReport.chatReportID));
}

Note: we add the fallback route so that it also works when we deeplink but the goback with fallback route doesn't work at the moment for reports and this will only work when this PR got merged: #26498

@luacmartins luacmartins removed the DeployBlockerCash This issue or pull request should block deployment label Aug 14, 2023
@melvin-bot melvin-bot bot added the Overdue label Aug 21, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 22, 2023

@grgia 8 days overdue is a lot. Should this be a Weekly issue? If so, feel free to change it!

@grgia
Copy link
Contributor

grgia commented Aug 22, 2023

@luacmartins is this a bug or something that is going to be cleaned up internally? Or can I make this external

@melvin-bot melvin-bot bot removed the Overdue label Aug 22, 2023
@luacmartins
Copy link
Contributor

You can make this external

@luacmartins luacmartins added External Added to denote the issue can be worked on by a contributor Bug Something is broken. Auto assigns a BugZero manager. labels Aug 22, 2023
@melvin-bot melvin-bot bot changed the title Chat - It's not here" displayed when click on back bin after all request money was deleted [$1000] Chat - It's not here" displayed when click on back bin after all request money was deleted Aug 22, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 22, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Aug 22, 2023

Job added to Upwork: https://www.upwork.com/jobs/~01c14c4b05f0985c77

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 22, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 22, 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

@melvin-bot
Copy link

melvin-bot bot commented Aug 22, 2023

Triggered auto assignment to @sophiepintoraetz (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@melvin-bot
Copy link

melvin-bot bot commented Aug 22, 2023

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

@adelekennedy adelekennedy changed the title [$1000] Chat - It's not here" displayed when click on back bin after all request money was deleted [HOLD for #26498][$1000] Chat - It's not here" displayed when click on back bin after all request money was deleted Sep 4, 2023
@melvin-bot melvin-bot bot added the Overdue label Sep 11, 2023
@adelekennedy
Copy link

on hold still

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Sep 11, 2023
@adelekennedy
Copy link

still holding

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Sep 14, 2023
@adelekennedy
Copy link

@mollfpr is this still on hold?

@melvin-bot melvin-bot bot removed the Overdue label Sep 18, 2023
@mollfpr
Copy link
Contributor

mollfpr commented Sep 18, 2023

@adelekennedy Yes!

@melvin-bot melvin-bot bot added the Overdue label Sep 21, 2023
@adelekennedy adelekennedy added Monthly KSv2 and removed Daily KSv2 labels Sep 21, 2023
@melvin-bot melvin-bot bot removed the Overdue label Sep 21, 2023
@adelekennedy
Copy link

deprioritizing for the hold

@melvin-bot melvin-bot bot added the Overdue label Oct 23, 2023
@adelekennedy
Copy link

I believe this is still on hold!

@melvin-bot melvin-bot bot removed the Overdue label Oct 24, 2023
@melvin-bot melvin-bot bot added the Overdue label Nov 24, 2023
@adelekennedy
Copy link

@mollfpr it looks like the connected PR was deployed a bit ago - are we still on hold here?

@melvin-bot melvin-bot bot removed the Overdue label Nov 26, 2023
@mollfpr
Copy link
Contributor

mollfpr commented Dec 19, 2023

I can't reproduce the issue on 1.4.14-0 Develop.

@melvin-bot melvin-bot bot added the Overdue label Jan 19, 2024
@adelekennedy
Copy link

hmm - I also can't reproduce @lanitochka17 will you see if you're still running into this?

@melvin-bot melvin-bot bot removed the Overdue label Jan 19, 2024
@melvin-bot melvin-bot bot added the Overdue label Feb 20, 2024
@adelekennedy
Copy link

I still can't reproduce on IOS, closing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Monthly KSv2
Projects
None yet
Development

No branches or pull requests