With IEF 1.5 and the widget: Inline entity form - Multiple values

Up until IEF 1.3 (at least), we were able to edit the price of a line item on a completed Drupal Commerce order. However, as of IEF 1.5, it seems that we can no longer see and edit a line item price.

Has this been disabled, or is there additional configuration we need to do now to get it to work?

The use case is an order that needs a manual adjustment after the fact.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

bojanz’s picture

Yes, it was removed in 1.5 because it wasn't working properly. It needs another round of tweaking.

dave bruns’s picture

Is it (reasonably) safe to use IEF 1.3 until this feature is back again?

bojanz’s picture

Well yes, but the next two releases had more than 20 major bug fixes :/
Still, if it works for you, go for it.

rszrama’s picture

Title: Can no longer edit price for a line item in a completed order? » Restore unit price edit support for line items
Version: 7.x-1.5 » 7.x-1.x-dev
Category: Support request » Feature request
Status: Active » Needs review
FileSize
3 KB

After a bit of diffing and testing, it appears to me the situation changed in these ways since 1.3:

  • Whereas 1.3 rebased the unit price every time the form was submitted, 1.5 only does it for non-product line items.
  • Whereas 1.3 reset the line item label to the product SKU, 1.5 does a full on product line item populate for product line items. This includes setting the unit price, blowing away any changes a user may have made. Perhaps this change came before the other and introduced the regression.
  • In both cases, core only rebases unit prices on order edit when it detects an actual change to the unit price data. Neither version did this.

The attached patch restores access to the unit price on the inline entity form. It also adds a helpful touch by providing a description to the amount textfield indicating that cart order unit prices will be automatically generated, hence the disabled fields.

I'm going to assume that the full line item populate is necessary, so what I've done is added a line of code on either side of that to preserve the unit price as submitted on the inline entity form. I then load the unchanged line item and compare the unit price from that to the unit price entered on the form. If the amount or currency code changed, IO rebase the unit price.

I've tested this with a product line item with and without sales tax and it rebased fine. A shipping line item rebased just fine, too. I'd appreciate a sanity check on this, but unless there's some other problem I don't know about, this seems ready to go.

rszrama’s picture

Discovered in a follow-up test that I didn't properly check if the line item was actually pre-existing. In the event that we're adding a new line item, we now skip the unchanged entity check and always rebase.

dave bruns’s picture

@rszrama - thanks for the super-quick patch! We've got this installed on a live server and so far edited a line item in one order with no issues. I will post here if we run into any trouble. If there is anything in particular you want us to try, please let me know.

rszrama’s picture

The only issue I'm running into has to deal with editing / creating new shipping line items; I don't think it has anything to do with this patch, but if you're running shipping it'd be cool to get a confirmation.

bojanz’s picture

Just a note to myself, the patch needs to be modified before committing to do a module_exists('commerce_cart') instead of module_exists('commerce_order'). The original line in 1.4 was

if (!empty($form_state['commerce_order']) && module_exists('commerce_cart')) {
cobenash’s picture

@rszrama The unit price can be edit in this patch.
But when i add the new line item in a new order, the unit price default value is blank.
Could your please let the default price as default value in the unit price?

rszrama’s picture

Status: Needs review » Needs work

That's really a disconnected thing, but it might be possible. I've also noticed that in a shopping cart status, this patch may not work since the unit price is disabled. Marking needs work until I can confirm / fix.

Anybody’s picture

Does this patch also cover the re-calculation of a lineitem based on the related rules event?
Perhaps it would make sense to trigger it?

Some related snippets:

rules_invoke_event('commerce_product_calculate_sell_price', $line_item);
// Now calculate the total price and save the order again.
    commerce_order_calculate_total($order);
    $order_wrapper->save();

(This is just a question - no criticism, I'm so happy about your module!!)

Anybody’s picture

Is there any progress so far?
We have been running into this problem several times now and I think there are lots of customers that don't understand why the price is being reset to zero, when they edit a line item.

I will dig deeper into this now but I could need some help, especially from someone with a lot of know-how in this area, like rszrama.

Anybody’s picture

The patch from #5 works great in the latest .dev version and is a major improvement from my point of view.
- Adding
- Deletion
of line item works absolutely fine!

The only thing that does not work well is updating. Here I have two issues I could not really track down:

1. I have an option list (multiple checkboxes) in my line item. If I unselect one and select another and save the values afterwards both values are selected (so the unselect does not work)

2. Changing the commerce_unit_price does not work at all. On adding a new line item everything works fine, but when I change a commerce_unit_price value this change is not being saved.
The strangest thing about it is, that the new value is not even set in the $form_state array. It contains the original value (in both the beginning of entityFormSubmit and entityFormValidate).

It would be very very cool if we could get this working and improve the next .dev release with this.

Anybody’s picture

Additional information: This is really curious: $form_state['values'] contains 4 values (the unchecked plus 3 truely checked). Any idea where this behaviour may result from?

I think the edit functionality is the last important step.

Anybody’s picture

Forget about #13 and #14. I found the reason and it was a damn JavaScript library copying the form like crazy!

Regarding #11 I can confirm that the price is being set correctly. The only problem I have with it is related to the use of commerce_vat. It complains that an array value "rate" is missing.
The exact message is: "Undefined index: rate in commerce_var_commerce_line_item_rebase_unit_price (line 523 of commerce_vat/commerce_vat.module)".
No VAT value is being calculated. This may not be related, see #2366943: Notice: Undefined index: rate in commerce_vat_commerce_line_item_rebase_unit_price() (line 535 of sites/all/modules/contrib/commerce_vat/commerce_vat.module).

But I think that is just a BONUS. Firstofall it would be more important to have line item editing working again at all. Then we can focus on these edge cases.

Everything works fine for me now (create / edit / delete). Only #10 was not tested by me.
So from my point of view this should be finally tested an get commited ASAP to have it back working :)

Thanks a lot for your great great work!!

entrepreneur2000’s picture

#5 works like a charm, thanks a lot for sharing.

Anybody’s picture

@rszrama: I found something very very important that fixed a lot of problems for me:

At the end of

public function entityFormSubmit(&$entity_form, &$form_state){
//...
}

simply add the following call:

rules_invoke_event('commerce_product_calculate_sell_price', $line_item);

This calls all related rules which are important to calculate the line item and dependent order price.

For me it for example fixed:
- VAT calculation (based on commerce_vat / commerce_eu_vat) for the line item AND the order
- Specific line item calculations

Please help to test this improvement and create a patch. I think this is a very very important step.

nvahalik’s picture

@rszrama, the patch does explicitly disable the unit price when the order is in the shopping cart mode.

andyg5000’s picture

Category: Feature request » Bug report

I'm changing this to a bug report because not only does #2174995: commerce_product_line_item_populate() not called for product line item types disable the unit price, it also causes product line items to be recalculated even for completed orders. Admins should have the ability to edit product line items including the price after the sale. The bug can be reproduced by changing the price of a sku then editing the line item containing that sku.

Anybody’s picture

Status: Needs work » Needs review
FileSize
5.22 KB

Hi @all.

After a long period of time and a new DC project I think I finally found a hopefully good and robust solution that should fix all problems and requests from above!
It is based on all the previous patches and comments. #19 should also be fixed now.

So please test it and let's get this commited as soon as possible, if everything is alright :)

--

Now there is a further last task to solve with this patch, which is not yet included, because I have not yet found a way to do it:

This module is currently incompatible with the commerce_vat module which is commonly used. The reason is that commerce_vat currently uses the 'commerce_product_calculate_sell_price' rules action to trigger VAT calculation for the order. #2567509: Why using "commerce_product_calculate_sell_price" action (by default)?
When saving a line item change this rule is not being triggered of course because we would never want to reset the line item prices to product defaults!

Generally I think that problem should finally be solved in commerce_vat module, but I don't think that will happen in a few days. Furthermore it is possible that similar problems exist with other modules using that hook.

So I created a well tested and logically robust workaround as hook to trigger the price calculation for the changed line items and update the order total correctly this way. It works great but how can we get this into inline_entity_form to be only called if the commerce_line_item form was used?

/**
 * Implements HOOK_commerce_order_presave().
 */
function MY_MODULE_commerce_order_presave($entity) {
  // Ensure that price calculation is properly done for edited line items
  // and especially that commerce_vat module price calculation takes place.

  // HINT: Currently we only execute this if the commerce_vat module is enabled, because
  // we know about the compatibility issue. Please fill an issue if there
  // are other compatibility problems with other rules or modules.
  // Generally it should be OK to remove the module_exists check.
  if ($entity->type == 'commerce_order' && module_exists('commerce_vat')) {
    // This is a commerce order before save (insert / update):
    foreach (entity_metadata_wrapper('commerce_order', $entity)->commerce_line_items as $delta => $line_item_wrapper) {
      $line_item = $line_item_wrapper->value();
      // Set the unit price based on the custom values:
      commerce_line_item_rebase_unit_price($line_item);

      // Call calculation routines to create the correct VAT positions:
      rules_invoke_event('commerce_product_calculate_sell_price', $line_item);

      // Update the total price (this is required by commerce_order_calculate_total to set correct components in the order price):
      $unit_price = $line_item_wrapper->commerce_unit_price->value();
      $line_item_wrapper->commerce_total->amount = $line_item->quantity * $unit_price['amount'];
      $line_item_wrapper->commerce_total->currency_code = $unit_price['currency_code'];
      // Add the components multiplied by the quantity to the data array.
      foreach ($unit_price['data']['components'] as $key => &$component) {
        $component['price']['amount'] *= $line_item->quantity;
      }
      // Set the updated data array to the total price.
      $line_item_wrapper->commerce_total->data = $unit_price['data'];
    }
    // Calculate the new order total.
    commerce_order_calculate_total($entity);
  }
}

Currently I'm using this code in a separate module. One idea might be to add a flag to the line item which we can check in this hook.... but there may hopefully be better ways.

Final note: The attached patch works without this hook, but commerce_vat calculation will not work cleanly then for projects using both modules. The patch itself is clean and better than the current module implementation, so we may split this up into separate issues later on.

bojanz’s picture

Issue tags: +commerce-sprint
mglaman’s picture

Status: Needs review » Reviewed & tested by the community

I tested and this worked like a charm. I was able to make an order as an admin and modify unit price.

Anybody’s picture

Status: Reviewed & tested by the community » Needs review

I added a new and important patch which is based on my patch in #20. It

  • adds some fixes for commerce_vat.module (calls the required "commerce_product_calculate_sell_price" rule). The VAT values are now recalculated correctly based on the products VAT rate even if the price is changed
  • adds an option to disable the price edit funcionality for the IEF
  • adds a checkbox to restore the original price

Please review and provide feedback! :)

Status: Needs review » Needs work
mglaman’s picture

Anybody, please provide an interdiff.

Anybody’s picture

Sorry, interdiff attached.
We're using the version from #24 in several environments and it works great now. :)

Anybody’s picture

It simply fails because there are no tests. But there also were no before and I don't have the time currently... the code should be just fine like before.

scotta’s picture

@Anybody Patch from #24 above worked for me to an extent. I can edit the price of line items once an order has been placed and the status is processing. However, I can't edit the price of items before that, such as when they are still in the cart of a user or when creating an order for someone. I hope this helps.

scotta’s picture

I have tried all of the patches in the discussion and none work perfectly:

The patches from #5 and #20 allow the price to be editable when you are creating a new order but the patch in #24 loses this functionality. However neither #5 nor #20 have the default price showing up when creating a new order which I feel should be added because that is the normal price and editing the price is normally an exception.

I like the checkbox in #24 to revert to the original price, but I don't see the option to disable the edit price functionality.

b_sharpe’s picture

#24 works with the exception of setting a line item price to 0. I will submit a patch if I can get some time, but it's due to the submit handler using empty to check the price. The validate should check that and then we can assume in the submit its not:

      if (isset($line_item_values['line_item_price_reset']) && $line_item_values['line_item_price_reset'] == 0 && !empty($line_item_values['commerce_unit_price'][$language][0]['amount'])) {
        // If the unit price was manually set, change the line item unit price ::
        $line_item->commerce_unit_price = $line_item_values['commerce_unit_price'];
      }
matthiasm11’s picture

The patch from #24 works. I suppose b_sharpe is right too, but have not tested it with a line item price = 0.

pvdjay’s picture

Is anyone else seeing this behavior?

Using patch in #24

1. Click 'Edit' on a line item
2. Set a new unit price, click 'Update line item'
(New line item unit price is displayed and line item total is calculated properly)
3. Click 'Save order'
BUG -->(Line item total is correct, but the unit price has reverted back to original amount)
4. Click 'Edit' on the line item (as if to edit some other aspect of the line item--not the price)
(Unit price field is set to original amount)
5. Click 'Update line item'
(Line item price is changed back to original amount; manual unit price edit is lost.)