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

[$250] Can't Hold card expenses #56444

Open
garrettmknight opened this issue Feb 6, 2025 · 8 comments
Open

[$250] Can't Hold card expenses #56444

garrettmknight opened this issue Feb 6, 2025 · 8 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors

Comments

@garrettmknight
Copy link
Contributor

garrettmknight commented Feb 6, 2025

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: N/A
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: @puneetlath
Slack conversation (hyperlinked to channel name): #expense

Preconditions:

  • Assign a card to the submitter via domain control/Classic that's importing transactions
  • Enable approvals

Action Performed:

  1. As a submitter, submit a card transaction
  2. As an approver, open the details menu of the expense

Expected Result:

The approver should see the option to Hold an expense

Actual Result:

The approver doesn't see the option to Hold an expnese

Workaround:

No workaround.

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021887314499991264995
  • Upwork Job ID: 1887314499991264995
  • Last Price Increase: 2025-02-06
Issue OwnerCurrent Issue Owner: @suneox
@garrettmknight garrettmknight added Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor labels Feb 6, 2025
Copy link

melvin-bot bot commented Feb 6, 2025

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

@melvin-bot melvin-bot bot changed the title Can't Hold card expenses [$250] Can't Hold card expenses Feb 6, 2025
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 6, 2025
Copy link

melvin-bot bot commented Feb 6, 2025

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

Copy link

melvin-bot bot commented Feb 6, 2025

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

@garrettmknight
Copy link
Contributor Author

@dylanexpensify Gonna start it as External since this might actually be a FE issue.

@NJ-2020
Copy link
Contributor

NJ-2020 commented Feb 6, 2025

@garrettmknight Actually this a BE issue
We will show the hold option if it's match these 3 conditions

textTranslateKey: 'iou.hold',
icon: Expensicons.Stopwatch,
shouldShow: ({type, moneyRequestAction, areHoldRequirementsMet}) =>
type === CONST.CONTEXT_MENU_TYPES.REPORT_ACTION && areHoldRequirementsMet && canHoldUnholdReportAction(moneyRequestAction).canHoldRequest,

All the conditions were matched but only the last one is returning false which is canHoldUnholdReportAction(moneyRequestAction).canHoldRequest

Inside canHoldUnholdReportAction function we will return false if the action name is not IOU and if the IOUReportID is not exist

App/src/libs/ReportUtils.ts

Lines 3639 to 3642 in a8daefb

function canHoldUnholdReportAction(reportAction: OnyxInputOrEntry<ReportAction>): {canHoldRequest: boolean; canUnholdRequest: boolean} {
if (!isMoneyRequestAction(reportAction)) {
return {canHoldRequest: false, canUnholdRequest: false};
}

App/src/libs/ReportUtils.ts

Lines 3644 to 3649 in a8daefb

const moneyRequestReportID = getOriginalMessage(reportAction)?.IOUReportID;
const moneyRequestReport = getReportOrDraftReport(String(moneyRequestReportID));
if (!moneyRequestReportID || !moneyRequestReport) {
return {canHoldRequest: false, canUnholdRequest: false};
}

function isMoneyRequestAction(reportAction: OnyxInputOrEntry<ReportAction>): reportAction is ReportAction<typeof CONST.REPORT.ACTIONS.TYPE.IOU> {
return isActionOfType(reportAction, CONST.REPORT.ACTIONS.TYPE.IOU);
}

But since the BE is returning the action name as PREVIEW instead of IOU and some properties regarding to IOU is not exists like IOUReportID, so the above function canHoldUnholdReportAction will return false

Image


But I think maybe we can also solve this by using workaround solution
Right here instead of checking the action name only, we can check if the childType is equal to expense

function isMoneyRequestAction(reportAction: OnyxInputOrEntry<ReportAction>): reportAction is ReportAction<typeof CONST.REPORT.ACTIONS.TYPE.IOU> {
return isActionOfType(reportAction, CONST.REPORT.ACTIONS.TYPE.IOU);
}

And instead of IOUReportID we can check if the action name is preview then we will use the report id

and so on..

But yeah this workaround solution may work but maybe it cause any error/bugs in the future

@dylanexpensify
Copy link
Contributor

@suneox can we get a review on @NJ-2020's proposal? 🙇‍♂

@suneox
Copy link
Contributor

suneox commented Feb 7, 2025

@suneox can we get a review on @NJ-2020's proposal? 🙇‍♂

Yes i'll review this comment within few hours

@suneox
Copy link
Contributor

suneox commented Feb 7, 2025

We have a logic check for the iou.hold menu that depends on moneyRequestAction, with a lot of logic in the canHoldUnholdReportAction function. However, the childReport only loads when we open the details from reportAction, so I think we have a limitation on this case before open report detail and need the internal team to take a look on this issue.
We are still looking solution on this one

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. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors
Projects
Status: No status
Development

No branches or pull requests

4 participants