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
306 stars 48 forks source link

[Canada Sales Tax] rate property value seems higher than it should for ON, NB and NS #33

Closed tomkcey closed 4 years ago

tomkcey commented 4 years ago

Hi, I would like to use this library in my application for sales tax in Canada, but after executing the following code to check the values currently offered, I stumbled upon something I think might be a bug.

const salesTax = require("sales-tax")
const provinces = ["QC", "ON", "NB", "NS", "NT", "NU", "YK", "BC", "AB", "MB"];
(async ()=> {
    for (const province of provinces) {
        console.log(province + " has sales tax: " + salesTax.hasStateSalesTax("CA", province))
        await salesTax.getSalesTax("CA", province).then((v)=>{
            console.log(v)
        })
    }  
})()

Results in:

...
ON has sales tax: true
{
  type: 'gst+hst',
  rate: 0.18,
  area: 'worldwide',
  exchange: 'consumer',
  charge: { direct: true, reverse: false },
  details: [ { type: 'gst', rate: 0.05 }, { type: 'hst', rate: 0.13 } ]
}
NB has sales tax: true
{
  type: 'gst+hst',
  rate: 0.2,
  area: 'worldwide',
  exchange: 'consumer',
  charge: { direct: true, reverse: false },
  details: [ { type: 'gst', rate: 0.05 }, { type: 'hst', rate: 0.15 } ]
}
NS has sales tax: true
{
  type: 'gst+hst',
  rate: 0.2,
  area: 'worldwide',
  exchange: 'consumer',
  charge: { direct: true, reverse: false },
  details: [ { type: 'gst', rate: 0.05 }, { type: 'hst', rate: 0.15 } ]
}
...

In all of those, that rate would actually be the hst - gst, because the hst includes the gst already. Ontarians, for example, don't pay 18% taxes, they pay 13%.

If you agree, I could produce a PR to fix this for you, but wanted first to open up a discussion just in case.

My solution would be to remove the gst from the details when there's an hst and calculate the rate by sum of detailed rate. It's something I'm thinking about on the fly.

valeriansaliou commented 4 years ago

Thanks, I'd accept a PR for this.

tomkcey commented 4 years ago

I'm not authorized to post a PR.

remote: Permission to valeriansaliou/node-sales-tax.git denied to tomkcey.
fatal: unable to access 'https://github.com/valeriansaliou/node-sales-tax.git/': The requested URL returned error: 403

I'm on a branch I called fix/rate.

valeriansaliou commented 4 years ago

Please fork the repo and PR from there.

tomkcey commented 4 years ago

Done, thanks! Here's the PR link.

valeriansaliou commented 4 years ago

Sorry about that, but a regression was introduced by PR #32 ; it's been fixed in v2.2.3. I assume I can close this.