Child issue of #2860796: Plan for contributed modules with Media Entity API in core

Here we need to:
- Check the tests that interact with media_entity and update them if they need to use the new API
- Move the Upload.php EntityBrowser widget plugin integration from media_entity_image to the Entity Browser project.

CommentFileSizeAuthor
#13 interdiff-2902324-11-13.txt2.38 KBchr.fritsch
#13 2902324-13.patch19.58 KBchr.fritsch
#11 interdiff-2-11.txt12.44 KBmarcoscano
#11 2902324-11.patch18.79 KBmarcoscano
#2 interdiff-plugin-class.txt5.57 KBmarcoscano
#2 2902324-2.patch6.71 KBmarcoscano

Comments

marcoscano created an issue. See original summary.

marcoscano’s picture

marcoscano
Primary language Portuguese, Brazil
Location Barcelona, Spain
commented
Status: Active » Needs review
StatusFileSize
new 2902324-2.patch6.71 KB
new interdiff-plugin-class.txt5.57 KB

First stab at moving and refactoring the plugin. I'm including also a diff of both plugin classes to make it easier to review the changes.

I haven't done any work on the tests though.

mstef’s picture

mstef commented

edit: The error I posted was stemming from dropzone.

mstef’s picture

mstef commented

Looks like the only upload widget option is "Upload images as media items". Can we get support for other media bundles?

marcoscano’s picture

marcoscano
Primary language Portuguese, Brazil
Location Barcelona, Spain
commented

@mstef I don't understand your comment from #3, did you edit it and removed the original content?

Re: #4, the purpose of this issue is to ensure parity between the scenario "Media Entity + Media Entity Image + Entity Browser" and "core Media + Entity Browser", after Drupal 8.4.x and higher. So the only work here is to ensure the widget that before was provided by Media Entity Image will still be available, now as part of the Entity Browser module.

If you need something else, the best approach would be to open a feature request as a separate issue.

mstef’s picture

mstef commented

You can disregard everything I said. I had some confusion.

Thanks for the patch.

garphy’s picture

garphy
Primary language French
Location Montreal, Canada
commented

Apologies if i'm mixing up thing here but isn't there a need to also port Media Entity Browser to the new "Media in core" API ?

I just installed a dev 8.5.x Core with patched Entity Browser and when I try to select "Entity browser" as a widget for my ER field, it says "No entity browser selected"

Unless I'm mistaken, there's yet no way to add Media Entity Browser into the mix right now as it stills depends on Media Entity which we wouldn't enable in this case.

marcoscano’s picture

marcoscano
Primary language Portuguese, Brazil
Location Barcelona, Spain
commented

@garphy Yes, Media Entity Browser will need a separate issue for dealing with its upgrade to the new branch. I just opened #2906068: Create a new branch compatible with the new Media in core API for that. This issue is about upgrading Entity Browser itself.

Thanks!

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
commented
Status: Needs review » Needs work

I tested it functional and it worked very well. Looked over the code and just found one minor:

+++ b/entity_browser.info.yml
@@ -7,6 +7,6 @@ test_dependencies:
+  - drupal:media

Should be drupal:media (>= 8.4)

marcoscano’s picture

marcoscano
Primary language Portuguese, Brazil
Location Barcelona, Spain
commented
Assigned: Unassigned » marcoscano

working on it, also updating the tests.

marcoscano’s picture

marcoscano
Primary language Portuguese, Brazil
Location Barcelona, Spain
commented
Assigned: marcoscano » Unassigned
Status: Needs work » Needs review
StatusFileSize
new 2902324-11.patch18.79 KB
new interdiff-2-11.txt12.44 KB

Addressed #9 and tried to update the tests. Let's see what the testbot has to say about it.

Status: Needs review » Needs work

The last submitted patch, 11: 2902324-11.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
commented
Status: Needs work » Needs review
StatusFileSize
new 2902324-13.patch19.58 KB
new interdiff-2902324-11-13.txt2.38 KB

Trying to fix the tests. Let's see...

jzavrl’s picture

jzavrl commented
Status: Needs review » Reviewed & tested by the community

Can confirm the patch from #13 works great for me.

slashrsm’s picture

slashrsm commented
Status: Reviewed & tested by the community » Fixed

Committed, pushed new branch and rolled the first alpha release. Thanks!

Status: Fixed » Closed (fixed)

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