Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 6 additions & 7 deletions src/components/ReportActionItem/MoneyRequestView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ type MoneyRequestViewProps = {

parentReportID?: string;

/** Policy that the report belongs to, or the target transaction policy in merge transaction flow */
/** Policy that the report belongs to */
expensePolicy: OnyxEntry<OnyxTypes.Policy>;

/** Whether we should display the animated banner above the component */
Expand Down Expand Up @@ -311,10 +311,9 @@ function MoneyRequestView({
const shouldShowCard = isManagedCardTransaction && cardProgramName;

const taxRates = policy?.taxRates;
const formattedTaxAmount =
updatedTransaction?.taxAmount !== undefined
? convertToDisplayString(Math.abs(updatedTransaction.taxAmount), actualCurrency)
: convertToDisplayString(Math.abs(transactionTaxAmount ?? 0), actualCurrency);
const formattedTaxAmount = updatedTransaction?.taxAmount
? convertToDisplayString(Math.abs(updatedTransaction?.taxAmount), transactionCurrency)
: convertToDisplayString(Math.abs(transactionTaxAmount ?? 0), transactionCurrency);
Comment on lines +314 to +316

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Treat zero updated tax amounts as valid display values

The new truthy check for updatedTransaction?.taxAmount treats 0 as “no updated value”, so if a user changes tax to zero (for example via a 0% rate), the UI falls back to transactionTaxAmount and can show an outdated non-zero tax amount in the merge/preview display. The previous !== undefined behavior handled zero correctly, so this introduces incorrect totals in that scenario.

Useful? React with 👍 / 👎.


const taxRatesDescription = taxRates?.name;
const taxRateTitle = updatedTransaction ? getTaxName(policy, updatedTransaction, isExpenseUnreported) : getTaxName(policy, transaction, isExpenseUnreported);
Expand Down Expand Up @@ -413,7 +412,7 @@ function MoneyRequestView({
canEditFieldOfMoneyRequest(parentReportAction, CONST.EDIT_REQUEST_FIELD.REIMBURSABLE, undefined, isChatReportArchived, undefined, transaction, moneyRequestReport, policy);
const shouldShowAttendees = shouldShowAttendeesTransactionUtils(iouType, policy);

const shouldShowTax = !!transaction?.taxName || isTaxTrackingEnabled(isPolicyExpenseChat || isExpenseUnreported, policy, isDistanceRequest, isPerDiemRequest, isTimeRequest);
const shouldShowTax = isTaxTrackingEnabled(isPolicyExpenseChat || isExpenseUnreported, policy, isDistanceRequest, isPerDiemRequest, isTimeRequest);
const tripID = getTripIDFromTransactionParentReportID(parentReport?.parentReportID);
const shouldShowViewTripDetails = hasReservationList(transaction) && !!tripID;

Expand Down Expand Up @@ -601,7 +600,7 @@ function MoneyRequestView({
const decodedCategoryName = getDecodedCategoryName(categoryValue);
const categoryCopyValue = !canEdit ? decodedCategoryName : undefined;
const cardCopyValue = cardProgramName;
const taxRateValue = transaction?.taxName ?? taxRateTitle ?? fallbackTaxRateTitle;
const taxRateValue = taxRateTitle ?? fallbackTaxRateTitle;
const taxRateCopyValue = !canEditTaxFields ? taxRateValue : undefined;
const taxAmountTitle = formattedTaxAmount ? formattedTaxAmount.toString() : '';
const taxAmountCopyValue = !canEditTaxFields ? taxAmountTitle : undefined;
Expand Down
31 changes: 7 additions & 24 deletions src/hooks/useMergeTransactions.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
import type {OnyxEntry} from 'react-native-onyx';
import {useSearchContext} from '@components/Search/SearchContext';
import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID';
import {getReportIDForExpense, getTransactionFromMergeTransaction} from '@libs/MergeTransactionUtils';
import {isExpenseUnreported} from '@libs/TransactionUtils';
import {getTransactionFromMergeTransaction} from '@libs/MergeTransactionUtils';
import ONYXKEYS from '@src/ONYXKEYS';
import type {MergeTransaction, Policy, Report, SearchResults, Transaction} from '@src/types/onyx';
import type {MergeTransaction, Report, SearchResults, Transaction} from '@src/types/onyx';
import useOnyx from './useOnyx';
import usePolicyForMovingExpenses from './usePolicyForMovingExpenses';

type UseMergeTransactionsProps = {
mergeTransaction?: MergeTransaction;
Expand All @@ -17,8 +15,6 @@ type UseMergeTransactionsReturn = {
sourceTransaction?: Transaction;
targetTransactionReport?: Report;
sourceTransactionReport?: Report;
targetTransactionPolicy?: Policy;
sourceTransactionPolicy?: Policy;
};

function getTransaction(
Expand All @@ -41,40 +37,27 @@ function getTransaction(

function useMergeTransactions({mergeTransaction}: UseMergeTransactionsProps): UseMergeTransactionsReturn {
const {currentSearchHash, currentSearchResults} = useSearchContext();
const {policyForMovingExpensesID} = usePolicyForMovingExpenses();

const [onyxTargetTransaction] = useOnyx(`${ONYXKEYS.COLLECTION.TRANSACTION}${getNonEmptyStringOnyxID(mergeTransaction?.targetTransactionID)}`);
const [onyxSourceTransaction] = useOnyx(`${ONYXKEYS.COLLECTION.TRANSACTION}${getNonEmptyStringOnyxID(mergeTransaction?.sourceTransactionID)}`);

const targetTransaction = getTransaction(mergeTransaction, mergeTransaction?.targetTransactionID, onyxTargetTransaction, currentSearchResults);
const sourceTransaction = getTransaction(mergeTransaction, mergeTransaction?.sourceTransactionID, onyxSourceTransaction, currentSearchResults);

const targetTransactionReportID = getReportIDForExpense(targetTransaction);
const sourceTransactionReportID = getReportIDForExpense(sourceTransaction);
let [targetTransactionReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${getNonEmptyStringOnyxID(targetTransactionReportID)}`);
let [sourceTransactionReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${getNonEmptyStringOnyxID(sourceTransactionReportID)}`);

const targetTransactionPolicyID = isExpenseUnreported(targetTransaction) ? policyForMovingExpensesID : targetTransactionReport?.policyID;
const sourceTransactionPolicyID = isExpenseUnreported(sourceTransaction) ? policyForMovingExpensesID : sourceTransactionReport?.policyID;
let [targetTransactionPolicy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${getNonEmptyStringOnyxID(targetTransactionPolicyID)}`);
let [sourceTransactionPolicy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${getNonEmptyStringOnyxID(sourceTransactionPolicyID)}`);
let [targetTransactionReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${getNonEmptyStringOnyxID(targetTransaction?.reportID)}`);
let [sourceTransactionReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${getNonEmptyStringOnyxID(sourceTransaction?.reportID)}`);

// If we're on search and main collection reports are not available, get them from the search snapshot
if (currentSearchHash && currentSearchResults?.data) {
// If we're on search and main collection reports are not available, get them from the search snapshot
targetTransactionReport = targetTransactionReport ?? currentSearchResults?.data[`${ONYXKEYS.COLLECTION.REPORT}${targetTransactionReportID}`];
sourceTransactionReport = sourceTransactionReport ?? currentSearchResults?.data[`${ONYXKEYS.COLLECTION.REPORT}${sourceTransactionReportID}`];
// If we're on search, search snapshot policies are more up to date
targetTransactionPolicy = currentSearchResults?.data[`${ONYXKEYS.COLLECTION.POLICY}${targetTransactionPolicyID}`];
sourceTransactionPolicy = currentSearchResults?.data[`${ONYXKEYS.COLLECTION.POLICY}${sourceTransactionPolicyID}`];
targetTransactionReport = targetTransactionReport ?? currentSearchResults?.data[`${ONYXKEYS.COLLECTION.REPORT}${targetTransaction?.reportID}`];
sourceTransactionReport = sourceTransactionReport ?? currentSearchResults?.data[`${ONYXKEYS.COLLECTION.REPORT}${sourceTransaction?.reportID}`];
}

return {
targetTransaction,
sourceTransaction,
targetTransactionReport,
sourceTransactionReport,
targetTransactionPolicy,
sourceTransactionPolicy,
};
}

Expand Down
11 changes: 1 addition & 10 deletions src/hooks/useSelectedTransactionsActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -341,16 +341,7 @@ function useSelectedTransactionsActions({
text: translate('common.merge'),
icon: expensifyIcons.ArrowCollapse,
value: MERGE,
onSelected: () =>
setupMergeTransactionDataAndNavigate(
transactionID,
selectedTransactionsList,
localeCompare,
[],
false,
isOnSearch,
selectedTransactionsList.length > 1 ? [policy, policy] : undefined,
),
onSelected: () => setupMergeTransactionDataAndNavigate(transactionID, selectedTransactionsList, localeCompare, [], false, isOnSearch),
});
}
}
Expand Down
2 changes: 0 additions & 2 deletions src/libs/DebugUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -952,7 +952,6 @@ function validateTransactionDraftProperty(key: keyof Transaction, value: string)
case 'category':
case 'merchant':
case 'taxCode':
case 'taxName':
case 'modifiedCurrency':
case 'modifiedMerchant':
case 'transactionID':
Expand Down Expand Up @@ -1120,7 +1119,6 @@ function validateTransactionDraftProperty(key: keyof Transaction, value: string)
isDemoTransaction: CONST.RED_BRICK_ROAD_PENDING_ACTION,
splitExpensesTotal: CONST.RED_BRICK_ROAD_PENDING_ACTION,
taxValue: CONST.RED_BRICK_ROAD_PENDING_ACTION,
taxName: CONST.RED_BRICK_ROAD_PENDING_ACTION,
pendingAutoCategorizationTime: CONST.RED_BRICK_ROAD_PENDING_ACTION,
groupAmount: CONST.RED_BRICK_ROAD_PENDING_ACTION,
groupCurrency: CONST.RED_BRICK_ROAD_PENDING_ACTION,
Expand Down
Loading
Loading