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

Adds Client-side Violations when creating money requests #32528

Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
e4135d1
Possible solution to not passing down tags and categories
cdanwards Nov 21, 2023
8fe667d
Updated withPolicy with new properties and also passed through to IOU…
cdanwards Nov 28, 2023
1cd0e08
Added some logs for testing -- remove these
cdanwards Nov 28, 2023
22ee011
Fixed merge conflicts
cdanwards Nov 29, 2023
441788c
Merge branch 'trevor-coleman/violations/violation-utils' of github.co…
cdanwards Nov 30, 2023
0cf233c
Fixed merge conflicts
cdanwards Dec 5, 2023
6f84a87
remove unused export
cdanwards Dec 5, 2023
57f2aa8
Merge branch 'main' of github.com:infinitered/ExpensifyApp into cdanw…
cdanwards Dec 5, 2023
f6f9cac
Remove console.log statements and fix import path
cdanwards Dec 5, 2023
9a041cc
Remove unused function getPolicyTags
cdanwards Dec 5, 2023
150487e
Fixed withOnyx call
cdanwards Dec 6, 2023
5b8bcc5
Merge branch 'main' of github.com:infinitered/ExpensifyApp into cdanw…
cdanwards Dec 6, 2023
b2128e5
Merge branch 'main' of github.com:infinitered/ExpensifyApp into cdanw…
cdanwards Dec 8, 2023
6c30158
Update src/libs/actions/IOU.js
cdanwards Dec 11, 2023
705329c
Update src/pages/workspace/withPolicy.tsx
cdanwards Dec 11, 2023
c93a59d
Update src/pages/iou/steps/MoneyRequestConfirmPage.js
cdanwards Dec 11, 2023
b6e959f
Update src/pages/workspace/withPolicy.tsx
cdanwards Dec 11, 2023
f0c609b
Update src/pages/workspace/withPolicy.tsx
cdanwards Dec 11, 2023
dbf3e5a
Update src/pages/workspace/withPolicy.tsx
cdanwards Dec 11, 2023
9783c47
Update src/libs/actions/IOU.js
cdanwards Dec 11, 2023
66d2767
Merge branch 'main' of github.com:infinitered/ExpensifyApp into cdanw…
cdanwards Dec 13, 2023
3c0b52c
Updated proptypes and reimplemented the second withonyx
cdanwards Dec 13, 2023
1547445
Merge branch 'cdanwards/violations/money-request-violations' of githu…
cdanwards Dec 13, 2023
6c3a853
Merge branch 'main' into cdanwards/violations/money-request-violations
lindboe Dec 14, 2023
633199b
Fix withOnyx references
lindboe Dec 15, 2023
919f967
Add policy data to new verison of money request confirmation screen
lindboe Dec 18, 2023
9885e31
Merge remote-tracking branch 'upstream/main' into cdanwards/violation…
lindboe Dec 18, 2023
4b6f463
Merge remote-tracking branch 'upstream/main' into cdanwards/violation…
lindboe Dec 19, 2023
0f91f22
Fix hook dependencies
lindboe Dec 19, 2023
e5b607e
Fix transaction violation storage
lindboe Dec 19, 2023
a8741ab
Use SET instead
lindboe Dec 19, 2023
8d417a9
Check policy violations locally for distance requests
lindboe Dec 20, 2023
34c080e
Merge remote-tracking branch 'upstream/main' into cdanwards/violation…
lindboe Jan 2, 2024
ab06fbe
Use policy arg for fetching paid policy info
lindboe Jan 3, 2024
da3469e
Merge remote-tracking branch 'upstream/main' into cdanwards/violation…
lindboe Jan 3, 2024
9b2fb39
Merge remote-tracking branch 'upstream/main' into cdanwards/violation…
lindboe Jan 4, 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
58 changes: 57 additions & 1 deletion src/libs/actions/IOU.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import * as ReportActionsUtils from '@libs/ReportActionsUtils';
import * as ReportUtils from '@libs/ReportUtils';
import * as TransactionUtils from '@libs/TransactionUtils';
import * as UserUtils from '@libs/UserUtils';
import ViolationsUtils from '@libs/ViolationsUtils';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
Expand Down Expand Up @@ -126,6 +127,9 @@ function buildOnyxDataForMoneyRequest(
optimisticPolicyRecentlyUsedTags,
isNewChatReport,
isNewIOUReport,
policy,
policyTags,
policyCategories,
) {
const optimisticData = [
{
Expand Down Expand Up @@ -352,6 +356,25 @@ function buildOnyxDataForMoneyRequest(
},
];

if (!policy || !policy.id) {
return [optimisticData, successData, failureData];
}

const violationsOnyxData = ViolationsUtils.getViolationsOnyxData(transaction, [], policy.requiresTags, policyTags, policy.requiresCategory, policyCategories);

if (violationsOnyxData) {
optimisticData.push({
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transaction.transactionID}`,
value: violationsOnyxData,
});
failureData.push({
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transaction.transactionID}`,
value: [],
});
}

return [optimisticData, successData, failureData];
}

Expand All @@ -373,6 +396,9 @@ function buildOnyxDataForMoneyRequest(
* @param {String} [category]
* @param {String} [tag]
* @param {Boolean} [billable]
* @param {Object} [policy]
* @param {Object} [policyTags]
* @param {Object} [policyCategories]
* @returns {Object} data
* @returns {String} data.payerEmail
* @returns {Object} data.iouReport
Expand Down Expand Up @@ -402,6 +428,9 @@ function getMoneyRequestInformation(
category = undefined,
tag = undefined,
billable = undefined,
policy = undefined,
policyTags = undefined,
policyCategories = undefined,
) {
const payerEmail = OptionsListUtils.addSMSDomainIfPhoneNumber(participant.login);
const payerAccountID = Number(participant.accountID);
Expand Down Expand Up @@ -561,6 +590,9 @@ function getMoneyRequestInformation(
optimisticPolicyRecentlyUsedTags,
isNewChatReport,
isNewIOUReport,
policy,
policyTags,
policyCategories,
);

return {
Expand Down Expand Up @@ -809,6 +841,9 @@ function editDistanceMoneyRequest(transactionID, transactionThreadReportID, tran
* @param {String} [category]
* @param {String} [tag]
* @param {Boolean} [billable]
* @param {Object} [policy]
* @param {Object} [policyTags]
* @param {Object} [policyCategories]
*/
function requestMoney(
report,
Expand All @@ -824,12 +859,33 @@ function requestMoney(
category = undefined,
tag = undefined,
billable = undefined,
policy = undefined,
policyTags = undefined,
policyCategories = undefined,
) {
// If the report is iou or expense report, we should get the linked chat report to be passed to the getMoneyRequestInformation function
const isMoneyRequestReport = ReportUtils.isMoneyRequestReport(report);
const currentChatReport = isMoneyRequestReport ? ReportUtils.getReport(report.chatReportID) : report;
const {payerAccountID, payerEmail, iouReport, chatReport, transaction, iouAction, createdChatReportActionID, createdIOUReportActionID, reportPreviewAction, onyxData} =
getMoneyRequestInformation(currentChatReport, participant, comment, amount, currency, created, merchant, payeeAccountID, payeeEmail, receipt, undefined, category, tag, billable);
getMoneyRequestInformation(
currentChatReport,
participant,
comment,
amount,
currency,
created,
merchant,
payeeAccountID,
payeeEmail,
receipt,
undefined,
category,
tag,
billable,
policy,
policyTags,
policyCategories,
);
const activeReportID = isMoneyRequestReport ? report.reportID : chatReport.reportID;

API.write(
Expand Down
32 changes: 29 additions & 3 deletions src/pages/iou/steps/MoneyRequestConfirmPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import * as OptionsListUtils from '@libs/OptionsListUtils';
import * as ReportUtils from '@libs/ReportUtils';
import {iouDefaultProps, iouPropTypes} from '@pages/iou/propTypes';
import reportPropTypes from '@pages/reportPropTypes';
import {policyDefaultProps, policyPropTypes} from '@pages/workspace/withPolicy';
import useThemeStyles from '@styles/useThemeStyles';
import * as IOU from '@userActions/IOU';
import * as Policy from '@userActions/Policy';
Expand All @@ -47,12 +48,28 @@ const propTypes = {
/** Holds data related to Money Request view state, rather than the underlying Money Request data. */
iou: iouPropTypes,

/** The policy of the current report */
policy: policyPropTypes,

policyTags: PropTypes.shape({
/** List of tags */
tags: PropTypes.arrayOf(PropTypes.string),
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 pretty sure this and categories should be objects. They're {} in defaultProps, and in Onyx they're objects as well
image
image

}),

policyCategories: PropTypes.shape({
/** List of categories */
categories: PropTypes.arrayOf(PropTypes.string),
}),

...withCurrentUserPersonalDetailsPropTypes,
};

const defaultProps = {
report: {},
policyCategories: {},
policyTags: {},
iou: iouDefaultProps,
policy: policyDefaultProps,
...withCurrentUserPersonalDetailsDefaultProps,
};

Expand Down Expand Up @@ -163,6 +180,9 @@ function MoneyRequestConfirmPage(props) {
props.iou.category,
props.iou.tag,
props.iou.billable,
props.policy,
props.policyTags,
props.policyCategories,
);
},
[
Expand All @@ -176,6 +196,9 @@ function MoneyRequestConfirmPage(props) {
props.iou.category,
props.iou.tag,
props.iou.billable,
props.policy,
props.policyTags,
props.policyCategories,
],
);

Expand Down Expand Up @@ -423,11 +446,14 @@ export default compose(
selectedTab: {
key: `${ONYXKEYS.COLLECTION.SELECTED_TAB}${CONST.TAB.RECEIPT_TAB_ID}`,
},
}),
// eslint-disable-next-line rulesdir/no-multiple-onyx-in-file
withOnyx({
Copy link
Contributor

Choose a reason for hiding this comment

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

Why remove this? I think this is wrong, because having a separate withOnyx makes the keys in it "dependent" on the values of the previous withOnyx, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wasn't aware of this! I'll ask in the open-source channel, but I'm happy to change this back.

policy: {
key: ({report}) => `${ONYXKEYS.COLLECTION.POLICY}${report ? report.policyID : '0'}`,
},
policyCategories: {
key: ({policy}) => `${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${policy ? policy.id : '0'}`,
Copy link
Contributor

Choose a reason for hiding this comment

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

When/why can this be called without a policy?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So it looks that when this file is converted to TS that the policy will be of the OnyxEntry type which could return null so this was just to go ahead and account for that. Once it is converted it can easily just be replaced with optional chaining like policy?.id.

Copy link
Contributor

Choose a reason for hiding this comment

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

I asked about the multiple withOnyx thing from the comment above on slack

So it looks that when this file is converted to TS that the policy will be of the OnyxEntry type which could return null so this was just to go ahead and account for that

But is it possible that this is because these are no longer defined in a separate withOnyx?

},
policyTags: {
key: ({policy}) => `${ONYXKEYS.COLLECTION.POLICY_TAGS}${policy ? policy.id : '0'}`,
},
Copy link
Contributor

Choose a reason for hiding this comment

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

I got an error

Screenshot 2023-12-16 at 03 33 24

I think the policy data is not yet found. Either we go with the multiple onyx or use the report.policyID instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

Updated to use report.policyID.

}),
)(MoneyRequestConfirmPage);
14 changes: 14 additions & 0 deletions src/pages/workspace/withPolicy.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,20 @@ const policyPropTypes = {
* }
*/
errorFields: PropTypes.objectOf(PropTypes.objectOf(PropTypes.string)),

/** Whether or not the policy requires tags */
requiresTags: PropTypes.bool,

/** Whether or not the policy requires categories */
requiresCategories: PropTypes.bool,

/** Whether or not the policy has multiple tag lists */
hasMultipleTagLists: PropTypes.bool,

/** Whether or not the policy has tax tracking enabled */
isTrackingTaxEnabled: PropTypes.bool,

/** */
}),

/** The employee list of this policy */
Expand Down