Skip to content
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

Added woocommerce_reduce_order_item_stock action hook to let other plugin hook functionalities without looping through the order items again and again. #34721

Merged
merged 3 commits into from Mar 8, 2023

Conversation

yousuf-hossain-shanto
Copy link
Contributor

All Submissions:

Changes proposed in this Pull Request:

Assume, you have to do something when the WooCommerce reduce order item stock by iterating through the line items. How can you do that?

The answer will, by using the woocommerce_reduce_order_stock hook which send order data to pass to callback function. But, you have to call $order->get_items() again and iterate again. And per iteration, you have to check 4-3 more conditions and call more queries again and again. But if we add a new hook at the end of the iteration block then we can reduce a lot of query call and complexities. Is't it good? I also have a real world example from my current project but can't disclose it before the plugin release, because of some company policies.

Into this PR, I have added a new woocommerce_reduce_order_item_stock hook which will pass line item, product and order data to the callback functions to do necessary things into the loop WooCommerce called to reduce stock. This will help the developers to reduce extra complexities.

How to test the changes in this Pull Request:

  1. By running pnpm test command

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?
  • Have you created a changelog file for each project being changed, ie pnpm changelog add --filter=<project>?

…ugins do somthing without looping again the order line items
@github-actions github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Sep 17, 2022
@jorgeatorres jorgeatorres self-requested a review January 19, 2023 14:04
Copy link
Member

@jorgeatorres jorgeatorres left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @yousuf-hossain-shanto!

Thanks for your contribution. It would've been nice to get more details about your use case here, but we see no harm on adding this filter.
That said, we have a few suggestions before this can be merged:

  • We think it'd be useful to have all the info in the filter, so maybe the arguments should be: $item, $changes, $order instead. That way you can use the from and to values, but also the product.
  • The docblock needs to be updated with these changes, but also the @since tag bumped to 7.4.0.

Please let me know what you think. Thanks again!

@jorgeatorres jorgeatorres added the needs: author feedback The issue/PR needs a response from any of the parties involved in the issue. label Jan 19, 2023
@yousuf-hossain-shanto
Copy link
Contributor Author

Hello @jorgeatorres, thanks for your feedback. I just made the changes as you said. Let me explain a bit about my use case

I'm working on a WC plugin where I have to execute some functions right after the stock change of a specific order item. Now I'm doing it using woocommerce_reduce_order_stock hook. WC is already using a loop through order items in the wc_reduce_stock_levels( $order_id ) function. To access specific order item right after the stock change, there is no other option without using another loop through order items. This repeatedly looping issue can be resolved by adding the proposed woocommerce_reduce_order_item_stock hook.

Please feel free to ask me more about it

@jorgeatorres jorgeatorres self-requested a review March 7, 2023 19:51
Copy link
Member

@jorgeatorres jorgeatorres left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi there!

Thanks for making the changes. I've left some feedback that needs to be addressed: a tiny coding standards issue and an update to the version tag in the docblock, which has to be changed because now the target WC version is 7.6.0.

Also, for me to be able to merge this, you need to add a changelog file to the PR, by running pnpm changelog add --filter=woocommerce locally and then adding the resulting file and pushing via Git.

I tried to make all these fixes for you, but your fork doesn't let me push changes, so I believe you'll have to do this yourself.

Once this is taken care of, we'll be ready to merge. Thanks again for your contribution!

plugins/woocommerce/includes/wc-stock-functions.php Outdated Show resolved Hide resolved
plugins/woocommerce/includes/wc-stock-functions.php Outdated Show resolved Hide resolved
@yousuf-hossain-shanto
Copy link
Contributor Author

Hello @jorgeatorres , thanks a lot for your suggestion. I just committed the changes. Consider checking it

Copy link
Member

@jorgeatorres jorgeatorres left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you @yousuf-hossain-shanto for your work on this one!

@jorgeatorres jorgeatorres merged commit 92f9424 into woocommerce:trunk Mar 8, 2023
19 of 20 checks passed
@github-actions github-actions bot added this to the 7.6.0 milestone Mar 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs: author feedback The issue/PR needs a response from any of the parties involved in the issue. plugin: woocommerce Issues related to the WooCommerce Core plugin. type: community contribution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants