-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Enable Tax and Tax Amount fields in selfDM #74441
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
Changes from all commits
aea224f
428465e
179f6ba
5a88e20
d852a72
ebf57af
f2e829d
bb80896
6c1d57b
a885995
b8da753
3957df3
92db469
f7d472e
c3340d1
706078c
61abb63
959c49e
dcca29f
6b2d20f
d8560d5
8dd4474
5cb7582
1379bbf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -253,6 +253,8 @@ function MoneyRequestConfirmationList({ | |||||||||||
| selector: mileageRateSelector, | ||||||||||||
| canBeMissing: true, | ||||||||||||
| }); | ||||||||||||
| const {policyForMovingExpenses} = usePolicyForMovingExpenses(); | ||||||||||||
| const isMovingTransactionFromTrackExpense = isMovingTransactionFromTrackExpenseUtil(action); | ||||||||||||
| const [defaultMileageRateReal] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, { | ||||||||||||
| selector: mileageRateSelector, | ||||||||||||
| canBeMissing: true, | ||||||||||||
|
|
@@ -280,7 +282,6 @@ function MoneyRequestConfirmationList({ | |||||||||||
| isTestDriveReceipt || isManagerMcTestReceipt, | ||||||||||||
| ); | ||||||||||||
|
|
||||||||||||
| const {policyForMovingExpenses} = usePolicyForMovingExpenses(); | ||||||||||||
| const isTrackExpense = iouType === CONST.IOU.TYPE.TRACK; | ||||||||||||
| const policy = isTrackExpense ? policyForMovingExpenses : (policyReal ?? policyDraft); | ||||||||||||
| const policyCategories = policyCategoriesReal ?? policyCategoriesDraft; | ||||||||||||
|
|
@@ -298,7 +299,6 @@ function MoneyRequestConfirmationList({ | |||||||||||
| const isTypeInvoice = iouType === CONST.IOU.TYPE.INVOICE; | ||||||||||||
| const isScanRequest = useMemo(() => isScanRequestUtil(transaction), [transaction]); | ||||||||||||
| const isCreateExpenseFlow = !!transaction?.isFromGlobalCreate && !isPerDiemRequest; | ||||||||||||
| const isMovingTransactionFromTrackExpense = isMovingTransactionFromTrackExpenseUtil(action); | ||||||||||||
|
|
||||||||||||
| const transactionID = transaction?.transactionID; | ||||||||||||
| const customUnitRateID = getRateID(transaction); | ||||||||||||
|
|
@@ -342,16 +342,18 @@ function MoneyRequestConfirmationList({ | |||||||||||
|
|
||||||||||||
| const policyTagLists = useMemo(() => getTagLists(policyTags), [policyTags]); | ||||||||||||
|
|
||||||||||||
| const shouldShowTax = isTaxTrackingEnabled(isPolicyExpenseChat, policy, isDistanceRequest, isPerDiemRequest); | ||||||||||||
| const shouldShowTax = isTaxTrackingEnabled(isPolicyExpenseChat || isTrackExpense, policy, isDistanceRequest, isPerDiemRequest); | ||||||||||||
|
|
||||||||||||
| // Update the tax code when the default changes (for example, because the transaction currency changed) | ||||||||||||
| const defaultTaxCode = getDefaultTaxCode(policy, transaction) ?? ''; | ||||||||||||
| const defaultTaxCode = getDefaultTaxCode(policy, transaction) ?? (isMovingTransactionFromTrackExpense ? (getDefaultTaxCode(policyForMovingExpenses, transaction) ?? '') : ''); | ||||||||||||
|
|
||||||||||||
| useEffect(() => { | ||||||||||||
| if (!transactionID || isReadOnly) { | ||||||||||||
| if (!transactionID || isMovingTransactionFromTrackExpense || isReadOnly) { | ||||||||||||
| return; | ||||||||||||
| } | ||||||||||||
| setMoneyRequestTaxRate(transactionID, defaultTaxCode); | ||||||||||||
| }, [defaultTaxCode, transactionID, isReadOnly]); | ||||||||||||
| // trigger this useEffect also when policyID changes - the defaultTaxCode may stay the same | ||||||||||||
| }, [defaultTaxCode, isMovingTransactionFromTrackExpense, isReadOnly, transactionID, policyID]); | ||||||||||||
|
|
||||||||||||
| const distance = getDistanceInMeters(transaction, unit); | ||||||||||||
| const prevDistance = usePrevious(distance); | ||||||||||||
|
|
@@ -502,15 +504,18 @@ function MoneyRequestConfirmationList({ | |||||||||||
|
|
||||||||||||
| // Calculate and set tax amount in transaction draft | ||||||||||||
| const taxableAmount = isDistanceRequest ? DistanceRequestUtils.getTaxableAmount(policy, customUnitRateID, distance) : (transaction?.amount ?? 0); | ||||||||||||
| const taxPercentage = getTaxValue(policy, transaction, transaction?.taxCode ?? defaultTaxCode) ?? ''; | ||||||||||||
| // First we'll try to get the tax value from the chosen policy and if not found, we'll try to get it from the policy for moving expenses (only if the transaction is moving from track expense) | ||||||||||||
| const taxPercentage = | ||||||||||||
| getTaxValue(policy, transaction, transaction?.taxCode ?? defaultTaxCode) ?? | ||||||||||||
| (isMovingTransactionFromTrackExpense ? getTaxValue(policyForMovingExpenses, transaction, transaction?.taxCode ?? defaultTaxCode) : ''); | ||||||||||||
|
Comment on lines
+508
to
+510
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @parasharrajat no, the condition is ok - first we want to check if we get the taxValue for the chosen policy, otherwise we would always get the taxValue for policyForMovingExpenses. If there's no tax value we fallback to this another condition
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've added a comment to make it clear |
||||||||||||
| const taxAmount = calculateTaxAmount(taxPercentage, taxableAmount, transaction?.currency ?? CONST.CURRENCY.USD); | ||||||||||||
| const taxAmountInSmallestCurrencyUnits = convertToBackendAmount(Number.parseFloat(taxAmount.toString())); | ||||||||||||
| useEffect(() => { | ||||||||||||
| if (!transactionID || isReadOnly) { | ||||||||||||
| if (!transactionID || isMovingTransactionFromTrackExpense || isReadOnly) { | ||||||||||||
| return; | ||||||||||||
| } | ||||||||||||
| setMoneyRequestTaxAmount(transactionID, taxAmountInSmallestCurrencyUnits); | ||||||||||||
| }, [transactionID, taxAmountInSmallestCurrencyUnits, isReadOnly]); | ||||||||||||
| }, [transactionID, taxAmountInSmallestCurrencyUnits, isMovingTransactionFromTrackExpense, isReadOnly]); | ||||||||||||
|
|
||||||||||||
| // If completing a split expense fails, set didConfirm to false to allow the user to edit the fields again | ||||||||||||
| if (isEditingSplitBill && didConfirm) { | ||||||||||||
|
|
@@ -835,7 +840,7 @@ function MoneyRequestConfirmationList({ | |||||||||||
| if (!transactionID || iouCategory || !shouldShowCategories || enabledCategories.length !== 1 || !isCategoryRequired) { | ||||||||||||
| return; | ||||||||||||
| } | ||||||||||||
| setMoneyRequestCategory(transactionID, enabledCategories.at(0)?.name ?? '', policy?.id); | ||||||||||||
| setMoneyRequestCategory(transactionID, enabledCategories.at(0)?.name ?? '', policy?.id, isMovingTransactionFromTrackExpense); | ||||||||||||
| // Keep 'transaction' out to ensure that we auto select the option only once | ||||||||||||
| // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps | ||||||||||||
| }, [shouldShowCategories, policyCategories, isCategoryRequired, policy?.id]); | ||||||||||||
|
|
@@ -914,6 +919,11 @@ function MoneyRequestConfirmationList({ | |||||||||||
| return; | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| if (shouldShowTax && !Object.keys(policy?.taxRates?.taxes ?? {}).some((key) => key === transaction.taxCode)) { | ||||||||||||
| setFormError('violations.taxOutOfPolicy'); | ||||||||||||
| return; | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| if (isPerDiemRequest && (transaction.comment?.customUnit?.subRates ?? []).length === 0) { | ||||||||||||
| setFormError('iou.error.invalidSubrateLength'); | ||||||||||||
| return; | ||||||||||||
|
|
@@ -971,6 +981,7 @@ function MoneyRequestConfirmationList({ | |||||||||||
| isMerchantRequired, | ||||||||||||
| isMerchantEmpty, | ||||||||||||
| shouldDisplayFieldError, | ||||||||||||
| shouldShowTax, | ||||||||||||
| transaction, | ||||||||||||
| iouCategory.length, | ||||||||||||
| policyTags, | ||||||||||||
|
|
||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| import type {OnyxEntry} from 'react-native-onyx'; | ||
| import {isExpenseUnreported} from '@libs/TransactionUtils'; | ||
| import CONST from '@src/CONST'; | ||
| import ONYXKEYS from '@src/ONYXKEYS'; | ||
| import type {Policy, Report, Transaction} from '@src/types/onyx'; | ||
| import useOnyx from './useOnyx'; | ||
| import usePolicyForMovingExpenses from './usePolicyForMovingExpenses'; | ||
|
|
||
| type UsePolicyForTransactionParams = { | ||
| /** The transaction to determine the policy for */ | ||
| transaction: OnyxEntry<Transaction>; | ||
|
|
||
| /** The report associated with the transaction */ | ||
| report: OnyxEntry<Report>; | ||
|
|
||
| /** The current action being performed */ | ||
| action: string; | ||
|
|
||
| /** The type of IOU (split, track, submit, etc.) */ | ||
| iouType: string; | ||
| }; | ||
|
|
||
| type UsePolicyForTransactionResult = { | ||
| /** The policy to use for the transaction */ | ||
| policy: OnyxEntry<Policy>; | ||
| }; | ||
|
|
||
| function usePolicyForTransaction({transaction, report, action, iouType}: UsePolicyForTransactionParams): UsePolicyForTransactionResult { | ||
| const {policyForMovingExpenses} = usePolicyForMovingExpenses(); | ||
| const isUnreportedExpense = isExpenseUnreported(transaction); | ||
| const isCreatingTrackExpense = action === CONST.IOU.ACTION.CREATE && iouType === CONST.IOU.TYPE.TRACK; | ||
|
|
||
| const [reportPolicy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${report?.policyID}`, {canBeMissing: true}); | ||
| const policy = isUnreportedExpense || isCreatingTrackExpense ? policyForMovingExpenses : reportPolicy; | ||
|
|
||
| return {policy}; | ||
| } | ||
|
|
||
| export default usePolicyForTransaction; |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did we add policyID here? This looks like a side-effect and not an obvious reason for the policyID to be present in this hook. IDE auto-fix will remove this dependency if we try that option. Can you please add a comment here to explain why we need this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because if you have the same rates for 2 policies defaultTaxCode can be the same and the useEffect won't run. That's why we had this bug with tax rate title not displaying.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a comment for the same so others don't remove it.