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

[$250] iOS - IOU - "Not here" error appears briefly after deleting expense #50085

Closed
1 of 6 tasks
lanitochka17 opened this issue Oct 2, 2024 · 52 comments
Closed
1 of 6 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 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

Comments

@lanitochka17
Copy link

lanitochka17 commented Oct 2, 2024

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: 9.0.43-1
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause - Internal Team

Issue found when executing PR #49878

Action Performed:

  1. Open the app
  2. Log in with a Gmail account
  3. Create a new workspace
  4. Open the workspace chat
  5. Submit a manual expense with any amount and merchant
  6. Open the expense
  7. Set the "Date" to a future one
  8. Tap on the expense header
  9. Delete the expense

Expected Result:

I should be able to delete it without any errors

Actual Result:

"Not here" error appears briefly after chenging expense date and deleting it after

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

Add any screenshot/video evidence

Bug6622222_1727881924347.HABH7257.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021841519823485482301
  • Upwork Job ID: 1841519823485482301
  • Last Price Increase: 2024-10-30
Issue OwnerCurrent Issue Owner: @getusha
@lanitochka17 lanitochka17 added the DeployBlockerCash This issue or pull request should block deployment label Oct 2, 2024
Copy link

melvin-bot bot commented Oct 2, 2024

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

Copy link
Contributor

github-actions bot commented Oct 2, 2024

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

@tgolen tgolen added Daily KSv2 and removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 labels Oct 2, 2024
@tgolen
Copy link
Contributor

tgolen commented Oct 2, 2024

I don't think this needs to be a blocker. I cannot find anything listed in the deploy checklist that stands out to me, so I am going to move this to be external and get some help trying to find what caused it and a solution.

@tgolen tgolen added the External Added to denote the issue can be worked on by a contributor label Oct 2, 2024
@melvin-bot melvin-bot bot changed the title iOS - IOU - "Not here" error appears briefly after deleting expense [$250] iOS - IOU - "Not here" error appears briefly after deleting expense Oct 2, 2024
Copy link

melvin-bot bot commented Oct 2, 2024

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

@tgolen tgolen added the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 2, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 2, 2024
Copy link

melvin-bot bot commented Oct 2, 2024

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

@tgolen tgolen added the Bug Something is broken. Auto assigns a BugZero manager. label Oct 2, 2024
Copy link

melvin-bot bot commented Oct 2, 2024

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

@huult
Copy link
Contributor

huult commented Oct 3, 2024

Edited by proposal-police: This proposal was edited at 2024-10-03 04:45:01 UTC.

Proposal

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

"Not here" error appears briefly after deleting expense

What is the root cause of that problem?

After deleting the expense, this report will be set to null.

const deleteTransaction = useCallback(() => {

And after calling the delete expense function, it will navigate back to the previous screen, where the report returns undefined because it was set in the IOU.deleteMoneyRequest function. As a result, the 'not here' page appears.

useEffect(() => {
if (!report?.reportID || shouldHideReport) {
wasReportAccessibleRef.current = false;
return;
}
wasReportAccessibleRef.current = true;
}, [shouldHideReport, report]);

The condition shows the 'Not here' page in this issue

if (!wasReportAccessibleRef.current && !firstRenderRef.current && !reportID && !isOptimisticDelete && !reportMetadata?.isLoadingInitialReportActions && !userLeavingStatus) {

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

We prevent updating wasReportAccessibleRef after completing the navigation back to avoid showing the 'Not Here' page, something like this:

// .src/pages/home/ReportScreen.tsx#L270
    useEffect(() => {
+        InteractionManager.runAfterInteractions(() => {
+            requestAnimationFrame(() => {
                if (!report?.reportID || shouldHideReport) {
                    wasReportAccessibleRef.current = false;
                    return;
                }
                wasReportAccessibleRef.current = true;
+            });
+        });
    }, [shouldHideReport, report]);

Or we could delay navigating back in onModalHide

POC
Screen.Recording.2024-10-03.at.11.16.58.mov

REPRODUCED THIS ISSUE:

Screen.Recording.2024-10-03.at.11.18.21.mov

@mkzie2
Copy link
Contributor

mkzie2 commented Oct 3, 2024

Proposal

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

"Not here" error appears briefly after chenging expense date and deleting it after

What is the root cause of that problem?

  • The issue ""Hmm, not there" briefly is shown while navigating to the IOU preview" was already fixed in PR.

  • But after PR, we encountered that bug again. It is because of:

wasReportAccessibleRef.current = false;

where we set wasReportAccessibleRef.current to false althought it was true before.

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

  • We should remove:

wasReportAccessibleRef.current = false;

  • I tested it, and after removing the mentioned line, the bug that the line wasReportAccessibleRef.current = false was intended to fix is no longer present.

What alternative solutions did you explore? (Optional)

@isabelastisser
Copy link
Contributor

@getusha, please review the proposals above. Thanks!

@getusha
Copy link
Contributor

getusha commented Oct 6, 2024

@mkzie2 i am still able to reproduce the issue after applying your solution

Screen.Recording.2024-10-06.at.8.27.54.in.the.morning.mov

@getusha
Copy link
Contributor

getusha commented Oct 6, 2024

@huult your solution works well, can we pin point why this issue resurfaced? perhaps #36477 caused this?

@mkzie2
Copy link
Contributor

mkzie2 commented Oct 6, 2024

@getusha Can you provide steps to reproduce the bug, or try refreshing your browser to ensure the changes are applied? I tested the solution on my end, and it works as expected. Here’s a slowed-down version to demonstrate: after deleting an expense, the skeleton page appears instead of the 'Not here' page:

Screen.Recording.2024-10-06.at.13.18.10.mov

@huult
Copy link
Contributor

huult commented Oct 6, 2024

Hi @getusha ,
Yes, I tried reverting the PR, but the issue didn't occur as a result.

POC
Screen.Recording.2024-10-06.at.21.28.53.mov

@mkzie2
Copy link
Contributor

mkzie2 commented Oct 6, 2024

@getusha Just a heads-up, my solution effectively reverts the PR that caused this bug. Someone had flagged the issue in this comment, but C+ couldn't reproduce it at the time.

@isabelastisser
Copy link
Contributor

@getusha, can you please provide an update? Thanks!

Copy link

melvin-bot bot commented Oct 18, 2024

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

@melvin-bot melvin-bot bot added the Overdue label Oct 18, 2024
@getusha
Copy link
Contributor

getusha commented Oct 20, 2024

@getusha , Can you stop the Metro connection and run it again?

Yes, i'll get a ton of issues.

@melvin-bot melvin-bot bot removed the Overdue label Oct 20, 2024
@getusha
Copy link
Contributor

getusha commented Oct 22, 2024

report becomes undefined for a sec. i think it's happening because of useOnyx migration.
Not entirely sure but i remember similar issue.

@getusha
Copy link
Contributor

getusha commented Oct 22, 2024

@mkzie2 your solution seems to work, have you tested the related issues?

@mkzie2
Copy link
Contributor

mkzie2 commented Oct 22, 2024

the related issues

What do you mean this issue, right?

@getusha
Copy link
Contributor

getusha commented Oct 22, 2024

@mkzie2 After applying your solution this page appears briefly, any way to fix this?

@huult
Copy link
Contributor

huult commented Oct 22, 2024

@getusha Does my solution cause the issue mentioned above?

@getusha
Copy link
Contributor

getusha commented Oct 22, 2024

@huult not caused, seems like an existing issue when there is only one expense.

@mkzie2
Copy link
Contributor

mkzie2 commented Oct 22, 2024

After applying your solution this page appears briefly, any way to fix this?

Can you give me a step to reproduce it?

@getusha
Copy link
Contributor

getusha commented Oct 22, 2024

@mkzie2

Screen.Recording.2024-10-22.at.1.54.28.in.the.afternoon.mov

@mkzie2
Copy link
Contributor

mkzie2 commented Oct 22, 2024

The bug in this issue is that the "Not here" appears briefly after deleting expense. To address the issue, we don't display the "Not here" page by applying solution but it will introduce the behavior. @tgolen Do you think it is bug? If so, what should be displayed in this case? cc @getusha

@huult
Copy link
Contributor

huult commented Oct 22, 2024

I believe we should consider keeping the old logic here because:

  • It may introduce a new bug.
  • After removing it, we are unable to test all cases related to it

Additionally, I think using my solution is a safer option and will effectively resolve this issue

cc: @getusha

@mkzie2
Copy link
Contributor

mkzie2 commented Oct 23, 2024

After applying your solution this page appears briefly, any way to fix this?

@getusha I saw this issue in the latest main as well, so I think it is not a regression from my solution.

Copy link

melvin-bot bot commented Oct 23, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@isabelastisser
Copy link
Contributor

@Getush, can you please follow up on the comments above? Thanks!

@melvin-bot melvin-bot bot added the Overdue label Oct 25, 2024
Copy link

melvin-bot bot commented Oct 25, 2024

@tgolen, @isabelastisser, @getusha Whoops! This issue is 2 days overdue. Let's get this updated quick!

@getusha
Copy link
Contributor

getusha commented Oct 28, 2024

@getusha I saw this issue in the latest main as well, so I think it is not a regression from my solution.

@mkzie2 It makes sense to include that fix here as well if possible, the issue is similar

@melvin-bot melvin-bot bot removed the Overdue label Oct 28, 2024
Copy link

melvin-bot bot commented Oct 30, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@melvin-bot melvin-bot bot added the Overdue label Oct 30, 2024
Copy link

melvin-bot bot commented Oct 30, 2024

@tgolen @isabelastisser @getusha this issue is now 4 weeks old, please consider:

  • Finding a contributor to fix the bug
  • Closing the issue if BZ has been unable to add the issue to a VIP or Wave project
  • If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

Copy link

melvin-bot bot commented Oct 31, 2024

@tgolen, @isabelastisser, @getusha Whoops! This issue is 2 days overdue. Let's get this updated quick!

@wildan-m
Copy link
Contributor

wildan-m commented Nov 1, 2024

I think it's duplicate #45576

@tgolen
Copy link
Contributor

tgolen commented Nov 1, 2024

Ah, it looks like you're right. Thanks! I'll close this out.

@tgolen tgolen closed this as not planned Won't fix, can't repro, duplicate, stale Nov 1, 2024
@melvin-bot melvin-bot bot removed the Overdue label Nov 1, 2024
Copy link

melvin-bot bot commented Nov 1, 2024

@tgolen @isabelastisser Be sure to fill out the Contact List!

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. Daily KSv2 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
Projects
None yet
Development

No branches or pull requests

7 participants