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

Allow items to be changed in wc_save_order_items function #20490

Merged
merged 3 commits into from Jul 11, 2018

Conversation

dezio1900
Copy link
Contributor

@dezio1900 dezio1900 commented Jun 11, 2018

All Submissions:

Changes proposed in this Pull Request:

Allow other plugins to be able to change $item object, for example to adjust line subtotal/total before its saved.

My user case for this change is:
Because I have custom booking product item, and attached custom inputs for the item ( checkin / checkout dates etc ). So whenever those inputs are changed in order edit page and ajax save button is triggered, I need to auto adjust booking item's line subtotal/total ( because of booking dates changed ) before item is saved.

Closes #20467 .

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you successfully ran tests with your changes locally?

Changelog entry

Allow other plugins to be able to change $item object, for example to adjust line subtotal/total before its saved. 

My need for this change is because I have custom booking product item, and attached custom inputs for the item ( checkin / checkout dates etc ). So whenever those inputs are changed in order edit page and ajax save is triggered, I need to auto adjust booking item's line subtotal/total ( because of booking dates changed ) before item is saved.
@@ -251,6 +251,9 @@ function wc_save_order_items( $order_id, $items ) {
}
}

// Allow other plugins to change item object before it is saved.
do_action( 'woocommerce_before_save_order_item', $item, $item_data, $order_id );
Copy link
Member

Choose a reason for hiding this comment

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

Is item data and order id needed? Doesn't the item already contain these?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, $item already contain these, but usually related data is still passed to filters/actions. For example in add_order_item() function, you can find this filter:

$item    = apply_filters( 'woocommerce_ajax_order_item', $order->get_item( $item_id ), $item_id );

So even if $item has of course its ID, $item_id is still passed to that filter.

Copy link
Member

Choose a reason for hiding this comment

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

If it's not necessary I wouldn't include it. If we do need to add args in the future not accessible via $item, this makes it more difficult.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you think that this would be enough then?

do_action( 'woocommerce_before_save_order_item', $item, $order_id );

Copy link
Member

Choose a reason for hiding this comment

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

I don't even think order_id is needed - it's stored within the item.

@mikejolley mikejolley added needs: author feedback The issue/PR needs a response from any of the parties involved in the issue. and removed status: needs review labels Jun 15, 2018
@kloon kloon added status: needs review and removed needs: author feedback The issue/PR needs a response from any of the parties involved in the issue. labels Jul 10, 2018
Copy link
Member

@kloon kloon left a comment

Choose a reason for hiding this comment

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

LGTM

@kloon kloon added this to the 3.5.0 milestone Jul 11, 2018
@kloon kloon merged commit 7fa8020 into woocommerce:master Jul 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants