Inline Blocks on Layout Builder Fail to Clone Correctly

https://www.drupal.org/project/quick_node_clone has a fix for this built-in, perhaps you could join efforts and take the solution for this module. As paragraphs (if not paragraph library used) are always "owned" by the parent entity, they have to be copied and not referenced.

Until that is fixed we're using https://www.drupal.org/project/quick_node_clone

entity_clone helps us to clone bundles / configuration.

This must not be considered "works as designed": Paragraphs (P) are Entity Reference Revisions (ERR), and contrary to normal Entity References like media an ERR must have exactly one parent. If we don't adhere this role, this will lead to inconsistent data at best (every time a shared P parent is edited, a new revision of the P is also created, all other parents would point to an outdated P version), or unrecoverable data loss at worst (when one parent of a shared P is deleted).

Bumping status to critical due the potential data loss in this scenario.

Not only other cloning modules, but also devel_generate and similar modules have the same problems, perhaps we should place a common issue into ERR?

See #2568187: Improve EntityReferenceRevisionsItem::generateSampleValue to be recursion-safe and devel_generate compatible

This is definitely not critical, but cloning content with blocks added via Layout builder isn't working properly in general (across both entity_clone and replicate). The scenario described as working in the Issue summary isn't really working properly due to the fact that the blocks themselves aren't cloned which leads to both Nodes referencing the same block via LB (these are marked as not reusable so should only ever be used by 1 entity). I think fixing that would fix the paragraphs issue as well.

Been working on making layout builder and entity clone work together. This patch seemed to have been working for my case.

Testing steps:
1. Enable layout builder
2. Create a custom block type and two paragraph types. Place child paragraph within parent paragraph, then place parent paragraph within custom block type
3. Create a content type
4. In Manage Display, enable "Use Layout Builder" and "Allow each content item to have its layout customized."
5. In "BLOCKS AVAILABLE FOR PLACEMENT", allow placement of custom blocks
6. Create test node, in [node_path]/layout, add a block and select create custom block and select the block type you created
7. Save layout.
8. Clone node
9. In the cloned node, go to [node_path]/layout, edit the block and save
10. Go back to the original node layout ([node_path]/layout), try to edit the block and save.
11. You should see no locked paragraph or blocks

Removed the layout_builder dependency and moved it into after the check of layout builder existence

After applying patch in https://www.drupal.org/project/entity_clone/issues/3060223, I noticed there is an issue where the layout field in the original node is referencing the new block revision. This patch will fix the issue, and also make sure this patch work with the patch in issue https://www.drupal.org/project/entity_clone/issues/3060223

I was not able to apply both my patch and the patch in https://www.drupal.org/project/entity_clone/issues/3060223 through composer, since they are modifying the same file. So I rolled the patch in https://www.drupal.org/project/entity_clone/issues/3060223 into my patch as well. For those who need both, here it is

The previous roll was incorrect. This should work now. I hope...

Fixing a problem in the previous patch where node clone with multiple blocks or sections

Fixing issue with fields in custom layout that has no block revision ID is not getting cloned properly

I am afraid that this patch does not work well on multilingual systems.

If you create a node that is using layout builder with two translations and then visit the 'clone' tab you only see the layout builder blocks for the locale you are on, but when you click 'clone' you get a copy containing all locales. The resultant clone will likely always be not what the user expects. The locales you were not on will just be pointers ... is kinda like if this patch wasnt being used for those locales.

I fear the core problem here, though, may not be with your addition. I could be understanding this wrong, but ContentEntityCloneBase::cloneEntity probably should be looping over the locales associated with the entity. It calls 'get' on the field, but this is only going to return the field version of the active language (see ContentEntityBase::get)

@weilinggu I am working on the next version of this patch ... am nearly there.

I just noticed in your change that you save the 'original' entity as well as the clone. Was this intentional? I am taking it out, but please let me know, if you can.

Cheers!

Attached is another patch. This handles the multilingual case.

One question I have, though. Do we need any adjustments to the form for this entire goals? The previous version of the patch modified the form so I kinda left it (although made the field set and disabled). The code that processes the form ignores the settings on the form for layout fields, which actually seem right (a layout builder field should always be deeply cloned to avoid any system integrity issues). Maybe it is needed for even deeper cloning within a layout field? (not my use-case).

Some Findings to patch:

While I am cloning a page with so many entity references.It is throwing this error:

Maximum functioning nesting level of '256' reached aborting! in Drupal\Component\Utility\Html::getId() (line 218 of core/lib/Drupal/Component/Utility/Html.php).

Usually all the entities are displayed before clicking on clone button.But if there are many entities then it is not able to display and it is throwing above error.
So I have removed these lines:

if ($referenced_entity instanceof ContentEntityInterface) {
$form_element[$field_definition->id()]['references'][$referenced_entity->id()]['children'] = $this->getChildren($referenced_entity, $discovered_entities);
}

in getRecursiveLayoutFormElement() in 3050027-inline-block-clone-combine-3060223-v4.patch.

So now only first level elements are displayed before clicking on clone button.
But I am able to clone all the entities.

Hi,

thanks for the detailed steps on how to reproduce this error. I followed your steps but never got your error.

Side note, I had an error with the paragraphs module when saving the layout on the original note. Integrity constraint violation: 1048 Column 'langcode' cannot be null

I used this patch to fix it. I am not certain at this point if that somehow affected the result.

I will leave this to review hoping that someone might have a different answer.

I'm wondering if this issue is really related to Layout builder since there's a similar tread on Paragraphs module bug with Entity Clone and no Paragraphs is mentioned there: https://www.drupal.org/project/entity_clone/issues/3060223

This should be fixed in Entity Reference Revisions module.

Please check this issue #3190915: New revision is not created via Layout Builder

I have an issue with blocks in layout builder. I have a simple block with just a body field.
When I clone a node with one of those blocks added in layout builder everything looks fine. However after I edit the block in the clone it just gets edited there (which is fine, it's what I expect), but after that I can't edit the block on the original node. They seem to still reference the same block id but have different revisions.
I tried the patch in #19 and after that I can edit the two blocks individually. I want to hide the checkbox on the "Clone" page but even after I check everything on /admin/config/system/entity-clone it still shows.

I have updated the patch from #19 so it works with the latest version of the module. I have also added a setting to hide the Layout builder checkboxes on the clone page.

Rerolled again against latest dev.

Face an issue when applying the patch on comment #26 with the patch mentioned on the following issue https://www.drupal.org/project/entity_clone/issues/3114597,
that the label "- Cloned" starts to replicate depending on the number of available translations.
So I updated the patch and tested it with a multilanguage site and it worked well for me.

After deep testing, the patch on comment #30 fails to work with translation entities.
So the patch on comment #26 working well for me as it fix two related issues also:
https://www.drupal.org/project/entity_clone/issues/3114597 & https://www.drupal.org/project/entity_clone/issues/3060223

I aplied the patch #30, and i found just a little issue. You forgot to set config "hide_layout_builder_referenced_entities" on submit form.

Thank you for the patch! It's working fine cloning blocks with images, blocks with paragraphs inside, nodes with taxonomies, etc...

I'm attaching the same patch with this additional little fix.

Thanks for finding that out @tobas1996 - could you perhaps provide an interdiff?

Afterwards it would be cool if someone from the issue could review it :)

Updated patch #32 to work with the latest version (8.x-1.0-beta7).

@RedwanJamous thanks for the patch for the latest beta version, but there is no need to let the test runners run for a patch that does not apply to the latest dev branch.

For the rest are here some remarks for the patches:

• The patches create a not needed/willing dependency on Layout Builder. Needs to be adjusted so there is no dependency on a module that is not always installed.
• The patches create a not needed/willing dependency on Block content. Needs to be adjusted so there is no dependency on a module that is not always installed.
• Doc comment not complete for construct function in latest patch.

As the issue is also existent in the 2.x, I suggest we focus on fixing it in the 2.x

The patch has been rewritten to work for 2.x, but also applies to 1.x.
I only tested the functionality in the 2.x branch.

After testing with a multilingual setup, the translated content is still broken.

Tried to fix the multilingual part, but needs proper testing.

Manual functional testing of the patch in #40 checks out.

I plan to work on functional tests, including demonstration of the multilingual work, in the coming days.

From #36:

The patches create a not needed/willing dependency on Layout Builder. Needs to be adjusted so there is no dependency on a module that is not always installed.

Is this still an issue? The ContentEntityCloneBase class includes two classes that invoke Layout Builder services/classes, but they are conditioned on $this->moduleHandler->moduleExists('layout_builder'), and the Form classes' methods involving the SectionListInterface are conditioned on $field_definition->getType() === 'layout_section'.

We could make conditional the rendering of $form['hide_layout_builder_referenced_entities'], but its presence isn't problematic if sites don't have Layout Builder installed.

Hey Mark, the dependencies were not a problem as the modules are core dependencies. I was a bit to fast saying that the dependencies could be a problem.

To me the patch works, but one problem still remains and that is that paragraphs in an inline block are still referenced. So changing the data on a node changes it on the clone and visa versa. So I created a simple extra patch removing the blocks with paragraphs. Will upload that one when I am after my pc. Still a big issue but with the patch it is workable.

But we may look at #29 approach. Found out that the later patches were a bit different. But #29 looks like they maybe have code/functionality that could help.

General FYI for folks following this issue: our Drupal development team, which supports many sites that have been using entity_clone, was concerned about the potential data loss described in this issue. Although the latest patch resolves the issue for newly created blocks, blocks created before the patch is applied will still be vulnerable to being deleted from referenced entities when the original entity is deleted (as explained in the issue description).

So we wrote a process to fix existing blocks. The snippet in https://gist.github.com/markfullmer/80c606d4e3945c33b34eba9615b23d91 will only remediate entities of type "node," which is probably sufficient for almost all use cases, but it could be abstracted to account for other entities. This code could be called in a site update hook or via a custom drush command.

We hope this is useful for other folks facing the same concern.

Notes: the above code accounts for inline blocks referenced across multiple clones by duplicating the original block for each entity reference, and also cleans up tempStore values (i.e., if someone has modified a layout but not yet hit "save").

I've tried apply #38 and #40 to 8.x-1.0 but it fails (at least for me).

As 2.x is still in Alpha, I hope the patch can be a subject to 1.x?

@NicklasMF, this confused me at first as well.

The #40 patch applies cleanly to the dev.

Although I am still running into what seems to be a clone issue.

The inline_block_usage references are saved without layout_entity_id. I don't know... but I'm guessing there is another query that runs to apply the layout_entity_id. Seems like it needs an on duplicate update... but then on revisions it would likely need an insert.

With or without the patch I encounter the same result.

If I go into the database, and manually add the layout_entity_id values, the issue goes away.

When the clone fails, all of the inline_block_usage layout_entity_id will be NULL.

I'm not positive, but I think the issue is in InlineBlockEntityOperations, line 234, $this->usage->addUsage(). An entity is always passed, but in some cases the entity id is not set. So when the entity hits addUsage(), there is no entity id. Then you end up with orphaned block_content_id rows, empty layout_entity_id.

If a check is added for entity id before addUsage() is fired, the clone works fine.

I made a patch... of a patch.

When I went to apply my patch, I was editing already patched code.

Original patch.

My combined patch is attached. I did it this way as I'm not all that convinced its the correct way to do it; but at my experience level it seems like the proper place.

This seems to make the clone work fine, and then saves as expected.

I've tested the latest code from MR #42, and it works as expected in my scenarios.

Without cloning the inline blocks, when I edited the cloned page via the Layout tab, the blocks were there; however, preview of paragraphs was not possible due to the parent entity being incorrect.

Attaching the patch to use in composer.json.

Thank you everyone working on this! :))

For those tracking different parts of this across several issues:

Using #52, I was running into the issue mentioned in #48.

Just added a new patch (#36) on 2062819 that should solve the missing id issue by using reflection to get the entity id from the plugin:

https://www.drupal.org/project/drupal/issues/3062819#comment-15053937

Needs reroll.

Updated the MR with the latest code from beta4. Working as expected on my layout builder projects :)

Last MR diff, updated by @carolpettirossi applies to the beta4 version and fixes the issue.

I tested on a couple of projects the last fix. Last MR fixes the problem on both.
I think it is ready for RTBC but we need somebody else to verify everything is fine.

I've tested the last MR and it works on my project.

Reroll the patch to version 2.0.0-beta4

Merged, release to follow shortly. Thank you for working on it!!

Thanks for everyone that worked on this.

Whenever a major bug that can cause data to be deleted is fixed/merged, can we get a new release with it? Please.

Why on earth has this not been released yet? This is so important!

Sorry for delay, released!

The merged code from this issue introduced a bug on the latest beta5 release!
I understand the urgency of the layout builder issue, but we can't merge code without tests!

I see code fragments from #3258535: Translatable entity reference fields are not cloned correctly ended up in the merge request, which basically loops through the languages and clones entities all over the place without actually preventing to create several duplicates and filling up the database with orphan entities. With manual testing, you can't detect such problems! Not to mention that the issue is not even fixed yet!
Nevertheless, adding layout builder specific logic in a base class seems far from correct.

Apologies to the layout builder community members, but I opened a merge request in #3437645: Revert "Inline Blocks on Layout Builder" issue which introduced a bug of duplicate entity clones to revert this issue and make a new release as it is currently broken and create an opportunity to fix the problems and implement tests.

As the latest release with merge introduced #3437645: Revert "Inline Blocks on Layout Builder" issue which introduced a bug of duplicate entity clones hence reverted and released new tag. Thank you @joevagyok for reporting, you can close the another issue.

The MR 67 is working as expected on my project. However, in my case, the content editors must be very careful and select the checkbox to clone the blocks inside blocks.
We are not using paragraphs.
For example, we have an Accordion block. Within the Accordion block type we have a Accordion Items field that you can reference Accordion Item block type.
If the editors are not careful in selecting those fields, they may have references to existing blocks and edits will be performed on the origin as well.

It would be nice to get a config to "Select all block_content" when cloning the entity.

Brought patch up to date with the latest in the 2.x branch.

@carolpettirossi you might check out Admin > Configuration > System > Entity clone settings (/admin/config/system/entity-clone. It has some configuration options that can be very helpful to users.

Enforcing "Checkboxes default value" and "Disable checkboxes" means that all (non-reusable?) blocks will be cloned by default and the boxes will be shown to the user as checked and disabled. Additionally, checking "Hide Layout builder referenced entities checkboxes" will hide the checkboxes altogether for an even cleaner cloning experience.

Rolled the latest MR included in https://www.drupal.org/project/entity_clone/issues/3050027#comment-15745284 as a patch for those who need to apply this, but for security reasons cannot use a diff of a MR.

So couple of questions:

• Are we anywhere close to having this ready to merge? I've tested https://git.drupalcode.org/project/entity_clone/-/merge_requests/67#note... and it fixed the issue on a client project. However this project is not multilingual.
• From the thread above it looks like there may still be some work involved to fix translated content entities? Is this correct?

This issue is not close to be merged and here are some reasons why:

• The MR still introduces a bug which creates more cloned entities in certain conditions that it should. This can be only detected by automated tests asserting the amount of entities in the database after each clone operation.
• The base class meant for content entities should not contain layout builder specific logic, not every site has layout builder.
• There are certain entity types and entity reference combinations that are not entirely clear how we should proceed like Accordion and its items.
• The MR needs automated tests.

These have to be addressed before we make a review.

Thank you @joevagyok!

In my particular instance the client has LayoutBuilder, which is the root of the issue we were having.

For those having issues with Layout Builder sites, the patch in #81 is a roll of the MR in #78, and applies cleanly to a site running Drupal 10.3 with LayoutBuilder enabled, and Entity Clone version 2.1.0-beta1.

When a Layout Builder (LB) block with media is cloned, the media is cloned too. If the media is removed, it is also removed from the parent node.

@whereiam
I have not reproduced the problem you described. https://www.drupal.org/project/entity_clone/issues/3050027#comment-16066461
Please go to /admin/config/system/entity-clone and make sure you selected "Media" as a "clonable" entity type.

Thanks for the patch #81 @lisa.rae. Have fixed 2 things which I found:

1. In layout builder, content block which are not revision able are skipped from cloning.
2. Missing label suffix for content block in layout builder.

Missed to add a fix in above patch.

I think there is another approach to take into account for this issue. Layout builder inline content blocks are actually "inline entities".
This means that they really belong to their host entity and the module that "declare" the entity have to implement the "inline entity" mecanism :

• ensure that the entity is associated to only one host entity
• expose the access dependency to allow the host entity to define the child access (see \Drupal\Core\Access\AccessibleInterface\DependantAccessInterface)
• save the host-child relationship somewhere to be able to attach the host-child operations (delete orphan...).

We should rely on that to know if the child entity has to be cloned or not (whatever the config). => If child is an inline entity then clone.

I rebased the 2.x fork so MR88 can be applied to the 2.2.0-beta1 version successfully.