New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Track stock quantity for this product should be disabled when Enable stock management within settings is disabled, and enabled otherwise #37957
Conversation
…stock management within settings is disabled, and enabled otherwise.
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: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice addition with the disabled
prop for the toggle block! Testing well and LGTM!
Left one comment just thinking out loud about how we use this in the future, butI think we can address this when we get there.
@@ -14,6 +14,10 @@ | |||
}, | |||
"property": { | |||
"type": "string" | |||
}, | |||
"disabled": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call!
@@ -625,6 +625,7 @@ public static function register_post_types() { | |||
array( | |||
'label' => __( 'Track stock quantity for this product', 'woocommerce' ), | |||
'property' => 'manage_stock', | |||
'disabled' => 'yes' !== get_option( 'woocommerce_manage_stock' ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just thinking about this in the future: once the settings screens are Reactified, we might want the ability to either:
a. Refetch this template (so we have the correct disabled
property) OR
b. Move this setting check to the frontend
But this issue doesn't exist for the time being and I think punting it down the road is fine.
Test Results SummaryCommit SHA: a776c64
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. |
Submission Review Guidelines:
Changes proposed in this Pull Request:
Closes #37888
How to test the changes in this Pull Request:
Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:
/wp-admin/tools.php?page=woocommerce-admin-test-helper
product-block-editor
Manage stock
is active under/wp-admin/admin.php?page=wc-settings&tab=products§ion=inventory
thenTrack stock quantity for this product
under/wp-admin/admin.php?page=wc-admin&path=/add-product&tab=inventory
should be enabledManage stock
is not active under/wp-admin/admin.php?page=wc-settings&tab=products§ion=inventory
thenTrack stock quantity for this product
under/wp-admin/admin.php?page=wc-admin&path=/add-product&tab=inventory
should be disabled