tsolucio / corebos

core Business Operating System. An OPEN SOURCE business application that helps small and medium business handle all the day to day tasks.
https://corebos.com
151 stars 142 forks source link

Validating an empty string as a VAT no. returns true #1010

Closed Luke1982 closed 3 years ago

Luke1982 commented 3 years ago

https://github.com/tsolucio/corebos/blob/3a8a2fca16a8c2e70a190d933ecb978d86520d77/include/validation/Validations.php#L72-L74

I think this is fundamentally wrong. We essentially say that an empty string is a valid EU VAT no., which it most definitely is not.

joebordes commented 3 years ago

I think this was totally on purpose to support creating accounts without a VAT number but forcing a valid one if given. That is a very typical use case when you are still prospecting. If you don't want to support that make the field mandatory and it should work. Open the issue again if I'm wrong.

Luke1982 commented 3 years ago

You're not wrong, but I do think we're taking a wrong approach. I wanted to re-use this function in a module I created to see if the VAT no. was correct. But I had to manually re-check the field for presence of any string at all. I think the functionality should be split over two logical functions:

joebordes commented 3 years ago

that sounds reasonable to me. go ahead I will accept it

reetp commented 3 years ago
* Validate the VAT field, which **should** allow an empty string (since yes, there are sales stages in which this could occur) but if the field is not empty, it should be a valid VAT no.

Absolutely.

We have LOTS of clients who do not have a VAT number. Enforcing this would be dire for us.