[HIGH] Hold doesn't show in three dot menu until report is opened

[m-natarajan]

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.60-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?:
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: @garrettmknight
Slack conversation (hyperlinked to channel name): ts_external_expensify_expense

Action Performed:

1. Create a workspace
2. Invite Submitter + Approver A
3. Enable Workflows > Approval + Delay Submission
4. Set Approver A as approver
5. As Submitter, create expense in WS chat + submit
6. Tag the approver in the workspace chat and invite them
7. As Approver A, navigate to Submitter's WS chat + click the three dot menu on the expense

Expected Result:

Since this is a one expense report, you should see Hold in the three dot menu for the report

Actual Result:

The Hold action is not available in the menu until you open the expense and go back

Workaround:

You don't see Hold until you access the expense/report and back out into the WS chat again.

Platforms:

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

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

HoldMenu2024-12-12_15-59-35.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021857521697236720139
• Upwork Job ID: 1857521697236720139
• Last Price Increase: 2024-11-22

Issue Owner

Current Issue Owner: @neil-marcellini

[melvin-bot]

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

[CortneyOfstad]

I was able to recreate this under my expensifail.com account, so getting some eyes on this!

[melvin-bot]

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

[melvin-bot]

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

[me-ZaidAli]

Edited by proposal-police: This proposal was edited at 2024-11-16 23:02:19 UTC.

Proposal

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

The three dot menu doesn't show Hold menu item when a new report is created

What is the root cause of that problem?

The issue is in the file BaseReportActionContextMenu file. This is the piece of code where the problem is.

  const requestParentReportAction = useMemo(() => {
        if (isMoneyRequestReport || isInvoiceReport) {
            if (!paginatedReportActions || !transactionThreadReport?.parentReportActionID) {
                return undefined;
            }
            return paginatedReportActions.find((action) => action.reportActionID === transactionThreadReport.parentReportActionID);
        }
        return parentReportAction;
    }, [parentReportAction, isMoneyRequestReport, isInvoiceReport, paginatedReportActions, transactionThreadReport?.parentReportActionID]);

    const moneyRequestAction = transactionThreadReportID ? requestParentReportAction : parentReportAction;

The moneyRequestAction is required to be true to show Hold menu item. We have transactionThreadReportID and so the first operation of the ternary operator gets evaluated. requestParentReportAction returns undefined since this condition gets evaluated to true due to absence of parentReportActionID in transactionThreadReport.

if (!paginatedReportActions || !transactionThreadReport?.parentReportActionID) {
                return undefined;
}

like mentioned in the workaround, once we open the report and get back to the chat, we start seeing Hold coz transactionThreadReport get populated with all it's fields including parentReportActionID and

return paginatedReportActions.find((action) => action.reportActionID === transactionThreadReport.parentReportActionID);

gets evaluated to true.

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

The problem is with report loading since our condition relies on transactionThreadReport and when the report is first created, transactionThreadReport object is not loaded fully (missing major fields including parentReportActionID). So have got to make sure it gets loaded properly in Onyx collection.

What alternative solutions did you explore? (Optional)

Alternate would be to change the condition required to show Hold but I wouldn't go that route as it might break some other stuff that we are unaware of.

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

[melvin-bot]

📣 @me-ZaidAli! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[me-ZaidAli]

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/~01dc2d382489d84c0b

[melvin-bot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[CortneyOfstad]

@ahmedGaber93 any thoughts on the proposal above? Thanks!

[ahmedGaber93]

Sorry for the delay, Reviewing today

[me-ZaidAli]

@CortneyOfstad can clarify whether individual expenses inside multiple expense report holdable or no?

[ahmedGaber93]

I think this is a BE issue.

The reportAction data that comes from pusher doesn't have childReportID that used here which break showing the hold item. And if you open any other report, then go back to the expense report to load the API data, you will see the issue is fixed.

20241119203158394.mp4

Comparison of reportAction between pusher and API data.

1. add console.log for reportAction
2. reproduce the issue
3. hover on the reportAction view to log the reportAction data that come from the pusher (doesn't have childReportID)
4. open any other report, then back to the expense report
5. hover on the reportAction view to log the reportAction data that come from the API (have the childReportID)

---

the missed reportAction.childReportID is not the only problem, in the data above I see the pusher data have reportAction?.originalMessage?.linkedReportID which is equal to the missedreportAction?.childReportID so we can use linkedReportID in FE here (just for testing) . but after trying to use it the code starts to work fine then break again in canHoldUnholdReportAction, so I think we have another missed data.

[ahmedGaber93]

@CortneyOfstad I think this is a BE issue and should be internally.

[me-ZaidAli]

@ahmedGaber93 which environment are you testing it on? for me on staging the childReportId is present in reportAction . The issue that you mentioned with canHoldUnholdReportAction happens here since we are unable to fetch childReport for the IOU report action. So when we open the report, FE fetches the report data from BE and it's cached locally for us to access in the chat. transactionThreadReport becomes available and so we start seeing the Hold option.
In addition to this issue there is a lot of refactoring needed in the component since it's filled with stuff and conditions that are not even needed.

[ahmedGaber93]

@me-ZaidAli Testing on DEV with latest main branch.

[ahmedGaber93]

Testing on staging childReportID is not present with pusher data and present only on API data.

20241120053857369.mp4

[iwiznia]

Other PR making solid progress

[CortneyOfstad]

Draft PR is here — #59191.

Just a heads up that I will be OoO this afternoon (March 28) to April 7 — if action is needed on the BZ side of things, please reapply the BZ label. Otherwise I will review when I get back. Thanks!

[CortneyOfstad]

PR is still being worked on 👍

[mvtglobally]

Issue is reproducible during KI retests.

1744463515306.20250412_211114.mp4

[mallenexpensify]

• Enable ESLint rule / DEV alert() to match with useOnyx canBeMissing option #59191 (comment)

Took off hold, the above PR hit production a few days ago. @iwiznia what's next step here?

[CortneyOfstad]

Thanks @mallenexpensify!

[CortneyOfstad]

@iwiznia — bump on the comment here. Thanks!

[iwiznia]

I need to look into it again but should not hold anymore

[mallenexpensify]

Working on the #quality weekly update and posted this

@iwiznia this has stalled, can you prioritize working on it? If expert devs can help, can you please post in #expert-contributors?

[CortneyOfstad]

@iwiznia is there an update on Matt's comment above? Thanks!

[iwiznia]

No update, sorry. I also just realized this was changed to critical, any idea why?

[mallenexpensify]

It's been CRITICAL for months, was on hold for a lil while though.

I'm unsure how often Approvers put reports on hold. We could demote to HIGH if we think this only affects a "subset of users"

• CRITICAL - UX Reliability, API Reliability and Performance bugs that are noticeable for a majority of users
• HIGH -  UX Reliability, API Reliability and Performance issues that are part of main flows and affect a subset of users
• MEDIUM - Debugging Tools to help make fixing existing bugs easier and getting fixes to customers faster and Architectural Improvements to the underlying framework and design of the app to improve performance, scalability, and maintainability
• LOW - Not very noticeable UX Reliability, API Reliability or Performance issues that only affect a single user or subset of users

[garrettmknight]

I think HIGH works - approvers are a subset of users.

[CortneyOfstad]

Hey @iwiznia — checking to see if there is an update? Thanks!