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 2024-06-28] [$250] Track expenses - Unavailable workspace shown &app crashes if select share it with my accountant #43260

Closed
2 of 6 tasks
izarutskaya opened this issue Jun 7, 2024 · 19 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Weekly KSv2

Comments

@izarutskaya
Copy link

izarutskaya commented Jun 7, 2024

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: 1.4.80-9
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4605669
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

  1. Open the app and log in with a new account
  2. Tap FAB > Track Expenses
  3. Enter the amount and complete the expense creation
  4. Navigate to self DM
  5. Select "Share it with my accountant" in actionable whisper
  6. On confirmation page tap Submit

Expected Result:

The new workspace name displayed in on the confirmation page and user is able to proceed to the participant page

Actual Result:

The Unavailable workspace is displayed on the confirmation page and the app crashes after tapping "Submit"

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

Bug6504667_1717741958847.IMG_7347.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~016f7ddb2eef36b28f
  • Upwork Job ID: 1799111618417504843
  • Last Price Increase: 2024-06-07
  • Automatic offers:
    • akinwale | Reviewer | 102686818
    • dominictb | Contributor | 102686821
Issue OwnerCurrent Issue Owner: @garrettmknight
@izarutskaya izarutskaya added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jun 7, 2024
Copy link

melvin-bot bot commented Jun 7, 2024

Triggered auto assignment to @garrettmknight (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
Copy link
Author

We think this issue might be related to the #collect project.

@garrettmknight garrettmknight added the External Added to denote the issue can be worked on by a contributor label Jun 7, 2024
@melvin-bot melvin-bot bot changed the title Track expenses - Unavailable workspace shown &app crashes if select share it with my accountant [$250] Track expenses - Unavailable workspace shown &app crashes if select share it with my accountant Jun 7, 2024
Copy link

melvin-bot bot commented Jun 7, 2024

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

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

melvin-bot bot commented Jun 7, 2024

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

@dominictb
Copy link
Contributor

dominictb commented Jun 7, 2024

Proposal

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

The Unavailable workspace is displayed on the confirmation page and the app crashes after tapping "Submit"

What is the root cause of that problem?

In the IOU confirmation screen here, the app is using the withWritableReportOrNotFound

In withWritableReportOrNotFound, there's code to call openReport if the report is empty and the reportID exists in the param

ReportActions.openReport(route.params.reportID);

The issue here is that the report in this case is a draft report, so when calling openReport, it will return Not found in the report data, this corrupted data causes error when the user tries confirm the expense.

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

In

if (!!report?.reportID || !route.params.reportID) {
, also early return if the report exists in draft, by checking the reportDraft

if (!!report?.reportID || !route.params.reportID || !!reportDraft) {
    return;
}

This will not lead to any issue because if the reportID exists in reportDraft, it for sure doesn't exist in back-end. Because it's only draft, if it was successfully created, the draft would've been cleared already. So there's no reason to call openReport on it.

What alternative solutions did you explore? (Optional)

We can attempt to put reportDraft to the dependency list of that useEffect, but I don't think it's necessary.

There're other ways to implement the draft checking like:

  • After creating the draft report, when navigating to the confirmation page, set the param isDraft=true, and check that param in withWritableReportOrNotFound

Copy link

melvin-bot bot commented Jun 10, 2024

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

@melvin-bot melvin-bot bot added the Overdue label Jun 10, 2024
@garrettmknight
Copy link
Contributor

@akinwale mind giving this proposal a review?

@akinwale
Copy link
Contributor

We can move forward with @dominictb's proposal here.

🎀👀🎀 C+ reviewed.

@melvin-bot melvin-bot bot removed the Overdue label Jun 11, 2024
Copy link

melvin-bot bot commented Jun 11, 2024

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

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Jun 11, 2024
Copy link

melvin-bot bot commented Jun 11, 2024

📣 @akinwale 🎉 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

Copy link

melvin-bot bot commented Jun 11, 2024

📣 @dominictb 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

@amyevans
Copy link
Contributor

I'm unassigning since I'm headed out on extended leave after today. When the PR is approved Melvin should auto-assign a new internal engineer to review it.

@amyevans amyevans removed their assignment Jun 14, 2024
Copy link

melvin-bot bot commented Jun 16, 2024

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

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Jun 21, 2024
@melvin-bot melvin-bot bot changed the title [$250] Track expenses - Unavailable workspace shown &app crashes if select share it with my accountant [HOLD for payment 2024-06-28] [$250] Track expenses - Unavailable workspace shown &app crashes if select share it with my accountant Jun 21, 2024
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Jun 21, 2024
Copy link

melvin-bot bot commented Jun 21, 2024

Reviewing label has been removed, please complete the "BugZero Checklist".

Copy link

melvin-bot bot commented Jun 21, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.85-7 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2024-06-28. 🎊

For reference, here are some details about the assignees on this issue:

Copy link

melvin-bot bot commented Jun 21, 2024

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

  • [@akinwale] The PR that introduced the bug has been identified. Link to the PR:
  • [@akinwale] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
  • [@akinwale] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
  • [@akinwale] Determine if we should create a regression test for this bug.
  • [@akinwale] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
  • [@garrettmknight] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jun 27, 2024
@garrettmknight
Copy link
Contributor

Payment summary:

@akinwale can you complete the checklist when you get a chance?

@garrettmknight garrettmknight added Weekly KSv2 and removed Daily KSv2 labels Jun 28, 2024
@akinwale
Copy link
Contributor

  • [@akinwale] The PR that introduced the bug has been identified. Link to the PR:
  • [@akinwale] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
  • [@akinwale] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:

Not a regression.

  • [@akinwale] Determine if we should create a regression test for this bug.
  • [@akinwale] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Regression Test Steps

  1. Launch Expensify.
  2. Login with a brand new account.
  3. Open the global create menu and select Track Expenses.
  4. Enter an amount and finish creating the expense.
  5. Navigate to the self-DM report.
  6. Select "Share it with my accountant" from the list of actions in the self-DM.
  7. Verify that the name of the new workspace (not Unavailable workspace) is displayed on the confirmation page.
  8. On the confirmation page, click or tap Submit.
  9. Verify that the app does not crash.

Do we agree 👍 or 👎?

@garrettmknight
Copy link
Contributor

All set

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. External Added to denote the issue can be worked on by a contributor Weekly KSv2
Projects
No open projects
Status: CRITICAL
Development

No branches or pull requests

6 participants