Skip to content

Commit

Permalink
Merge pull request #40153 from gijoe0295/gijoe/feat-surface-potential…
Browse files Browse the repository at this point in the history
…-duplicates

feat: surfacing potential duplicates
  • Loading branch information
pecanoro authored Jun 6, 2024
2 parents 6b70845 + 10535a6 commit 4679122
Show file tree
Hide file tree
Showing 8 changed files with 92 additions and 13 deletions.
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 ?? '');
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 @@ -104,6 +104,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 @@ -160,7 +161,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 @@ -171,7 +176,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 @@ -180,7 +185,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), 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 @@ -5209,7 +5209,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 @@ -5225,7 +5225,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 @@ -5251,6 +5251,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 @@ -7029,6 +7037,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);
}

/**
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

0 comments on commit 4679122

Please sign in to comment.