Remove Postal Code Format Validation (and Twitter CLDR dependency) #2233
Conversation
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
We shouldn't validate the format of zipcodes. They are vastly different across countries, and the current validation does not tell us anything about whether the address can be shipped to, it only checks the format of the zipcode entered using a regular expresssion. I've briefly thought about adding a customizable address validator class, but that would've prompted users to hook address validation that hits the internet into the ActiveRecord callback chain. We can not want that. The replacement for this must be to validate addresses from host apps. Preferably this takes place at the interaction level rather than at the persistence level, as it is done here. The format validation using Twitter CLDR comes at a cost of a 60MB gem.
In the address factory, we were using Twitter CLDR for generating sample zip codes. We can just as well use FFaker, which conveniently comes with an address faking module for different countries. Our default address is in the USA, so we can use the FFaker::AddressUS module.
We don't need this gem anymore, and it's heavy.
tvdeyen
reviewed
Sep 20, 2017
|
The presence validation is still there, two lines above the deleted line: |
cbrunsdon
approved these changes
Sep 21, 2017
|
I like this. For business owners, address verification is done when generating the shipping label and checking fraud. For me, I can verify the address of orders through EasyPosts address Verification API and Stripes Radar. This covers malicious users. For non-malicious users, or rather, the ones making honest mistakes, we can detect them through services such as Google Maps. I'm 👍 on removing this and perhaps adding fraud checking and verification to the guides for new business owners. They will find much more value out of this. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The removes the validation of postal code format.
This validation is done through a pretty expensive dependency,
twitter_cldr, and actually provides little tangible benefit: We don't know whether the postal code enters actually exists if it passes, we only check the format through a regular expression.Stores that rely on this functionality are encouraged to validate the zipcode through something like https://github.com/dgilperez/validates_zipcode, which is a much smaller dependency. We could add that to core, and it would keep the functionality and reduce the size of our bundle, but really I question the benefit of this validation entirely.