App
App copied to clipboard
[HOLD 53738][$250] Hold doesn't show in three dot menu until report is opened
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:
- Create a workspace
- Invite Submitter + Approver A
- Enable Workflows > Approval + Delay Submission
- Set Approver A as approver
- As Submitter, create expense in WS chat + submit
- 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:
Describe what actually happened
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
- [x] MacOS: Chrome / Safari
- [x] MacOS: Desktop
Screenshots/Videos
Add any screenshot/video evidence
https://github.com/user-attachments/assets/0c884c5e-83c8-4125-8087-45f44aca66eb
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
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.
I was able to recreate this under my expensifail.com account, so getting some eyes on this!
Job added to Upwork: https://www.upwork.com/jobs/~021857521697236720139
Triggered auto assignment to Contributor-plus team member for initial proposal review - @ahmedGaber93 (External)
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.
📣 @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:
- Make sure you've read and understood the contributing guidelines.
- 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.
- 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.
- 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>
Contributor details Your Expensify account email: [email protected] Upwork Profile Link: https://www.upwork.com/freelancers/~01dc2d382489d84c0b
✅ Contributor details stored successfully. Thank you for contributing to Expensify!
@ahmedGaber93 any thoughts on the proposal above? Thanks!
Sorry for the delay, Reviewing today
@CortneyOfstad can clarify whether individual expenses inside multiple expense report holdable or no?
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.
https://github.com/user-attachments/assets/f6851eb4-b66e-4cbf-8806-fca1fa4fc0a0
Comparison of reportAction between pusher and API data.
- add console.log for reportAction
- reproduce the issue
- hover on the
reportActionview to log thereportActiondata that come from thepusher(doesn't havechildReportID) - open any other report, then back to the expense report
- hover on the
reportActionview to log thereportActiondata that come from theAPI(have thechildReportID)
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.
@CortneyOfstad I think this is a BE issue and should be internally.
@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.
@me-ZaidAli Testing on DEV with latest main branch.
Testing on staging childReportID is not present with pusher data and present only on API data.
https://github.com/user-attachments/assets/059233e1-d97b-4295-b38a-1c54bfc9fd94
The issue that you mentioned with canHoldUnholdReportAction happens here since we are unable to fetch childReport for the IOU report action.
I agree with this part, the parentReportActionID also is not present on transactionThreadReport that come from pusher, so we also need to fix it from BE side
I think the missed data from pusher that need to fix this is:
reportAction.childReportIDtransactionThreadReport.parentReportActionID
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸
@CortneyOfstad bump https://github.com/Expensify/App/issues/52401#issuecomment-2487676060
Moving to Hot Picks
Thanks Matt! Sorry for the delay here!
@CortneyOfstad, @ahmedGaber93 Whoops! This issue is 2 days overdue. Let's get this updated quick!
Not overdue, waiting for an internal developer.
@CortneyOfstad, @ahmedGaber93 Huh... This is 4 days overdue. Who can take care of this?
Not overdue, waiting for an internal developer.
Can someone please open a separate GH issue for an internal engineer using this template? It will ensure that an engineer is assigned and they have the full context of what they need to do without reading through the entire comment history and try to piece together what the problem is.
I'll grab it and sort it out
Sounds good @tgolen and thank you @neil-marcellini!
I wasn't able to reproduce this on the latest dev environment. I tried to reproduce on staging, but I'm blocked by a separate bug. I will report it, put this one on HOLD, and try to fix that bug first.
This was reported in Slack here, we'll discuss from there.