Skip to content

Commit

Permalink
Merge pull request #38892 from bernhardoj/fix/37643-distance-request-…
Browse files Browse the repository at this point in the history 
…shows-wrong-currency-initially

Fix created distance request briefly shows the user local currency instead of the workspace currency
  • Loading branch information
@roryabraham
roryabraham authored Apr 15, 2024
2 parents 2d19c95 + fdf9748 commit 02059dd
Show file tree
Hide file tree
Showing 7 changed files with 83 additions and 122 deletions.
4 changes: 2 additions & 2 deletions src/ROUTES.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -352,8 +352,8 @@ const ROUTES = {
},
MONEY_REQUEST_STEP_CURRENCY: {
route: ':action/:iouType/currency/:transactionID/:reportID/:pageIndex?',
getRoute: (action: ValueOf<typeof CONST.IOU.ACTION>, iouType: ValueOf<typeof CONST.IOU.TYPE>, transactionID: string, reportID: string, pageIndex = '', backTo = '') =>
getUrlWithBackToParam(`${action}/${iouType}/currency/${transactionID}/${reportID}/${pageIndex}`, backTo),
getRoute: (action: ValueOf<typeof CONST.IOU.ACTION>, iouType: ValueOf<typeof CONST.IOU.TYPE>, transactionID: string, reportID: string, pageIndex = '', currency = '', backTo = '') =>
getUrlWithBackToParam(`${action}/${iouType}/currency/${transactionID}/${reportID}/${pageIndex}?currency=${currency}`, backTo),
},
MONEY_REQUEST_STEP_DATE: {
route: ':action/:iouType/date/:transactionID/:reportID',
Expand Down
4 changes: 4 additions & 0 deletions src/libs/Navigation/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,7 @@ type MoneyRequestNavigatorParamList = {
transactionID: string;
reportID: string;
backTo: Routes;
currency?: string;
};
[SCREENS.MONEY_REQUEST.STEP_TAG]: {
action: ValueOf<typeof CONST.IOU.ACTION>;
Expand Down Expand Up @@ -459,6 +460,7 @@ type MoneyRequestNavigatorParamList = {
// for IOURequestStepDistance and IOURequestStepAmount components
backTo: never;
action: never;
currency: never;
};
[SCREENS.MONEY_REQUEST.START]: {
iouType: ValueOf<typeof CONST.IOU.TYPE>;
Expand All @@ -472,6 +474,7 @@ type MoneyRequestNavigatorParamList = {
transactionID: string;
backTo: Routes;
action: ValueOf<typeof CONST.IOU.ACTION>;
currency?: string;
};
[SCREENS.MONEY_REQUEST.STEP_PARTICIPANTS]: {
iouType: ValueOf<typeof CONST.IOU.TYPE>;
Expand Down Expand Up @@ -501,6 +504,7 @@ type MoneyRequestNavigatorParamList = {
reportID: string;
pageIndex?: string;
backTo?: Routes;
currency?: string;
};
};

Expand Down
18 changes: 2 additions & 16 deletions src/libs/actions/IOU.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -371,11 +371,7 @@ function startMoneyRequest(iouType: ValueOf<typeof CONST.IOU.TYPE>, reportID: st
}

// eslint-disable-next-line @typescript-eslint/naming-convention
function setMoneyRequestAmount_temporaryForRefactor(transactionID: string, amount: number, currency: string, removeOriginalCurrency = false) {
if (removeOriginalCurrency) {
Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${transactionID}`, {amount, currency, originalCurrency: null});
return;
}
function setMoneyRequestAmount_temporaryForRefactor(transactionID: string, amount: number, currency: string) {
Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${transactionID}`, {amount, currency});
}

Expand All @@ -385,20 +381,11 @@ function setMoneyRequestCreated(transactionID: string, created: string, isDraft:
}

// eslint-disable-next-line @typescript-eslint/naming-convention
function setMoneyRequestCurrency_temporaryForRefactor(transactionID: string, currency: string, removeOriginalCurrency = false, isEditing = false) {
function setMoneyRequestCurrency_temporaryForRefactor(transactionID: string, currency: string, isEditing = false) {
const fieldToUpdate = isEditing ? 'modifiedCurrency' : 'currency';
if (removeOriginalCurrency) {
Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${transactionID}`, {[fieldToUpdate]: currency, originalCurrency: null});
return;
}
Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${transactionID}`, {[fieldToUpdate]: currency});
}

// eslint-disable-next-line @typescript-eslint/naming-convention
function setMoneyRequestOriginalCurrency_temporaryForRefactor(transactionID: string, originalCurrency: string) {
Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${transactionID}`, {originalCurrency});
}

function setMoneyRequestDescription(transactionID: string, comment: string, isDraft: boolean) {
Onyx.merge(`${isDraft ? ONYXKEYS.COLLECTION.TRANSACTION_DRAFT : ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, {comment: {comment: comment.trim()}});
}
Expand Down Expand Up @@ -5567,7 +5554,6 @@ export {
setMoneyRequestCreated,
setMoneyRequestCurrency_temporaryForRefactor,
setMoneyRequestDescription,
setMoneyRequestOriginalCurrency_temporaryForRefactor,
setMoneyRequestParticipants_temporaryForRefactor,
setMoneyRequestPendingFields,
setMoneyRequestReceipt,
Expand Down
40 changes: 16 additions & 24 deletions src/pages/iou/request/step/IOURequestStepAmount.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ type IOURequestStepAmountProps = IOURequestStepAmountOnyxProps &
function IOURequestStepAmount({
report,
route: {
params: {iouType, reportID, transactionID, backTo, action},
params: {iouType, reportID, transactionID, backTo, action, currency: selectedCurrency = ''},
},
transaction,
splitDraftTransaction,
Expand All @@ -54,13 +54,13 @@ function IOURequestStepAmount({
const textInput = useRef<BaseTextInputRef | null>(null);
const focusTimeoutRef = useRef<NodeJS.Timeout | null>(null);
const isSaveButtonPressed = useRef(false);
const originalCurrency = useRef<string | null>(null);
const iouRequestType = getRequestType(transaction);
const isEditing = action === CONST.IOU.ACTION.EDIT;
const isSplitBill = iouType === CONST.IOU.TYPE.SPLIT;
const isEditingSplitBill = isEditing && isSplitBill;
const {amount: transactionAmount} = ReportUtils.getTransactionDetails(isEditingSplitBill && !isEmptyObject(splitDraftTransaction) ? splitDraftTransaction : transaction) ?? {amount: 0};
const {currency} = ReportUtils.getTransactionDetails(isEditing ? draftTransaction : transaction) ?? {currency: CONST.CURRENCY.USD};
const {currency: originalCurrency} = ReportUtils.getTransactionDetails(isEditing ? draftTransaction : transaction) ?? {currency: CONST.CURRENCY.USD};
const currency = CurrencyUtils.isValidCurrencyCode(selectedCurrency) ? selectedCurrency : originalCurrency;

useFocusEffect(
useCallback(() => {
Expand All @@ -75,30 +75,19 @@ function IOURequestStepAmount({
);

useEffect(() => {
if (isEditing) {
// A temporary solution to not prevent users from editing the currency
// We create a backup transaction and use it to save the currency and remove this transaction backup if we don't save the amount
// It should be removed after this issue https://github.com/Expensify/App/issues/34607 is fixed
TransactionEdit.createBackupTransaction(isEditingSplitBill && !isEmptyObject(splitDraftTransaction) ? splitDraftTransaction : transaction);

return () => {
if (isSaveButtonPressed.current) {
return;
}
TransactionEdit.removeBackupTransaction(transaction?.transactionID ?? '');
};
}
if (transaction?.originalCurrency) {
originalCurrency.current = transaction.originalCurrency;
} else {
originalCurrency.current = currency;
IOU.setMoneyRequestOriginalCurrency_temporaryForRefactor(transactionID, currency);
if (!isEditing) {
return;
}
// A temporary solution to not prevent users from editing the currency
// We create a backup transaction and use it to save the currency and remove this transaction backup if we don't save the amount
// It should be removed after this issue https://github.com/Expensify/App/issues/34607 is fixed
TransactionEdit.createBackupTransaction(isEditingSplitBill && !isEmptyObject(splitDraftTransaction) ? splitDraftTransaction : transaction);

return () => {
if (isSaveButtonPressed.current) {
return;
}
IOU.setMoneyRequestCurrency_temporaryForRefactor(transactionID, originalCurrency.current ?? '', true);
TransactionEdit.removeBackupTransaction(transaction?.transactionID ?? '');
};
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);
Expand All @@ -108,14 +97,17 @@ function IOURequestStepAmount({
};

const navigateToCurrencySelectionPage = () => {
Navigation.navigate(ROUTES.MONEY_REQUEST_STEP_CURRENCY.getRoute(action, iouType, transactionID, reportID, backTo ? 'confirm' : '', Navigation.getActiveRouteWithoutParams()));
Navigation.navigate(
ROUTES.MONEY_REQUEST_STEP_CURRENCY.getRoute(action, iouType, transactionID, reportID, backTo ? 'confirm' : '', currency, Navigation.getActiveRouteWithoutParams()),
);
};

const navigateToNextPage = ({amount}: AmountParams) => {
isSaveButtonPressed.current = true;
const amountInSmallestCurrencyUnits = CurrencyUtils.convertToBackendAmount(Number.parseFloat(amount));

IOU.setMoneyRequestAmount_temporaryForRefactor(transactionID, amountInSmallestCurrencyUnits, currency || CONST.CURRENCY.USD, true);
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
IOU.setMoneyRequestAmount_temporaryForRefactor(transactionID, amountInSmallestCurrencyUnits, currency || CONST.CURRENCY.USD);

if (backTo) {
Navigation.goBack(backTo);
Expand Down
79 changes: 31 additions & 48 deletions src/pages/iou/request/step/IOURequestStepConfirmation.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react';
import {View} from 'react-native';
import type {OnyxEntry} from 'react-native-onyx';
import {withOnyx} from 'react-native-onyx';
import FullScreenLoadingIndicator from '@components/FullscreenLoadingIndicator';
import HeaderWithBackButton from '@components/HeaderWithBackButton';
import * as Expensicons from '@components/Icon/Expensicons';
import MoneyRequestConfirmationList from '@components/MoneyTemporaryForRefactorRequestConfirmationList';
Expand Down Expand Up @@ -103,15 +102,6 @@ function IOURequestStepConfirmation({
const isPolicyExpenseChat = useMemo(() => ReportUtils.isPolicyExpenseChat(ReportUtils.getRootParentReport(report)), [report]);
const formHasBeenSubmitted = useRef(false);

useEffect(() => {
if (!transaction?.originalCurrency) {
return;
}
// If user somehow lands on this page without the currency reset, then reset it here.
IOU.setMoneyRequestCurrency_temporaryForRefactor(transactionID, transaction.originalCurrency, true);
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);

useEffect(() => {
const policyExpenseChat = participants?.find((participant) => participant.isPolicyExpenseChat);
if (policyExpenseChat?.policyID) {
Expand Down Expand Up @@ -461,10 +451,6 @@ function IOURequestStepConfirmation({
IOU.setMoneyRequestBillable_temporaryForRefactor(transactionID, billable);
};

// This loading indicator is shown because the transaction originalCurrency is being updated later than the component mounts.
// To prevent the component from rendering with the wrong currency, we show a loading indicator until the correct currency is set.
const isLoading = !!transaction?.originalCurrency;

return (
<ScreenWrapper
includeSafeAreaPaddingBottom={false}
Expand All @@ -486,40 +472,37 @@ function IOURequestStepConfirmation({
},
]}
/>
{isLoading && <FullScreenLoadingIndicator />}
<View style={[styles.flex1, isLoading && styles.opacity0]}>
<MoneyRequestConfirmationList
transaction={transaction}
hasMultipleParticipants={iouType === CONST.IOU.TYPE.SPLIT}
selectedParticipants={participants}
iouAmount={transaction?.amount ?? 0}
iouComment={transaction?.comment.comment ?? ''}
iouCurrencyCode={transaction?.currency}
iouIsBillable={transaction?.billable}
onToggleBillable={setBillable}
iouCategory={transaction?.category}
onConfirm={createTransaction}
onSendMoney={sendMoney}
onSelectParticipant={addNewParticipant}
receiptPath={receiptPath}
receiptFilename={receiptFilename}
iouType={iouType}
reportID={reportID}
isPolicyExpenseChat={isPolicyExpenseChat}
// The participants can only be modified when the action is initiated from directly within a group chat and not the floating-action-button.
// This is because when there is a group of people, say they are on a trip, and you have some shared expenses with some of the people,
// but not all of them (maybe someone skipped out on dinner). Then it's nice to be able to select/deselect people from the group chat bill
// split rather than forcing the user to create a new group, just for that expense. The reportID is empty, when the action was initiated from
// the floating-action-button (since it is something that exists outside the context of a report).
canModifyParticipants={!transaction?.isFromGlobalCreate}
policyID={report?.policyID}
bankAccountRoute={ReportUtils.getBankAccountRoute(report)}
iouMerchant={transaction?.merchant}
iouCreated={transaction?.created}
isDistanceRequest={requestType === CONST.IOU.REQUEST_TYPE.DISTANCE}
shouldShowSmartScanFields={requestType !== CONST.IOU.REQUEST_TYPE.SCAN}
/>
</View>
<MoneyRequestConfirmationList
transaction={transaction}
hasMultipleParticipants={iouType === CONST.IOU.TYPE.SPLIT}
selectedParticipants={participants}
iouAmount={transaction?.amount ?? 0}
iouComment={transaction?.comment.comment ?? ''}
iouCurrencyCode={transaction?.currency}
iouIsBillable={transaction?.billable}
onToggleBillable={setBillable}
iouCategory={transaction?.category}
onConfirm={createTransaction}
onSendMoney={sendMoney}
onSelectParticipant={addNewParticipant}
receiptPath={receiptPath}
receiptFilename={receiptFilename}
iouType={iouType}
reportID={reportID}
isPolicyExpenseChat={isPolicyExpenseChat}
// The participants can only be modified when the action is initiated from directly within a group chat and not the floating-action-button.
// This is because when there is a group of people, say they are on a trip, and you have some shared expenses with some of the people,
// but not all of them (maybe someone skipped out on dinner). Then it's nice to be able to select/deselect people from the group chat bill
// split rather than forcing the user to create a new group, just for that expense. The reportID is empty, when the action was initiated from
// the floating-action-button (since it is something that exists outside the context of a report).
canModifyParticipants={!transaction?.isFromGlobalCreate}
policyID={report?.policyID}
bankAccountRoute={ReportUtils.getBankAccountRoute(report)}
iouMerchant={transaction?.merchant}
iouCreated={transaction?.created}
isDistanceRequest={requestType === CONST.IOU.REQUEST_TYPE.DISTANCE}
shouldShowSmartScanFields={requestType !== CONST.IOU.REQUEST_TYPE.SCAN}
/>
</View>
)}
</ScreenWrapper>
Expand Down
21 changes: 14 additions & 7 deletions src/pages/iou/request/step/IOURequestStepCurrency.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,16 @@ type CurrencyListItem = ListItem & {
function IOURequestStepCurrency({
currencyList,
route: {
params: {backTo, iouType, pageIndex, reportID, transactionID, action},
params: {backTo, iouType, pageIndex, reportID, transactionID, action, currency: selectedCurrency = ''},
},
draftTransaction,
}: IOURequestStepCurrencyProps) {
const {translate} = useLocalize();
const [searchValue, setSearchValue] = useState('');
const {currency = ''} = ReportUtils.getTransactionDetails(draftTransaction) ?? {};
const {currency: originalCurrency = ''} = ReportUtils.getTransactionDetails(draftTransaction) ?? {};
const currency = CurrencyUtils.isValidCurrencyCode(selectedCurrency) ? selectedCurrency : originalCurrency;

const navigateBack = () => {
const navigateBack = (selectedCurrencyValue = '') => {
// If the currency selection was done from the confirmation step (eg. + > request money > manual > confirm > amount > currency)
// then the user needs taken back to the confirmation page instead of the initial amount page. This is because the route params
// are only able to handle one backTo param at a time and the user needs to go back to the amount page before going back
Expand All @@ -57,16 +58,22 @@ function IOURequestStepCurrency({
backTo as string,
`/${ROUTES.MONEY_REQUEST_STEP_CONFIRMATION.getRoute(CONST.IOU.ACTION.CREATE, iouType, transactionID, reportID)}`,
);
Navigation.goBack(routeToAmountPageWithConfirmationAsBackTo as Route);
if (selectedCurrencyValue) {
Navigation.navigate(`${routeToAmountPageWithConfirmationAsBackTo}&currency=${selectedCurrencyValue}` as Route);
} else {
Navigation.goBack(routeToAmountPageWithConfirmationAsBackTo as Route);
}
return;
}
Navigation.goBack(backTo);
};

const confirmCurrencySelection = (option: CurrencyListItem) => {
Keyboard.dismiss();
IOU.setMoneyRequestCurrency_temporaryForRefactor(transactionID, option.currencyCode, false, action === CONST.IOU.ACTION.EDIT);
navigateBack();
if (pageIndex !== 'confirm') {
IOU.setMoneyRequestCurrency_temporaryForRefactor(transactionID, option.currencyCode, action === CONST.IOU.ACTION.EDIT);
}
navigateBack(option.currencyCode);
};

const {sections, headerMessage, initiallyFocusedOptionKey} = useMemo(() => {
Expand Down Expand Up @@ -101,7 +108,7 @@ function IOURequestStepCurrency({
return (
<StepScreenWrapper
headerTitle={translate('common.selectCurrency')}
onBackButtonPress={navigateBack}
onBackButtonPress={() => navigateBack()}
shouldShowWrapper
testID={IOURequestStepCurrency.displayName}
includeSafeAreaPaddingBottom={false}
Expand Down
Loading

0 comments on commit 02059dd

Please sign in to comment.