Features integration contribution for uc_product product classes (content types)

Uploading as a text file for easier reviewing.

And here it is in patch form. Nice!

Note: Credit on this patch goes to wizonesolutions, not me!

I actually think this is worth of review (and possibly commit) all on its own, and product classes could be handled in a follow-up issue.

For those who are more visual, here is what this patch adds to Features module:

Great screenshot webchick.

The patch works, it adds the UI when you create a new features.

This is a good start, but I find that this doesn't really do anything useful. Problems found:
#1. With Strongarm, when you add a content type, it auto populates all of its settings. This does not.
#2. When you select a product, on the right hand side it adds 'Ubercart product types', and the product type underneath it. Once you downloaded and installed this new feature, the 'Ubercart product type' does not show up.

I have also notice that in the files features creates, there is no record/settings of the ubercart product type.

This needs more work. I don't think this should be committed until it actually functions.

Yep, sadly when I poked a bit deeper, I realized those limitations, too. So this isn't quite ready yet.

Oh, but to answer your question, I tried it against Features RC1 and it still doesn't pick up Ubercart products under CCK. This isn't that surprising since CCK doesn't actually manage them from a hook_node_info() standpoint; Ubercart does. But hopefully it's possible to fix this either here or in Features.

I had a discussion with yhahn in #open_atrium about whether this bug fix belongs to UC Product or to Features.

He pointed out that there are two models around dynamic content types:

1. A module that takes an existing content type that was already created and signifies that it's "special" in some way, e.g. "This is an organic group".
2. A module that programmatically creates dynamic content types based on some sort of setting. This is the Ubercart Product way, which creates content types based on defined "Product Classes".

In the first case, some other module's hook_node_info() will have taken care of exporting the content type, so Features module should be able to deal with that fine. However, uc_product_node_info() has code in it like this:

    $result = db_query("SELECT pcid, name, description FROM {uc_product_classes}");
    while ($class = db_fetch_object($result)) {
      $types[$class->pcid] = array(
        'name' => $class->name,
        'module' => 'uc_product',
        'description' => $class->description,
        'title_label' => $title_label,
        'body_label' => $body_label,
      );
    }

Since Features doesn't know about the contents of the uc_product_classes table, it can't do the right thing here.

So, from the Features module maintainer's POV, it's the job of UC Product to expose information about Product Classes, so that hook_node_info() can report the right information. Therefore, the patch being against UC Product, rather than Features, is correct.

I also touched base with IslandUsurper in #drupal-ubercart to ask him if fixing this with CTools Export.inc (which would require some changes to load()/save() functions and hook_schema()) was on the table or not. He said he's very willing to look at patches that help UC Product work better with Features module. Yay!

yhahn mentioned that product classes seem to be a really natural match for CTools Export.inc, and that we could do something like a "soft" dependency on CTools export in the load/save functions so that if they're there, they're used properly, and if they're not they'll fall back to the database.

So I'll try and take a look into this "soft dependency" approach, once I do some basic debugging to determine why the patch here isn't quite working properly. Assigning to myself, for now.

Oh, incidentally, this also explains (to me) why this patch works great on an *existing* site which wants to export its own stuff, but not on a *new* site where things still need to get imported: it's because on the new site, the uc_product_classes table won't be populated, and therefore there's nothing to get the list of content types from. D'oh!

Reading up about CTools now. :)

@webchick - yep, this is awesome - i'm excited about ctools export for our features. Is there anything that would be helpful for people to tag team at this point?

Huh. Well.

After many hours of dead ends, the following, along with the patch at #928484: Supply $items in order to provide additional context for hook_features_export_alter(), seem to actually get us past step 0, which is getting the product types exported. I'm pretty sure #imdoingitwrong, but posting this up for review nonetheless because it seems to work for me.

Yeeeahhh... that gets the content type there, with the UC product information, even, but without the corresponding record in the uc_product_classes table, which means there's no way to track it. Drat.

Ok, so now it's time for a brain-dump of what I learned about CTools.

CTools integration is a little heavy, but not too bad. We basically have to:
- Add some properties to hook_schema() to provide additional metadata and mark tables/fields for exportability.
- Make changes to the load/save functions (uc_product_class_load() / uc_product_class_form_submit() - ick! need to pull that into a save() function) so that they pull data in the CTools Way (tm) if CTools is enabled.
- Add a new hook for specifying default Ubercart product classes so that Features (and people in general) can use it.
- (optionally) UI integration to get our own "Export" / "Import" / "Revert" / etc. tools. This UI is pretty heavy and Views-esque, though, and in my brief experimentation, it'll likely take quite a lot of effort to integrate it nicely with Ubercart's existing UI. I'd recommend pushing that off to another patch, if ever.

Here are some resources that I scrounged up that talk more about this:

• The Advanced Help which was actually quite helpful!
• Stella Power's tutorial on how she added CTools exportable integration to Migrate module.
• The original and revised tutorial on how Hannes Lilljequist, the FFMpeg Converter maintainer Features-enabled that module.

Patch forthcoming, once I figure out wtf I'm doing. ;)

Huh. Well just this and absolutely nothing else allows this to be exported via CTools's Bulk Export module. Unfortunately, not Features, though.

Actually, that does cause it to get picked up by Features (as a "Product" component). Here's a bit more, which hooks it into node types as well. Credit to yhahn for a push in the right direction. With this, I can successfully export my stuff.

The problem is getting it back into a new site now. The next step for this - integrating CTools into load/save functions and whatnot - I can't quite figure out atm. I'm trying to put some CTools code into hook_node_info() and it doesn't seem to pick up the fact that my feature implements the default hook, for reasons I can't quite ascertain...

I'm going to have to unassign myself from this issue, I think, since it's looking more and more like I'm just going to have to hand-code what I need to do in the interest of time. :\ Bleh. :\

As webchick said,

The next step for this - integrating CTools into load/save functions and whatnot

I put together the following hunk of code, which when called directly, seems to create product classes based on the export in a Features module created with the patch in #14.

However, Features seems unaware of changes to the product class, an even that it doesn't exist. I take it from that that uc_product needs to call this code from a different Features or CTools hook implementation.

Can anyone suggest which hook(s) that might be :) ?

/**
 * Implementation of hook_features_revert().
 */
function uc_product_features_revert($module = 'uc_product') {
  node_features_revert($module);
  $export_classes = module_invoke_all('default_uc_product_classes');
  $query = db_query("SELECT * FROM {uc_product_classes}");
  while ($result = db_fetch_object($query)) {
    $existing_classes[$result->pcid] = $result;
  }

  module_load_include('inc', 'uc_product', 'uc_product.admin');
  foreach ($export_classes as $pcid => $class) {
    $form = array();
    // If this product class doesn't exist on the site, we need to 
    // set $form['pcid']['#type'] == 'textfield' and populate its value.
    if (empty($existing_classes[$pcid])) {
      $form['pcid']['#type'] = 'textfield';
      $form['pcid']['#value'] = $pcid;
    }
    $form_state['values'] = (array)$class;
    // Ubercart takes care of updating or creating a new class for us.
    uc_product_class_form_submit($form, $form_state);
  }
}

Thanks to merlinofchaos for pointing me in the right direction -- If we correctly implement CTools crud functionality (which we should), we don't have to take care of creating new objects -- that's the whole point of the 'export' section of the schema.

See ctools/help/export.html for more.

After some troubleshooting and great help from yhahn in IRC, the array in hook_features_api() implementation should be keyed on the component name, not the module/owner name.
With this change, exports now generate a feature_name.features.uc_product_classes.inc file :) .

Here's a re-roll that takes us that one small step forward.

We still need to prevent exported classes from being deleted in the UI. Also, it seems that Features does not detect changes to the exported classes.

scribe

subscribe

subscribing

patch in #17 re-declares hook_node_info(), causing

Fatal error: Cannot redeclare vmf_courses_node_info()

if any non-uc_products are exported.

updating version - presumably this should be developed against dev.

subscribe

Sub. (ie "Um. Yeah. Me too.")

Hmm...I ran the patch in #17 to no avail. When exporting my feature, I was able to choose my product class, however when enabling the feature on a blank install, the class wasn't created.

The product class is included in the resulting module, it just doesn't do anything when enabled.

UPDATE: After some searching I found this comment over on Ubercart.org which refers to #525612: Allow any node type to become a product and states that creation of product classes with hook_node_info() won't work in 6.x-2.x. This is exactly what the output of my feature is attempting to do, which explains why there's no resulting content type created.

subscribe

Being able to clone product classes w/ fields, display & content type settings would be great. I do not currently know of any way to do this, so this feature module integration could be very helpful in that respect. Being able to rename the product class/ content type within the feature export & then re-importing the feature back onto the original site would solve this problem.

Furthermore multi-site UC setups could use this to quickly align the product content type settings on both sites -> that way it will be much easier to import products b/w them using feeds or some other module.

hopefully we can find a work-around from the problem mentioned in #24:UPDATE.

I hope Drupal modules settings, content structure etc exportable becomes a part of the Drupal design philosophy. Good on you features :)

Hey Timeless Domain, I was just creating an install profile and am using this code, its a workaround to do most of it manually. This will help get someone started, if you need help with other cck fields I suggest looking into install_profile_api module.


// Taken from the uc_product_class_form_submit form

function create_uc_product_type ( $name , $pcid , $description )
{
$pcid = preg_replace ( array ( '/\s+/' , '/\W/' ) , array ( '_' , '' ) , strtolower ( $pcid ) );

db_query ( "INSERT INTO {uc_product_classes} (pcid, name, description) VALUES ('%s', '%s', '%s')" , $pcid , $name , $description );
uc_product_node_info ( TRUE );
variable_set ( 'node_options_' . $pcid , variable_get ( 'node_options_product' , array ( 'status' , 'promote' ) ) );

if ( module_exists ( 'comment' ) ) {
variable_set ( 'comment_' . $pcid , variable_get ( 'comment_product' , COMMENT_NODE_READ_WRITE ) );
}

module_invoke_all ( 'product_class' , $pcid , 'insert' );
node_types_rebuild ( );

//weird have to do this
$type = node_get_types ( 'type' , $pcid );
$type->custom = 1;
node_type_save ( $type );

menu_rebuild ( );
drupal_set_message ( t ( 'Product class ' . $pcid . ' created.' ) );

}

While I wait for this feature, I also used this same code as imp7 shows in #27 in the ubercart_event_registration module...works well for now, but boy I'd love a feature!

+subscribe

Assigning to myself as I am working on a full patch for this. Thanks to everyone who contributed patches and comments so far in this issue.

Looks like #654764: Name and Description not synced between product class and associated content type will need to be fixed at the same time, this seems related to the revert issue described in #17.

As long as you're working on it--would you consider writing a patch for Drupal 7.x as well?

That won't happen immediately - this work is being sponsored in D6, and I may not have free time to implement it for D7 as well - but I will post the full patch here and hopefully it won't be too much work to convert at a later date.

The attached patch has only had some brief testing so far but seems to work as expected when creating, updating and reverting a product class feature.

It is based on #17 with the following changes:

• Allows use of a new hook_uc_product_default_classes() to define classes in code.
• Removes the 'delete' option from the product class page for classes provided by the hook.
• Implements hook_features_revert() so the class name and description can be reverted.
• Incorporates the sync patch from #31 to handle product class vs content type edit forms.

Note the CTools integration is not really finished, but exporting a feature requires it to be enabled. However, I think perhaps CTools is overkill for exporting just two fields (name and description), which is all a product class really consists of.

Following up webchick's comments in #12:

- Make changes to the load/save functions (uc_product_class_load() / uc_product_class_form_submit() - ick! need to pull that into a save() function) so that they pull data in the CTools Way (tm) if CTools is enabled.

This isn't done the CTools way at present, but adding a uc_product_class_save() function would clean up the code a bit.

- Add a new hook for specifying default Ubercart product classes so that Features (and people in general) can use it.

This is now implemented, but needs documenting.

- (optionally) UI integration to get our own "Export" / "Import" / "Revert" / etc. tools. This UI is pretty heavy and Views-esque, though, and in my brief experimentation, it'll likely take quite a lot of effort to integrate it nicely with Ubercart's existing UI. I'd recommend pushing that off to another patch, if ever.

I am not sure this is really needed; Features provides this when needed, and do we really need another import or revert for just two fields? It would be just as quick to do them by hand.

Comments welcome. I guess it would be nice to extend this to attributes, options and the product class variants of these, where CTools integration might come in more useful, but that is for another issue.

Second attempt at this. CTools is no longer used, product class definitions can include extended content type fields such as title_label and body_label, the default "Product" type can no longer be exported (as it won't work from the default hook) and the revert hook is based on node_features_revert() which seems to work better.

If someone can explain the benefits of having CTools integration here I'd be happy to re-add it, otherwise I'm still not sure I see the need.

The benefits of integrating with ctools include, among others, reducing functions like uc_product_class_load(), uc_product_classes_features_revert(), and uc_product_class_save() to one-liners.

But then wouldn't Ubercart have to depend on CTools? Not really a dependency we want to include in 6.x at this stage of the game, but I guess it's worth it for 7.x.

longwave - right, i see your point.
nevermind me, carry on then!

@longwave so far this is working really nicely! I also see no reason why this should be done any differently for 6.x. It'd be good to get more reviews.

Attached patch includes the following changes:

• Default product class can now be exported.
• #654764: Name and Description not synced between product class and associated content type has been committed, so is no longer needed here.
• Product class delete access callback has been improved.

My client is using the previous version of this patch (without needing the default product class to be exported) and we haven't encountered any issues, so if someone can review and confirm this all works as expected I will commit it.

More changes:

• hook_uc_product_default_classes() is now documented.
• The previous patch caused the default product class to be treated as a real product class if it was defined in the hook; this has been corrected. The default class can still be exported, but will not show up on the "manage classes" page.

This update adds support for strongarm, so the shippable flag and product image variables are exported where available.

#44 works as expected

Committed to 6.x.

OH! HOW! AWESOME!!

Thank you SO much for resolving this issue!!! :D

Nice work!

After carefully picking over #44 and dropping its contents into the Latest -dev for Ubercart on D7, it works!
The docs.php file isn't present in Ubercart for D7, but doesn't seem to carry any side-effects without it.

Excellent work, sir!

Here's a D7 port of today's Ubercart 7x-dev.

EDIT: When trying to add a new product class without features, it won't show up--this appears to be a caching problem. Flush the cache and it'll show. Features still has some shared field conflict issues. And it looks like Ubercart in D7 requires a parameter for uc_product_class_load. NULL seems to be suitable when requesting any/all product classes.

sub

+1

also, for product_kit

@HnLn,
@LPCA

If you're interested in this, try out the patch in #50 and report back on your experience with it. It's not getting committed until it gets some community testing.

Committed #50 with a couple of minor bug fixes: http://drupalcode.org/project/ubercart.git/commitdiff/dfb3531

After getting up close and personal with git bisect, I reverted commit dfb3531, which was applied in #54, because it broke the "Products" test cases. Test cases need to be updated to accommodate changes made in that commit, or patch has to be reworked so it passes the test cases. Test cases now run green in HEAD.

Let's see what testbot thinks of this.

Actually, is testbot even working at the moment for us? I haven't seen any completed test in a good while in this queue.

http://qa.drupal.org/pifr/test/132134 says

FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in @filename.

[05:38:27] Review complete. test_id=132134 result code=6 details=Array
(
    [@reason] => No such module [uc_ca_sms]
)

Committed #56, as local tests all succeed with that patch in place.

@TR: if you get any test failures, please paste them here so I can try to diagnose further.

Confirmed that the tests work with the patch in #56.

Re #57: The testbot still has some problems dealing with contributed modules, so it is of limited use for Ubercart at the moment. I will be very happy when those issues are finally cleared up.

The testbot works by first running tests on the branch to make sure everything is in working order, then running tests with the patch in place. Currently, the tests on the Ubercart branches are failing, so the testbot won't even try testing patches. @rfay can manually restart the branches, but even then the testbot doesn't handle module dependencies properly so Ubercart patch tests will fail.

You can see the status of the branch tests by clicking on the "View details" link of the patch above then clicking on the branch link in the resulting page. As you noted, it's currently choking with an error "No such module [uc_ca_sms]", due to the testbot dependencies list being very old and out of date. See #1095218: Testbot finds dependency that isn't a dependency for details in the issue I filed about this.

Commit db524d93 introduced some Drupal 6 syntax database code into the 7.x-3.x branch, and also triggered some Coder module errors. Corrected both in commit dd9d403cf5.

I am having a problem associated with this.

I enabled my feature with the product class. GREAT.
But now when I disable the feature, the product class remains.

When non-product class features are disabled, the node types are disabled. I just want it disabled and the content remain.

Product classes are content-types.
Unless the features integration for ubercart explicitly tells Drupal to turn them off, they'll always be present after the feature is installed.

Still not included in a stable release?!?
Also, all of the patches, as of now, FAIL the tests.

What to do to export with Features the Ubercart Product content type??

This was committed back in 2011, which means it has been in every stable release since 7.x-3.0. If you are having problems with this feature please open a separate bug report.