Default file entities are not exportable by features (Sibling Issue)

Adds a new step in install process which creates the default displays (_file_entity_default_displays()).
The patch then removes the defaults where they are set in file_entity.module.

Works perfectly in combination with [2104193]

1: file_entity_remove_file_display-2192391-01.patch queued for re-testing.

Tests pass! Any chance this one will get committed today as well? Thanks!

Committed to http://drupalcode.org/project/file_entity.git/commit/13fa2b0

Thanks again, @aaron, you rock. :-)

See #2271909: Default file display for file entities no longer set for details on the revert.

Reopening, the patch was reverted.

Here is a slightly updated patch: it matches the image settings better and applies to the latest Git. It works for me on a vanilla Drupal 7.28 site.

One thing it doesn't do (which I'll work on in a moment) is match the Video and Audio settings.

Honestly, this might be better as taking a data structure like was in file_entity.file_default_displays.inc and creating those, rather than code...

Ok, here's my last version of this patch. It matches the settings exactly like the old exports, including the audio and video settings. It works in all my testing!

You can see the steps I took to test in the issue about the regression:

https://drupal.org/node/2271909#comment-8878571

Never say "last"! Hopefully, this really is my last patch. :-) I found a bug in the related Media patch, that I described in this comment:

https://drupal.org/node/2104193#comment-8878701

It was causing the display to only get created if there were ZERO other displays on the individual view mode. This bug is also present in previous versions of the patch here! So, it worked for me on a completely fresh install with no displays in the database, but not if even one other was created somehow for a give view mode. This may be the cause of @brantwynn's problems with Demo Framework!

So, here is an updated version of the patch with that bug fixed.

Please let me know what you think!

#16 works for me. Thanks!

Marking RTBC. Removing the default config into a file_display_save() is a good approach.

Committed to http://drupalcode.org/project/file_entity.git/commit/08509fd.

Hi aaron,

When looking at the commit (http://cgit.drupalcode.org/file_entity/commit/?id=08509fd) I see that the install file has been modified, but the
file_default_displays hook is still implemented in file_entity.file_default_displays.inc (http://cgit.drupalcode.org/file_entity/tree/file_entity.file_default_dis...). Shouldn't that hook implementation be removed as well, cfr the patch in #16?

Eh. Yes, confirm that aaron's commit was not the right one. That file needs to go.

This patch remove file_entity.file_default_displays.inc (ref #21 and #22).

Since this was part of the original patch, and that was RTBC, marking this as RTBC as well!

Sorry for any past confusion. Trying to set things right by committing this patch, but unable to do so because file file_entity.file_default_displays.inc is missing.

Sure you are on the correct branch aaron?

git branch
* 7.x-2.x
git log --oneline file_entity.file_default_displays.inc
85ae95f Issue #2067909 by mongolito404: Stopped altering ..
cdd70ca Issue #2067909 by Devin Carlson: Remove default fil..
4869238 Issue #2073001 by Dave Reid: Assigned low weigh..
08cfb0f Issue #2066517 by Devin Carlson: Provided a default display..

Reverted #20.

As mentioned in #2271909-2: Default file display for file entities no longer set, file displays are ctools exportables like many other configuration objects in D7 and the ctools api specifies a standard way to include default content, so I'm not sure why it isn't (or maybe can't be) supported by Features?

Also, if I remember correctly, when it comes to Views, modules are able to provide/alter default views, which users can override and then export to features without issue.

If that's the case, then I'm not exactly sure how that is possible (is there a problem with File Entity's ctools integration or does Features do some special casing for views) but it seems like there should be a solution that would allow file displays to work in the same manner as views without requiring modules/installation profiles to do all of their customizations and cleanup in hook_install() hook_uninstall().

The use of these hooks is also makes it difficult to easily revert back to a stable file display configuration (where problems configuring file displays is one of the most common support requests) and is a pain to maintain (this same ~50 line manual approach will have to be done for every media provider module and adding/removing file displays will require tricky update hooks).

I'd appreciate any insight.

Could this be connected to #2418147: module_enable/disable should reset static caches. ?

I am also pretty confused by this patch. The sibling issue for the Media module #1702700: Implementation of hook_file_default_displays makes it impossible to export image display settings via features references #1702700: Implementation of hook_file_default_displays makes it impossible to export image display settings via features, which states:

[It's] impossible to export file display settings for the Image display style for the preview, teaser, and full build modes. This is because default display style configuration is implemented in hook_file_default_displays() which competes with the implementation of hook_file_default_displays from Features.

However, I was not able to find any implementations of hook_file_default_displays in Features version 7.x-2.3 (although it's possible my grep skills are merely lacking). Could anyone give me some insight as to why exactly we have to store the defaults in the database rather than use hooks to define them?

For anyone for read my post, I think I was a bit confused. The hook_file_default_displays implementation in Features that was mentioned in the quotation above refers to the fact that the generated Features modules contain an implementation of hook_file_default_displays and *not* that Features itself implements hook_file_default_displays.

I looked a little bit into the Views stuff that was mentioned in #27. I created a module which used hook_views_default_views to define a View. I enabled it, made some changes to the View, and then went to Features to see if I could test the theory that "when it comes to Views, modules are able to provide/alter default views, which users can override and then export to features without issue."

The View created by my module appeared in the Features components UI, but it showed up in red, meaning there was a conflict. I was still able to create a Features module, but I was not able to enable it due to the conflict. Upon closer inspection, it appeared that my Features module was also attempting to use hook_views_default_views to define the View. It therefore made sense that I couldn't enable the View; Drupal had no concept of which one should be considered a "base" or default and which one should be allowed to override it. All it knew was that there were two different hook implementations attempting to define the same view.

I think a similar issue is present with hook_file_default_displays. Any Features-generated module would use hook_file_default_displays to define the display settings, but then Drupal wouldn't know which implementation should take precedence. Therefore, the only way to make the file settings exportable via hook_file_default_displays is to define the defaults elsewhere, either in the database, or perhaps in another module that could be disabled if someone wanted to create a Feature for the file display settings.

IMHO this is a bug in features then. There is a way to tell drupal which module overrides which, and that is module weights. The feature simply needs to have a higher weight than the module defining the defaults - and another way to check for conflicts.

Features seems to be severely broken in that regard at the moment. And more than that: There is no official way to "just make it work".

Just for the record. My idea with module weights just works:

Modules which come later in module_implements('ctools_plugin_api') override the earlier ones. That means exporting file_displays into a module which's name comes after "file_entity" in the alphabet should work fine out of the box. For all other modules you have to increase the module-weight.

I'm also attaching a test module that overrides the default video display to be 600x400 and enable loops. When you just enable the feature those overrides won't work since the module is called "aaa_file_entity_test". If you increase the module's weight and clear all caches the values suddenly become effective.

The features module should deal with this not the file_entity module.

Just for the record. My idea with module weights just works:

Modules which come later in module_implements('ctools_plugin_api') override the earlier ones. That means exporting file_displays into a module which's name comes after "file_entity" in the alphabet should work fine out of the box. For all other modules you have to increase the module-weight.

I'm also attaching a test module that overrides the default video display to be 600x400 and enable loops. When you just enable the feature those overrides won't work since the module is called "aaa_file_entity_test". If you increase the module's weight and clear all caches the values suddenly become effective.

The features module should deal with this not the file_entity module.

Good news: I've found a workaround. It also fixes-up this issue for media and all its extensions.

Bad news: It hacks ctools into invoking non-feature implementations of hook_file_default_displays() first. Also it needs the patch from #2075885: ctools_plugin_api_info() should provide $owner and $hook to {api-hook}_alter to work.

In _ctools_export_get_defaults(), there is a drupal_alter for the 'default hook'. Just implement hook_file_default_displays_alter() and everything should be fine.

@heddn: And your suggestion is that features should generate a hook_file_default_displays_alter() instead of a hook_file_default_displays()? Could you please explain that in a bit more detail? Is there already a related issue for features?

Features uses ctools exportables internally. For example, to override a view that is stored in features, you'd implement hook_views_default_views_alter. To override a file display, implement hook_file_default_displays_alter. I believe that 'file_default_displays' is the 'default hook' for file_entity displays.

@heddn I think at least one of us is mistaken what this issue is about. IMHO this issue is about the following problem:

1. Edit the settings of some file_display which has a default provided by file_entity.
2. Export the changed file_display into a feature named "abc" (anything that comes before "file_entity" in the alphabet is fine).
3. Install this feature in a new site.

Expected result: feature overrides built-in defaults of file_entity.
Actual result: file_entity's built-in defaults override the feature. (No amount of feature reverting is able to change that)

It's not about writing custom modules -- thanks for explaining the basics though ;)

re #27: The downside to providing a "default" file entity display means that it is not easily overridable by features/ctools. There's feature_overrides, but even then there are still problems with it. I think I prefer inserting/removing from the database in some type of hook_install/enable than through a ctools/features "default hook".

re #42: You are right, I wasn't very clear. I guess I'm in favor of the original commit that got reverted in #27. I'm not a big fan of working around functionality in ctools/features through hook_ctools_plugin_api_alter. It seems rather fragile.

re #43: Lets compare those two approaches:

The defaults-in-DB-approach (reverted in #27) needs three things to work:

1. The file_entity module has to put it's defaults into the DB instead of the default-hook.
2. All other modules defining defaults (media and it's extensions) need to do the same.
3. The resulting features are then overridden initially. Site-builders have to care for that and implement an extra feature-revert into their installation profiles.

---

The alter-hook-approach (patch in #37) implements the desired behaviour (features override modules) in a direct way. No additional workarounds in media or it's extension or from the site-builders needed.

So which one exactly does look fragile?

I'm setting this to needs review again to get other opinions.

I've been using the first approach for quite some time using the file_entity patch from #23 along with the media patch from #91.

These patches remove all default file displays from file_entity and media, so upon installation, nothing is selected in admin/structure/file-types/manage/*/file-display/*. This allows exporting all those display settings in a custom feature which can safely implement hook_file_default_displays without the fear of being overridden by file_entity or media. But for some reason, as outlined by @torotil in the last comment, this feature is initially overridden so it needs to be reverted after being installed.

This solution is good when using a features workflow, but not so much when file_entity / media are installed as-is. This is what this everlasting issue is all about!

I'll try #37 later today, as I think this is the way to go. There would need to be a similar hook_ctools_plugin_api_alter for the media module (could file_entity check for both?).

There would need to be a similar hook_ctools_plugin_api_alter for the media module (could file_entity check for both?).

The patch takes care of this for all file_default_displays plugins (not just it's own). So no additional change to media is needed.

I just tried a minimal installation with ctools patched with #1 + features + strongarm + file_entity 2.x-dev patched with #37 + media 2.x-dev. I then modified the image file display settings, created a feature with all of these, reset the site, enabled my feature.

Once enabled, my feature doesn't appear overridden. So far so good. But when I look at the image file display settings, though the ones I added do appear, the ones I've disabled (defined by file_entity / media) are still active and their settings are not affected by my feature.

In fact, the image__default__file_field_image and image__default__file_field_image variables aren't available for export (as they were when I was using the #23 / #91 approach) so this explains that. If I define these manually, everything works fine!!

Why are these two variables missing from the export list !?!

Update: The missing exports are caused by the media module, some part of patch #91 fixes that. We're getting close! Nope... I just tried patching media with #91 along with #37 and the exports are still missing.

I've traced it to _features_export_build() to the section:

if ($map = features_get_default_map($component)) {
  foreach ($map as $k => $v) {
    if (isset($options[$k]) && (!isset($feature->name) || $v !== $feature->name)) {
      unset($options[$k]);
    }
  }
}

$options is the array of components (file_displays) that will be shown in the features interface. features_get_default_map() maps all items from the default hook to their modules. The loop removes all components that are defined by any module from the $options, which means they can't be exported (except if "allow conflicts" is checked).

So while #37 makes things work once the feature is exported, it doesn't deal with features notion of conflicts. It seems that it's not possible to solve this in file_entity alone.

Maybe having default displays defined in hook_install + the #23 / #91 patches could be a better solution then?

Also, I was wondering if there was a reason for the numerous *__*__file_field_* exports instead of something similar to a variable field_bundle_settings_file__* export (like what is used for node settings export). Wouldn't this be a lot simpler (and more coherent with node entities), even if requiring some refactoring of ctools stuff in file_entity?

I think that tells us that features/ctools simply don't include support for "module provided defaults that may be overridden via features". IMHO that's a major bug in features.

Regardless of whether #31/#91, #37 or some other solution is the way to go, it needs to be supported directly in ctools/features.

Re-rolled patch #23 to apply cleanly on 7.x-2.0-beta1+26-dev

Re rolled patch from #37.

Patch #55 produces terrible load on file system because of system_rebuild_module_data();
I adapted patch to use system_get_info('module'); so it works much faster.

@dmitriy.pokhodenko do you have a patch and interdiff for review and testing? Thanks.

Hi, sorry for delay here is a patch.

#59 works really well! See also #2104193-101: Default file entities are not exportable by features (Media File Entity Overridden) for the same approach for the media module.

+1 for RTBC

#59 works really we

+1 for RTBC

#59 doesnt works!

How can this below be overwritten by features? I dont see any setting in features for this field_image. Maybe if i manually write into myfeature.file_default_displays.inc file but i havent tryed yet. Or what am i doing wrong? thx

from file_entity.file_default_displays.inc:

    // Image previews should be displayed as image thumbnails by default.
    $file_display = new stdClass();
    $file_display->api_version = 1;
    $file_display->name = 'image__preview__file_field_image';
    $file_display->weight = 49;
    $file_display->status = TRUE;
    $file_display->settings = array(
      'image_style' => 'thumbnail',
      'image_link' => '',
    );
    $file_displays['image__preview__file_field_image'] = $file_display;

@mibfire : This patch is for exporting the file entities themselves, not the file display settings as your comment suggests you are trying to achieve.

@PascalAnimateur

https://www.drupal.org/files/issues/file_entity_remove_file_display-2192...

This patch is aiming exactly what i am talking about. So it is related to file displayes as well! The question is do we need both of them or somehow it can be solved without removing the default file displayes(provided by file entity module).

@mibfire : Sorry, my bad, I thought this was another issue I worked on recently!

Indeed, you're right: patch #59 doesn't work despite my comment #60 (forgot to update this thread when I rolled back to #51).

As for overriding / exporting the default file displays provided by file_entity / media, this is how what I currently use in my custom distribution:

; File entity
projects[file_entity][type] = module
projects[file_entity][patch][2192391] = https://www.drupal.org/files/issues/default_file_entities-2192391-51.patch
; Media
projects[media][type] = module
projects[media][patch][2104193] = https://www.drupal.org/files/issues/media_remove_file_display_alter-2104193-100.patchf.diff

In short, I remove file_entity.file_default_displays.inc and media.file_default_displays.inc so that I can export the file displays properly. I think you also have to revert the file displays when enabling your module for the first time.

Find this issue a little confusing and unable to grasp that what the real cause of this problem is even after reading all comments in the this thread.

@PascalAnimateur in #66 run file_entity without file_entity.file_default_displays.inc (patch from back in #51) and a patch from #101 found in #2104193: Default file entities are not exportable by features (Media File Entity Overridden). Would love to test and provide feedback though the issue would do with some information about how and why.

Some information found in this, but it stalled two years ago.

I think I have a fix for this issue over in #2446485: Proper way to define module defaults.. The patch includes a generic version of #59 and also changes the component listing so that non-feature implementations of a default hook (ie. hook_file_default_displays()) are not seen as conflicting (perhaps addressing #63 and #66)

Please help me test this!

follow up in:
#2446485: Proper way to define module defaults.

Re-roll of patch in #51 against 7.x-2.x.

None of these patches were good, because file_default_displays wasn't removed from file_entity_ctools_plugin_api and if you use drush make(normal case without --overwrite) to build a site it won't remove the file_entity.file_default_displays.inc. I fixed this in the attached patch.

probably should follow up in #2446485: Proper way to define module defaults.

See related issue there is a.patch

Is this patch (#72) still necessary for latest Drupal 7 version?

@drupalfan2 , this issue is closed as a duplicate

if still you're interested in this functionality, please try the patch in the parent issue and provide feedback in that issue, not this one please.

follow up in THIS issue please:
#2446485-41: Proper way to define module defaults.

actually a more appropriate status for this issue would be postponed

postponing this on:

#2446485-41: Proper way to define module defaults.

Appears that the solution to this might be best done in the 'features' module

I'm open to being convinced otherwise however.

Updating patch #74 to be able to patch the current module version.

Re-roll of patch in #72 against 7.x-2.x.

Hi xlin, have you tested out the alternative solution?
#2446485-41: Proper way to define module defaults.

Can you please review and test both solutions and report your findings back to us?

Thanks!

@joseph.olstad I work with @xlin and we have switched to using the features patch. It seems to be working. With patch from features #2446485-57: Proper way to define module defaults. I am able to export and take ownership of "file_type" and "file_display" components, with default hooks in place i.e. without needing patch in #83 or media patch #2104193-113: Default file entities are not exportable by features (Media File Entity Overridden).

One quirk to note is that if there is an alter hook that appends data (e.g. hook_file_default_types_alter() is used to append a mime type to the array), it will be added again on each feature export so you'll end up with duplicate entries. I've noticed this before in features so I don't think it is caused by the patch in #2446485-57: Proper way to define module defaults. just something to watch out for.

Re-roll #37 for 7.x-2.37.

based on #85 , I'm going to close this as 'Closed (works as designed)'

see the patch for the features module.
#2446485: Proper way to define module defaults.

Which I'm also the maintainer of, so I think I'll jam in 2446485 now and tag a new release.