[HOLD for payment 2024-07-24] [$250] [One expense reports] Red dot on LHN does not appear for participant when create 1:1 hold expense

[lanitochka17]

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.83-0
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/4627347&group_by=cases:section_id&group_order=asc&group_id=309128
Email or phone of affected tester (no customers): gocemate+a271@gmail.com
Issue reported by: Applause - Internal Team

Action Performed:

Submitter

1. Create an IOU with several expenses in a 1:1 with another user you have access to
   Participant
2. Log in as the other user
3. Navigate to a expense, click the three dots in the header, and choose Hold
4. Leave a hold comment and confirm

Expected Result:

There should be a red dot on LHN on participant side

Actual Result:

Red dot on LHN does not appear for participant when create 1:1 hold expense

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

Bug6512986_1718358239458.Recording__3217.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0161264d9154f7a521
• Upwork Job ID: 1802728525946815546
• Last Price Increase: 2024-07-01
• Automatic offers:
• truph01 | Contributor | 103017602

Issue Owner

Current Issue Owner: @lschurr

[melvin-bot]

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

[lanitochka17]

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

[truph01]

Proposal

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

Red dot on LHN does not appear for participant when create 1:1 hold expense

What is the root cause of that problem?

• We early return false in this logic:
  

  App/src/libs/ReportUtils.ts

  Lines 5278 to 5280 in 75c104b

  	if (!isCurrentUserSubmitter(IOUReportID)) {
  	return false;
  	}

  
  if the current user is not submitter, that leads to there is no RBR displays for participant when creating 1:1 hold expense.

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

• With the new feature mentioned in this comment, we need to make two change:

1. Change the getOrderedReportIDs to make sure with the oneTransactionReport will be pinned if its transaction has violations.

• We need to update:
  

  App/src/libs/SidebarUtils.ts

  Lines 95 to 96 in 4622e19

  	const hasErrorsOtherThanFailedReceipt =
  	doesReportHaveViolations || Object.values(allReportErrors).some((error) => error?.[0] !== Localize.translateLocal('iou.error.genericSmartscanFailureMessage'));

  
  to:

        const transactionReportActions = ReportActionsUtils.getAllReportActions(report.reportID);
        const oneTransactionThreadReportID = ReportActionsUtils.getOneTransactionThreadReportID(report.reportID, transactionReportActions, undefined);
        let doesTransactionThreadReportHasViolations = false;
        if (oneTransactionThreadReportID) {
            const transactionReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${oneTransactionThreadReportID}`];
            doesTransactionThreadReportHasViolations = OptionsListUtils.shouldShowViolations(transactionReport, betas ?? [], transactionViolations);
        }
        const hasErrorsOtherThanFailedReceipt =
            doesTransactionThreadReportHasViolations ||
            doesReportHaveViolations ||
            Object.values(allReportErrors).some((error) => error?.[0] !== Localize.translateLocal('iou.error.genericSmartscanFailureMessage'));

2. Change the getOptionData function to make sure the RBR is displayed in oneTransactionReport in LHN if its transaction has violations.

• We need to update:
  

  App/src/libs/SidebarUtils.ts

  Line 278 in 4622e19

  result.brickRoadIndicator = hasErrors || hasViolations ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : '';

  
  to:

    result.brickRoadIndicator = hasErrors || hasViolations ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : '';
    const oneTransactionThreadReportID = ReportActionsUtils.getOneTransactionThreadReportID(report.reportID, ReportActionsUtils.getAllReportActions(report.reportID));
    if (oneTransactionThreadReportID) {
        const oneTransactionThreadReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${oneTransactionThreadReportID}`];
        if (
            Permissions.canUseViolations(betas) &&
            ReportUtils.shouldDisplayTransactionThreadViolations(
                oneTransactionThreadReport,
                transactionViolations,
                ReportActionsUtils.getAllReportActions(report.reportID)[oneTransactionThreadReport?.parentReportActionID ?? '-1'],
            )
        ) {
            result.brickRoadIndicator = CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR;
        }
    }

• The above changes are just the main changes, if need I can raise a draft branch for it.

What alternative solutions did you explore? (Optional)

• We can update this logic:
  

  App/src/libs/ReportUtils.ts

  Lines 5278 to 5280 in 75c104b

  	if (!isCurrentUserSubmitter(IOUReportID)) {
  	return false;
  	}

  
  to:

    const transaction = allTransactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${IOUTransactionID}`] ?? {};
    const isOnHold = TransactionUtils.isOnHold(transaction);
    if (!isCurrentUserSubmitter(IOUReportID) && !isOnHold) {
        return false;
    }

[melvin-bot]

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

[melvin-bot]

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

[Pujan92]

@lschurr Based on this comment it seems only for the submitter we are showing Red dot(RBR) in LHN. Would like to confirm the expected result here.
cc: @cead22

[cead22]

I don't know about HOLD, so tagging @JmillsExpensify

[melvin-bot]

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

[melvin-bot]

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

[lschurr]

@JmillsExpensify will you take a peek at this one?

[melvin-bot]

@Pujan92, @lschurr 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[trjExpensify]

We were discussing this in this thread.

It seems like we're broadly not adding the RBR to the LHN and pinning the "combined" one expense report when it contains violations, one of which is a held request.

So we want to fix that. When an expense report contains just one expense, the RBR for the violation (incl. hold) should be added to the report row in the LHN and it should be pinned - because that is the "lowest" context chat in this scenario.

The logic for who the RBR should be displayed to in the one expense report case should be exactly the same as the multi-expense report case.

[melvin-bot]

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

[lschurr]

Thanks @trjExpensify - @Pujan92 are you able to review?

[truph01]

Proposal updated

• I updated proposal to match the new feature.

[Pujan92]

Thanks @trjExpensify

The logic for who the RBR should be displayed to in the one expense report case should be exactly the same as the multi-expense report case.

At the moment the RBR in LHN is only displayed on the submitter side for the multi-expense specific report case, So I believe this task is all about showing the RBR in the LHN for one expense report case. @trjExpensify could you plz confirm once?

Mutli Expense(current behaviour of submitter side and then payer side)

Screen.Recording.2024-07-01.at.16.11.02.mov

One Expense(current behaviour of submitter side and then payer side)

Screen.Recording.2024-07-01.at.16.18.19.mov

[trjExpensify]

So I believe this task is all about showing the RBR in the LHN for one expense report case. @trjExpensify could you plz confirm once?

Correct. See below!

So we want to fix that. When an expense report contains just one expense, the RBR for the violation (incl. hold) should be added to the report row in the LHN and it should be pinned - because that is the "lowest" context chat in this scenario.

[truph01]

on updating the hold/unhold status for one expense report isn't getting rerenders in the sidebar.

I am still investigating it. But FYI, this bug also appears in the latest main.

[truph01]

on updating the hold/unhold status for one expense report isn't getting rerenders in the sidebar.

@Pujan92 Maybe it is a BE bug. When we hold any expense, there is no transactionViolations_ data returned from BE side.

[Pujan92]

But FYI, this bug also appears in the latest main.

Yes @truph01, it exists on main too.

[Pujan92]

We can proceed with point 2 of @truph01's proposal which is to check violation for the child report if it is a one-expense report.

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[robertjchen]

Thanks for the discussions and review, let's move forward with the @truph01 's proposal 👍

[melvin-bot]

📣 @truph01 🎉 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 📖

[truph01]

@Pujan92 PR #44810 is ready.

[melvin-bot]

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

[melvin-bot]

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

• Fix: Add red dot to one transaction reports #44810

If no regressions arise, payment will be issued on 2024-07-24. 🎊

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

• @Pujan92 does not require payment (Contractor)
• @truph01 requires payment automatic offer (Contributor)

[melvin-bot]

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:

• [@Pujan92] The PR that introduced the bug has been identified. Link to the PR:
• [@Pujan92] 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:
• [@Pujan92] 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:
• [@Pujan92] Determine if we should create a regression test for this bug.
• [@Pujan92] 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.
• [@lschurr] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[Pujan92]

We don't have an offending PR here as it is kind of a new thing for the one expense report.

Regression Test Steps

1. Submit a first expense for any of the participant
2. Hold that expense / Violate it by skipping required fields(category/tag..) if it is the policy expense chat
3. Verify the one expense report in the LHN has the RBR and is pinned.

Do you agree 👍 or 👎 ?

[lschurr]

Payment summary:

• Contributor: @truph01 $250 (paid in Upwork)
• Reviewer: @Pujan92 $250 (please request via NewDot)

[JmillsExpensify]

$250 approved for @Pujan92