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

Implement ViolationUtils lib #31656

Merged
merged 78 commits into from
Dec 4, 2023
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
78 commits
Select commit Hold shift + click to select a range
f251978
feat(Violations): Add PolicyCategories type to match PolicyTags
trevor-coleman Nov 17, 2023
51c2e7e
feat(Violations): Create stub for ViolationUtils
trevor-coleman Nov 17, 2023
f001066
feat(Violations): Add TRANSACTION_VIOLATION to ONYXKEYS.COLLECTIONS
trevor-coleman Nov 17, 2023
2705280
feat(Violations): Add TransactionViolation and related types to src/t…
trevor-coleman Nov 17, 2023
e2aaa6a
feat(Violations): create possibleViolationsByField lookup
trevor-coleman Nov 17, 2023
7f39f5a
feat(Violations): implement getViolationForField and getViolationsOny…
trevor-coleman Nov 17, 2023
ccb4206
prettier
trevor-coleman Nov 20, 2023
6a67fb8
Merge pull request #73 from infinitered/trevorcoleman/violations/viol…
trevor-coleman Nov 21, 2023
0a04347
feat(Violations): remove unnecessary callback
trevor-coleman Nov 21, 2023
15f43ba
Merge remote-tracking branch 'origin/main' into trevorcoleman/violati…
trevor-coleman Nov 22, 2023
575ea9b
update package-lock.json
trevor-coleman Nov 22, 2023
17ef290
Merge remote-tracking branch 'upstream/main' into trevorcoleman/viola…
trevor-coleman Nov 22, 2023
cf65dbe
Merge remote-tracking branch 'upstream/main' into violation-utils
trevor-coleman Nov 22, 2023
0353afa
restore package-lock.json
trevor-coleman Nov 22, 2023
8c16f79
Merge remote-tracking branch 'upstream/main' into violation-utils
trevor-coleman Nov 22, 2023
1e2e7c8
Update src/types/onyx/TransactionViolation.ts
trevor-coleman Nov 28, 2023
0c8d067
Update src/libs/Violations/ViolationsUtils.ts
trevor-coleman Nov 28, 2023
434fd9f
Update src/types/onyx/TransactionViolation.ts
trevor-coleman Nov 28, 2023
d251b9a
Update src/types/onyx/TransactionViolation.ts
trevor-coleman Nov 28, 2023
19b11b1
Merge branch 'Expensify:main' into violation-utils
trevor-coleman Nov 28, 2023
7f83963
feat(Violations): remove ViolationType, sort violation names
trevor-coleman Nov 28, 2023
e4a56b9
feat(Violations): add dummy translations for complete list
trevor-coleman Nov 28, 2023
12a2aa3
feat(Violations): remove barrel file
trevor-coleman Nov 28, 2023
12682bb
feat(Violations): extract getViolationsForField into useViolations hook
trevor-coleman Nov 28, 2023
d7ca133
Merge branch 'violation-utils' into trevorcoleman/violations/violatio…
trevor-coleman Nov 28, 2023
58ea8d8
feat(Violations): export violationFields for testing
trevor-coleman Nov 28, 2023
8f951e0
feat(Violations): add tests for useViolations
trevor-coleman Nov 28, 2023
04c23f6
feat(Violations): remove comments on arguments and replace with JSDoc
trevor-coleman Nov 28, 2023
d1c8792
Merge remote-tracking branch 'origin/trevorcoleman/violations/violati…
trevor-coleman Nov 28, 2023
ddbe029
feat(Violations): lint
trevor-coleman Nov 28, 2023
7054d05
package-lock
trevor-coleman Nov 28, 2023
532c96a
prettier
trevor-coleman Nov 28, 2023
6568133
feat(Violations): remove possibleViolationsByField
trevor-coleman Nov 28, 2023
6aef0ba
Merge pull request #77 from infinitered/trevorcoleman/violations/viol…
trevor-coleman Nov 29, 2023
dfb444b
feat(Violations): update violationFields
trevor-coleman Nov 29, 2023
ae8acc5
feat(Violations): update translations for violations
trevor-coleman Nov 29, 2023
a796c2c
feat(Violations): update spanish translations for violations
trevor-coleman Nov 29, 2023
f2792e5
feat(Violations): create ViolationUtilsTest with stubs.
trevor-coleman Nov 29, 2023
3710f82
feat(Violations): fix missing tags and categories logic
trevor-coleman Nov 29, 2023
5158f20
feat(Violations): add tests for ViolationUtils
trevor-coleman Nov 29, 2023
eaa9c85
feat(Violations): remove test for case sensitivity
trevor-coleman Nov 29, 2023
4fcc994
feat(Violations): add tests for ViolationUtils
trevor-coleman Nov 29, 2023
a66d253
feat(Violations): return dummy translations
trevor-coleman Nov 29, 2023
f504508
feat(Violations): remove unused imports
trevor-coleman Nov 29, 2023
ae1731b
feat(Violations): memoize return values for hasViolations, and return…
trevor-coleman Nov 29, 2023
de45660
feat(Violations): update tests for new return values.
trevor-coleman Nov 29, 2023
e239e41
Merge pull request #78 from infinitered/trevor-coleman/violations/vio…
trevor-coleman Nov 29, 2023
2b27509
feat(Violations): remove index
trevor-coleman Nov 29, 2023
47afbe9
feat(Violations): add quotes
trevor-coleman Nov 30, 2023
bc3b872
feat(Violations): Remove unnecessary casts
trevor-coleman Nov 30, 2023
9fd9bf9
feat(Violations): capitalize
trevor-coleman Nov 30, 2023
e72bacc
feat(Violations): Add condition
trevor-coleman Nov 30, 2023
2de9615
feat(Violations): Remove cast
trevor-coleman Nov 30, 2023
478f934
feat(Violations): add condition
trevor-coleman Nov 30, 2023
ef5ebb3
feat(Violations): remove unused types
trevor-coleman Nov 30, 2023
68c5485
feat(Violations): harmonize tag and category handling
trevor-coleman Nov 30, 2023
75f003a
feat(Violations): reorder types
trevor-coleman Nov 30, 2023
016b438
feat(Violations): Apply suggestions from code review
trevor-coleman Nov 30, 2023
9c356f2
feat(Violations): remove hasViolationsMap
trevor-coleman Nov 30, 2023
9b0c88a
feat(Violations): refactor useViolations tests
trevor-coleman Nov 30, 2023
e0d8d97
feat(Violations): refactor ViolationUtils tests
trevor-coleman Dec 1, 2023
96544c9
Merge remote-tracking branch 'origin/violation-utils' into violation-…
trevor-coleman Dec 1, 2023
06b7012
Merge branch 'main' into violation-utils
trevor-coleman Dec 1, 2023
ebb5751
feat(Violations): revert package.json
trevor-coleman Dec 1, 2023
74b80df
prettier
trevor-coleman Dec 1, 2023
d3be1f6
feat(Violations): remove execute
trevor-coleman Dec 1, 2023
4ab9931
feat(Violations): refactor tests
trevor-coleman Dec 1, 2023
3737752
feat(Violations): refactor tests
trevor-coleman Dec 1, 2023
1876a0b
feat(Violations): refactor tests
trevor-coleman Dec 1, 2023
72667ec
feat(Violations): refactor tests to have simpler base case
trevor-coleman Dec 1, 2023
27cfac1
Update tests/unit/ViolationUtilsTest.js
trevor-coleman Dec 4, 2023
e5ae614
feat(Violations): Suggested changes from PR Review
trevor-coleman Dec 4, 2023
8498e80
feat(Violations): remove useViolationsTest
trevor-coleman Dec 4, 2023
e6e7612
feat(Violations): fixes from PR Comments
trevor-coleman Dec 4, 2023
0aa36b8
Merge remote-tracking branch 'origin/violation-utils' into violation-…
trevor-coleman Dec 4, 2023
391822c
prettier
trevor-coleman Dec 4, 2023
dd48fe1
Merge remote-tracking branch 'upstream/main' into violation-utils
trevor-coleman Dec 4, 2023
d2cfd33
feat(Violations): remove useViolationsTest
trevor-coleman Dec 4, 2023
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
1 change: 1 addition & 0 deletions src/ONYXKEYS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,7 @@ const ONYXKEYS = {
REPORT_USER_IS_LEAVING_ROOM: 'reportUserIsLeavingRoom_',
SECURITY_GROUP: 'securityGroup_',
TRANSACTION: 'transactions_',
TRANSACTION_VIOLATIONS: 'transactionViolations_',

// Holds temporary transactions used during the creation and edit flow
TRANSACTION_DRAFT: 'transactionsDraft_',
Expand Down
69 changes: 69 additions & 0 deletions src/libs/Violations/ViolationsUtils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import reject from 'lodash/reject';
import Onyx from 'react-native-onyx';
import ONYXKEYS from '@src/ONYXKEYS';
import {PolicyCategories, PolicyTags, Transaction, TransactionViolation} from '@src/types/onyx';
import possibleViolationsByField, {ViolationField} from './possibleViolationsByField';

const ViolationsUtils = {
getViolationForField(transactionViolations: TransactionViolation[], field: ViolationField, translate: (key: string) => string): string[] {
return transactionViolations.filter((violation) => possibleViolationsByField[field]?.includes(violation.name)).map((violation) => translate(violation.name));
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm actually not a huge fan of the extra complexity added by possibleViolationsByField, and I suggest we get rid of it.

In practice, it'll turn this line into something like this, which is just as simple

return transactionViolations.filter((violation) => violationFields[violation.name] === field).map((violation) => translate(violation.name));

Let's also get rid of src/libs/Violations/possibleViolationsByField.ts and move that logic into this file

Copy link
Contributor

Choose a reason for hiding this comment

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

I also don't like using lodash/invertBy

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah this was all a bit clunky, but as I used it in components I found a better solution. I was going to include it as part of my PR on the MoneyRequestView issue, but I will bring the changes into this branch so we can look at it here.

},

getViolationsOnyxData(
/** The transaction to check for policy violations. */
transaction: Transaction,
/** An array of existing transaction violations. */
transactionViolations: TransactionViolation[],
/** Indicates if the policy requires tags. */
policyRequiresTags: boolean,
/** Collection of policy tags and their enabled states. */
policyTags: PolicyTags,
/** Indicates if the policy requires categories. */
policyRequiresCategories: boolean,
/** Collection of policy categories and their enabled states. */
policyCategories: PolicyCategories,
): {
trevor-coleman marked this conversation as resolved.
Show resolved Hide resolved
onyxMethod: string;
key: string;
value: TransactionViolation[];
trevor-coleman marked this conversation as resolved.
Show resolved Hide resolved
} {
let newTransactionViolations = [...transactionViolations];

if (policyRequiresCategories) {
const categoryViolationExists = transactionViolations.some((violation) => violation.name === 'categoryOutOfPolicy');
const categoryIsInPolicy = policyCategories[transaction.category]?.enabled;
trevor-coleman marked this conversation as resolved.
Show resolved Hide resolved

// Add 'categoryOutOfPolicy' violation if category is not in policy
if (!categoryViolationExists && transaction.category && !categoryIsInPolicy) {
newTransactionViolations.push({name: 'categoryOutOfPolicy', type: 'violation', userMessage: ''});
trevor-coleman marked this conversation as resolved.
Show resolved Hide resolved
}

// Remove 'missingCategory' violation if category is valid according to policy
if (categoryIsInPolicy) {
newTransactionViolations = reject(newTransactionViolations, {name: 'missingCategory'});
trevor-coleman marked this conversation as resolved.
Show resolved Hide resolved
}
}

if (policyRequiresTags) {
// Add 'tagOutOfPolicy' violation if tag is not in policy
const tagViolationExists = transactionViolations.some((violation) => violation.name === 'tagOutOfPolicy');
trevor-coleman marked this conversation as resolved.
Show resolved Hide resolved
const tagInPolicy = policyTags[transaction.tag]?.enabled;
trevor-coleman marked this conversation as resolved.
Show resolved Hide resolved
if (!tagViolationExists && transaction.tag && !tagInPolicy) {
newTransactionViolations.push({name: 'tagOutOfPolicy', type: 'violation', userMessage: ''});
}

// Remove 'missingTag' violation if tag is valid according to policy
if (tagInPolicy) {
newTransactionViolations = reject(newTransactionViolations, {name: 'missingTag'});
}
}

return {
onyxMethod: Onyx.METHOD.SET,
key: `${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transaction.transactionID}`,
value: newTransactionViolations,
};
},
};

export default ViolationsUtils;
3 changes: 3 additions & 0 deletions src/libs/Violations/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import * as ViolationsUtils from './ViolationsUtils';
trevor-coleman marked this conversation as resolved.
Show resolved Hide resolved

export default ViolationsUtils;
41 changes: 41 additions & 0 deletions src/libs/Violations/possibleViolationsByField.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import invertBy from 'lodash/invertBy';
import {ViolationName} from '@src/types/onyx';

/**
* Map from Violation Names to the field where that violation can occur
*/
const violationFields: Record<ViolationName, ViolationField> = {
perDayLimit: 'amount',
maxAge: 'date',
overLimit: 'amount',
overLimitAttendee: 'amount',
overCategoryLimit: 'amount',
receiptRequired: 'receipt',
missingCategory: 'category',
categoryOutOfPolicy: 'category',
missingTag: 'tag',
tagOutOfPolicy: 'tag',
missingComment: 'comment',
taxRequired: 'tax',
taxOutOfPolicy: 'tax',
billableExpense: 'billable',
trevor-coleman marked this conversation as resolved.
Show resolved Hide resolved
};

/**
* Names of Fields where violations can occur
*/
type ViolationField = 'merchant' | 'amount' | 'category' | 'date' | 'tag' | 'comment' | 'billable' | 'receipt' | 'tax';
trevor-coleman marked this conversation as resolved.
Show resolved Hide resolved

/**
* Map from field name to array of violation types that can occur on that field.
* @example
* {
* // ...
* category: ['missingCategory', 'categoryOutOfPolicy']
* // ...
* }
*/
const possibleViolationsByField = invertBy(violationFields) as Record<ViolationField, ViolationName[]>;

export default possibleViolationsByField;
export type {ViolationField};
2 changes: 2 additions & 0 deletions src/types/onyx/PolicyCategory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,6 @@ type PolicyCategory = {
origin: string;
};

type PolicyCategories = Record<string, PolicyCategory>;
export default PolicyCategory;
export type {PolicyCategories};
34 changes: 34 additions & 0 deletions src/types/onyx/TransactionViolation.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/**
* @module TransactionViolation
* @description Transaction Violation
*/

trevor-coleman marked this conversation as resolved.
Show resolved Hide resolved
/**
* Names of the various Transaction Violation types
trevor-coleman marked this conversation as resolved.
Show resolved Hide resolved
*/
type ViolationName =
| 'perDayLimit'
| 'maxAge'
| 'overLimit'
| 'overLimitAttendee'
| 'overCategoryLimit'
| 'receiptRequired'
| 'missingCategory'
| 'categoryOutOfPolicy'
| 'missingTag'
| 'tagOutOfPolicy'
| 'missingComment'
| 'taxRequired'
| 'taxOutOfPolicy'
| 'billableExpense';
trevor-coleman marked this conversation as resolved.
Show resolved Hide resolved

type ViolationType = string;
trevor-coleman marked this conversation as resolved.
Show resolved Hide resolved

type TransactionViolation = {
type: ViolationType;
name: ViolationName;
userMessage: string;
data?: Record<string, string>;
};

export type {TransactionViolation, ViolationName, ViolationType};
7 changes: 6 additions & 1 deletion src/types/onyx/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import PersonalBankAccount from './PersonalBankAccount';
import PersonalDetails from './PersonalDetails';
import PlaidData from './PlaidData';
import Policy from './Policy';
import PolicyCategory from './PolicyCategory';
import PolicyCategory, {PolicyCategories} from './PolicyCategory';
import PolicyMember, {PolicyMembers} from './PolicyMember';
import PolicyTag, {PolicyTags} from './PolicyTag';
import PrivatePersonalDetails from './PrivatePersonalDetails';
Expand All @@ -42,6 +42,7 @@ import SecurityGroup from './SecurityGroup';
import Session from './Session';
import Task from './Task';
import Transaction from './Transaction';
import {TransactionViolation, ViolationName, ViolationType} from './TransactionViolation';
import User from './User';
import UserWallet from './UserWallet';
import WalletAdditionalDetails from './WalletAdditionalDetails';
Expand Down Expand Up @@ -78,6 +79,7 @@ export type {
PlaidData,
Policy,
PolicyCategory,
PolicyCategories,
PolicyMember,
PolicyMembers,
PolicyTag,
Expand All @@ -101,8 +103,11 @@ export type {
Session,
Task,
Transaction,
TransactionViolation,
User,
UserWallet,
ViolationName,
ViolationType,
WalletAdditionalDetails,
WalletOnfido,
WalletStatement,
Expand Down
Loading