I have added a pretty basic custom checkout pane for people to select a charity. For some reason, however, it prevents checkout from happening, and I have no idea why. You enter all the information on the page correctly, and hit continue, and the page reloads as if there is an error...but there is no error.

If I disable the pane, checkout seems to work just fine.

I narrowed it down to the validate function, which did nothing.

function choose_charity_checkout_form_validate($form, &$form_state, $checkout_pane, $order) {

}

Apparently it gets choked up on it, however. I commented out the validate function, and everything seemed to work fine. Strange, because shouldn't the validate function return nothing anyway, and just call form_set_error if there is an error?

I have a one-page checkout setup, with the charity pane the last pane. There is only a billing address.

Comments

rszrama’s picture

rszrama commented

This is somewhat related to the other thread we have going on checkout form validation, but basically since we're calling validate handlers for checkout panes from the one central validate function, the pane validators aren't functioning exactly like normal form validate handlers. Because of the way we're trapping error messages, it may be we can devise an alternate way of detecting errors, but as written if a checkout pane has a validate callback function, it is expected to return TRUE or FALSE. If it doesn't exist, no sweat, but if it's there we're looking for that affirmative "there was an error" or "everything is clear."

The code in question is on lines 271-274 in commerce_checkout.pages.inc:

    if ($callback = commerce_checkout_pane_callback($checkout_pane, 'checkout_form_validate')) {
      // Give it a chance to process the submitted data.
      $validate &= $callback($form, $form_state, $checkout_pane, $order);
    }

Interestingly, this is one of the few places in Commerce that uses the bitwise assignment operator. You can trace the value of the $validate variable above to see why we're using it if you're interested (involves not allowing a TRUE to override a FALSE as set when other error messages are detected).

In any event, I'm not sure there's a bug here, as it's the documented behavior in commerce_checkout.api.php. It may leave it open to question, so a quick clarification would be to throw a "must" in there. Alternately, we could expand the code above to look for a return value of TRUE or FALSE before performing the bitwise operation on $validate.

jazzdrive3’s picture

jazzdrive3 commented
Category: bug » support
Status: Active » Fixed

Understood. Changing to support request. It might help someone else searching.

rszrama’s picture

rszrama commented

Ok, cool. To be honest, I wouldn't mind simplifying that code. There's no reason we can't make a return value from the validate callback optional, treating NULL as TRUE. Even better, though, would be to get it to work without a return value... but relying on checking for error messages from the pane created during the validation step may be too brittle / insufficient of a check.

I'm guessing that since I knew I needed a FALSE, I decided to require a TRUE to keep it explicit that a return value was expected. I feel like in Ubercart I had made it optional and was maybe reacting against that.

Status: Fixed » Closed (fixed)

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