vat icon indicating copy to clipboard operation
vat copied to clipboard

Allow to chose behavior if country is not supported

Open VincentLanglet opened this issue 1 year ago • 6 comments

Closes https://github.com/ibericode/vat/issues/40

cc @gemal @dannyvankooten

The option doesn't make sens for validateVatNumber, since the validateVatNumberExistence only work for EU country since the check is done on https://ec.europa.eu/taxation_customs/vies/#/vat-validation

VincentLanglet avatar Nov 22 '23 12:11 VincentLanglet

@VincentLanglet Personally I am fine throwing a something like an InvalidArgumentException or LogicException (after a major version bump because of the BC break) in case the method is called with a country not supported by this library. That would help keep the API a bit cleaner. What are your thoughts on that?

dannyvankooten avatar Nov 23 '23 19:11 dannyvankooten

@VincentLanglet Personally I am fine throwing a something like an InvalidArgumentException or LogicException (after a major version bump because of the BC break) in case the method is called with a country not supported by this library. That would help keep the API a bit cleaner. What are your thoughts on that?

Currently the VAT validator check mainly EU countries VAT but

  • San Morino is not in the EU https://github.com/ibericode/vat/blob/master/src/Validator.php#L45
  • GB is not in the EU anymore https://github.com/ibericode/vat/blob/master/src/Validator.php#L29 and XI/XU prefix should be supported
  • People are looking for more VAT support (like https://github.com/ibericode/vat/pull/57 or https://github.com/ibericode/vat/issues/40#issue-994527318 which use it for Norway)

IMHO, in the futur this lib should support all possible VAT prefix and have the related regex. One issue we currently have with the Validator is the fact that we don't know if the error is coming from an invalid pattern FR12 (for instance), an invalid country XX or a not yet supported country RU (for instance).

Instead it is better for the API to have

if (! isset($this->patterns[$country])) {
     throw new \InvalidArgumentException('The country prefix is invalid or not implemented yet');
}

but this still require to expose the hasSupportedCountryPrefix method.

We can also throw a custom VatException (?)

VincentLanglet avatar Nov 23 '23 19:11 VincentLanglet

Currently the VAT validator check mainly EU countries VAT but

IMHO, in the futur this lib should support all possible VAT prefix and have the related regex. One issue we currently have with the Validator is the fact that we don't know if the error is coming from an invalid pattern FR12 (for instance), an invalid country XX or a not yet supported country RU (for instance).

Instead it is better for the API to have

if (! isset($this->patterns[$country])) {
     throw new \InvalidArgumentException('The country prefix is invalid or not implemented yet');
}

but this still require to expose the hasSupportedCountryPrefix method.

We can also throw a custom VatException (?)

WDYT @dannyvankooten

VincentLanglet avatar Jan 15 '24 19:01 VincentLanglet

Hi @VincentLanglet,

I'd like to go with throwing an InvalidArgumentException() when any of the validateVatNumber* methods are called with an unsupported country prefix. I'm fine with having an additional method to check for support before this method is called, but alternatively people could also catch the exception to accomplish the same.

Since we're not in need of this functionality ourselves and I am starting a new job soon, I don't have any time to work on this for the next few months though.

dannyvankooten avatar Jan 29 '24 11:01 dannyvankooten

Hi @VincentLanglet,

I'd like to go with throwing an InvalidArgumentException() when any of the validateVatNumber* methods are called with an unsupported country prefix. I'm fine with having an additional method to check for support before this method is called, but alternatively people could also catch the exception to accomplish the same.

Hi, I changed to the throw exception behavior.

VincentLanglet avatar Jan 29 '24 13:01 VincentLanglet

HI @dannyvankooten ; do you have some time to review/merge this ? Thanks.

VincentLanglet avatar Jun 17 '24 18:06 VincentLanglet