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.

Sign up for GitHub

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

feat: [Held requests] option does not show in the preview overflow menu. #42034

Merged
merged 13 commits into from
Jun 11, 2024
Merged

feat: [Held requests] option does not show in the preview overflow menu. #42034

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
be5bcfa
feat: [Held requests] option does not show in the preview overflow m…
Krishna2323 May 13, 2024
2e36d00
Merge branch 'main' into krishna2323/issue/41440
Krishna2323 May 16, 2024
d8fc20b
minor fixes.
Krishna2323 May 17, 2024
9f330e4
canHoldUnholdReportAction util function refactoring.
Krishna2323 May 21, 2024
0fac245
add getParentReportAction in ReportUtils.
Krishna2323 May 21, 2024
3b2e151
Merge branch 'Expensify:main' into krishna2323/issue/41440
Krishna2323 May 24, 2024
ed98b40
resolve conflicts.
Krishna2323 May 29, 2024
051180b
Merge branch 'Expensify:main' into krishna2323/issue/41440
Krishna2323 May 29, 2024
7946f0c
Merge branch 'Expensify:main' into krishna2323/issue/41440
Krishna2323 Jun 3, 2024
f33575d
Merge branch 'Expensify:main' into krishna2323/issue/41440
Krishna2323 Jun 8, 2024
2df4f55
use deprecated ReportActionsUtils.getParentReportAction.
Krishna2323 Jun 8, 2024
721e407
fix lint issue.
Krishna2323 Jun 8, 2024
41276b8
Merge branch 'Expensify:main' into krishna2323/issue/41440
Krishna2323 Jun 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 75 additions & 8 deletions src/libs/ReportUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2404,6 +2404,14 @@ function isReportFieldOfTypeTitle(reportField: OnyxEntry<PolicyReportField>): bo
return reportField?.type === 'formula' && reportField?.fieldID === CONST.REPORT_FIELD_TITLE_FIELD_ID;
}

/**
* Check if Report has any held expenses
*/
function isHoldCreator(transaction: OnyxEntry<Transaction>, reportID: string): boolean {
const holdReportAction = ReportActionsUtils.getReportAction(reportID, `${transaction?.comment?.hold ?? ''}`);
return isActionCreator(holdReportAction);
}

/**
* Check if report fields are available to use in a report
*/
Expand Down Expand Up @@ -2721,6 +2729,71 @@ function canEditReportAction(reportAction: OnyxEntry<ReportAction>): boolean {
);
}

function canHoldUnholdReportAction(reportAction: OnyxEntry<ReportAction>): {canHoldRequest: boolean; canUnholdRequest: boolean} {
if (reportAction?.actionName !== CONST.REPORT.ACTIONS.TYPE.IOU) {
return {canHoldRequest: false, canUnholdRequest: false};
}

const moneyRequestReportID = reportAction?.originalMessage?.IOUReportID ?? 0;
const moneyRequestReport = getReport(String(moneyRequestReportID));

if (!moneyRequestReportID || !moneyRequestReport) {
return {canHoldRequest: false, canUnholdRequest: false};
}

const isRequestSettled = isSettled(moneyRequestReport?.reportID);
const isApproved = isReportApproved(moneyRequestReport);
const transactionID = moneyRequestReport ? reportAction?.originalMessage?.IOUTransactionID : 0;
const transaction = allTransactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`] ?? ({} as Transaction);

const parentReport = getReport(String(moneyRequestReport.parentReportID));
const parentReportAction = ReportActionsUtils.getParentReportAction(moneyRequestReport);

const isRequestIOU = parentReport?.type === 'iou';
const isRequestHoldCreator = isHoldCreator(transaction, moneyRequestReport?.reportID) && isRequestIOU;
const isTrackExpenseMoneyReport = isTrackExpenseReport(moneyRequestReport);
const isActionOwner =
typeof parentReportAction?.actorAccountID === 'number' &&
typeof currentUserPersonalDetails?.accountID === 'number' &&
parentReportAction.actorAccountID === currentUserPersonalDetails?.accountID;
const isApprover = isMoneyRequestReport(moneyRequestReport) && moneyRequestReport?.managerID !== null && currentUserPersonalDetails?.accountID === moneyRequestReport?.managerID;
const isOnHold = TransactionUtils.isOnHold(transaction);
const isScanning = TransactionUtils.hasReceipt(transaction) && TransactionUtils.isReceiptBeingScanned(transaction);

const canModifyStatus = !isTrackExpenseMoneyReport && (isPolicyAdmin || isActionOwner || isApprover);
const isDeletedParentAction = isEmptyObject(parentReportAction) || ReportActionsUtils.isDeletedAction(parentReportAction);

const canHoldOrUnholdRequest = !isRequestSettled && !isApproved && !isDeletedParentAction;
const canHoldRequest = canHoldOrUnholdRequest && !isOnHold && (isRequestHoldCreator || (!isRequestIOU && canModifyStatus)) && !isScanning;
const canUnholdRequest = !!(canHoldOrUnholdRequest && isOnHold && (isRequestHoldCreator || (!isRequestIOU && canModifyStatus)));

return {canHoldRequest, canUnholdRequest};
}

const changeMoneyRequestHoldStatus = (reportAction: OnyxEntry<ReportAction>): void => {
if (reportAction?.actionName !== CONST.REPORT.ACTIONS.TYPE.IOU) {
return;
}
const moneyRequestReportID = reportAction?.originalMessage?.IOUReportID ?? 0;

const moneyRequestReport = getReport(String(moneyRequestReportID));
if (!moneyRequestReportID || !moneyRequestReport) {
return;
}
Comment on lines +2774 to +2782
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we merge these 2 conditions?

if (reportAction?.actionName !== CONST.REPORT.ACTIONS.TYPE.IOU || !moneyRequestReportID || !moneyRequestReport) {
return;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think thats fine, reportAction?.actionName !== CONST.REPORT.ACTIONS.TYPE.IOU check is required before defining moneyRequestReport or moneyRequestReportID.


const transactionID = reportAction?.originalMessage?.IOUTransactionID ?? '';
const transaction = allTransactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`] ?? ({} as Transaction);
const isOnHold = TransactionUtils.isOnHold(transaction);
const policy = allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${moneyRequestReport.policyID}`] ?? null;

if (isOnHold) {
IOU.unholdRequest(transactionID, reportAction.childReportID ?? '');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In here, you updated the hold expense with its childReportID, in our BE we didn't send Pusher event to update its parent that's why our replies didn't increase when you held the expense.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems that we will need BE changes here cc @robertjchen.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hungvu193, thanks a lot for looking into this, I agree with you.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, so the backend should also push updates for the parent report here, is that correct?

Could you let me know which particular field changes in the parent report that makes the update from the backend necessary? 👀

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

childVisibleActionCount is the current field to count the replies.
Since we also added a comment when we held request, I think the pusher event will be exactly the same when we added a new comment inside a thread.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, we can investigate- can this PR go out ahead of the backend changes or is it blocked until that field is available?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can go ahead with this PR first, but let's leave a note for QA team that the issue about counting replies will be fixed once BE PR is deployed.

} else {
const activeRoute = encodeURIComponent(Navigation.getActiveRouteWithoutParams());
Navigation.navigate(ROUTES.MONEY_REQUEST_HOLD_REASON.getRoute(policy?.type ?? CONST.POLICY.TYPE.PERSONAL, transactionID, reportAction.childReportID ?? '', activeRoute));
}
};

/**
* Gets all transactions on an IOU report with a receipt
*/
Expand Down Expand Up @@ -6386,14 +6459,6 @@ function navigateToPrivateNotes(report: OnyxEntry<Report>, session: OnyxEntry<Se
Navigation.navigate(ROUTES.PRIVATE_NOTES_LIST.getRoute(report.reportID));
}

/**
* Check if Report has any held expenses
*/
function isHoldCreator(transaction: OnyxEntry<Transaction>, reportID: string): boolean {
const holdReportAction = ReportActionsUtils.getReportAction(reportID, `${transaction?.comment?.hold ?? ''}`);
return isActionCreator(holdReportAction);
}

/**
* Get all held transactions of a iouReport
*/
Expand Down Expand Up @@ -6905,6 +6970,7 @@ export {
canCreateTaskInReport,
canCurrentUserOpenReport,
canDeleteReportAction,
canHoldUnholdReportAction,
canEditFieldOfMoneyRequest,
canEditMoneyRequest,
canEditPolicyDescription,
Expand Down Expand Up @@ -7132,6 +7198,7 @@ export {
shouldShowMerchantColumn,
isCurrentUserInvoiceReceiver,
isDraftReport,
changeMoneyRequestHoldStatus,
createDraftWorkspaceAndNavigateToConfirmationScreen,
};

Expand Down
34 changes: 34 additions & 0 deletions src/pages/home/report/ContextMenu/ContextMenuActions.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,40 @@ const ContextMenuActions: ContextMenuAction[] = [
},
getDescription: () => {},
},
{
isAnonymousAction: false,
textTranslateKey: 'iou.unholdExpense',
icon: Expensicons.Stopwatch,
shouldShow: (type, reportAction) =>
type === CONST.CONTEXT_MENU_TYPES.REPORT_ACTION && ReportUtils.canEditReportAction(reportAction) && ReportUtils.canHoldUnholdReportAction(reportAction).canUnholdRequest,
onPress: (closePopover, {reportAction}) => {
if (closePopover) {
hideContextMenu(false, () => ReportUtils.changeMoneyRequestHoldStatus(reportAction));
return;
}

// No popover to hide, call changeMoneyRequestHoldStatus immediately
ReportUtils.changeMoneyRequestHoldStatus(reportAction);
},
getDescription: () => {},
},
{
isAnonymousAction: false,
textTranslateKey: 'iou.hold',
icon: Expensicons.Stopwatch,
shouldShow: (type, reportAction) =>
type === CONST.CONTEXT_MENU_TYPES.REPORT_ACTION && ReportUtils.canEditReportAction(reportAction) && ReportUtils.canHoldUnholdReportAction(reportAction).canHoldRequest,
onPress: (closePopover, {reportAction}) => {
if (closePopover) {
hideContextMenu(false, () => ReportUtils.changeMoneyRequestHoldStatus(reportAction));
return;
}

// No popover to hide, call changeMoneyRequestHoldStatus immediately
ReportUtils.changeMoneyRequestHoldStatus(reportAction);
},
getDescription: () => {},
},
{
isAnonymousAction: false,
textTranslateKey: 'reportActionContextMenu.joinThread',
Expand Down
Loading