Skip to content

Commit

Permalink
Merge pull request #37461 from Expensify/carlos-multitags
Browse files Browse the repository at this point in the history
  • Loading branch information
@blimpich
blimpich authored Mar 8, 2024
2 parents 838e3fd + e77b01a commit 4ada067
Show file tree
Hide file tree
Showing 7 changed files with 222 additions and 45 deletions.
16 changes: 9 additions & 7 deletions src/components/ReportActionItem/MoneyRequestView.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,10 @@ function MoneyRequestView({
const shouldShowBillable = isPolicyExpenseChat && (!!transactionBillable || !(policy?.disabledFields?.defaultBillable ?? true));

const {getViolationsForField} = useViolations(transactionViolations ?? []);
const hasViolations = useCallback((field: ViolationField): boolean => !!canUseViolations && getViolationsForField(field).length > 0, [canUseViolations, getViolationsForField]);
const hasViolations = useCallback(
(field: ViolationField, data?: OnyxTypes.TransactionViolation['data']): boolean => !!canUseViolations && getViolationsForField(field, data).length > 0,
[canUseViolations, getViolationsForField],
);

let amountDescription = `${translate('iou.amount')}`;

Expand Down Expand Up @@ -197,7 +200,7 @@ function MoneyRequestView({
const getPendingFieldAction = (fieldPath: TransactionPendingFieldsKey) => transaction?.pendingFields?.[fieldPath] ?? pendingAction;

const getErrorForField = useCallback(
(field: ViolationField, data?: OnyxTypes.TransactionViolation['data'], shouldShowViolations = true) => {
(field: ViolationField, data?: OnyxTypes.TransactionViolation['data']) => {
// Checks applied when creating a new money request
// NOTE: receipt field can return multiple violations, so we need to handle it separately
const fieldChecks: Partial<Record<ViolationField, {isError: boolean; translationPath: TranslationPaths}>> = {
Expand All @@ -223,9 +226,8 @@ function MoneyRequestView({
}

// Return violations if there are any
// At the moment, we only return violations for tags for workspaces with single-level tags
if (canUseViolations && shouldShowViolations && hasViolations(field)) {
const violations = getViolationsForField(field);
if (canUseViolations && hasViolations(field, data)) {
const violations = getViolationsForField(field, data);
return ViolationsUtils.getViolationTranslation(violations[0], translate);
}

Expand Down Expand Up @@ -395,8 +397,8 @@ function MoneyRequestView({
ROUTES.MONEY_REQUEST_STEP_TAG.getRoute(CONST.IOU.ACTION.EDIT, CONST.IOU.TYPE.REQUEST, index, transaction?.transactionID ?? '', report.reportID),
)
}
brickRoadIndicator={getErrorForField('tag', {}, policyTagLists.length === 1) ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined}
error={getErrorForField('tag', {}, policyTagLists.length === 1)}
brickRoadIndicator={getErrorForField('tag', {tagListIndex: index, tagListName: name}) ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined}
error={getErrorForField('tag', {tagListIndex: index, tagListName: name})}
/>
</OfflineWithFeedback>
))}
Expand Down
29 changes: 28 additions & 1 deletion src/hooks/useViolations.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,34 @@ function useViolations(violations: TransactionViolation[]) {
}
return violationGroups ?? new Map();
}, [violations]);
const getViolationsForField = useCallback((field: ViolationField) => violationsByField.get(field) ?? [], [violationsByField]);

const getViolationsForField = useCallback(
(field: ViolationField, data?: TransactionViolation['data']) => {
const currentViolations = violationsByField.get(field) ?? [];

// someTagLevelsRequired has special logic becase data.errorIndexes is a bit unique in how it denotes the tag list that has the violation
// tagListIndex can be 0 so we compare with undefined
if (currentViolations[0]?.name === 'someTagLevelsRequired' && data?.tagListIndex !== undefined && Array.isArray(currentViolations[0]?.data?.errorIndexes)) {
return currentViolations
.filter((violation) => violation.data?.errorIndexes?.includes(data?.tagListIndex ?? -1))
.map((violation) => ({
...violation,
data: {
...violation.data,
tagName: data?.tagListName,
},
}));
}

// tagOutOfPolicy has special logic because we have to account for multi-level tags and use tagName to find the right tag to put the violation on
if (currentViolations[0]?.name === 'tagOutOfPolicy' && data?.tagListName !== undefined && currentViolations[0]?.data?.tagName) {
return currentViolations.filter((violation) => violation.data?.tagName === data?.tagListName);
}

return currentViolations;
},
[violationsByField],
);

return {
getViolationsForField,
Expand Down
2 changes: 1 addition & 1 deletion src/languages/en.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2430,7 +2430,7 @@ export default {
return '';
},
smartscanFailed: 'Receipt scanning failed. Enter details manually.',
someTagLevelsRequired: 'Missing tag',
someTagLevelsRequired: ({tagName}: ViolationsTagOutOfPolicyParams) => `Missing ${tagName ?? 'Tag'}`,
tagOutOfPolicy: ({tagName}: ViolationsTagOutOfPolicyParams) => `${tagName ?? 'Tag'} no longer valid`,
taxAmountChanged: 'Tax amount was modified',
taxOutOfPolicy: ({taxName}: ViolationsTaxOutOfPolicyParams) => `${taxName ?? 'Tax'} no longer valid`,
Expand Down
2 changes: 1 addition & 1 deletion src/languages/es.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2922,7 +2922,7 @@ export default {
return '';
},
smartscanFailed: 'No se pudo escanear el recibo. Introduce los datos manualmente',
someTagLevelsRequired: 'Falta etiqueta',
someTagLevelsRequired: ({tagName}: ViolationsTagOutOfPolicyParams) => `Falta ${tagName ?? 'Tag'}`,
tagOutOfPolicy: ({tagName}: ViolationsTagOutOfPolicyParams) => `La etiqueta ${tagName ? `${tagName} ` : ''}ya no es válida`,
taxAmountChanged: 'El importe del impuesto fue modificado',
taxOutOfPolicy: ({taxName}: ViolationsTaxOutOfPolicyParams) => `${taxName ?? 'El impuesto'} ya no es válido`,
Expand Down
137 changes: 107 additions & 30 deletions src/libs/Violations/ViolationsUtils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,106 @@ import type {TranslationPaths} from '@src/languages/types';
import ONYXKEYS from '@src/ONYXKEYS';
import type {PolicyCategories, PolicyTagList, Transaction, TransactionViolation} from '@src/types/onyx';

/**
* Calculates tag out of policy and missing tag violations for the given transaction
*/
function getTagViolationsForSingleLevelTags(
updatedTransaction: Transaction,
transactionViolations: TransactionViolation[],
policyRequiresTags: boolean,
policyTagList: PolicyTagList,
): TransactionViolation[] {
const policyTagKeys = Object.keys(policyTagList);
const policyTagListName = policyTagKeys[0];
const policyTags = policyTagList[policyTagListName]?.tags;
const hasTagOutOfPolicyViolation = transactionViolations.some((violation) => violation.name === CONST.VIOLATIONS.TAG_OUT_OF_POLICY);
const hasMissingTagViolation = transactionViolations.some((violation) => violation.name === CONST.VIOLATIONS.MISSING_TAG);
const isTagInPolicy = policyTags ? !!policyTags[updatedTransaction.tag ?? '']?.enabled : false;
let newTransactionViolations = [...transactionViolations];

// Add 'tagOutOfPolicy' violation if tag is not in policy
if (!hasTagOutOfPolicyViolation && updatedTransaction.tag && !isTagInPolicy) {
newTransactionViolations.push({name: CONST.VIOLATIONS.TAG_OUT_OF_POLICY, type: 'violation'});
}

// Remove 'tagOutOfPolicy' violation if tag is in policy
if (hasTagOutOfPolicyViolation && updatedTransaction.tag && isTagInPolicy) {
newTransactionViolations = reject(newTransactionViolations, {name: CONST.VIOLATIONS.TAG_OUT_OF_POLICY});
}

// Remove 'missingTag' violation if tag is valid according to policy
if (hasMissingTagViolation && isTagInPolicy) {
newTransactionViolations = reject(newTransactionViolations, {name: CONST.VIOLATIONS.MISSING_TAG});
}

// Add 'missingTag violation' if tag is required and not set
if (!hasMissingTagViolation && !updatedTransaction.tag && policyRequiresTags) {
newTransactionViolations.push({name: CONST.VIOLATIONS.MISSING_TAG, type: 'violation'});
}
return newTransactionViolations;
}

/**
* Calculates some tag levels required and missing tag violations for the given transaction
*/
function getTagViolationsForMultiLevelTags(
updatedTransaction: Transaction,
transactionViolations: TransactionViolation[],
policyRequiresTags: boolean,
policyTagList: PolicyTagList,
): TransactionViolation[] {
const policyTagKeys = Object.keys(policyTagList);
const selectedTags = updatedTransaction.tag?.split(CONST.COLON) ?? [];
let newTransactionViolations = [...transactionViolations];
newTransactionViolations = newTransactionViolations.filter(
(violation) => violation.name !== CONST.VIOLATIONS.SOME_TAG_LEVELS_REQUIRED && violation.name !== CONST.VIOLATIONS.TAG_OUT_OF_POLICY,
);

// We first get the errorIndexes for someTagLevelsRequired. If it's not empty, we puth SOME_TAG_LEVELS_REQUIRED in Onyx.
// Otherwise, we put TAG_OUT_OF_POLICY in Onyx (when applicable)
const errorIndexes = [];
for (let i = 0; i < policyTagKeys.length; i++) {
const isTagRequired = policyTagList[policyTagKeys[i]].required ?? true;
const isTagSelected = Boolean(selectedTags[i]);
if (isTagRequired && (!isTagSelected || (selectedTags.length === 1 && selectedTags[0] === ''))) {
errorIndexes.push(i);
}
}
if (errorIndexes.length !== 0) {
newTransactionViolations.push({
name: CONST.VIOLATIONS.SOME_TAG_LEVELS_REQUIRED,
type: 'violation',
data: {
errorIndexes,
},
});
} else {
let hasInvalidTag = false;
for (let i = 0; i < policyTagKeys.length; i++) {
const selectedTag = selectedTags[i];
const tags = policyTagList[policyTagKeys[i]].tags;
const isTagInPolicy = Object.values(tags).some((tag) => tag.name === selectedTag && Boolean(tag.enabled));
if (!isTagInPolicy) {
newTransactionViolations.push({
name: CONST.VIOLATIONS.TAG_OUT_OF_POLICY,
type: 'violation',
data: {
tagName: policyTagKeys[i],
},
});
hasInvalidTag = true;
break;
}
}
if (!hasInvalidTag) {
newTransactionViolations = reject(newTransactionViolations, {
name: CONST.VIOLATIONS.TAG_OUT_OF_POLICY,
});
}
}
return newTransactionViolations;
}

const ViolationsUtils = {
/**
* Checks a transaction for policy violations and returns an object with Onyx method, key and updated transaction
Expand All @@ -22,6 +122,7 @@ const ViolationsUtils = {
): OnyxUpdate {
let newTransactionViolations = [...transactionViolations];

// Calculate client-side category violations
if (policyRequiresCategories) {
const hasCategoryOutOfPolicyViolation = transactionViolations.some((violation) => violation.name === 'categoryOutOfPolicy');
const hasMissingCategoryViolation = transactionViolations.some((violation) => violation.name === 'missingCategory');
Expand Down Expand Up @@ -49,36 +150,12 @@ const ViolationsUtils = {
}
}

// Calculate client-side tag violations
if (policyRequiresTags) {
const policyTagKeys = Object.keys(policyTagList);

// At the moment, we only return violations for tags for workspaces with single-level tags
if (policyTagKeys.length === 1) {
const policyTagListName = policyTagKeys[0];
const policyTags = policyTagList[policyTagListName]?.tags;
const hasTagOutOfPolicyViolation = transactionViolations.some((violation) => violation.name === CONST.VIOLATIONS.TAG_OUT_OF_POLICY);
const hasMissingTagViolation = transactionViolations.some((violation) => violation.name === CONST.VIOLATIONS.MISSING_TAG);
const isTagInPolicy = policyTags ? !!policyTags[updatedTransaction.tag ?? '']?.enabled : false;

// Add 'tagOutOfPolicy' violation if tag is not in policy
if (!hasTagOutOfPolicyViolation && updatedTransaction.tag && !isTagInPolicy) {
newTransactionViolations.push({name: CONST.VIOLATIONS.TAG_OUT_OF_POLICY, type: 'violation'});
}

// Remove 'tagOutOfPolicy' violation if tag is in policy
if (hasTagOutOfPolicyViolation && updatedTransaction.tag && isTagInPolicy) {
newTransactionViolations = reject(newTransactionViolations, {name: CONST.VIOLATIONS.TAG_OUT_OF_POLICY});
}

// Remove 'missingTag' violation if tag is valid according to policy
if (hasMissingTagViolation && isTagInPolicy) {
newTransactionViolations = reject(newTransactionViolations, {name: CONST.VIOLATIONS.MISSING_TAG});
}
// Add 'missingTag violation' if tag is required and not set
if (!hasMissingTagViolation && !updatedTransaction.tag && policyRequiresTags) {
newTransactionViolations.push({name: CONST.VIOLATIONS.MISSING_TAG, type: 'violation'});
}
}
newTransactionViolations =
Object.keys(policyTagList).length === 1
? getTagViolationsForSingleLevelTags(updatedTransaction, newTransactionViolations, policyRequiresTags, policyTagList)
: getTagViolationsForMultiLevelTags(updatedTransaction, newTransactionViolations, policyRequiresTags, policyTagList);
}

return {
Expand Down Expand Up @@ -181,7 +258,7 @@ const ViolationsUtils = {
case 'smartscanFailed':
return translate('violations.smartscanFailed');
case 'someTagLevelsRequired':
return translate('violations.someTagLevelsRequired');
return translate('violations.someTagLevelsRequired', {tagName});
case 'tagOutOfPolicy':
return translate('violations.tagOutOfPolicy', {tagName});
case 'taxAmountChanged':
Expand Down
3 changes: 3 additions & 0 deletions src/types/onyx/TransactionViolation.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ type TransactionViolation = {
isTransactionOlderThan7Days?: boolean;
member?: string;
taxName?: string;
tagListIndex?: number;
tagListName?: string;
errorIndexes?: number[];
};
};

Expand Down
78 changes: 73 additions & 5 deletions tests/unit/ViolationUtilsTest.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,6 @@ describe('getViolationsOnyxData', () => {
Lunch: {name: 'Lunch', enabled: true},
Dinner: {name: 'Dinner', enabled: true},
},
Tag: {
name: 'Tag',
required: true,
tags: {Lunch: {enabled: true}, Dinner: {enabled: true}},
},
},
};
transaction.tag = 'Lunch';
Expand Down Expand Up @@ -201,4 +196,77 @@ describe('getViolationsOnyxData', () => {
expect(result.value).not.toContainEqual([missingTagViolation]);
});
});
describe('policy has multi level tags', () => {
beforeEach(() => {
policyRequiresTags = true;
policyTags = {
Department: {
name: 'Department',
tags: {
Accounting: {
name: 'Accounting',
enabled: true,
},
},
required: true,
},
Region: {
name: 'Region',
tags: {
Africa: {
name: 'Africa',
enabled: true,
},
},
},
Project: {
name: 'Project',
tags: {
Project1: {
name: 'Project1',
enabled: true,
},
},
required: true,
},
};
});
it('should return someTagLevelsRequired when a required tag is missing', () => {
const someTagLevelsRequiredViolation = {
name: 'someTagLevelsRequired',
type: 'violation',
data: {
errorIndexes: [0, 1, 2],
},
};

// Test case where transaction has no tags
let result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories);
expect(result.value).toEqual([someTagLevelsRequiredViolation]);

// Test case where transaction has 1 tag
transaction.tag = 'Accounting';
someTagLevelsRequiredViolation.data = {errorIndexes: [1, 2]};
result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories);
expect(result.value).toEqual([someTagLevelsRequiredViolation]);

// Test case where transaction has 2 tags
transaction.tag = 'Accounting::Project1';
someTagLevelsRequiredViolation.data = {errorIndexes: [1]};
result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories);
expect(result.value).toEqual([someTagLevelsRequiredViolation]);

// Test case where transaction has all tags
transaction.tag = 'Accounting:Africa:Project1';
result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories);
expect(result.value).toEqual([]);
});
it('should return tagOutOfPolicy when a tag is not enabled in the policy but is set in the transaction', () => {
policyTags.Department.tags.Accounting.enabled = false;
transaction.tag = 'Accounting:Africa:Project1';
const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories);
const violation = {...tagOutOfPolicyViolation, data: {tagName: 'Department'}};
expect(result.value).toEqual([violation]);
});
});
});

0 comments on commit 4ada067

Please sign in to comment.