fix(rate): hst includes gst

valeriansaliou / node-sales-tax

:moneybag: International sales tax calculator for Node (offline, but provides optional online VAT number fraud check). Tax rates are kept up-to-date.

https://www.npmjs.com/package/sales-tax

MIT License

299 stars 45 forks source link

fix(rate): hst includes gst #34

Closed tomkcey closed 3 years ago

tomkcey commented 3 years ago

Linked to issue #33

Added logical statements to forego taxes other than hst tax when that one is present in the list for a given province of Canada.
Removed tests that had become unecessary, made changes to others to reflect new reality and also fixed a failing test.

valeriansaliou commented 3 years ago

Can't we just change it to GST+PST? As far as I understand this, there is still a province + a federal tax, which is called "HST" but which can still be split?

tomkcey commented 3 years ago

Can't we just change it to GST+PST? As far as I understand this, there is still a province + a federal tax, which is called "HST" but which can still be split?

Well, it can't be split, since it's harmonized. For display purposes, it still can't be split from what I understand.

If you really want to split it, I understand, but we'd have to add a boolean field or somesuch to the json so we could know if the province we're iterating on is under the harmonized structure or not. Do you find that solution acceptable?

blemoine commented 3 years ago

Hi, just to explain the problem: in Canada, Provincse can have either

2 taxes: GST + (PST or QST) - PST may be 0% (Alberta for example).
only 1 tax ( HST )

Ideally for Ontario (for example) we would see only 1 tax: HST with a value of 13%. Worst case scenario - if it's easier - it could a GST (0%) + HST(13%) , but for legal reason, HST needs to be displayed with the total rate.

(And BTW : thanks for the hardwork on this library!)

valeriansaliou commented 3 years ago

A regression in Canadian tax rates was introduced 1 week ago in the library due to PR #32

I've commented on https://github.com/valeriansaliou/node-sales-tax/pull/32#issuecomment-717554565 w/ a "post-mortem" and a fix.

Can you tell me if that fits all of you, Canadian users?

To me, it looks like you could get the actual HST (to comply w/ your legal requirements on invoices) given the tax type returned (gst+hst), which you could parse on your end as "HST" and extract the final summed tax rate.

My comment explains the behavior of node-sales-tax regarding its special treatment of HST. This trick allows me to avoid Canada-specific logic in the main source code, which could be bug-prone in the future.

tomkcey commented 3 years ago

Considering the top-level rate property should now reflect the right final rate, I can close this PR. If I need anything else or have further concerns I'll go through the same process I just did (opening an issue first) if that's ok with you, but I should be fine.