Add Features support

I don't fully understand the implications of this patch, but I wanted to chime in with a few things:

First off, thanks so much for posting this patch.

• I really like the idea of being able to use ctools for import/export. It has a lot of advantages.
• I don't like introducing another dependency... Only panels users will already have Ctools.
• I don't like loosing the bulk export because exporting one Skin at a time is pretty useless (maybe I am misunderstanding this though)

It also makes changes to the data structure. I am not personally tied to the current structure (though moonray might be), but given how far along we are into 6.2, and where this stands, I wonder if we are better off postponing this until the next version.

Just no easy way as it stands to export 5 at one time. Just export each one individually.

Maybe "useless" is a bit strong, but exporting one-by-one is (a) a regression and (b) not something most use cases will call for.

Well, CTools is pretty light weight it would only use the Export part of it. CTools is already a requirement for UI (via Dialog). And I actually don't use Panels yet I use CTools all the time so I challenge your assertion.

In Drupal 7 we don't have a CTools dependency anymore, so I'm hesitant about adding it back. This isn't a deal breaker, just something that needs to be considered.

Like I said - I do like the idea of this patch. I'm not against it; it's just late for this release and it needs a decent bit of work still. We are looking at wrapping things up for this release over the next couple of days.

I'm sorry, but I'm really not interested in arguing about this.

Show me a fully working patch that doesn't take away current functionality and we'll talk.

subscribed. I think this a very useful feature.

I'm working on the same things, except I'm only doing the skinr rules. The reason why I started with the rules is because that's a lot easier to to make exportable being that we can easily add the machine name to the skinr rules form that's already there.

Also I don't think ctools needs to be a requirement, the ctools code could had been wrapped around in a module_exists that way skinr can still fall back to it's regular ways if ctools is not available.

Also the big problem is really exporting skinr settings for blocks. I'm sure it'll work fine with Views blocks because views uses machine names but Drupal blocks don't play well with exportables because of their auto increment value. They're already multiple projects trying to solve this very same problem so maybe there should be a consensus on which route to go.

I think this is a good start but there has to be more discussing on which route to take regarding exportable for blocks settings. Just my thoughts :-)

Subscribe. Yeah, we need exportables on Skinr in order to use it to move sites from one build server to another. It's pretty important. IMO, requiring CTools is no big deal. And using CTools to make exportables easy is one of the current best practices.

If we want to let people who don't need exportables be able to use Skinr without CTools, then perhaps we make the exportable support a separate module inside the Skinr module package. Views for instances is actually 3 modules in 1: Views (core), Views exportables, and Views UI. If we have Skinr, Skinr UI, and Skinr exportables, then only Skinr exportables could require CTools. And we could have the best of both worlds. Features support for largers sites, and simplicity for smaller sites.

Without solid exportables and support for Features, Skinr won't be as awesome as it should be.

If a patch is submitted for a separate skinr_exportables module, I'll be happy to include it in skinr's package.

Moonray, that sounds great. Anyone want to work on this with me? I might not have time till this weekend but maybe we could move the dev to github in the time being until is stable to be committed to skinr?

If anyone is interested. I started working on this as a separate module. It's no where near being completed yet but if anyone wants to help out I'm working on github (until is ready to be contributed back here). url: http://github.com/ericduran/skinr_export

Right now all it does is exposes the skinr table to ctools so it can be exported using features or ctools bulk export module.

The missing feature is getting skinr to read the exported data. This might end up being a tiny patch to skinr, but not sure yet. I haven't started working on that part yet.

Just a quick follow up. I believe the skinr_export (http://github.com/ericduran/skinr_export) is in pretty good shape. Any testers? The one feature that I know is missing is the exporting of page rules. I'm working on that now. I'll post a follow up here when I get that done. When is ready and tested I'll post a patch here so more people can test.

@ericduran It would be helpful to have some documentation here as to how to use and expected results.

I guess, just looking at the gui, I'm missing that things have changed. I see your hook_form_alter, I just don't see anything different from what's expected in 2.x-dev.

@dstol, Thanks for reviewing.

So the skinr export module doesn't do much on it's own. It really just exposes Skinr settings to either Features or Ctools Bulk export. Or any other module that implements the hook_default_skinr_skins.

I'll see if I can record a video or put up a blog post showing what this module does.

@jacine this actually brings up a good question. Should the exporter in skinr be remove? or even better replace with this one? It's a lot easier to implement the exporter using ctools (this one currently doesn't have a exporter ui but it can get one ;-) ). So in other words this module will still be separate from skinr but it'll provide a ui for exporting skinr settings using ctools, and it'll also have Features support.

This might be to ambitious for 2.x release. But let me know.

@ericduran, it should replace the existing export functionality (definitely needs a UI), and is fine in a separate module.

It's not too ambitious, IMO. If you get it working, and we can get a few people to test it, I'll happily commit it! ;)

@jacine, cool. Working on it now.

Ha, so after writing all the code, I really dislike the implementation with the separate module :-/. It just add so much extra complexity for something that should be simple or at least not as complex.

Also after working on this, I've notice that skinr has ctools function sprinkle around it.

Here's my Proposed Plan:
- Make the skinr.module support exportable using ctools without exportable api without explicitly requiring ctools. So in other words if it's there we'll use it if not, we don't do anything. This is nice because you won't need to enable skinr and an extra module just to load your exported skins.

- Replace parts of the skinr_ui.module with ctools export_ui. This would ideally lower the amount of code in skinr_ui and there's no need for an extra dependency since ctools is already required by skinr_ui since it's a dependency fo dialog.

That being said I'm going to work on these issue each individually and create patches in separate issues. I just think it makes more sense than trying to get one big patch in at once.

Hope this is ok? All comments welcome.

ericduran: Considering the things you've mentioned above, I have no problem with your proposed way of implementing things.

I'm moving this to 7.x I still plan on fixing this

Changing title and priority.

Hey ho. It might be useful to know that Views 3 for Drupal 7 is dependent on ctools. So pretty much every site will be running ctools.

Any news about this?
It would be very useful to have features support in the 7.x-2.0 version...

Many thanks!

sub.

Perhaps we can take another swing at this now that our Skin Configuration data structure has solidified?

So what needs to be done here? ericduran built a separate module but wasn't happy with the end result. It sounds like the next step was to strip out any existing import/export functionality and replace it with ctool exportables. Is that right? @ericduran, do you have any suggestions?

Some of the stuff that made ericduran unhappy is probably gone now (I hope).

If someone could tell me how much code would be eliminated by NOT having a separate module for skinr export functionality, that would help make a decision whether to create a new module or not.

The separate module was for an earlier version of skinr, so I'm sure that's completely unworkable now.

I haven't check out skinr in a while, but I might just give this a try again this weekend and see how it turns out.

/me starts now!

Ok, here goes a 1st pass at making skinr rules exportable.

This doesn't add any dependancies and support exporting out your rule when using feature.

This patch does not contain an update hook, so it will require a clean install.

Hmm, we'll this is pretty much it for the rules, now we need to do the skins ;)

@ericduran Your patch in #35 is working with Features directly rather than CTools export.inc. Is there a particular reason for taking this direction? Aside from having (awesome) exportables support I was hoping that using exportables would allow skin configuration to be read from disk rather than the database, saving the many queries that Skinr creates.

Let me know your thoughts. Either way thank you for your work on this.

@rickvug, Hey,

So yea, in all the other patches I was trying to make skinr work with ctools instead of features directly and in this patch I took the complete opposite approach.

There's a couple of reasons for that.
- Skinr is using auto-increment ids (I didn't want to rewrite the schema).
- I found it easier to just add a machine_name and do matching of that, instead of trying to actually make the skinr schema exportable.
- I wanted to make the patch as light as possible. I didn't want to make one big patch, especially since I wasn't involve in most of the drupal 7 rework I didn't want to just jump in here and be like hey, here's a 300 line patch.

There was a couple of other reasons, I don't remember all of them right now.

The patch above works with Skinr Rules. I'm going to make another one for skins. But I wanted to separate them out, since they are essentially two different task.

But all in all, the Patch above should work great with Skinr Rules. Skinr skins coming soon ;)

@ericduran Thank you for giving the background. I completely respect not wanting to rip-apart too much of Skinr to make the change. At the same time I'm also wondering if the change would be worth the effort and if now is the right time for API changes. I think everyone would rather have Features support than nothing and you're the one taking the time on the patch so I'll leave it at that! Thanks again for your work here.

Once #1007482: Make use of Context API instead of custom rules goes in we'll have nailed down our whole db/objects structure. We can then create an import/export scheme that will no longer need to be changed.

Any updates on this? I'm planning to release a dashboard module that uses skinr to manage it's layout so I'll have to somehow include the skinr configuration in the module.

No updates on this one. It's a bit of a beast to tackle.
Help on getting CTools exportables would be appreciated. :)

Importing Skinr settings with Features.

1. In the file myfeature.install

// Import skins from export file
function myfeature_update_skins(){
	$files = file_scan_directory(drupal_get_path('module', 'myfeature'). '/skins', '/.inc/');
	
	foreach ($files as $filepath => $file) {
	        $content = file_get_contents($filepath);
		$skins = '';
	        ob_start();
	        eval($content);
	        ob_end_clean();
	
		foreach ($skins as $skin) {
			// Find existing skin configuration and grab its sid.
			$params = array(
				'theme' => $skin['theme'],
				'module' => $skin['module'],
				'element' => $skin['element'],
				'skin' => $skin['skin'],
				);
			$sids = skinr_skin_get_sids($params);
	                $skin = (object) $skin;
			
			if (!empty($sids)) {
				$skin->sid = reset($sids);
			}
			// Save skin configuration.
			if (!skinr_skin_save($skin)) {
			        // some kind of dog that watching 
			}
		}

	}

}

// optional myfeature_install()
function myfeature_enable(){
	myfeature_update_skins();
}

2. Create folder 'skins' next to myfeature.install file and put exported skinr code in the myfile.inc

@ericduran, @Scott Renolds: Apologies for this taking so long that your code became outdated.

Attached patch adds basic features support for Skin Configuration objects (applied skins).

Still to do:

1. Add tests
2. Implement features support for Skin Configuration Groups (Skinr Context module)
3. Update Skinr UI to indicate status of Skinr Configurations (default/in code, overridden)
4. Update Skinr UI to allow reverting of Skinr Configurations to default (code)

Related: #1163676: Allow cloning of exisitng skin configuration to apply to another theme

EDIT: Note that I've chosen to copy uuid.inc from UUID module, rather than depend on UUID module; we want to keep dependencies to a minimum, and UUID module (like CTools) does a lot more than we need it to. Additionally, with UUID coming to core in D8, I feel using UUID for skin configurations is the right way to go here.

Looks like the tests failed due to a stray dpm() stuck in there somewhere. Otherwise the patch is good.

Attached patch adds in support for features for the skinr_context module. It includes tests for the changes made to skinr_context as well.

I've updated the patch with the following changes:

• Changed $group->gid to be a machine name instead of adding a new machine_name field; the latter complicated matters needlessly.
• Added some styling to the skin listing admin view to make it more clear which skin configs are enabled or disabled. Hid the status field and added a storage field to display whether the skin config is in code, in code and overridden, or in database.
• Added ability to revert skin configs in the admin ui.
• Added missing api docs for new hooks (and some previous ones that missed docs).
• Moved import/export logic out of features include and made part of main api for more portability and better consistency.
• Default skin configs and skin group configs are imported when a module that has them is enabled.
• Added more tests.

Re-review failed test due to test system bug.

#47: skinr-features_support-744706-47.patch queued for re-testing.

Fixed a few more bugs and added more tests.

Committed. See http://drupalcode.org/project/skinr.git/commit/cfe800a