Cloning a new nested paragraph creates shared child reference

Hi @peterwcm,
I've updated the createDuplicate method please check.

Also @berdir pls check is this a good solution?

Thanks,
Arun

@peterwcm,

I've updated the code and now no more orphaned paragraphs and everything saves when the node saves by referencing the entity directly instead of manually setting target_id.
and I also think this condition !$item->entity->isNew() is not necessary.
For the translation thing there is one limitation that - Paragraphs are referenced entities, not simple text fields.
Drupal does not natively support translating entity reference fields.
When I tried to translate it shows this error

* (unsupported) Paragraphs fields do not support translation

. Yo can checkout this link for more info .

I think there is one tot check translation with Paragraphs Library module but I'm not sure.
Could you pls check it, if it works then I think we can safely remove that condition.

The check is in preparation for the new hook in Drupal 11.2 to avoid additional duplication. The changes remove the relevant todo and there is no test coverage for the bug that this seems to fix.

We have fairly extensive duplication tests, if there is a bug then that should be covered by tests.

This problem is not happening when using drupal core 11.2 In this case, the \Drupal\paragraphs\Hook\EntityHooks::duplicate is doing the job.
@peterwcm, @arunsahijpal can you please tell us if you're using a version of drupal prior to 11.2?

Still, I want to confirm that there is a bug when using Drupal core prior to 11.2 (when the new hook "entity_duplicate" is not run).

It happens when we duplicate a new introduced paragraph (with nested paragraphs).
In that case the new condition !$item->entity->isNew() is FALSE and the nested paragraphs are not duplicated. They end up being used in the original and in the duplicated paragraph.
Sorry that I can't provide a test ATM.

Just removing the new condition !$item->entity->isNew() fixes this.

I am not able to add the https://www.drupal.org/project/paragraphs/issues/3079729 patch with this patch. Instead of writing whole function createDuplicate(). Can we simply remove the !$item->entity->isNew() condition line?

Per #11-12, what about centralizing code then?
Update \Drupal\paragraphs\Entity\Paragraph::createDuplicate to something like

$duplicate = parent::createDuplicate();
if (version_compare(\Drupal::VERSION, '11.2.0', '<')) {
  /** @var \Drupal\paragraphs\Hook\EntityHooks $service */
  $service = \Drupal::service(\Drupal\paragraphs\Hook\EntityHooks::class);
  $service->duplicate($duplicate, $this);
}

Then it's clear that this code can be removed once <11.2 is no longer supported.

I tried the suggestion in #29—I had to manually instantiate the class since it is not a service, but it resolved the problem we were having with duplicated child references (as of paragraphs 1.19.0 in Drupal 10.5).

Here's the full implementation of createDuplicate we're using:

/**
 * {@inheritdoc}
 */
public function createDuplicate() {
  $duplicate = parent::createDuplicate();
  if (version_compare(\Drupal::VERSION, '11.2.0', '<')) {
    $hooks = new \Drupal\paragraphs\Hook\EntityHooks();
    $hooks->duplicate($duplicate, $this);
  }

  return $duplicate;
}

I've updated the issue title and description to emphasize the fact that the paragraph that is being cloned must be new aka not yet saved to the database.

This detail eluded myself and predecessor looking into this issue in some projects, making us doubt if the proposed patch here would fix our issue.

Without the patch, when cloning a new nested paragraph, the respective child paragraphs will be created as different revision of the same paragraph, each referring to different parent paragraph.
You can verify this when comparing the records in paragraphs_item_field_data table versus paragraphs_item_revision_field_data for given node and parent paragraphs.

With patch, you can see each child paragraph being created as separate entities.

Also, after more testing, we noticed issues when the nested paragraphs contained entity reference fields to Media, and in our case Bynder Media.
In the duplicates, the Media references disappeared saving the cloned paragraphs with the patch applied.

I didn't debug what happens in de creatDuplicate function, but it made sense that entity references to anything other than the (child) paragraphs, should not be duplicated (like nodes, users, terms,...).

For our project scope, an additional check on the entity being a paragraph, fixed the unwanted side behaviour:

          ...
          // Duplicate child paragraphs, remove when requiring 10.2+.
          // @see \Drupal\paragraphs\Hook\EntityHooks::duplicate()
-         if ($item->entity && !$item->entity->isNew()) {
+        if ($item->entity && $item->entity->entityTypeId === 'paragraph') {
              $fieldItemList[$delta] = $item->entity->createDuplicate();
          }
          ...

After checking the entity_duplicate hook, it seems that the if above could be the same as https://git.drupalcode.org/project/paragraphs/-/blob/8.x-1.x/src/Hook/En... (same check, just different way to do it and actually prefer the 'instance of' approach).

Are you on D10? Per recent comments, this should not happen on D11.2+, nor should it happen when using the replicate module.

The MR is definitely not correct and should not be used. Feel free to create a new MR with the proposal from #29/#30 as a workaround for sites stuck on older core versions.

For #30 the EntityHooks class can be registered as service, see #3079729: Remove file on original/duplicated paragraph removes the file on all original/duplicated paragraphs's MR for example.

> @herved While we could manually register the hook class as a service, the module already supports Drupal 11, where hook classes are automatically registered as services from 11.1+. Manually registering it would double-define it on newer sites and leave more code to clean up once Drupal 10 support is removed. I think @mediabounds’ suggestion would introduce less technical debt.

No, manually registering does not duplicate the service if it's using the same name. But that alone would not do anything. The reason this is not done here is deliberate, this hook does not exist in D10.

Passes review and does what it says.
No functional problems found. The change is minimal, correctly scoped to the reported regression, and the regression test is sound for what it can test in this environment.