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: [Violations] Distance - Incorrect error message when distance amount is changed to smaller amount. #41649

Merged
merged 43 commits into from
Jul 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
2f4d561
fix: [Violations] Distance - Incorrect error message when distance am…
Krishna2323 May 4, 2024
bac54f1
remove tax rate changed violation.
Krishna2323 May 4, 2024
d399453
remove ReceiptAuditHeader.
Krishna2323 May 4, 2024
c7ec1df
Merge branch 'Expensify:main' into krishna2323/issue/41401
Krishna2323 May 10, 2024
34d5e2e
updated amountModified translation.
Krishna2323 May 10, 2024
0549056
minor update.
Krishna2323 May 10, 2024
ef8e655
Merge branch 'Expensify:main' into krishna2323/issue/41401
Krishna2323 May 17, 2024
f039038
remove taxAmountChanged & taxRateChanged notice violations.
Krishna2323 May 17, 2024
be4f9be
fix conflicts.
Krishna2323 Jun 2, 2024
c9fd9aa
fix conflicts.
Krishna2323 Jun 2, 2024
b801997
Merge branch 'Expensify:main' into krishna2323/issue/41401
Krishna2323 Jun 2, 2024
4240c2f
Merge branch 'krishna2323/issue/41401' of https://github.com/Krishna2…
Krishna2323 Jun 2, 2024
1043d16
Merge branch 'Expensify:main' into krishna2323/issue/41401
Krishna2323 Jun 6, 2024
3c71e67
use violation data type from backend.
Krishna2323 Jun 6, 2024
12f0433
add ViolationMessages for receipt.
Krishna2323 Jun 7, 2024
407e552
return all type of violations for paid policies.
Krishna2323 Jun 7, 2024
7611122
fix: merge conflicts.
Krishna2323 Jun 12, 2024
4ebb250
Merge branch 'Expensify:main' into krishna2323/issue/41401
Krishna2323 Jun 12, 2024
03a9345
add changes back in MoneyRequestView.
Krishna2323 Jun 12, 2024
d693489
Merge branch 'krishna2323/issue/41401' of https://github.com/Krishna2…
Krishna2323 Jun 12, 2024
245d667
updated receiptViolationNames.
Krishna2323 Jun 12, 2024
efd639c
Merge branch 'Expensify:main' into krishna2323/issue/41401
Krishna2323 Jun 13, 2024
4e8f365
fix type check.
Krishna2323 Jun 13, 2024
9e452c4
minor fixes.
Krishna2323 Jun 14, 2024
49954fb
Merge branch 'Expensify:main' into krishna2323/issue/41401
Krishna2323 Jun 18, 2024
d671670
Merge branch 'main' into krishna2323/issue/41401
Krishna2323 Jun 23, 2024
e2851c4
fix lint issue.
Krishna2323 Jun 23, 2024
7ae2d6b
get displayPercentVariance from violation data.
Krishna2323 Jun 23, 2024
e5420c1
Merge branch 'Expensify:main' into krishna2323/issue/41401
Krishna2323 Jun 26, 2024
7c38e5c
Merge branch 'Expensify:main' into krishna2323/issue/41401
Krishna2323 Jul 5, 2024
1595f33
minor fixes.
Krishna2323 Jul 5, 2024
2589ba6
update isPaidGroupPolicy param to shouldShowOnlyViolations.
Krishna2323 Jul 5, 2024
6e63f57
add ReceiptAuditMessages back.
Krishna2323 Jul 5, 2024
9fec7f9
add comments.
Krishna2323 Jul 5, 2024
4fe1913
Merge branch 'Expensify:main' into krishna2323/issue/41401
Krishna2323 Jul 8, 2024
68ebaa7
update comments.
Krishna2323 Jul 8, 2024
2a76377
minor update.
Krishna2323 Jul 8, 2024
c5d5a3c
fix: amount violation shows under receipt.
Krishna2323 Jul 8, 2024
995eaed
fix amount violation not showing.
Krishna2323 Jul 8, 2024
e3b692e
remove shouldShowAuditSuccess & shouldShowAuditFailure prop.
Krishna2323 Jul 10, 2024
0a92c99
add shouldShowAuditResult prop in ReceiptAudit.
Krishna2323 Jul 12, 2024
1c4d2a1
refactor.
Krishna2323 Jul 12, 2024
7cdc36c
add comment.
Krishna2323 Jul 12, 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
9 changes: 9 additions & 0 deletions src/CONST.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3905,6 +3905,15 @@ const CONST = {
WARNING: 'warning',
},

/**
* Constants with different types for the modifiedAmount violation
*/
MODIFIED_AMOUNT_VIOLATION_DATA: {
DISTANCE: 'distance',
CARD: 'card',
SMARTSCAN: 'smartscan',
},

/**
* Constants for types of violation names.
* Defined here because they need to be referenced by the type system to generate the
Expand Down
23 changes: 19 additions & 4 deletions src/components/ReceiptAudit.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,31 @@ import Icon from './Icon';
import * as Expensicons from './Icon/Expensicons';
import Text from './Text';

function ReceiptAuditHeader({notes, shouldShowAuditMessage}: {notes: string[]; shouldShowAuditMessage: boolean}) {
type ReceiptAuditProps = {
/** List of audit notes */
notes: string[];

/** Whether to show audit result or not (e.g.`Verified`, `Issue Found`) */
shouldShowAuditResult: boolean;
};

function ReceiptAudit({notes, shouldShowAuditResult}: ReceiptAuditProps) {
const styles = useThemeStyles();
const theme = useTheme();
const {translate} = useLocalize();

const auditText = notes.length > 0 ? translate('iou.receiptIssuesFound', notes.length) : translate('common.verified');
let auditText = '';
if (notes.length > 0 && shouldShowAuditResult) {
auditText = translate('iou.receiptIssuesFound', notes.length);
} else if (!notes.length && shouldShowAuditResult) {
auditText = translate('common.verified');
}

return (
<View style={[styles.ph5, styles.mbn1]}>
<View style={[styles.flexRow, styles.alignItemsCenter]}>
<Text style={[styles.textLabelSupporting]}>{translate('common.receipt')}</Text>
{shouldShowAuditMessage && (
{!!auditText && (
<>
<Text style={[styles.textLabelSupporting]}>{` • ${auditText}`}</Text>
<Icon
Expand All @@ -39,4 +53,5 @@ function ReceiptAuditMessages({notes = []}: {notes?: string[]}) {
return <View style={[styles.mtn1, styles.mb2, styles.ph5, styles.gap1]}>{notes.length > 0 && notes.map((message) => <Text style={[styles.textLabelError]}>{message}</Text>)}</View>;
}

export {ReceiptAuditHeader, ReceiptAuditMessages};
export {ReceiptAuditMessages};
export default ReceiptAudit;
24 changes: 13 additions & 11 deletions src/components/ReportActionItem/MoneyRequestView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import MenuItem from '@components/MenuItem';
import MenuItemWithTopDescription from '@components/MenuItemWithTopDescription';
import OfflineWithFeedback from '@components/OfflineWithFeedback';
import {useSession} from '@components/OnyxProvider';
import {ReceiptAuditHeader, ReceiptAuditMessages} from '@components/ReceiptAudit';
import ReceiptAudit, {ReceiptAuditMessages} from '@components/ReceiptAudit';
import ReceiptEmptyState from '@components/ReceiptEmptyState';
import Switch from '@components/Switch';
import Text from '@components/Text';
Expand Down Expand Up @@ -163,7 +163,7 @@ function MoneyRequestView({
const canEditReceipt = ReportUtils.canEditFieldOfMoneyRequest(parentReportAction, CONST.EDIT_REQUEST_FIELD.RECEIPT);
const hasReceipt = TransactionUtils.hasReceipt(transaction);
const isReceiptBeingScanned = hasReceipt && TransactionUtils.isReceiptBeingScanned(transaction);
const didRceiptScanSucceed = hasReceipt && TransactionUtils.didRceiptScanSucceed(transaction);
const didReceiptScanSucceed = hasReceipt && TransactionUtils.didReceiptScanSucceed(transaction);
const canEditDistance = ReportUtils.canEditFieldOfMoneyRequest(parentReportAction, CONST.EDIT_REQUEST_FIELD.DISTANCE);

const isAdmin = policy?.role === 'admin';
Expand All @@ -189,7 +189,7 @@ function MoneyRequestView({
const tripID = ReportUtils.getTripIDFromTransactionParentReport(parentReport);
const shouldShowViewTripDetails = TransactionUtils.hasReservationList(transaction) && !!tripID;

const {getViolationsForField} = useViolations(transactionViolations ?? []);
const {getViolationsForField} = useViolations(transactionViolations ?? [], isReceiptBeingScanned || !ReportUtils.isPaidGroupPolicy(report));
const hasViolations = useCallback(
(field: ViolationField, data?: OnyxTypes.TransactionViolation['data'], policyHasDependentTags = false, tagValue?: string): boolean =>
!!canUseViolations && getViolationsForField(field, data, policyHasDependentTags, tagValue).length > 0,
Expand Down Expand Up @@ -340,14 +340,17 @@ function MoneyRequestView({
const receiptViolationNames: OnyxTypes.ViolationName[] = [
CONST.VIOLATIONS.RECEIPT_REQUIRED,
CONST.VIOLATIONS.RECEIPT_NOT_SMART_SCANNED,
CONST.VIOLATIONS.MODIFIED_DATE,
Copy link
Contributor Author

@Krishna2323 Krishna2323 Jul 8, 2024

Choose a reason for hiding this comment

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

@sobitneupane @cead22, I mistakenly added modified amount violation here while resolving conflict, thats why the violation was shown under receipt. I have removed that and also CONST.VIOLATIONS.MODIFIED_DATE because I believe the date violation should also be shown under date field.

CONST.VIOLATIONS.CASH_EXPENSE_WITH_NO_RECEIPT,
CONST.VIOLATIONS.SMARTSCAN_FAILED,
];
const receiptViolations =
transactionViolations?.filter((violation) => receiptViolationNames.includes(violation.name)).map((violation) => ViolationsUtils.getViolationTranslation(violation, translate)) ?? [];
const shouldShowNotesViolations = !isReceiptBeingScanned && canUseViolations && ReportUtils.isPaidGroupPolicy(report);
const shouldShowReceiptHeader = isReceiptAllowed && (shouldShowReceiptEmptyState || hasReceipt);

// Whether to show receipt audit result (e.g.`Verified`, `Issue Found`) and messages (e.g. `Receipt not verified. Please confirm accuracy.`)
// `!!(receiptViolations.length || didReceiptScanSucceed)` is for not showing `Verified` when `receiptViolations` is empty and `didReceiptScanSucceed` is false.
const shouldShowAuditMessage =
!isReceiptBeingScanned && hasReceipt && !!(receiptViolations.length || didReceiptScanSucceed) && !!canUseViolations && ReportUtils.isPaidGroupPolicy(report);
Copy link
Contributor Author

@Krishna2323 Krishna2323 Jul 12, 2024

Choose a reason for hiding this comment

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

@sobitneupane @cead22, updated shouldShowAuditMessage, now this is will be enough for handling violation messages and audit result both. It works correctly as per my testing.

const shouldShowReceiptAudit = isReceiptAllowed && (shouldShowReceiptEmptyState || hasReceipt);

const errors = {
...(transaction?.errorFields?.route ?? transaction?.errors),
Expand Down Expand Up @@ -389,10 +392,10 @@ function MoneyRequestView({
<View style={styles.pRelative}>
{shouldShowAnimatedBackground && <AnimatedEmptyStateBackground />}
<>
{shouldShowReceiptHeader && (
<ReceiptAuditHeader
{shouldShowReceiptAudit && (
<ReceiptAudit
notes={receiptViolations}
shouldShowAuditMessage={!!(shouldShowNotesViolations && didRceiptScanSucceed)}
shouldShowAuditResult={shouldShowAuditMessage}
/>
)}
{(hasReceipt || errors) && (
Expand Down Expand Up @@ -448,7 +451,7 @@ function MoneyRequestView({
/>
)}
{!shouldShowReceiptEmptyState && !hasReceipt && <View style={{marginVertical: 6}} />}
{shouldShowNotesViolations && <ReceiptAuditMessages notes={receiptViolations} />}
{shouldShowAuditMessage && <ReceiptAuditMessages notes={receiptViolations} />}
<OfflineWithFeedback pendingAction={getPendingFieldAction('amount')}>
<MenuItemWithTopDescription
title={amountTitle}
Expand Down Expand Up @@ -555,7 +558,6 @@ function MoneyRequestView({
/>
</OfflineWithFeedback>
)}

{shouldShowTax && (
<OfflineWithFeedback pendingAction={getPendingFieldAction('taxAmount')}>
<MenuItemWithTopDescription
Expand Down
24 changes: 20 additions & 4 deletions src/hooks/useViolations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,26 +40,42 @@ const violationFields: Record<ViolationName, ViolationField> = {
smartscanFailed: 'receipt',
someTagLevelsRequired: 'tag',
tagOutOfPolicy: 'tag',
taxRateChanged: 'tax',
taxAmountChanged: 'tax',
taxOutOfPolicy: 'tax',
taxRateChanged: 'tax',
taxRequired: 'tax',
hold: 'none',
};

type ViolationsMap = Map<ViolationField, TransactionViolation[]>;

function useViolations(violations: TransactionViolation[]) {
// We don't want to show these violations on NewDot
const excludedViolationsName = ['taxAmountChanged', 'taxRateChanged'];

/**
* @param violations – List of transaction violations
* @param shouldShowOnlyViolations – Whether we should only show violations of type 'violation'
*/
function useViolations(violations: TransactionViolation[], shouldShowOnlyViolations: boolean) {
const violationsByField = useMemo((): ViolationsMap => {
const filteredViolations = violations.filter((violation) => violation.type === CONST.VIOLATION_TYPES.VIOLATION);
const filteredViolations = violations.filter((violation) => {
if (excludedViolationsName.includes(violation.name)) {
return false;
}
if (shouldShowOnlyViolations) {
return violation.type === CONST.VIOLATION_TYPES.VIOLATION;
}
return true;
});

const violationGroups = new Map<ViolationField, TransactionViolation[]>();
for (const violation of filteredViolations) {
const field = violationFields[violation.name];
const existingViolations = violationGroups.get(field) ?? [];
violationGroups.set(field, [...existingViolations, violation]);
}
return violationGroups ?? new Map();
}, [violations]);
}, [violations, shouldShowOnlyViolations]);

const getViolationsForField = useCallback(
(field: ViolationField, data?: TransactionViolation['data'], policyHasDependentTags = false, tagValue?: string) => {
Expand Down
15 changes: 14 additions & 1 deletion src/languages/en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ import type {
ViolationsInvoiceMarkupParams,
ViolationsMaxAgeParams,
ViolationsMissingTagParams,
ViolationsModifiedAmountParams,
ViolationsOverCategoryLimitParams,
ViolationsOverLimitParams,
ViolationsPerDayLimitParams,
Expand Down Expand Up @@ -3633,7 +3634,19 @@ export default {
missingCategory: 'Missing category',
missingComment: 'Description required for selected category',
missingTag: ({tagName}: ViolationsMissingTagParams) => `Missing ${tagName ?? 'tag'}`,
modifiedAmount: 'Amount greater than scanned receipt',
modifiedAmount: ({type, displayPercentVariance}: ViolationsModifiedAmountParams): string => {
switch (type) {
case 'distance':
return 'Amount differs from calculated distance';
case 'card':
return 'Amount greater than card transaction';
default:
if (displayPercentVariance) {
return `Amount ${displayPercentVariance}% greater than scanned receipt`;
}
return 'Amount greater than scanned receipt';
}
},
Copy link
Contributor

Choose a reason for hiding this comment

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

#41401 (comment)

  • {data: {type: 'distance'}}
  • {data: {type: 'card'}}
  • {data: {type: 'smartscan'}}
  • {data: {type: 'smartscan', displayPercentVariance: $displayPercentVariance}}

displayPercentVariance is present in type 'smartscan' not 'card'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated 👀

modifiedDate: 'Date differs from scanned receipt',
nonExpensiworksExpense: 'Non-Expensiworks expense',
overAutoApprovalLimit: ({formattedLimit}: ViolationsOverLimitParams) => `Expense exceeds auto approval limit of ${formattedLimit}`,
Expand Down
15 changes: 14 additions & 1 deletion src/languages/es.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ import type {
ViolationsInvoiceMarkupParams,
ViolationsMaxAgeParams,
ViolationsMissingTagParams,
ViolationsModifiedAmountParams,
ViolationsOverAutoApprovalLimitParams,
ViolationsOverCategoryLimitParams,
ViolationsOverLimitParams,
Expand Down Expand Up @@ -4141,7 +4142,19 @@ export default {
missingCategory: 'Falta categoría',
missingComment: 'Descripción obligatoria para la categoría seleccionada',
missingTag: ({tagName}: ViolationsMissingTagParams) => `Falta ${tagName ?? 'etiqueta'}`,
modifiedAmount: 'Importe superior al del recibo escaneado',
modifiedAmount: ({type, displayPercentVariance}: ViolationsModifiedAmountParams) => {
switch (type) {
case 'distance':
return 'Importe difiere del calculado basado en distancia';
case 'card':
return 'Importe mayor al de la transacción de la tarjeta';
default:
if (displayPercentVariance) {
return `Importe ${displayPercentVariance}% mayor al del recibo escaneado`;
}
return 'Importe mayor al del recibo escaneado';
}
},
modifiedDate: 'Fecha difiere del recibo escaneado',
nonExpensiworksExpense: 'Gasto no proviene de Expensiworks',
overAutoApprovalLimit: ({formattedLimit}: ViolationsOverAutoApprovalLimitParams) =>
Expand Down
4 changes: 4 additions & 0 deletions src/languages/types.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type {OnyxInputOrEntry, ReportAction} from '@src/types/onyx';
import type {Unit} from '@src/types/onyx/Policy';
import type {ViolationDataType} from '@src/types/onyx/TransactionViolation';
import type en from './en';

type AddressLineParams = {
Expand Down Expand Up @@ -222,6 +223,8 @@ type ViolationsMaxAgeParams = {maxAge: number};

type ViolationsMissingTagParams = {tagName?: string};

type ViolationsModifiedAmountParams = {type?: ViolationDataType; displayPercentVariance?: number};

type ViolationsOverAutoApprovalLimitParams = {formattedLimit?: string};

type ViolationsOverCategoryLimitParams = {formattedLimit?: string};
Expand Down Expand Up @@ -431,6 +434,7 @@ export type {
ViolationsInvoiceMarkupParams,
ViolationsMaxAgeParams,
ViolationsMissingTagParams,
ViolationsModifiedAmountParams,
ViolationsOverAutoApprovalLimitParams,
ViolationsOverCategoryLimitParams,
ViolationsOverLimitParams,
Expand Down
4 changes: 2 additions & 2 deletions src/libs/TransactionUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -527,7 +527,7 @@ function isReceiptBeingScanned(transaction: OnyxInputOrEntry<Transaction>): bool
return [CONST.IOU.RECEIPT_STATE.SCANREADY, CONST.IOU.RECEIPT_STATE.SCANNING].some((value) => value === transaction?.receipt?.state);
}

function didRceiptScanSucceed(transaction: OnyxEntry<Transaction>): boolean {
function didReceiptScanSucceed(transaction: OnyxEntry<Transaction>): boolean {
return [CONST.IOU.RECEIPT_STATE.SCANCOMPLETE].some((value) => value === transaction?.receipt?.state);
}

Expand Down Expand Up @@ -950,7 +950,7 @@ export {
hasEReceipt,
hasRoute,
isReceiptBeingScanned,
didRceiptScanSucceed,
didReceiptScanSucceed,
getValidWaypoints,
isDistanceRequest,
isFetchingWaypointsFromServer,
Expand Down
3 changes: 2 additions & 1 deletion src/libs/Violations/ViolationsUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,7 @@ const ViolationsUtils = {
maxAge = 0,
tagName,
taxName,
type,
} = violation.data ?? {};

switch (violation.name) {
Expand Down Expand Up @@ -288,7 +289,7 @@ const ViolationsUtils = {
case 'missingTag':
return translate('violations.missingTag', {tagName});
case 'modifiedAmount':
return translate('violations.modifiedAmount');
return translate('violations.modifiedAmount', {type, displayPercentVariance: violation.data?.displayPercentVariance});
case 'modifiedDate':
return translate('violations.modifiedDate');
case 'nonExpensiworksExpense':
Expand Down
14 changes: 13 additions & 1 deletion src/types/onyx/TransactionViolation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ import type CONST from '@src/CONST';
*/
type ViolationName = ValueOf<typeof CONST.VIOLATIONS>;

/**
* Types for the data in the modifiedAmount violation
* Derived from CONST.VIOLATION_DATA_TYPES to maintain a single source of truth.
*/
type ViolationDataType = ValueOf<typeof CONST.MODIFIED_AMOUNT_VIOLATION_DATA>;

/** Model of transaction violation data */
type TransactionViolationData = {
/** Who rejected the transaction */
Expand Down Expand Up @@ -63,6 +69,12 @@ type TransactionViolationData = {
/** Whether the current violation is `pending RTER` */
pendingPattern?: boolean;

/** modifiedAmount violation type (eg, 'distance', 'card') */
type?: ViolationDataType;

/** Percent Variance for modified amount violations */
displayPercentVariance?: number;

/** List of duplicate transactions */
duplicates?: string[];
};
Expand All @@ -82,5 +94,5 @@ type TransactionViolation = {
/** Collection of transaction violations */
type TransactionViolations = TransactionViolation[];

export type {TransactionViolation, ViolationName};
export type {TransactionViolation, ViolationName, ViolationDataType};
export default TransactionViolations;
Loading