Hello,

The idea is to get checkout panes exportable, so that a feature can contain settings about checkout panes (page in which one pane appears, weight, collapse settings).
I will provide a patch soon.

Regards,

David

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

David Stosik’s picture

Soon meaning immediately. ^^
I used the "faux-exportable" pattern, as it felt like I would not be able to use the normal way.
That means that in-feature checkout panes are written to database when the feature is enabled or reverted.

Any comment or advice is welcome.

This patch was developed by Alethia INC. and sponsored by The Japan Association for Language Teaching (JALT).

David Stosik’s picture

Status: Active » Needs review

Patch still works on 1.0-rc1. :)

Thanks for your help.

dysrama’s picture

Hi
This sounds very good indeed, and just what I need, but after applying the patch I see no changes in features? No new area or anything.
A guideline to steps to take to make this patch work would be appreciated.

sarvab’s picture

The latest version of features requires the commerce_checkout_panes call here to accurately reflect the panes saved to the DB by resetting the static cache. I've submitted a patch over at commerce and am updating the patch here to make this work with features 2.0

The only difference is:

-  $panes = commerce_checkout_panes();
+  $panes = commerce_checkout_panes(array(), FALSE, TRUE);

The issue + patch required in commerce https://drupal.org/node/2137553

bojanz’s picture

Status: Needs review » Needs work

Also, commerce_checkout_panes() returns both the panes from the database and the panes that are already in code (commerce_checkout_pane_info). We need only the db ones, so that will require writing a custom function.

logaritmisk’s picture

Status: Needs work » Needs review
FileSize
3.24 KB

Didn't know about this module and this patch so I made my own features implementation, but now I have tried to merge my code with this patch and it seems to work. Install/revert works like a charm and I have tested against Commerce 1.8 and 1.9.

sarvab’s picture

Here is a rebuild of the latest patch that includes the new commerce_checkout_panes_reset call to ensure up-to-date panes on revert.

Also the hooks hook_features_enable_feature and hook_features_disable_feature were added back in too.

discipolo’s picture

for some reason the patch from #7 didnt apply all lines for me. so i rebuilt it again

lucastockmann’s picture

I successfully tested the patch from #8. (With latest dev)
U have to clear the cache, after that everything works as expected.

Use: drush fe [feature_name_here] %commerce_checkout_pane%:%% to export your settings.

joelpittet’s picture

Status: Needs review » Reviewed & tested by the community

Seems to do the trick. Marking as RTBC.

I exported most of my checkout panes to code. Made some changes and updated the feature. And deployed it successfully to staging.

Maybe a follow-up would be to save some of their config forms too?

mqanneh’s picture

with patch #4
enabling the features generates a PDO exception
duplicate primary key

I checked the table and found that the checkout panes settings is inserted, so the problem is that the feature is trying to insert the settings 2 times

with patch #8
this problem was solved

dmsmidt’s picture

+1 for getting #8 in, works like a charm.

joelpittet’s picture

Status: Reviewed & tested by the community » Needs work

I've reverted this patch as it doesn't seem to update the features well when changes are made. Feel free to mark it as RTBC if you feel different.

BR0kEN’s picture

Here is the working patch, used on my project :)

P.S. We need to enable automated testing for this project.

BR0kEN’s picture

Sorry, I've accidentally remove necessary code. Brought it back.

m1r1k’s picture

Status: Needs review » Needs work
  1. +++ b/commerce_checkout_panes.features.inc
    @@ -0,0 +1,108 @@
    +  $panes = commerce_checkout_panes();
    

    You have removed two manual dependencies:
    + $export['dependencies']['features'] = 'features';
    + $export['dependencies']['commerce_checkout'] = 'commerce_checkout';

    Features are clever enough to add them selves as a dependency, when I am not sure about commerce_checkout.

  2. +++ b/commerce_checkout_panes.features.inc
    @@ -0,0 +1,108 @@
    +  return array(COMMERCE_CHECKOUT_PANES_DEFAULT_HOOK => implode(PHP_EOL, $code));
    

    PHP_EOL will use "\r\n" if we will update feature on windows - do we need this modification?

BR0kEN’s picture

  1. No need to add features as dependency for feature.
  2. commerce_checkout will be added automatically.
  3. Agree, bring it back.
BR0kEN’s picture

Status: Needs work » Needs review
FileSize
6.89 KB
413 bytes
m1r1k’s picture

Status: Needs review » Reviewed & tested by the community
DamienMcKenna’s picture

+1, thanks for all of the effort to make this work!

DamienMcKenna’s picture

rooby’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
6.89 KB
481 bytes

I think that using the pane title is problematic because they are more likely to be less useful in an administration situation. For example the title may be empty or multiple panes may have the same title.

As an example the Commerce Views Pane module will give an empty title if the view has no title.

Instead I think we should use the pane name like this.

BR0kEN’s picture

Agree with you, @rooby, but want to use title anyway.

Xano’s picture

Status: Needs review » Needs work
  1. +++ b/commerce_checkout_panes.features.inc
    @@ -0,0 +1,108 @@
    +define('COMMERCE_CHECKOUT_PANES_INTEGRATION_NAME', 'commerce_checkout_panes');
    +define('COMMERCE_CHECKOUT_PANES_DEFAULT_HOOK', 'commerce_checkout_panes_default');
    
    +++ b/commerce_features.module
    @@ -54,26 +45,34 @@ function commerce_features_features_api() {
       if (module_exists('commerce_customer')) {
    

    Docblocks must be added for these constants to explain what they are for.

  2. +++ b/commerce_checkout_panes.features.inc
    @@ -0,0 +1,108 @@
    +    $options[$pane_id] = sprintf('%s (%s)', $pane['title'], $pane['name']);
    

    Nice solution.

  3. +++ b/commerce_checkout_panes.features.inc
    @@ -0,0 +1,108 @@
    +    $export['dependencies'][] = $panes[$pane_id]['module'];
    

    commerce_checkout must be added as well.

  4. +++ b/commerce_checkout_panes.features.inc
    @@ -0,0 +1,108 @@
    +  $schema = drupal_get_schema('commerce_checkout_pane');
    

    This prevents us from exporting any chckout pane configuration stored by other modules, since commerce_checkout_panes() runs all data (even that from the DB) through hook_commerce_checkout_pane_info_alter(). Is this really what we want?

  5. +++ b/commerce_checkout_panes.features.inc
    @@ -0,0 +1,108 @@
    +function commerce_checkout_panes_features_revert($module_name) {
    +  $defaults = module_invoke($module_name, COMMERCE_CHECKOUT_PANES_DEFAULT_HOOK);
    +
    +  if (!empty($defaults)) {
    +    commerce_checkout_panes_reset();
    +    $existing = commerce_checkout_panes();
    +
    +    foreach ($defaults as $pane_id => $pane) {
    +      $pane['saved'] = isset($existing[$pane_id]['saved']) ? $existing[$pane_id]['saved'] : FALSE;
    +
    +      commerce_checkout_pane_save($pane);
    

    Can't we do this by simply removing the override records from the DB?

ruloweb’s picture

Hi all,

just updated the patch #23 to work on last dev, which allows payment export as well.

Thanks!

joelpittet’s picture

Status: Needs work » Needs review

@ruloweb does it address the feedback in #24? Could you provide an interdiff?

BR0kEN’s picture

@joelpittet, no, it doesn't.

@Xano,
1. I don't understand why constants should be documented when they are have understandable names.
3. Agree, this is potential issue.
4. Yep. Any alterations should be in code.
5. This should be as is.

P.S. Damn, let's have this merged! Half of year we're talking about nothing and do nothing. I'm very disappointed in Drupal community because it ready to do something when maintainer interested in this.

Ronino’s picture

#27 works for me, thanks!

DamienMcKenna’s picture

I'm hitting a problem with #27 this when it's used in an installation profile, the site ends running commerce_checkout_pane_save() on a commerce_fieldgroup_pane that's already in the database. More details shortly.

DamienMcKenna’s picture

So the problem is that commerce_checkout_panes_features_revert() is being triggered twice on the same process execution - the first time via hook_features_rebuild(), the second via hook_features_rebuild().

DamienMcKenna’s picture

Both hook_features_rebuild() and hook_features_rebuild() are triggered by features_modules_enabled(), so it's something in this patch that's misbehaving.

DamienMcKenna’s picture

So it turns out that there was no need to add hook_features_enable_feature().

DamienMcKenna’s picture

This changes all occurrences of $module_name with the shorter $module, for consistency sake.

DamienMcKenna’s picture

Issue tags: +SprintWeekend2016
geek-merlin’s picture

Status: Needs review » Reviewed & tested by the community

Patch #33 applies well and does what it announces here.

mglaman’s picture

#33 looks good. I'd like a few other issue subscribers to test, then will commit.

mglaman’s picture

Issue tags: +commerce-sprint

Tagging for sprint, see if some AcroMedia contributors can give it a run.

pontus.froden’s picture

Confirm patch #33 works.

All on checkout panes from dev site was transferred to staging without any problem.

Changed the order on dev some more times and everything was transferred to staging when pushed.

Votes up for commit :)

  • mglaman committed 1777646 on 7.x-1.x authored by DamienMcKenna
    Issue #1973602 by BR0kEN, DamienMcKenna, sarvab, rooby, David Stosik,...
mglaman’s picture

Status: Reviewed & tested by the community » Fixed

Thanks everyone :) Committed!

Status: Fixed » Closed (fixed)

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