Allow modules to skip updates to line items in commerce_cart_order_refresh()

Hey Alan, definitely appreciate the patch and understand you're working with a legacy system. However, I don't think we want to introduce such a change at this time. In your case, if manual pricing through the Node Checkout module (I haven't played with it) is part of the pricing process, then what should happen is a pricing rule should be used to set the price during the normal product pricing phase. In other words, the line item price is properly reset to $0, but you should have a rule that then properly updates it to the administrator entered value on the node.

For a comparison use case, please refer to how we handle custom donation amounts with price fields on line items: http://www.drupalcommerce.org/videos/tutorials/donations-custom-line-ite...

The idea is we have a donation product with a base price of $0 but put a price field on the donation line item type where the customer enters the desired donation amount. We use a pricing rule to swap the custom donation amount out for that $0 amount during the pricing system. It seems like you could be doing the same thing without a need for this additional hook.

We can definitely re-evaluate the refresh process in Commerce 2.x, but in your case such a change should be unnecessary.

Ahh, ok. Sorry it took me so long to get back to ya.

How about something like this. It's not introducing a new hook, but leveraging an existing one. Gives more control over the refresh process, while keeping what's there the same.

Two points:

- I needed to roll this to 7.x-1.7, so patch is attached.
- The patch attached in #6 assumes that the module_involk_all returns a boolean, when in fact it returns an array. So I had to update that.

Wrong:

<?php 
    $refresh = module_invoke_all('commerce_cart_line_item_refresh', $cloned_line_item, $order_wrapper);
     if ($refresh) {
          continue;
     }
?>

Right:

<?php
     $refresh = module_invoke_all('commerce_cart_line_item_refresh', $cloned_line_item, $order_wrapper);
     if (in_array(FALSE, $refresh)) {
          continue;
     }
?>

Note that all patches must be against the latest -dev version of the module or the test bot is gonna choke.

Second this. Our use case is that we currently need to flag our non-product line items as product as they are standalone line items and we need to get them through the checkout process, which is otherwise not possible due to #1458766: API change: Allow other modules to verify that an order may proceed to checkout.

That works perfectly, except in some strange scenarios (right now, for us, it's only on the Paypal IPN callback, which is really unfortunate because we lose the payment information), this function is triggered, there is no product, so $product is NULL, it tries to set that and then entity wrapper explodes.

Moving the hook above that check to prevent this. Alternatively, we could add a check to only do the product price stuff if there is a product id.

Updated patch does that, fixed the return value check and comes in a version against 7.x-1.x and one for 7.x-1.7 and older.

Patch #10 works great on the current 1.x-dev build.

Hmm, this seems like a potentially breaking API change that may better be served by just adding an additional hook. Overloading the one hook currently used to facilitate third party updates to a line item during the refresh to also use it to prevent alterations to the line item just doesn't seem wise. Additionally, moving the hook invocation means any implementations of this hook currently designed to work after product pricing rules have been applied would break.

I think #1458766: API change: Allow other modules to verify that an order may proceed to checkout probably solved Berdir's immediate need, so I'm going to move this back to needs work and probably not address it until after 1.9 unless I'm wrong about holding you up there Berdir.

Yes, you are correct, fixing the other issue did solve my problem so I don't need this workaround anymore. Other people still might, for different reasons, though.

I als agree with your review, a new hook seems better.

Great, thanks for the feedback. Didn't wanna leave you hanging if you still needed this. : )

I'd like to chime in on this as I've been running into a similar type of need.

The patch in #10 works for my use case as far as I can tell at this point.

The situation is a non-profit that is not selling any product but is heavy with donations. I am aware of the other donation solutions using custom line items and modules like "Select or Other". In my case I just needed to be able to programmatically create a line item based on a donation product, set it's price and add it to the cart.

The commerce_cart_order_refresh function was always resetting the amount that I added to the product.

My initial attempts look like this as a submit function from a custom form:

function mymodule_donate_form_submit($form, &$form_state) {
  global $user;
  $amount = check_plain($form_state['values']['amount']); //needs work

  // create line item
  $product = commerce_product_load(1); //hard coded product id for now
  $product->commerce_price[LANGUAGE_NONE][0]['amount'] = $amount;
  $product->field_cost[LANGUAGE_NONE][0]['amount'] = $amount;
  $line_item = commerce_product_line_item_new($product);

  commerce_line_item_save($line_item);
  commerce_cart_product_add($user->uid, $line_item, FALSE);

  $form_state['redirect'] = 'checkout';
}

Then by implementing the hook_commerce_cart_line_item_refresh I can keep that product from being set back to the base price of the donation product which is $0.00

function mymodule_commerce_cart_line_item_refresh($line_item, $order_wrapper) {
  if($line_item->line_item_label == 'donation'){
    return FALSE;
  }
}

I won't say I fully understand Ryan's comment in #12. I feel like I am one of the "Other people that still might" and I'm hoping that there is a way to allow this kind of use case without causing other issues.

This was the first place that I got some traction on trying to accomplish this after being through many other threads elsewhere.

Same requirement here. We have to allow administrative users to edit the price when creating an order (via inline_entity_form). When saving the order commerce_cart_order_refresh() overrides the custom line item prices which is not wanted in this case.

@rszrama suggested to add a new hook in #12, I think that is a wise idea.

How can we work around that cleanly in the future? How can we proceed here? The solution in #10 is surely NOT general.

I see two possible options:

1. Provide an additional API hook to prevent price updates from code.
2. Add an option in line items to generally prevent price updates by a setting which is checked in commerce_cart_order_refresh().

Oppinions? :)

In your case, can you not just ensure the order settings are configured to only refresh the current user's cart on load? That would prevent administrative edits from triggering price calculation, wouldn't it?

I am looking for this as well. I am working with a custom module that I am using to split quantities into separate line items. The problem is that I am also using Commerce Pricing Attributes to alter the price when a customer adds the product to their cart, like a discount. My module at first works great, splits the qty into the # of line items needed and they all have the proper altered price. Then hitting refresh on the cart results in the added line items reverting back to their original price.

I wasn't sure what the problem was until I looked at the commerce_cart_order_refresh() hook and it appears that's causing the newly added line items to revert back to a different price based off of the original product price, not the altered product price.

Having the ability to stop the order refresh, or something similar is really import in this case.

Here is my question currently posted in Drupal Answers that shows my code and example: http://drupal.stackexchange.com/questions/182445/split-line-items-by-qty-and-retain-altered-price

Update: Thanks to Ryan's input, I was able to get this finally resolved in http://drupal.stackexchange.com/questions/182445/split-line-items-by-qty-and-retain-altered-price

w00t w00t, glad I could help. : )

Given the age on this issue and the availability of workarounds, I'm going to "won't fix" this to avoid introducing regressions while 1.x has essentially moved to maintenance mode. It's worth noting that we did change this logic in 2.x to work much closer to what folks were needing.