Respect / Validation

The most awesome validation engine ever created for PHP
https://respect-validation.readthedocs.io
MIT License
5.75k stars 774 forks source link

phone() in 2.3 dont work correctly #1441

Closed axessweb closed 4 months ago

axessweb commented 5 months ago

Hello all!

After updating I got a 500 error because the package giggsey/libphonenumber-for-php was missing. (why not put it in the require?)

I installed it, but the parameterless phone() validator no longer works.

It's a shame, because it was a useful addition!

Thanks

henriquemoody commented 5 months ago

Yes, you're correct, we should have added it to require.

I can imagine we didn't do that because since not everyone would use the Phone rule, that could be an optional improvement. Nonetheless, we should have kept backward compatibility. I will write a patch for it.

Thank you for reporting! 🐼

henriquemoody commented 5 months ago

Could you give me an example of a phone with parenthesis that doesn't work? I've tried +44 (0)207 123 4567 and it worked fine.

axessweb commented 5 months ago

Thx @henriquemoody for reply !

I have been using this validation method for telephone numbers for several years.

(!v::notEmpty()->phone()->validate('0123456789')) ? 'Phone number is invalid' : null,

I like being able to validate a country's phone number. It's a real plus. But the code above no longer works with a FR number

Since the update, I have to do this:

(!v::notEmpty()->phone('FR')->validate('0123456789')) ? 'Phone number is invalid' : null,

For example, on a contact form for an international site, I don't necessarily know the user's country. And so it blocks. Sorry for my f****g bad English.

henriquemoody commented 4 months ago

My apologies for my delay! And no worries about your English, I didn't see anything wrong with it! -- and I'm not a native English speaker either.

We changed the Phone rule because it was inconsistent. It recognised patterns, but it was hard to ensure the number was correct -- phone numbers are complicated.

If you want to validate the format, you might consider using the Regex rule. Have you tried to use that instead?

libib commented 4 months ago

This is definitely a breaking change. Things that used to validate now don't.

555-433-5234 doesn't validate, it used to. 523-2343 doesn't validate, it used to.

4234325 doesn't validate, it used to.

This validator is way too restrictive.

That's fine, if you can provide the REGEX validation that we can use instead that matches the former validator...

axessweb commented 4 months ago

My apologies for my delay! And no worries about your English, I didn't see anything wrong with it! -- and I'm not a native English speaker either.

We changed the Phone rule because it was inconsistent. It recognised patterns, but it was hard to ensure the number was correct -- phone numbers are complicated.

If you want to validate the format, you might consider using the Regex rule. Have you tried to use that instead?

The old rules work wonders. I didn't have time to look into it anymore, I stuck at 2.2

henriquemoody commented 4 months ago

Looking closely, that is a breaking change. I could work around that, but that will be a breaking change for people using version 2.3. I'm a bit torn about that. Any suggestions?

libib commented 4 months ago

Maybe offer up both validators, at least until version 3 where breaking changes might be more acceptable?

Or if you don't want to do that, in the docs, just show us how to use the REGEX validator to simulate the legacy phone validation. That would work well as a quick workaround.

axessweb commented 4 months ago

@henriquemoody If you use without parameters, reuse old rules.

If a parameter is entered, then we take the new rule.

This solution should satisfy everyone. it's backwards compatibility, while adding something new.

I have several international sites, I don't know where my user comes from, and checking with the country beforehand requires more logistics.

libib commented 4 months ago

@henriquemoody's solution would work for us as well.

henriquemoody commented 4 months ago

Excellent suggestion, @libib!

I've just created a fix, which is already available on version 2.3.4!

Thank you for reporting this, @axessweb! 🐼

axessweb commented 4 months ago

@libib we suggested the same thing at the same time. 😉 Thank you for your intervention and your English better than me.

Thanks @henriquemoody for future fixes 👍

henriquemoody commented 4 months ago

No @axessweb, it was me who got confused!

Thank you for the excellent suggestion! In the end, I did what you suggested. And really, don't worry about your English, your English is fine!