Create recipe to clone entities with ECA

Adding related issue.

I've provided a test ECA model which is contained in the MR above and it does the following:

• It removes the quick_node_edit module
• It adds ECA and BPMN.io
• It provides an ECA with 4 options on how to clone any content entity:
  • On the canonical page there is a button called "Clone me" - this is likely to get removed later again
  • A contextual link for content entities to clone that entity
  • An entity operation that can be found in the list of operational links for each entity, e.g. in the content overview
  • In the entity edit form, there is now also a secondary button that's called "Save as new entity" - so instead of cloning and then editing, with this approach the user can edit an existing entity and then save is as a new one instead of updating the existing one
• Each of those redirects to the new entity's canonical page afterwards
• The label, the published state and the owner are configured in the ECA model

What's not implemented yet is access control, I'll add that when we move forward with this proposal.

I've not provided a primary tab as I'm not sure that's what a good UX needed. But if that's what should be done too, I'll be adding that as well.

Had to add a couple of new features to ECA for this to work, that's why it currently requires ECA 2.1.x-dev, but we will make that an official release before Drupal CMS becomes a thing, of course.

Assigning to @pameeela for evaluation, since she is the relevant track lead. I'll do a technical review on this once she approves it.

Thanks @phenaproxima, I'll also provide tests when the decision and scope is being determined.

I kept meaning to add some requirements to this and thought I had some time but no, @jurgenhaas was too quick!

Here is what I was planning to say:
As a content editor, I can easily duplicate a node (just specifying for the sake of clarity here) to use it as a basis for new content. Specifically:

1. When I click to trigger the action, I should see the source node's data in a node/add form (rather than viewing the saved/cloned node)
2. The owner of the cloned node should be the person doing the clone, not the owner of the source
3. The cloned node should not be published by default, regardless of the status of the source node

So based on this there are a few changes required, are these do-able without a lot of rework? Sorry that I didn't get to post this before you started.

When I click to trigger the action, I should see the source node's data in a node/add form (rather than viewing the saved/cloned node)

This is kinda what I thought too and implemented a slight variation of this as the fourth option to edit an entity and use "Save as new entity". But I can also have a look to implement your exact version with a create new entity with all fields pre-populated from the original entity.

The owner of the cloned node should be the person doing the clone, not the owner of the source

That's already implemented like that.

The cloned node should not be published by default, regardless of the status of the source node

Also, that's already implemented.

In addition to that, I was wondering about the create timestamp. While cloning an entity, the original timestamp is usually kept, but I assume that we should set the new timestamp for when the entity got cloned?

This simplified the model even further. There are still 3 options to clone an entity: a button on the canonical page, a contextual link and an operation link. They all open the form to add a new entity, pre-populated with the values from the cloned entity.

The author is set to the current user, and the creation date is set to now. The new entity will be unpublished.

Here is how the model looks like visually:

I have now also added access control to the button and links so that they only show up, if the current user has the permission to create a new entity of the given type. Also removed the draft state from the MR.

jurgenhaas → closed merge request !110

jurgenhaas → reopened merge request !110

This will need strong dedicated functional test coverage, and I'm also not entirely convinced it should be in its own recipe. But let's get the test coverage in first.

Does this work with paragraphs and Layout Builder?

I like the flow a lot better now, but the 'Clone me' button is too prominent. I do think this should be grouped with the primary tabs for now, until / if that UI changes. That feels like the correct prominence and also matches the behaviour of all of the contrib modules. This was mentioned as possible in #4 so I assume this isn't a problem.

I tested changes with LB and they were also cloned, e.g. if you a block to the node via LB and clone the node, the block is there on the clone.

But the media behaviour is another story! When you add a media field, the 'Clone me' button also appears for this, which obviously we can resolve -- but the media cloning doesn't work. When you click clone, it just takes you to the 'Add media' page where you have to select the media type, and even if you correctly choose 'Image' it doesn't work.

My 2c is that we only need this on nodes for now anyway because that is the 99% use case, so my preference would be to just not support it for other entity types by default. But that leaves it as somewhat of a 'dead end' in that if you wanted to extend it to other entity types, you would realistically need contrib.

Then again, if you look at 📌 Evaluate cloning modules Active you can see that only one module (Content entity clone) has the desired UX, plus support for non-node entities. So maybe what we should do is a direct comparison of that module with this solution?

#17's findings lean me even more strongly towards "we should not be reinventing this wheel". If Content Entity Clone gets us most of the way there, and the maintainer is available/willing to accept improvements if we have them, I think that would be preferable to wiring a bespoke solution together with ECA.

Yeah, I think I agree on this one. I also think we should come up with a framework for evaluating ECA vs contrib in general and this issue has helped shape my thinking a bit.

Here are some thoughts:

• Use ECA if it's not easily achieved with existing contrib options (example: email notifications based on various conditions). Conversely, if contrib can do it, we should consider that the preference (example: content cloning).
• Use ECA if we know exactly what we want and we don't care whether it's easily modified (example: default redirects). Conversely, if we want to give site owners the ability to easily change the configuration, without having to learn ECA (example: pathauto?)

I do think that a cloning with ECA recipe is something that will work for some sites so I definitely think it would be used in the wider world, but for the purposes of Drupal CMS is not ideal.

@jurgenhaas what do you think about this?

#17

I tested changes with LB and they were also cloned, e.g. if you a block to the node via LB and clone the node, the block is there on the clone.

Awesome. But I am wondering if it pointed to the same block as the one cloned and not a duplicated one. Paragraphs and LB when overrides are composite so they should be duplicated too and not referenced as with other entity references.

A big + with using an existing contrib is that is hopefully already is battle tested in production too!

+1 for Content Entity Clone, we will need to create a feature request in the module issues queue :

+ When you click to clone an entity, the module redirect you to /{entity}/add/{entity_type}?content_entity_clone={entity_id} and show you the source entity data (depends on the configuration)
- Status = Published, This need to be changed following @pameeela request and Status should be Unpublished by default
- Missing a button "Clone" in Primary tabs / Local tasks

++ Thankfully this issue helped improve ECA a new issue ( #3481200 ECA Render: add support for local tasks ✨ ECA Render: add support for local tasks Active ) was resolved.

@jurgenhaas what do you think about this?

Doing a solid comparison between contrib candidates and an ECA model is certainly a great thing. It would even work much better, if we had a solid specification of the requirements first, but often times we go with "let's find the solution that we like the most" which is perfectly OK as we only have limited influence and a short timeframe to get things done.

However, on the ECA side, we've addressed all the concerns raised so far:

• The "Clone me" button was only there for testing purposes, it's now replaced with a primary tab - I understand that's what's perceived the preferred UX
• The ECA model also provides the feature as a context and as an operational link
• Having the functionality available for all content entity types is a plus in my view; restricting it to nodes ootb is easy to do, if that's what we want
• The media clone issue was a bug and got fixed; although it's not required right now

In summary, ECA for entity cloning comes with now additional overhead and no extra issue queue from yet another module would have to be considered. As I'm not aware of any short coming of the latest ECA model compared to the modules, I'd vote for ECA - but it's not something "to die for on this hill" ;-)

Whether we'll utilize that or not, I'm going to update the MR with the latest model in a minute.

OK, I'm calling this one, we're going with ECA for this :) We'd like to limit it to just nodes though as we want to limit the scope so that we can be sure that it works.

Just one change, we'd like to change the name of the action to 'Duplicate' instead of 'Clone'. This aligns with not only other systems but other modules such as Webform and Pathauto, and hopefully avoids some of the problematic assumptions that come with the word 'clone'.

Hadn't even realised this was possible with ECA, but +1 to one less module/dependency!

Nice, I'll work on this today.

I've updated the recipe in MR!110 and took this approach:

• Renamed the recipe to drupal_cms_eca_entity_duplicate
• Change Clone to Duplicate everywhere
• Left entity instead of changing to node in case the scope will be broadened later
• The ECA model is limited to duplicating nodes at this point

It's working as expected, but the tests that I've added are failing when checking for the links. @phenaproxima is that a limitation in functional tests and do I have to switch to cypress tests instead? I guess, I need your help on this.

Left some suggestions to improve the test coverage.

@phenaproxima I've resolved all your suggestions and moved the test to cypress. It's still failing as the primary tab "Duplicate" doesn't show up. However, when I apply that recipe on a local Drupal site, it works as expected and that primary tab is visible.

We got bugs, my friends! I discovered this by writing real strong test coverage in Cypress, covering the three ways you can clone content (tab, contextual link, drop button).

The following things need to be investigated and fixed:

• If you visit /admin/content and clone a node using the "Duplicate" drop button link, it appears to clone it twice!
• A less grievous, but still annoying and confusing bug is that, if you clone a node via a contextual link, it will redirect you to the original node, not the clone.

Kicking back for those things. But these are already covered by the tests, so all we have to do is fix these problems, plus some manual testing to uncover any more weird stuff that might exist.

Thanks a lot @phenaproxima on your support on this one.

If you visit /admin/content and clone a node using the "Duplicate" drop button link, it appears to clone it twice!

I can't reproduce this. I get exactly one duplicate after saving the form.

A less grievous, but still annoying and confusing bug is that, if you clone a node via a contextual link, it will redirect you to the original node, not the clone.

I've analysed this and we run into a core behaviour here: it appends destination=/CURRENTPAGE to all contextual links and there is no way to prevent that from happening. Even if we provide our own destination in the link, core overrides that always. I've also tried to hook into the form submit and set a destination there, with no luck, as the destination argument always comes first. @catch do you know of any way to prevent this from happening? If that's not possible, we should probably remove the duplicate feature from the contextual links.

If core is being a pain with the destination thing, I'd say we should removing cloning from the contextual links. That seems like the easiest path forward.

Yeah removing it from the contextual links and opening a follow-up makes sense.

I can't reproduce this. I get exactly one duplicate after saving the form.

Ah, you're right - it turned out to be a flaw in the test, which is seeing the link in the "node was saved!" message and and the link in the table. That's fixed now.

Leaving at Needs Work in order to decide what to do about the contextual link behavior.

The contextual link and its test has been removed and tests are all green now.

Awesome! Assigning to Pam for
manual testing and acceptance.

LGTM, left a comment about a seemingly random ddev change but otherwise all good!

Merged into 0.x, thank you everyone! It's great to add this essential feature in a way that really fits our needs and has such heavy customization possibilities. The strong test coverage will really help cover our asses into the future. Onward!

Nice work !

Automatically closed - issue fixed for 2 weeks with no activity.