Problem/Motivation

When cloning a node that contains paragraphs, the source entity's paragraphs parent_id field data is replaced with the new cloned node nid.
Currently it seems that it doesn't matter if you configure the cloning to clone the paragraph or not, either way the parent_id gets replaced on the source entity.

So let's say we have a node with nid 1 and it contains a paragraph with id 1 and the paragraph will have parent_id 1 (which references the node). Now when the user clones nid 1, node 2 is created and also paragraph with id 2. But on node nid 1 (source entity), the paragraph parent_id is now not 1, but 2 (the cloned nid).

I created a simple D8 site with Paragraphs and Entity Clone on simplytest as well to reproduce this bug.

Source entity: https://d09m6.ply.st/node/1
Cloned entity: https://d09m6.ply.st/node/2
JSON:API request to view the data:
https://d09m6.ply.st/jsonapi/node/page?include=field_paragraphs&fields[n...

Screenshots:
Source node with corrupted paragraph parent_id:
Modified source
Cloned node with correct paragraph parent_id:
Cloned node

Not a bug on jsonapi, the data is the same in the db, checked locally. Why I am mentioning this is that we are using jsonapi and a headless Drupal backend so, we are using the parent_id to normalize the data attributes and the included fields from jsonapi response. That is how we stumbled upon this issue.

Currently we started using quick_node_clone module as a workaround, as I didn't have enough time to dwell into entity_clone to find the issue. I looked through the cloning process and didn't find a way how a source entity could be modified, but my guess is that it has to do with somewhere a PHP reference being used. Or with revisions being used?

But the strange thing is that when viewing the nodes in the Drupal ui, everything seems to be normal and there is no way of noticing that something goes wrong, unless you use the parent_id field somewhere. So probably $paragraph->getParentEntity() should result in incorrect node as well.

Steps to reproduce:
1. Create a new node that contains some Paragraphs.
2. Clone the new node
3. View the parent_id value of the original node's Paragraph. It will now point to the cloned node nid, instead of the original node's nid.

Remaining tasks

Figure out what is causing the bug.
Patch.

___

I might find time to look more into it at some point, but not right now.

CommentFileSizeAuthor
#64 corrupted_data_paragraphs-3060223-64.patch8.82 KBn.ghunaim
#62 entity_clone-fix_corerupted_data-3060223-61.patch912 bytescharginghawk
#60 entity_clone-fix_corerupted_data-3060223-60.patch546 bytescharginghawk
#59 entity_clone-fix_corerupted_data-3060223-59.patch451 bytescharginghawk
#57 entity_clone-fix_corerupted_data-3060223-56.patch936 bytescolan
#49 corrupted_data_paragraphs-WITH-UPDATE-3060223-49.patch8.76 KBcharginghawk
#49 corrupted_data_paragraphs-3060223-49.patch4.75 KBcharginghawk
#49 corrupted_data_paragraphs-TEST-ONLY-3060223-49.patch3.99 KBcharginghawk
#46 corrupted_data_paragraphs-3060223-46.patch775 bytesjastraat
#43 entity_clone_corrupted_data_on_cloning_nodes_paragraphs_cloned_paragraph_3060223-42.patch2.06 KBlamliheussama
#42 entity_clone_corrupted_data_on_cloning_nodes_paragraphs_cloned_paragraph_3060223-42.patch1.74 KBlamliheussama
#35 entity_clone-corrupted-paragraph-cloning-3060223-35.patch7.4 KBjasonawant
#35 entity_clone-corrupted-paragraph-cloning-TEST-ONLY-3060223-35.patch3.6 KBjasonawant
#32 entity_clone-corrupted-paragraph-cloning-3060223-32.patch7.28 KBjasonawant
#32 entity_clone-corrupted-paragraph-cloning-TEST-ONLY-3060223-32.patch3.48 KBjasonawant
#28 entity_clone-corrupted-paragraph-cloning-3060223-28.patch7.58 KBjasonawant
#28 entity_clone-corrupted-paragraph-cloning-TEST-ONLY-3060223-28.patch3.79 KBjasonawant
#24 interdiff_15-24.txt3.51 KBjasonawant
#24 entity_clone-corrupted-paragraph-cloning-TEST-ONLY-3060223-5-24.patch2.63 KBjasonawant
#15 interdiff_6-15.txt2.2 KBngkoutsaik
#15 add_test-3060223-15.patch6 KBngkoutsaik
#6 entity_clone-corrupted-paragraph-cloning-3060223-5.patch3.75 KBaudacus
#4 entity_clone-corrupted-paragraph-cloning-3060223-4.patch3.72 KBdaveferrara1
#2 entity_clone-corrupted-paragraph-cloning-3060223-2.patch857 bytesdpavon
cloned.png205.57 KBainarend
source.png241.39 KBainarend
Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

ainarend created an issue. See original summary.

dpavon’s picture

Hi, I found that saving the paragraph along with "$duplicate" copy of the entity meant saving the old target_id paragraph for the new node, and then overwriting its parent_id, and triggering this issue.

In order to avoid that, I'd suggest something similar to this patch where we just save it once, avoiding side effect problem mentioned in https://www.drupal.org/project/entity_clone/issues/3030806#comment-13047353, whereas keeping the fix to avoid any circular dependency, already applied by https://www.drupal.org/node/3030806.

Any suggestions are welcomed, though, since I've tested it manually and automatically but I would like to confirm we can avoid saving before the "foreach" piece of code.

daveferrara1’s picture

Priority: Normal » Major

I can reproduce. Having the paragraphs go missing on the original node after unpublishing the clone warrants at least Major. Resaving the original node, restores the parent_id back to its correct nid and the missing content is restored.

Patch worked for me in testing. I am wondering why that save in the patch before doing work was necessary.

daveferrara1’s picture

Here is an updated patch. This removes the extra save but still sets a created and changed date to request time.

idebr’s picture

Status: Active » Needs review
audacus’s picture

I had to add an additional saving of the original entity to get the patch working properly.

hudri’s picture

Priority: Major » Critical

Bumping to criticial, in our project we are extensivly using (nested) Paragraphs, and this bug causes errors left and right. The "hidden" problem is fairly simple, reproduceable and repairable. But infrequently we are also getting "Non-translatable fields can only be changed when updating the current revision." on cloned nodes (without any translation!), resulting in completely broken nodes that can't be edited anymore (re-saving the original node as suggested in #3 didn't help in this case).

Update, this is definitely critical: While doing tests with the node described above, deleting the cloned node also deleted the paragraphs on the original node.

This bugs causes unwanted, unrecoverable data loss.

hudri’s picture

I've written a simple module that provides a drush command to fix the parent id of cloned entities:

fix_cloned_paragraphs.info.yml

name: Fix cloned Paragraphs
type: module
description: Fixes corrupt database entries for Paragraphs module caused by Entity clone module
package: Paragraphs
core: 8.x
dependencies:
  - paragraphs
  - entity_clone

drush.services.yml

services:
  fix_cloned_paragraphs.commands:
    class: \Drupal\fix_cloned_paragraphs\Commands\FixClonedParagraphsCommands
    arguments: ['@database']
    tags:
      - { name: drush.command }

src/Commands/FixClonedParagraphsCommands.php


namespace Drupal\fix_cloned_paragraphs\Commands;

use Drush\Commands\DrushCommands;
use Drupal\Core\Database\Connection;

/**
 * A Drush commandfile.
 *
 * In addition to this file, you need a drush.services.yml
 * in root of your module, and a composer.json file that provides the name
 * of the services file to use.
 *
 * See these files for an example of injecting Drupal services:
 *   - http://cgit.drupalcode.org/devel/tree/src/Commands/DevelCommands.php
 *   - http://cgit.drupalcode.org/devel/tree/drush.services.yml
 */
class FixClonedParagraphsCommands extends DrushCommands {

  /**
   * @var $database \Drupal\Core\Database\Connection;
   */
  protected $database;

  public function __construct(Connection $database) {
    parent::__construct();
    $this->database = $database;
  }


  /**
   * Fix paragraphs having corrupted parent_ids due cloning with entity_clone module
   *
   * @command fix_cloned_paragraphs:fix
   * @aliases fcp
   */
  public function fix() {
    $this->io()->text("Looking for paragraphs with corrupt parent_ids ...");
    $preQuery = $this->database->select('paragraphs_item_field_data', 'p');
    $preQuery->addField('p', 'parent_type', 'parent_type');
    $preQuery->addField('p', 'parent_field_name', 'parent_field_name');
    $preResult = $preQuery->distinct()->execute()->fetchAll();

    $updatesTotal = 0;
    $updatesPerHost = 0;

    foreach ($preResult as $parent) {
      $this->io()->text("Checking {$parent->parent_type}.{$parent->parent_field_name} ...");
      $updatesPerHost = 0;

      $query = $this->database->select("{$parent->parent_type}__{$parent->parent_field_name}", 'parent');
      $query->join('paragraphs_item_field_data', 'paragraph', "parent.{$parent->parent_field_name}_target_id = paragraph.id AND parent.{$parent->parent_field_name}_target_revision_id = paragraph.revision_id AND parent.entity_id <> paragraph.parent_id");
      $query->addField('parent', 'entity_id', 'host_id');
      $query->addField('parent', "{$parent->parent_field_name}_target_id", 'target_id');
      $query->addField('parent', "{$parent->parent_field_name}_target_revision_id", 'rev_id');
      $hosts = $query->execute();

      foreach ($hosts as $host) {
        $numUpdates = $this->database->update('paragraphs_item_field_data')
          ->condition('id', $host->target_id)
          ->condition('revision_id', $host->rev_id)
          ->condition('parent_id', $host->host_id, '<>')
          ->condition('parent_type', $parent->parent_type)
          ->condition('parent_field_name', $parent->parent_field_name)
          ->fields(['parent_id' => $host->host_id])
          ->execute();
        $updatesPerHost += $numUpdates;
        $updatesTotal += $numUpdates;
      }
      if ($updatesPerHost > 0 ) {
        $this->io()->note("{$parent->parent_type}.{$parent->parent_field_name}: Updated $updatesPerHost paragraphs");
      }
    }
    if ($updatesPerHost > 0 ) {
      $this->io()->note("Updated $updatesTotal paragraphs in total");
    }
    else {
      $this->io()->text("Did not update any paragraphs.");
    }

    $this->io()->text('Doing sanity checks ...');
    $totalFailed = 0;
    foreach ($preResult as $parent) {
      $query = $this->database->select("{$parent->parent_type}__{$parent->parent_field_name}", 'parent');
      $query->join('paragraphs_item_field_data', 'paragraph', "parent.{$parent->parent_field_name}_target_id = paragraph.id AND parent.{$parent->parent_field_name}_target_revision_id = paragraph.revision_id AND parent.entity_id <> paragraph.parent_id");
      $query->addField('parent', 'entity_id', 'host_id');
      $query->addField('parent', "{$parent->parent_field_name}_target_id", 'target_id');
      $query->addField('parent', "{$parent->parent_field_name}_target_revision_id", 'rev_id');
      $hosts = $query->execute();
      $numFailed = $query->countQuery()->execute()->fetchField();
      $totalFailed += $numFailed;

      if ($numFailed > 0) {
        $this->io()->error("{$parent->parent_type}.{$parent->parent_field_name}: Still $numFailed corrupt paragraphs, failed fixing this field");
      }
    }

    if ($totalFailed === 0) {
      $this->io()->success("No corrupt paragraphs found.");
    }
  }
}

hudri’s picture

I've further examinded my bug report from #7, it seems this bug is not connected to this issue: In addition to the incorrent parent_id, those paragraphs have not been cloned at all, two different host nodes used the same paragraph entity id, but different paragraph revision ids.

cobadger’s picture

Status: Needs review » Reviewed & tested by the community

I was able to reproduce the problem - the value of paragraphs_item_field_data.parent_id in the source node was changed to the node id of the cloned node.

I found that the patch in comment #6 works - after applying it, the value of paragraphs_item_field_data.parent_id in the source node was *not* changed to the nid of the cloned node.

mpp’s picture

Issue summary: View changes
acbramley’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

This absolutely needs test coverage.

mahipal46’s picture

Review and applied patch #6 and is working as expected. the value of paragraphs_item_field_data.parent_id in the source node was not changed to the nid of the cloned node.

ngkoutsaik’s picture

Assigned: Unassigned » ngkoutsaik
ngkoutsaik’s picture

Status: Needs work » Needs review
StatusFileSize
new6 KB
new2.2 KB

I added a test that clones a node of type page and its' referenced paragraph. Then check if the parent ID of the first paragraph remains intact.

ngkoutsaik’s picture

Assigned: ngkoutsaik » Unassigned

Status: Needs review » Needs work

The last submitted patch, 15: add_test-3060223-15.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

kbrodej’s picture

Hi. Reviewed the patch. Code looks okay. Run the test and they passed localy when I had paragraphs required as composer dependency. After I removed it they failed the same. It looks like a missing dependency.

rick hood’s picture

I believe I have this issue also.

1. Create node 1 with paragraphs,
2. Clone node 1, creating node 2.
3. Now paragraphs in node 1 disappear, but only when not logged-in. Logged-in users see the paragraphs. Note that node 2 is not yet published, if that makes a difference (wondering if that is why it is an issue only for logged-out users).
4. Edit node 1 and save it, and now the paragraphs are visible to all on node 1.

Question: is this a permanent fix? To edit node 1 and save?

I will probably ask this client not to use entity clone for now, but wondering what the fix is for nodes already created?

UPDATE: I have more info from our client: when cloning nodes with paragraphs, the paragraphs were not cloned (checkboxes not checked). This sounds bad as the instructions at the top of the clone page say this:

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:
If you have a Paragraph field in your entity, and you choose not to clone it here, deleting the original or cloned entity will also delete the Paragraph field from the other one. So you probably want to clone Paragraph fields.
However, if you have a User reference field, you probably don't want to clone it here because a new User will be created for referencing by the clone.

UPDATE (again):
More info from my testing, steps to reproduce:

A. Clone* a node with paragraphs - both the original node and the clone are fine (logged in and logged out)
* with none of the checkboxes selected so none of paras are being cloned I believe
B. Un-publish the clone - now the original node is missing the paragraphs, but only when logged out.
I made cache was cleared to see if that made a difference

This fixes the issue:
1. Edit the original node and simply save it again
2. Re-publish the clone
We have been using #1 as a fix so far, but I am not sure it is really a fix.

I also tried deleting the clone instead of just un-publishing. That cause no problems, which makes no sense at all.

Any advice on this would be appreciated. Thank you.

pietermouton’s picture

Same issue over here. Following this issue!

rick hood’s picture

I have done more testing on my issue (#19 above).

I do not encounter the issue if I clone all the paragraphs when I clone the node.

When cloning you are shown a set of checkboxes to select referenced entities. In my case I had one reference taxonomy, which I did NOT clone, as those should not get duplicated. But I DID clone all the paragraphs.

STEPS:

1. Clone Node A to create Node B. Select all paragraphs during the clone.

2. Unpublish Node B. Node A is still fine.

Previously, Node A would lose it's paragraphs if I unpublished Node B (the clone).

Without knowing much about how this module works, it seems kind of obvious that one would need to clone the paragraphs so as not to "share" * paragraphs between nodes.
(* I am not sure that previous clones (where paras were not cloned) we in fact "sharing " paras because a change to a para in one node did not show up on the other node. )

Is that correct? Is this the answer to the issue?

mogio_hh’s picture

I still destroy the source node when cloning it after the cron ran. This seems to be a bigger issue than expected. 1 1/2 years should be enough to have a stable patch for being able to clone nodes with paragraphs. I don't even know a site that does not use paragraphs these days. The whole thing of cloning becomes kind of dangerous and useless when using the entity_clone module as. Is Replicate not able to handle nodes with entities? Has anybody tried?

hudri’s picture

Yes, Replicate works with Paragraphs, but also keep in mind that Replicate is aimed at developers. Replicate (respectively ReplicateUI) only has very minimal UI settings (basically "allow clone yes/no", which builds on top of the "create entity" permission).

Replicate is rock solid, but if you want recursive cloning or timestamp updates on cloning, you have to code your own ReplicateEventSubscriber.

Paragraphs work out of the box though, because Replicate support is part of Paragraphs module itself. But e.g. there is no file entity cloning by default in Replicate.

jasonawant’s picture

Status: Needs work » Needs review
StatusFileSize
new2.63 KB
new3.51 KB

Experiencing this issue too on a project, but I'm not yet able to reproduce this issue using the paragraphs_demo module's content.

Many are reporting this issue, but it's still unclear what version and with what setup is causing the issue. Has anyone been able to reproduce it using the paragraphs_demo module?

Though, I should say the fix presented here does appear to resolve the issue.

Regarding comment #18, the test runner does not know that it needs paragraphs module and its dependencies. I think this can be solved by adding a composer.json file and declaring paragraphs as a dev dependency.

The test included in #15, passes with our without the proposed fixes. Here's a patch with the test only and a composer.json file to include paragraphs. Let's see if the test passes here.

If this patch/test passes, then I think we need to first working on a test that fails without the fix.

acbramley’s picture

Status: Needs review » Needs work
  1. +++ b/composer.json
    @@ -0,0 +1,10 @@
    +    "require-dev": {
    +        "drupal/paragraphs": "^1.0"
    +    }
    

    We can use test_dependencies in info.yml instead of a composer.json.

  2. +++ b/tests/src/Functional/EntityCloneContentTest.php
    @@ -93,4 +102,44 @@ class EntityCloneContentTest extends NodeTestBase {
    +    $this->assertEqual($paragraph->getParentEntity()->id(), $node->id());
    

    I think you'd need to re-load the paragraph/node here to get a fail.

jasonawant’s picture

Thanks for the feedback; I wasn't familiar with test_dependencies!

Re-loading entities did not fail the test. I tried to reset entity cache, and then to did not fail the test.

jasonawant’s picture

Assigned: Unassigned » jasonawant

I think I've reproduced it using paragraphs_demo module. I'm going to work on a new test, self-assigning for now.

jasonawant’s picture

Assigned: jasonawant » Unassigned
Status: Needs work » Needs review
StatusFileSize
new3.79 KB
new7.58 KB

Here's a test only patch and patch with the fix from #6. The test is now in a separate file. I did not add an interdiff b/c I didn't think it would be useful here. The fix is the same.

This test relies on the paragraphs_demo module completely. This module does a lot for us; it adds translations and content moderation. It's possible to add additional tests for other possible edge cases related to those.

The test from #15 used EntityReferenceTestTrait::createEntityReferenceField(), which may have been the issue b/c paragraphs do not use entity reference, instead it uses entity reference revision instead.

Adding paragraphs as a test dependency for the entire module seems like a lot of overhead. paragraphs_demo itself requires even more dependencies. We may want to consider splitting off this test and its dependencies somehow, but I'm new to testing, so unsure what that looks like. And, entity_clone may simply not want to rely on paragraphs at all.

The last submitted patch, 28: entity_clone-corrupted-paragraph-cloning-TEST-ONLY-3060223-28.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Status: Needs review » Needs work

The last submitted patch, 28: entity_clone-corrupted-paragraph-cloning-3060223-28.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

jasonawant’s picture

Assigned: Unassigned » jasonawant

Looks like missing dependencies b/c of paragraphs_demo usage. I'll try this again. I'm going to omit the .info changes to see if test runner will fallback to using composer.json's require-dev. Paragraphs uses composer.json to declare dev dependencies here.

jasonawant’s picture

trying again with only composer.json to see if dependencies are managed correctly.

The last submitted patch, 32: entity_clone-corrupted-paragraph-cloning-TEST-ONLY-3060223-32.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Status: Needs review » Needs work

The last submitted patch, 32: entity_clone-corrupted-paragraph-cloning-3060223-32.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

jasonawant’s picture

Looks like test runner isn't installing a dependency's require-dev dependencies.

From https://www.drupal.org/pift-ci-job/1912818

Unable to install modules: module 'paragraphs_demo' is missing its dependency module search_api_db

Though paragraphs and entity reference revisions is from https://dispatcher.drupalci.org/job/drupal8_contrib_patches/56362/console

19:16:30 - Downloading drupal/entity_reference_revisions (1.8.0)
19:16:30 - Downloading drupal/paragraphs (1.12.0)
19:16:33 - Installing drupal/entity_reference_revisions (1.8.0): Extracting archive
19:16:33 - Installing drupal/paragraphs (1.12.0): Extracting archive

Trying again by declaring paragraphs_demo dependencies as require-dev.

The last submitted patch, 35: entity_clone-corrupted-paragraph-cloning-TEST-ONLY-3060223-35.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

barone’s picture

@jasonwant i just tested patch #35 and can confirm that the Issue still persists. I'm using a stack with Drupal 8.9.11, Paragraphs 8.x-1.12 and Layout Builder 8.9.10 (I think LB is relevant here because of a related issue: https://www.drupal.org/project/entity_clone/issues/3050027)

jasonawant’s picture

Hi @barone, thanks for reporting and pointing out the related issue, #3050027: Inline Blocks on Layout Builder Fail to Clone Correctly.

You said that "the Issue still persist". Are you referring to this issue or another issue related to layout builder?

barone’s picture

Jason, i was referring to this tread but i'm starting to think they are the same. I'm gonna run some local tests with and without paragraphs and post the results in both threads.

colan’s picture

Status: Needs review » Needs work

Thanks for digging into it.

jasonawant’s picture

Status: Needs work » Needs review

@barone, were able to review your particular issue. To me, the two issues are different; particularly b/c #3050027-24: Inline Blocks on Layout Builder Fail to Clone Correctly points to an issue with Entity Reference Revision module, #3190915: New revision is not created via Layout Builder.

Marking as Needs Review to help get attention on the provided patches.

lamliheussama’s picture

in my project the paragraphes of cloned node crushed the paragraphes of original language from source cloned node wen i translate it or translate cloned node, i have patch ContentEntityCloneBase class to create a new paragraphes for cloned node.

lamliheussama’s picture

dinazaur’s picture

Hello, Wanted to adjust the patch to the latest version of the module (beta6) and discovered that they already added the ability to set created time of a cloned entity. So it seems that the bug already fixed, tested on the latest @beta6 version.

jastraat’s picture

Version: 8.x-1.0-beta3 » 8.x-1.0-beta6

Unfortunately the patch in #43 creates a dependency on the paragraph module which I imagine is not desirable.

I can confirm that the original issue still exists in the latest beta, but none of the patches apply anymore.

In our case, we have nested paragraphs (e.g. 'sections' field on node page that contains 'columns' paragraphs that can in turn contain 'text' or 'image' paragraphs) and this may or may not be related to the issue.

To be really specific, the entity_id (node ID) in the node__field_[fieldname] table is correct for both the original node and the clone.

The parent_id in paragraphs_item_revision_field_data is correct for the child paragraphs. They correctly reference the new paragraphs created when cloning the parent node.

What is incorrect are the parent_ids for the top level paragraphs in paragraphs_item_revision_field_data. For some reason, the clone operation is updating the original paragraphs on the original node to now use the NEW cloned node as the parent. This results in all paragraphs associated with both the original and cloned nodes referencing the clone as the parent.

I don't understand why any of the fields or entities associated with the original node are being edited at all.

jastraat’s picture

StatusFileSize
new775 bytes

I believe most of the problem is fixed in beta6, but there was an extra entity save call in the cloneEntity function that was causing the incorrect setting of parent id on paragraph entities. When the extra save call is removed, cloning proceeds as expected.

Patch attached -

acbramley’s picture

Status: Needs review » Needs work

Nice work! It'd be good to pull the tests from #24 (and fixing them with suggestions from #25) and post a red/green patch combo.

yfma’s picture

Tested #46 patch with the latest @beta6 version, working as expected from our end.

charginghawk’s picture

Combined the tests from #35 with the patch from #46. Re: #25's comment:

We can use test_dependencies in info.yml instead of a composer.json.

You actually can't unless you commit the info.yml first. I've included both composer.json and info.yml updates, and we can remove the composer.json dependencies post-commit.

Since the fix prevents new corrupted paragraphs but doesn't address existing ones, I'm also uploading a patch with an update hook based on the drush command from #8. If anybody is happening upon this issue for the first time, be sure to fix existing paragraphs as well.

The last submitted patch, 49: corrupted_data_paragraphs-TEST-ONLY-3060223-49.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

artem_sylchuk’s picture

Latest dev + #49 worked perfectly for me

rade’s picture

Can confirm, patch #49 applies when using dev version, but not when using beta6. I suspect this is due to the info.yml file being modified by both the patch and the Drupal packaging system.

charginghawk’s picture

Status: Needs review » Reviewed & tested by the community
colan’s picture

Status: Reviewed & tested by the community » Postponed (maintainer needs more info)

I want to get this in & cut a release, but I need more guidance. Could I get a list of steps to clean this up first, a release plan? Which patch do I commit? Both? In what order? What else needs to be done?

Something like:

  1. Commit patch 1.
  2. Do something else.
  3. Commit patch 2.
  4. Cut a release.

Thanks!

charginghawk’s picture

@colan We can disregard the patch with the update hook for now, it hasn't been as thoroughly tested and if anybody wants it it's here.

So, I believe...

1. Commit this patch: https://www.drupal.org/files/issues/2021-09-14/corrupted_data_paragraphs...
2. Add a new patch removing composer.json to confirm that test_dependencies in info.yml work on their own. If successful, commit.
3. Cut a release.

I'm not sure why we wouldn't want a composer.json but I defer to prior requests to not have one. In any case, we wouldn't want the modules under require-dev.

  • colan committed 1650ec8 on 8.x-1.x authored by jasonawant
    Issue #3060223 by jasonawant, charginghawk, lamliheUssama, ngkoutsaik,...
colan’s picture

Status: Postponed (maintainer needs more info) » Needs review
StatusFileSize
new936 bytes

Thanks! Here's the patch.

colan’s picture

Status: Needs review » Needs work

The dev branch is failing now; see test results.

charginghawk’s picture

Status: Needs work » Needs review
StatusFileSize
new451 bytes

The #57 error looks like it doesn't like losing composer.json completely. The dev branch error seems related to search_api, maybe it's running its tests on top of ours? Let's try just removing the require-dev and see if that gets us back on track.

charginghawk’s picture

Issue tags: -Needs tests
StatusFileSize
new546 bytes

Fix trailing comma.

Status: Needs review » Needs work

The last submitted patch, 60: entity_clone-fix_corerupted_data-3060223-60.patch, failed testing. View results

charginghawk’s picture

Status: Needs work » Needs review
StatusFileSize
new912 bytes

Alright, I think we got it. We need paragraphs_demo, so instead of paragraphs:paragraphs in test_dependencies, we need paragraphs:paragraphs_demo. But because this is a test_dependencies change, it will probably need to be committed in order for the changes to take effect. Unfortunate that we have to commit it to test, but I'm fairly confident this will get us there.

Status: Needs review » Needs work

The last submitted patch, 62: entity_clone-fix_corerupted_data-3060223-61.patch, failed testing. View results

n.ghunaim’s picture

StatusFileSize
new8.82 KB

Re-rolling the patch in comment #49 to work with the latest version of beta6.

sun-fire’s picture

Hi!

With the re-rolled patch from #49, it works fine for cloning nodes with paragraphs but still faces the problem when trying to clone nodes with translations (using asymmetric translation flow), so trying to prepare some patch that will cover this solution as well.

ajv009’s picture

Is the code included in all the above patches listed to update existing messed up nodes, working?
Because its not working for me! And this is a very important issue, around 200-300 pages of content is already at risk on my site and around 50-70 pages are already broken!

Example.

node/643 cloned to node/644
node/643 content changed
node/644 unpublished
certain paragraphs from node/643 just vanishes! (only after node/644 was unpublished)

Few observations from @guptahemant about this issue. (he actually found the issue on our site, Thanks @guptahemant for figuring this out.)

Investigation:

- Took the backup from production and setup the site on local.
- Started with node 653 and observed that it was rendering for admin but not for anonymous users.
- Used below code to get the rendered output and observed the following:

$nid = 653;
// Result with root user.
$root_user = \Drupal::entityTypeManager()->getStorage('user')->load(1);
$accountSwitcher = \Drupal::service('account_switcher');
$accountSwitcher->switchTo($root_user);
\Drupal::entityTypeManager()->getStorage('node')->load($nid)->field_sections->view();
$accountSwitcher->switchBack();

// Result without root user.
\Drupal::entityTypeManager()->getStorage('node')->load($nid)->field_sections->view();

// As admin
=> [
     "#theme" => "field",
     "#title" => "Sections",
     "#label_display" => "above",
     "#view_mode" => "_custom",
     "#language" => "en",
     "#field_name" => "field_sections",
     "#field_type" => "entity_reference_revisions",
     "#field_translatable" => true,
     "#entity_type" => "node",
     "#bundle" => "case__study",
     "#object" => Drupal\node\Entity\Node {#7154
       +in_preview: null,
     },
     "#items" => Drupal\entity_reference_revisions\EntityReferenceRevisionsFieldItemList {#7158},
     "#formatter" => "entity_reference_revisions_entity_view",
     "#is_multiple" => true,
     "#third_party_settings" => [],
     0 => [
       "#paragraph" => Drupal\paragraphs\Entity\Paragraph {#7183},
       "#view_mode" => "default",
       "#cache" => [
         "tags" => [
           "config:paragraphs.settings",
           "paragraph:3244",
           "paragraph_view",
         ],
         "contexts" => [
           "route.name.is_layout_builder_ui",
           "user.permissions",
         ],
         "max-age" => -1,
       ],
       "#theme" => "paragraph",
       "#weight" => 0,
       "#pre_render" => [
         [
           Drupal\paragraphs\ParagraphViewBuilder {#6934},
           "build",
         ],
       ],
     ],
     // Trimmed data----
     "#cache" => [
       "contexts" => [],
       "tags" => [],
       "max-age" => -1,
     ],
     "#weight" => 0,
   ]

As anonymous
=> [
     "#cache" => [
       "contexts" => [
         "user.node_grants:view",
         "user.permissions",
       ],
       "tags" => [
         "config:paragraphs.settings",
         "node:904",
       ],
       "max-age" => -1,
     ],
   ]

- Key point to observe in the anonymous was that in metadata node:904 is referenced which is not associated with the loaded node 653.
- When further investigated the status of node 904 we found that the node is unpublished and its content was somewhat similar of what we have in node 653.
- on further investigation and confirmation from team we found that pages are being cloned multiple times,
- Then we ran a small experiment by creating a new clone of node 656 (which is rendering correctly), while cloning we selected clone button directly with using any sub cloning options.
- The resulting node id was 910.
- When we unpublished this node we observed that the node 656 content stopped rendering and hence we were able to reproduce our issue.

Cause of the issue
- When we use entity clone to clone the node it doesn’t clones the paragraph entities i.e same paragraph is attached to multiple nodes.
- This also results in parent_id getting changed in paragraphs_item table to the new entity.
- Due to this in-consistency the content cannot render correctly on unpublishing the cloned node.

Conclusion
- We need to stop using the clone feature for now.
- A high amount of content is impacted with this issue on production.
- Sample query to get affected nodes for one particular paragraph

SELECT id, parent_id FROM `paragraphs_item_revision_field_data` WHERE id = 3244 GROUP by parent_id;

'652','653','655','656','657','658','659','660','661','664','665','666','672','796','798','805','811','858','867','871','892','904','905','906','910,

- Entity clone module does not work correctly for cloning paragraphs in our case

these 652, 652, 655, etc... will all be affected if the status of the paragraph 3244 in anyone of them is changed. I hope that explains how dangerous this is.

A rough script to list the messed up paragraphs and nodes with there current parents for quick testing and debugging -> https://gist.github.com/AJV009/a0f06885a2a8db632be18e99b753b67c

charginghawk’s picture

@AJV009 The update hook was NOT committed, but you can find it here:

https://www.drupal.org/files/issues/2021-09-14/corrupted_data_paragraphs...

And copy it to your site. It's not committed because, per above, "it hasn't been as thoroughly tested and if anybody wants it it's here." If it works for you, please let us know and maybe we can merge the update hook as well.

A code fix was merged into a release branch to resolve this issue moving forward, but that hasn't been packaged into a proper release. @colan Any chance we can get https://www.drupal.org/files/issues/2021-12-07/entity_clone-fix_corerupt... merged into to resolve the failing tests?

tim-diels’s picture

What is the status of this issue? What still needs to be done?

charginghawk’s picture

@tim-diels A fix has been merged into the development branch:

https://www.drupal.org/commitlog/commit/85325/1650ec83100613525a798bf723...

But it caused tests to fail because of a dependency issue, we install paragraphs when we need to install paragraphs_demo. It's kind of dumb, but test_dependencies issues can't pass tests in patch form, they need to be merged.

"the change to the info.yml file must be committed to the git repository before the test run starts, in order for the test runner to discover the dependency."

https://www.drupal.org/docs/develop/using-composer/managing-dependencies...

Once we merge this patch then the test should pass:

https://www.drupal.org/files/issues/2021-12-07/entity_clone-fix_corerupt...

Then we should cut a proper release.

charginghawk’s picture

Status: Needs work » Needs review

charginghawk’s picture

Status: Needs review » Fixed

I actually think this is fixed. Tests are passing for the dev branch now, which was the original blocker after the fix was merged.

Using the new Gitlab integration I can see that my ideas for using test_dependencies instead of composer.json require-dev don't work and it's probably not worth the time / not in scope for this ticket to fix.

The only remaining issue is that we don't have anything to fix existing corrupted paragraphs, because the update hook was never RBTC. IMO that should be it's own ticket.

charginghawk’s picture

Here's the ticket for the update hook:

https://www.drupal.org/project/entity_clone/issues/3320509

Status: Fixed » Closed (fixed)

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