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-04-15] [$500] IOU - Unable to dismiss error message in combined report by clicking X button #39420

Closed
6 tasks done
lanitochka17 opened this issue Apr 2, 2024 · 27 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 Reviewing Has a PR in review Weekly KSv2

Comments

@lanitochka17
Copy link

lanitochka17 commented Apr 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: 1.4.59-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: N/A
Issue reported by: Applause - Internal Team

Action Performed:

  1. Go to staging.new.expensify.com
  2. [User A] Request money from User B (no unsettled IOU)
  3. [User A] Go to combined IOU report > Click Amount
  4. [User B] Pay the request
  5. [User A] Edit the amount
  6. [User A] Click X to dismiss the error message

Expected Result:

The error message is dismissed

Actual Result:

Unable to dismiss the error message by clicking X button

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

Bug6435672_1712068890949.20240402_223238.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01f33b86436968b4d0
  • Upwork Job ID: 1775220715733753856
  • Last Price Increase: 2024-04-02
@lanitochka17 lanitochka17 added the DeployBlockerCash This issue or pull request should block deployment label Apr 2, 2024
Copy link

melvin-bot bot commented Apr 2, 2024

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

Copy link
Contributor

github-actions bot commented Apr 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.

@lanitochka17
Copy link
Author

@yuwenmemon 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

@lanitochka17
Copy link
Author

We think that this bug might be related to #vip-vsp

@yuwenmemon
Copy link
Contributor

I wouldn't consider this a deploy blocker. Going to remove the label.

@yuwenmemon yuwenmemon added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. and removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 labels Apr 2, 2024
Copy link

melvin-bot bot commented Apr 2, 2024

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

@yuwenmemon
Copy link
Contributor

There also seem to be a lot of issues in that video. But I think the specific inability to close out an error with the 'X' button would be external.

@yuwenmemon yuwenmemon added the External Added to denote the issue can be worked on by a contributor label Apr 2, 2024
@melvin-bot melvin-bot bot changed the title IOU - Unable to dismiss error message in combined report by clicking X button [$500] IOU - Unable to dismiss error message in combined report by clicking X button Apr 2, 2024
Copy link

melvin-bot bot commented Apr 2, 2024

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

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

melvin-bot bot commented Apr 2, 2024

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

@nkdengineer
Copy link
Contributor

nkdengineer commented Apr 2, 2024

Proposal

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

Unable to dismiss the error message by clicking X button

What is the root cause of that problem?

After this PR #36934, all actions of transaction thread report are displayed in expense report if we only one transaction.

When we click on x button, the originalReportID is returned as expense report which makes the error isn't cleared because this action is still stored in transaction thread report in Onyx.

const originalReportID = ReportUtils.getOriginalReportID(reportID, reportAction);

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

We should check this case in getOriginalReportID, if the report is oneTransactionReport we will check if current action isn't present in the expense report we will return the transactionThreadReportID.

const reportActions = reportActionsByReport?.[reportID];
const currentReportAction = reportActions?.[reportAction?.reportActionID ?? ''] ?? null;
const transactionThreadReportID = ReportActionsUtils.getOneTransactionThreadReportID(reportActions ?? ([] as ReportAction[]));
if (transactionThreadReportID !== null) {
    return Object.keys(currentReportAction ?? {}).length === 0 ? transactionThreadReportID : reportID;
}
return isThreadFirstChat(reportAction, reportID) && Object.keys(currentReportAction ?? {}).length === 0
    ? allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]?.parentReportID
    : reportID;

function getOriginalReportID(reportID: string, reportAction: OnyxEntry<ReportAction>): string | undefined {

I also see that, we are getting reportActions wrong here makes isOneTransactionReport is always return false. We should fix this to use this function as well

const reportActions = reportActionsByReport?.[reportID] ?? ([] as ReportAction[]);

App/src/libs/ReportUtils.ts

Lines 1271 to 1274 in 5986481

function isOneTransactionReport(reportID: string): boolean {
const reportActions = reportActionsByReport?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`] ?? ([] as ReportAction[]);
return ReportActionsUtils.getOneTransactionThreadReportID(reportActions) !== null;
}

What alternative solutions did you explore? (Optional)

NA

@yuwenmemon
Copy link
Contributor

@NikkiWines I see there were several bugs from #36934 - are there plans to revert? Or does the above solution look sufficient?

@NikkiWines
Copy link
Contributor

no plans to revert, handling them individually as they pop up. Looking at the proposal now

@NikkiWines
Copy link
Contributor

@nkdengineer's proposal looks good 👍

@NikkiWines
Copy link
Contributor

In the interest of getting this resolved quickly going to assign @nkdengineer

@nkdengineer
Copy link
Contributor

Will raise the PR in a few hours.

@nkdengineer
Copy link
Contributor

@Santhosh-Sellavel The PR is here cc @NikkiWines.

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Apr 8, 2024
@melvin-bot melvin-bot bot changed the title [$500] IOU - Unable to dismiss error message in combined report by clicking X button [HOLD for payment 2024-04-15] [$500] IOU - Unable to dismiss error message in combined report by clicking X button Apr 8, 2024
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Apr 8, 2024
Copy link

melvin-bot bot commented Apr 8, 2024

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

Copy link

melvin-bot bot commented Apr 8, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.60-13 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-04-15. 🎊

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

Copy link

melvin-bot bot commented Apr 8, 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:

  • [@Santhosh-Sellavel] The PR that introduced the bug has been identified. Link to the PR:
  • [@Santhosh-Sellavel] 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:
  • [@Santhosh-Sellavel] 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:
  • [@Santhosh-Sellavel] Determine if we should create a regression test for this bug.
  • [@Santhosh-Sellavel] 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.
  • [@adelekennedy] 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 Apr 14, 2024
@adelekennedy adelekennedy removed the Awaiting Payment Auto-added when associated PR is deployed to production label Apr 15, 2024
Copy link

melvin-bot bot commented Apr 15, 2024

Payment Summary

Upwork Job

BugZero Checklist (@adelekennedy)

  • I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
  • I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants/1775220715733753856/hired)
  • I have paid out the Upwork contracts or cancelled the ones that are incorrect
  • I have verified the payment summary above is correct

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Apr 17, 2024
@adelekennedy
Copy link

ah - I accidentally invite dyou to apply to the job @nkdengineer I just sent you a manual offer. Once you accept I'll end it and pay it out immediately

@Santhosh-Sellavel
Copy link
Collaborator

@adelekennedy
Not sure why payment was mentioned incorrectly in automated summary. May be we cut default fee to $250 from $500.
But this was completed before the change so I think payment should be $500.

So Requested $500 on New Dot!
cc: @JmillsExpensify

@JmillsExpensify
Copy link

Re-opening for payment summary

@adelekennedy
Copy link

ah sorry @Santhosh-Sellavel I just double checked the dates for these changes and this should be $500 @JmillsExpensify just updated the payment summary!

@JmillsExpensify
Copy link

$500 approved for @Santhosh-Sellavel

@nkdengineer
Copy link
Contributor

@JmillsExpensify We can close this issue now.

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 Reviewing Has a PR in review Weekly KSv2
Projects
None yet
Development

No branches or pull requests

7 participants