[$250] [Track Tax] Add the ability to configure tax rates on distance rates in NewDot

trjExpensify commented 2 weeks ago

Coming from here. Figma here.

Problem

International customers tracking tax on distance expenses are forced to use OldDot to configure their settings. This leads to them not being suitable to migrate to NewDot. In turn, prevents us from achieving full reunification for all collect workspaces.

Why is this important?

We can't migrate these workspaces otherwise. We're already building the ability to see tax rates applied to distance expenses in NewDot, so we need to ensure that's configurable in the workspace settings as well for complete feature parity.

Solution

1. Add a Track Tax toggle to the Workspace > Distance rates > Settings page

The toggle should be greyed out and locked if the Taxes feature isn't enabled.
If Taxes are enabled the Track Tax toggled it's greyed out by default, but can freely be enabled at that point.
If an admin disables Taxes on the workspace, Track tax on Distance rates should get disabled as well.

2. When the Track Tax toggle is enabled, add two push rows for Tax rate and Tax reclaimable on within the RHP page of each individual distance rate in the Distance rates table.

The two push rows should be hidden when Track Tax is disabled
When revealed, the values should be blank by default
If an admin configures a rate/reclaimable portion, and then disables Track Tax or Taxes on the workspace, the two rows should be hidden.
If an admin configures a rate/reclaimable portion > disables Track Tax > re-enables Track Tax the rate selection/reclaimable portion values should be remembered when revealed again. (Similar to how we don't blow away your categories or tags lists if you toggle Categories / Tags off/on).

3. The Tax rate push row should push to a page that uses the standard SelectList component and contains a list of all the tax rates added to the workspace (this should basically be the same list as seen on an expense).

4. The Tax reclaimable on push row should push to a page that uses the BNP to enter the reclaimable portion

There should be no currency selector on the BNP here though. The currency matches the workspace's reportOutputCurrency.
The maximum amount that can be entered is the value of the distance rate. E.g distance rate = £0.30, reclaimable portion maxLimit = £0.30. If an admin enters a value higher than that, we'll show an error message onPress of the Save confirmation button:

CC: @Expensify/design

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~012ad19bb3a1a1f8d8
Upwork Job ID: 1788904084959670272
Last Price Increase: 2024-05-10
Automatic offers:

nkdengineer | Contributor | 0

melvin-bot[bot] commented 2 weeks ago

Current assignee @trjExpensify is eligible for the NewFeature assigner, not assigning anyone new.

shawnborton commented 2 weeks ago

Looks good! Only minor comment - do we want to get rid of these right carets on the table rows? Not sure if that is already being handled or not, but if so, let's update the mocks.

shawnborton commented 2 weeks ago

I assume most people who use this feature know what "Tax reclaimable on" means? I don't quite know what it should mean, but I assume it just means you can claim tax up to a certain amount or something like that? All that's to say is, this one might benefit from some hint/explainer text.

dannymcclain commented 2 weeks ago

Looks good! Only minor comment - do we want to get rid of these right carets on the table rows? Not sure if that is already being handled or not, but if so, let's update the mocks.

Was just about to say the same thing! I think we're moving towards getting rid of the carets in all of these tables, so we might as well start here. I'm happy to jump in and update the mocks if need be.

trjExpensify commented 2 weeks ago

Looks good! Only minor comment - do we want to get rid of these right carets on the table rows? Not sure if that is already being handled or not, but if so, let's update the mocks. Was just about to say the same thing! I think we're moving towards getting rid of the carets in all of these tables, so we might as well start here. I'm happy to jump in and update the mocks if need be.

Yeah, cool. I don't think we have an issue for that yet. So we should probably spin one up!

I assume most people who use this feature know what "Tax reclaimable on" means? I don't quite know what it should mean, but I assume it just means you can claim tax up to a certain amount or something like that? All that's to say is, this one might benefit from some hint/explainer text.

Yeah, they will haha. It's the same label name as OldDot for this feature as well, I kept it consistent. But yeah, this is the portion of the distance rate amount that is eligible to have tax reclaimed on. The TL;DR

You pay back an employee a mileage rate set by HMRC which covers both fuel and wear and tear. I.e £0.45 / mile
As a business, you can only reclaim the VAT on the portion of that rate that covers fuel i.e £0.13 / mile
The reclaimable portion pertaining to fuel differs based on things like engine size, fuel type, miles travelled per annum and other bollocks set by HMRC and reviewed (I think) quarterly.
So we can't just apply X% tax rate to the whole mileage expense amount like a "normal" expense and call it a day. We have to let the business enter the amount of the rate that is eligible to reclaim tax on, and then do the calculation based on that.

MonilBhavsar commented 1 week ago

Looking into this

MonilBhavsar commented 1 week ago

Whoever works on this issue can reuse MoneyRequestAmountForm and TaxPicker components and should be fairly straightforward.

For API's,

For Track Tax toggle we're adding in bullet 1, we'll add an API command - EnableDistanceRequestTax. I'll add it, we don't need to HOLD on it

Example customUnit param -

{"customUnitName":"Distance","customUnitID":"658abfcfc1c96","attributes":{"unit":"mi","taxEnabled":true}}

For tax rate and tax reclaimable that we're adding in bullets 3. and 4., we need to call UPDATE_POLICY_DISTANCE_RATE_VALUE

Example customUnitRate param -

{"customUnitRateID":"658abfcfc2519","name":"Default Rate","currency":"USD","enabled":true,"rate":65.1,"attributes":{"taxRateExternalID":"id_TAX_RATE_1","taxClaimablePercentage":0.49155145929339483},"subRates":[]}

melvin-bot[bot] commented 1 week ago

Job added to Upwork: https://www.upwork.com/jobs/~012ad19bb3a1a1f8d8

melvin-bot[bot] commented 1 week ago

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

nkdengineer commented 1 week ago

Proposal

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

[Track Tax] Add the ability to configure tax rates on distance rates in NewDot

What is the root cause of that problem?

This is a new feature

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

We just need to follow the steps listed out in here and here. It's quite straight forward UI changes, and I think the steps are already clear to start implementing.

What alternative solutions did you explore? (Optional)

NA

MonilBhavsar commented 1 week ago

Okay cool! Let's get started then 👍

melvin-bot[bot] commented 1 week ago

📣 @nkdengineer 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Keep in mind: Code of Conduct | Contributing 📖

MonilBhavsar commented 1 week ago

@nkdengineer let us know if you have any questions and when you'll have a draft PR up. I would like to test new API EnableDistanceRequestTax against frontend changes before submitting it for review

nkdengineer commented 1 week ago

@MonilBhavsar i'll raise draft PR in 1-2 days

nkdengineer commented 5 days ago

@MonilBhavsar Please let me know: Which field in the policy to know if Track Tax is enable in bullet 1

MonilBhavsar commented 5 days ago

policy_<> onxy key and inside it customUnits > a unique custom unit ID > attributes > taxEnabled

trjExpensify commented 5 days ago

Let us know if you have any other questions at this stage, @nkdengineer! Also, don't forget to tag the design team on the PR when it's raised for this. Thanks!

nkdengineer commented 4 days ago

@MonilBhavsar @trjExpensify I have completed the UI part and have some questions as follows:

What is params of the EnableDistanceRequestTax API?
What fields are tax rate vs reclaimable on it, and what API will be called when saving those fields?

MonilBhavsar commented 4 days ago

Nice, do you have a draft PR? I think https://github.com/Expensify/App/issues/41574#issuecomment-2104496814 answers both questions! 😄

nkdengineer commented 3 days ago

@MonilBhavsar we have a draft PR here

I have a question:

Is the EnableDistanceRequestTax api active? I try using params

{
     "customUnitName": "Distance",
     "customUnitID": "97E633FEF2869",
     "attributes": {
         "taxEnabled": true,
         "unit": "mi"
     }
}

and it is returning status = 404, check the following video:

https://github.com/Expensify/App/assets/161821005/7a19d81a-ec91-4937-aeab-b781d5a2f67b

MonilBhavsar commented 3 days ago

Thanks for the draft PR!

Is the EnableDistanceRequestTax api active?

it's not. I'll test it against draft PR and let you know once it is live

MonilBhavsar commented 3 days ago

@nkdengineer I'm testing the PR The API takes customUnit as a param of type CustomUnit You can refer to parameters of API command setPolicyDistanceRatesUnit. EnableDistanceRequestTax takes same parameters

Example: {"customUnitName":"Distance","customUnitID":"658abfcfc1c96","attributes":{"unit":"mi","taxEnabled":true}}

Expensify / App