search

FLUX-SE / SyliusEUVatPlugin

This Sylius Plugin allow you to add European VAT Number to the addresses and manage European VAT number rule
MIT License
16 stars 8 forks source link

[BUG] CountryVatNumberValidator unexpected notice fix #24

Closed sebastianlew closed 4 years ago

sebastianlew commented 4 years ago

\Prometee\VIESClient\Util\VatNumberUtil::split can return null. \Prometee\SyliusVIESClientPlugin\Constraints\CountryVatNumberValidator throws notice then: Notice: Trying to access array offset on value of type null. In my opinion this kind of behavior isn' t correct. I just simply check for null and then build violation in validator, for me it's better way rather than get unexpected notice in this case.

Prometee commented 4 years ago

Thanks for fixing this issue, I have to add some php spec tests for this class I assume.

Could you please share the value causing this bug ? I need to add it to the behat tests (I need to fix the menu taxon missing bug into it first).

sebastianlew commented 4 years ago

@Prometee for every value that doesn't meet regexp condition src/Util/VatNumberUtil.php:43. E.g. 123 , 123123 etc. But remember that this is just NOTICE. So probably most of users on production environment have disabled this level of error reporting. But still, i think that is annoying problem especially in dev environment.

Could you tag new release with this change on master branch please?

Prometee commented 4 years ago

@sebastianlew thanks for the advice, this plugin will require some phpspec tests to avoid this kind of issues in the futur. I'll release a new version this morning and add phpspec during the week.

Again thanks for the PR 😉

Prometee commented 4 years ago

@sebastianlew release done, I also refactor a little bit the validator to avoid displaying validation message when the VAT number is not a valid one.

sebastianlew commented 4 years ago

@Prometee It was important for me, thank you for your fast reaction 😄

Prometee commented 4 years ago

I'm glad it helped you !