Skip to content

Commit

Permalink
Merge pull request #36821 from Expensify/carlos-api
Browse files Browse the repository at this point in the history
  • Loading branch information
@cead22
cead22 authored Feb 23, 2024
2 parents a709122 + 1ee821d commit 7550a7e
Show file tree
Hide file tree
Showing 8 changed files with 46 additions and 117 deletions.
6 changes: 3 additions & 3 deletions src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ function MoneyRequestPreviewContent({
const requestMerchant = truncate(merchant, {length: CONST.REQUEST_PREVIEW.MAX_LENGTH});
const hasReceipt = TransactionUtils.hasReceipt(transaction);
const isScanning = hasReceipt && TransactionUtils.isReceiptBeingScanned(transaction);
const hasViolations = TransactionUtils.hasViolation(transaction, transactionViolations);
const hasViolations = TransactionUtils.hasViolation(transaction?.transactionID ?? '', transactionViolations);
const hasFieldErrors = TransactionUtils.hasMissingSmartscanFields(transaction);
const shouldShowRBR = hasViolations || hasFieldErrors;
const isDistanceRequest = TransactionUtils.isDistanceRequest(transaction);
Expand Down Expand Up @@ -152,13 +152,13 @@ function MoneyRequestPreviewContent({

let message = translate('iou.cash');
if (hasViolations && transaction) {
const violations = TransactionUtils.getTransactionViolations(transaction, transactionViolations);
const violations = TransactionUtils.getTransactionViolations(transaction.transactionID, transactionViolations);
if (violations?.[0]) {
const violationMessage = ViolationsUtils.getViolationTranslation(violations[0], translate);
const isTooLong = violations.filter((v) => v.type === 'violation').length > 1 || violationMessage.length > 15;
message += ` • ${isTooLong ? translate('violations.reviewRequired') : violationMessage}`;
}
} else if (ReportUtils.isPaidGroupPolicyExpenseReport(iouReport) && ReportUtils.isReportApproved(iouReport) && !ReportUtils.isSettled(iouReport)) {
} else if (ReportUtils.isPaidGroupPolicyExpenseReport(iouReport) && ReportUtils.isReportApproved(iouReport) && !ReportUtils.isSettled(iouReport?.reportID)) {
message += ` • ${translate('iou.approved')}`;
} else if (iouReport?.isWaitingOnBankAccount) {
message += ` • ${translate('iou.pending')}`;
Expand Down
24 changes: 7 additions & 17 deletions src/components/ReportActionItem/MoneyRequestView.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,10 +145,7 @@ function MoneyRequestView({
const shouldShowBillable = isPolicyExpenseChat && (!!transactionBillable || !(policy?.disabledFields?.defaultBillable ?? true));

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

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

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

const getErrorForField = useCallback(
(field: ViolationField, data?: OnyxTypes.TransactionViolation['data']) => {
(field: ViolationField, data?: OnyxTypes.TransactionViolation['data'], shouldShowViolations = true) => {
// 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 @@ -228,8 +225,9 @@ function MoneyRequestView({
}

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

Expand Down Expand Up @@ -400,16 +398,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', {
tagName: name,
})
? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR
: undefined
}
error={getErrorForField('tag', {
tagName: name,
})}
brickRoadIndicator={getErrorForField('tag', {}, policyTagLists.length === 1) ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined}
error={getErrorForField('tag', {}, policyTagLists.length === 1)}
/>
</OfflineWithFeedback>
))}
Expand Down
14 changes: 1 addition & 13 deletions src/hooks/useViolations.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,19 +58,7 @@ function useViolations(violations: TransactionViolation[]) {
}
return violationGroups ?? new Map();
}, [violations]);

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

if (data?.tagName) {
return currentViolations.filter((violation) => violation.data?.tagName === data.tagName);
}

return currentViolations;
},
[violationsByField],
);
const getViolationsForField = useCallback((field: ViolationField) => violationsByField.get(field) ?? [], [violationsByField]);

return {
getViolationsForField,
Expand Down
6 changes: 2 additions & 4 deletions src/libs/ReportUtils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -722,12 +722,10 @@ function hasParticipantInArray(report: Report, policyMemberAccountIDs: number[])
/**
* Whether the Money Request report is settled
*/
function isSettled(reportOrID: Report | OnyxEntry<Report> | string | undefined): boolean {
if (!allReports || !reportOrID) {
function isSettled(reportID: string | undefined): boolean {
if (!allReports || !reportID) {
return false;
}
const reportID = typeof reportOrID === 'string' ? reportOrID : reportOrID?.reportID;

const report: Report | EmptyObject = allReports[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`] ?? {};
if (isEmptyObject(report) || report.isWaitingOnBankAccount) {
return false;
Expand Down
13 changes: 2 additions & 11 deletions src/libs/TransactionUtils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -587,20 +587,11 @@ function isOnHold(transaction: OnyxEntry<Transaction>): boolean {
/**
* Checks if any violations for the provided transaction are of type 'violation'
*/
function hasViolation(transactionOrID: Transaction | OnyxEntry<Transaction> | string, transactionViolations: OnyxCollection<TransactionViolation[]>): boolean {
if (!transactionOrID) {
return false;
}
const transactionID = typeof transactionOrID === 'string' ? transactionOrID : transactionOrID.transactionID;
function hasViolation(transactionID: string, transactionViolations: OnyxCollection<TransactionViolation[]>): boolean {
return Boolean(transactionViolations?.[ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS + transactionID]?.some((violation: TransactionViolation) => violation.type === 'violation'));
}

function getTransactionViolations(transactionOrID: OnyxEntry<Transaction> | string, transactionViolations: OnyxCollection<TransactionViolation[]>): TransactionViolation[] | null {
if (!transactionOrID) {
return null;
}
const transactionID = typeof transactionOrID === 'string' ? transactionOrID : transactionOrID.transactionID;

function getTransactionViolations(transactionID: string, transactionViolations: OnyxCollection<TransactionViolation[]>): TransactionViolation[] | null {
return transactionViolations?.[ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS + transactionID] ?? null;
}

Expand Down
67 changes: 17 additions & 50 deletions src/libs/Violations/ViolationsUtils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import reject from 'lodash/reject';
import Onyx from 'react-native-onyx';
import type {OnyxUpdate} from 'react-native-onyx';
import type {Phrase, PhraseParameters} from '@libs/Localize';
import * as TransactionUtils from '@libs/TransactionUtils';
import CONST from '@src/CONST';
import type {TranslationPaths} from '@src/languages/types';
import ONYXKEYS from '@src/ONYXKEYS';
Expand Down Expand Up @@ -31,7 +30,7 @@ const ViolationsUtils = {

// Add 'categoryOutOfPolicy' violation if category is not in policy
if (!hasCategoryOutOfPolicyViolation && categoryKey && !isCategoryInPolicy) {
newTransactionViolations.push({name: 'categoryOutOfPolicy', type: 'violation', userMessage: ''});
newTransactionViolations.push({name: 'categoryOutOfPolicy', type: 'violation'});
}

// Remove 'categoryOutOfPolicy' violation if category is in policy
Expand All @@ -46,72 +45,40 @@ const ViolationsUtils = {

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

if (policyRequiresTags) {
const selectedTags = TransactionUtils.getTagArrayFromName(updatedTransaction.tag ?? '') ?? [];
const policyTagKeys = Object.keys(policyTagList);

if (policyTagKeys.length === 0) {
newTransactionViolations.push({
name: CONST.VIOLATIONS.TAG_OUT_OF_POLICY,
type: 'violation',
userMessage: '',
});
}

policyTagKeys.forEach((key, index) => {
const hasTagOutOfPolicyViolation = transactionViolations.some((violation) => violation.name === CONST.VIOLATIONS.TAG_OUT_OF_POLICY && violation.data?.tagName === key);
const hasMissingTagViolation = transactionViolations.some((violation) => violation.name === CONST.VIOLATIONS.MISSING_TAG && violation.data?.tagName === key);
const selectedTag = selectedTags[index];
const isTagInPolicy = Boolean(policyTagList[key]?.tags[selectedTag]?.enabled);
// 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 && selectedTag && !isTagInPolicy) {
newTransactionViolations.push({
name: CONST.VIOLATIONS.TAG_OUT_OF_POLICY,
type: 'violation',
userMessage: '',
data: {
tagName: key,
},
});
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 && selectedTag && isTagInPolicy) {
newTransactionViolations = reject(newTransactionViolations, {
name: CONST.VIOLATIONS.TAG_OUT_OF_POLICY,
data: {
tagName: key,
},
});
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,
data: {
tagName: key,
},
});
newTransactionViolations = reject(newTransactionViolations, {name: CONST.VIOLATIONS.MISSING_TAG});
}

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

return {
Expand Down
1 change: 0 additions & 1 deletion src/types/onyx/TransactionViolation.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ type ViolationName = (typeof CONST.VIOLATIONS)[keyof typeof CONST.VIOLATIONS];
type TransactionViolation = {
type: string;
name: ViolationName;
userMessage: string;
data?: {
rejectedBy?: string;
rejectReason?: string;
Expand Down
32 changes: 14 additions & 18 deletions tests/unit/ViolationUtilsTest.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,21 @@ import ONYXKEYS from '@src/ONYXKEYS';
const categoryOutOfPolicyViolation = {
name: 'categoryOutOfPolicy',
type: 'violation',
userMessage: '',
};

const missingCategoryViolation = {
name: 'missingCategory',
type: 'violation',
userMessage: '',
};

const tagOutOfPolicyViolation = {
name: 'tagOutOfPolicy',
type: 'violation',
userMessage: '',
};

const missingTagViolation = {
name: 'missingTag',
type: 'violation',
userMessage: '',
};

describe('getViolationsOnyxData', () => {
Expand Down Expand Up @@ -56,8 +52,8 @@ describe('getViolationsOnyxData', () => {

it('should handle multiple violations', () => {
transactionViolations = [
{name: 'duplicatedTransaction', type: 'violation', userMessage: ''},
{name: 'receiptRequired', type: 'violation', userMessage: ''},
{name: 'duplicatedTransaction', type: 'violation'},
{name: 'receiptRequired', type: 'violation'},
];
const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories);
expect(result.value).toEqual(expect.arrayContaining(transactionViolations));
Expand Down Expand Up @@ -90,8 +86,8 @@ describe('getViolationsOnyxData', () => {
it('should add categoryOutOfPolicy violation to existing violations if they exist', () => {
transaction.category = 'Bananas';
transactionViolations = [
{name: 'duplicatedTransaction', type: 'violation', userMessage: ''},
{name: 'receiptRequired', type: 'violation', userMessage: ''},
{name: 'duplicatedTransaction', type: 'violation'},
{name: 'receiptRequired', type: 'violation'},
];

const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories);
Expand All @@ -102,8 +98,8 @@ describe('getViolationsOnyxData', () => {
it('should add missingCategory violation to existing violations if they exist', () => {
transaction.category = undefined;
transactionViolations = [
{name: 'duplicatedTransaction', type: 'violation', userMessage: ''},
{name: 'receiptRequired', type: 'violation', userMessage: ''},
{name: 'duplicatedTransaction', type: 'violation'},
{name: 'receiptRequired', type: 'violation'},
];

const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories);
Expand Down Expand Up @@ -157,39 +153,39 @@ describe('getViolationsOnyxData', () => {

const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories);

expect(result.value).toEqual(expect.arrayContaining([{...missingTagViolation, data: {tagName: 'Meals'}}]));
expect(result.value).toEqual(expect.arrayContaining([{...missingTagViolation}]));
});

it('should add a tagOutOfPolicy violation when policy requires tags and tag is not in the policy', () => {
policyTags = {};

const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories);

expect(result.value).toEqual(expect.arrayContaining([tagOutOfPolicyViolation]));
expect(result.value).toEqual([]);
});

it('should add tagOutOfPolicy violation to existing violations if transaction has tag that is not in the policy', () => {
transaction.tag = 'Bananas';
transactionViolations = [
{name: 'duplicatedTransaction', type: 'violation', userMessage: ''},
{name: 'receiptRequired', type: 'violation', userMessage: ''},
{name: 'duplicatedTransaction', type: 'violation'},
{name: 'receiptRequired', type: 'violation'},
];

const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories);

expect(result.value).toEqual(expect.arrayContaining([{...tagOutOfPolicyViolation, data: {tagName: 'Meals'}}, ...transactionViolations]));
expect(result.value).toEqual(expect.arrayContaining([{...tagOutOfPolicyViolation}, ...transactionViolations]));
});

it('should add missingTag violation to existing violations if transaction does not have a tag', () => {
transaction.tag = undefined;
transactionViolations = [
{name: 'duplicatedTransaction', type: 'violation', userMessage: ''},
{name: 'receiptRequired', type: 'violation', userMessage: ''},
{name: 'duplicatedTransaction', type: 'violation'},
{name: 'receiptRequired', type: 'violation'},
];

const result = ViolationsUtils.getViolationsOnyxData(transaction, transactionViolations, policyRequiresTags, policyTags, policyRequiresCategories, policyCategories);

expect(result.value).toEqual(expect.arrayContaining([{...missingTagViolation, data: {tagName: 'Meals'}}, ...transactionViolations]));
expect(result.value).toEqual(expect.arrayContaining([{...missingTagViolation}, ...transactionViolations]));
});
});

Expand Down

0 comments on commit 7550a7e

Please sign in to comment.