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

Add confirmation prompt when approving held request via report preview #42896

Merged
merged 20 commits into from
Jun 10, 2024
Merged

Add confirmation prompt when approving held request via report preview #42896

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
2974def
Add confirmation prompt when approving held request via report preview
nkdengineer May 31, 2024
68aa796
fix lint
nkdengineer May 31, 2024
4c67715
safely check iouReport
nkdengineer May 31, 2024
21b35f7
fix test
nkdengineer May 31, 2024
b34f9da
Merge branch 'main' into fix/39335
nkdengineer May 31, 2024
ac8b499
fix perf test
nkdengineer May 31, 2024
fca320f
fix lint
nkdengineer May 31, 2024
78f8f52
Merge branch 'main' into fix/39335
nkdengineer Jun 3, 2024
99eebd8
add new const
nkdengineer Jun 3, 2024
0091e82
fix lint
nkdengineer Jun 3, 2024
5866cb9
Merge branch 'main' into fix/39335
nkdengineer Jun 4, 2024
7392bd5
fix perf test
nkdengineer Jun 4, 2024
fd6d0f7
update suggestion
nkdengineer Jun 4, 2024
2897d7f
fix perf test
nkdengineer Jun 4, 2024
e7969de
fix type
nkdengineer Jun 4, 2024
7ccb8ff
refactor suggestion
nkdengineer Jun 4, 2024
28220dd
run prettier
nkdengineer Jun 4, 2024
3df8d03
merge main
nkdengineer Jun 5, 2024
dfad444
resolve conflict
nkdengineer Jun 5, 2024
083823d
Merge branch 'main' into fix/39335
nkdengineer Jun 10, 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
4 changes: 4 additions & 0 deletions src/CONST.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1552,6 +1552,10 @@ const CONST = {
CATEGORIZE: 'categorize',
SHARE: 'share',
},
ACTION_TYPE: {
Copy link
Contributor

Choose a reason for hiding this comment

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

@nkdengineer IMO, we don't need to create a new type here. Let's reuse REPORT_ACTION_TYPE and create a new type PreviewPaymentMethodType like this

type PreviewPaymentMethodType = DeepValueOf<typeof CONST.IOU.REPORT_ACTION_TYPE.PAY | typeof CONST.IOU.REPORT_ACTION_TYPE.APPROVE>;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@DylanDylann I updated.

APPROVE: 'approve',
PAY: 'pay',
},
DEFAULT_AMOUNT: 0,
TYPE: {
SEND: 'send',
Expand Down
7 changes: 4 additions & 3 deletions src/components/MoneyReportHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import type * as OnyxTypes from '@src/types/onyx';
import type {PaymentMethodType} from '@src/types/onyx/OriginalMessage';
import type DeepValueOf from '@src/types/utils/DeepValueOf';
import {isEmptyObject} from '@src/types/utils/EmptyObject';
import Button from './Button';
import ConfirmModal from './ConfirmModal';
Expand Down Expand Up @@ -79,7 +80,7 @@ function MoneyReportHeader({policy, report: moneyRequestReport, transactionThrea
isActionOwner && (ReportUtils.canAddOrDeleteTransactions(moneyRequestReport) || ReportUtils.isTrackExpenseReport(transactionThreadReport)) && !isDeletedParentAction;
const [isHoldMenuVisible, setIsHoldMenuVisible] = useState(false);
const [paymentType, setPaymentType] = useState<PaymentMethodType>();
const [requestType, setRequestType] = useState<'pay' | 'approve'>();
const [requestType, setRequestType] = useState<DeepValueOf<typeof CONST.IOU.ACTION_TYPE>>();
const canAllowSettlement = ReportUtils.hasUpdatedTotal(moneyRequestReport, policy);
const policyType = policy?.type;
const isPayer = ReportUtils.isPayer(session, moneyRequestReport);
Expand Down Expand Up @@ -124,7 +125,7 @@ function MoneyReportHeader({policy, report: moneyRequestReport, transactionThrea
return;
}
setPaymentType(type);
setRequestType('pay');
setRequestType(CONST.IOU.ACTION_TYPE.PAY);
if (ReportUtils.hasHeldExpenses(moneyRequestReport.reportID)) {
setIsHoldMenuVisible(true);
} else if (chatReport) {
Expand All @@ -133,7 +134,7 @@ function MoneyReportHeader({policy, report: moneyRequestReport, transactionThrea
};

const confirmApproval = () => {
setRequestType('approve');
setRequestType(CONST.IOU.ACTION_TYPE.APPROVE);
if (ReportUtils.hasHeldExpenses(moneyRequestReport.reportID)) {
setIsHoldMenuVisible(true);
} else {
Expand Down
6 changes: 4 additions & 2 deletions src/components/ProcessMoneyReportHoldMenu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@ import useLocalize from '@hooks/useLocalize';
import Navigation from '@libs/Navigation/Navigation';
import {isLinkedTransactionHeld} from '@libs/ReportActionsUtils';
import * as IOU from '@userActions/IOU';
import CONST from '@src/CONST';
import ROUTES from '@src/ROUTES';
import type * as OnyxTypes from '@src/types/onyx';
import type {PaymentMethodType} from '@src/types/onyx/OriginalMessage';
import type DeepValueOf from '@src/types/utils/DeepValueOf';
import DecisionModal from './DecisionModal';

type ProcessMoneyReportHoldMenuProps = {
Expand Down Expand Up @@ -35,7 +37,7 @@ type ProcessMoneyReportHoldMenuProps = {
paymentType?: PaymentMethodType;

/** Type of action handled */
requestType?: 'pay' | 'approve';
requestType?: DeepValueOf<typeof CONST.IOU.ACTION_TYPE>;
};

function ProcessMoneyReportHoldMenu({
Expand All @@ -50,7 +52,7 @@ function ProcessMoneyReportHoldMenu({
moneyRequestReport,
}: ProcessMoneyReportHoldMenuProps) {
const {translate} = useLocalize();
const isApprove = requestType === 'approve';
const isApprove = requestType === CONST.IOU.ACTION_TYPE.APPROVE;

const onSubmit = (full: boolean) => {
if (isApprove) {
Expand Down
49 changes: 47 additions & 2 deletions src/components/ReportActionItem/ReportPreview.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import truncate from 'lodash/truncate';
import React, {useMemo} from 'react';
import React, {useMemo, useState} from 'react';
import type {StyleProp, ViewStyle} from 'react-native';
import {View} from 'react-native';
import type {OnyxCollection, OnyxEntry} from 'react-native-onyx';
Expand All @@ -9,13 +9,15 @@ import Icon from '@components/Icon';
import * as Expensicons from '@components/Icon/Expensicons';
import OfflineWithFeedback from '@components/OfflineWithFeedback';
import PressableWithoutFeedback from '@components/Pressable/PressableWithoutFeedback';
import ProcessMoneyReportHoldMenu from '@components/ProcessMoneyReportHoldMenu';
import SettlementButton from '@components/SettlementButton';
import {showContextMenuForReport} from '@components/ShowContextMenuContext';
import Text from '@components/Text';
import useLocalize from '@hooks/useLocalize';
import usePermissions from '@hooks/usePermissions';
import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';
import useWindowDimensions from '@hooks/useWindowDimensions';
import ControlSelection from '@libs/ControlSelection';
import * as CurrencyUtils from '@libs/CurrencyUtils';
import * as DeviceCapabilities from '@libs/DeviceCapabilities';
Expand All @@ -33,6 +35,7 @@ import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import type {Policy, Report, ReportAction, Transaction, TransactionViolations, UserWallet} from '@src/types/onyx';
import type {PaymentMethodType} from '@src/types/onyx/OriginalMessage';
import type DeepValueOf from '@src/types/utils/DeepValueOf';
import type {PendingMessageProps} from './MoneyRequestPreview/types';
import ReportActionItemImages from './ReportActionItemImages';

Expand Down Expand Up @@ -119,6 +122,12 @@ function ReportPreview({
[transactions, iouReportID, action],
);

const [isHoldMenuVisible, setIsHoldMenuVisible] = useState(false);
const [requestType, setRequestType] = useState<DeepValueOf<typeof CONST.IOU.ACTION_TYPE>>();
const [nonHeldAmount, fullAmount] = ReportUtils.getNonHeldAndFullAmount(iouReport, policy);
const {isSmallScreenWidth} = useWindowDimensions();
const [paymentType, setPaymentType] = useState<PaymentMethodType>();

const managerID = iouReport?.managerID ?? 0;
const {totalDisplaySpend, reimbursableSpend} = ReportUtils.getMoneyRequestSpendBreakdown(iouReport);

Expand Down Expand Up @@ -162,6 +171,28 @@ function ReportPreview({
[chatReport?.isOwnPolicyExpenseChat, policy?.harvesting?.enabled],
);

const confirmPayment = (type: PaymentMethodType | undefined) => {
if (!type) {
return;
}
setPaymentType(type);
setRequestType(CONST.IOU.ACTION_TYPE.PAY);
if (ReportUtils.hasHeldExpenses(iouReport?.reportID)) {
setIsHoldMenuVisible(true);
} else if (chatReport && iouReport) {
IOU.payMoneyRequest(type, chatReport, iouReport, false);
}
};

const confirmApproval = () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we create a utility function to avoid code repetition?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this these function, we also have logic to set state of the component so we shouldn't create a util for this.

Copy link
Contributor

Choose a reason for hiding this comment

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

we also have logic to set state of the component so we shouldn't create a util for this.

Do you mean we shouldn't pass setIsHoldMenuVisible,... to the util function? Could you explain more why we shouldn't do that

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 mean we shouldn't create a util function for this case since this function also controls the state of a component.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And Since we only use this logic for report preview and MoneyReportHeader, the duplicate is acceptable.

setRequestType(CONST.IOU.ACTION_TYPE.APPROVE);
if (ReportUtils.hasHeldExpenses(iouReport?.reportID)) {
setIsHoldMenuVisible(true);
} else {
IOU.approveMoneyRequest(iouReport ?? {}, true);
}
};

const getDisplayAmount = (): string => {
if (totalDisplaySpend) {
return CurrencyUtils.convertToDisplayString(totalDisplaySpend, iouReport?.currency);
Expand Down Expand Up @@ -368,7 +399,8 @@ function ReportPreview({
policyID={policyID}
chatReportID={chatReportID}
iouReport={iouReport}
onPress={(paymentType?: PaymentMethodType) => chatReport && iouReport && paymentType && IOU.payMoneyRequest(paymentType, chatReport, iouReport)}
onPress={confirmPayment}
confirmApproval={confirmApproval}
enablePaymentsRoute={ROUTES.ENABLE_PAYMENTS}
addBankAccountRoute={bankAccountRoute}
shouldHidePaymentOptions={!shouldShowPayButton}
Expand Down Expand Up @@ -399,6 +431,19 @@ function ReportPreview({
</View>
</PressableWithoutFeedback>
</View>
{isHoldMenuVisible && iouReport && requestType !== undefined && (
<ProcessMoneyReportHoldMenu
nonHeldAmount={!ReportUtils.hasOnlyHeldExpenses(iouReport?.reportID ?? '') ? nonHeldAmount : undefined}
requestType={requestType}
fullAmount={fullAmount}
isSmallScreenWidth={isSmallScreenWidth}
onClose={() => setIsHoldMenuVisible(false)}
isVisible={isHoldMenuVisible}
paymentType={paymentType}
chatReport={chatReport}
moneyRequestReport={iouReport}
/>
)}
</OfflineWithFeedback>
);
}
Expand Down
4 changes: 2 additions & 2 deletions src/libs/ReportUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6399,8 +6399,8 @@ function getNonHeldAndFullAmount(iouReport: OnyxEntry<Report>, policy: OnyxEntry
}

return [
CurrencyUtils.convertToDisplayString((iouReport?.unheldTotal ?? 0) * -1, iouReport?.currency ?? ''),
CurrencyUtils.convertToDisplayString((iouReport?.total ?? 0) * -1, iouReport?.currency ?? ''),
CurrencyUtils.convertToDisplayString((iouReport?.unheldTotal ?? 0) * -1, iouReport?.currency ?? CONST.CURRENCY.USD),
CurrencyUtils.convertToDisplayString((iouReport?.total ?? 0) * -1, iouReport?.currency ?? CONST.CURRENCY.USD),
nkdengineer marked this conversation as resolved.
Show resolved Hide resolved
];
}

Expand Down
Loading