🇷🇸Serbia @sakiland

Here's the patch for Drupal 10.2 (this version of Drupal use $this->renderer->renderPlain() instead of $this->renderer->renderInIsolation()

Here's a great elaboration 🐛 More link disappears when time-based views cache is enabled Needs work for the issue, and why we have issue with caching.

Here's the patch if somebody needed it until final fix.

I've just created MR with implemented @wendell and @malik.kotob work and additional check.

Here's also patch for this

sakiland → made their first commit to this issue’s fork.

I'm not sure if this is the right place to put this (the issue is fixed and merged to the 10.3.x and 11.x) or I need to open another bug, but this issue needs more work.

The fix doesn't check type of the returning value for titleResolver

$title = $this->titleResolver->getTitle($request, $route_match->getRouteObject())->render();

Function getTitle() can return an array or a string

<?php
namespace Drupal\Core\Controller;

interface TitleResolverInterface {

  /**
   * Returns a static or dynamic title for the route.
   *
   * @return array|string|\Stringable|null
   *   The title for the route.
   */
  public function getTitle(Request $request, Route $route);

}

This can produce error if some module overwrite this function and return a string. For example, with Webform module I'm getting following error:
Error: Call to a member function render() on string in Drupal\Core\Render\MainContent\DialogRenderer->getTitleAsStringable() (line 145 of /var/www/html/web/core/lib/Drupal/Core/Render/MainContent/DialogRenderer.php).

So, we need to put additional check, for example:

  protected function getTitleAsStringable(array $main_content, Request $request, RouteMatchInterface $route_match): \Stringable|string|null {
    $title = NULL;
    if (array_key_exists('#title', $main_content)) {
      if (is_array($main_content['#title'])) {
        $title = $this->renderer->renderInIsolation($main_content['#title']);
      }
      else {
        $title = $main_content['#title'];
      }
    }
    elseif ($titleObj = $this->titleResolver->getTitle($request, $route_match->getRouteObject())) {
      if (is_string($titleObj) || $titleObj instanceof \Stringable) {
        $title = (string) $titleObj;
      }
      else {
        $title = $this->titleResolver->getTitle($request, $route_match->getRouteObject())->render();
      }
    }
    return $title;
  }

sakiland → created an issue.

I started to work on this 3 years ago in order to fix issue with duplicate Entity reference 🐛 New revision is not created via Layout Builder Active

The main check in that issue 🐛 New revision is not created via Layout Builder Active is the code
if ($host->isNew() && $host->isDuplicate()) {

And this line will fix all issues related to the cloning process in multiple contributed modules (I've already wrote about this one year ago).
For that purpose we only need to know if entity is new one and also duplicate.

As initial solution, I've implemented this method in \Drupal\Core\Entity\EntityBase (in the similar way as \Drupal\Core\Entity\EntityBase::isNew

That was working solution, easy to test and maintain.

Solution with hook is also ok if that can fix the issue with the related issue.
Unfortunately, I don't have any live project related to this issue (client left the Drupal technology).

So, for this issue the main task should be to define in which direction we want to go. Otherwise we will waste valuable developer's time, loosing clients due the the issue with loosing data, etc.

Here's the patch

sakiland → created an issue.

I've tested this patch for Drupal 9.5, with Drupal core patch #57 from the issue → and everything works ok.
Still I haven't tested this for the Drupal 10.1, but I hope I will have time in the following month to do this too.

Here's the patch for the 9.5.x branch.

IMPORTANT: I will change the base branch for MR 204 to 10.1.x, so it will be no valid for the 9.5.

@aaronmchale Thank you for the recommendation. Ok, I will do as you suggested and create a patch file for 9.5
At the end, most important is to make it easier review process :)

@aaronmchale Thank you for the suggestion, I appreciate it, and I understand what is your point. But, it seems you are missing the context of this issue. This issue is almost 4 years old, and there are couple of production sites already using this MR. These sites are probably still on the Drupal 9.5 (for my sites I'm sure they are on the 9.5 :), and it will be couple of next months)
That's why we need MR against 9.5 branch, in order to not break existing sites already using this patch.

Also, I don't expect that this patch will be back-ported to the 9.5, this is too much effort for nothing (sites with Drupal 9.5 will continue to work with this patch in the same way as in previous couple of years. so there is no need for back-port).
I don't see any extra effort if we have separate MR for 9.5 branch. Core committer should only check the main branch (main MR) that will target 10.1.x (same approach when we have multiple patches on the issue, core committer will check only the last one that is tested by community). Yes, having multiple branches is not so clear when we have one branch and one MR, but I don't see any other solution for this kind of complications with multiple active versions (but, this out of scope for this issue, so for any other improvement regarding this, we can discuss on Slack :) )

I will change status to need work until I make a valid MR to the 10.1

Yes, I follow you, but Drupal 9.5 is supported until Nov 2023. We need MR against 9.5 too. That's why we need a separate branch. And the main branch should be against 10.1 as you suggested. And reviewing should only be done on the main branch (I use this patch over two years from 8.9 to 9.5 without issue, and I think there are couple of people who use it too, so I don't think we need extra reviewing for old version branches)

Yes, that's right. Version should be 10.1.x
I will change that next week, but first we need branch for 9.5

@ksenzee after can you please rebase your branch 3040556-duplicate-entity-source-9.5.x to the latest 9.5.x code, in order to have a valid MR (patch) for the 9.5.x code base.

After that I will change base version for main branch. Otherwise I will create a new branch for 9.5. But, I will prefer to keep as little as possible number of different branches if we don't need them.

I fixed failed test via adding mocked storage service.

In commets above I can see some issues with search_api module. Can you please check if the last code from branch 3040556-duplicate-entity-source produce this issue and give me more info regarding this?

Thank you!

I've fixed main branch 3040556-duplicate-entity-source. Now it contains valid code. Currently, MR 204 related to this branch, is created against 9.5.x; but after 9.5.x release date, I will move this to the 10.1.x

@ksenzee after 9.5.x release date, please rebase your branch 3040556-duplicate-entity-source-9.5.x to the latest 9.5.x code, in order to have a valid MR (patch) for the 9.5.x code base.

All other branches without open MR are not valid (these are deprecated or temporary branches, but I don't have options to delete them).

Regarding failed test; I'm working on this, but I don't have an idea how to solve this.
In the unit test Drupal\Tests\Core\Config\Entity\ConfigEntityBaseUnitTest there is configuration entity with random id. And for this entity, the storage is empty, and that's why test failed.

$storage = $this->entityTypeManager()->getStorage($this->getEntityTypeId());

/** @var \Drupal\Core\Entity\EntityDuplicateInterface $duplicate */
$duplicate = $storage->createDuplicate($this);

It's impossible to do rebase against 9.5.x on the existing branch `3040556-duplicate-entity-source` (also on the branch `3040556-duplicate-entity-source-9.5.x`), so I've created a new branch `3040556-duplicate-entity-source-fix-rebase`

Let's wait weather test passing and I will do some branch clean up in the following days.
Thank you @ksenzee for your branch `3040556-duplicate-entity-source-9.5.x`, otherwise I will lost some code (it's my mistake, my Phpstorm rebased and pushed code at once :( )

Thank you for your contribution on this issue,

Definitely, we need an appropriate way to react on entity being cloned (duplicated). This is needed for the proper functionality of the core Layout builder. Here's the issue → that I had with customizing the layout on the node level. It is related to the Entity Reference Revisions module, it doesn't react in the appropriate way when an entity is cloned, but it doesn't have a way how to react to this event. Here's some solution 🐛 New revision is not created via Layout Builder Active , but it depends on the method isDuplicate

Also, each clone module has an issue with the paragraphs, and each fixes in a different way, but the core issue is in the Entity Reference Revisions module. For example:

• Entity Clone 🐛 Inline Blocks on Layout Builder Fail to Clone Correctly Needs work
• Quick Node Clone →

If you check the beginning of this issue

Every entity has a createDuplicate() method, but that method blasts away the original ID without saving it anywhere.
This makes it impossible to react to an entity being duplicated.

so we need somewhere the reference to the original entity and also a way to check if an entity is a duplicate.

EntityInterface contains the method isNew() and I think it should also contains the method isDuplicate. Or if you think this is BC, we can move this method into EntityBase

I've created an initial quick solution without creating hook support and move logic into entity storage. I need this so I can fix issue with duplicating paragraph via Layout Builder #3190523: Paragraph is not cloned in Layout builder → . This is just start point for this issue, but it's working solution.

I've also crated two branches (one for 8.x and another for 9.x) and created two MR.

If someone needs patch file to apply these changes via composer, near each MR there is 'plan diff' url to the appropriate patch.

sakiland → made their first commit to this issue’s fork.