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.

Sign up for GitHub

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

Jump to bottom

Handle tax for split requests #40240

Merged
merged 19 commits into from
May 24, 2024
Merged

Handle tax for split requests #40240

Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
c611187
Pass taxCode and taxAmount params for split bill request
MonilBhavsar Apr 15, 2024
c7ec5f2
Merge branch 'main' into monil-splitBillTaxUpdate
MonilBhavsar Apr 17, 2024
8cb3a49
Merge branch 'main' into monil-splitBillTaxUpdate
MonilBhavsar May 13, 2024
bb6d480
Merge branch 'monil-splitBillTaxUpdate' of github.com:Expensify/App i…
MonilBhavsar May 13, 2024
98e0e40
Merge branch 'main' into monil-splitBillTaxUpdate
MonilBhavsar May 15, 2024
477b381
Split amount based on new logic and update optimistic transaction
MonilBhavsar May 15, 2024
ec48476
Fix lint
MonilBhavsar May 15, 2024
bd20eed
Merge branch 'main' into monil-splitBillTaxUpdate
MonilBhavsar May 16, 2024
de50156
Fix bug when selectedReportID and hence policy is undefined on confir…
MonilBhavsar May 16, 2024
c3db0c8
Merge branch 'main' into monil-splitBillTaxUpdate
MonilBhavsar May 17, 2024
c667482
Fix taxAmount not being calculated
MonilBhavsar May 21, 2024
cdfb3ad
Handle editing split bill case
MonilBhavsar May 21, 2024
78bf98a
Handle scan split requests optimistically
MonilBhavsar May 21, 2024
776856c
Fix lint
MonilBhavsar May 21, 2024
6dd21cb
Check for ownerAccountID in case of split with policy
MonilBhavsar May 22, 2024
c7d8b42
Merge branch 'main' into monil-splitBillTaxUpdate
MonilBhavsar May 23, 2024
787c520
Fix taxAmount is negative optimistically
MonilBhavsar May 23, 2024
d136b4a
Fix lint and typecheck
MonilBhavsar May 23, 2024
c671c7a
Fix bug when updating taxAmount and refactor
MonilBhavsar May 23, 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
11 changes: 3 additions & 8 deletions src/components/MoneyRequestConfirmationList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,8 +315,8 @@
const formattedTaxAmount = CurrencyUtils.convertToDisplayString(transaction?.taxAmount, iouCurrencyCode);
const taxRateTitle = TransactionUtils.getTaxName(policy, transaction);

const previousTransactionAmount = usePrevious(transaction?.amount);

Check failure on line 318 in src/components/MoneyRequestConfirmationList.tsx

View workflow job for this annotation

GitHub Actions / Run ESLint 

'previousTransactionAmount' is assigned a value but never used
const previousTransactionCurrency = usePrevious(transaction?.currency);

Check failure on line 319 in src/components/MoneyRequestConfirmationList.tsx

View workflow job for this annotation

GitHub Actions / Run ESLint 

'previousTransactionCurrency' is assigned a value but never used

const isFocused = useIsFocused();
const [formError, debouncedFormError, setFormError] = useDebouncedState('');
Expand Down Expand Up @@ -372,14 +372,9 @@
// Calculate and set tax amount in transaction draft
useEffect(() => {
const taxAmount = getTaxAmount(transaction, policy).toString();
const amountInSmallestCurrencyUnits = CurrencyUtils.convertToBackendAmount(Number.parseFloat(taxAmount));

if (transaction?.taxAmount && previousTransactionAmount === transaction?.amount && previousTransactionCurrency === transaction?.currency) {
return IOU.setMoneyRequestTaxAmount(transactionID, transaction?.taxAmount ?? 0, true);
}
Comment on lines -377 to -379
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 mostly think this was pre optimization. When the user updates transactionAmount, previousTransactionAmount was not being changed, and we we were not setting updated taxAmount in Onyx

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, this introduced a bug where taxAmount is not being updated in draft transaction. looking...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is fixed now with latest commit - c671c7a


IOU.setMoneyRequestTaxAmount(transactionID, amountInSmallestCurrencyUnits, true);
}, [policy, transaction, transactionID, previousTransactionAmount, previousTransactionCurrency]);
const taxAmountInSmallestCurrencyUnits = CurrencyUtils.convertToBackendAmount(Number.parseFloat(taxAmount));
IOU.setMoneyRequestTaxAmount(transactionID, taxAmountInSmallestCurrencyUnits, true);
}, [policy, transaction, transactionID]);

// If completing a split expense fails, set didConfirm to false to allow the user to edit the fields again
if (isEditingSplitBill && didConfirm) {
Expand Down
Loading