Do not disable "Used for variations" checkbox #39502

mattsherman · 2023-07-31T23:00:25Z

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:

This PR makes a few changes related to the "Used for variations" checkbox:

The "Used for variations" checkbox is never disabled. I've reverted the behavior that disabled the checkbox when the product type was not variable.
- This returns the behavior of the checkbox to what it was in WooCommerce 7.8.
- I have determined that there would not be a way to implement disabling the checkbox without potentially breaking extensions, such as WooCommerce Subscriptions. If we were to add such a feature in the future we would need to be careful to proactively let extensions know that this change would be coming and provide them with a clear way to modify the behavior.
The woocommerce_added_attribute trigger has been restored. This is needed by extensions such as WooCommerce Subscriptions in order to re-show the "Used for variations" checkbox when it is hidden.
The requesting and generation of the empty attribute form has been moved from when the product edit screen loads to when the "Attributes" tab is shown. This allows for the scenario where the product type is changed from "simple" to something else before clicking the "Attributes" tab. The "Used for variations" checkbox will now be correctly defaulted to checked when on a variable product and unchecked for all other product types. This change will also partially address the lingering issue Do not show "Adding new attribute failed" when no attributes are set (on a classic theme) #39377.

Fixes #39490.
Fixes #39338 (this was closed previously even though it was not fixed)
Fixes #39502 (this was closed previously even though it was not fixed)

How to test the changes in this Pull Request:

Perform general regression testing by creating and editing Simple products, Variable products, etc.
- The "Used for variations" checkbox should be checked by default for Variable products.
- The "Used for variations" checkbox should be hidden for all other product types (unless an extension has re-shown it, which is actually what WooCommerce Subscriptions is doing to get it to appear).
Install WooCommerce Subscriptions 5.2.0 and create and edit Subscriptions and Variable Subscriptions.
- WooCommerce Subscriptions 5.2.0 does not contain any fixes to handle the issues fixed by this PR, so it is a good test of an extension that was experiencing problems.
- The "Used for variations" checkbox should be unchecked by default for Variable Subscription products.
- The "Used for variations" checkbox should be hidden for Subscription products.
- (In other words, WooCommerce Subscriptions 5.2.0 should work with this fix)
Install WooCommerce Subscriptions 5.3.1 and verify that the fixes in that version still work with the fixes in this PR.
- The "Used for variations" checkbox should be unchecked by default for Variable Subscription products.
- The "Used for variations" checkbox should be hidden for Subscription products.

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

…s variable

…for product type switching

github-actions · 2023-07-31T23:38:31Z

Test Results Summary

Commit SHA: 144606f

Test 🧪	Passed ✅	Failed 🚨	Broken 🚧	Skipped ⏭️	Unknown ❔	Total 📊	Duration ⏱️
API Tests	259	0	0	2	0	261	0m 54s
E2E Tests	189	0	0	19	0	208	15m 6s

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.

github-actions · 2023-07-31T23:44:03Z

Hi , @woocommerce/mothra

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

AnnaMag · 2023-08-01T08:16:33Z

Fixes #39338 (this was closed previously even though it was not fixed)

@mattsherman any related issues were closed in favor of #39490 during triage.

AnnaMag

Thanks for working on this @mattsherman.

Tested as described. Approving, pending a fix to the failing test.

AnnaMag · 2023-08-01T10:30:08Z

plugins/woocommerce/includes/class-wc-ajax.php

@@ -619,7 +621,13 @@ public static function add_attribute() {
 		$attribute->set_name( sanitize_text_field( wp_unslash( $_POST['taxonomy'] ) ) );
 		/* phpcs:disable WooCommerce.Commenting.CommentHooks.MissingHookComment */
 		$attribute->set_visible( apply_filters( 'woocommerce_attribute_default_visibility', 1 ) );
-		$attribute->set_variation( apply_filters( 'woocommerce_attribute_default_is_variation', 1 ) );
+		$attribute->set_variation(
+			apply_filters(


Would reverting to the previous functionality be sufficient?

This would also fix the test-- setting a simple product as the default could create more edge cases in the future 🤔

@AnnaMag I don't see any failing test. What test are you referring to?

I'd prefer not to revert to the previous functionality, since Jarek has been quite set that this change will help prevent confusion based on his user research.

* Do not disable "Used for variations" checkbox (#39502) * Prep for cherry pick 39502 --------- Co-authored-by: Matt Sherman <matt.sherman@automattic.com> Co-authored-by: WooCommerce Bot <no-reply@woocommerce.com>

mattsherman added 7 commits July 31, 2023 18:11

Only set attribute as used for variation by default if product type i…

119ab3c

…s variable

Trigger woocommerce_tab_shown when a tab is shown

8f4b123

Add the empty attribute when the attributes tab is shown, to account …

850acd0

…for product type switching

Do not disable Used for variations checkbox by default

a78d4d6

Remove disable_or_enable_fields() implementation

a38a867

Restore woocommerce_added_attribute trigger

956aa7b

Changelog

479c4ed

github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Jul 31, 2023

Check if product_type query arg is set before using

144606f

mattsherman changed the title ~~Fix/used for variations checkbox disabled~~ Do not disable "Used for variations" checkbox Jul 31, 2023

This was referenced Jul 31, 2023

Do not show "Adding new attribute failed" when no attributes are set (on a classic theme) #39377

Open

"Used for variations" default value changed from FALSE to TRUE #39265

Closed

mattsherman self-assigned this Jul 31, 2023

mattsherman marked this pull request as ready for review July 31, 2023 23:43

mattsherman requested a review from a team July 31, 2023 23:43

AnnaMag approved these changes Aug 1, 2023

View reviewed changes

mattsherman merged commit 85e3773 into trunk Aug 1, 2023
25 of 26 checks passed

mattsherman deleted the fix/used-for-variations-checkbox-disabled branch August 1, 2023 14:01

github-actions bot added this to the 8.1.0 milestone Aug 1, 2023

github-actions bot added the needs: analysis Indicates if the PR requires a PR testing scrub session. label Aug 1, 2023

jonathansadowski modified the milestones: 8.1.0, 8.0.0 Aug 1, 2023

This was referenced Aug 1, 2023

Cherry pick 39502 into release/8.0 #39510

Merged

Delete changelog files based on PR 39502 #39511

Merged

github-actions bot pushed a commit that referenced this pull request Aug 1, 2023

Do not disable "Used for variations" checkbox (#39502)

51eee9e

alopezari removed the needs: analysis Indicates if the PR requires a PR testing scrub session. label Aug 2, 2023

alopezari added the needs: internal testing Indicates if the PR requires further testing conducted by Solaris label Aug 2, 2023

AnnaMag mentioned this pull request Aug 4, 2023

"Used for variations" checkbox: simple to variable product switches it off. #39578

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Do not disable "Used for variations" checkbox #39502

Do not disable "Used for variations" checkbox #39502

mattsherman commented Jul 31, 2023 •

edited

github-actions bot commented Jul 31, 2023

github-actions bot commented Jul 31, 2023

AnnaMag commented Aug 1, 2023 •

edited

AnnaMag left a comment

AnnaMag Aug 1, 2023

AnnaMag Aug 1, 2023

mattsherman Aug 1, 2023

Do not disable "Used for variations" checkbox #39502

Do not disable "Used for variations" checkbox #39502

Conversation

mattsherman commented Jul 31, 2023 • edited

Submission Review Guidelines:

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 Jul 31, 2023

Test Results Summary

github-actions bot commented Jul 31, 2023

AnnaMag commented Aug 1, 2023 • edited

AnnaMag left a comment

Choose a reason for hiding this comment

AnnaMag Aug 1, 2023

Choose a reason for hiding this comment

AnnaMag Aug 1, 2023

Choose a reason for hiding this comment

mattsherman Aug 1, 2023

Choose a reason for hiding this comment

mattsherman commented Jul 31, 2023 •

edited

AnnaMag commented Aug 1, 2023 •

edited