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.

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

fix: remove settled transactions from duplicates #47479

Merged
merged 9 commits into from
Aug 29, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -123,14 +123,20 @@ function MoneyRequestPreviewContent({
const isFullyApproved = ReportUtils.isReportApproved(iouReport) && !isSettlementOrApprovalPartial;

// Get transaction violations for given transaction id from onyx, find duplicated transactions violations and get duplicates
const duplicates = useMemo(
const allDuplicates = useMemo(
() =>
transactionViolations?.[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transaction?.transactionID}`]?.find(
(violation) => violation.name === CONST.VIOLATIONS.DUPLICATED_TRANSACTION,
)?.data?.duplicates ?? [],
[transaction?.transactionID, transactionViolations],
);

// Remove settled transactions from duplicates
const duplicates = TransactionUtils.removeSettledTransactions(allDuplicates);
daledah marked this conversation as resolved.
Show resolved Hide resolved

// When there are no settled transactions in duplicates, show the "Keep this one" button
const shouldShowKeepButton = allDuplicates.length === duplicates.length;
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to check if there are any elements in duplicates before making this comparison. More details here #48366


const hasDuplicates = duplicates.length > 0;

const shouldShowRBR = hasNoticeTypeViolations || hasViolations || hasFieldErrors || (!isFullySettled && !isFullyApproved && isOnHold) || hasDuplicates;
Expand Down Expand Up @@ -438,7 +444,7 @@ function MoneyRequestPreviewContent({
]}
>
{childContainer}
{isReviewDuplicateTransactionPage && (
{isReviewDuplicateTransactionPage && !isSettled && shouldShowKeepButton && (
<Button
text={translate('violations.keepThisOne')}
success
Expand Down
1 change: 1 addition & 0 deletions src/languages/en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -846,6 +846,7 @@ export default {
expenseOnHold: 'This expense was put on hold. Please review the comments for next steps.',
expensesOnHold: 'All expenses were put on hold. Please review the comments for next steps.',
expenseDuplicate: 'This expense has the same details as another one. Please review the duplicates to remove the hold.',
someDuplicatesArePaid: 'Some of these duplicates have been approved or paid already.',
reviewDuplicates: 'Review duplicates',
keepAll: 'Keep all',
confirmApprove: 'Confirm approval amount',
Expand Down
1 change: 1 addition & 0 deletions src/languages/es.ts
Original file line number Diff line number Diff line change
Expand Up @@ -840,6 +840,7 @@ export default {
expenseOnHold: 'Este gasto está bloqueado. Revisa los comentarios para saber como proceder.',
expensesOnHold: 'Todos los gastos quedaron bloqueados. Revisa los comentarios para saber como proceder.',
expenseDuplicate: 'Esta solicitud tiene los mismos detalles que otra. Revisa los duplicados para eliminar el bloqueo.',
someDuplicatesArePaid: 'Algunos de estos duplicados ya han sido aprobados o pagados.',
reviewDuplicates: 'Revisar duplicados',
keepAll: 'Mantener todos',
confirmApprove: 'Confirmar importe a aprobar',
Expand Down
10 changes: 8 additions & 2 deletions src/libs/TransactionUtils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import * as PolicyUtils from '@libs/PolicyUtils';
// eslint-disable-next-line import/no-cycle
import * as ReportActionsUtils from '@libs/ReportActionsUtils';
import * as ReportConnection from '@libs/ReportConnection';
import * as ReportUtils from '@libs/ReportUtils';
import type {IOURequestType} from '@userActions/IOU';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
Expand Down Expand Up @@ -895,6 +896,10 @@ type FieldsToChange = {
reimbursable?: Array<boolean | undefined>;
};

function removeSettledTransactions(transactionIDs: string[]) {
return transactionIDs.filter((transactionID) => !ReportUtils.isSettled(allTransactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`]?.reportID));
}

/**
* This function compares fields of duplicate transactions and determines which fields should be kept and which should be changed.
*
Expand All @@ -916,7 +921,7 @@ type FieldsToChange = {
function compareDuplicateTransactionFields(transactionID: string): {keep: Partial<ReviewDuplicates>; change: FieldsToChange} {
const transactionViolations = allTransactionViolations?.[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transactionID}`];
const duplicates = transactionViolations?.find((violation) => violation.name === CONST.VIOLATIONS.DUPLICATED_TRANSACTION)?.data?.duplicates ?? [];
const transactions = [transactionID, ...duplicates].map((item) => getTransaction(item));
const transactions = removeSettledTransactions([transactionID, ...duplicates]).map((item) => getTransaction(item));
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const keep: Record<string, any> = {};
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Expand Down Expand Up @@ -1030,7 +1035,7 @@ function buildTransactionsMergeParams(reviewDuplicates: OnyxEntry<ReviewDuplicat
currency: getCurrency(originalTransaction as OnyxEntry<Transaction>),
created: getFormattedCreated(originalTransaction as OnyxEntry<Transaction>),
transactionID: reviewDuplicates?.transactionID ?? '',
transactionIDList: reviewDuplicates?.duplicates ?? [],
transactionIDList: removeSettledTransactions(reviewDuplicates?.duplicates ?? []),
billable: reviewDuplicates?.billable ?? false,
reimbursable: reviewDuplicates?.reimbursable ?? false,
category: reviewDuplicates?.category ?? '',
Expand Down Expand Up @@ -1115,6 +1120,7 @@ export {
buildTransactionsMergeParams,
getReimbursable,
isPayAtEndExpense,
removeSettledTransactions,
getCardName,
};

Expand Down
5 changes: 5 additions & 0 deletions src/pages/TransactionDuplicate/Review.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,14 @@ import FullPageNotFoundView from '@components/BlockingViews/FullPageNotFoundView
import Button from '@components/Button';
import HeaderWithBackButton from '@components/HeaderWithBackButton';
import ScreenWrapper from '@components/ScreenWrapper';
import Text from '@components/Text';
import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails';
import useLocalize from '@hooks/useLocalize';
import useThemeStyles from '@hooks/useThemeStyles';
import Navigation from '@libs/Navigation/Navigation';
import type {TransactionDuplicateNavigatorParamList} from '@libs/Navigation/types';
import * as ReportActionsUtils from '@libs/ReportActionsUtils';
import * as ReportUtils from '@libs/ReportUtils';
import * as TransactionUtils from '@libs/TransactionUtils';
import * as Transaction from '@userActions/Transaction';
import CONST from '@src/CONST';
Expand Down Expand Up @@ -42,6 +44,8 @@ function TransactionDuplicateReview() {
Navigation.goBack();
};

const hasSettledTransaction = transactions.some((transaction) => ReportUtils.isSettled(transaction?.reportID));

return (
<ScreenWrapper testID={TransactionDuplicateReview.displayName}>
<FullPageNotFoundView shouldShow={transactionID === '-1'}>
Expand All @@ -51,6 +55,7 @@ function TransactionDuplicateReview() {
text={translate('iou.keepAll')}
onPress={keepAll}
/>
{!!hasSettledTransaction && <Text style={[styles.textNormal, styles.colorMuted, styles.mt3]}>{translate('iou.someDuplicatesArePaid')}</Text>}
</View>
<DuplicateTransactionsList transactions={transactions} />
</FullPageNotFoundView>
Expand Down
Loading