App: [HOLD for payment 2024-01-17] Implement Client-side Violations when creating money requests

Design Doc
The following should only apply to users in the violations beta
Let’s connect MoneyRequestConfirmPage to the POLICY, POLICY_CATEGORIES, and POLICY_TAGS Onyx collections so we can get
- requiresTag, requiresCategory, hasMultipleTagLists, and isTaxTrackingEnabled from the policy
- The policy categories, and
- The policy tags
In src/pages/iou/steps/MoneyRequestConfirmPage.js let’s pass props.policy.policyID to IOU.requestMoney > from there to getMoneyRequestInformation > and from there to buildOnyxDataForMoneyRequest
In buildOnyxDataForMoneyRequest
- We’ll check If policyID is not available, like for the case of P2P, and return early in that case
- Then call ViolationUtils.getViolationsOnyxData(transaction, [], policyRequiresTags, policyTags, policyRequiresCategories, policyCategories) and push the return value to optimisticData
- Note: we’re not gonna block users from submitting a money request with violations
- Refer https://github.com/Expensify/App/issues/31083 to see the data returned by getViolationsOnyxData
- We can set the value of failure data to an object like the snippet below, to remove the violations if the request isn’t created
The idea here is that when requests are created, the transactions calculated on the client by ViolationUtils.getViolationsOnyxData should show right away (eg, missing tag, missing category, category out of policy,…)

{
    onyxMethod: Onyx.METHOD.MERGE,
    key: `${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transactionID}`,
    value: [],
}

About this issue

Original URL
State: closed
Created 8 months ago
Comments: 19 (13 by maintainers)

Most upvoted comments

Off hold

cead22 on Dec 4, 2023

@cead22 Great! This is mostly finished, just making sure 31656 doesn’t have any last changes then I’ll get this up to a PR.

cdanwards on Nov 30, 2023

While originally I was thinking this would be most useful when using the getter getPolicy (which we won’t do), I do still think this would be valuable to document in the type that a policy can have these properties.

Thanks for clarifying, that makes sense to me, so long as it doesn’t break anything (and let’s not block on it)

cead22 on Nov 30, 2023

@cead22

You mean MoneyRequestConfirmPage?

Yes! Sorry, I thought I was referring to the correct component.

You mean as part of this PR you’re working on? I don’t see the connection to those collection in main. If that’s the case, let me know and I’ll check the design doc to see if we’re going to use them somewhere else, and we just haven’t added the code for it yet

I think this was just me making an assumption. Those properties weren’t listed as being passed down through the functions but they are needed in the subsequent ViolationsUtils.getViolationsOnyxData call. And after looking through how those getter methods are working I think the point I was getting to was a bit of a moot point 😅 So we can do as you specified originally.

had a thought that maybe we could update the policy PropTypes to include requiresTag, requiresCategory…

I don’t see the benefit of this, but I’m kind of a react noob, can you perhaps spell it out?

While originally I was thinking this would be most useful when using the getter getPolicy (which we won’t do), I do still think this would be valuable to document in the type that a policy can have these properties.

cdanwards on Nov 27, 2023