Version: 2.x (not yet available in list of choices)

I implemented the hook_commerce_coupon_add_coupon_ajax_alter() (to update the order total which is in a nonstandard place on my checkout page) -- and discovered that the $order object passed is outdated. The reason is that in commerce_coupon_pane_checkout_form()$form_state['order'] and $order are not the same object anymore, after you re-load $order... so $form_state['order'] is not refreshed with the newest discounts.

Fix attached. And I have a habit of re-commenting code that I don't understand at first/second/third sight, so there :)

---

A slightly related observation in the same (don't understand at first sight) category:

I thought "It's a real shame that the drupal_alter('commerce_coupon_add_coupon_ajax', $commands, $form_state['order']); does not pass $form as an argument! Because all I want to do on my alter is this:$commands[] = ajax_command_replace('#my-selector', drupal_render($form['my-custom-pane-thingy']['order_total']));
...and now I don't have $form, I need to reconstruct this form element myself!

After testing, it turned out that I would have to do that anyway, because my $form['my-custom-pane-thingy'] was outdated. (It is built before the commerce_coupon pane, i.e. with the old order.)

So I was wondering if your not passing $form as an argument to the drupal_alter was intentional, because $form cannot be trusted 100%? :)

CommentFileSizeAuthor
commerce_coupon-2.x-ajax-order.patch973 bytesroderik

Comments

dpolant’s picture

Not passing $form was just an oversight. Also updating $form_state['order'] with the newly loaded order seems logical. I committed both changes.

dpolant’s picture

Status: Needs review » Fixed
dpolant’s picture

I actually had to revert the part of this that replaces the formstate order object. It seems to cause conflicts with other modules that use this variable during checkout. You can always do a commerce_order_load in your hook to get the most up to date order.

roderik’s picture

Will do. I hadn't thought about it but I can see how that could somehow cause conflicts; sorry for moving the commerce_order_load().

Since passing an outdated order seems bad DX, I suggest

  • removing $form_state['order'] from your commerce_coupon_add_coupon_ajax_alter (or replacing it by $form_state, since that seems kinda customary / maybe someone needs data from it in certain conditions?)
  • documenting the fact that $form_state['order'] does not contain a just-added coupon so it should be re-loaded (and why), in the future api.php

Self edit: removing $form_state['order'] without passing $form_state instead, isn't very handy, as that doesn't leave much room to determine the order ID...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

yogaf’s picture

Version: 7.x-1.x-dev » 7.x-2.0-beta1