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

feat: surfacing potential duplicates #40153

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
b7cdaf8
feat: surfacing potential duplicates
gijoe0295 Apr 11, 2024
513909a
revert mistake changes
gijoe0295 Apr 11, 2024
473adaa
Merge branch 'main' of https://github.com/gijoe0295/App into gijoe/fe…
gijoe0295 Apr 12, 2024
9aba989
Merge branch 'main' of https://github.com/gijoe0295/App into gijoe/fe…
gijoe0295 Apr 15, 2024
369585f
modify copy
gijoe0295 Apr 15, 2024
3843a61
resolve conflicts
gijoe0295 Apr 19, 2024
20b4048
show RBR on violation preview
gijoe0295 Apr 19, 2024
38c9188
resolve conflicts
gijoe0295 Apr 25, 2024
4a6a285
show hold message on iou preview
gijoe0295 Apr 25, 2024
462b3df
show rbr on report preview
gijoe0295 Apr 25, 2024
fd8e18c
resolve conflicts
gijoe0295 Apr 25, 2024
7e2d605
fix crash
gijoe0295 Apr 25, 2024
870b6ac
resolve conflicts
gijoe0295 Apr 30, 2024
7c3136f
Resolve conflicts
gijoe0295 May 2, 2024
e68f9f1
hide unhold expense for duplicate expenses
gijoe0295 May 2, 2024
3d844a0
revert unneccessary changes
gijoe0295 May 2, 2024
4947382
Update copy
gijoe0295 May 4, 2024
bd6693b
Merge main
gijoe0295 May 5, 2024
5e558a8
Merge branch 'gijoe/feat-surface-potential-duplicates' of https://git…
gijoe0295 May 5, 2024
22b6cd9
revert HoldBanner changes
gijoe0295 May 5, 2024
edafa52
Resolve conflicts
gijoe0295 May 7, 2024
8185218
Merge branch 'main' of https://github.com/gijoe0295/App into gijoe/fe…
gijoe0295 May 10, 2024
1e7acb4
Resolve conflicts
gijoe0295 May 10, 2024
795dc56
resolve conflicts
gijoe0295 May 14, 2024
951daeb
Merge branch 'main' of https://github.com/gijoe0295/App into gijoe/fe…
gijoe0295 May 15, 2024
742c12d
resolve conflicts
gijoe0295 May 16, 2024
fc07750
fix type error
gijoe0295 May 16, 2024
2c425b5
Merge branch 'main' of https://github.com/gijoe0295/App into gijoe/fe…
gijoe0295 May 16, 2024
796e531
Merge branch 'main' of https://github.com/gijoe0295/App into gijoe/fe…
gijoe0295 May 24, 2024
f934be5
Merge branch 'main' of https://github.com/gijoe0295/App into gijoe/fe…
gijoe0295 May 30, 2024
1060cc7
Merge branch 'main' of https://github.com/gijoe0295/App into gijoe/fe…
gijoe0295 Jun 1, 2024
10535a6
Merge branch 'main' of https://github.com/gijoe0295/App into gijoe/fe…
gijoe0295 Jun 6, 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
23 changes: 21 additions & 2 deletions src/components/MoneyRequestHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ function MoneyRequestHeader({report, parentReportAction, policy, shouldUseNarrow
const isApproved = ReportUtils.isReportApproved(moneyRequestReport);
const isDraft = ReportUtils.isOpenExpenseReport(moneyRequestReport);
const isOnHold = TransactionUtils.isOnHold(transaction);
const isDuplicate = TransactionUtils.isDuplicate(transaction?.transactionID ?? '');
pecanoro marked this conversation as resolved.
Show resolved Hide resolved
const {isSmallScreenWidth, windowWidth} = useWindowDimensions();

// Only the requestor can take delete the expense, admins can only edit it.
Expand Down Expand Up @@ -120,7 +121,7 @@ function MoneyRequestHeader({report, parentReportAction, policy, shouldUseNarrow

const getStatusBarProps: () => MoneyRequestHeaderStatusBarProps | undefined = () => {
if (isOnHold) {
return {title: translate('iou.hold'), description: translate('iou.expenseOnHold'), danger: true};
return {title: translate('iou.hold'), description: isDuplicate ? translate('iou.expenseDuplicate') : translate('iou.expenseOnHold'), danger: true};
}

if (TransactionUtils.isExpensifyCardTransaction(transaction) && TransactionUtils.isPending(transaction)) {
Expand Down Expand Up @@ -150,7 +151,7 @@ function MoneyRequestHeader({report, parentReportAction, policy, shouldUseNarrow
const isHoldCreator = ReportUtils.isHoldCreator(transaction, report?.reportID) && isRequestIOU;
const isTrackExpenseReport = ReportUtils.isTrackExpenseReport(report);
const canModifyStatus = !isTrackExpenseReport && (isPolicyAdmin || isActionOwner || isApprover);
if (isOnHold && (isHoldCreator || (!isRequestIOU && canModifyStatus))) {
if (isOnHold && !isDuplicate && (isHoldCreator || (!isRequestIOU && canModifyStatus))) {
threeDotsMenuItems.push({
icon: Expensicons.Stopwatch,
text: translate('iou.unholdExpense'),
Expand Down Expand Up @@ -224,6 +225,14 @@ function MoneyRequestHeader({report, parentReportAction, policy, shouldUseNarrow
onPress={markAsCash}
/>
)}
{isDuplicate && !shouldUseNarrowLayout && (
<Button
success
medium
text={translate('iou.reviewDuplicates')}
style={[styles.p0]}
/>
)}
</HeaderWithBackButton>
{shouldShowMarkAsCashButton && shouldUseNarrowLayout && (
<View style={[styles.ph5, styles.pb3]}>
Expand All @@ -236,6 +245,16 @@ function MoneyRequestHeader({report, parentReportAction, policy, shouldUseNarrow
/>
</View>
)}
{isDuplicate && shouldUseNarrowLayout && (
<View style={[styles.ph5, styles.pb3]}>
<Button
success
medium
text={translate('iou.reviewDuplicates')}
style={[styles.w100, styles.pr0]}
/>
</View>
)}
{statusBarProps && (
<View style={[styles.ph5, styles.pb3, styles.borderBottom]}>
<MoneyRequestHeaderStatusBar
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ function MoneyRequestPreviewContent({
const isFullySettled = isSettled && !isSettlementOrApprovalPartial;
const isFullyApproved = ReportUtils.isReportApproved(iouReport) && !isSettlementOrApprovalPartial;
const shouldShowRBR = hasNoticeTypeViolations || hasViolations || hasFieldErrors || (!isFullySettled && !isFullyApproved && isOnHold);
const shouldShowHoldMessage = !(isSettled && !isSettlementOrApprovalPartial) && isOnHold;

/*
Show the merchant for IOUs and expenses only if:
Expand Down Expand Up @@ -164,7 +165,11 @@ function MoneyRequestPreviewContent({
const isTooLong = violationsCount > 1 || violationMessage.length > 15;
const hasViolationsAndFieldErrors = violationsCount > 0 && hasFieldErrors;

return `${message} ${CONST.DOT_SEPARATOR} ${isTooLong || hasViolationsAndFieldErrors ? translate('violations.reviewRequired') : violationMessage}`;
message += ` ${CONST.DOT_SEPARATOR} ${isTooLong || hasViolationsAndFieldErrors ? translate('violations.reviewRequired') : violationMessage}`;
if (shouldShowHoldMessage) {
message += ` ${CONST.DOT_SEPARATOR} ${translate('iou.hold')}`;
}
return message;
}

const isMerchantMissing = TransactionUtils.isMerchantMissing(transaction);
Expand All @@ -175,7 +180,7 @@ function MoneyRequestPreviewContent({
message += ` ${CONST.DOT_SEPARATOR} ${translate('iou.missingAmount')}`;
} else if (isMerchantMissing) {
message += ` ${CONST.DOT_SEPARATOR} ${translate('iou.missingMerchant')}`;
} else if (!(isSettled && !isSettlementOrApprovalPartial) && isOnHold) {
} else if (shouldShowHoldMessage) {
message += ` ${CONST.DOT_SEPARATOR} ${translate('iou.hold')}`;
}
} else if (hasNoticeTypeViolations && transaction && !ReportUtils.isReportApproved(iouReport) && !ReportUtils.isSettled(iouReport?.reportID)) {
Expand All @@ -184,7 +189,7 @@ function MoneyRequestPreviewContent({
message += ` ${CONST.DOT_SEPARATOR} ${translate('iou.approved')}`;
} else if (iouReport?.isCancelledIOU) {
message += ` ${CONST.DOT_SEPARATOR} ${translate('iou.canceled')}`;
} else if (!(isSettled && !isSettlementOrApprovalPartial) && isOnHold) {
} else if (shouldShowHoldMessage) {
message += ` ${CONST.DOT_SEPARATOR} ${translate('iou.hold')}`;
}
return message;
Expand Down
8 changes: 5 additions & 3 deletions src/components/ReportActionItem/ReportPreview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,11 @@ function ReportPreview({

const hasReceipts = transactionsWithReceipts.length > 0;
const isScanning = hasReceipts && areAllRequestsBeingSmartScanned;

// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
const hasErrors = hasMissingSmartscanFields || (canUseViolations && ReportUtils.hasViolations(iouReportID, transactionViolations)) || ReportUtils.hasActionsWithErrors(iouReportID);
const hasErrors =
hasMissingSmartscanFields ||
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
(canUseViolations && (ReportUtils.hasViolations(iouReportID, transactionViolations) || ReportUtils.hasWarningTypeViolations(iouReportID, transactionViolations))) ||
ReportUtils.hasActionsWithErrors(iouReportID);
const lastThreeTransactionsWithReceipts = transactionsWithReceipts.slice(-3);
const lastThreeReceipts = lastThreeTransactionsWithReceipts.map((transaction) => ReceiptUtils.getThumbnailAndImageURIs(transaction));
const showRTERViolationMessage =
Expand Down
4 changes: 3 additions & 1 deletion src/languages/en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -763,6 +763,8 @@ export default {
holdReasonRequired: 'A reason is required when holding.',
expenseOnHold: 'This expense was put on hold. Review the comments for next steps.',
expensesOnHold: 'All expenses were put on hold. Review the comments for next steps.',
expenseDuplicate: 'This expense has the same details as another one. Review the duplicates to remove the hold.',
reviewDuplicates: 'Review duplicates',
confirmApprove: 'Confirm approval amount',
confirmApprovalAmount: "Approve what's not on hold, or approve the entire report.",
confirmPay: 'Confirm payment amount',
Expand Down Expand Up @@ -3116,7 +3118,7 @@ export default {
categoryOutOfPolicy: 'Category no longer valid',
conversionSurcharge: ({surcharge}: ViolationsConversionSurchargeParams) => `Applied ${surcharge}% conversion surcharge`,
customUnitOutOfPolicy: 'Unit no longer valid',
duplicatedTransaction: 'Potential duplicate',
duplicatedTransaction: 'Duplicate',
fieldRequired: 'Report fields are required',
futureDate: 'Future date not allowed',
invoiceMarkup: ({invoiceMarkup}: ViolationsInvoiceMarkupParams) => `Marked up by ${invoiceMarkup}%`,
Expand Down
4 changes: 3 additions & 1 deletion src/languages/es.ts
Original file line number Diff line number Diff line change
Expand Up @@ -758,6 +758,8 @@ export default {
holdReasonRequired: 'Se requiere una razón para bloquear.',
expenseOnHold: 'Este gasto está bloqueado. Revisa los comentarios para saber como proceder.',
expensesOnHold: 'Todos los gastos quedaron bloqueado. Revisa los comentarios para saber como proceder.',
expenseDuplicate: 'Esta solicitud tiene los mismos detalles que otra. Revise los duplicados para eliminar la retención.',
reviewDuplicates: 'Revisar duplicados',
confirmApprove: 'Confirmar importe a aprobar',
confirmApprovalAmount: 'Aprueba lo que no está bloqueado, o aprueba todo el informe.',
confirmPay: 'Confirmar importe de pago',
Expand Down Expand Up @@ -3620,7 +3622,7 @@ export default {
categoryOutOfPolicy: 'La categoría ya no es válida',
conversionSurcharge: ({surcharge}: ViolationsConversionSurchargeParams = {}) => `${surcharge}% de recargo aplicado`,
customUnitOutOfPolicy: 'La unidad ya no es válida',
duplicatedTransaction: 'Posible duplicado',
duplicatedTransaction: 'Duplicado',
fieldRequired: 'Los campos del informe son obligatorios',
futureDate: 'Fecha futura no permitida',
invoiceMarkup: ({invoiceMarkup}: ViolationsInvoiceMarkupParams) => `Incrementado un ${invoiceMarkup}%`,
Expand Down
13 changes: 11 additions & 2 deletions src/libs/ReportUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5160,7 +5160,7 @@ function shouldHideReport(report: OnyxEntry<Report>, currentReportId: string): b
}

/**
* Checks to see if a report's parentAction is an expense that contains a violation
* Checks to see if a report's parentAction is an expense that contains a violation type of either violation or warning
*/
function doesTransactionThreadHaveViolations(report: OnyxEntry<Report>, transactionViolations: OnyxCollection<TransactionViolation[]>, parentReportAction: OnyxEntry<ReportAction>): boolean {
if (parentReportAction?.actionName !== CONST.REPORT.ACTIONS.TYPE.IOU) {
Expand All @@ -5176,7 +5176,7 @@ function doesTransactionThreadHaveViolations(report: OnyxEntry<Report>, transact
if (report?.stateNum !== CONST.REPORT.STATE_NUM.OPEN && report?.stateNum !== CONST.REPORT.STATE_NUM.SUBMITTED) {
return false;
}
return TransactionUtils.hasViolation(IOUTransactionID, transactionViolations);
return TransactionUtils.hasViolation(IOUTransactionID, transactionViolations) || TransactionUtils.hasWarningTypeViolation(IOUTransactionID, transactionViolations);
}

/**
Expand All @@ -5202,6 +5202,14 @@ function hasViolations(reportID: string, transactionViolations: OnyxCollection<T
return transactions.some((transaction) => TransactionUtils.hasViolation(transaction.transactionID, transactionViolations));
}

/**
* Checks to see if a report contains a violation of type `warning`
*/
function hasWarningTypeViolations(reportID: string, transactionViolations: OnyxCollection<TransactionViolation[]>): boolean {
const transactions = TransactionUtils.getAllReportTransactions(reportID);
return transactions.some((transaction) => TransactionUtils.hasWarningTypeViolation(transaction.transactionID, transactionViolations));
}

/**
* Takes several pieces of data from Onyx and evaluates if a report should be shown in the option list (either when searching
* for reports or the reports shown in the LHN).
Expand Down Expand Up @@ -6970,6 +6978,7 @@ export {
hasSmartscanError,
hasUpdatedTotal,
hasViolations,
hasWarningTypeViolations,
isActionCreator,
isAdminRoom,
isAdminsOnlyPostingRoom,
Expand Down
40 changes: 39 additions & 1 deletion src/libs/TransactionUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,16 @@ Onyx.connect({
callback: (value) => (allReports = value),
});

let currentUserEmail = '';
let currentUserAccountID = -1;
Onyx.connect({
key: ONYXKEYS.SESSION,
callback: (val) => {
currentUserEmail = val?.email ?? '';
currentUserAccountID = val?.accountID ?? -1;
},
});

function isDistanceRequest(transaction: OnyxEntry<Transaction>): boolean {
// This is used during the expense creation flow before the transaction has been saved to the server
if (lodashHas(transaction, 'iouRequestType')) {
Expand Down Expand Up @@ -623,6 +633,23 @@ function getRecentTransactions(transactions: Record<string, string>, size = 2):
.slice(0, size);
}

/**
* Check if transaction has duplicatedTransaction violation.
* @param transactionID - the transaction to check
* @param checkDismissed - whether to check if the violation has already been dismissed as well
*/
function isDuplicate(transactionID: string, checkDismissed = false): boolean {
const hasDuplicatedViolation = !!allTransactionViolations?.[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transactionID}`]?.some(
(violation: TransactionViolation) => violation.name === CONST.VIOLATIONS.DUPLICATED_TRANSACTION,
);
if (!checkDismissed) {
return hasDuplicatedViolation;
}
const didDismissedViolation =
allTransactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`]?.comment?.dismissedViolations?.duplicatedTransaction?.[currentUserEmail] === `${currentUserAccountID}`;
return hasDuplicatedViolation && !didDismissedViolation;
}

/**
* Check if transaction is on hold
*/
Expand All @@ -631,7 +658,7 @@ function isOnHold(transaction: OnyxEntry<Transaction>): boolean {
return false;
}

return !!transaction.comment?.hold;
return !!transaction.comment?.hold || isDuplicate(transaction.transactionID, true);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

add isDuplicate to condition here cause normal "Unhold" option shows for duplicated expense. and we handled it here #44892


/**
Expand Down Expand Up @@ -661,6 +688,15 @@ function hasNoticeTypeViolation(transactionID: string, transactionViolations: On
return Boolean(transactionViolations?.[ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS + transactionID]?.some((violation: TransactionViolation) => violation.type === 'notice'));
}

/**
* Checks if any violations for the provided transaction are of type 'warning'
*/
function hasWarningTypeViolation(transactionID: string, transactionViolations: OnyxCollection<TransactionViolation[]>): boolean {
return Boolean(
transactionViolations?.[ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS + transactionID]?.some((violation: TransactionViolation) => violation.type === CONST.VIOLATION_TYPES.WARNING),
);
}

/**
* Calculates tax amount from the given expense amount and tax percentage
*/
Expand Down Expand Up @@ -798,6 +834,7 @@ export {
isFetchingWaypointsFromServer,
isExpensifyCardTransaction,
isCardTransaction,
isDuplicate,
isPending,
isPosted,
isOnHold,
Expand All @@ -818,6 +855,7 @@ export {
hasReservationList,
hasViolation,
hasNoticeTypeViolation,
hasWarningTypeViolation,
isCustomUnitRateIDForP2P,
getRateID,
};
Expand Down
2 changes: 2 additions & 0 deletions src/types/onyx/Transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import type {Participant, Split} from './IOU';
import type * as OnyxCommon from './OnyxCommon';
import type RecentWaypoint from './RecentWaypoint';
import type ReportAction from './ReportAction';
import type {ViolationName} from './TransactionViolation';

type Waypoint = {
/** The name associated with the address of the waypoint */
Expand Down Expand Up @@ -55,6 +56,7 @@ type Comment = {
source?: string;
originalTransactionID?: string;
splits?: Split[];
dismissedViolations?: Record<ViolationName, Record<string, string>>;
};

type TransactionCustomUnit = {
Expand Down
Loading