Coming from the Rules issue queue and #519954: integrate with the features module, we need to rename rule IDs in exports to avoid clashes. It would be really ugly to do the renaming with the existing hooks, so fago suggested to ask for a hook_features_export_alter() here. It should be called at the end of an export to allow modules to inspect the export and apply any final changes, like in our case renaming of exported items.

CommentFileSizeAuthor
#1 features_component_name.patch660 bytesfago

Comments

fago’s picture

Title: Provide hook_features_export_alter() » Allow renaming of components
Status: Active » Needs review
StatusFileSize
new660 bytes

I had another look at this and got it finally working right :)

However I noticed that those alter() hook wouldn't help us a lot, as still we would need to track somehow to which original rule a renamed rules refer - which is getting complicated. Thus I reverted the initial way - just put the whole $rule in $data. This is basically working fine right now, however there is one issue in features if doing so: In the info file "Array" gets written as component name. Attached patch fixes the info file writing to use the keys of $data for writing component names. As elsewhere only the keys are used I think this change is fine and allows integrating modules to put arbitrary stuff in $data.

(Though if a module does so, it has to watch out for additional selected components which might get added in the third step.)

So it would be cool, if this patch could get in soon, as I'm about to release rules 1.1 with features support depending on this... :)

@info-file generation code:
Omg, this code is really cryptic. :D

yhahn’s picture

Assigned: Unassigned » yhahn
Status: Needs review » Needs work

Ok, first I promise we will make sure Rules - Features integration can happen.

I need to talk this one over with miccolis - basically one API concept in Features is that the whole process happens in two cleanly separated steps -- the generation of the feature "info", and the actual writing of the export code. This is one reason why we only write keys/identifiers to the $data array in the first step and why the second step cannot expect a fully loaded object (and should do its own loading) as it should be possible for it to work strictly from the information contained in the feature's .info file.

Rules' need to rename, reference and track the source of a renamed component obviously throws a real wrench into these concepts. : /

I'm wondering if there's some way the Rules - features integration could track this outside of the features stack for the time being (e.g. in a function with a static cache that maps original to renamed identifiers). I know that's not ideal but blurring the lines between the export & render stages would be a major change in the API concepts, even if it seems largely harmless in code.

fago’s picture

I see.

>I know that's not ideal but blurring the lines between the export & render stages would be a major change in the API concepts, even if it seems largely harmless in code.

Hm, render is still used for rendering, however the difference is that export puts in the whole rule in $export, not only the identifier. We could change that to use a 'renamed' => 'old id' map, which I agree makes more sense. However then still this patch is needed, because features treats the $export['features'] / $data array a bit inconsistent.

1. In the first step one gets $data, which contains a numerically indexed array of component ids.
2. One has to transform it to be associative, as features uses then uses the key of $export['features]['component'] to identify components.
3. Then again when writing the info file, it uses the value of $export['features]['component'] and not the key as component id.

The patch basically fixes 3 to also use the key. So to be completely consistent here, 1. would need to be fixed too.
I think it makes sense to use the key as component identifier and not the value, so the value would be free for modules to add in customized information, like the origin of a mapped id.

@static:
I thought about the static cache, but that won't work as features uses a multiple step form. So the static cache would get lost. So if we go down this route, rules would need a way to add something to the form state storage or place in $export where it could put that information.

>Ok, first I promise we will make sure Rules - Features integration can happen.

Thanks, this is a killer-feature and I'm already looking forward to it (as well as many others.) =)

jmiccolis’s picture

Fago, I just looked and rules with yhahn and I think we'd like to advocate for a small change in rules that should make it just work. As you're already using string ids for rules in the db, if you have those id's generated from the label of the rule, rather than being something like 'rules_2' we wouldn't need to change anything in features.

Could this work for you?

amitaibu’s picture

fago’s picture

Hm autogenerating the names from the label would be a possibility, but it won't ensure that there aren't colliding rule names. We could ensure that for one site, but as soon rules get exported and exchanged they could quickly collide - consider often used labels like "Publish content". In turn the colliding names would cause odd bugs... :( So I really want to prevent that.

Thus rule-names have to be prefixed by the providing module, as it's now. However that way we are in the need of renaming the rules when we export them to a module...

@user-editable names:
Basically triggered-rules are quite small units and should be quickly creatable. Also they don't need to be referenced from anywhere (in distinction to rule sets) so it really doesn't makes much sense to burden the user with the task of finding a good-not-colliding rule name.
The name is not important for the user or anything else, so users shouldn't have to care.

The patch from #1 is a rather small change and even helps to make features more consistent by using the key as component name when creating the .info file as it does all over the export code else too. So I think this changes makes even sense regardless from the rules integration!

Anyway I'd really love to see rules features support working but staying with the current naming scheme. So it would be awesome if it could get in as is!
However I agree that it's hackish to put the whole $rule into the export array as rules does now. This needs a cleanup to use a simple id map instead, which then is used when rendering the export.

jmiccolis’s picture

Status: Needs work » Fixed

@fago, As promised at drupalcon I've committed the patch from #1. Rules support here we come!

fago’s picture

Awesome, thanks!! :)

Status: Fixed » Closed (fixed)

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