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.

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

[Payment Due 08/23][$250] [Track Tax] Add the ability to configure tax rates on distance rates in NewDot #41574

Closed
trjExpensify opened this issue May 3, 2024 · 76 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item. Weekly KSv2

Comments

@trjExpensify
Copy link
Contributor

trjExpensify commented May 3, 2024

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

image

  • 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.

image

  • 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).

image

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

image

  • 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:

image

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-08-08
  • Automatic offers:
    • nkdengineer | Contributor | 0
Issue OwnerCurrent Issue Owner: @trjExpensify
@trjExpensify trjExpensify added Daily KSv2 NewFeature Something to build that is a new item. labels May 3, 2024
Copy link

melvin-bot bot commented May 3, 2024

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

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels May 3, 2024
@shawnborton
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor Author

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 MonilBhavsar added Daily KSv2 and removed Weekly KSv2 labels May 10, 2024
@MonilBhavsar
Copy link
Contributor

Looking into this

@MonilBhavsar
Copy link
Contributor

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

For API's,

  1. 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}}
  1. 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":[]}

@MonilBhavsar MonilBhavsar added the External Added to denote the issue can be worked on by a contributor label May 10, 2024
@melvin-bot melvin-bot bot changed the title [Track Tax] Add the ability to configure tax rates on distance rates in NewDot [$250] [Track Tax] Add the ability to configure tax rates on distance rates in NewDot May 10, 2024
Copy link

melvin-bot bot commented May 10, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label May 10, 2024
Copy link

melvin-bot bot commented May 10, 2024

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

@nkdengineer
Copy link
Contributor

nkdengineer commented May 10, 2024

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
Copy link
Contributor

Okay cool! Let's get started then 👍

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label May 10, 2024
Copy link

melvin-bot bot commented May 10, 2024

📣 @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
Copy link
Contributor

@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
Copy link
Contributor

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

@nkdengineer
Copy link
Contributor

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

@melvin-bot melvin-bot bot added the Overdue label May 13, 2024
@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Aug 2, 2024
Copy link

melvin-bot bot commented Aug 5, 2024

@eVoloshchak, @trjExpensify, @MonilBhavsar, @nkdengineer Whoops! This issue is 2 days overdue. Let's get this updated quick!

@trjExpensify
Copy link
Contributor Author

@nkdengineer @MonilBhavsar @eVoloshchak can we get an update please?

@melvin-bot melvin-bot bot removed the Overdue label Aug 5, 2024
@nkdengineer
Copy link
Contributor

PR is still reviewing, need to fix some edge case bugs.

Copy link

melvin-bot bot commented Aug 8, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@melvin-bot melvin-bot bot added the Overdue label Aug 8, 2024
@nkdengineer
Copy link
Contributor

PR is reviewing and just confirmed the behavior. Will investigate the update tomorrow.

@trjExpensify
Copy link
Contributor Author

Any update?

@melvin-bot melvin-bot bot removed the Overdue label Aug 9, 2024
@nkdengineer
Copy link
Contributor

@trjExpensify i updated this PR, waiting C+ review

@trjExpensify
Copy link
Contributor Author

Great, let's get it done!

@melvin-bot melvin-bot bot added the Overdue label Aug 12, 2024
@trjExpensify
Copy link
Contributor Author

All yours for the secondary, @MonilBhavsar!

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Aug 12, 2024
@MonilBhavsar
Copy link
Contributor

PR was merged!

Copy link

melvin-bot bot commented Aug 16, 2024

@eVoloshchak, @trjExpensify, @MonilBhavsar, @nkdengineer Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

Copy link

melvin-bot bot commented Aug 20, 2024

@eVoloshchak, @trjExpensify, @MonilBhavsar, @nkdengineer Still overdue 6 days?! Let's take care of this!

@MonilBhavsar
Copy link
Contributor

PR was deployed to production 5 days ago. We can process the payment on payment on Monday once tom is back

@MonilBhavsar MonilBhavsar added the Awaiting Payment Auto-added when associated PR is deployed to production label Aug 21, 2024
@MonilBhavsar MonilBhavsar changed the title [$250] [Track Tax] Add the ability to configure tax rates on distance rates in NewDot [Payment Due 08/23][$250] [Track Tax] Add the ability to configure tax rates on distance rates in NewDot Aug 21, 2024
Copy link

melvin-bot bot commented Aug 22, 2024

@eVoloshchak, @trjExpensify, @MonilBhavsar, @nkdengineer Now this issue is 8 days overdue. Are you sure this should be a Daily? Feel free to change it!

@MonilBhavsar MonilBhavsar added Weekly KSv2 and removed Daily KSv2 labels Aug 22, 2024
@melvin-bot melvin-bot bot removed the Overdue label Aug 22, 2024
@trjExpensify
Copy link
Contributor Author

Confirming payment summary as follows:

@nkdengineer paid, @eVoloshchak go ahead and request!

@JmillsExpensify
Copy link

$250 approved for @eVoloshchak

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item. Weekly KSv2
Projects
No open projects
Status: Done
Development

No branches or pull requests

8 participants