Change the line item quantity update handler to queue changes instead of sequentially evaluating them

I added a drupal message action to output the line items and the order:

{ "rules_add_product_on_new_line_item" : {
    "LABEL" : "Add Product on New Line Item",
    "PLUGIN" : "reaction rule",
    "REQUIRES" : [ "commerce_cart", "rules", "php", "entity" ],
    "ON" : [ "commerce_line_item_insert" ],
    "IF" : [
      { "commerce_order_is_cart" : { "commerce_order" : [ "commerce-line-item:order" ] } }
    ],
    "DO" : [
      { "commerce_cart_product_add_by_sku" : {
          "USING" : {
            "user" : [ "site:current-user" ],
            "sku" : "TPS-TP6-TUT-LPD",
            "quantity" : "1",
            "combine" : 1
          },
          "PROVIDE" : { "product_add_line_item" : { "product_add_line_item" : "Added product line item" } }
        }
      },
      { "variable_add" : {
          "USING" : { "type" : "commerce_order", "value" : [ "commerce-line-item:order" ] },
          "PROVIDE" : { "variable_added" : { "added_order" : "Added order" } }
        }
      },
      { "drupal_message" : { "message" : "MANUALLY ADDED LINE ITEM:\r\n\u003c?php echo(\"\u003cpre\u003e\" . print_r($commerce_line_item, true) . \"\u003c\/pre\u003e\"); ?\u003e\r\nAUTOMATICALLY ADDED LINE ITEM:\r\n\u003c?php echo(\"\u003cpre\u003e\" . print_r($product_add_line_item, true) . \"\u003c\/pre\u003e\"); ?\u003e\r\nCART:\r\n\u003c?php echo(\"\u003cpre\u003e\" . print_r($added_order, true) . \"\u003c\/pre\u003e\"); ?\u003e" } }
    ]
  }
}

I have attached a screenshot of the output. You can see that the line item is created and it has the correct order ID, but that doesn't seem to effect what is actually in the order.

I am wondering if the real issue is that the line item is discretely failing to save in some contexts. Here is another rule that does not work under certain conditions:

{ "rules_test_remove_product" : {
    "LABEL" : "Test remove product",
    "PLUGIN" : "reaction rule",
    "TAGS" : [ "Volume Discount" ],
    "REQUIRES" : [ "rules", "commerce_cart", "php" ],
    "ON" : [ "commerce_cart_product_remove" ],
    "IF" : [
      { "entity_is_of_type" : { "entity" : [ "commerce-product" ], "type" : "commerce_product" } }
    ],
    "DO" : [
      { "commerce_cart_product_add_by_sku" : {
          "USING" : {
            "user" : [ "site:current-user" ],
            "sku" : [ "commerce-product:sku" ],
            "quantity" : "1",
            "combine" : 1
          },
          "PROVIDE" : { "product_add_line_item" : { "product_add_line_item" : "Added product line item" } }
        }
      },
      { "LOOP" : {
          "USING" : { "list" : [ "commerce-order:commerce-line-items" ] },
          "ITEM" : { "list_item" : "Current list item" },
          "DO" : [
            { "component_rules_test_subtract_1_from_other_line_items" : { "product" : [ "commerce-product" ], "line_item" : [ "list-item" ] } }
          ]
        }
      },
      { "drupal_message" : { "message" : "ORDER:\r\n\u003c?php echo(\"\u003cpre\u003e\" . print_r($commerce_order, true) . \"\u003c\/pre\u003e\"); ?\u003e\r\n\u003c?php\r\nforeach($commerce_order-\u003ecommerce_line_items['und'] as $line){\r\n  $temp_line_item = commerce_line_item_load($line['line_item_id']);\r\n  echo(\"\u003cpre\u003e\" . print_r($temp_line_item, true) . \"\u003c\/pre\u003e\");\r\n}\r\n?\u003e" } }
    ]
  }
}

Here is the component referenced:

{ "rules_test_subtract_1_from_other_line_items" : {
    "LABEL" : "Test subtract 1 from other line items",
    "PLUGIN" : "rule",
    "TAGS" : [ "test" ],
    "REQUIRES" : [ "rules" ],
    "USES VARIABLES" : {
      "product" : { "label" : "Product", "type" : "commerce_product" },
      "line_item" : { "label" : "Line Item", "type" : "commerce_line_item" }
    },
    "IF" : [
      { "entity_is_of_type" : { "entity" : [ "product" ], "type" : "commerce_product" } },
      { "entity_is_of_type" : { "entity" : [ "line-item" ], "type" : "commerce_line_item" } },
      { "data_is" : { "data" : [ "line-item:type" ], "value" : "product" } },
      { "NOT data_is" : {
          "data" : [ "line-item:commerce-product:product-id" ],
          "value" : [ "product:product-id" ]
        }
      }
    ],
    "DO" : [
      { "data_set" : {
          "data" : [ "line-item:quantity" ],
          "value" : { "select" : "line-item:quantity", "num_offset" : { "value" : "-1" } }
        }
      }
    ]
  }
}

What this should do is as follows: when a user removes a product from his cart, that product should be put back into the cart (this part works by the way), then it should remove 1 from the quantity of all other line items. When you click the "remove" button in the cart, all works as planned, but not if you update the quantity of a product to 0. You will see in the output that the quantities of the line item objects do get updated, but they don't ever seem to be written to the database because the cart still has the old quantity for the supposedly decremented line item(s).

So, to sum up, the above rule manipulates the line item entities properly whether a user removes a product by clicking "remove" or by updating the quantity to 0, but the entity only gets saved to the database under the first scenario. Obviously, I've tried adding actions to save the line item entity and order to no avail.

I am just going to keep on hitting this from different angles.

{ "rules_test_remove_product" : {
    "LABEL" : "Test set quant",
    "PLUGIN" : "reaction rule",
    "TAGS" : [ "Volume Discount" ],
    "REQUIRES" : [ "commerce_checkout", "rules", "commerce_cart" ],
    "ON" : [ "commerce_cart_product_remove" ],
    "IF" : [
      { "entity_exists" : {
          "type" : "commerce_line_item",
          "property" : "line_item_id",
          "value" : "507"
        }
      }
    ],
    "DO" : [
      { "entity_fetch" : {
          "USING" : { "type" : "commerce_line_item", "id" : "507" },
          "PROVIDE" : { "entity_fetched" : { "entity_fetched" : "Fetched entity" } }
        }
      },
      { "data_set" : { "data" : [ "entity-fetched:quantity" ], "value" : "7" } },
      { "entity_save" : { "data" : [ "entity-fetched" ], "immediate" : 1 } }
    ]
  }
}

To reproduce:

1. Add 2 products to your cart
2. Create and activate the above rule, replacing the number "507" with the ID of one of the product line items in your cart
3. Remove the other item from your cart by updating the quantity to 0

If the rule worked as I would expect it to the quantity of the remaining item will be updated to 7 and this is exactly what happens if you click "remove" instead of setting the quantity equal to 0.

Now I have looked at the MySQL log to see what is happening and it looks like the issue is that the line item is saved, but then it is saved again with the old values.

With the rule I put in #4 I found that when i click on the "remove" button (thus triggering the rule) the following SQL command was run (I changed the quantity to 7777 to make it easier to find):

UPDATE commerce_line_item SET order_id='3', type='product', line_item_label='7NN-P2-NEW-VXXZ-BE', quantity='7777', created='1324581457', changed='1324586249', data='a:0:{}'
WHERE  (line_item_id = '507')

All is right with the world, that is what we would expect. But then I changed the quantity to 8888 and tried it again, this time triggering the rule by updating the quantity in the cart to 0 and this is what i got on line 152

UPDATE commerce_line_item SET order_id='3', type='product', line_item_label='7NN-P2-NEW-VXXZ-BE', quantity='8888', created='1324581457', changed='1324586332', data='a:0:{}'
WHERE  (line_item_id = '507')

Looks good, but then... disaster on line 423:

UPDATE commerce_line_item SET order_id='3', type='product', line_item_label='7NN-P2-NEW-VXXZ-BE', quantity='7777', created='1324581457', changed='1324586332', data='a:0:{}'
WHERE  (line_item_id = '507')

So the line item is saved once under the working context and twice under the broken one, once with new data and once with old.

mrfelton, You may be right. I have found no workaround and, in fact I have not documented this here, but I have been over this with numerous rules triggers and I had the whole thing in a custom module for a while and I have come across the same problem with line items. Sometimes they don't want to save (or are being saved over).

I would be interested to know if your line item is being saved properly to that database and if not, is it being overwritten as mine seems to be?

I may try to implement this again in a module and document the issues here.

OK, so I have implemented this in PHP. Still no love. I am surprised that this issue is not halting development on hundreds of sites. Under certain conditions line items do not save. Seems pretty important to me.

Anyway, the code below is in a module called "nv_commerce_helper_functions". To test this yourself all you need to do is:

1. create a module with that name and past in this code, or implement hook_commerce_cart_product_remove yourself
2. Then add multiple products to your cart
3. View your cart and remove a product by clicking "remove" (note the behavior is as expected, the remaining line items had their quantities updated to 4444)
4. Start over with new products in the cart and remove one of them, this time by setting the quantity to 0 and clicking "Update cart" (note the unexpected behavior, the remaining line items retain their original quantities)

/**
 * implementation of hook_commerce_cart_product_remove
 */ 
function nv_commerce_helper_functions_commerce_cart_product_remove($order, $product, $quantity, $line_item){
  // load the line items from the order
  $order_wrapper = entity_metadata_wrapper('commerce_order', $order);
  
  // Loop through the line items
  foreach ($order_wrapper->commerce_line_items as $line_item_wrapper) {
    $iterated_line_item = $line_item_wrapper->value();
    // Change the quantities of all line items to some number
	$iterated_line_item->quantity = 4444;
	$result = commerce_line_item_save($iterated_line_item);
	
	// Just to let us know that the function was called
    drupal_set_message("Line item saved with result: " . $result);
  }
}

So to sum up, changes made to line items are unsavable when removing a product from the cart by updating the quantity to 0.

Checked the database log and the same thing is happening here with the PHP implementation that was happening with the rules implementation.

To set up, I put two products in my cart, one with a quantity of 1 and another with a quantity of 121. Then I removed the product with quantity 1 by clicking "remove". The remaining line item was updated with the quantity of 4444 as we would expect. The attached "click_remove.txt" file is the MySQL log from that action. Note lines 146 and 147:

17 Query UPDATE commerce_line_item SET order_id='3', type='product', line_item_label='7NN-P2-NEW-VXXZ-BE', quantity='4444', created='1325532745', changed='1325532764', data='a:0:{}'
WHERE  (line_item_id = '644')

The I reset again with 2 products of quantities 1 and 121. This time I removed the product with quantity 1 by setting the quantity to 0 and clicking "Update cart". The attached "update_to_zero.txt" is the MySQL log from this action. Note lines 146 and 147:

32 Query UPDATE commerce_line_item SET order_id='3', type='product', line_item_label='7NN-P2-NEW-VXXZ-BE', quantity='4444', created='1325532851', changed='1325532865', data='a:0:{}'
WHERE  (line_item_id = '646')

But then on lines 418 and 419:

32 Query UPDATE commerce_line_item SET order_id='3', type='product', line_item_label='7NN-P2-NEW-VXXZ-BE', quantity='121', created='1325532851', changed='1325532865', data='a:0:{}'
WHERE  (line_item_id = '646')

So, the line item was updated with the new value and then overwritten with the old value again. This is a bug right? I guess the line item is being loaded elsewhere before the hook hook_commerce_cart_product_remove is called and then saved after.

The problem seems to be in the function defined on line 75 of the commerce_line_item_handler_field_edit_quantity class in commerce/modules/line_item/includes/views/handlers/commerce_line_item_handler_field_edit_quantity.inc.

function views_form_submit($form, &$form_state) {
    $field_name = $this->options['id'];
    foreach (element_children($form[$field_name]) as $row_id) {
      $line_item_id = $form[$field_name][$row_id]['#line_item_id'];
      // If the line item hasn't been deleted...
      if ($line_item = commerce_line_item_load($line_item_id)) {
        // And the quantity on the form is different...
        if ($form_state['values'][$field_name][$row_id] != $line_item->quantity) {
          if ($form_state['values'][$field_name][$row_id] == 0) {
            // If the quantity specified is 0, the line item is deleted.
            $order = commerce_order_load($form_state['order']->order_id);
            commerce_cart_order_product_line_item_delete($order, $line_item_id);
          }
          else {
            // Change the quantity and save the line item.
            $line_item->quantity = $form_state['values'][$field_name][$row_id];

            commerce_line_item_save($line_item);
            entity_get_controller('commerce_line_item')->resetCache(array($line_item->line_item_id));
          }
        }
      }
    }
  }

As the logic is written, each line item is loaded, then if the quantity submitted in the form is different from the quantity in the loaded line item, it is then either deleted (if the quantity is 0) or updated with the quantity submitted from the form. The problem with this is that it allows developers to change the quantities of line items that come before the deleted line item through hook_commerce_cart_product_remove, but we cannot change the quantities of line items that come after (I am referring to the order the line items display in the Shopping cart view).

The distinction between line items that can be updated and line items seems arbitrary, and I think that the order that items appear in the cart should not govern whether they are updatable through hook_commerce_cart_product_remove.

I think the solution is to load all the line items and submitted quantities before looping through the line items and updating the quantities, that way, you can be sure you are comparing the original quantity to the submitted quantity and not comparing the quantity calculated by hook_commerce_cart_product_remove to the submitted quantity. I don't mind having my changes to a line item overwritten by a value submitted by the user, but if they do not submit a new value I should be able to calculate the quantity using hook_commerce_cart_product_remove regardless of the sort order of the line items in the cart.

I would be interested to know if anyone else thinks my assessment is correct. If so, I'll make a patch.

Here is a little background for why this is important to me. In the store I am building, we have "full products" and "volume products". A user needs to have 1 full product in the cart o add any number of the volume products, but we want this to be as transparent as possible to the user, so, he adds 5 full products to his cart and I replace 4 of them with the volume product. This works, but when he removed the full product, I can't just leave the volume products in there, I want to add the full product back and instead remove one of the volume products. This part does not work in a consistent way. The results are different depending on whether he clicks "remove" or sets the quantity to zero.

I've made a patch. What it does is preload the changes to the cart before applying any changes. That way you are free update quantities with hook_commerce_cart_product_remove with consistent results.

rszrama, Here is a rule that will demonstrate the issue:

{ "rules_test_remove_product" : {
    "LABEL" : "Test set quant",
    "PLUGIN" : "reaction rule",
    "TAGS" : [ "Volume Discount" ],
    "REQUIRES" : [ "rules", "commerce_cart" ],
    "ON" : [ "commerce_cart_product_remove" ],
    "DO" : [
      { "LOOP" : {
          "USING" : { "list" : [ "commerce-order:commerce-line-items" ] },
          "ITEM" : { "list_item" : "Current list item" },
          "DO" : [ { "data_set" : { "data" : [ "list-item:quantity" ], "value" : "7" } } ]
        }
      }
    ]
  }
}

We would expect it to change the quantities of all remaining line items in a cart to 7 when a line item is removed from the cart. But if you:

1. Add 2 items to the cart
2. Change the quantity of the 1st item to 0 and update the cart

The quantity of the remaining item will not be updated to 7 (as we would expect). But if you add the 2 items to the cart and change the quantity of the 2nd one to 0 then the remaining item quantity will be updated to 7.