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.

Sign up for GitHub

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: add missing violation data #54603

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
+132 −9
Open

fix: add missing violation data #54603

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
87a8010
fix: add missing violation data
daledah Dec 27, 2024
8e3ecda
fix: lint
daledah Dec 27, 2024
5b40b67
fix: violation test
daledah Dec 27, 2024
b81691d
fix: add new tests for violation utils
daledah Dec 30, 2024
5a9b1a0
fix: change future date violation logics and tests, remove redundant …
daledah Jan 6, 2025
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
10 changes: 10 additions & 0 deletions src/libs/DateUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -678,6 +678,15 @@ const getDayValidationErrorKey = (inputDate: Date): string => {
return '';
};

/**
* Checks if the input time is after the reference date
* param {Date} inputDate - The date to validate.
* returns {boolean} - Returns true if the input date is after the reference date, otherwise false.
*/
const isFutureDate = (inputDate: Date): boolean => {
return isAfter(startOfDay(inputDate), startOfDay(new Date()));
};

Comment on lines +681 to +689
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we remove startOfDay?

/**
* Checks if the input time is at least one minute in the future compared to the reference time.
* param {Date} inputTime - The time to validate.
Expand Down Expand Up @@ -937,6 +946,7 @@ const DateUtils = {
isValidDateString,
getFormattedDurationBetweenDates,
getFormattedDuration,
isFutureDate,
};

export default DateUtils;
57 changes: 56 additions & 1 deletion src/libs/Violations/ViolationsUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import reject from 'lodash/reject';
import Onyx from 'react-native-onyx';
import type {OnyxUpdate} from 'react-native-onyx';
import type {LocaleContextProps} from '@components/LocaleContextProvider';
import * as CurrencyUtils from '@libs/CurrencyUtils';
import DateUtils from '@libs/DateUtils';
import {getDistanceRateCustomUnitRate, getSortedTagKeys} from '@libs/PolicyUtils';
import * as TransactionUtils from '@libs/TransactionUtils';
import CONST from '@src/CONST';
Expand Down Expand Up @@ -187,6 +189,12 @@ const ViolationsUtils = {
const hasMissingCategoryViolation = transactionViolations.some((violation) => violation.name === 'missingCategory');
const categoryKey = updatedTransaction.category;
const isCategoryInPolicy = categoryKey ? policyCategories?.[categoryKey]?.enabled : false;
const inputDate = new Date(updatedTransaction.created);
const isInputDateInTheFuture = DateUtils.isFutureDate(inputDate);
const hasFutureDateViolation = transactionViolations.some((violation) => violation.name === 'futureDate');
const hasReceiptRequiredViolation = transactionViolations.some((violation) => violation.name === 'receiptRequired');
const hasOverLimitViolation = transactionViolations.some((violation) => violation.name === 'overLimit');
const policyMaxExpenseAmount = policy.maxExpenseAmount;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const policyMaxExpenseAmount = policy.maxExpenseAmount;

Unnecessary const


// Add 'categoryOutOfPolicy' violation if category is not in policy
if (!hasCategoryOutOfPolicyViolation && categoryKey && !isCategoryInPolicy) {
Expand All @@ -205,7 +213,54 @@ const ViolationsUtils = {

// Add 'missingCategory' violation if category is required and not set
if (!hasMissingCategoryViolation && policyRequiresCategories && !categoryKey) {
newTransactionViolations.push({name: 'missingCategory', type: CONST.VIOLATION_TYPES.VIOLATION});
newTransactionViolations.push({name: 'missingCategory', type: CONST.VIOLATION_TYPES.VIOLATION, showInReview: true});
}

// Add 'futureDate' violation if transaction date is in the future
if (!hasFutureDateViolation && isInputDateInTheFuture) {
newTransactionViolations.push({name: CONST.VIOLATIONS.FUTURE_DATE, type: CONST.VIOLATION_TYPES.VIOLATION, showInReview: true});
}

if (hasFutureDateViolation && !isInputDateInTheFuture) {
newTransactionViolations = reject(newTransactionViolations, {name: CONST.VIOLATIONS.FUTURE_DATE});
}

if (
!hasReceiptRequiredViolation &&
policy.maxExpenseAmountNoReceipt &&
Math.abs(updatedTransaction.amount) > policy.maxExpenseAmountNoReceipt &&
!updatedTransaction.receipt?.receiptID
Copy link
Contributor

Choose a reason for hiding this comment

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

Declare this as a new const

) {
newTransactionViolations.push({
name: CONST.VIOLATIONS.RECEIPT_REQUIRED,
data: {
formattedLimit: CurrencyUtils.convertAmountToDisplayString(policy.maxExpenseAmountNoReceipt),
},
type: CONST.VIOLATION_TYPES.VIOLATION,
showInReview: true,
});
}

if (
hasReceiptRequiredViolation &&
(!!updatedTransaction.receipt?.receiptID || !policy.maxExpenseAmountNoReceipt || Math.abs(updatedTransaction.amount) <= policy.maxExpenseAmountNoReceipt)
) {
newTransactionViolations = reject(newTransactionViolations, {name: CONST.VIOLATIONS.RECEIPT_REQUIRED});
}

if (!hasOverLimitViolation && policyMaxExpenseAmount && Math.abs(updatedTransaction.amount) > policyMaxExpenseAmount) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same ^ (only the part after !hasOverLimitViolation &&)

newTransactionViolations.push({
name: CONST.VIOLATIONS.OVER_LIMIT,
data: {
formattedLimit: CurrencyUtils.convertAmountToDisplayString(policyMaxExpenseAmount),
},
type: CONST.VIOLATION_TYPES.VIOLATION,
showInReview: true,
});
}

if (hasOverLimitViolation && (!policyMaxExpenseAmount || Math.abs(updatedTransaction.amount) <= policyMaxExpenseAmount)) {
newTransactionViolations = reject(newTransactionViolations, {name: CONST.VIOLATIONS.OVER_LIMIT});
}
}

Expand Down
61 changes: 53 additions & 8 deletions tests/unit/ViolationUtilsTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {beforeEach} from '@jest/globals';
import Onyx from 'react-native-onyx';
import * as CurrencyUtils from '@libs/CurrencyUtils';
import ViolationsUtils from '@libs/Violations/ViolationsUtils';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
Expand All @@ -13,6 +14,31 @@ const categoryOutOfPolicyViolation = {
const missingCategoryViolation = {
name: CONST.VIOLATIONS.MISSING_CATEGORY,
type: CONST.VIOLATION_TYPES.VIOLATION,
showInReview: true,
};

const futureDateViolation = {
name: CONST.VIOLATIONS.FUTURE_DATE,
type: CONST.VIOLATION_TYPES.VIOLATION,
showInReview: true,
};

const receiptRequiredViolation = {
name: CONST.VIOLATIONS.RECEIPT_REQUIRED,
type: CONST.VIOLATION_TYPES.VIOLATION,
showInReview: true,
data: {
formattedLimit: CurrencyUtils.convertAmountToDisplayString(CONST.POLICY.DEFAULT_MAX_AMOUNT_NO_RECEIPT),
},
};

const overLimitViolation = {
name: CONST.VIOLATIONS.OVER_LIMIT,
type: CONST.VIOLATION_TYPES.VIOLATION,
showInReview: true,
data: {
formattedLimit: CurrencyUtils.convertAmountToDisplayString(CONST.POLICY.DEFAULT_MAX_EXPENSE_AMOUNT),
},
};

const customUnitOutOfPolicyViolation = {
Expand Down Expand Up @@ -117,6 +143,9 @@ describe('getViolationsOnyxData', () => {
policy.requiresCategory = true;
policyCategories = {Food: {name: 'Food', unencodedName: '', enabled: true, areCommentsRequired: false, externalID: '1234', origin: '12345'}};
transaction.category = 'Food';
transaction.amount = 100;
policy.maxExpenseAmount = CONST.POLICY.DEFAULT_MAX_EXPENSE_AMOUNT;
policy.maxExpenseAmountNoReceipt = CONST.POLICY.DEFAULT_MAX_AMOUNT_NO_RECEIPT;
});

it('should add missingCategory violation if no category is included', () => {
Expand All @@ -125,6 +154,26 @@ describe('getViolationsOnyxData', () => {
expect(result.value).toEqual(expect.arrayContaining([missingCategoryViolation, ...transactionViolations]));
});

it('should add futureDate violation if the transaction has a future date', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we sure that all workspaces disallow future-dated expenses?

const futureDate = new Date();
futureDate.setDate(futureDate.getDate() + 1);
transaction.created = futureDate.toISOString();
const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policy, policyTags, policyCategories, false);
expect(result.value).toEqual(expect.arrayContaining([futureDateViolation, ...transactionViolations]));
});

it('should add receiptRequired violation if the transaction has no receipt', () => {
transaction.amount = 1000000;
const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policy, policyTags, policyCategories, false);
expect(result.value).toEqual(expect.arrayContaining([receiptRequiredViolation, ...transactionViolations]));
});

it('should add overLimit violation if the transaction amount is over the policy limit', () => {
transaction.amount = 1000000;
const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policy, policyTags, policyCategories, false);
expect(result.value).toEqual(expect.arrayContaining([overLimitViolation, ...transactionViolations]));
});

it('should add categoryOutOfPolicy violation when category is not in policy', () => {
transaction.category = 'Bananas';
const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policy, policyTags, policyCategories, false);
Expand All @@ -144,10 +193,8 @@ describe('getViolationsOnyxData', () => {

it('should add categoryOutOfPolicy violation to existing violations if they exist', () => {
transaction.category = 'Bananas';
transactionViolations = [
{name: 'duplicatedTransaction', type: CONST.VIOLATION_TYPES.VIOLATION},
{name: 'receiptRequired', type: CONST.VIOLATION_TYPES.VIOLATION},
];
transaction.amount = 1000000;
transactionViolations = [{name: 'duplicatedTransaction', type: CONST.VIOLATION_TYPES.VIOLATION}, receiptRequiredViolation];

const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policy, policyTags, policyCategories, false);

Expand All @@ -156,10 +203,8 @@ describe('getViolationsOnyxData', () => {

it('should add missingCategory violation to existing violations if they exist', () => {
transaction.category = undefined;
transactionViolations = [
{name: 'duplicatedTransaction', type: CONST.VIOLATION_TYPES.VIOLATION},
{name: 'receiptRequired', type: CONST.VIOLATION_TYPES.VIOLATION},
];
transaction.amount = 1000000;
transactionViolations = [{name: 'duplicatedTransaction', type: CONST.VIOLATION_TYPES.VIOLATION}, receiptRequiredViolation];

const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policy, policyTags, policyCategories, false);

Expand Down
Loading