Exportables and Features support for 6.x

This is Dagmar's patch from the (now) 7.x issue un-altered...

The file that wysiwyg was putting its features stuff into was named "module..inc" due to a missing 'api' element in the export part of the schema. This is a patch against 6.x-2.3 which fixes that, based on the patch in #1 of this issue.

I've tried this patch against wysiwyg 6.x-2.3 and exportables 6.x-2.0-beta1 and the patch applies fine but there are some errors appearing. I can no longer select a wysiwyg profile for each input filter and the edit and delete operations have disappeared.
is anyone working on a patch for the 6.x branch or does anyone know which versions this applies to cleanly (if it does?)

I had the same problems but saving the input format default selection page made them go away. I have not investigated why further though.

Many thanks to @ergonlogic, @bleen18, @Josh Benner and all the folks in the Drupal 7 version of this issue who've worked on this. This patch works great for me. (So well, I abandoned about two days of code I had done to accomplish the same thing.) I'd like to change this to reviewed and tested, but instead I'm changing it to 'needs review' based on comments #3 and #4 above.

@tayzlor and @auth can you confirm you're problems with this patch after clearing your cache? Also @tayzlor you mention wysiwyg and exportables versions, but not input_formats. Are you using input_formats-6.x-1.0-beta6?

Thanks!

subscribe

subscribe

@Josh Benner: do u know in which way your patch differs with the patch suggested in this comment? It has quite a few changes since the original patch from Dagmar.

I have tried this again with both the patch in #2 and the patch referenced in #8 and it will not work properly. admin/settings/wysiwyg/profile has no editors selected even though there are editors selected stored in the database. It seems as wysiwyg can not find the old configuration in the database and tries to resave it with an insert resulting in this error: Duplicate entry '1' for key 'PRIMARY' query: INSERT INTO wysiwyg (format, editor) VALUES (1, 'fckeditor'); upon save.

getting similar duplicate entry errors..

Here is a new version of the patch in #2 that applies cleanly to latest dev.

To answer some other questions:
@xgmorkx:
The patch here and the patch in #624018-139: Exportables and Features support for WYSIWYG 7.x are the same (except for Josh Benner's one line fix in #2 of this issue).

@auth:
I saw this once when first setting this up but the feature I was using was missing some important dependencies so I'm not sure if it was related to that.
I have not seen and errors since then.
If I am able to make this error happen again I will investigate.

Patch is created with git so applies with patch -p1.

Also, seeing as the errors mentioned occur in the code in the Input formats module it is likely their problem.

See this issue (I have not looked at or tested that patch yet) #991344: "Duplicate entry" SQL errors in input_formats_save() [PATCH]

subscribing

subscribing

+1

subscribing

whew, finally got this working in both importing and exporting Features, with drush make.

here's some feedback to help future improvements and other users with their troubleshooting:

• patch #11 isn't really useful anymore, it appears to fail for WYSIWYG 2.4 and i assume what was then "latest dev" is not an easy release to track down now
• patch #2 works with wysiwyg-6.x-2.3, exportables-6.x-2.0-beta1, input_formats-6.x-1.0-beta6 (and nothing else afaik)
• input formats is not listed as a dependency in the newly included "wysiwyg_copy" sub-module, so don't forget to include the dependency in your Feature as you bundle it (ideally it would be listed as a dependency in a patch rolled against WYSIWYG 2.4)
• if you want to apply patch #2 using drush_make, you'll need to apply the patch (to drush make) in comment #128 here: #745224: Apply patches from git diff and git format-patch to allow it to read patches created by git
• the patch in #2 does not apply totally cleanly through drush_make, there are some whitespace errors, but it seems to function OK anyway
• if you use a drush make file to pull in exportables, and don't set the version explicitly, you'll get version 1 af the API, and some lovely fatal errors
• "just" clearing the cache when you first enable all aforementioned modules isn't enough to get everything working, you have to actually visit the "input formats" UI screen in the site admin section for WYSIWYG (or maybe input_formats) to "notice" the input formats for some reason - this may help fix the problems raised in #9 as i was having similar troubles
• if you have existing WYSIWYG profiles stored in your database for input formats before you try to do an import, you may run into errors or odd behaviour - i don't know yet if this would cause troubles if you have two Features trying to define WYSIWYG profiles for the same input format

@thedavidmeister:
Thanks for testing,
* Patch in #2 is an old version and has been superseded by patch #11.
* You can tell by the date of post #11 that it was created for dev as of June 21st (2 days after 2.4 was released). If it doesn't apply any more then a new patch has to be made.

We can make a new version of the patch in #11 for 2.4, that shouldn't be too much effort.
The only downside is that module maintainers prefer patches to apply to the dev version before they will commit so unfortunately we also have to have another version of the patch that chases dev.

Marking needs work to:
* Add input formats dependency to wysiwyg_copy
* Make a version of the patch for 2.4 for easy testing and a new version for the latest dev.

I made a boo-boo. Please disregard the attachment.

@Shiraz Dindar,

The reason your patch no longer works is because you removed all the code in the patch that defines the sub-module that provides the primary export functionality.... bad.

Personally, the patch at #11 still applies fine for me, but if a re-roll is required at least be sure to test it.

I have re-rolled #11, the only differences is in the format of the patch so that it will apply in a Drush Makefile. No one needs to use it, I'm simply providing it for myself and anyone who wishes to use it in their own makefile.

Cheers,
Deciphered.

@Deciphered,

Thank you very much for your correction. I was hasty in removing what I thought was duplicate code for a the patch generator's backup copy of the module. My bad indeed.

Incidentally, the reason for me going in to edit the patch in the first place is it wouldn't apply via a git apply, reporting whitespace errors. So I went to remove the whitespace, and did that in the process. But it turns out my problem wasn't whitespace; it was this: http://data.agaric.com/git-apply-does-not-work-from-within-local-checkou.... Conclusion: if your code is already in a git rep, use patch -p1 < patchname.

Thanks again.

Shiraz

subscribing

Hi,

I tried the patch from #20, but now I can no longer select an editor for a profile. When I choose one and hit save, the fields reset...

I've use exportables version 2 beta, if I use the 1.x version after I patched the WYSIWYG module, I get a fatal error.

Any thoughts on this? Thanks in advance

The patch from #20 is working with 2.4 and I can now export my profile into the right file. No more module..inc file !

Thanks Deciphered.

Just curious - Will the patch in #11 be committed as some point?

I have now the same problem as in #23.

I got it working by changing 2 lines of the @Deciphered patch.

The function wysiwyg_profile_load_all() is used to load the profiles from ctools (Features, flat files).

The condition to load profiles from DB is that ctools and exportables are existing in the system, which is a problem if you already use those modules in your website.
DB profiles will always override flat files profiles from Features, so, the previous condition is not good and DB profiles must be always loaded.

Here are the two patches.

The first for the version 2.4 and the second for the 6.x-2.x branch.

Update of the patch.

The last patch introduced a bug in the admin interface with the link to delete and/or to revert.

Re rolled the new patch with a condition added in the function wysiwyg_profile_load_all().

Now links in the admin interface displays correctly.

Also, useless to make 2 different patches, this patch can be applied to 6.x-2.4 and 6.x-2.x.

@Pol: nice one! works like a charm, through the UI and with Drush.

Marking RBTC at risk of jumping the gun, because… this has been in the works for a while, and really just needed this cleanup

Patch works but hook_features_api needs to return an array, you will get PHP warnings whenever features tries to rebuild it's cache and exportables is not enabled.

Invalid argument supplied for foreach() features.module:272
Invalid argument supplied for foreach() features.module:386

Features and wysiwyg are not dependent on exportables, so it's possible to run wysiwyg without it.

New patch attached. Marking needs review… since i RTBC'd it last time.

PS, I'm emphasizing this line only because i want to try out the new BuEditor on d.o!

@scottrigby

I tried your patch from #31, yet I don't see any option in the features ui when it comes to exporting WYSIWYG profiles. Not in a separate category nor in the strongarm section.

I'm using the latest wysiwyg dev, also installed 2.x branche of exportables and input_formats module just to see if that would help considering #624018: Exportables and Features support for WYSIWYG 7.x.

I'm also using CKEditor as the designated driver, in terms of the editor.

Am I perhaps missing something?

@AlfTheCat: are you sure? CKEditor here too. The Features UI seems to work fine as well as drush for us

@scottrigby

Yeah, I'm really sure. The patch is applied via Drush through Aegir. I'm using features 1.1, since the 6.x dev branche isn't listed anymore...

I'm at a bit of a loss. WYSIWYG is retrieved via Drush make, then patched via Drush, then installed in my install profile. On arrival there is no category added to export WYSIWYG settings. Nor is the category there after I configure it.

I've been looking for a solution for almost 2 years now so I'm incredibly excited about the success you've produced with your patch.

@scottrigby after some puzzling i found that my problem was related to a Drush issue, which didn't apply the patch although it did report differently. So, a different issue entirely #745224: Apply patches from git diff and git format-patch.

I now patched the features module manually and I do get the the wysiwyg category in the features ui! When I export the wysiwyg settings into a feature and enable it in an installation profile though, the settings are not propagated. The feature is not appearing as overridden, yet wysiwyg remains unconfigured.

I switched to the wysiwyg module from the ckeditor module because it has this problem (too?) (#1103980: Features module - automated install fails

Do you think the same problem is occurring here?

FYI, if you get the following fatal error after applying the patch in #31 check your version of Exportables:

Fatal error: Call to undefined function exportables_api_version() in wysiwyg.module on line 635

If you are using exportables-1.x (this is the version downloaded by drush if you don't specify 2.x) you will need to switch to 2.x for the patch to work.

Latest patch appears to be working correctly with wysiwyg-2.4, I will report back any issues found re-deploying the exported config when I get to that part.

I have now moved the wysiwyg exported profiles as well as the input format exports into a separate feature and have the install profile revert it at the end of its task list. It now pretty much works!!!!!
The input formats are still overridden on arrival though but it does not seem to bother wysiwyg. Awesome!

What time is it?

Commit time!

Shiraz Dindar,

As the issue is still in 'Needs review' status, I would suggest reviewing and if no issues are present and code is up to Drupal coding standards then set the status to RTBC, then we might get the patch committed.

Cheers,
Deciphered.

Patch #31 working perfectly.

The difference between that patch and #29, is that I forgot to return an array for the hook_features_api(), the patch #31 is adding that.

Let's commit it ! :-)

I'm getting the same symptoms described by AlfTheCat, generally unreliable behaviour when trying to export/import Filtered HTML, Full HTML and PHP Code when building install profiles through Aegir.

I'm having a tinker at the moment, perhaps it works better with input formats that aren't provided by Drupal core.. I dunno, I'll report back if I find anything. Perhaps I need to change what versions of each module I'm using.

Is this really ready to commit if the only way to get it to work reliably in install profiles is as AlfTheCat described:

- create a new Feature just for your input formats
- revert the feature at the end of the install script
- the Feature will claim that it is overridden, but just ignore that...

That doesn't seem "finished" to me, does anyone else agree?

Just got it working fine by creating a new input filter and working with that instead of trying to use any of the filters that are provided by core.

Unless nobody else can re-create the experiences that AlfTheCat and I were having with the Drupal core input filters, maybe the option to export those filters through the Features UI should simply be removed to save confusion.

That said, I've got a feeling that the problems described have more to do with bugs in the input_filters module than in this patch per-se because I was struggling to properly revert or override the filters on that screen.

@thedavidmeister the fact that my feature appears as overridden is likely due to an old entry in my install profile, where it inserts a db query providing an input format. I will be adjusting that soon and see if all is well then. But for now it's safe to assume the overridden feature is unique to my setup and with good reason (the install profile implementing a filter after the feature does).

And I tend to agree with your statement, the patch is probably very ok, Drupal's core input filters are.... not so ok.

Setting back to RTBC per #40

Is there any reason we can't use the Features module approach for D6, too? I.e., essentially backporting the patch of #624018: Exportables and Features support for WYSIWYG 7.x?

I've just applied the patch in #31 on a site but it is only showing the last of my custom WYSIWYG profiles in the WYSIWYG section in the Features UI. Has anyone else tested this patch with multiple profiles? I had this working on another site, but I think I may only have had one custom profile to export.

@ericaordinary I've got two profiles in my feature export and its working quite fine. I haven't seen any indication that multiple profiles doesn't work on any of my sites.

I have good results with a clean apply of the #31 patch against wysiwyg-6.x-2.4. We are using exportables 6.x-2.x and TinyMCE. I will give more feedback after additional testing; but, a clean-room enable of the feature went off without a hitch.

I agree, sun, it seems wrong to add dependencies to two modules that do not even seem to have active development.

Also, the exportables modules this relies on hits the database a lot and does some Weird Shit on my install where I have to visit certain admin screens in a certain order to get things to revert properly after the initial deploy.

D7 ftw.

It's unlikely that this will help many people going forward, but I found myself in need of features support for the 6.x-2.x-dev version of wysiwyg during an upgrade I'm working on. Currently none of the patches from this issue apply cleanly, and reading through the thread, it sounds like this should ultimately just be a backport of the D7 features work.

I took a look at the issue @sun mentioned, and found that the feature code didn't need to be adapted much for a backport. the use of db_merge() had to be changed to a drupal_write_record(), and a single lookup query had to take place to determine if this was an insert or update. Beyond that, it worked like a charm, and now exports in the expected format for D7 for upward compatibility.

Take a look at the patch & if it helps, maybe there's still time to get it in before d6 support is dropped :)