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.29k stars 2.72k forks source link

[$250] Violation on pending Expensify Card expense isn't showing RBR in LHN. #45039

Closed m-natarajan closed 1 month ago

m-natarajan commented 1 month 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: Reproducible in staging?: Needs reproduction (Cannot create Expensify card expense) Reproducible in production?: Needs reproduction 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: @puneetlath Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1720474065535179

Action Performed:

  1. Create an one-transaction report pending Expensify Card expense with a violation
  2. As employee submit an one-transaction Expensify Card expense with a violation
  3. Observe the LHN

    Expected Result:

    The LHN displays the report with RBR

    Actual Result:

    RBR not displayed in LHN

    Workaround:

    Unknown

    Platforms:

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

    • [ ] Android: Native
    • [ ] Android: mWeb Chrome
    • [ ] iOS: Native
    • [ ] iOS: mWeb Safari
    • [X] MacOS: Chrome / Safari
    • [ ] MacOS: Desktop

Screenshots/Videos

image (4)

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0187b792f69a4da7ea
  • Upwork Job ID: 1811378378226493917
  • Last Price Increase: 2024-07-25
Issue OwnerCurrent Issue Owner: @situchan
MelvinBot commented 1 month ago

This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989

melvin-bot[bot] commented 1 month ago

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

HaleyJacobson commented 1 month ago

I am experiencing the same issue - A pending expense with a violation that is not being pinned in the LHN with an RBR

2024-07-09_11-03-34 2024-07-09_11-03-34 2024-07-09_11-16-23
melvin-bot[bot] commented 1 month ago

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

melvin-bot[bot] commented 1 month ago

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

nkdengineer commented 1 month ago

Proposal

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

Violation on pending Expensify Card expense isn't showing RBR in LHN

What is the root cause of that problem?

In https://github.com/Expensify/App/blob/105dce5151053bf6c81e60e68a610dd888c5da45/src/libs/ReportUtils.ts#L5362, we only check the violations of the current report.

In case the report is an IOU report and is one transaction report, the IOU report/parentReportAction itself will not have the violations, only the transaction thread has.

So we're missed out on the transaction thread violations

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

In https://github.com/Expensify/App/blob/105dce5151053bf6c81e60e68a610dd888c5da45/src/libs/ReportUtils.ts#L5362, check whether the report is the one transaction report (can use isOneTransactionReport).

If so, check the shouldDisplayTransactionThreadViolations eligibility of the report/parentReportAction associated with the transaction thread of that one-transaction report too (or only check the transaction thread in this case). The transaction thread id can easily be obtained from getOneTransactionThreadReportID, from there we can get the report and parent report action to check. Alternatively, we can get those transaction thread data from here, use it as params of OptionRowLHNData and add another check for shouldDisplayTransactionThreadViolations for the transaction thread report here.

What alternative solutions did you explore? (Optional)

We can check again other functions that are validating the transaction's condition, such as doesTransactionThreadHaveViolations and can add the one transaction thread logic above to it.

Another alternative: In https://github.com/Expensify/App/blob/105dce5151053bf6c81e60e68a610dd888c5da45/src/components/LHNOptionsList/OptionRowLHNData.tsx#L38, check whether the report is an IOU report. If so, get all transaction threads of that IOU report and check shouldDisplayTransactionThreadViolations on them. So as long as any transaction thread has violations, we'll show RBR in the IOU report item in LHN.

nkdengineer commented 1 month ago

Proposal updated to add alternative

situchan commented 1 month ago

@nkdengineer can you share repro step (full reproduction video is preferred) without creating Expensify card expense?

melvin-bot[bot] commented 1 month ago

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

slafortune commented 1 month ago

@nkdengineer can you give an update on this?

nkdengineer commented 1 month ago

Sure, let me check and update tomorrow

melvin-bot[bot] commented 1 month ago

@slafortune, @situchan Huh... This is 4 days overdue. Who can take care of this?

situchan commented 1 month ago

Not overdue

melvin-bot[bot] commented 1 month ago

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

nkdengineer commented 1 month ago

I cannot reproduce this any more, perhaps some other changes fixed it. I think we can add Retest label.

situchan commented 1 month ago

@puneetlath as a bug reporter, can you please help re-test?

melvin-bot[bot] commented 1 month ago

@slafortune @situchan this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

melvin-bot[bot] commented 1 month ago

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

puneetlath commented 1 month ago

Yep, I will try it next time I have a card transaction.

melvin-bot[bot] commented 1 month ago

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

slafortune commented 1 month ago

Not overdue - waiting on a retest 👍

puneetlath commented 1 month ago

Looks like it's working!

Screenshot 2024-07-25 at 4 12 53 PM Screenshot 2024-07-25 at 4 12 59 PM