Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Tighten Canadian postal code validation.
D, F, I, O, Q, U should not be valid anywhere in a canadian postal code. Fixes #3708
- Loading branch information
Showing
2 changed files
with
7 additions
and
1 deletion.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
83de70e
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably also update (the redundant) https://github.com/cakephp/localized
83de70e
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe all localized validations should be moved to the localized plugin, also i noticed phone() method uses 'can' as country, where postal uses 'ca'
83de70e
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree.
PS: The same "spelling mistake" I noted a few days ago in the core dev channel.
83de70e
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would also prefer moving all localized stuff into the localized plugin. But we can't break BC in 2.x.
83de70e
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah we started the localized plugin after there were more and more demands for various other locales. That's the main reason the localized hooks exist in the core. I'm fine with extracting the localizations out in a future release. However, which 'postal' implementation is the canonical one? USA is just another locale to me as a non-american.
83de70e
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
like the phone validation there should then be some "all" generic postal code validation regexp allowing "A-Z0-9" and dash + space etc (which would be valid for all countries then).
83de70e
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Except that A-Z0-9 isn't valid in all countries. So the default validation would just be wrong.
83de70e
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You forgot the missing chars like dash and space
[A-Z0-9- ]
. Default validation would at least filter always-invalid chars like % and $ etc then.But I see that this would not be ideal.
83de70e
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Such a generic filter wouldn't be useful to anybody. In such a case perhaps the method could simply act as a stub with actual rules plugged in through the localized plugin.
83de70e
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And throwing an exception maybe if not properly extended by the localized plugin?
83de70e
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A stub method might work, we'd need to be better about keeping localized up to date though. I haven't looked at it recently and don't know how complete/well tested it is.