[Experiment] Additional field extensible sanitisation and validation handling

[mikejolley]

Submission Review Guidelines:

• I have followed the WooCommerce Contributing Guidelines and the WordPress Coding Standards.
• I have checked to ensure there aren't other open Pull Requests for the same update/change.
• I have reviewed my code for security best practices.
• Following the above guidelines will result in quick merges and clear and detailed feedback when appropriate.

Changes proposed in this Pull Request:

Introduces new hooks for extensions to sanitise and validate registered checkout fields.

• Validation now uses an action hook with the error object passed by reference. This negates the need to compare the error object before and after hooks to ensure the object did not change
• Add sanitisation hook for custom sanitisation rules on additional fields
• Validation hook for custom validation rules on additional fields - key and value only
• Location validation hook for custom validation rules on additional fields where there are dependencies on the values of other fields in that location
• Validation applied to my account area

Closes #44019

How to test the changes in this Pull Request:

Developers should use this snippet this add some custom fields. Gov ID and email have validation methods.

add_action(
	'woocommerce_blocks_loaded',
	function() {
		woocommerce_blocks_register_checkout_field(
			array(
				'id'                => 'plugin-namespace/alt-email',
				'label'             => 'Alternative Email Field',
				'location'          => 'contact',
				'type'              => 'text',
				'required'          => true,
				'sanitize_callback' => function( $field_value ) {
					return sanitize_email( $field_value );
				},
				'validate_callback' => function( $field_value ) {
					if ( ! is_email( $field_value ) ) {
						return new \WP_Error( 'invalid_alt_email', 'Please ensure your alternative email matches the correct format.' );
					}
				},
			),
		);
		woocommerce_blocks_register_checkout_field(
			array(
				'id'                => 'plugin-namespace/gov-id',
				'label'             => 'Government ID',
				'location'          => 'address',
				'type'              => 'text',
				'required'          => true,
				'sanitize_callback' => function( $field_value ) {
					return str_replace( ' ', '', $field_value );
				},
			),
		);
		woocommerce_blocks_register_checkout_field(
			array(
				'id'       => 'plugin-namespace/confirm-gov-id',
				'label'    => 'Confirm Government ID',
				'location' => 'address',
				'type'     => 'text',
				'required' => true,
			),
		);
		woocommerce_blocks_register_checkout_field(
			array(
				'id'       => 'plugin-namespace/leave-on-porch',
				'label'    => __( 'Please leave my package on the porch if I\'m not home', 'woocommerce' ),
				'location' => 'additional',
				'type'     => 'checkbox',
				'required' => true,
			),
		);
		woocommerce_blocks_register_checkout_field(
			array(
				'id'       => 'plugin-namespace/location-on-porch',
				'label'    => __( 'Describe where we should hide the parcel', 'woocommerce' ),
				'location' => 'additional',
				'type'     => 'text',
			)
		);
		woocommerce_blocks_register_checkout_field(
			array(
				'id'       => 'plugin-namespace/leave-with-neighbor',
				'label'    => __( 'Which neighbor should we leave it with if unable to hide?', 'woocommerce' ),
				'location' => 'additional',
				'type'     => 'select',
				'options'  => array(
					array(
						'label' => 'Neighbor to the left',
						'value' => 'left',
					),
					array(
						'label' => 'Neighbor to the right',
						'value' => 'right',
					),
					array(
						'label' => 'Neighbor across the road',
						'value' => 'across',
					),
					array(
						'label' => 'Do not leave with a neighbor',
						'value' => 'none',
					),
				),
			)
		);

		add_action(
			'woocommerce_blocks_sanitize_additional_field',
			function( $value, $key ) {
				if ( 'plugin-namespace/confirm-gov-id' === $key ) {
					return str_replace( ' ', '', $value );
				}
				return $value;
			},
			10,
			2
		);
		add_action(
			'woocommerce_blocks_validate_additional_field',
			function ( \WP_Error $errors, $field_key, $field_value ) {
				if ( 'plugin-namespace/gov-id' === $field_key ) {
					$match = preg_match( '/[A-Z0-9]{5}/', $field_value );
					if ( 0 === $match || false === $match ) {
						$errors->add( 'invalid_gov_id', 'Please ensure your government ID matches the correct format.' );
					}
				}
			},
			10,
			3
		);
		add_action(
			'woocommerce_blocks_validate_location_address_fields',
			function ( \WP_Error $errors, $fields, $group ) {
				if ( $fields['plugin-namespace/gov-id'] !== $fields['plugin-namespace/confirm-gov-id'] ) {
					$errors->add( 'gov_id_mismatch', 'Please ensure your government ID matches the confirmation.' );
				}
			},
			10,
			3
		);
	}
);

1. Add items to cart and go to checkout.
2. Enter something in the custom "additional email" field that is not an email, and enter some letters in the Gov ID fields. Ensure both Gov ID address fields are different.
3. After typing in Gov ID the checkout will refresh. You'll see an error notice with 2 bullets (formatting is wrong, and fields do not match). Correct this and ensure the errors go away.
4. Place order and it will throw another error this time about email. Correct this and place the order again. It should go through.
5. Check the confirmation page values match what you submitted.
6. Go to the My Account page.
7. Under addresses, edit an address. Try changing the Gov ID fields and submit with errors. You'll see an error notice if it fails validation.
8. If you refresh the page before fixing the validation errors, correct values are restored.
9. If you fix the validation errors, the new values should save to the account.
10. Under "account details" change "Alternative Email" to something else. Save. See error message.
11. Correct the email field and save. Change should persist.

Changelog entry

• Automatically create a changelog entry from the details below.

Significance

• Patch
• Minor
• Major

Type

• Fix - Fixes an existing bug
• Add - Adds functionality
• Update - Update existing functionality
• Dev - Development related task
• Tweak - A minor adjustment to the codebase
• Performance - Address performance issues
• Enhancement - Improvement to existing functionality

Message

Comment

[github-actions]

Test Results Summary

Commit SHA: 94d2f73

Test 🧪	Passed ✅	Failed 🚨	Broken 🚧	Skipped ⏭️	Unknown ❔	Total 📊	Duration ⏱️
API Tests	259	0	0	2	0	261	0m 38s
E2E Tests	65	0	0	256	0	321	5m 21s

---

To view the full API test report, click here.
To view the full E2E test report, click here.
To view all test reports, visit the WooCommerce Test Reports Dashboard.

[mikejolley]

Removed on purpose; core has a set_shipping_phone method now.

[github-actions]

Hi @nielslange,

Apart from reviewing the code changes, please make sure to review the testing instructions as well.

You can follow this guide to find out what good testing instructions should look like:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

[opr]

Tested this out and seems to be working fine - I did look into the code and couldn't spot anything that should be actioned on this PR (left you a question on Slack for a follow up)

I will revisit on Monday and try some more edge cases too.

One other thing to mention is the order of arguments on the new hooks you added. Their args go $value first then $key second. I'd suggest making the key first and value second, but that's just my personal opinion, I am more used to saying "key/value" so that's what seems more natural to me.

If you decide not to change it then maybe consider updating the args of validate_field to match the order in the hook, just for consistency and to make it a little easier for future us to reason with.

Good work!

[mikejolley]

One other thing to mention is the order of arguments on the new hooks you added. Their args go $value first then $key second. I'd suggest making the key first and value second, but that's just my personal opinion, I am more used to saying "key/value" so that's what seems more natural to me.

Usually it depends if its the value thats the focus of the function/hook to what comes first. If its a hook, and we're filtering value, value must come first. Unless you spotted more places?

[opr]

One other thing to mention is the order of arguments on the new hooks you added. Their args go $value first then $key second. I'd suggest making the key first and value second, but that's just my personal opinion, I am more used to saying "key/value" so that's what seems more natural to me.

Usually it depends if its the value thats the focus of the function/hook to what comes first. If its a hook, and we're filtering value, value must come first. Unless you spotted more places?

Yeah I'm fine with either I just think validate_field should match then

[nielslange]

Thanks for working on this, @mikejolley. I've successfully tested the PR. While checking the code and the code snippet, I noticed that the validation hooks might have inconsistent names.

In the code snippet, I see the following two hooks:

• woocommerce_blocks_validate_additional_field
• woocommerce_blocks_validate_address_fields

I noticed that woocommerce_blocks_validate_additional_field validates both the custom fields in the "Contact information" section and the one in the "Additional order information" section.

Furthermore, I noticed that woocommerce_blocks_validate_additional_field is using the singular ([...]_field), while woocommerce_blocks_validate_address_fields is using the plural ([...]_fields).

I wonder if we should stick to either singular or plural for consistency reasons. I also wonder if we should have individual validation hooks (e.g. woocommerce_blocks_validate_contact_fields, woocommerce_blocks_validate_address_fields and woocommerce_blocks_validate_additional_fields) or one general validation hook (e.g. woocommerce_blocks_validate_custom_fields).

As this PR works as expected, I'm approving it. As for the validation hooks, I'll leave this up to you and the extensibility squad to decide if it's worth renaming them, introducing one general validation hook or creating individual hooks for each section. Or simply keep the validation as it is. I might have missed earlier discussions about that.

[opr]

I noticed that woocommerce_blocks_validate_additional_field validates both the custom fields in the "Contact information" section and the one in the "Additional order information" section.

Furthermore, I noticed that woocommerce_blocks_validate_additional_field is using the singular ([...]_field), while woocommerce_blocks_validate_address_fields is using the plural ([...]_fields).

To clarify woocommerce_blocks_validate_additional_field will be called for each field no matter where it is registered (contact, address, additional) and it is only intended to cover a single field (it is called multiple times, once for each field).

woocommerce_blocks_validate_address_fields will run once per location, and information about each field in that location will be passed.

The difference is how many fields each hook is responsible for validating, so I think the plural/singular use here is fine.

[opr]

When the additional area is validated, the hook is:
woocommerce_blocks_validate_additional_fields which may be easily confused with woocommerce_blocks_validate_additional_field. Do you think this is an issue? Should we consider renaming one or the other? Maybe adding location to the location hook would work? woocommerce_blocks_validate_location_additional_fields

[mikejolley]

Added in b424149

[mikejolley]

Good now @opr ? Ill get tests passing again.

[opr]

Working nicely now, thanks!

We should ensure sanitization is also tested, here's the example code I used:

add_action(
	'woocommerce_blocks_sanitize_additional_field',
	function( $value, $key ) {
		if ( 'plugin-namespace/gov-id' === $key || 'plugin-namespace/confirm-gov-id' === $key ) {
			return str_replace( ' ', '', $value );
		}
		return $value;
	},
	10,
	2
);

feel free to add that to the instructions if you like.

[opr]

Thanks Mike, this is working great. My errors are being caught, checkout can continue 🙌🏼