Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.34k stars 2.77k forks source link

[$250] Tax - Split scan - Selected rate from confirmation page does not show up in split detail view #42643

Closed lanitochka17 closed 3 months ago

lanitochka17 commented 3 months ago

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Version Number: 1.4.76-0 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: N/A Issue reported by: Applause - Internal Team

Issue found when executing PR https://github.com/Expensify/App/pull/40240

Action Performed:

Precondition:

Expected Result:

The selected tax rate will show up in Tax rate row in split details view

Actual Result:

The selected tax rate does not show up in Tax rate row in split details view. The row is empty

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/78819774/b133585f-2336-49b7-9782-db0ed700653e

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~016b6987c10090fc2e
  • Upwork Job ID: 1795297947813523456
  • Last Price Increase: 2024-05-28
Issue OwnerCurrent Issue Owner: @DylanDylann
melvin-bot[bot] commented 3 months ago

Triggered auto assignment to @kadiealexander (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

lanitochka17 commented 3 months ago

@kadiealexander FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

lanitochka17 commented 3 months ago

We think that this bug might be related to #vip-split

dragnoir commented 3 months ago

Proposal

Please re-state the problem that we are trying to solve in this issue.

Split scan - Selected rate from confirmation page does not show up in split detail view

What is the root cause of that problem?

On a Workspace, when we split an expense, we go throw this function

https://github.com/Expensify/App/blob/9a87ce919f32b0a32023e01747cd6d5992c20d8e/src/pages/iou/request/step/IOURequestStepConfirmation.tsx#L344-L355

We are not passing the Tax detials like rate and amout to startSplitBill

What changes do you think we should make in order to solve the problem?

as we do for IOU.splitBill

https://github.com/Expensify/App/blob/9a87ce919f32b0a32023e01747cd6d5992c20d8e/src/pages/iou/request/step/IOURequestStepConfirmation.tsx#L380-L381

We need to pass taxCode and taxAmount to IOU.startSplitBill

then use it inside the TransactionUtils.buildOptimisticTransaction

https://github.com/Expensify/App/blob/9a87ce919f32b0a32023e01747cd6d5992c20d8e/src/libs/actions/IOU.ts#L4315-L4332

POC Video:

https://github.com/Expensify/App/assets/12425932/6b2ddc6e-b5dd-43ec-b8a6-a0b1ff7215e0

What alternative solutions did you explore?

bondydaa commented 3 months ago

the revert https://github.com/Expensify/App/pull/42670 has been deployed, please retest and confirm this is no longer reproducible.

kavimuru commented 3 months ago

Issue is not fixed.

https://github.com/Expensify/App/assets/43996225/d39549af-782f-4f01-9309-1c226d56d8f2

melvin-bot[bot] commented 3 months ago

Job added to Upwork: https://www.upwork.com/jobs/~016b6987c10090fc2e

melvin-bot[bot] commented 3 months ago

Triggered auto assignment to Contributor-plus team member for initial proposal review - @DylanDylann (External)

cretadn22 commented 3 months ago

Proposal

Please re-state the problem that we are trying to solve in this issue.

Split scan - Selected rate from confirmation page does not show up in split detail view

What is the root cause of that problem?

When we split expense by scanning we invoke startSplitBill function, but we don't pass taxRate and taxAmount to this function.

https://github.com/Expensify/App/blob/9a87ce919f32b0a32023e01747cd6d5992c20d8e/src/libs/actions/IOU.ts#L4559

One more thing, when adjusting the tax rate and amount, we invoke the completeSplitBill function, but we also don't pass taxRate and taxAmount

https://github.com/Expensify/App/blob/95d328fb90c2bca9b57255ed645dcb3f363d64e5/src/libs/actions/IOU.ts#L4800

What changes do you think we should make in order to solve the problem?

We need to update the optimisticData and params within the completeSplitBill and startSplitBill function to incorporate taxRate and taxAmount.

What alternative solutions did you explore? (Optional)

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

nkdengineer commented 3 months ago

Proposal

Please re-state the problem that we are trying to solve in this issue.

The selected tax rate does not show up in Tax rate row in split details view. The row is empty

What is the root cause of that problem?

We have two bug here

  1. When we start split bill here, we don't accept taxCode and taxAmount param in this function to create optimistic transaction and pass it to param of StartSplitBill API

https://github.com/Expensify/App/blob/2514f2988eb26858d30fa501c0040b1f8fc2e239/src/pages/iou/request/step/IOURequestStepConfirmation.tsx#L342

  1. When we edit tax rate and tax amount, we're calling UpdateMoneyRequestTaxRate API because we don't have the case for edit split bill here

https://github.com/Expensify/App/blob/2514f2988eb26858d30fa501c0040b1f8fc2e239/src/pages/iou/request/step/IOURequestStepTaxRatePage.tsx#L76

What changes do you think we should make in order to solve the problem?

  1. Accept taxCode, taxAmount param in startSplitBill function and then use this to create optimistic transaction here.

  2. Pas taxCode as transactionTaxCode and taxAmount as transactionTaxAmount into startSplitBill function here

https://github.com/Expensify/App/blob/2514f2988eb26858d30fa501c0040b1f8fc2e239/src/pages/iou/request/step/IOURequestStepConfirmation.tsx#L342

  1. Pass taxCode and taxAmount as param of StartSplitBill API here. We also need BE change to return taxCode and taxAmount of the transaction when we call StartSplitBill API

https://github.com/Expensify/App/blob/2514f2988eb26858d30fa501c0040b1f8fc2e239/src/libs/actions/IOU.ts#L4540

  1. When we edit split bill tax rate, we should call setDraftSplitTransaction as the revert PR here https://github.com/Expensify/App/pull/42670/files#diff-2d4b9c38f6fb668f527da301a60d3c9bb05c865f33aa475d29148bbda45e3cc0

https://github.com/Expensify/App/blob/2514f2988eb26858d30fa501c0040b1f8fc2e239/src/pages/iou/request/step/IOURequestStepTaxRatePage.tsx#L76

What alternative solutions did you explore? (Optional)

NA

DylanDylann commented 3 months ago

This should be hold https://github.com/Expensify/App/issues/39690

cc @MonilBhavsar

MonilBhavsar commented 3 months ago

We can close this. This is new feature and not a bug