Change order status to cart if the contents of the shopping cart is changed.

Since this depends on the other issue and I put it back to needs work, we'll need to wait for its inclusion before putting this one in. One observation, though, is that instead of hard coding an array of $changeable_order_types (which is a misnomer anyways, since it's order statuses), we should instead use the cart property of order statuses. This will make it consistent with the cart module:

  // Create an array of valid shopping cart order statuses.
  $status_ids = array_keys(commerce_order_statuses(array('cart' => TRUE)));

Alrighty, pulled this in and tested it. I also added support for deleting line items. I think these may need to be optimized and could benefit from the use of entity_metadata_wrapper(), but we can let it sit for now and see if any other edge cases turn up.

https://github.com/rszrama/drupalcommerce/commit/10a834cbd377a2f0849ec10...

Your first change is a no brainer and a good catch. I think you had it originally, or at least my subconscious thinks I saw that code before. ; )

For the second, though, I don't think the Cart module should be responsible for cleaning up a line item reference field's data when the line item is deleted. In fact, this is already happening in commerce_line_item_field_attach_delete() in the Line Item module. (Funny enough, last month I thought it was happening because of the foreign key declaration, but it was just because I'd already written other code to handle the cleanup.)

Any way to get a follow-up commit to this branch that removes the line item removal code and just keeps the cart if check?

Hm. Frankly, this whole idea makes me uncomfortable.

If anything all this should be rules-based, we should not make assumptions like this in the cart module.

Could we please remove all these hooks and replace them by rules?

It's precisely when we start hardcoding business assumptions that we fail. I can imagine *a lot* of use cases where changing the order status back to 'cart' is just not the right thing to do. For example: upsell during the checkout.

If I understand your use-case correctly, you want to make sure that "if the cart has been modified *after* the shipping calculation has been done, the order status is reset to the page of the shipping". This is completely different from the rule you got implemented. Nothing says that shipping calculation happens on the 'cart' stage. Nothing says that the order needs to be unconditionally reset to 'cart' as soon as it gets modified.

Could we please just revert all this and come up with a better solution? As far as I'm concerned this is currently a net regression is flexibility.

Mmm, true - thanks for the heads up, Damien. Perhaps this should've been tied more specifically to the Add to Cart form instead of just any old line item / order->line_items change. The only redeeming thing I see here is that it limits the status reset to orders in a status w/ the cart property set to TRUE (i.e. by default that's the initial checkout form step and the review step).

At present we can't write the exact some logic using Rules, because of our use of EntityFieldQuery to find orders referencing a particular line item in commerce_cart_commerce_line_item_presave(). We can turn that whole bit into an action I suppose. : ?

Perhaps we should instead consider using the Cart Rules events (plus add one for updating an item in the cart and one for removing an item from the cart) and say that we're really going to reset the status when those events occur? Really what we're going after is inappropriate user action, not administrator / code action... but that still might not be delicate enough if an upsell module decides to integrate commerce_cart_add_to_cart_form() on any but the first step of checkout. However... I suppose someone could just add a condition to exempt particular checkout steps if necessary as I think Damien hinted above?

Just a minor note to say that I've begun working toward an alternate solution by changing the add to cart form behavior a bit... basically we needed an API and Rules events for altering the products on a shopping cart order. Still more work to do here, but it's a start toward a purely Rules solution.

Tagging.

Alrighty, I removed the presave / delete hooks from the Cart module. They've actually been the source of a couple trickier bugs involving infinite loops since we were loading an order in a presave hook for orders... it was messy. There is now a default rule that resets the cart order status when a product is added to or removed from the cart. Additionally, since the cart module explicitly controls the cart form, there's a reset baked into the update cart button to "place" the order on that page and one for the checkout button to "place" it on the first checkout page (instead of cart, since Checkout doesn't depend on Cart).

I believe this brings a lot of sanity to the process and is obviously less buggy than our existing solution.

Commits:
https://github.com/rszrama/drupalcommerce/commit/9e932b53300a5cd4d89df50...
https://github.com/rszrama/drupalcommerce/commit/593688973394de683c6495a...