Problem/Motivation

File entity provides field widgets for core file and image fields that use Entity browser for selection. File entity browser provided very similar widgets. To prevent redundant implementations we decided to merge and move them to the Entity browser itself, which happened in #2763505: Move file_entity and file_browser's Field Widget for Files and Images to Entity Browser.

Proposed resolution

Deprecate field widgets provided by file entity. Provide BC layer that will prevent duplicate widgets from appearing in UI. Idea was proposed in #2763505-4: Move file_entity and file_browser's Field Widget for Files and Images to Entity Browser.

CommentFileSizeAuthor
#21 file_entity-browser-2767185-20.patch13.36 KBBerdir
#21 file_entity-browser-2767185-20-interdiff.patch2.34 KBBerdir
#20 unneeded_parameters-2775173-6.patch9.53 KBBerdir
#20 unneeded_parameters-2775173-6-interdiff.txt4.45 KBBerdir
#17 file_entity-browser-2767185-17.patch11.03 KBBerdir
#14 file_entity-browser-2769695-14.patch9.5 KBdmsmidt
#14 interdiff-2769695-10-14.txt681 bytesdmsmidt
#10 file_entity-browser-2769695-10.patch9.33 KBBerdir
#5 file_entity-browser-2769695-3.patch9.34 KBBerdir
#2 2767185-depricate_entity_browser_widget-2.patch9.89 KBtlyngej
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

slashrsm created an issue. See original summary.

tlyngej’s picture

tlyngej CreditAttribution: tlyngej at Annertech commented
FileSize
2767185-depricate_entity_browser_widget-2.patch9.89 KB

Just to get something started (and my website back to a state where it is working) I'll post a patch that deletes the FileEntityBrowser class and the entity browser related settings in the schema.yml file.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Active » Needs review

Here's a patch that keeps the class empty and extend from the one. Haven't tested it yet, doing that now.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
FileSize
file_entity-browser-2769695-3.patch9.34 KB

Third try :)

The last submitted patch, 2: 2767185-depricate_entity_browser_widget-2.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 5: file_entity-browser-2769695-3.patch, failed testing.

The last submitted patch, 5: file_entity-browser-2769695-3.patch, failed testing.

The last submitted patch, 2: 2767185-depricate_entity_browser_widget-2.patch, failed testing.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
FileSize
file_entity-browser-2769695-10.patch9.33 KB

Not my day :)

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 10: file_entity-browser-2769695-10.patch, failed testing.

The last submitted patch, 10: file_entity-browser-2769695-10.patch, failed testing.

dmsmidt’s picture

dmsmidt
he/him
Primary language Dutch
Location Amsterdam 🇳🇱🇪🇺
CreditAttribution: dmsmidt at Triquanta commented
FileSize
interdiff-2769695-10-14.txt681 bytes
file_entity-browser-2769695-14.patch9.5 KB

Added @depricated to the comment block.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented

Discussed with @slashrsm. Tests aren't working because this relies on entity_browser, it fails if that module is not around. Note that this was a problem since this feature was added.

Due to that, we agreed that the best way forward is probably in fact going back to #2, deleting it. But we also need an update function then that looks at all form displays and tries to find a component that uses this and replaces it with the entity browser replacement.

dmsmidt’s picture

dmsmidt
he/him
Primary language Dutch
Location Amsterdam 🇳🇱🇪🇺
CreditAttribution: dmsmidt at Triquanta commented

@Berdir, yes that would be a better approach imho, since it leaves you with a cleaner state of a site.
That way we also don't have to figure out how to not show the widget in the UI.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Needs work » Needs review
FileSize
file_entity-browser-2767185-17.patch11.03 KB

Ok, here is a patch that does exactly that.

Worked fine here on one site, some additional confirmations would be great.

Status: Needs review » Needs work

The last submitted patch, 17: file_entity-browser-2767185-17.patch, failed testing.

Michelle’s picture

Michelle
she/her
Primary language English
Location Wisconsin, USA
CreditAttribution: Michelle at Mediacurrent commented

This patched fixed the broken form display config problem for me. Thanks!

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Priority: Normal » Critical
Status: Needs work » Needs review
FileSize
unneeded_parameters-2775173-6-interdiff.txt4.45 KB
unneeded_parameters-2775173-6.patch9.53 KB

Fixing the failing tests. Unrelated, but wouldn't be possible to make them work without removing this class, so combining into a single test. interdiff does apply separately but will fail.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
FileSize
file_entity-browser-2767185-20-interdiff.patch2.34 KB
file_entity-browser-2767185-20.patch13.36 KB

Wrong patches, sorry.

The last submitted patch, 20: unneeded_parameters-2775173-6.patch, failed testing.

The last submitted patch, 21: file_entity-browser-2767185-20-interdiff.patch, failed testing.

slashrsm’s picture

slashrsm CreditAttribution: slashrsm at MD Systems GmbH for Acquia commented 
+++ b/file_entity.install
@@ -98,3 +98,28 @@ function file_entity_update_8002(&$sandbox) {
+
+  /** @var \Drupal\Core\Entity\Entity\EntityFormDisplay $form_display */
+  foreach (EntityFormDisplay::loadMultiple() as $form_display) {

Should we check version of Entity browser and fail if it is not >= alpha7?

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented

Hm, version checking is not reliable with git checkouts and so on.

We could check if the widget exists, but accessing that in an update function is also tricky, as the caches might not have been rebuild. Version dependency is also not possible as it not a dependency.

I'd just commit it, do a new release and put it in the release notes as a bold notice..

slashrsm’s picture

slashrsm CreditAttribution: slashrsm at MD Systems GmbH for Acquia commented
Status: Needs review » Reviewed & tested by the community

OK.

slashrsm’s picture

slashrsm CreditAttribution: slashrsm at MD Systems GmbH for Acquia commented
Status: Reviewed & tested by the community » Fixed

Committed. Thanks.

  • slashrsm committed 1af9fe0 on 8.x-2.x authored by Berdir 
    Issue #2767185 by Berdir, dmsmidt, tlyngej: Deprecate file/image field...

The last submitted patch, 17: file_entity-browser-2767185-17.patch, failed testing.

The last submitted patch, 20: unneeded_parameters-2775173-6.patch, failed testing.

The last submitted patch, 21: file_entity-browser-2767185-20-interdiff.patch, failed testing.

Status: Fixed » Closed (fixed)

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