Ensure that each <CheckboxControl> component has a unique ID #45655

nielslange · 2024-03-18T09:57:01Z

Changes proposed in this Pull Request:

Closes #42046.

A while ago, we noticed that not all checkbox labels in the Checkout block had unique IDs. This led to the fact that when clicking one checkbox label, another section was triggered. In #42046, I shared the following observation:

These are the checkboxes and their IDs in incognito mode:

Checkbox:	ID:
`Create an account?`	`checkbox-control-0`
`Use same address for billing`	`checkbox-control-1`
`Add a note to your order`	`checkbox-control-0`

These are the checkboxes and their IDs in logged-in mode:

Checkbox:	ID:
`Use same address for billing`	`checkbox-control-0`
`Add a note to your order`	`checkbox-control-0`

@senadir addressed this issue in woocommerce/woocommerce-blocks#12015. In this issue, I've added e2e tests, which allow us to detect potential problems in an early stage. By default, the Checkout block can show the following four checkboxes:

Create an account?
Use same address for billing
Add a note to your order
You must accept our Terms and Conditions and Privacy Policy to continue with your purchase.

Our e2e test environment already shows the second, third, and fourth checkbox by default. To fully test all checkboxes, I ensured that the first checkbox Create an account? is also visible. To do that, this PR introduced the following changes:

Allow guests to place an order → allSettings.checkoutAllowsGuest = true;
Allow account creation during the checkout → allSettings.checkoutAllowsSignup = true;
Set the customer ID to 0 → dispatch( CHECKOUT_STORE_KEY ).__internalSetCustomerId( 0 );

Only if all three conditions are met, the Create an account? checkbox becomes visible. The PR also restores the initial values after executing the test.

How to test the changes in this Pull Request:

Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:

Verify that all JS tests are passing.

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

Add an e2e test to ensure that each component has a unique ID.

Comment

github-actions · 2024-03-18T10:11:13Z

Test Results Summary

Commit SHA: f9dc91b

Test 🧪	Passed ✅	Failed 🚨	Broken 🚧	Skipped ⏭️	Unknown ❔	Total 📊	Duration ⏱️
API Tests	259	0	0	2	0	261	0m 38s
E2E Tests	332	0	0	59	0	391	8m 7s

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.

...erce-blocks/tests/e2e/tests/checkout/checkout-block.shopper.block_theme.side_effects.spec.ts

github-actions · 2024-03-21T06:25:24Z

Hi @senadir, @wavvves, @woocommerce/rubik

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

nielslange · 2024-03-21T06:31:19Z

@senadir I've opened up this PR for review, even though there's one failing test. The test is flaky and not related to this PR, as it tests the Filter Products by Price block when using the All Products block.

wavvves

LGTM!

senadir · 2024-03-21T11:15:56Z

This can also be done via an integration test instead of an E2E.

nielslange · 2024-03-21T12:59:23Z

This can also be done via an integration test instead of an E2E.

Thanks for your review, @senadir. Could you share a starting point on this? I'm asking as I could only find integration tests of individual components. I could not find any integration tests where we're testing the entire checkout form. 🤔

senadir · 2024-03-21T13:05:38Z

There isn't any so far, only for Cart. I'm creating one for Checkout that we can use.

nielslange · 2024-03-21T14:30:10Z

I'm changing this back to draft to implement integration tests instead of e2e tests.
cc: @senadir and @wavvves

…mmerce-blocks

senadir · 2024-03-27T15:43:33Z

plugins/woocommerce-blocks/assets/js/blocks/checkout/test/block.js

+	it( 'Ensures checkbox labels have unique IDs', async () => {
+		allSettings.checkoutAllowsGuest = true;
+		allSettings.checkoutAllowsSignup = true;
+		dispatch( CHECKOUT_STORE_KEY ).__internalSetCustomerId( 0 );


Suggested change

dispatch( CHECKOUT_STORE_KEY ).__internalSetCustomerId( 0 );

allSettings.checkoutData.customer_id = 1;

Using this works as well and reduces the risk of depending on data store and reflect actual use case (of the value coming from the server).

I tried allSettings.checkoutData.customer_id = 0;, but this results in the following checkbox label IDs:

[["checkbox-control-6", "checkbox-control-7", "terms-and-conditions"]]

While using dispatch( CHECKOUT_STORE_KEY ).__internalSetCustomerId( 0 );, provides these checkbox label IDs:

[["checkbox-control-6", "checkbox-control-7", "checkbox-control-8", "terms-and-conditions"]]

For allSettings.checkoutData.customer_id = 0;, I'm also seeing this error:

'allSettings.checkoutData' is of type 'unknown'.ts(18046)

I do not see any TS errors for allSettings.checkoutAllowsGuest = true; and allSettings.checkoutAllowsSignup = true;.

Switching to allSettings.checkoutData.customer_id = 1; did pass the test for me, should the test have failed?

The test passed as there are still three checkboxes visible. Setting the customer ID to 0 then shows a fourth checkbox.

senadir · 2024-03-27T15:44:21Z

plugins/woocommerce-blocks/assets/js/blocks/checkout/test/block.js

+		allSettings.checkoutAllowsGuest = true;
+		allSettings.checkoutAllowsSignup = true;
+		dispatch( CHECKOUT_STORE_KEY ).__internalSetCustomerId( 0 );


You must reset those values to their original values at the end of the your test to not pollute other tests. Each test should leave the global state like it was found.

Looking at

woocommerce/plugins/woocommerce-blocks/tests/js/setup-globals.js

Line 216 in 7d6d2c9

customer_id: 1,

the customer_id is set to 1 by default. Given that allSettings.checkoutData.customer_id = 0; does not lead to the expected result, as one of the checkboxes is not visible then, I'd place the following line at the end of the test, to reset the global state:

dispatch( CHECKOUT_STORE_KEY ).__internalSetCustomerId( 1 );

Would you agree that this is the way forward in this case, @senadir?

Also reset allSettings.checkoutAllowsGuest , allSettings.checkoutAllowsSignup to their default values.

For allSettings.checkoutAllowsGuest and allSettings.checkoutAllowsSignup, I'm seeing that they're undefined by default. So, I'd add the following lines at the end of the test:

allSettings.checkoutAllowsGuest = undefined; allSettings.checkoutAllowsSignup = undefined;

* Ensure that each <CheckboxControl> component has a unique id * Add changefile(s) from automation for the following project(s): woocommerce-blocks * Optimise setup and teardown settings * Wrapping setup and teardown in act() --------- Co-authored-by: github-actions <github-actions@github.com>

nielslange · 2024-03-30T07:18:50Z

Thanks for reviewing and approving this PR, @senadir. 🙌

nielslange added the team: Rubik Store API checkout endpoints, Mini-Cart, Cart and Checkout related issues label Mar 18, 2024

nielslange self-assigned this Mar 18, 2024

nielslange closed this Mar 18, 2024

nielslange reopened this Mar 18, 2024

WunderBart reviewed Mar 18, 2024

View reviewed changes

...erce-blocks/tests/e2e/tests/checkout/checkout-block.shopper.block_theme.side_effects.spec.ts Outdated Show resolved Hide resolved

github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Mar 20, 2024

nielslange requested review from WunderBart, a team and senadir and removed request for a team March 21, 2024 06:25

nielslange removed the request for review from WunderBart March 21, 2024 06:27

nielslange marked this pull request as ready for review March 21, 2024 06:27

woocommercebot requested a review from a team March 21, 2024 06:27

wavvves approved these changes Mar 21, 2024

View reviewed changes

nielslange marked this pull request as draft March 21, 2024 14:29

This was referenced Mar 21, 2024

Try fixing additional checkout fields E2E test #45789

Merged

Create integration test to ensure that <CheckboxControl> component label links to correct element #42046

Closed

senadir removed their request for review March 25, 2024 08:02

Ensure that each <CheckboxControl> component has a unique id

ed144e9

nielslange force-pushed the add/42046-CheckboxControl-e2e-tests branch from c7f5392 to ed144e9 Compare March 27, 2024 15:05

github-actions bot removed the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Mar 27, 2024

Add changefile(s) from automation for the following project(s): wooco…

f9dc91b

…mmerce-blocks

nielslange closed this Mar 27, 2024

nielslange reopened this Mar 27, 2024

github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Mar 27, 2024

senadir reviewed Mar 27, 2024

View reviewed changes

nielslange requested a review from senadir March 27, 2024 16:01

nielslange added 2 commits March 27, 2024 23:15

Optimise setup and teardown settings

422637e

Wrapping setup and teardown in act()

6c1875b

nielslange marked this pull request as ready for review March 27, 2024 17:17

nielslange requested a review from wavvves March 27, 2024 17:17

woocommercebot requested a review from a team March 27, 2024 17:17

senadir approved these changes Mar 29, 2024

View reviewed changes

senadir merged commit cad1aa6 into trunk Mar 29, 2024
48 checks passed

senadir deleted the add/42046-CheckboxControl-e2e-tests branch March 29, 2024 11:46

github-actions bot added this to the 8.9.0 milestone Mar 29, 2024

github-actions bot added the needs: analysis Indicates if the PR requires a PR testing scrub session. label Mar 29, 2024

veljkho added status: analysis complete Indicates if a PR has been analysed by Solaris and removed needs: analysis Indicates if the PR requires a PR testing scrub session. labels Mar 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ensure that each <CheckboxControl> component has a unique ID #45655

Ensure that each <CheckboxControl> component has a unique ID #45655

nielslange commented Mar 18, 2024 •

edited

Loading

github-actions bot commented Mar 18, 2024 •

edited

Loading

github-actions bot commented Mar 21, 2024 •

edited

Loading

nielslange commented Mar 21, 2024

wavvves left a comment

senadir commented Mar 21, 2024

nielslange commented Mar 21, 2024

senadir commented Mar 21, 2024

nielslange commented Mar 21, 2024 •

edited

Loading

senadir Mar 27, 2024

nielslange Mar 27, 2024

senadir Mar 27, 2024

nielslange Mar 27, 2024

senadir Mar 27, 2024

nielslange Mar 27, 2024

senadir Mar 27, 2024

nielslange Mar 27, 2024

nielslange commented Mar 30, 2024

	dispatch( CHECKOUT_STORE_KEY ).__internalSetCustomerId( 0 );
	allSettings.checkoutData.customer_id = 1;

Ensure that each <CheckboxControl> component has a unique ID #45655

Ensure that each <CheckboxControl> component has a unique ID #45655

Conversation

nielslange commented Mar 18, 2024 • edited Loading

Changes proposed in this Pull Request:

How to test the changes in this Pull Request:

Changelog entry

Significance

Type

Message

Comment

github-actions bot commented Mar 18, 2024 • edited Loading

Test Results Summary

github-actions bot commented Mar 21, 2024 • edited Loading

nielslange commented Mar 21, 2024

wavvves left a comment

Choose a reason for hiding this comment

senadir commented Mar 21, 2024

nielslange commented Mar 21, 2024

senadir commented Mar 21, 2024

nielslange commented Mar 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nielslange commented Mar 30, 2024

nielslange commented Mar 18, 2024 •

edited

Loading

github-actions bot commented Mar 18, 2024 •

edited

Loading

github-actions bot commented Mar 21, 2024 •

edited

Loading

nielslange commented Mar 21, 2024 •

edited

Loading