Entity Embed 8.x-1.0.0 release

This is of course blocked on #2577891: Entity Embed 8.x-1.0.0-rc1 release. Clarifying that in the issue metadata.

RC1 just shipped.

We need a plan of attack here, just like for #2577891: Entity Embed 8.x-1.0.0-rc1 release.

There will be a Media sprint at https://cluj2019.drupaldays.org in a few days. By the end of that week, we should have a plan here.

The two key blockers are IMHO test coverage:

1. For the text filter: #3052482: Expand EntityEmbedFilter's test coverage: test many more edge cases, change to kernel test
2. For the CKEditor integration: #3060396: Add CKEditor Widget JS test coverage

#3060396: Add CKEditor Widget JS test coverage Landed! There were a few small bugs with the caption transition between the drupal form and the widget's editable.

For the record, I have tested commit 3e7a6f84390bbf45c6cabc10301af00c1bc11ee8 (HEAD as of now) on our client project which makes extensive use of Entity Embed (even customizing some of the behaviors), and nothing broke.

The only remark I see of something new is that the ability to add links around _any_ rendered entity might be confusing for some users. I opened #3061449: Prevent drupal links being added to embedded entities that contain links when rendered so we can discuss it there, but I don't believe it should be a blocker for 1.0.

🥳 Thanks so much, @marcoscano! This is super valuable! Two questions, that would inform me and other users of Entity Embed on how much this helps validate the 1.0-worthiness of the current code base:

1. How complex is this site?
2. How extensive are the tests?

How complex is this site?

- We currently have 14 buttons (10 node types, 4 media types) in our most complete CKEditor toolbar.
- We have custom code to group these buttons in semantic groups, so instead of a direct button in the toolbar we have dropdowns for each group
- We have extended \Drupal\entity_embed\Form\EntityEmbedDialog to have more control on the form elements (we are adding new elements on the embed step).
- We are swapping the preview in the CKEditor by a placeholder inside hook_entity_embed_alter(), where we also inject metadata that is consumed by our theming layer to adjust things in twig.

FWIW, I haven't had real users test the new version for real, but I did manually test scenarios that exercise all custom code indicated above, and everything seems to work as expected.

How extensive are the tests?

Our test coverage is not really intended to exhaust all entity_embed features, but we do have explicit test coverage for our main critical features, such as:
- Embedding a couple of content types
- Embedding 3 media types
- Checking the grouping of the buttons
- Checking our customizations on the embed dialog step
- Checking some basic aspects of the FE rendering of embedded entities.

I quickly reviewed all open issues in Entity Embed queue as of now.

As of 1.0 blockers, apart from the ones mentioned in #6, IMHO it would be important to fix #3060642: Follow-up for #2924391: how to remove override and revert back to the inherited/original metadata as well before tagging a stable release.

As for timing and stability of the code, I think it would make sense to let the dust settle a little bit and try to gather more opinions of sites using the module. We tagged beta3 6 weeks ago, rc1 6 days ago, and since beta3 29 issues/commits shipped in the past recent weeks (which is huge! Thanks for all the hard work here 👏👏👏). I believe it would be great to have real sites test those out a bit more before calling the current code stable :)

My suggestion is we tag rc2 now (or after #3060642: Follow-up for #2924391: how to remove override and revert back to the inherited/original metadata, depending on how fast we think we can fix that), and start a call-out for sites to test it. We could wait then a month or two, and if no critical or major bugs appear, we 🚢it!

Also, I believe the more disruptive changes and new ideas being discussed in issues such as:

#2246533: Add 'embed' view mode for all entity types, to allow sensible embedding out-of-the-box
#2845085: Avoid conflating @EntityEmbedDisplay plugins and view modes
#2864705: Entity Embed's `image` @EntityEmbedDisplay plugin has poor usability when there are many image styles on the site.
#2844822: The preview in CKEditor does not use the same Twig template as the one on the front end (default theme)
#3061449: Prevent drupal links being added to embedded entities that contain links when rendered

do make sense in a new branch if necessary, but we can get into more details about how to prioritize those once 1.0 is done.

Thanks!

#10: Wow! That sounds pretty complex :) But that's great for Entity Embed, because that means we can quite safely conclude that in the path from beta2 to rc1 we seem to not have broken any existing customization code 🥳

#11: Thanks for your assessment, that's very, very, very much appreciated. 🙏🙏🙏 I will dig into #3060642: Follow-up for #2924391: how to remove override and revert back to the inherited/original metadata, but I think I agree with making it a blocker for 1.0. I definitely agree with postponing more disruptive issues to after 1.0 at best, and perhaps doing them requires a new branch to avoid disruption.

The only thing I do not completely agree with is to wait a few months before tagging a stable release. Per https://www.drupal.org/project/usage/entity_embed, 25% already updated to beta3, and we haven't seen a significant increase in bug reports. I think it'd be great to tag a stable release if say 10K sites are on the (yet-to-be-released) rc2 version and there's still no significant bug report increase. Besides, nothing is stopping us from shipping 1.1 with bugfixes. I think it's more important we bring these 40K sites on a release that gets Security Team coverage sooner than it is to have them wait longer for that with slightly fewer bugs.

I think it's more important we bring these 40K sites on a release that gets Security Team coverage sooner

You are right, I hadn't thought this from the perspective of the security support. I agree this is a strong incentive for getting there soon.

I think it'd be great to tag a stable release if say 10K sites are on the (yet-to-be-released) rc2 version and there's still no significant bug report increase

➕1️⃣ to this! :)
I was reasoning in a time-based mindset, but it's IMO totally fine too to have this "user validation" metric also be the number of sites upgrading. I'm not even sure what that number should be TBH... as long as we see sites are adopting a release with the new features introduced in the past weeks, and we see no significant bugs introduced by the new code, I think we should be good to go.

Thanks!

Entity Embed 8.x-1.x is down to 37 open (non-fixed) issues. Only seven of those are bugs. 2 of those are upstream bugs in CKEditor. 4 are bugs in entity_browser integration. One is not really a bug, it's an incorrect use of an interface, and fixing it involves some BC risk.

Looking at https://www.drupal.org/project/usage/entity_embed, over 30% of Entity Embed 8.x-1.x users have updated to beta3, rc1 or rc2. No significant problems have arisen.

The filter + CKEditor integration test coverage have landed, which I considered in #6 to be the only real blockers. Much more clean-up has happened since, and many more minor fixes have landed, especially while working on porting this to Drupal core patches.

This also enabled the following core patches to be extracted from Entity Embed's current code base plus #2998005: [PP-1] Support Drupal core's Media Library:

1. #2940029: Add an input filter to display embedded Media entities
2. #2994696: Render embedded media items in CKEditor
3. #2994699: Create a CKEditor plugin to select and embed a media item from the Media Library
4. #2994702: Allow editors to alter embed-specific metadata, as well as `data-align` and `data-caption`

See #2801307-66: [META] Support WYSIWYG embedding of media entities for an overview of the current status of Media embedding in Drupal core.

So … time to tag 8.x-1.0: https://www.drupal.org/project/entity_embed/releases/8.x-1.0 🥳 Special thanks to @phenaproxima and @marcoscano. Extra super special thanks to @oknate. Without you we would be in a far worse place. Thanks so much, @oknate!

I would recommend that @oknate also be made a maintainer of this module, if he'd be willing to 😊

And of course, thanks to @wimleers, without who's leadership on this initiative we wouldn't be where we are today. It's been a pleasure to work on this initiative with you, and learn from you. And yes, I'm willing to help out as a maintainer. :)