Update inline block usage on import [#3359137]

Problem/Motivation

When reinstalling a website with default content using layout builder overrides, there are access problem when editing inline blocks due to #3047022: Layout builder fails to assign inline block access dependencies for the overrides section storage on entities with pending revisions (and #3090200: Paragraph view access check using incorrect revision of its parent, leading to issues viewing paragraphs when reverted host entities or content moderation is used if using paragraphs in inline blocks).

I tried to apply patches from those issues. It was then ok for paragraphs but not for inline blocks with media, even after reinstallation.

I dug to see were the problem was coming from and found out it was due to the inline_block_usage table not populated when default content are imported.

Proposed resolution

I initially made a service to insert data into the inline block usage table, then wanting something generic, I saw that Default Content provides events. So I converted that into a generic optional event subscriber.

Finally using only this event subscriber, I no more need the patches on Core or Paragraphs and after reinstallation it is ok out-of-the-box.

I will provide the MR.

For info in my default_content usage, here are the patches I have applied:

            "drupal/default_content": {
                "1 - Import should overwrite files - https://www.drupal.org/project/default_content/issues/3200212#comment-14332801": "https://www.drupal.org/files/issues/2021-12-08/default_content-3200212.patch",
                "2 - Don't add translation, if exist - https://www.drupal.org/project/default_content/issues/3176839#comment-13859359": "https://www.drupal.org/files/issues/2020-10-14/default_content-3176839-2.patch",
                "3 - Add a Normalizer and Denormalizer to support Layout Builder - https://www.drupal.org/project/default_content/issues/3160146#comment-14814050": "https://www.drupal.org/files/issues/2022-12-06/default_content-3160146-53.patch"
            },

Comment	File	Size	Author
#17	default_content-3359137-17.patch	5.44 KB	drdam
#3	default_content-3359137-3.patch	6.12 KB	grimreaper

Issue fork default_content-3359137

Show commands

Start within a Git clone of the project using the version control instructions.

Add & fetch this issue fork’s repository

Or, if you do not have SSH keys set up on git.drupalcode.org:

Add & fetch this issue fork’s repository

3359137-update-inline-block-usage-on-import changes, plain diff MR !33
Check out this branch for the first time

Check out existing branch, if you already have it locally

About issue forks

Comments

Comment #1

9 May 2023 at 13:41

Grimreaper created an issue. See original summary.

Comment #2

9 May 2023 at 13:46

Grimreaper opened merge request !33

Comment #3

grimreaper

French

France 🇫🇷

commented 9 May 2023 at 13:48

Assigned:	grimreaper	» Unassigned
Status:	Active	» Needs review

Status	File	Size
new	default_content-3359137-3.patch	6.12 KB

Uploading patch for easier Composer usage.

Thanks for the review.

Comment #4

andypost

he/him

Russian

commented 9 May 2023 at 17:52

+++ b/src/EventSubscriber/InlineBlockUsageFixer.php
@@ -0,0 +1,156 @@
+    EntityTypeManagerInterface $entityTypeManager,
+    ?InlineBlockUsageInterface $usage,
+    ?SectionStorageManagerInterface $sectionStorageManager

could use https://www.php.net/manual/en/language.oop5.decon.php#language.oop5.deco... to simplify definition above

Comment #5

grimreaper

French

France 🇫🇷

commented 10 May 2023 at 06:49

Thanks @andypost for the feedback and pointing me to a constructor syntax I forgot or didn't know it exists.

But that would raise the PHP minimum version required by the module. And as the module still supports Drupal 9, I am not sure it is a possible change to do right now?

Comment #6

andypost

he/him

Russian

commented 10 May 2023 at 08:52

Good point, yes, we need to keep BC!

Is there a way to add basic test for new class?

Comment #7

berdir

German

Switzerland

commented 10 May 2023 at 09:48

I did't look at the patch yet and may layout builder understanding is very limited. I don't quite understand why we need to help it maintain its internal info, seems like a workaround for limitations in layout builder itself?

also, the layout builder integration isn't even committed yet, that should be the focus, specifically in regards to tests which it hasn't yet. And if this is really required for it to work properly, it could also be part of the same issue.

Comment #8

grimreaper

French

France 🇫🇷

commented 12 May 2023 at 14:29

Hi,

Thanks both of you for your feedbacks.

and may layout builder understanding is very limited

Me too, so I may have taken the problem in a wrong way.

seems like a workaround for limitations in layout builder itself?

Yes, it may be a workaround for a core bug.

The only place were the inline block usage is inserting entries inthis table is in InlineBlockEntityOperations::saveInlineBlockComponent(), which is only call in InlineBlockEntityOperations::handlePreSave() which is call in layout_builder_entity_presave. So I guess that it should work out of the box.

But in InlineBlockEntityOperations::saveInlineBlockComponent(), there is an if statement before inserting in the table. So I guess the way default_content is exporting Layout Builder data may be wrong (at least when reinstalling a website). And in this case it should be handled in the other issue.

Is there a way to add basic test for new class?

Before doing that, like @Berdir pointed out, it may be a workaround for either a core bug or a bug in the other default_content issue about Layout Builder.

also, the layout builder integration isn't even committed yet, that should be the focus, specifically in regards to tests which it hasn't yet. And if this is really required for it to work properly, it could also be part of the same issue.

Agreed. I needed a generic quick fix now for multiple projects using both Layout Builder and Default Content and I didn't wanted to risk breaking the MR on the other issue with this change in case people get patches for Composer using the MR URL directly.

Comment #9

berdir

German

Switzerland

commented 12 May 2023 at 20:36

> Agreed. I needed a generic quick fix now for multiple projects using both Layout Builder and Default Content and I didn't wanted to risk breaking the MR on the other issue with this change in case people get patches for Composer using the MR URL directly.

That makes perfect sense, thanks contributing and working on this publicly.

Comment #10

jcandan commented 4 August 2023 at 21:11

Status:

Needs review

» Reviewed & tested by the community

Specified this patch along with those listed in the description:

            "drupal/default_content": {
                "1 - Import should overwrite files - https://www.drupal.org/project/default_content/issues/3200212#comment-14332801": "https://www.drupal.org/files/issues/2021-12-08/default_content-3200212.patch",
                "2 - Don't add translation, if exist - https://www.drupal.org/project/default_content/issues/3176839#comment-13859359": "https://www.drupal.org/files/issues/2020-10-14/default_content-3176839-2.patch",
                "3 - Add a Normalizer and Denormalizer to support Layout Builder - https://www.drupal.org/project/default_content/issues/3160146#comment-14814050": "https://www.drupal.org/files/issues/2022-12-06/default_content-3160146-53.patch",
                "4 - Update inline block usage on import - https://www.drupal.org/project/default_content/issues/3359137#comment-15042582": "https://www.drupal.org/files/issues/2023-05-09/default_content-3359137-3.patch"
            },

With this, I successfully ran drush dcer node ${NID} --folder=modules/custom/my_custom/content/, imported my database without the content, enabled my_custom module, and the content is there.

THANK YOU for these patches!

Note: I found this while trying to get past the issue described in #3160146. I did not specifically check for the issue described here.