Fix stock status saving for variable products on bulk and quick edit #26174

Konamiman · 2020-04-14T11:09:45Z

All Submissions:

Have you followed the WooCommerce Contributing guideline?
Does your code follow the WordPress' coding standards?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

Fixes related to stock status for variable products on bulk and quick edit.

For bulk edit: Even if stock status was left as "No change", the status of all variations was being changed to whatever the status of the product was before it was converted to variable. Now no change is performed when "No change" is selected, and all variations change to whatever is selected otherwise.
For quick edit: A new "No change" option is added to the "Stock status" selector that will be preselected when the product is variable. Previously, whatever status the product had before being converted to variable was being shown, and that's the status that would be set when saving. Also, a "This will change the stock status of all variations" message is displayed before the selector.

Other improvements introduced:

WC_Unit_Test_Case class gets two new utility methods: login_as_role and login_as_administrator.
WC_Helper_Product::create_simple_product gets a new optional $props argument.

Closes #25843.

How to test the changes in this Pull Request:

Create a variable product with two variations. Set stock status to "Out of stock" for one of them and to "On backorder" for the other.
Bulk edit the product, don't change anything and save. Verify that the stock status of the variations hasn't changed (without the fix, both would have changed to "In stock").
Bulk edit the product again, se the stock status selector to "In stock" and save. Verify that this time the status of both variations has changed.
Change the stock status of both variations to anything but "In stock" again.
Quick edit the product. Verify that the preselected value for the stock status selector is now "No change". Leave it that way and save. Verify that the stock status of the variations hasn't changed.
Quick edit the product again. Select "In stock" in the stock status selector. Verify that this time the status of both variations has changed.

Other information:

Have you added an explanation of what your changes do and why you'd like us to include them?
Have you written new tests for your changes, as applicable?
Have you successfully run tests with your changes locally?

Changelog entry

Fix - don' change the stock status of variations when bulk editing a variable product and leaving the "Stock status" selector as "No change".
Enhancement - Add a "No change" option to the "Stock status" selector in quick edit, preselect it when the product being edited is a variable product.

includes/admin/class-wc-admin-post-types.php

vedanshujain

Great work! Overall its looking pretty good and nice job in refactoring some messy logic! I have left some comments, but mostly they nitpick and code is testing well.

assets/js/admin/quick-edit.js

includes/admin/class-wc-admin-post-types.php

includes/admin/views/html-quick-edit-product.php

includes/admin/class-wc-admin-post-types.php

assets/js/admin/quick-edit.js

includes/admin/class-wc-admin-post-types.php

Konamiman · 2020-06-15T14:10:14Z

I've removed that unnecessary //phpcs:ignore. @claudiosanches @vedanshujain could you plz review this again?

rrennick · 2020-07-07T18:30:33Z

@vedanshujain Could you go through the comments above and clear the resolved items?

vedanshujain

Changes look great and are testing well!! I have left a few comments, most of them are not blockers but one needs to be addressed before we can merge.

includes/admin/class-wc-admin-post-types.php

includes/admin/views/html-quick-edit-product.php

includes/admin/class-wc-admin-post-types.php

…min_Post_Types. Two methods have been created: - update_stock_status, replaces code that was duplicated in the quick_edit_save and bulk_edit_save methods. - set_new_price, replaces code that was duplicated-ish in the bulk_edit_save for setting the new regular and sales prices (code was not identical but very similar). Also, `round` is now used on sale price calculations that involve multiplying by a percent, the same was as it was done already to calculate the regular price.

For bulk edit: even if stock status was left as "No change", the status of all variations was being changed to whatever the status of the product was before it was converted to variable. Now no change is performed when "No change" is selected, and all variations change to whatever is selected otherwise. For quick edit: a new "No change" option is added that will be preselected when the product is variable. Previously, whatever status the product had before being converted to variable was being shown, and that's the status that would be set when saving. Also, a "This will change the stock status of all variations" message is displayed before the selector.

Create a new `request_data` method in WC_Admin_Post_Types that just returns $_REQUEST. This is intended to ease unit testing, as this method can be easily mocked to return test data.

Those methods are a convenient replacement for "this->factory->user->create". Tests that were using that to simulate user login have been modified to use the new methods.

The test added checks that stock status of variations when saving a variable product is changed or not appropriately depending on the request data supplied.

…tus_for_variable_product. At some point the 'change_stock' key is assumed to be present in the request data, but it might not. Fixed to test for existence before using the value.

Add a new optional $props parameter containing the properties to be set in the newly created product.

The test added checks that the new regular or sale price is appropriately set when bulk saving.

- Remove no longer needed phpcs disablers. - Rename method. - Remove no longer needed logic. - Simplify some logic.

vedanshujain

LGTM!

claudiosanches

Works great!

Konamiman added Bug The issue is a confirmed bug. status: needs review labels Apr 14, 2020

Konamiman self-assigned this Apr 14, 2020

vedanshujain reviewed Apr 14, 2020

View reviewed changes

includes/admin/class-wc-admin-post-types.php Outdated Show resolved Hide resolved

claudiosanches added status: in progress This is being worked on. and removed status: needs review labels Apr 15, 2020

Konamiman force-pushed the fix/25843 branch from 8bbacad to d610f1c Compare April 16, 2020 07:17

Konamiman commented Apr 16, 2020

View reviewed changes

includes/admin/class-wc-admin-post-types.php Outdated Show resolved Hide resolved

Konamiman added status: needs review and removed status: in progress This is being worked on. labels Apr 16, 2020

Konamiman force-pushed the fix/25843 branch from d610f1c to 0032572 Compare April 16, 2020 08:11

Konamiman marked this pull request as ready for review April 17, 2020 14:57

Konamiman force-pushed the fix/25843 branch from 0032572 to 31d27b4 Compare May 7, 2020 13:30

vedanshujain suggested changes May 21, 2020

View reviewed changes

claudiosanches suggested changes Jun 2, 2020

View reviewed changes

assets/js/admin/quick-edit.js Outdated Show resolved Hide resolved

includes/admin/class-wc-admin-post-types.php Show resolved Hide resolved

Konamiman force-pushed the fix/25843 branch from b3d879c to 5c6cc52 Compare June 15, 2020 14:08

vedanshujain self-requested a review July 13, 2020 14:26

vedanshujain reviewed Jul 13, 2020

View reviewed changes

includes/admin/class-wc-admin-post-types.php Outdated Show resolved Hide resolved

includes/admin/views/html-quick-edit-product.php Outdated Show resolved Hide resolved

includes/admin/class-wc-admin-post-types.php Outdated Show resolved Hide resolved

Konamiman added 10 commits July 14, 2020 11:42

Fix code sniffer errors in WC_Admin_Post_Types.

7d5b599

Fix code sniffer errors on quick-edit.js

39e5a1b

Extract $_REQUEST to a method in WC_Admin_Post_Types.

b71a9c3

Create a new `request_data` method in WC_Admin_Post_Types that just returns $_REQUEST. This is intended to ease unit testing, as this method can be easily mocked to return test data.

Fix code sniffer errors in a few unit test files.

9c2a43b

Add login_as_role and login_as_administrator to WC_Unit_Test_Case.

a3ee904

Those methods are a convenient replacement for "this->factory->user->create". Tests that were using that to simulate user login have been modified to use the new methods.

Add unit test for WC_Admin_Post_Types::bulk_and_quick_edit_save_post.

e05aa8a

The test added checks that stock status of variations when saving a variable product is changed or not appropriately depending on the request data supplied.

Small fix in WC_Tests_Admin_Post_Types::bulk_and_quick_edit_stock_sta…

4531af3

…tus_for_variable_product. At some point the 'change_stock' key is assumed to be present in the request data, but it might not. Fixed to test for existence before using the value.

Improve WC_Helper_Product::create_simple_product.

320fa38

Add a new optional $props parameter containing the properties to be set in the newly created product.

Konamiman added 4 commits July 14, 2020 11:42

Add unit test for WC_Admin_Post_Types::bulk_and_quick_edit_save_post.

fc0be07

The test added checks that the new regular or sale price is appropriately set when bulk saving.

Small adjustments in WC_Admin_Post_Types

a39e15f

- Remove no longer needed phpcs disablers. - Rename method. - Remove no longer needed logic. - Simplify some logic.

Remove unnecessary phpcs disabler in quick-edit.js

a33bcaf

Fix error when screen not set in request, remove redundand rounds.

c53d6da

Konamiman force-pushed the fix/25843 branch from 84e53e1 to c53d6da Compare July 14, 2020 09:49

Rename quick-edit-warning style with wc- prefix, and add styling.

d278781

vedanshujain approved these changes Jul 24, 2020

View reviewed changes

Fixed docblocks

419e5c2

claudiosanches approved these changes Jul 24, 2020

View reviewed changes

Merge branch 'master' into fix/25843

7bd34e3

claudiosanches merged commit aafb069 into master Jul 24, 2020

claudiosanches deleted the fix/25843 branch July 24, 2020 20:05

woocommercebot added release: add changelog Mark all PRs that have not had their changelog entries added. [auto] release: add testing instructions PRs that have not had testing instructions added to the wiki. [auto] labels Jul 24, 2020

claudiosanches added this to the 4.4.0 milestone Jul 24, 2020

claudiosanches added status: approved and removed status: needs review labels Jul 24, 2020

Konamiman removed status: approved release: add changelog Mark all PRs that have not had their changelog entries added. [auto] labels Jul 27, 2020

Fix stock status saving for variable products on bulk and quick edit #26174

Fix stock status saving for variable products on bulk and quick edit #26174

Uh oh!

Conversation

Konamiman commented Apr 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

All Submissions:

Changes proposed in this Pull Request:

Other improvements introduced:

How to test the changes in this Pull Request:

Other information:

Changelog entry

Uh oh!

Uh oh!

Uh oh!

vedanshujain left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Konamiman commented Jun 15, 2020

Uh oh!

rrennick commented Jul 7, 2020

Uh oh!

vedanshujain left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vedanshujain left a comment

Choose a reason for hiding this comment

Uh oh!

claudiosanches left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Konamiman commented Apr 14, 2020 •

edited

Loading