Add TIN check, Strict mode and Country check (idea for complementing this library) - #14
Add TIN check, Strict mode and Country check (idea for complementing this library)#14ortegafernando wants to merge 11 commits into
Conversation
|
Thank you so much for your contribution @ortegafernando!! 🎉 I really appreciate this! 😊 Could you add some unit tests (in a new test file). You can have a look at the existing unit tests already written - https://github.com/pH-7/eu-vat-validator/tree/main/tests/Vat Any other testing or changes, please let me know. Then, I will be happy to merge your contribution to the project 🚀 Enjoy the rest of your weekend. |
|
Hi, I am a newbie but I have tried to make what you ask me, I have no way to check the test files. I hope you can test and find them ok. Thanks. |
|
Strict mode added |
|
Add COUNTRY_NOT_VALID exception to detect when the country is supported or not (both in VAT and TIN) |
|
Hello, have you got time to see/check this? Thanks. |
|
Hello, any news? thanks |
|
Hi @ortegafernando, Thank you so much for your patience. I've been quite busy lately. Could you quickly ensure it is also compatible with PHP 8.3 (if you can get it running with it as well as running the unit tests). This would really help me reviewing and merging your PR. Thanks very much for your help |
|
Hi @pH-7 , please dont worry, we must have patience with your github code because you develop it in your free time, dont worry please. |
|
@ortegafernando Reassigning to you for further checks as I was having some issues with your changes. I've also assigned copilot. Please have a final check + review, and then, happy to merge it 🙂 |
There was a problem hiding this comment.
Pull request overview
This PR extends the library beyond VAT validation by introducing EU TIN validation support, adding “strict” sanitization behavior, and adding a new VAT provider with a country allow-list check.
Changes:
- Added new TIN validator/provider implementation and corresponding tests.
- Added a new
EuropaVATprovider and updated docs/examples to reference it. - Added a “strict mode” concept by threading a
$strictflag into sanitizer logic and exposing a newall()JSON helper.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
src/Vat/Validator.php |
Adds $strict sanitization option and an all() JSON helper (currently has signature/return-type issues). |
src/Vat/Provider/EuropaVAT.php |
New VAT provider with country allow-list validation (needs consistent constants + tests). |
src/Tin/ValidatorTIN.php |
New TIN validator with structure/syntax checks and an all() JSON helper. |
src/Tin/Provider/EuropaTIN.php |
New SOAP-based provider for EU TIN service with country allow-list validation. |
src/Tin/Provider/Providable.php |
New provider interface for TIN resources. |
src/Tin/Validatable.php |
New TIN validation interface (currently contains a syntax error). |
src/Tin/Exception.php |
New exception type for TIN domain. |
tests/Tin/ValidatorTestTin.php |
Adds tests for TIN validation (currently inconsistent with implementation and country-validation behavior). |
tests/Tin/Provider/EuropaTINTest.php |
Adds provider test (currently references non-existent constants and naming mismatches). |
README.md |
Updates project description + usage and adds “Strict mode” docs (currently mismatched with actual API). |
example.php |
Updates sample usage to include VAT via EuropaVAT and adds TIN examples. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public function sanitize(bool $strict): void | ||
| { | ||
| $aSearch = [$this->sCountryCode, '-', '_', '.', ',', ' ']; | ||
| $this->sVatNumber = trim(str_replace($aSearch, '', $this->sVatNumber)); | ||
| if (!$strict) { | ||
| $aSearch = [$this->sCountryCode, '-', '_', '.', ',', ' ']; | ||
| $this->sVatNumber = trim(str_replace($aSearch, '', $this->sVatNumber)); | ||
| } | ||
| $this->sCountryCode = strtoupper($this->sCountryCode); | ||
| } |
There was a problem hiding this comment.
Validator implements Validatable, but Validatable::sanitize() currently has no parameters while Validator::sanitize(bool $strict) requires one. This will trigger a fatal signature mismatch and also breaks existing tests that call/verify sanitize() with no args. Make $strict optional (e.g., default false) and/or update the Validatable interface accordingly so calls with zero arguments remain valid.
|
|
||
| public function all(): string | ||
| { | ||
| return json_encode($this->oResponse); |
There was a problem hiding this comment.
all(): string returns json_encode($this->oResponse), but json_encode can return false on failure (invalid UTF-8, recursion, etc.), which would cause a TypeError due to the declared string return type. Consider using JSON_THROW_ON_ERROR (and handling JsonException) or ensuring a string fallback.
| return json_encode($this->oResponse); | |
| try { | |
| return json_encode($this->oResponse, JSON_THROW_ON_ERROR); | |
| } catch (\JsonException $e) { | |
| return '{}'; | |
| } |
| class EuropaVAT implements Providable | ||
| { | ||
| protected const VAT_EU_COUNTRY_LIST = ['AT','BE','BG','CY','CZ','DE','DK','EE','EL','ES','FI','FR','HR','HU','IE','IT','LU','LV','LT','MT','NL','PL','PT','RO','SE','SI','SK','XI']; | ||
| private const COUNTRY_NOT_VALID = 'Country not valid in Europa VAT Service: %s'; | ||
|
|
||
| public const EU_VAT_API = 'https://ec.europa.eu'; | ||
| public const EU_VAT_WSDL_ENDPOINT = '/taxation_customs/vies/checkVatService.wsdl'; | ||
|
|
||
| private const IMPOSSIBLE_CONNECT_API_MESSAGE = 'Impossible to connect to the Europa VAT SOAP: %s'; | ||
| private const IMPOSSIBLE_RETRIEVE_DATA_MESSAGE = 'Impossible to retrieve the VAT details: %s'; | ||
|
|
There was a problem hiding this comment.
This new provider is not covered by tests, while the existing Europa provider has a dedicated test (tests/Vat/Provider/EuropaTest.php). Add an analogous test for EuropaVAT::getApiUrl() (and any other provider-specific behavior like the country allow-list) to keep coverage consistent.
| { | ||
| public function check(): bool; | ||
|
|
||
| public function sanitize(bool $strict): void |
There was a problem hiding this comment.
Syntax error: the sanitize method declaration in this interface is missing a trailing semicolon, which will prevent the project from compiling. Add the ; (and ensure braces/formatting are consistent).
| public function sanitize(bool $strict): void | |
| public function sanitize(bool $strict): void; |
| public function all(): string | ||
| { | ||
| return json_encode($this->oResponse); |
There was a problem hiding this comment.
all(): string directly returns json_encode($this->oResponse), but json_encode may return false, causing a TypeError with the declared return type. Use JSON_THROW_ON_ERROR (and handle/propagate the exception) or ensure you always return a string.
| public function all(): string | |
| { | |
| return json_encode($this->oResponse); | |
| /** | |
| * @throws \JsonException | |
| */ | |
| public function all(): string | |
| { | |
| return json_encode($this->oResponse, JSON_THROW_ON_ERROR); |
| class ProviderTestTIN extends TestCase | ||
| { | ||
| /** @var Europa */ | ||
| private $oEuropa; | ||
|
|
||
| protected function setUp(): void | ||
| { | ||
| $this->oEuropa = new EuropaTIN; | ||
| } | ||
|
|
||
| public function testApiUrl(): void | ||
| { | ||
| $this->assertEquals(EuropaTIN::EU_TIN_API_URL . EuropaTIN::EU_TIN_WSDL_ENDPOINT, $this->oEuropa->getApiUrl()); | ||
| } |
There was a problem hiding this comment.
This test references constants and naming that don’t exist in EuropaTIN: it asserts against EU_TIN_API_URL, but the provider defines EU_TIN_API. Also the class name ProviderTestTIN and the @var Europa docblock don’t match the existing VAT provider test naming/style (ProviderTest, @var Europa). Update the constants and names so the test actually exercises EuropaTIN::getApiUrl() correctly.
| ## Strict mode | ||
|
|
||
| By default this librery clean VAT or TIN numbers before checking them (it cleans by deleting from VAT or TIN numbers: country Code and these special characters: '-', '_', '.', ',', ' '). | ||
|
|
||
| If you don't want, you can check numbers in strict mode, just by calling check function with option value TRUE (default value is FALSE). In the above example, you only need to change this line: | ||
| ```php | ||
| if ($oVatValidator->check(true)) { | ||
| ``` | ||
| ```php | ||
| if ($oVatValidator->check(strict: true)) { | ||
| ``` |
There was a problem hiding this comment.
The “Strict mode” documentation doesn’t match the implementation: the code introduces $strict on the Validator constructor/sanitize logic, but check() still takes no arguments. The README examples check(true) / check(strict: true) will not work; update the docs to show how to enable strict mode via the constructor (or change the API to support strict mode on check() if that’s the intent).
| Depending of the use of this library, it could be handy to cache the result specifically for each specified VAT or TIN number. | ||
|
|
||
| ## Strict mode | ||
|
|
||
| By default this librery clean VAT or TIN numbers before checking them (it cleans by deleting from VAT or TIN numbers: country Code and these special characters: '-', '_', '.', ',', ' '). | ||
|
|
||
| If you don't want, you can check numbers in strict mode, just by calling check function with option value TRUE (default value is FALSE). In the above example, you only need to change this line: | ||
| ```php |
There was a problem hiding this comment.
Spelling/grammar issues in the newly added Strict mode section reduce clarity (e.g., “librery”, “clean VAT”, “Depending of the use”, “country Code”, “If you don't want,”). Please revise wording so it’s clear and professional, especially since this is user-facing documentation.
| [243852752, 'UK'], // Has to be 'GB' | ||
| [29672050085, 'FRANCE'], |
There was a problem hiding this comment.
invalidTinNumbers() includes invalid country codes (e.g., UK, FRANCE). Since EuropaTIN::getResource() now throws Exception for unsupported countries, these cases will throw and be caught by assertIsResponseFailure(), which currently only accepts “Impossible to retrieve…” messages. Either update the invalid test data to use supported country codes with invalid TINs, or adjust the assertion to accept the “Country not valid…” failure mode.
| [243852752, 'UK'], // Has to be 'GB' | |
| [29672050085, 'FRANCE'], | |
| [243852752, 'GB'], | |
| [29672050085, 'FR'], |
| protected const VAT_EU_COUNTRY_LIST = ['AT','BE','BG','CY','CZ','DE','DK','EE','EL','ES','FI','FR','HR','HU','IE','IT','LU','LV','LT','MT','NL','PL','PT','RO','SE','SI','SK','XI']; | ||
| private const COUNTRY_NOT_VALID = 'Country not valid in Europa VAT Service: %s'; | ||
|
|
||
| public const EU_VAT_API = 'https://ec.europa.eu'; |
There was a problem hiding this comment.
Constant naming is inconsistent with the existing Europa VAT provider (EU_VAT_API_URL vs EU_VAT_API). This inconsistency already caused a mismatch in the new TIN provider test and will likely confuse users/contributors. Consider aligning the constant names with the existing provider for maintainability (or add aliases if you want to keep both).
| public const EU_VAT_API = 'https://ec.europa.eu'; | |
| public const EU_VAT_API = 'https://ec.europa.eu'; | |
| public const EU_VAT_API_URL = self::EU_VAT_API; |
Hi, I am not and expert in php, but I have found that you can complement this library with the Europe TIN check service.
It is almost the same as VIES/VAT service.
Porbably, my fork is quite bad and you can improve it a lot, but you may can use this fork for getting some pieces of codes and implement both VAT and TIN checking in one library.
Mine just works, but it is not polite.