Wrong behaviour: When a content type is being exported (by the means of the features.module) with an insert-enabled field, insert.module is not added to the list of dependencies, though the exported field definition does have the insert-enabled cruft. If one enables this exported feature without explicitly enabling insert.module, the insert functionality will be lost.

Expected behaviour: The insert.module should be automatically added to the dependencies of the exported feature in such a case, thus avoiding the need to manually enable the insert.module.

Comments

Category:bug» feature

This isn't a bug in Insert, modules don't inherently work with Features AFAIK. Moving to a feature request.

Status:Active» Needs review
StatusFileSize
new1016 bytes
new969 bytes

Finally made patches for this issue. Honestly I'm not sure this is worth it. Each features' list of 20+ dependencies drives me a little nuts. While a minor inconvenience, not including the dependency makes the feature more portable in the event that you don't have/want Insert on all your sites.

Well, I must admit that using features.module in some scenarios is… khm… at least debatable. OTOH I would say that if I don't want a field to be insert-enabled on a site, that would be a different feature. In other words: if I export an insert-enabled field to a feature, that's an explicit call from me for insert.module as dependency.

Additionally, your suggestion raises another question: if I enable a feature on a site without insert.module enabled that does have an insert-enabled field, wouldn't that feature be overridden in the very same second it gets enabled? Honestly, I would gladly trade in "portability of a feature" for the convenience of having a glimpse over all the features of a site if they are overridden or not.

Gonna test the patches tomorrow, though; thanks for your time working on this.

Took me a second to understand your question, so rephrased:

if I enable a feature [that includes insert configuration] on a site without insert.module enabled, wouldn't that feature be overridden in the very same second it gets enabled?

Good question, I don't know what the behavior would be. Though before or after this patch I don't think that behavior would change. Field module doesn't do the best job of setting default values on fields for newly enabled modules. It seems you have to go edit the field and resave it just to get the defaults to load properly.

Er, I think it doesn't have anything to do with fields' default values. Even if none of the fields have any default value, theoretically the feature (with insert-enabled fields, but without dependency on the insert.module) could be overridden (on a site without insert.module enabled), simply because the fields' settings have "changed" (I haven't tested it yet, though, but definitely going to do so later today). So, in this sense, your precious patches do should change the behavior, since enabling the feature (with insert-enabled fields, which would even mean dependency on the insert.module) on a site without insert.module enabled would definitely enable the insert.module (or the feature simply couldn't be enabled if the insert.module is not available at all), thus the feature wouldn't get overridden.

But anyway, I'm gonna check this later today.

Here's what I did:

  1. Installed D7 with minimal.profile.
  2. Added a basic Page CT with a multivalue, insert-enabled image field called Illustrations.
  3. Exported it via Features (without the patch in #2); insert.module wasn't autodetected, so wasn't added as dependency to the feature (though the field settings did have the insert functionality). FTR: the listed dependencies were features, field_sql_storage, image, node, text.
  4. Deleted the whole Page CT and flushed all the caches.
  5. Disabled, uninstalled and removed insert.module.
  6. Untarred the feature as usual.
  7. Enabled the feature via Features UI – and it wasn't overridden, even after a refresh, even after a cache flush and again a refresh.
  8. Re-exported the feature (without touching anything) via Features UI, and the insert-related field settings were still there, intact (the .features.field.inc had the same md5sum). OTOH, only features and image were listed as dependency.
  9. Re-saved the Illustrations field's settings (at the http://example.com/admin/structure/types/manage/page/fields/field_illust... page, without changing anything). Features UI still didn't show my feature as overridden.
  10. Re-exported the feature via the Features UI (again, without touching anything). Insert-related field settings were still intact; dependencies were still only features and image.
  11. Re-saved the Illustration field's instance settings (at the http://example.com/admin/structure/types/manage/page/fields/field_illust... page, without changing anything). Features UI DID show my feature as overridden.
  12. Re-exported the feature via the Features UI (again, without touching anything). Insert-related field settings were gone; dependencies were still only features and image.

So, to sum it up, this was a great lesson for me: looks like Features(?) is blindly copying in the field (instance) settings when enabling a feature, and only re-saving them gets the feature overridden. Sounds a bit weird (from a clickety-click view), but sounds pretty straightforward (from a coder view) at the same time. (Attached are all the exported features in the order they have been created, for reference.)

Moving on to actually test the patches.

Okay, the D7 version from #2 is working great: if there is at least one field with insert _button_ enabled, then insert.module will be added as dependency, automatically.

However, I think it would be even better if:

  • the other insert-related settings would trigger the insert.module dependency (to avoid the case when re-saving a field (instance) settings form without changes makes the feature overridden), and/or
  • the other insert-related settings would show up only when the "Enable insert button" is checked.

I must admit that this second one would be almost only a UX-only improvement. OTOH, while looking at this latter one, I could imagine that the Insert fieldset's #collapsed value would be TRUE only if there is no insert-related setting enabled – but this one is almost definitely a UX-only improvement, which should be another issue, I guess.

Gonna test the D6 patch before moving to RTBC.

Status:Needs review» Reviewed & tested by the community

D6 patch does its job as well, thanks @quicksketch for the great work!

Should I file a different issue for the UX improvements? (Or even try to forge a patch?)

Wow, thanks for that extensive testing! That does indeed sound exactly the way I would have expected Features to work, though I reached different conclusions. I think that the behavior is pretty much exactly what you would want, and adding the dependency is largely unnecessary. Insert module is a convenience, and if it's not on a different site, it's not a big loss in functionality. My take on this is the optional dependency is *increased* portability, rather than decreased, but apparently we're on different wavelengths about how useful the dependency is. Despite having them written, I'm still not sure I want to include either of the patches.

Should I file a different issue for the UX improvements? (Or even try to forge a patch?)

If you mean, "expand the Insert fieldset when the Enable Insert checkbox is enabled", I'd prefer not to make that change. The Insert fieldset can get quite long. Combine that with having FileField Sources (which I also maintain), doing the same thing, and the content type form can get 3-4x longer than it is currently. I'd like to keep these fieldsets collapsed at all times for the sake of display. Though in D7, we can use the group description ability to show that Insert is enabled in the title of the collapsed fieldset (similar to the way Vertical Tabs show you the enabled state of items inside them without being open).