Support for sub-entity cloning

Currently the clone for content entities is fully supported.
On the other hand, as you point out, all referenced entities (with entity reference fields) are simply attached and all target entities are not cloned.

I had already started this development.

Now, with this patch, in the clone form, a new checkbox has appeared to propose you a "recursive mode".
All referenced entities (except Files / Images / Taxonomy terms) are cloned too.

It should work with all entities including entities provided by the paragraphs module.
If you have time, can you test this patch ?

Please consider the Entity Reference Revision definition that is the underlying field storage of Paragraphs:
Entity Reference Revision defines a composite relationship and it is never allowed to have a target entity reused.

You don't need a setting for that, instead you are required to ALWAYS recurse on composite relationship entity reference revision fields.
This support makes sense as other modules consider building on top of ERR and we already have similar integration for this.
- The diff module will also recurse to show diffs on host entity.
- The TMGMT module makes sure the paragraphs are also passed to translation of a node / host entity.
- The collect module also embeds the paragraphs.
Complexity significantly goes down.
Happy to mark our ERR / Paragraph issue as fixed when ready... #2676226: Needs tests: Support cloning

Also i recommend to you to define recursing on entity references on a per-field level. Entity references are used in so many different ways:
From user reference, over term references, to related content references.
A general "recurse cloning" simply doesn't work semantically.

I've supplied a simpler patch that just handles ERR fields. It does not offer a setting since like miro said, you always have to clone them.

There is also a bit in the patch to make it work with Multiversion.

Here is a better patch.

How about the status of multilingual support of this approach?
There's usually always more to consider with it than initially expected... ;-)

Here a first draft patch based on that of the second comment.
It add checkboxes on clone form to choose which entity have to be clone.
It do not add support for entity clone revisions/dynamic entity references yet

What if we add support to complex fields within an event?

I created a patch which dispatch some events before and after cloning #2800203: Event dispatcher for clone events.

Attached is a patch which depends on #2800203: Event dispatcher for clone events and implements recursives event subscribers to clone paragraphs and field collections.

I think this is a good approach because you just need to create an EventSubscriber class per each entity implementation, without the need to make changes on the existing files.

Cloning entity reference revisions is based on comment #2706639-9: Support for sub-entity cloning

I was able to clone entities like this one:

• Node
  • String field
  • Image field
  • Paragraph field
    • Paragraph field
  • Paragraph field
    • Field Collection
    • Paragraph field
  • Field Collection
  • Field Collection
  • Paragraph field
    • Field Collection

New patch and its diff.

For field collections now it erases any previous values and then clone the new items. The last patch did that in the same foreach. It fixes some issues related to relations like Node -> Field Collection -> Paragraphs -> Field Collection.

Some typo fixed and deleted unnecessary functions.

Thanks!

Tried #19 patch on a node with nested paragraphs. It did not work. Paragraphs are not being recursively cloned. The new node references the paragraphs of the original node. Edits to paragraphs of the clone appear in original.

Node structure:

Node
  Paragraph
    Text
    Image
  Paragraph
    Integer
    Integer
    Paragraph
      Text
      Image
    Paragraph
      Text
      Image
  Paragraph
    Text
    Image

@RogerB did you clear cache? Events needs to be subscribed in order to works.

@Rober and also you need to apply #2800203: Event dispatcher for clone events too, as I described in commit #18.

Yes @ruloweb I later spotted that I had to add the #2800203: Event dispatcher for clone events patch as well. When I did it worked perfectly.

@RoberB are these patches still working for you on Drupal 8.2.3? I'm getting WSOD on /entity_clone/node/### with

Uncaught PHP Exception Drupal\Component\Plugin\Exception\PluginNotFoundException: "The "field_collection_item" entity type does not exist." at /Applications/MAMP/htdocs/go-d8/core/lib/Drupal/Core/Entity/EntityTypeManager.php line 133

Sequence:

1. entity_clone-event_dispatcher-2800203-5.patch
2. entity_clone-attached_entities-2706639-19.patch

I added a condition to escape the onEntityCloned() method in FieldCollectionItem.php that tests to see if the 'field_collection_item' entity type exists. This fixes the error found in comment #24.

Here is the patch.

+++ b/src/EventSubscriber/EntityReferenceRevisions.php
@@ -0,0 +1,68 @@
+use Drupal\entity_clone\Events\EntityCloneEvent;
+use Drupal\entity_clone\Events\EntityCloneEvents;

There are somethings missings, you call class who doesn't exist.

@BizuTétu: Both of those classes existed when I used the patches below. Note: The event subscriber patch #2800203: Event dispatcher for clone events is required for #2706639: Support for sub-entity cloning.

The following worked for me. I verified that the paragraphs on the clone node had new IDs.
Patches applied.

     "drupal/entity_clone": {
        "2800203 - Event dispatcher for clone events": "https://www.drupal.org/files/issues/entity_clone-event_dispatcher-2800203-5.patch",
        "2706639 - Support for cloning nodes that have another entities attached": "https://www.drupal.org/files/issues/entity_clone-attached_entities-2706639-25.patch",
        "2769823 - Getting error after clicking on abort clone button": "https://www.drupal.org/files/issues/entity_clone-abort_clone_causes_fatal_error-2769823-4.patch"
      }

I can also verify that with the three patches in #29 applied, cloning a node with paragraphs creates new paragraph entities. A cache clear was required for it to start working correctly.

The three patches as supplied in #29 appear to be working well for me too, thank you!
using 9265e35 specific dev version in composer: "drupal/entity_clone": "1.x-dev#9265e35",

The tests were falling because they depend on other patches' issues.
These are related to the following patches:

• https://www.drupal.org/files/issues/entity_clone-event_dispatcher-280020...
• https://www.drupal.org/files/issues/entity_clone-attached_entities-27066...
• https://www.drupal.org/files/issues/entity_clone-abort_clone_causes_fata...

I just applied the 3 patches of the comment #29 in the right order and cloning a node with multiple paragraphs, it created new paragraphs entities on Drupal 8.3.5 version.

After losing a bunch of contents after deleting only one node I detect same problems. All affected nodes refer to the same field and field revision entries because they are clones of one original node and after deletion all still existing nodes refer to the fields and revisions which are deleted.

I've tested #29 resp. #34 and the three patches work for me as well. Cloned node creates new entries in field and field revision tables and a deletion is possible without affecting the original.

I've tested this in a multilingual environment on Drupal 8.3.5. with Paragraphs 8.x-1.1 and Entity Reference Revisions 8.x-1.3

I've updated the patch to reflect the changes from #2800203-10: Event dispatcher for clone events where the namespace Drupal\entity_clone\Events is changed to the singular Drupal\entity_clone\Event.

Looks like EntityCloneEvents::PRE_SAVE was renamed to EntityCloneEvents::PRE_CLONE, and same with EntityCloneEvents::POST_SAVE. Updated patch for that issue as well. See #2800203-10: Event dispatcher for clone events.

Fixed coding-standards violations and removed deep nesting.

Requires the patches from #2800203: Event dispatcher for clone events and #2908414: Error on abort and redirects - one patch to solve everything as prerequisites.

Didn't look too deeply into the failing tests, but I'm guessing that they're failing because the other patches haven't been applied yet. We should commit those, and then retest here.

I just tested the last patch (in comment #41), but it doesn't work for me.
Step to reproduce:

1. Install a new drupal instance with standard profile
2. Add new entity reference field "field_article" in page content type to allow renferences with article content types
3. Create an article content "Test article"
4. Create a page content "Test page" including reference with the previous created article
5. Clone the "Test page" content

Only the page is cloned.

The target of this issue is to clone recursively, with a choice (field by field), all referenced entities (entity_reference field type).

In this patch, only entity_reference_revisions and field_collection field are supported.

I think the best way to resolve this issue is re-rollling the #16 patch (that implement the correct way), and maybe include the entity_reference_revisions field type support.

Reroll the patch in comment #16

Fix errors

Tests are now OK with the rerolled patch.
Now, from this patch, it remains to be done:

• Allow entity_reference_revisions field to be cloned recursively
• Improve the UX/UI for the recursive clone form

As for field_collection fields, why this module doesn't use an entity_reference or an entity_reference_revision fields?

As per #2509254: Field collection fields should extend entity reference fields, it looks like Field Collection fields do extend Entity Reference fields in the third rewrite of the module (3.x). I can't find any reference to ERR over there.

Having said that, I think we should spend more time supporting Paragraphs/ERR and less time supporting Field Collection. After reading #2784931: Proposal: Deprecate Field Collections for Drupal 8, focus on Entity Reference Revisions & Paragraphs, I don't see too much continued effort going into Field Collection, especially when the following text is on the project page:

Paragraphs is likely to replace field collection for Drupal 8. Field collection is on its way to being deprecated. It is recommended to use paragraphs instead of field collection for Drupal 8 projects.

As far as this patch goes, I think we should get the above functionality in very soon, and create a new follow-up issue for recursion support. This way, we'll at least have basic cloning for these field types in alpha2, as planned. I've spun off #2913949: Add support for recursive cloning for that.

I should be able to review and test the above patch today, after which I'll either post another patch (if updates are required) or RTBC it. Barring any other major problems, we should be able to get this into Friday's release.

The above patch works well. I just made some minor coding standards and style changes.

Looks good for me.

You need to use interface instead of class directly :

+  protected function getChildren(FieldItemListInterface $field, FieldConfig $field_definition) {
+    $children = [];

+  public function __construct(EntityTypeManager $entity_type_manager, TranslationManager $translation_manager) {
+    $this->entityTypeManager = $entity_type_manager;
+    $this->translationManager = $translation_manager;
+  }

I've tried #50 with latest dev release and it doesn't work as expected. All paragraph items of cloned and original node will be deleted if I delete the cloned or the original node. It's the same behavior like described on #35.

I have only paragraphs attached to a content type, no recursion. With the mentioned patches in #29 it works well.

So I have to do a rollback to my last working version.

@colan
I think it's better to support entity_reference_revisions field now.
Many projects used the paragraphs module.
For the UX/UI improvement, I'm OK to postpone that

@rwam
The patch #50 does not allow the entity_reference_revisions field type to be cloned recursively.
Here a new patch.
Can you test this ?

Ups, cloning of node with paragraphs attached leads to:

Fatal error: Call to a member function getEntityTypeId() on null in /…/modules/entity_clone/src/EntityClone/Content/ContentEntityCloneFormBase.php on line 102

I fixed that problem, but found some others. When I get checkboxes on the clone form (which I never got before), and don't select any, submitting the form leads to an error, with warnings leading up to it in the log:

1. Warning: Illegal string offset 'target_id' in Drupal\entity_reference_revisions\Plugin\DataType\EntityReferenceRevisions->setValue() (line 111 of ...)
2. Warning: Illegal string offset 'target_revision_id' in Drupal\entity_reference_revisions\Plugin\DataType\EntityReferenceRevisions->setValue() (line 111 of ...)
3. Warning: Illegal string offset 'target_id' in Drupal\entity_reference_revisions\Plugin\DataType\EntityReferenceRevisions->setValue() (line 115 of ...)
4. Warning: Illegal string offset 'target_revision_id' in Drupal\entity_reference_revisions\Plugin\DataType\EntityReferenceRevisions->setValue() (line 116 of ...)
5. Error: Call to a member function getRevisionId() on null in Drupal\entity_reference_revisions\Plugin\Field\FieldType\EntityReferenceRevisionsItem->onChange() (line 214 of ...)

From the backtrace, it looks like it's coming from src/EntityClone/Content/ContentEntityCloneBase.php(99). This method is way too big to see what's going on. It needs to be broken into smaller ones as it is doing more than one thing. I'll see what I can do to fix.

In other news, I think I can finally see the UX problem. I get a form with 2 checkboxes, which both say the same thing. The text says,

Choose which referenced entity you want to clone, default action "alias entity".

This raises some questions:

• If I get two referenced entities with the same name, which one do I choose? What's the difference? What happens if I select none? Or both? In my case, they both say 240 - Illum > Test paragraph.
• What does default action "alias entity" mean? What are we aliasing here and why?

Actually, I'll be out for part of the day so if someone else wants to pick this up, please do (and assign it to yourself first so we're not duplicating effort).

Sorry, the patch provided in #56 shows the same behavior: if I delete a cloned node, all paragraphs of the source node are missing/deleting too because of same field reference ;(

Yeah it's absolutely critical to test cover the delete case exactly as @rwam mentioned.

We had some similar custom code bug that caused so much pain because data is lost in context B while you act in context A. It took way too long to identify the issue and we couldn't simply revert the content DB anymore... Dangerous and painful.

I've updated the issue summary with everything that needs to be done. (Thanks for the feedback!) Please edit as necessary. We can strike out items after they're completed.

I'd really like help in understanding what we're asking for on the clone form as that's really not clear to me now.

This patch contain:
* Refactor
* Little UI improvement
* New test to ensure referenced entities are cloned too

This patch works with entity_reference_revisions field types (used by paragraphs)

For example :
1 node (eg. article):
- Body field
- Entity reference revisions field to paragraphs (type A & type B)
- Entity reference field to tags taxonomy term
2 paragraphs types:
- type A with a simple text field
- type B with an entity reference revisions to the type A paragraphs

Here the clone Form:

First: thank you guys for working on this.

I've tested patch #63, but it doesn't work out of the box ;( And I have several issues with this new patch. Cloning of nodes with paragraph items works indeed but only if you understand the clone form correctly. I will try to demonstrate it with a screenshot. Here you can see a working form:

but wait:

One clicked option for cloning the user entity leads to:

Please try again later.Drupal\Core\Entity\EntityStorageException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry 'admin_cloned-en' for key 'user__name': INSERT INTO {users_field_data} (
…
)
in Drupal\Core\Entity\Sql\SqlContentEntityStorage->save() (line 805 of core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php).

Drupal\Core\Database\Statement->execute(Array, Array) (Line: 624)
Drupal\Core\Database\Connection->query('INSERT INTO {users_field_data} (uid, langcode, preferred_langcode, preferred_admin_langcode, name, pass, mail, timezone, status, created, changed, access, login, init, default_langcode, ldap_user_puid_sid, ldap_user_puid, ldap_user_puid_property, ldap_user_current_dn, ldap_user_last_checked, ldap_user_ldap_exclude) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4, :db_insert_placeholder_5, :db_insert_placeholder_6, :db_insert_placeholder_7, :db_insert_placeholder_8, :db_insert_placeholder_9, :db_insert_placeholder_10, :db_insert_placeholder_11, :db_insert_placeholder_12, :db_insert_placeholder_13, :db_insert_placeholder_14, :db_insert_placeholder_15, :db_insert_placeholder_16, :db_insert_placeholder_17, :db_insert_placeholder_18, :db_insert_placeholder_19, :db_insert_placeholder_20)', Array, Array) (Line: 87)
Drupal\Core\Database\Driver\mysql\Connection->query('INSERT INTO {users_field_data} (uid, langcode, preferred_langcode, preferred_admin_langcode, name, pass, mail, timezone, status, created, changed, access, login, init, default_langcode, ldap_user_puid_sid, ldap_user_puid, ldap_user_puid_property, ldap_user_current_dn, ldap_user_last_checked, ldap_user_ldap_exclude) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4, :db_insert_placeholder_5, :db_insert_placeholder_6, :db_insert_placeholder_7, :db_insert_placeholder_8, :db_insert_placeholder_9, :db_insert_placeholder_10, :db_insert_placeholder_11, :db_insert_placeholder_12, :db_insert_placeholder_13, :db_insert_placeholder_14, :db_insert_placeholder_15, :db_insert_placeholder_16, :db_insert_placeholder_17, :db_insert_placeholder_18, :db_insert_placeholder_19, :db_insert_placeholder_20)', Array, Array) (Line: 32)
Drupal\Core\Database\Driver\mysql\Insert->execute() (Line: 952)
Drupal\Core\Entity\Sql\SqlContentEntityStorage->saveToSharedTables(Object) (Line: 890)
Drupal\Core\Entity\Sql\SqlContentEntityStorage->doSaveFieldItems(Object, Array) (Line: 27)
Drupal\user\UserStorage->doSaveFieldItems(Object) (Line: 257)
Drupal\Core\Entity\ContentEntityStorageBase->doSave(NULL, Object) (Line: 392)
Drupal\Core\Entity\EntityStorageBase->save(Object) (Line: 796)
Drupal\Core\Entity\Sql\SqlContentEntityStorage->save(Object) (Line: 377)
Drupal\Core\Entity\Entity->save() (Line: 109)
Drupal\entity_clone\EntityClone\Content\ContentEntityCloneBase->cloneEntity(Object, Object, Array) (Line: 18)
Drupal\entity_clone\EntityClone\Content\UserEntityClone->cloneEntity(Object, Object, Array) (Line: 83)
Drupal\entity_clone\EntityClone\Content\ContentEntityCloneBase->cloneEntity(Object, Object, Array) (Line: 150)
Drupal\entity_clone\Form\EntityCloneForm->submitForm(Array, Object)
call_user_func_array(Array, Array) (Line: 111)
Drupal\Core\Form\FormSubmitter->executeSubmitHandlers(Array, Object) (Line: 51)
Drupal\Core\Form\FormSubmitter->doSubmitForm(Array, Object) (Line: 585)
Drupal\Core\Form\FormBuilder->processForm('entity_clone_form', Array, Object) (Line: 314)
Drupal\Core\Form\FormBuilder->buildForm(Object, Object) (Line: 74)
Drupal\Core\Controller\FormController->getContentResult(Object, Object)
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 576)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}()
call_user_func_array(Object, Array) (Line: 153)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 68)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 57)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 99)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 78)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 50)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 657)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

It seems like we're trying to do too many things here. Here's a plan to get back to basics:

• Remove the form from the process (no options).
• Clone Entity Reference Revisions (e.g. Paragraphs) sub-entities.
• Don't clone any other sub-entities.

This has the benefit of:

• Streamlining the UX by reducing the number of steps.
• Not confusing the user. (e.g. Which checkboxes should I check?)

I think it's safe to make assumptions about what types of entities to clone. Generally, we only want to clone entities that are tightly coupled with (and meaningless without) their host entities. For example, we do want to clone Paragraphs because they only exist in the host entity context, but we don't want to clone Users because they don't. Users exist on their own outside the referencing entity and have value in other contexts.

Even though Field Collections are another example of a tightly-coupled sub-entity, let's forget about them for now as they don't implement ERR and this makes it more complicated. This can be implemented in a separate follow-up issue. If there are use cases for cloning other types of sub-entities (those that aren't tightly coupled), these can be handled in other follow-up issues, and discussed there. Let's got a basic implementation of this working first so that we can cut another alpha release with it, and have something to build on.

Thoughts?

Hi @colan,

sounds good for me. And I'm ready to test further patches.

Ciao
Ralf

@vpeltot: What do you think?

Hi, we have recently faced the issue with paragraph references on two projects and your proposed solution (#67) would be exactly solving our problem.

This module provide a core to allows cloning any kind of entities.

The clone system, and specially for this issue "Support for sub-entity cloning", must be the same regardless of the target entity types attached on entity_reference (maybe entity_reference_revisions) fields.

Not all sites use paragraphs, not all sites use field collections, but ALL SITES use some entity reference fields.
We can't consider and handle all use cases with this module.
We must be (as must be any contrib module) as generic as possible.

To solve this issue and this discussion, and improve the UX, what do you think about:

• Keep the clone process as it is in #63 (with the form)
• Add an admin UI to choose the default cloning action for each target entity types (cloning too, keep the same target, ...) and build the form considering this settings (checkboxes with default values TRUE/FALSE and maybe disable TRUE/FALSE).
• Add an admin UI to choose if the form element with checkboxes (regarding the previous settings) must be viewed by the user.

To handle all other uses cases, this module add two handler in entities:

• entity_clone
• entity_clone_form

Everyone can be create their own handler classes for custom entities or override all existing handler classes.

Add an admin UI to choose if the form element with checkboxes (regarding the previous settings) must be viewed by the user.

If by this you mean that there's a configuration option to skip the clone form (a checkbox like Allow users to specify sub-entity cloning behaviour that can be left unchecked), and just use the defaults, I think this is a good solution which (1) can provide a good UX (i.e. no clone form), and (2) allows for clone workflow flexibility. This should satisfy all of the feedback we've received so far.

@vpeltot: Do you have time to work on this? If not, I can work on it in the next couple of weeks.

@colan, thanks for your help, but I started this feature since some days.
Here a first patch with new settings form /admin/config/system/entity-clone to define the entity clone form settings.
This form allows to set (for referenced entities in the entity clone form regarding the target entity type):

• The default value checkboxes
• Disable (TRUE/FALSE) checkboxes
• Hide (TRUE/FALSE) the fieldset containing checkboxes

@colan, Can you test this patch? And maybe refine:

• Wording
• Settings UX

Fix tests errors

Fix settings manager class name in service file.

Yes, will test shortly. Thanks for working on this. I found some coding standards issues so fixing that and breaking up some of the big methods first.

Here's the code clean-up. I didn't get a chance to test it yet, but I will shortly.

Noticed this failed so unassigning in case someone wants to take a look while I'm off for the day.

New patch with:

• The same feature
• @colan Coding standards fixes with changes
• All other coding standards fixes

Hm, the patch works only for the following settings:

- CHECKBOXES DEFAULT VALUE – checked
– DISABLE CHECKBOXES – not checked
- HIDE CHECKBOXES – not checked

If either »DISABLE CHECKBOXES« or »HIDE CHECKBOXES« are checked, cloning doesn't clone referenced entities. And deleting a cloned node will delete all paragraphs. And all affected nodes have missing references ;-(

The problem is situated in the cloning form. If you disable or hide the checkboxes, the default value setting is not considered by the cloning form.

I want to have the following settings to work:

- CHECKBOXES DEFAULT VALUE – checked
– DISABLE CHECKBOXES – not checked
- HIDE CHECKBOXES – checked

The editor gets an well known interface (not checkboxes) and cloning works for referenced entities too.

Here's the last interdiff.

@vpeltot: The code looks a lot better now, which is great. Thanks for switching to ::class everywhere in entity_clone.module. That was the next step. I still think ContentEntityCloneBase::cloneReferencedEntities() is too long. Methods shouldn't be longer than 20 lines or so, unless they're producing a big form or some such. That's why I broke out getChildProperties() and getTargetIds(). Maybe these could use better names, but is there a reason we can't do something like this? It would make the code much more readable.

That aside, I'll start looking at functionality.

@rwam: Thanks for the extremely quick reviews!

What I've found so far UX-wise:

1. There's no configure link on the Extend (modules) page. I added this earlier, but it was removed. Not sure why.
2. The configuration page is under Configuration -> System, which it would be more appropriate to be under Configuration -> Content authoring.
3. We need a high-level description of what each form is about and why it's necessary, a short paragraph at the top. This can be implemented with $form['description'] = ['#markup' => 'Form description here'];. Needs to be done for both:
   • The config form
   • The clone form
4. I'm not seeing a Your configuration settings have been saved message after saving the configuration form. Maybe we need to specify that it's an admin form?

We can worry about the above items later; I'm happy to get this thing working first.

Functionally, on running the clone operation with Paragraph fields, I wasn't getting new entities (even though it looked like I was as I could change the clone's values without it affecting the original). As mentioned above, deleting the clone would delete the original's values.

I believe I found the problem over here:

+++ b/src/EntityClone/Content/ContentEntityCloneBase.php
@@ -65,4 +86,49 @@ class ContentEntityCloneBase implements EntityHandlerInterface, EntityCloneInter
+        && $properties['recursive'][$field_definition->id()]['references'][$referenced_entity->id()]['clone'] === 1) {

Dropping the ===1 seems to work better as the value is probably a string and not an integer (having come from the form submission). When I delete the clone, the original's values are still there. Can someone else confirm?

The ability to clone sub-entities, (entities within entities), is essential to be able to support cloning of Drupal Commerce 2.x entities. A Commerce entity always consists of a product entity with at least one product variation entities embedded, in which there can also be product attribute entities embedded within the product variation.

Currently, entity clone cannot correctly clone a Commerce 2.x entity.

I believe I found the problem over here:

+++ b/src/EntityClone/Content/ContentEntityCloneBase.php
@@ -65,4 +86,49 @@ class ContentEntityCloneBase implements EntityHandlerInterface, EntityCloneInter
+ && $properties['recursive'][$field_definition->id()]['references'][$referenced_entity->id()]['clone'] === 1) {
Dropping the ===1 seems to work better as the value is probably a string and not an integer (having come from the form submission). When I delete the clone, the original's values are still there. Can someone else confirm?

It seems to work better, right. But I think there are more tests necessary, because it was a quick test and for paragraphs only.

This should fix all of #85 and #86. Please test.

Hi @colan,

settings in combination with hidden checkboxes work well, but the following setting results in the same error of corrupt clone with not cloned sub entities:

- CHECKBOXES DEFAULT VALUE – checked
– DISABLE CHECKBOXES – checked
- HIDE CHECKBOXES – not checked

By the way: I'm undecided because of the amount of text on the clone form page. It's also too technical from my side. Which content editor knows all kind of build in terms. Especially if all options are hidden this could lead to some frustrations to the editor. Any other thoughts to this topic are appreciated.

Ciao
Ralf

The patch works for me on a paragraph with a nested paragraph. But it's not working on the translation.

So my node:

node:
- paragraph
-- paragraph

has a translation NL & EN

The EN translation has correct new nested paragraphs. The NL has an incorrect lowest level paragraph (still the original). It also didn't change the NL node title with the clone suffix.

I can confirm the bug in #86 re. "deleting the clone would delete the original's values", even with the latest patch.

I am working with a Paragraph field with multiple paragraphs, and while it appears that editing the original's paragraphs does not affect the cloned node, deleting the original node will delete the paragraphs in the new, cloned node.

Also, I was unable to hide the form shown when clicking the Clone menu item, regardless of the configuration of the module - is it forced shown for admin users?

Probably better to split the huge patch in parts, it has a lot of code style changes (array syntax & @file)

Hmm, stumbled across this and i have to admit that my wordings in #2706639-4: Support for sub-entity cloning was not fully correct.

ERR only is a forced composite relationship if it is annotated as such. (For instance Paragraphs annotates the fields that ERR can use as a parent reference).

If you use ERR without that annotation, it can be used to point to any revision of a non-composite entity and that case should be treated like regular ER. Side note: The ERR does not offer a widget at all that allows a user to see that the reference points to an outdated revision and neither offers tools to update that reference. This functionality could make sense for demanding legally strongly moderated content situations, but needs further investigation and support.

I think it's fine to focus on a simplified implementation and support the extra use-case in follow-ups.

@rwam

I tried to reproduce your use case, but I don't

My test, step by step:

Configuration:

• Install a new drupal instance with standard profile
• Enable Paragraphs module
• Enable entity_clone module
• Create 2 paragraphs types including the same title field:
  Paragraphs type A
  Paragraphs type B
• Add 1 entity_reference_revision field in Article content type. This field reference the 2 previous types
• Entity clone settings for paragraphs: 1 / 1 / 0 (Default value / Disable / Hide)

Create contents:

• Create 1 Article content
• Title: Article test
• Paragraphs:
  Paragraph A
  Paragraph B

Clone:

• Clone action: The form is correctly created regarding the entity clone settings
• Do clone

After clone:

• 2 articles with 2 paragraphs both:
  Article test
  Article test (Cloned)
• Edit the cloned article, add 'Cloned' to the paragraphs A and paragraphs B title
• Result:
  First article is always the same and paragraphs still the same too
  Cloned article and their paragraphs have the 'Cloned' values

Hi @vpeltot,

that's right, the revisions are different, but the entity id is the same. There is one more step missing:

Delete content:

• Delete the cloned article
• Result:
  All paragraphs are missing on the original too.

Ciao
Ralf

@rwam

Yeah ! I have reproduced your (last?) bug !

The cause was: When a form element is disabled, the #default_value is ignore in $form_state->getValues().

I've replace:
'#default_value' => $this->entityCloneSettingsManager->getDefaultValue($referenced_entity->getEntityTypeId()),
by
'#value' => $this->entityCloneSettingsManager->getDefaultValue($referenced_entity->getEntityTypeId()),

Here a new patch with:

• This fix
• Remove all file with only checkstyle fixes #2927589: Fix code style as @andypost suggest it

without kint() calls ... :-)

Hi @vpeltot,

Yeah ! I have reproduced your (last?) bug !

May be, we'll see ;)

I've tested your patch in #99 and it works fine. Cloning nodes with paragraphs and with disabled or hidden checkboxes (described in #84) creates new entities resp. revisions and it's possible to delete original or cloned content without losing any other content.

I think, other tests are necessary (Commerce, Media, Blocks), but it looks pretty good so far.

Thank you and ciao
Ralf

Attached is an interdiff for easier code review between my last patch, and the last one here which fixes the deletion issue, the failing test, and some coding standards.

I've updated the list of things to do / test in the IS based on recent comments.

Can folks test to ensure that the data loss problem is fixed? If so, maybe we can commit this, and then get everything else done in follow-up issues. This issue is getting rather long and complex.

Let's end with this issue.

I've created a new issues for less critical todo:

• "Make sure translations are still working": #2929334: Make sure translations are still working
• "Make help text on clone form less technical and more understandable": #2929336: Make help text on clone form less technical and more understandable
• "Ensure clone form isn't showing up if disabled": #2929338: Disable clone form for users

Remaining tasks:

• Test coverage: Delete cloned entities has no impact for original entity

Add test assertions:

• Delete cloned entity and their references: original entity and references are not deleted.

Tests OK, no more remains to be done.

Successfully tested on 8.4.3 with deeply nested Paragraphs. However, when the content has a Paragraph that references a Menu via an Entity Reference field on the Paragraph Type, the following error is thrown immediately after the Clone link is clicked:

Recoverable fatal error: Argument 1 passed to Drupal\entity_clone\EntityClone\Content\ContentEntityCloneFormBase::getChildren() must implement interface Drupal\Core\Entity\ContentEntityInterface, instance of Drupal\system\Entity\Menu given.

This is certainly a less common use case, but since Paragraphs can reference entities that aren't content, probably worth fixing at some point. FWIW, I am using the menu_reference_render module to output the referenced menu entity, although that isn't coming into play during the clone.

Added related issue

#2933820: Consider changing clone handlers to use core's entity duplication API would enable us to take advantage of paragraph module's existing deep-cloning support

1. +++ b/src/EntityClone/Content/ContentEntityCloneBase.php
   @@ -54,15 +58,136 @@ public static function createInstance(ContainerInterface $container, EntityTypeI
   +    $type_is_clonable = in_array($field_definition->getType(), $clonable_field_types);
   
   +++ b/src/EntityClone/Content/ContentEntityCloneFormBase.php
   @@ -0,0 +1,188 @@
   +        if ($field_definition instanceof FieldConfigInterface && in_array($field_definition->getType(), ['entity_reference', 'entity_reference_revisions'])) {
   

   nit, should use third argument, as we know these are strings

2. +++ b/src/EntityClone/Content/ContentEntityCloneBase.php
   @@ -54,15 +58,136 @@ public static function createInstance(ContainerInterface $container, EntityTypeI
   +      if (isset($properties['recursive'][$field_definition->id()]['references'][$referenced_entity->id()]['clone'])
   +        && $properties['recursive'][$field_definition->id()]['references'][$referenced_entity->id()]['clone']) {
   

   can just use !empty() here

3. +++ b/src/EntityClone/Content/ContentEntityCloneBase.php
   @@ -54,15 +58,136 @@ public static function createInstance(ContainerInterface $container, EntityTypeI
   +  protected function formatTargetIdsAsArray($target_id, $target_revision_id) {
   +    return [
   +      'target_id' => $target_id,
   +      'target_revision_id' => $target_revision_id,
   +    ];
   

   in theory, this shouldn't be needed. The setValue call with an entity object should handle this for you at the typed data layer. i.e. you should be able to set the value using the entity objects. See http://cgit.drupalcode.org/drupal/tree/core/lib/Drupal/Core/Field/Plugin... and http://cgit.drupalcode.org/drupal/tree/core/lib/Drupal/Core/Field/Plugin...

4. +++ b/src/EntityClone/Content/ContentEntityCloneFormBase.php
   @@ -0,0 +1,188 @@
   +  public function __construct(EntityTypeManager $entity_type_manager, TranslationManager $translation_manager, EntityCloneSettingsManager $entity_clone_settings_manager) {
   

   These should typehint interfaces, not concrete classes

5. +++ b/src/EntityClone/Content/ContentEntityCloneFormBase.php
   @@ -0,0 +1,188 @@
   +    if ($entity instanceof FieldableEntityInterface) {
   +      foreach ($entity->getFieldDefinitions() as $field_id => $field_definition) {
   +        if ($field_definition instanceof FieldConfigInterface && in_array($field_definition->getType(), ['entity_reference', 'entity_reference_revisions'])) {
   +          $field = $entity->get($field_id);
   +          /** @var \Drupal\Core\Field\Plugin\Field\FieldType\EntityReferenceItem $value */
   +          if ($field->count() > 0) {
   +            $form['recursive'] = array_merge($form['recursive'], $this->getRecursiveFormElement($field_definition, $field_id, $field));
   +          }
   +        }
   +      }
   ...
   +  protected function getRecursiveFormElement(FieldConfigInterface $field_definition, $field_id, FieldItemListInterface $field) {
   

   what about base fields? it is possible to define an entity reference field or paragraph field as a base field, using FieldConfigInterface we're limiting our options

6. +++ b/src/EntityClone/Content/ContentEntityCloneFormBase.php
   @@ -0,0 +1,188 @@
   +      $form_element[$field_definition->id()]['references'][$referenced_entity->id()]['clone'] = [
   +        '#type' => 'checkbox',
   +        '#title' => $this->t('Clone entity <strong>ID:</strong> <em>@entity_id</em>, <strong>Type:</strong> <em>@entity_type - @bundle</em>, <strong>Label:</strong> <em>@entity_label</em>', [
   +          '@entity_id' => $referenced_entity->id(),
   +          '@entity_type' => $referenced_entity->getEntityTypeId(),
   +          '@bundle' => $referenced_entity->bundle(),
   +          '@entity_label' => $referenced_entity->label(),
   +        ]),
   +        '#default_value' => $this->entityCloneSettingsManager->getDefaultValue($referenced_entity->getEntityTypeId()),
   +      ];
   

   this should include an access check, '#access' => $referenced_entity->access('view label'),

7. +++ b/src/EntityClone/Content/ContentEntityCloneFormBase.php
   @@ -0,0 +1,188 @@
   +  public function getValues(FormStateInterface $form_state) {
   +    return $form_state->getValues();
   

   is this needed?

Removed the related issue, I missed the fact that the form already called that.

Also, there are a lot of coding standards changes in the patch.

In my opinion, to minimize the disruption and size of the patch, those should be split into their own issue.

E.g, this patch conflicts with #2845094: Batch field creation but only because of the coding standards changes

+++ b/src/Form/EntityCloneForm.php
@@ -102,11 +102,26 @@ public function getFormId() {
+      $form['description'] = [
+        '#markup' => t("
+          <p>Specify the child entities (the entities referenced by this entity) that should also be cloned as part of
+          the cloning process.  If they're not included, these fields' referenced entities will be the same as in the
+          original.  In other words, fields in both the original entity and the cloned entity will refer to the same
+          referenced entity.  Examples:</p>
+

This should be hidden if all checkboxes are also hidden.

See screenshot

1. +++ b/src/EntityClone/Content/ContentEntityCloneBase.php
   @@ -54,15 +58,136 @@ public static function createInstance(ContainerInterface $container, EntityTypeI
   +   * @param Drupal\Core\Field\FieldDefinitionInterface $field_definition
   

   missing leading \

2. +++ b/src/EntityClone/Content/ContentEntityCloneBase.php
   @@ -54,15 +58,136 @@ public static function createInstance(ContainerInterface $container, EntityTypeI
   +  /**
   +   * Fetches the properties of a child entity.
   +   *
   +   * @param array $properties
   +   *   Properties of the clone operation.
   +   * @param \Drupal\Core\Field\FieldConfigInterface $field_definition
   +   *   The field definition.
   +   * @param \Drupal\Core\Entity\EntityInterface $referenced_entity
   +   *   The field's target entity.
   +   */
   ...
   +    return $child_properties;
   

   Missing @return tag

Fixes #114

this also needs an update hook to create the new config object and clear the cache, to setup the entity_clone_form handlers on the entity types

@larowlan Thanks for this review.

In this patch:

#111

1. Done
2. Done
3. Done
4. Done
5. I think it would be better to create a new issue with a concrete use case and no longer block this issue
6. Done
7. Yes, this method is needed and used by \Drupal\entity_clone\Form\EntityCloneForm::submitForm to get form values from specific form handlers

#115
Done

#116
Included with rework. Description and method descriptionShouldBeShown moved in \Drupal\entity_clone\EntityClone\Content\ContentEntityCloneFormBase

#117
Done

Thanks for this patch ! RTBC ++

• All cloned entities have a new id (they are really cloned)
• The EntityCloneSettingsForm is working well

Just corrected 3 typos (some trailing whitespace when applying the patch..)

I guess some english natives should verify the form description.
At least I suggest this one :

For each type of child entity (the entity that's referenced by the entity being cloned), please set your cloning preferences.

=>

Please set your cloning preferences for each type of child entity referenced by the entity being cloned.

I guess the 2 failing tests are not related to this patch, but also failing also on the 8.x-1.x branch !
Can't find why..

Awesome work here with this patch! Been testing it on a site that has lots of content and references, and I regularly run into this error:

Error: Call to a member function getValue() on null in Drupal\entity_clone\EntityClone\Content\ContentEntityCloneFormBase->getRecursiveFormElement() (line 160 of /modules/contrib/entity_clone/src/EntityClone/Content/ContentEntityCloneFormBase.php)

This happens when a reference field contains a target_id that points to an entity that has been deleted. I've added a new patch based on the previous one, where we simply skip the deleted entity instead of trying to load/clone it.

#123: Please provide an interdiff.

Attached an interdiff for the last patch #123.

Some .DS_Store stuff was introduced in #123, so this is a new patch

Does this support cloning panels pages? Along with the variants and block plugins inside it.

This new one should pass tests.

I guess this is the change : Issue #2911806: Remove unnecessary Crypt::hashBase64() call from Action UI by tim.plunkett.
This is impacting the whole module, not this patch particulary.
(I made another ticket there #2950432: EntityCloneActionTest fails)

In relation to this ticket https://www.drupal.org/project/entity_clone/issues/2945192

I noticed that all form elements in the formElement() method in the ContentEntityCloneFormBase class get built no matter what, even if the user has no access to them (because the "Hide checkboxes" option is checked at /admin/config/system/entity-clone). I've made an addition to the patch that simply skips the fields if we cannot access them.

This does not actually solve the problem of the timeout in #2945192 due to recursive looping, but it does offer a workaround: if an admin decides how to clone entities (and thus checks all the "Hide checkboxes" on the config page), we don't need to build a render array and get no error.

Again, I know it doesn't actually solve the problem but I think it's a good addition either way, no need to build stuff that will not be rendered anyway. The cloning actions seems to be successful without this "invisible" form too, though that might be because of my specific setup. I'm sure one of the module maintainers will have a more qualified opinion on this :)

#132: Please provide an interdiff so we can more easily review changes. Thanks.

Interdiff

Actually that latest change of mine makes it so that sub-entities don't get cloned :/ The cloned node just refers to the same entities as the original page. So please disregard the patch.

I guess the problem for issue 2945192 lies in the fact that recursive cloning can get quite complicated depending on your setup. Looking at the image below you can see that I'm already 5 levels deep, and that is just from one node referencing a paragraph that than in turn references another node that has another paragraph, ...

I am thinking that not displaying anything below the first unchecked checkbox by default could help, and when a user checks the "yes let me clone this entity" checkbox, ajax in the sub-elements? It would in any case avoid the rending of a huge form with a lot of nested elements from the beginning.

For the project that I'm working on now I've created a patch that does not display any sub-elements if the parent type shouldn't be cloned. It works for me because the editors have no control over what they can clone or not, so all the checkboxes are hidden by default. This is not a generic solution though.

#137: Please create a follow-up issue for that.

A follow-up issue on what? There is already an issue here https://www.drupal.org/project/entity_clone/issues/2945192 but I figured I'd keep it all in one thread since it's about the same functionality.

I was talking about this UX issue you raised:

I am thinking that not displaying anything below the first unchecked checkbox by default could help, and when a user checks the "yes let me clone this entity" checkbox, ajax in the sub-elements? It would in any case avoid the rending of a huge form with a lot of nested elements from the beginning.

My point was that we should focus exclusively on getting the primary goal here finalized and committed. UX improvements and the like can be handled afterwards, in separate issues. If we try to do too much in this issue, we'll never get it done. As can be seen in #2945192: Fatal error: Maximum function nesting level of '256' reached, aborting! With Support for sub-entity cloning patch, a lot of folks are waiting for this.

I agree, but I don't think the ajaxing in of sub-elements is a UX improvement, I saw it as a way of solving issue #2945192. But it might not be the best way, I don't know.

I added another follow-up issue/proposition : #2953641: Possibility to set fields that should not be cloned

Patch at #137 needs some extra work on the EntityCloneContentRecursiveTest.
A schema for our entity_clone form_settings was also missing.

This patch can not be applied to alpha2 version

You should try it on the 8.x-1.x-dev branch

#129 / #130
@matthew.h @DamienMcKenna This module supports to clone config entities, and provide a clone base / core concept for all drupal core entities.
If an entity (like panels & co) doesn't supported by this module, I suggest to create modules for each specificities (eg. entity_clone_panels)

#132 / #137 / ...
All problems around #2945192: Fatal error: Maximum function nesting level of '256' reached, aborting! With Support for sub-entity cloning patch must be discussed / resolved in #2945192: Fatal error: Maximum function nesting level of '256' reached, aborting! With Support for sub-entity cloning patch

This new patch has been created from #118 and contains all @pguillard updates. All updates about #2945192: Fatal error: Maximum function nesting level of '256' reached, aborting! With Support for sub-entity cloning patch was removed.

At least patch helps to clone paragraph references correctly.
I had added the checkbox for paragraphs entity references, and now deletion of the cloned entity has no influence on the references of the original entity.

Ok it's fine for me.
For a next feature improve the ux can be great. ;)

Commited !
Thanks @all

Cool, thanks !

Great news. Thanks @all.

Thanks a lot for all the work done here, the editors in my office are really keen on this patch, will save them a lot of time. Could we get a new release for this in the alpha channel?

Just saw this commit in latest dev I think we should address these before next release.

1. +++ b/entity_clone.install
   @@ -0,0 +1,30 @@
   + * Implements hook_update_N().
   

   Desc should be a summary of what this update is performing.

2. +++ b/entity_clone.install
   @@ -0,0 +1,30 @@
   +function entity_clone_update_8001() {
   

   This should be post-update hook.