Scald context exportable - overriden shared feature

This would definitely help in packaging some scald exportables in a base profile and allow different sites to export other scald exportables. Right now, this is impossible since only one feature can export all the scald contexts.

Thanks for the patch, I test an export and it looks clean. There are however a few bugs:

1. I think the "type" column mentioned in #3 is outdated, and the idea was to split the context_config object (in CTools means) into separate object, but now become a workaround in Features to avoid an update. I'm ok with that.

2. "You can now export only the context configuration by type in features." - several question:

- When I apply the patch and update my feature, code from mymodule.context_config.inc was copied into mymodule.features.inc and I have bug when mymodule_default_scald_context_config() is defined twice.
- I think we should continue to support the context_config export as a whole to keep upgrade easier. I.e. when I export a context_config, it exports all scald_context_type for that context. In the other way, context_config depends on all scald_context_type of existing providers. Then upgrade will be smooth because I don't have to change anything to support this new kind of export.

3. Why 4 spaces "____" is used as separator? Why not just 2 underscores? I think the best separator is the traditional one: dash (-).

4. Minor one: the export array is rather large, could be use the format $context_types['context_name-type'] = ...; to help debugging?

I have regenerated your patch, hope it is ok now. :)

And failed ... lol, ok let's check why ...

Mmmh I must admit that I don't understand the error right now :(

There were still some CRLF in the new file to be generated. Trying this one, better luck this time ?

Sorry, that was an accidental submission. Restored and added php tags as a bonus.

Patch seems to be working.

I think that backwards compatibility should get some attention.
It seems that there is no real way to migrate over to the new solution, since we cannot change code in features.
So the old feature should remain operational until the site builder decides to switch over to the new way of doing things.

However when the feature is recreated and the new method is used, the old file module.context_config.inc should not be present anymore i think.

Hi!

Thanks for the patch.

1) The following diff might be a mistake as its already in the menu hook:

+  $items['admin/structure/scald'] = array(
+    'title'             => 'Scald',
+    'description'       => 'Manage Scald Atom Types, Contexts, and their associated settings.',
+    'page callback'     => 'scald_admin_dashboard',
+    'access callback'   => 'user_access',
+    'access arguments'  => array('administer scald'),
+    'file'              => 'includes/scald.admin.inc',
+    'type'              => MENU_NORMAL_ITEM,
+  );

2) That deletion method will only work if php has access to write the module files if I'm not mistaken.
The best would be if when using drush fu, or simply feature recreate it would delete / not include that old file in that case.
Not sure that this will solve that.

The best solution would be of course to do something like the features module does. Is that it could read both the old and new config, but when drush fu or recreate, it will write the new config automatically deleting the old. And so it keeps backward compatibility, and then we dont need this extra variable, but after the first recreate it will use the new one.

If the new and the old are equivalent that should be doable, otherwise if switching to the new version requires some manual steps as well, then maybe thats not a good idea, and your switch variable is better.

2) Yes you can configure your development environment so php has access to all files. And you can use the feature to replace your files automatically on the UI.
However If someone uses the drush fu, or recreate from the UI, then when drupal creates the package with the feature, the old file should not be included. That was the issue i reported at least.

But wont this clean export function be called when recreating a feature (when i just download a zip of the feature)? And then it will try to delete a file when it should not?

It was more like a theoretical question.
But if it would be the case, then if you have an old feature with the old format, and then you press recreate on the features UI then it would delete the file from the feature (if php has write permission on the module's folder that is), and then it would give you the zip with the feature of course.

I was just wondering if in this case that happens or not.

Of course I will test the patch again soon.

But if you have an old feature in your folder. And then you go into the feature UI, you switch over to the new method and then click on recreate then it will use the new method and tries to delete the file when I was just recreating.
I think therefore when i recreate, it should not try to delete a file in the module's folder, because when i recreate on the UI i just receive a zip file and it should not manipulate files in the module's folder.

When the field configuration was split to instance and base, they did something like this: https://www.drupal.org/node/1959076
So just simply marking those as deprecated and not deleting them.

But then not sure if its possible to achieve to not have the file still in the zip when recreating the feature. Maybe the only way is to let the user know that the file should be deleted?

Sorry but I have a few more feedbacks about the switch to the new method:
- For security reasons (XSS) there should be no links that execute something right away. Like in this patch if i call "admin/structure/scald/switch-to-feature" then it will execute the switch right away.
Usually there should be a button on that page, or a button should be on some other page where the user can switch with clicking on the button.

- It would be nice if this functionality would be enabled by default for new installs.
So for new installs it would be TRUE by default, and for old installs it would be FALSE by default, and then that would mean that we prefer everyone used the new method, but we don't want to affect any old site running the old method. And any old site can choose to switch to the new method whenever they please.

And of course once the new system is in use, the switch button does not need to be there anymore.

I guess there wont be a way to switch it back to the old method, so depending on the value of the variable we can hide the button of course. And depending on whether this is on an existing page or on a new page, we can disable the menu entry as well.

I have made a few wording changes.

Also it turns out that you cannot even define the function for features, otherwise the ctools export wont work. So if we want to make it work for old installs until the functionality is enabled then we have to conditionally define the hook.
This of course means that after setting the option to TRUE, a cache clear is required.

In order to get this into 1.4 possibly, i reconsidered, and decided to set the option to FALSE in the install hook, and then the idea is that when 1.4 is released we can set it back to TRUE in the dev branch.
That way this can remain an experimental feature, which most likely works, but i would like to be extra careful and avoid a potential issue on new installs.

Please test, and I will still need to test it myself as well of course.

Moved the button in the scald dashboard below the atom type table, as the atom type table is used a lot, and this button belongs next to the context table.
Also added that it should only appear if the features module is installed, as otherwise not much point enabling it :)

Nice.

Maybe this patch will only go in right after 1.4 into the dev branch, we will see.

So we might have a few days delay here, but I will keep you posted :)

But you can safely use this patch for now. As it will get in soon.

Changing this to RTBC as the patch seems validated.

Thanks!

Sorry for the delay, its now committed.