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

Fix bug with Payment option is not selected #49345

Merged
merged 10 commits into from
Sep 18, 2024
Merged

Fix bug with Payment option is not selected #49345

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
2f95fd9
Fix bug with Payment option is not selected
ZhenjaHorbach Sep 17, 2024
60d0752
Update comment
ZhenjaHorbach Sep 17, 2024
5a90681
Update comment
ZhenjaHorbach Sep 17, 2024
a56d546
Revert come shanges
ZhenjaHorbach Sep 17, 2024
c965330
Update dependencies for useMemo
ZhenjaHorbach Sep 18, 2024
f567e7f
Remove redundant code
ZhenjaHorbach Sep 18, 2024
c07cd00
Update comment
ZhenjaHorbach Sep 18, 2024
2e8005d
Update comment x2
ZhenjaHorbach Sep 18, 2024
8b1fd4f
Update code to prevent potencial issues
ZhenjaHorbach Sep 18, 2024
3c8ad57
Update code to use isLoadingOnyxValue
ZhenjaHorbach Sep 18, 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
35 changes: 19 additions & 16 deletions src/components/MoneyReportHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,22 +145,25 @@ function MoneyReportHeader({policy, report: moneyRequestReport, transactionThrea
const {isDelegateAccessRestricted, delegatorEmail} = useDelegateUserDetails();
const [isNoDelegateAccessMenuVisible, setIsNoDelegateAccessMenuVisible] = useState(false);

const confirmPayment = (type?: PaymentMethodType | undefined, payAsBusiness?: boolean) => {
if (!type || !chatReport) {
return;
}
setPaymentType(type);
setRequestType(CONST.IOU.REPORT_ACTION_TYPE.PAY);
if (isDelegateAccessRestricted) {
setIsNoDelegateAccessMenuVisible(true);
} else if (isAnyTransactionOnHold) {
setIsHoldMenuVisible(true);
} else if (ReportUtils.isInvoiceReport(moneyRequestReport)) {
IOU.payInvoice(type, chatReport, moneyRequestReport, payAsBusiness);
} else {
IOU.payMoneyRequest(type, chatReport, moneyRequestReport, true);
}
};
const confirmPayment = useCallback(
(type?: PaymentMethodType | undefined, payAsBusiness?: boolean) => {
if (!type || !chatReport) {
return;
}
setPaymentType(type);
setRequestType(CONST.IOU.REPORT_ACTION_TYPE.PAY);
if (isDelegateAccessRestricted) {
setIsNoDelegateAccessMenuVisible(true);
} else if (isAnyTransactionOnHold) {
setIsHoldMenuVisible(true);
} else if (ReportUtils.isInvoiceReport(moneyRequestReport)) {
IOU.payInvoice(type, chatReport, moneyRequestReport, payAsBusiness);
} else {
IOU.payMoneyRequest(type, chatReport, moneyRequestReport, true);
}
},
[chatReport, isAnyTransactionOnHold, isDelegateAccessRestricted, moneyRequestReport],
);

const confirmApproval = () => {
setRequestType(CONST.IOU.REPORT_ACTION_TYPE.APPROVE);
Expand Down
41 changes: 22 additions & 19 deletions src/components/ReportActionItem/ReportPreview.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,26 +199,29 @@ function ReportPreview({
const [isNoDelegateAccessMenuVisible, setIsNoDelegateAccessMenuVisible] = useState(false);

const stopAnimation = useCallback(() => setIsPaidAnimationRunning(false), []);
const confirmPayment = (type: PaymentMethodType | undefined, payAsBusiness?: boolean) => {
if (!type) {
return;
}
setPaymentType(type);
setRequestType(CONST.IOU.REPORT_ACTION_TYPE.PAY);
if (isDelegateAccessRestricted) {
setIsNoDelegateAccessMenuVisible(true);
} else if (ReportUtils.hasHeldExpenses(iouReport?.reportID)) {
setIsHoldMenuVisible(true);
} else if (chatReport && iouReport) {
setIsPaidAnimationRunning(true);
HapticFeedback.longPress();
if (ReportUtils.isInvoiceReport(iouReport)) {
IOU.payInvoice(type, chatReport, iouReport, payAsBusiness);
} else {
IOU.payMoneyRequest(type, chatReport, iouReport);
const confirmPayment = useCallback(
(type: PaymentMethodType | undefined, payAsBusiness?: boolean) => {
if (!type) {
return;
}
}
};
setPaymentType(type);
setRequestType(CONST.IOU.REPORT_ACTION_TYPE.PAY);
if (isDelegateAccessRestricted) {
setIsNoDelegateAccessMenuVisible(true);
} else if (ReportUtils.hasHeldExpenses(iouReport?.reportID)) {
setIsHoldMenuVisible(true);
} else if (chatReport && iouReport) {
setIsPaidAnimationRunning(true);
HapticFeedback.longPress();
if (ReportUtils.isInvoiceReport(iouReport)) {
IOU.payInvoice(type, chatReport, iouReport, payAsBusiness);
} else {
IOU.payMoneyRequest(type, chatReport, iouReport);
}
}
},
[chatReport, iouReport, isDelegateAccessRestricted],
);

const confirmApproval = () => {
setRequestType(CONST.IOU.REPORT_ACTION_TYPE.APPROVE);
Expand Down
9 changes: 7 additions & 2 deletions src/components/SettlementButton/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,14 @@ function SettlementButton({
const {isOffline} = useNetwork();
// The app would crash due to subscribing to the entire report collection if chatReportID is an empty string. So we should have a fallback ID here.
const [chatReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${chatReportID || -1}`);
const [lastPaymentMethod = '-1'] = useOnyx(ONYXKEYS.NVP_LAST_PAYMENT_METHOD, {selector: (paymentMethod) => paymentMethod?.[policyID]});
const [nvpLastPaymentMethod] = useOnyx(ONYXKEYS.NVP_LAST_PAYMENT_METHOD);
const nvpLastPaymentMethodIsInitialized = nvpLastPaymentMethod !== undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain this change?

Copy link
Contributor Author

@ZhenjaHorbach ZhenjaHorbach Sep 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The purpose of these changes was to make sure that we only update the Options list by PaymentMethod when we have a PaymentMethod initialized (when it changes from undefined to object )
Other times we don't do this

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 should use a build in feature of useOnyx to handle this if we want https://github.com/Expensify/App/blob/b7c0055c531f668c683cc048cb01f6f6afc33f97/src/pages/home/ReportScreen.tsx#L135C5-L135C66

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes
It works the same as comparing with undefined
But looks cleaner !
Thanks

const [policy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`);
const isInvoiceReport = (!isEmptyObject(iouReport) && ReportUtils.isInvoiceReport(iouReport)) || false;
const shouldShowPaywithExpensifyOption = !shouldHidePaymentOptions && policy?.reimbursementChoice !== CONST.POLICY.REIMBURSEMENT_CHOICES.REIMBURSEMENT_MANUAL;
const shouldShowPayElsewhereOption = !shouldHidePaymentOptions && !isInvoiceReport;
const paymentButtonOptions = useMemo(() => {
const lastPaymentMethod = nvpLastPaymentMethod?.[policyID] ?? '-1';
const buttonOptions = [];
const isExpenseReport = ReportUtils.isExpenseReport(iouReport);
const paymentMethods = {
Expand Down Expand Up @@ -154,9 +156,12 @@ function SettlementButton({
return buttonOptions.sort((method) => (method.value === lastPaymentMethod ? -1 : 0));
}
return buttonOptions;
// We don't want to reorder the options when the preferred payment method changes while the button is still visible
// We don't want to reorder the options when the preferred payment method changes while the button is still visible except for component initialization when the last payment method is not initialized yet.
// We need to be sure that onPress should be wrapped in an useCallback to prevent unnecessary updates
// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
}, [
policyID,
nvpLastPaymentMethodIsInitialized,
iouReport,
translate,
formattedAmount,
Expand Down
Loading