awesomemotive / easy-digital-downloads

Sell digital downloads through WordPress
https://easydigitaldownloads.com
GNU General Public License v2.0
862 stars 475 forks source link

Proper validation for zip/postal code when billing country is Ireland. #5307

Open pderksen opened 7 years ago

pderksen commented 7 years ago

Not sure if I'm 100% correct on this, but I believe we shouldn't require a zip code for Irish customers. Or possibly the validation should require but is not doing so correctly.

I've had several (potential) customers send this complaint to me the last few months.

One customer:

I live in Ireland and here we do not have 5 digits zip/postal code. Tried with 5 zeros but kept failing zip input validation.

Another:

Irish post codes are like the following: T12 Y275 - which is the one for my own card. When I enter this, I get: Error: The zip code you supplied failed validation

From my own research, it looks like there's a mix of both no postal codes and a new "Eircode" system introduced in 2014 spanning the country.

WooCommerce issue discussion & commit:

https://github.com/woocommerce/woocommerce/issues/10663 https://github.com/woocommerce/woocommerce/commit/3112c769395763617725add855e1a5857c2ac007

I'm trying to first nail down what it should validate if Ireland is the selected country.

It doesn't look like there's any special case logic for Ireland in EDD core so far (country IE).

I'm also using EDD-Stripe and not requiring a full billing address, just country and zip. Not sure if this is related.

Thanks!

pippinsplugins commented 7 years ago

Thanks for the report @pderksen, we'll get this addressed shortly after 2.7.

evertiro commented 7 years ago

@pippinsplugins OK, I've been trying to figure this out, but I'm at a loss on two fronts. First, if there's a chance of no zip code, we can't REQUIRE the zip field, but I don't see an existing way to disable that requirement. We have an existing way of removing state from the require list, but I don't see a way for zip... though I can't believe there isn't one. Second, I've determined I still suck at regex. Mind giving me some guidance on how to do the validation for that string?

pippinsplugins commented 7 years ago

@ghost1227 we can make the field not required here: https://github.com/easydigitaldownloads/easy-digital-downloads/blob/master/includes/process-purchase.php#L393

Perhaps we should add a new edd_maybe_require_zip() function that then programmatically determines if the zip code should be required based on the country that is submitted.

evertiro commented 7 years ago

@pippinsplugins I THINK this does what you want in terms of helper function flexibility. Right now, only excludes IE, but can certainly add others

pippinsplugins commented 7 years ago

@ghost1227 see PR review for a couple of minor changes./

evertiro commented 7 years ago

@pippinsplugins Changes made!

pippinsplugins commented 7 years ago

PR looks good to me.

@mintplugins could you please test it further?

pderksen commented 7 years ago

Thanks fellas!

SeanTOSCD commented 7 years ago

I'm actually unable to replicate an issue here on EDD master and Stripe master. Whether it's a 5 digit zip, or structured like T12 Y275, I'm able to complete a purchase with my billing address country set to Ireland. I've been testing with Stripe Checkout disabled. When enabled, the modal automatically removes the Zip field when you select Ireland as the country.

Am I missing something about how to replicate this issue? Has anyone replicated it besides the customers?

mintplugins commented 7 years ago

I was also unable to replicate the original issue.

pippinsplugins commented 7 years ago

Let's go ahead and punt this until we can actually replicate it.