Inline blocks from layout builder not cloned properly

I confirm I have the same problem.

The patch seems to not work for me. I have a warning and then leading to a fatal error.

Notice: Undefined index: block_revision_id in Drupal\quick_node_clone\Entity\QuickNodeCloneEntityFormBuilder->cloneInlineBlocks() (line 204 of /app/web/modules/contrib/quick_node_clone/src/Entity/QuickNodeCloneEntityFormBuilder.php)

It seems I don't have the block_revision_id that is used by the patch.

And the patch needs a reroll for dev.

Reroll attached without any fix.

Ok so the crash seems happening when using field blocks (extra_field_block):

My configuration is when I arrive where the code crashes:

array(3) {
  'label_display' =>
  string(1) "0"
  'context_mapping' =>
  array(1) {
    'entity' =>
    string(21) "layout_builder.entity"
  }
  'id' =>
  string(41) "extra_field_block:node:landing_page:links"
}

Fields block doesn't need and actually can't be cloned, so I just added a test to avoid going through the code when having a field block or any kind of a block that is not revisionable.

Have to re-roll patch from #11 since it doesn't apply to version 1.12
Also have some errors on the delta of the components so the logic there was updated.

If you changed something, please provide a patch for dev with an interdiff.
A patch for 1.12 can't be merged.

Patch against dev and interdiff
attached

Patch #15 works well. Thanks!

A few problems occur while using this patch.

Inline content block's are saved when rendering the node clone form. This means that every time you refresh the node clone form, new inline content blocks are created. I first thought it was the paragraph clone implementation mentioned here but totally forgot that this patch was still active on my locale environment :S.

Furthermore layout_builder keeps track of the related inline block content under inline_block_usage. These values are not correctly added (layout_entity_id is NULL).

#15 unfortunately doesn't work for me.

Also agree with #17 that the clone function should be called on save and not when the form renders.

This ticket may be actually fixed under https://www.drupal.org/project/drupal/issues/3062819#comment-14165004

To add on to #19, please check the patch in #3062819-017: Duplicated and new entities with inline blocks do not duplicate their inner blocks and/or track their usage and see if this resolves your issues.

If so, please note the patch in #3062819: Duplicated and new entities with inline blocks do not duplicate their inner blocks and/or track their usage resolves your issue so we have additional evidence and testing to move to RTBC, and reply here so we can scope this fix out of this module and into core. Thanks for helping to test open-source and resolving this issue!

This ticket can probably be closed as the patch from https://www.drupal.org/project/drupal/issues/3062819#comment-14165004 works.

The patch in #3062819: Duplicated and new entities with inline blocks do not duplicate their inner blocks and/or track their usage doesn't actually seem to clone the blocks, it creates a new revision of the layout builder blocks, so 2 nodes point to a different revision of the same block. I'm not yet sure whether this should be fixed in core or in the module that actually clones a node. I believe it would be nice if core would take care of it, but I guess that is at least debatable and probably also not likely to be fixed in the short term.

This patch should do the trick without creating new blocks every time the form loads. The blocks should be saved automatically when the form is submitted.

+++ b/quick_node_clone.services.yml
@@ -1,7 +1,7 @@
+    arguments: ['@form_builder', '@entity_type.bundle.info', '@config.factory', '@module_handler', '@entity_type.manager', '@current_user', '@tempstore.private', '@string_translation', '@uuid'']

Doh.. Attached is a new patch without the typo.

After some extensive testing I'm still seeing 1 issue with my latest patch. The layout builder modules uses layout_builder_entity_presave() to call InlineBlockEntityOperations::handlePreSave() which should save all the serialized blocks in the entity. When using translations though, this only seems to run for the current translation, not for all other translations. This causes serialized blocks to be stored in the database, instead of saving the actual block content and just saving a reference to the revision ID in the section configuration.

I'm not yet sure how to handle this (or if we should handle this at all and create a core issue). As said in #23: I believe it would be nice if core would take care of it, but I guess that is at least debatable and probably also not likely to be fixed in the short term.

Maybe we should add our own presave hook and call InlineBlockEntityOperations::handlePreSave() on all translations when saving a clone?

Since we need a node ID to properly save blocks and usage, we unfortunately have to save the node twice. I used SynchronizableEntityTrait in the second save so other modules can detect this in entity hooks and opt out of certain behaviour (just like done in #2803717: Allow 'syncing' content to be updated in content moderation without forcing the creation of a new revision).

This means we also can't use a custom presave hook (again, no entity ID available), so I've added the code to fix this in QuickNodeCloneNodeForm::save(). This seems to work quite well using content moderation and content translations, but I admit doing 2 saves is a bit hacky and might cause issues in other contrib or custom code.

While testing patch #28 it happened that the clone would not contain the blocks in the original node layout builder. Some nodes always work, others always fail. Only found this relevant error:

cli_1  | NOTICE: PHP message: ArgumentCountError: Too few arguments to function Drupal\quick_node_clone\Entity\QuickNodeCloneEntityFormBuilder::__construct(), 8 passed in /var/www/docroot/core/lib/Drupal/Component/DependencyInjection/Container.php on line 262 and exactly 9 expected in /var/www/docroot/modules/contrib/quick_node_clone/src/Entity/QuickNodeCloneEntityFormBuilder.php on line 127 #0 /var/www/docroot/core/lib/Drupal/Component/DependencyInjection/Container.php(262): Drupal\quick_node_clone\Entity\QuickNodeCloneEntityFormBuilder->__construct(Object(Drupal\Core\Form\FormBuilder), Object(Drupal\Core\Entity\EntityTypeBundleInfo), Object(Drupal\Core\Config\ConfigFactory), Object(Drupal\Core\Extension\ModuleHandler), Object(Drupal\Core\Entity\EntityTypeManager), Object(Drupal\Core\Session\AccountProxy), Object(Drupal\Core\TempStore\PrivateTempStoreFactory), Object(Drupal\Core\StringTranslation\TranslationManager))
cli_1  | #1 /var/www/docroot/core/lib/Drupal/Component/DependencyInjection/Container.php(176): Drupal\Component\DependencyInjection\Container->createService(Array, 'quick_node_clon...')
cli_1  | #2 /var/www/docroot/modules/contrib/quick_node_clone/src/Controller/QuickNodeCloneNodeController.php(51): Drupal\Component\DependencyInjection\Container->get('quick_node_clon...')
cli_1  | #3 /var/www/docroot/core/lib/Drupal/Core/DependencyInjection/ClassResolver.php(28): Drupal\quick_node_clone\Controller\QuickNodeCloneNodeController::create(Object(Drupal\Core\DependencyInjection\Container))
cli_1  | #4 /var/www/docroot/core/lib/Drupal/Core/Controller/ControllerResolver.php(117): Drupal\Core\DependencyInjection\ClassResolver->getInstanceFromDefinition('\\Drupal\\quick_n...')
cli_1  | #5 /var/www/docroot/core/lib/Drupal/Core/Controller/ControllerResolver.php(69): Drupal\Core\Controller\ControllerResolver->createController('\\Drupal\\quick_n...')
cli_1  | #6 /var/www/docroot/core/lib/Drupal/Core/Controller/ControllerResolver.php(85): Drupal\Core\Controller\ControllerResolver->getControllerFromDefinition('\\Drupal\\quick_n...', '/en/clone/29172...')
cli_1  | #7 /var/www/vendor/symfony/http-kernel/HttpKernel.php(142): Drupal\Core\Controller\ControllerResolver->getController(Object(Symfony\Component\HttpFoundation\Request))
cli_1  | #8 /var/www/vendor/symfony/http-kernel/HttpKernel.php(81): Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object(Symfony\Component\HttpFoundation\Request), 1)
cli_1  | #9 /var/www/docroot/core/lib/Drupal/Core/StackMiddleware/Session.php(58): Symfony\Component\HttpKernel\HttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
cli_1  | #10 /var/www/docroot/core/lib/Drupal/Core/StackMiddleware/KernelPreHandle.php(48): Drupal\Core\StackMiddleware\Session->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
cli_1  | #11 /var/www/docroot/core/modules/page_cache/src/StackMiddleware/PageCache.php(106): Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
cli_1  | #12 /var/www/docroot/core/modules/page_cache/src/StackMiddleware/PageCache.php(85): Drupal\page_cache\StackMiddleware\PageCache->pass(Object(Symfony\Component\HttpFoundation\Request), 1, true)
cli_1  | #13 /var/www/docroot/modules/contrib/cleantalk/src/EventSubscriber/BootSubscriber.php(320): Drupal\page_cache\StackMiddleware\PageCache->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
cli_1  | #14 /var/www/docroot/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(48): Drupal\cleantalk\EventSubscriber\BootSubscriber->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
cli_1  | #15 /var/www/docroot/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(51): Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
cli_1  | #16 /var/www/docroot/modules/contrib/pantheon_domain_masking/src/Middleware/DomainMaskingMiddleware.php(124): Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
cli_1  | #17 /var/www/vendor/stack/builder/src/Stack/StackedHttpKernel.php(23): Drupal\pantheon_domain_masking\Middleware\DomainMaskingMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
cli_1  | #18 /var/www/docroot/core/lib/Drupal/Core/DrupalKernel.php(709): Stack\StackedHttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
cli_1  | #19 /var/www/docroot/index.php(19): Drupal\Core\DrupalKernel->handle(Object(Symfony\Component\HttpFoundation\Request))
cli_1  | #20 {main}

@wranvaud: I just checked the #28 patch and there are definitely 9 arguments, however one of them is added in/new in this patch compared to the main branch. Usually this is a case of needing to clear caches because no matter what, YAML files never get reloaded unless the caches are cleared.

I have seen that same error multiple times in different scenarios, but each time it had to do with me adding an argument to a service but forgetting to add it to either a construct or create method or similar.

Hopefully you haven't been sitting on this for 2 weeks, but just in case, that should be the fix.

Rerolled patch from #28 against latest release.

Was using the previous patch on a project that included blocks with paragraphs, and we had a similar problem again: cloned blocks were referencing the same paragraphs as their source nodes. Editing paragraphs seemed to work since the actual field refers to revisions, but if you deleted one of the blocks the paragraph would be deleted as well. Here's a patch that includes cloning the paragraphs of the cloned blocks (and some slightly opinionated refactoring of the cloneParagraphs function).

Please include interdiffs when creating a new patch, or use the fork/MR functionality so folks can easily follow what changes have been made between contributions. Thanks!

My bad! Here's the interdiff.

Patch in #32 worked for me. I had Layout Builder blocks referencing paragraphs and found that cloned nodes that were unpublished were setting the source node's paragraphs to be unpublished - so anonymous visitors would see a blank page visiting the source node. This is working, thank you.

Just looked into this issue again. We might be able to remove the duplicate save with #3062819: Duplicated and new entities with inline blocks do not duplicate their inner blocks and/or track their usage.

About cloning entities (nested), our lives would be a lot easier if we have #3040556: It is not possible to react to an entity being duplicated.

To fix the (nested) cloning of paragraphs we have #3211482: Paragraph Library supports Entity Clone module. If we can make the paragraphs module support hook_entity_duplicate_create(), all the clone related modules would be a lot simpler.

We could even have a core issue to make layout builder implement hook_entity_duplicate_create(), so the layout builder module is responsible for cloning all the blocks when an entity contains a layout builder field.

#32 updated for 1.17/current 1.x

Could we please convert this work to a merge request so that GitLabCI will run against it?

Not sure why the the patch doesn't apply for me:
https://git.drupalcode.org/project/quick_node_clone/-/merge_requests/22/...

@phjou Strange, if you use `.diff` instead, then it does work.
https://git.drupalcode.org/project/quick_node_clone/-/merge_requests/22/...

Make sure to add an empty line at EOF, though.

The fix #43 worked for me. Thanks a lot.

Would it be possible to move the code to clone a specific section to a separate cloneLayoutSection method? I'm currently adding a custom controller that duplicates the code to clone a specific section in a layout, and it would be nice to reuse the code we have here.

This seems to do it. Updated the MR.

Just FYI, I needed to update from 1.16 to 1.18 and the original patch we had didn't work for the new version. So, I removed the patch and decided to test this issue and it all seemed to work fine. Was this fixed from 1.16 to 1.18?

Added a commit to fix an issue while cloning layout builder content for the default language. It appears InlineBlockEntityOperations::handlePreSave does not run for syncing entities.

I've removed all refactoring to make everything a little bit easier to review and added a BC layer for the service constructor arguments change

The changes in cloneParagraphs are not entirely unrelated. For example, cloneParagraphs only allowed cloning paragraphs in nodes, while inline blocks can also contain paragraphs. Can you revert your changes to cloneParagraphs?

I made cloneParagraph's parameter mixed so that it won't bomb on things like BlockContent, which also calls that method. This still feels like a bandaid, but allows my sites to work. Are there other types we should be looking out for to be explicit and replace mixed?

The parameter should probably be FieldableEntityInterface, I think we should just revert commit https://git.drupalcode.org/project/quick_node_clone/-/commit/4b2e8cbbb1f...

hi @dblanken @seanb

I've added FieldableEntityInterface back, the original change is doing a good refactoring, but it out of scope for this issue imo and makes it harder to understand and review (plus it's harder to combine patches)

Hi @taran2l

Thank you very much. I can confirm that with the FieldableEntityInterface it's working for us again. Thanks a lot for doing that.

When node has one or several translations and layout builder field is not translatable, layout is lost for whole node after clone.

kiseleva.t made their first commit to this issue’s fork.

Exported MR changes into static file patch.

Our team found ourselves wanting to use both this patch and the patch from #3049127: Cloning a node with existing translations, clones all translations of the node. The problem is, 3049127 introduced a bug into this patch.

We couldn't figure out a really 'clean' way to reconcile fixing the two patches while adhering to issue encapsulation best practices.

As a result, we'll upload the changes we made to this MR here for other users / future reference for the community. But we didn't want to put undesired changes back into this MR directly.

Essentially, here are our changes.

Line 156 was:
$section = $this->cloneLayoutSection($section);
now:
$section = $this->cloneLayoutSection($section, $entity->language()->getId());

Line 171 was:
public function cloneLayoutSection(Section $section) {
now:
public function cloneLayoutSection(Section $section, string $langcode) {

At line 201, we inserted the following conditional logic:

        if ($this->getConfigSettings('skip_translations')
          && $block_content instanceof TranslatableInterface
          && $block_content->isTranslatable()
        ) {
          // Loop through the unwanted translations and remove them.
          foreach ($cloned_block_content->getTranslationLanguages() as $lang => $language) {
            if ($lang !== $cloned_block_content->language()->getId() && $lang !== $langcode) {
              $cloned_block_content->removeTranslation($lang);
            }
          }

          // Save the block.
          $cloned_block_content->save();
        }

Thanks for the work on this one. The patch in #61 solves the main problem. New Block IDs are generated (not just new revision IDs). However, it does not work for nested blocks. In those cases, the top level (parent) blocks get a new ID but the child blocks attached using an entity reference field do not get updated.

This is somewhat of an edge case and a non-trivial thing to fix but I think it is a potential blocker. Nested blocks and nested Paragraphs are not uncommon for some builds.

I updated the MR with a solution to recursively clone blocks that contain entity_reference fields with a target_type set to block_content.

I'm also attaching a patch that breaks this out as a sort of interdiff letting it be applied after either patch #61 or #62 with translation support. The nested blocks fix requires one of those to be applied first.