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

Jump to bottom

[Held requests] Clean up the hold/unhold logic #45151

Merged
merged 23 commits into from
Aug 10, 2024
Merged

[Held requests] Clean up the hold/unhold logic #45151

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
948e1f7
fix hold/unhold logic
cdOut Jul 10, 2024
e43ffe0
fix lint errors
cdOut Jul 11, 2024
1384fe1
fix isPolicyAdmin not being properly implemented in ReportUtils check
cdOut Jul 19, 2024
94ae4d3
Merge branch 'main' into @cdOut/cleanup-hold-unhold
cdOut Jul 19, 2024
ade4976
fix prettier
cdOut Jul 19, 2024
c8dce56
fix lint errors after merge
cdOut Jul 19, 2024
c4289b4
clean up code
cdOut Jul 19, 2024
1259a4a
correct logic for hold/unhold for ContextMenu action
cdOut Jul 31, 2024
dbf2421
fix prettier
cdOut Jul 31, 2024
1238c05
Merge branch 'main' into @cdOut/cleanup-hold-unhold
cdOut Jul 31, 2024
7dba3b9
refactor code for latest main hold logic changes
cdOut Jul 31, 2024
6730a11
clean up context menu related components
cdOut Jul 31, 2024
e5c06b8
unify logic for when hold action is shown in report details and conte…
cdOut Jul 31, 2024
689662e
Merge branch 'main' into @cdOut/cleanup-hold-unhold
cdOut Jul 31, 2024
4e91940
remove unused comments
cdOut Jul 31, 2024
9c97c2c
fix can hold logic for IOUs and self-held expenses
cdOut Jul 31, 2024
522447f
Merge branch 'main' into @cdOut/cleanup-hold-unhold
cdOut Aug 1, 2024
3521502
Merge branch 'main' into @cdOut/cleanup-hold-unhold
cdOut Aug 5, 2024
9bbd7a7
fix isHoldCreator not working properly for money requests
cdOut Aug 9, 2024
07c607d
fix prettier
cdOut Aug 9, 2024
70691da
Merge branch 'main' into @cdOut/cleanup-hold-unhold
cdOut Aug 9, 2024
eee69cb
correct logic for non-IOU unhold author conditional
cdOut Aug 9, 2024
03e5b25
fix prettier and formatting
cdOut Aug 9, 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
17 changes: 10 additions & 7 deletions src/libs/ReportUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2955,27 +2955,30 @@ function canHoldUnholdReportAction(reportAction: OnyxInputOrEntry<ReportAction>)
const transactionID = moneyRequestReport ? ReportActionsUtils.getOriginalMessage(reportAction)?.IOUTransactionID : 0;
const transaction = allTransactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`] ?? ({} as Transaction);

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

const isRequestIOU = parentReport?.type === 'iou';
const isRequestHoldCreator = isHoldCreator(transaction, moneyRequestReport?.reportID) && isRequestIOU;
const isRequestIOU = isIOUReport(moneyRequestReport);
const isHoldActionCreator = isHoldCreator(transaction, reportAction.childReportID ?? '-1');

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 isAdmin = isPolicyAdmin(moneyRequestReport.policyID ?? '-1', allPolicies);
const isOnHold = TransactionUtils.isOnHold(transaction);
const isScanning = TransactionUtils.hasReceipt(transaction) && TransactionUtils.isReceiptBeingScanned(transaction);
const isClosed = isClosedReport(moneyRequestReport);

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

const canHoldOrUnholdRequest = !isRequestSettled && !isApproved && !isDeletedParentAction;
const canHoldRequest = canHoldOrUnholdRequest && !isOnHold && (isRequestHoldCreator || (!isRequestIOU && canModifyStatus)) && !isScanning && !!transaction?.reimbursable;
const canHoldOrUnholdRequest = !isRequestSettled && !isApproved && !isDeletedParentAction && !isClosed;
const canHoldRequest = canHoldOrUnholdRequest && !isOnHold && (isRequestIOU || canModifyStatus) && !isScanning && !!transaction?.reimbursable;
const canUnholdRequest =
!!(canHoldOrUnholdRequest && isOnHold && !TransactionUtils.isDuplicate(transaction.transactionID, true) && (isRequestHoldCreator || (!isRequestIOU && canModifyStatus))) &&
!!(canHoldOrUnholdRequest && isOnHold && !TransactionUtils.isDuplicate(transaction.transactionID, true) && (isRequestIOU ? isHoldActionCreator : canModifyUnholdStatus)) &&
!!transaction?.reimbursable;

return {canHoldRequest, canUnholdRequest};
Expand Down
55 changes: 54 additions & 1 deletion src/pages/home/report/ContextMenu/BaseReportActionContextMenu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import useEnvironment from '@hooks/useEnvironment';
import useKeyboardShortcut from '@hooks/useKeyboardShortcut';
import useLocalize from '@hooks/useLocalize';
import useNetwork from '@hooks/useNetwork';
import usePaginatedReportActions from '@hooks/usePaginatedReportActions';
import useResponsiveLayout from '@hooks/useResponsiveLayout';
import useStyleUtils from '@hooks/useStyleUtils';
import * as ReportActionsUtils from '@libs/ReportActionsUtils';
Expand Down Expand Up @@ -141,13 +142,64 @@ function BaseReportActionContextMenu({

const [download] = useOnyx(`${ONYXKEYS.COLLECTION.DOWNLOAD}${sourceID}`);

const childReport = ReportUtils.getReport(reportAction?.childReportID ?? '-1');
const parentReportAction = ReportActionsUtils.getReportAction(childReport?.parentReportID ?? '', childReport?.parentReportActionID ?? '');
const {reportActions: paginatedReportActions} = usePaginatedReportActions(childReport?.reportID ?? '-1');

const transactionThreadReportID = useMemo(
() => ReportActionsUtils.getOneTransactionThreadReportID(childReport?.reportID ?? '-1', paginatedReportActions ?? [], isOffline),
[childReport?.reportID, paginatedReportActions, isOffline],
);

const [transactionThreadReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${transactionThreadReportID}`);

const isMoneyRequestReport = useMemo(() => ReportUtils.isMoneyRequestReport(childReport), [childReport]);
const isInvoiceReport = useMemo(() => ReportUtils.isInvoiceReport(childReport), [childReport]);

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;

const [parentReportNameValuePairs] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${childReport?.parentReportID ?? '-1'}`);
const parentReport = ReportUtils.getReport(childReport?.parentReportID ?? '-1');

const isMoneyRequest = useMemo(() => ReportUtils.isMoneyRequest(childReport), [childReport]);
const isTrackExpenseReport = ReportUtils.isTrackExpenseReport(childReport);
const isSingleTransactionView = isMoneyRequest || isTrackExpenseReport;
const isMoneyRequestOrReport = isMoneyRequestReport || isInvoiceReport || isSingleTransactionView;
Comment on lines +145 to +177
Copy link
Member

Choose a reason for hiding this comment

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

Don't we have utils to get all these already?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you know of said utils then please point them out to me, but I don't think we already have one that would handle this sort of logic. This was recently written for just ReportDetailsPage which unified multiple pages with hold functionalities into one component.

Copy link
Member

Choose a reason for hiding this comment

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

Let me see.


const areHoldRequirementsMet = isMoneyRequestOrReport && !ReportUtils.isArchivedRoom(transactionThreadReportID ? childReport : parentReport, parentReportNameValuePairs);

const originalReportID = useMemo(() => ReportUtils.getOriginalReportID(reportID, reportAction), [reportID, reportAction]);

const shouldEnableArrowNavigation = !isMini && (isVisible || shouldKeepOpen);
let filteredContextMenuActions = ContextMenuActions.filter(
(contextAction) =>
!disabledActions.includes(contextAction) &&
contextAction.shouldShow(type, reportAction, isArchivedRoom, betas, anchor, isChronosReport, reportID, isPinnedChat, isUnreadChat, !!isOffline, isMini, isProduction),
contextAction.shouldShow(
type,
reportAction,
isArchivedRoom,
betas,
anchor,
isChronosReport,
reportID,
isPinnedChat,
isUnreadChat,
!!isOffline,
isMini,
isProduction,
moneyRequestAction,
areHoldRequirementsMet,
),
);

if (isMini) {
Expand Down Expand Up @@ -254,6 +306,7 @@ function BaseReportActionContextMenu({
interceptAnonymousUser,
openOverflowMenu,
setIsEmojiPickerActive,
moneyRequestAction,
};

if ('renderContent' in contextAction) {
Expand Down
53 changes: 42 additions & 11 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 @@ -67,6 +67,8 @@ type ShouldShow = (
isOffline: boolean,
isMini: boolean,
isProduction: boolean,
moneyRequestAction: ReportAction | undefined,
areHoldRequirementsMet: boolean,
) => boolean;

type ContextMenuActionPayload = {
Expand All @@ -84,6 +86,7 @@ type ContextMenuActionPayload = {
event?: GestureResponderEvent | MouseEvent | KeyboardEvent;
setIsEmojiPickerActive?: (state: boolean) => void;
anchorRef?: MutableRefObject<View | null>;
moneyRequestAction: ReportAction | undefined;
};

type OnPress = (closePopover: boolean, payload: ContextMenuActionPayload, selection?: string, reportID?: string, draftMessage?: string) => void;
Expand Down Expand Up @@ -262,35 +265,63 @@ const ContextMenuActions: ContextMenuAction[] = [
},
{
isAnonymousAction: false,
textTranslateKey: 'iou.unholdExpense',
textTranslateKey: 'iou.unhold',
icon: Expensicons.Stopwatch,
shouldShow: (type, reportAction) =>
type === CONST.CONTEXT_MENU_TYPES.REPORT_ACTION && ReportUtils.canEditReportAction(reportAction) && ReportUtils.canHoldUnholdReportAction(reportAction).canUnholdRequest,
onPress: (closePopover, {reportAction}) => {
shouldShow: (
type,
reportAction,
isArchivedRoom,
betas,
anchor,
isChronosReport,
reportID,
isPinnedChat,
isUnreadChat,
isOffline,
isMini,
isProduction,
moneyRequestAction,
areHoldRequirementsMet,
) => type === CONST.CONTEXT_MENU_TYPES.REPORT_ACTION && areHoldRequirementsMet && ReportUtils.canHoldUnholdReportAction(moneyRequestAction).canUnholdRequest,
onPress: (closePopover, {moneyRequestAction}) => {
if (closePopover) {
hideContextMenu(false, () => ReportUtils.changeMoneyRequestHoldStatus(reportAction));
hideContextMenu(false, () => ReportUtils.changeMoneyRequestHoldStatus(moneyRequestAction));
return;
}

// No popover to hide, call changeMoneyRequestHoldStatus immediately
ReportUtils.changeMoneyRequestHoldStatus(reportAction);
ReportUtils.changeMoneyRequestHoldStatus(moneyRequestAction);
},
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}) => {
shouldShow: (
type,
reportAction,
isArchivedRoom,
betas,
anchor,
isChronosReport,
reportID,
isPinnedChat,
isUnreadChat,
isOffline,
isMini,
isProduction,
moneyRequestAction,
areHoldRequirementsMet,
) => type === CONST.CONTEXT_MENU_TYPES.REPORT_ACTION && areHoldRequirementsMet && ReportUtils.canHoldUnholdReportAction(moneyRequestAction).canHoldRequest,
onPress: (closePopover, {moneyRequestAction}) => {
if (closePopover) {
hideContextMenu(false, () => ReportUtils.changeMoneyRequestHoldStatus(reportAction));
hideContextMenu(false, () => ReportUtils.changeMoneyRequestHoldStatus(moneyRequestAction));
return;
}

// No popover to hide, call changeMoneyRequestHoldStatus immediately
ReportUtils.changeMoneyRequestHoldStatus(reportAction);
ReportUtils.changeMoneyRequestHoldStatus(moneyRequestAction);
},
getDescription: () => {},
},
Expand Down
Loading