Skip to content
This repository has been archived by the owner on Feb 23, 2024. It is now read-only.

Client side postcode validation #7722

Closed
mikejolley opened this issue Nov 21, 2022 · 12 comments · Fixed by #7945 or #8503
Closed

Client side postcode validation #7722

mikejolley opened this issue Nov 21, 2022 · 12 comments · Fixed by #7945 or #8503
Assignees
Labels
block: checkout Issues related to the checkout block. type: enhancement The issue is a request for an enhancement.

Comments

@mikejolley
Copy link
Member

We need to implement postcode validation in the client to prevent erroneous requests to the server. Postcode validation rules can be found in core/Store API.

@mikejolley mikejolley added type: enhancement The issue is a request for an enhancement. block: checkout Issues related to the checkout block. labels Nov 21, 2022
@nielslange
Copy link
Member

Earlier attempt in validating postcodes on client-side: #4014

@nielslange nielslange self-assigned this Dec 6, 2022
@tarunvijwani
Copy link

#6989 issue could also be resolved when we introduce client-side validation.

@senadir
Copy link
Member

senadir commented Dec 6, 2022

postcode validation is extensible on PHP, so there are a couple of things to consider here:

  • Should we somehow serialize the PHP validation and passed it down via settings.
    • If we go that route, I think we should limit the countries to only the ones accepted by the store, this is to reduce how much data we print to the screen.
  • Do we instead copy/paste the PHP validation into a new JS validation?
    • How do we handle extensibility, like adding new countries? missing validation call still hit the server, we shouldn't strive for 100% coverage here.
    • How about cases when a merchant loosens a validation rule, we would need a way to reflect that in the client or we end up with false negatives.
  • Do we instead just use an external library? Ideally the PHP validation should match the JS validation (thou this is not a strict requirement).

@mikejolley
Copy link
Member Author

mikejolley commented Dec 6, 2022

I believe we should serialise the rules, not the regex. e.g. this country has digit limit of 2. Then we can handle the implementation details in JS/PHP separately.

How do we handle extensibility, like adding new countries? missing validation call still hit the server, we shouldn't strive for 100% coverage here.

I have a way to funnel errors from the API to the field validation - just haven't raised a PR yet.

@senadir
Copy link
Member

senadir commented Dec 6, 2022

So you want to translate rules to a regular expression?

@nielslange
Copy link
Member

Given that some folks complaints about performance in the past, I wonder if it isn't a faster approach to validating the postcode client-side using a TS-function. As for the current PHP-based validation, I noticed that many countries are not validated, e.g. Australia. Within the core validation of Woo, I also noticed this fallback value:

https://github.com/woocommerce/woocommerce/blob/2a56407ba125ab281f901817af2485438c18a9b0/plugins/woocommerce/includes/class-wc-validation.php#LL121-L123

I might be mistaken, but to me the current postcode validation from Woo core looks a bit incomplete.

@mikejolley
Copy link
Member Author

Client-side validation is the idea here — this is the main pain point (invalid data is being sent to the server, which fails the server side validation). We need parity with the validation functions in core.

I also noticed this fallback value

It needs to default to true to accommodate countries with no rules or incomplete rules.

So you want to translate rules to a regular expression?

Thats an option yes. But to avoid core refactors we can probably just aim for parity and log an issue for the future to share the same system. Any "custom" postcode rules will just come through via the API, if data is not valid.

@nielslange
Copy link
Member

It needs to default to true to accommodate countries with no rules or incomplete rules.

Ah, I see. Thanks for clarifying this.

@mikejolley
Copy link
Member Author

I looked into a few different external libraries, and the majority are indeed regex-based, so a similar approach as woo core and the earlier attempt.

Examples:

I looked to see if Stripe had a validation library for this, but I couldn't find anything. In the docs, they do say they validate numeric vs alphanumeric codes based on country.

Let's go with regex in the client. If we cover only the countries listed in core on the PHP side, that should be enough. Also, if we could get the GB postcode format done in regex, so all rules are consistent, that might also be a good idea instead of having dedicated functions for certain countries.

  • This will also need test coverage to test against valid and invalid formats.
  • Fallback for countries with no rules should be to pass.
  • I would prefer not to allow extensibility for validation libs because the rules should be true and not require modification cc @senadir do you disagree?
  • For cases where core validation does differ from the client (more strict), we should ensure API errors get funnelled into the validation context, so the error is presented inline. I have a method to do this but will tackle in a follow-up.

@senadir
Copy link
Member

senadir commented Dec 7, 2022

I would prefer not to allow extensibility for validation libs because the rules should be true and not require modification cc @senadir do you disagree?
For cases where core validation does differ from the client (more strict), we should ensure API errors get funnelled into the validation context, so the error is presented inline. I have a method to do this but will tackle in a follow-up.

Those are the 2 points I have in mind, having rules extensible isn't ideal (I think it might be the wrong approach).

The use case I'm afraid of is when a merchant uses code to remove validation for a country so now you have a client that is overly stricter than it should be.

However, I think this can be solved in the future by code as well, you can (using a simple woocommerce core hook) make the postcode not required for a country and that should solve it.

@senadir
Copy link
Member

senadir commented Dec 7, 2022

The code to loosen up client requirement is this:

add_filter(
	'woocommerce_get_country_locale',
	function ( $countries ) {
		$countries['DZ'] = array(
			'postcode' => array(
				'hidden' => true,
				'required' => false
			),
			'address_1' => array(
				'hidden' => false,
				'required' => false
			)
		);
		return $countries;
	},
	1,
	10
);

@nielslange
Copy link
Member

#7945 had been merged automatically due to a rebasing-conflict. I'm reopening this issue as its functionality should be implemented with #8503.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
block: checkout Issues related to the checkout block. type: enhancement The issue is a request for an enhancement.
Projects
None yet
4 participants