If you implement hook_commerce_checkout_pane_info_alter() and change the page a pane is assigned to, that will override UI settings.

Note here that the UI settings are gathered first ($saved_panes) but then the drupal_alter() will blow them all away.

    foreach (module_implements('commerce_checkout_pane_info') as $module) {
      foreach (module_invoke($module, 'commerce_checkout_pane_info') as $pane_id => $checkout_pane) {
        $checkout_pane['pane_id'] = $pane_id;
        $checkout_pane['module'] = $module;

        // Update the pane with saved data.
        if (!empty($saved_panes[$pane_id])) {
          $checkout_pane = array_merge($checkout_pane, $saved_panes[$pane_id]);
          $checkout_pane['saved'] = TRUE;
        }

        $checkout_panes[$pane_id] = $checkout_pane;
      }
    }

    drupal_alter('commerce_checkout_pane_info', $checkout_panes);

Comments

rszrama’s picture

Status: Active » Closed (works as designed)

I'm not sure this is a bug. It seems preferable to let developers ignore UI input as opposed to UI input preventing certain types of alterations. For example, you might have a site where you need to alter the normal checkout flow based on the products in the cart (such as removing the shipping pane if no shippable items are in it). If you can't override UI data with the hook, this becomes impossible.

In this case, we're treating settings from the form as default properties of the pane, letting them be overridden as necessary by the later alter hook.

rfay’s picture

Status: Closed (works as designed) » Active

That definitely violates the idea of alters(). They're supposed to be able to do the same thing as the original info hook, and then have processing from there. You're the arbiter of this, and can put it to closed, but I'm setting to active one time.

To restate, as far as I understand in Drupal,
* The idea of any info-type hook is to provide base information.
* The idea of any alter hook is to alter that information provided by the info hook.
* Processing of the information happens after both of these have taken place (after we've gathered what *everybody has to say about the info array)

rszrama’s picture

How would you propose a solution to the use case I mentioned above?

rszrama’s picture

(Also, the point I was making is that info from the settings form is equivalent to info from the info hook, because that's where the data originally comes from. The settings form data inclusion operates like a first level alter of the base info.)

rfay’s picture

What yo proposed in #1 would need another hook, with a different name, expressing what it intended to do (to lock a pane) rather than to alter the pane info.

rszrama’s picture

I'm not sure what you mean by "lock a pane." The more important point I guess was #4 - I should've posted that comment first. : )

rfay’s picture

Sorry, by "lock a pane" I really meant "override anything done with the pane in the UI".

Really, the developer's expected process is 1) info, 2) alter, 3) process based on result.

I don't disagree about the need to accomplish other functionality, but it shouldn't be done with things that are expected to behave predictably IMO.

amateescu’s picture

Category: bug » task
Status: Active » Postponed

I agree with @rfay here, but changing the order at this point would be an API change which we probably don't want in Commerce 1.x. Moving as a task to be taken into consideration for 2.x.

rszrama’s picture

Title: hook_commerce_checkout_pane_info_alter() overrides UI settings for panes » Resolve order of info alter hooks in relation to UI settings
Component: Cart » Developer experience

When we come back to this, let's make it a bit more broad look at any info structure that may be configured via the UI and altered. Per #1855900: Move invocation of drupal_alter in commerce_checkout_pages() so that defaults are filled in first, this same discussion applies at least to checkout pages as well and possibly other info structures.

bojanz’s picture

Issue summary: View changes
Status: Postponed » Closed (won't fix)

If this didn't happen in 2013, then late 2018 is certainly too late.