Revision user incorrectly appears as anonymous user when node author is canceled

It's be awesome to get a test to recreate this bug, we can then fix in a test driven development way.

Confirming this issue exists.

When one User who has created a revision to a node is deleted, all revisions to that node are attributed to Anonymous - regardless of who created them.

Reworked \node_user_cancel() to reassign authors and revision author independently.

API Additions

• Added new \Drupal\node\NodeStorageInterface::userRevisionAuthorRevisionIds API to get revisions created by a user.

Tests

• Added tests for \Drupal\node\NodeStorageInterface::userRevisionIds and \Drupal\node\NodeStorageInterface::userRevisionAuthorRevisionIds. I could not find any coverage for the pre-existing userRevisionIds method.
• Created new test class in node.module specifically for testing user cancellation + node.
• Moved existing coverage from user.module to here.
• Added coverage for testing both author and revision author.

Side note: exhaustive ->isPublished check was added in the original user cancellation feature back from this patch #8-122: Let users cancel their accounts. I cant say if accidentally unpublishing revisions was ever an issue.

Also tests only.

Only the new tests should fail.

Contains new tests only (without the delete/move of existing tests).

Mostly only style/nits, patch looks great! Nice work. Leaving NR for others, but will RTBC if there isn't any more feedback.

1. +++ b/core/modules/node/tests/src/Kernel/NodeStorageTest.php
   @@ -0,0 +1,103 @@
   +      'type' => strtolower($this->randomMachineName()),
   +      'name' => $this->randomMachineName(),
   ...
   +      'title' => $this->randomMachineName(),
   ...
   +    $owner = User::create(['name' => $this->randomMachineName()]);
   

   I don't think using random IDs and labels adds any value to the test here and personally I think it dilutes the test a bit, harder to parse and doesn't really add extra coverage since the labels and node type ID isn't really being asserted anywhere.

2. +++ b/core/modules/node/tests/src/Kernel/NodeStorageTest.php
   @@ -0,0 +1,103 @@
   +  /**
   +   * Test getting revisions created by a user.
   +   *
   +   * @covers ::userRevisionAuthorRevisionIds
   +   */
   

   Great test, nice one.

3. +++ b/core/modules/node/tests/src/Kernel/NodeStorageTest.php
   @@ -0,0 +1,103 @@
   +    $nodeType = NodeType::create([
   +      'type' => strtolower($this->randomMachineName()),
   +      'name' => $this->randomMachineName(),
   +      'revision' => TRUE,
   +    ]);
   +    $nodeType->save();
   

   Can be moved to ::setup and removed from the other test case too.

4. +++ b/core/modules/user/tests/src/Functional/UserCancelTest.php
   @@ -334,17 +334,6 @@ class UserCancelTest extends BrowserTestBase {
   -    // Confirm that user's content has been attributed to anonymous user.
   -    $anonymous_user = User::getAnonymousUser();
   -    $node_storage->resetCache([$node->id()]);
   -    $test_node = $node_storage->load($node->id());
   -    $this->assertTrue(($test_node->getOwnerId() == 0 && $test_node->isPublished()), 'Node of the user has been attributed to anonymous user.');
   -    $test_node = node_revision_load($revision, TRUE);
   -    $this->assertTrue(($test_node->getRevisionUser()->id() == 0 && $test_node->isPublished()), 'Node revision of the user has been attributed to anonymous user.');
   -    $node_storage->resetCache([$revision_node->id()]);
   -    $test_node = $node_storage->load($revision_node->id());
   -    $this->assertTrue(($test_node->getOwnerId() != 0 && $test_node->isPublished()), "Current revision of the user's node was not attributed to anonymous user.");
   

   I would have said you'll probably get push back moving code from a functional test into an integration test, but all of this is using entity API in the same way the kernel test is, so I think it's a good clean up and should be accepted. Nice one.

Thanks @Sam152,

Addressed feedback removing randomness and moving common node/type creation to setup.

Also fixed test, removed now dead code from tests, and added back misplaced anonymous-user variable.

+++ b/core/modules/node/src/NodeStorageInterface.php
@@ -23,7 +23,7 @@ interface NodeStorageInterface extends ContentEntityStorageInterface {
-   * Gets a list of revision IDs having a given user as node author.
+   * Gets a list of revision IDs where a user is the node author.

@@ -33,6 +33,17 @@ interface NodeStorageInterface extends ContentEntityStorageInterface {
+   * Gets a list of revision IDs having a given user as revision author.

Why did the usage of 'having' get removed here, but then used in the new docblock?

Otherwise, changes LGTM.

Addressed #13, thanks!

Reverted inconsistent comment.

A similar issue is that when a user account is deleted its UID can still appear in node_revision table in revision_uid column.

Rerolling patch for latest 9.1.

When I tested the patch, I found that, after cancelling the user, the nodes that had at least one revision made by that user ended up with one of those revisions becoming the default ("published") version of the node, whether or not there was a newer default revision. Looking at the database, this seems to happen because the older revisions aren't actually altered, but instead new revisions are created with the change in author to anonymous. These new revisions then are seen as the latest revision.

Fixed failed tests of patch #20.

Fixing test.

Fixing test.

Applied patch #27 and it works fine.Adding screenshots below.

Before patch:

After patch (Only name of deleted user is set as anonymous now):

Hi all -

I am also seeing the issue where an old revision gets published noted in comment #20. I'm seeing this issue on an existing 8.9.x site (with or without the most recent patch that applies), but I did not see it on a minimal D9 install, but I haven't had time to look into it further (and I think it probably needs its own issue). As a temporary workaround, we are using this simple module to remove the option altogether from the User Cancellation Form (/user/{user}/cancel) and the Bulk User Cancellation Form (/admin/people/cancel): Prevent User Delete, Reassign Content to Anonymous

We've run into the same issue. I had to backport the patch in 27 to 9.1.x but this solved our issue. Thanks for posting this!

Reroll of #27.

In testing #33 I'm still seeing problematic behavior. I see new revisions created based on the old revisions that belonged to the user who was deleted. They aren't actually getting published in my setup, but they do become the latest revision. That said, I'm testing this on a site with quite a bit of custom code that modifies how revisions behave so this shouldn't be taken as the final word.

I'm really wondering if we want to actually do real entity revision saves as part of this. It's very disruptive. In my case, at least, I have code that responds to what it perceives as transitions by doing things like sending emails and resaving each of the revisions the way this code does effectively replays through the transitions since each successive revision save causes the new revision to be considered the 'latest revision'. I know it's always a debate to determine what sorts of changes warrant going through the API to fire off all associated hooks and events but I think I might argue for doing these changes directly in the DB.

I know that maybe I just need to implement it myself, but I tend to think most people want this to behave, as much as possible, as though nothing at all changed...they just want to swap in the anonymous user in place of the user being deleted. I tend to think doing the swap directly in the appropriate DB tables (and probably firing off some commands to expire caches) is going to be the best way to do that and not have a bunch of inadvertent side-effects.

Re-rolled #33 for 9.4.

I'm working with a client who has seen this behaviour as well. The patch applies cleanly and seems to help with accounts as they are cancelled.

However they have already got content that has been reassigned incorrectly. Is there a way to update the existing content/revisions to resolve this? We have over a thousand revisions in the database assigned to a now-cancelled user ID and the customer can't save nodes.

Happy to try and pick at this a bit, but unfortuantely this client is still running on D8, so any input I can offer may be a bit outdated.

Followed the steps in the description and I was not able to reproduce. Are there different steps to follow?

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.

Retested and can confirm the issue.

At this time we will need a D10 version of the patch.

Elevated to major as this leads to data lose.

Patch for 10.1.x.

public function userRevisionAuthorRevisionIds(AccountInterface $account)
1. Should be typehinted as a new function

2. Both spots

Did not test yet.

Just trying to help you out here @smustgrave, the following patch address #44. I've also took a look at the testing part and it seems good to me.

Forgot to move to NR.

Thanks @murilohp just retested this and it's working as expected.

And change requested in #44 was addressed.

Random ckeditor5

Reran tests and they passed.

The patch itself looks fine, but I think we need to consider #35 a bit more - i.e. do we really want to be going through all the revisions that a user created a long time ago and resaving them? There's also #34 and #20 about this patch causing new revisions to be created in some situations - this might need more manual testing with draft + default revisions in history and similar.

That's a good point. I don't think Drupal.org does that as I see anonymous. I actually wonder if the data is being stored correctly and when the revision page loads since it can't load the user (it was deleted) it defaults to anonymous. Wonder if we could have just display as plain text?

I have found the root cause. Node revisions are getting created when save(); method is called in function _node_mass_update_helper.

The revisions are created with data from loaded node revision

The _node_mass_update_helper function should only update uid and revision_uid fields of the node revision entity instead of creating new revisions.

Solution: Disable creation of new revision in method _node_mass_update_helper when node revision is getting updated.

Please test my patch.

Can the MR be updated for 11.x or new one started as that’s the current development branch. Issue would be backported

Yes, let me add for 11.x as well

Switched branch to 11.x

Can issue summary be updated with new solution.

Also will need test coverage.

I found #3089747: Content moderation can wrongly set old revisions as default when they're resaved which asserts from several people the problem mentioned in #29 (and #20 I guess).

I added the data loss tag to this ticket since our node_field_data table was showing the UID as 0.

Same problem here on Drupal 9.5.10.

There are two different issues here.

• Issue 1: as described in this issue: the Revision user incorrectly appears as an anonymous user when the node author is canceled.
• Issue 2, referenced in comment this comment, is related to the duplication of old revisions and set as default.

The first case is related to what was pointed out in the task description and I think the proper approach is the previous patch. Therefore, I am recovering the this approach in the MR "2977362-incorrect-user-anonymous".

The second case, maybe is related to whether the Content Moderation module is active, which is being discussed in this issue https://www.drupal.org/project/drupal/issues/3089747 and there is a proposed solution.

Cancelled -> Canceled

Though both spellings are correct, core code uses "canceled" in every almost every case, so I'm changing the issue to use it as well.

Tests fixed
Tested on local. RTBC.
Please, take into account that most of the changes come from the user dpi, on the MR we applied the patch that he provided.

Adding PHPUnit tests to the issue https://www.drupal.org/project/drupal/issues/3089747 we found that:

• First user creates a content published
• Second user creates a translation over the same content published
• First user is canceled
• The translation is not accessible

We added the test to this branch to confirm that happens on a clear Drupal.

The problem seems that a new revision is created for en, but not for es, so the langcode es is not synchronised with the table field_data and then the langcode es is not accessible.

• Before deleting the user (no patch):

MySQL [drupal]> select * from test70491336node_field_data;
+-----+-----+-------------------+----------+--------+-----+----------------------------------+------------+------------+---------+--------+------------------+-------------------------------+----------------------------+------------------------------+
| nid | vid | type              | langcode | status | uid | title                            | created    | changed    | promote | sticky | default_langcode | revision_translation_affected | content_translation_source | content_translation_outdated |
+-----+-----+-------------------+----------+--------+-----+----------------------------------+------------+------------+---------+--------+------------------+-------------------------------+----------------------------+------------------------------+
|   1 |   2 | moderated_content | en       |      1 |   3 | First version of the content en. | 1696838795 | 1696838795 |       1 |      0 |                1 |                          NULL | und                        |                            0 |
|   1 |   2 | moderated_content | es       |      1 |   4 | First version of the content es. | 1696838798 | 1696838798 |       1 |      0 |                0 |                             1 | en                         |                            0 |
+-----+-----+-------------------+----------+--------+-----+----------------------------------+------------+------------+---------+--------+------------------+-------------------------------+----------------------------+------------------------------+
2 rows in set (0.000 sec)

MySQL [drupal]> select * from test70491336node_field_revision;
+-----+-----+----------+--------+-----+----------------------------------+------------+------------+---------+--------+------------------+-------------------------------+----------------------------+------------------------------+
| nid | vid | langcode | status | uid | title                            | created    | changed    | promote | sticky | default_langcode | revision_translation_affected | content_translation_source | content_translation_outdated |
+-----+-----+----------+--------+-----+----------------------------------+------------+------------+---------+--------+------------------+-------------------------------+----------------------------+------------------------------+
|   1 |   1 | en       |      1 |   3 | First version of the content en. | 1696838795 | 1696838795 |       1 |      0 |                1 |                             1 | und                        |                            0 |
|   1 |   2 | en       |      1 |   3 | First version of the content en. | 1696838795 | 1696838795 |       1 |      0 |                1 |                          NULL | und                        |                            0 |
|   1 |   2 | es       |      1 |   4 | First version of the content es. | 1696838798 | 1696838798 |       1 |      0 |                0 |                             1 | en                         |                            0 |
+-----+-----+----------+--------+-----+----------------------------------+------------+------------+---------+--------+------------------+-------------------------------+----------------------------+------------------------------+
3 rows in set (0.000 sec)

MySQL [drupal]> select * from test70491336node_revision;
+-----+-----+----------+--------------+--------------------+--------------+------------------+
| nid | vid | langcode | revision_uid | revision_timestamp | revision_log | revision_default |
+-----+-----+----------+--------------+--------------------+--------------+------------------+
|   1 |   1 | en       |            3 |         1696838795 | NULL         |                1 |
|   1 |   2 | en       |            4 |         1696838798 | NULL         |                1 |
+-----+-----+----------+--------------+--------------------+--------------+------------------+
2 rows in set (0.000 sec)

• After deleting the user (no patch):

MySQL [drupal]> select * from test70491336node_field_data;
+-----+-----+-------------------+----------+--------+-----+----------------------------------+------------+------------+---------+--------+------------------+-------------------------------+----------------------------+------------------------------+
| nid | vid | type              | langcode | status | uid | title                            | created    | changed    | promote | sticky | default_langcode | revision_translation_affected | content_translation_source | content_translation_outdated |
+-----+-----+-------------------+----------+--------+-----+----------------------------------+------------+------------+---------+--------+------------------+-------------------------------+----------------------------+------------------------------+
|   1 |   4 | moderated_content | en       |      1 |   0 | First version of the content en. | 1696838795 | 1696838887 |       1 |      0 |                1 |                             1 | und                        |                            0 |
|   1 |   4 | moderated_content | es       |      1 |   0 | First version of the content es. | 1696838798 | 1696838887 |       1 |      0 |                0 |                             1 | en                         |                            0 |
+-----+-----+-------------------+----------+--------+-----+----------------------------------+------------+------------+---------+--------+------------------+-------------------------------+----------------------------+------------------------------+
2 rows in set (0.000 sec)

MySQL [drupal]> select * from test70491336node_field_revision;
+-----+-----+----------+--------+-----+----------------------------------+------------+------------+---------+--------+------------------+-------------------------------+----------------------------+------------------------------+
| nid | vid | langcode | status | uid | title                            | created    | changed    | promote | sticky | default_langcode | revision_translation_affected | content_translation_source | content_translation_outdated |
+-----+-----+----------+--------+-----+----------------------------------+------------+------------+---------+--------+------------------+-------------------------------+----------------------------+------------------------------+
|   1 |   3 | en       |      1 |   0 | First version of the content en. | 1696838795 | 1696838887 |       1 |      0 |                1 |                             1 | und                        |                            0 |
|   1 |   4 | en       |      1 |   0 | First version of the content en. | 1696838795 | 1696838887 |       1 |      0 |                1 |                             1 | und                        |                            0 |
|   1 |   4 | es       |      1 |   0 | First version of the content es. | 1696838798 | 1696838887 |       1 |      0 |                0 |                             1 | en                         |                            0 |
+-----+-----+----------+--------+-----+----------------------------------+------------+------------+---------+--------+------------------+-------------------------------+----------------------------+------------------------------+
3 rows in set (0.000 sec)

MySQL [drupal]> select * from test70491336node_revision;
+-----+-----+----------+--------------+--------------------+--------------+------------------+
| nid | vid | langcode | revision_uid | revision_timestamp | revision_log | revision_default |
+-----+-----+----------+--------------+--------------------+--------------+------------------+
|   1 |   3 | en       |            0 |         1696838795 | NULL         |                1 |
|   1 |   4 | en       |            0 |         1696838798 | NULL         |                1 |
+-----+-----+----------+--------------+--------------------+--------------+------------------+

• Before delete the user (with patch):

MySQL [drupal]> select * from test76006253node_field_data;
+-----+-----+-------------------+----------+--------+-----+----------------------------------+------------+------------+---------+--------+------------------+-------------------------------+----------------------------+------------------------------+
| nid | vid | type              | langcode | status | uid | title                            | created    | changed    | promote | sticky | default_langcode | revision_translation_affected | content_translation_source | content_translation_outdated |
+-----+-----+-------------------+----------+--------+-----+----------------------------------+------------+------------+---------+--------+------------------+-------------------------------+----------------------------+------------------------------+
|   1 |   2 | moderated_content | en       |      1 |   3 | First version of the content en. | 1696839199 | 1696839199 |       1 |      0 |                1 |                          NULL | und                        |                            0 |
|   1 |   2 | moderated_content | es       |      1 |   4 | First version of the content es. | 1696839202 | 1696839202 |       1 |      0 |                0 |                             1 | en                         |                            0 |
+-----+-----+-------------------+----------+--------+-----+----------------------------------+------------+------------+---------+--------+------------------+-------------------------------+----------------------------+------------------------------+
2 rows in set (0.000 sec)

MySQL [drupal]> select * from test76006253node_field_revision;
+-----+-----+----------+--------+-----+----------------------------------+------------+------------+---------+--------+------------------+-------------------------------+----------------------------+------------------------------+
| nid | vid | langcode | status | uid | title                            | created    | changed    | promote | sticky | default_langcode | revision_translation_affected | content_translation_source | content_translation_outdated |
+-----+-----+----------+--------+-----+----------------------------------+------------+------------+---------+--------+------------------+-------------------------------+----------------------------+------------------------------+
|   1 |   1 | en       |      1 |   3 | First version of the content en. | 1696839199 | 1696839199 |       1 |      0 |                1 |                             1 | und                        |                            0 |
|   1 |   2 | en       |      1 |   3 | First version of the content en. | 1696839199 | 1696839199 |       1 |      0 |                1 |                          NULL | und                        |                            0 |
|   1 |   2 | es       |      1 |   4 | First version of the content es. | 1696839202 | 1696839202 |       1 |      0 |                0 |                             1 | en                         |                            0 |
+-----+-----+----------+--------+-----+----------------------------------+------------+------------+---------+--------+------------------+-------------------------------+----------------------------+------------------------------+
3 rows in set (0.000 sec)

MySQL [drupal]> select * from test76006253node_revision;
+-----+-----+----------+--------------+--------------------+--------------+------------------+
| nid | vid | langcode | revision_uid | revision_timestamp | revision_log | revision_default |
+-----+-----+----------+--------------+--------------------+--------------+------------------+
|   1 |   1 | en       |            3 |         1696839199 | NULL         |                1 |
|   1 |   2 | en       |            4 |         1696839202 | NULL         |                1 |
+-----+-----+----------+--------------+--------------------+--------------+------------------+
2 rows in set (0.000 sec)

• After deleting the user (with patch):

MySQL [drupal]> select * from test76006253node_field_data;
+-----+-----+-------------------+----------+--------+-----+----------------------------------+------------+------------+---------+--------+------------------+-------------------------------+----------------------------+------------------------------+
| nid | vid | type              | langcode | status | uid | title                            | created    | changed    | promote | sticky | default_langcode | revision_translation_affected | content_translation_source | content_translation_outdated |
+-----+-----+-------------------+----------+--------+-----+----------------------------------+------------+------------+---------+--------+------------------+-------------------------------+----------------------------+------------------------------+
|   1 |   6 | moderated_content | en       |      1 |   0 | First version of the content en. | 1696839199 | 1696839267 |       1 |      0 |                1 |                          NULL | und                        |                            0 |
+-----+-----+-------------------+----------+--------+-----+----------------------------------+------------+------------+---------+--------+------------------+-------------------------------+----------------------------+------------------------------+
1 row in set (0.000 sec)

MySQL [drupal]> select * from test76006253node_field_revision;
+-----+-----+----------+--------+-----+----------------------------------+------------+------------+---------+--------+------------------+-------------------------------+----------------------------+------------------------------+
| nid | vid | langcode | status | uid | title                            | created    | changed    | promote | sticky | default_langcode | revision_translation_affected | content_translation_source | content_translation_outdated |
+-----+-----+----------+--------+-----+----------------------------------+------------+------------+---------+--------+------------------+-------------------------------+----------------------------+------------------------------+
|   1 |   3 | en       |      1 |   0 | First version of the content en. | 1696839199 | 1696839267 |       1 |      0 |                1 |                             1 | und                        |                            0 |
|   1 |   4 | en       |      1 |   0 | First version of the content en. | 1696839199 | 1696839267 |       1 |      0 |                1 |                             1 | und                        |                            0 |
|   1 |   4 | es       |      1 |   0 | First version of the content es. | 1696839202 | 1696839267 |       1 |      0 |                0 |                             1 | en                         |                            0 |
|   1 |   6 | en       |      1 |   0 | First version of the content en. | 1696839199 | 1696839267 |       1 |      0 |                1 |                          NULL | und                        |                            0 |
+-----+-----+----------+--------+-----+----------------------------------+------------+------------+---------+--------+------------------+-------------------------------+----------------------------+------------------------------+
4 rows in set (0.000 sec)

MySQL [drupal]> select * from test76006253node_revision;
+-----+-----+----------+--------------+--------------------+--------------+------------------+
| nid | vid | langcode | revision_uid | revision_timestamp | revision_log | revision_default |
+-----+-----+----------+--------------+--------------------+--------------+------------------+
|   1 |   3 | en       |            3 |         1696839199 | NULL         |                1 |
|   1 |   4 | en       |            4 |         1696839202 | NULL         |                1 |
|   1 |   6 | en       |            0 |         1696839199 | NULL         |                1 |
+-----+-----+----------+--------------+--------------------+--------------+------------------+
3 rows in set (0.000 sec)

Upload only the PHPUnit test to see the differences with and without the fix.

The result with the fix + PHPUnit test https://www.drupal.org/pift-ci-job/2781901 is the translation "es" lost?

Testing Drupal\Tests\content_moderation\Functional\ModerationStateNodeTest
....E                                                               5 / 5 (100%)

Time: 01:32.262, Memory: 4.00 MB

There was 1 error:

1) Drupal\Tests\content_moderation\Functional\ModerationStateNodeTest::testUserCancel
InvalidArgumentException: Invalid translation language (es) specified.

/var/www/html/core/lib/Drupal/Core/Entity/ContentEntityBase.php:870
/var/www/html/core/modules/content_moderation/tests/src/Functional/ModerationStateNodeTest.php:310
/var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

The result without the fix + PHPUnit test https://www.drupal.org/pift-ci-job/2781940, here we are not sure why is failing in this step.

Testing Drupal\Tests\content_moderation\Functional\ModerationStateNodeTest
....E                                                               5 / 5 (100%)

Time: 01:36.983, Memory: 4.00 MB

There was 1 error:

1) Drupal\Tests\content_moderation\Functional\ModerationStateNodeTest::testUserCancel
Behat\Mink\Exception\ResponseTextException: The text "First version of the content es." was not found anywhere in the text of the current page.

/var/www/html/vendor/behat/mink/src/WebAssert.php:811
/var/www/html/vendor/behat/mink/src/WebAssert.php:262
/var/www/html/core/modules/content_moderation/tests/src/Functional/ModerationStateNodeTest.php:314
/var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

On my local with version 10.1.x only with the PHPUnit test, the result is:

wodby@php.container:/var/www/html $  ./vendor/phpunit/phpunit/phpunit web/core/modules/content_moderation/tests/src/Functional/ModerationStateNodeTest.php --filter=testUserCancel
PHPUnit 9.6.13 by Sebastian Bergmann and contributors.

Testing Drupal\Tests\content_moderation\Functional\ModerationStateNodeTest
F                                                                   1 / 1 (100%)

Time: 00:42.279, Memory: 10.00 MB

There was 1 failure:

1) Drupal\Tests\content_moderation\Functional\ModerationStateNodeTest::testUserCancel
Check user on translation.
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'4'
+'0'

/var/www/html/vendor/phpunit/phpunit/src/Framework/Constraint/Equality/IsEqual.php:94
/var/www/html/web/core/modules/content_moderation/tests/src/Functional/ModerationStateNodeTest.php:316
/var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

So it is possible that this solution is not stable or needs more investigation.

We applied the changes from https://www.drupal.org/project/drupal/issues/3089747 because have some relation and both patches collide.

The author was not set correctly because on the hook node_user_cancel, when the function node_mass_update is called to update the nodes, it does not take into account the langcode, so sets all revisions uid to 0.

      $nodeStorage = \Drupal::entityTypeManager()->getStorage('node');
      // Reassign node ownership.
      $revision_id = $nodeStorage->userRevisionIds($account);
      if (!empty($revision_id)) {
        node_mass_update($revision_id, ['uid' => 0], NULL, TRUE, TRUE);
      }

When a new translation is created, on the original language is also created a new revision, the method userRevisionIds($account), gets the information from the table 'node_field_revision', and if the deleted user is the author of the original revision, when a new translation is created, also in the original language will be created a new revision, so the method userRevisionIds returns that the user author of the original language is author of both translations.
Example:

MariaDB [drupal]> select nid, vid, langcode, uid from node_field_revision;
+-----+-----+----------+-----+
| nid | vid | langcode | uid |
+-----+-----+----------+-----+
|   1 |   1 | en       |   2 |
|   1 |   2 | en       |   2 |
|   1 |   2 | es       |   3 |
+-----+-----+----------+-----+
3 rows in set (0.016 sec)

We patch the function node_user_cancel to get the author of each translation filtering by the langcode.

      $query =  \Drupal::database()->select('node_field_revision', 'n');
      $query->fields('n', ['langcode']);
      $query->addExpression('GROUP_CONCAT(vid)', 'revisions');
      $query->condition('uid', $account->id());
      $query->groupBy('langcode');
      $result = $query->execute();
      $result->setFetchMode(\PDO::FETCH_ASSOC);
      $revision_list = [];
      foreach ($result->fetchAll() as $value) {
        $revision_list[$value['langcode']] = explode(',', $value['revisions']);
      }

      foreach ($revision_list as $langcode => $revisions) {
        node_mass_update($revisions, ['uid' => 0], $langcode, TRUE, TRUE, TRUE);
      }

Those are the tables node_field_data and node_field_revision before and after the patch:

• Create content moderated "en" published, with user1.
• Create a translation "es" published with user2

Before applying the patch:
Before deleting the user:

MariaDB [drupal]> select nid, vid, langcode, uid from node_field_revision;
+-----+-----+----------+-----+
| nid | vid | langcode | uid |
+-----+-----+----------+-----+
|   1 |   1 | en       |   2 |
|   1 |   2 | en       |   2 |
|   1 |   2 | es       |   3 |
+-----+-----+----------+-----+
3 rows in set (0.016 sec)

MariaDB [drupal]> select nid, vid, langcode, uid from node_field_data;
+-----+-----+----------+-----+
| nid | vid | langcode | uid |
+-----+-----+----------+-----+
|   1 |   2 | en       |   2 |
|   1 |   2 | es       |   3 |
+-----+-----+----------+-----+
2 rows in set (0.001 sec)

After delete the user:

MariaDB [drupal]> select nid, vid, langcode, uid from node_field_revision;
+-----+-----+----------+-----+
| nid | vid | langcode | uid |
+-----+-----+----------+-----+
|   1 |   1 | en       |   0 |
|   1 |   2 | en       |   0 |
|   1 |   2 | es       |   0 |
+-----+-----+----------+-----+
3 rows in set (0.016 sec)

MariaDB [drupal]> select nid, vid, langcode, uid from node_field_data;
+-----+-----+----------+-----+
| nid | vid | langcode | uid |
+-----+-----+----------+-----+
|   1 |   2 | en       |   0 |
|   1 |   2 | es       |   0 |
+-----+-----+----------+-----+
2 rows in set (0.001 sec)

After applying the patch:
Before deleting the user:

MariaDB [drupal]> select nid, vid, langcode, uid from node_field_revision;
+-----+-----+----------+-----+
| nid | vid | langcode | uid |
+-----+-----+----------+-----+
|   1 |   1 | en       |   2 |
|   1 |   2 | en       |   2 |
|   1 |   2 | es       |   3 |
+-----+-----+----------+-----+
3 rows in set (0.016 sec)

MariaDB [drupal]> select nid, vid, langcode, uid from node_field_data;
+-----+-----+----------+-----+
| nid | vid | langcode | uid |
+-----+-----+----------+-----+
|   1 |   2 | en       |   2 |
|   1 |   2 | es       |   3 |
+-----+-----+----------+-----+
2 rows in set (0.001 sec)

After deleting the user with UID 2:

MariaDB [drupal]> select nid, vid, langcode, uid from node_field_revision;
+-----+-----+----------+-----+
| nid | vid | langcode | uid |
+-----+-----+----------+-----+
|   1 |   1 | en       |   0 |
|   1 |   2 | en       |   0 |
|   1 |   2 | es       |   3 |
+-----+-----+----------+-----+
3 rows in set (0.016 sec)

MariaDB [drupal]> select nid, vid, langcode, uid from node_field_data;
+-----+-----+----------+-----+
| nid | vid | langcode | uid |
+-----+-----+----------+-----+
|   1 |   2 | en       |   0 |
|   1 |   2 | es       |   3 |
+-----+-----+----------+-----+
2 rows in set (0.001 sec)

The tests are failing on CI, but not in local.
We executed the same tests and is not failing.

We mounted the project using ddev and require drupal core 10.1.x-dev

    "require": {
        "composer/installers": "^2.0",
        "cweagans/composer-patches": "^1.7",
        "drupal/core": "10.1.x-dev as 10.1.5",
        "drupal/core-composer-scaffold": "^10.1",
        "drupal/core-project-message": "^10.1",
        "drupal/core-recommended": "^10.1",
        "drush/drush": "^12.4"
    },

Why the results are not the same?

Drupal is run in a subdirectory for DrupalCI and GitLab CI so paths will be relative to them.

I think, from seeing the MR, if you change $this->drupalGet('/es/' . $node->toUrl()->toString()); for $this->drupalGet($node->toUrl('canonical', ['language' => 'es'])->toString());, things should work locally and on the CI systems.

Thank you @fjgarlin seems working on https://git.drupalcode.org/issue/drupal-2977362/-/jobs/235674
We tried with "$this->drupalGet('es/node/' . $node->id());",

Upload the patch with only the PHPUnit tests to check that Drupal is failing in some scenarios related to the issue.

Upload only the tests

The only PHPUnit tests patch gives the following errors:
https://www.drupal.org/pift-ci-job/2797148

• Translation es disappear when the author of the original content is deleted

Testing Drupal\Tests\content_moderation\Functional\ModerationStateNodeTest
....F                                                               5 / 5 (100%)

Time: 01:23.869, Memory: 4.00 MB

There was 1 failure:

1) Drupal\Tests\content_moderation\Functional\ModerationStateNodeTest::testUserCancel
User with uid 4should have 1 rows on table node_field_data and language es
Failed asserting that '0' matches expected 1.

• This error can be ignored because the method does not exists on the patch only tests

Testing Drupal\Tests\node\Kernel\NodeStorageTest
.E                                                                  2 / 2 (100%)

Time: 00:02.821, Memory: 4.00 MB

There was 1 error:

1) Drupal\Tests\node\Kernel\NodeStorageTest::testUserRevisionAuthorRevisionIds
Error: Call to undefined method Drupal\node\NodeStorage::userRevisionAuthorRevisionIds()

• When the author of the original language is removed, all revisions authors are set to anonymous

Testing Drupal\Tests\node\Kernel\NodeUserCancelTest
F                                                                   1 / 1 (100%)

Time: 00:01.495, Memory: 4.00 MB

There was 1 failure:

1) Drupal\Tests\node\Kernel\NodeUserCancelTest::testCancelReassignToAnonymous
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'2'
+'0'

Ready to review

Nightwatch failed on the merge request, but We are not sure why https://git.drupalcode.org/issue/drupal-2977362/-/pipelines/44783 and probably not related to the MR.

  ️TEST FAILURE (4m 40s):  
   - 1 assertions failed; 1905 passed
   ✖ 20) Tests/a11yTestAdmin
   – Accessibility - Admin Theme: Structure | Block (2.449s)
   → ✖ NightwatchAssertError
   aXe rule: landmark-contentinfo-is-top-level - Contentinfo landmark should not be contained in another landmark
	In element: .messages-list__item

1 nitpicky issue.

FYI you don't need test-only patch anymore there is a test-only feature in gitlab that can be ran.

Reran the nightwatch and it was indeed a random.

Also not sure if needed searched for _node_mass_update_helper http://grep.xnddx.ru/search?text=_node_mass_update_helper&filename= and only found 1 hit.

@smustgrave, sorry I did not understand your question about _node_mass_update_helper. Should we remove or replace the function? what do you mean?

Oh I was just noting other modules that may call this. Nope you're good to leave as is.

Thank you @smustgrave, there is something left?

Left two small nitpicky things, going to leave in review for others to comment too.

Hiding the patches and leaving a note in the issue summary the fix is in MR4713

Changes proposed by @smustgrave applied.

Circling back to this one

Ran test-only feature again

There was 1 failure:
1) Drupal\Tests\content_moderation\Functional\ModerationStateNodeTest::testUserCancel
User with uid 4should have 1 rows on table node_field_data and language es
Failed asserting that '0' matches expected 1.
/builds/issue/drupal-2977362/vendor/phpunit/phpunit/src/Framework/Constraint/Equality/IsEqual.php:94
/builds/issue/drupal-2977362/core/modules/content_moderation/tests/src/Functional/ModerationStateNodeTest.php:347
/builds/issue/drupal-2977362/core/modules/content_moderation/tests/src/Functional/ModerationStateNodeTest.php:305
/builds/issue/drupal-2977362/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
FAILURES!

Tested following the steps and MR did address the issue.

Learned recently data loss is a critical issue. Will try and get some traction in slack

Mostly minor feedback

We will try to review the minor changes this week.
Thank you acbramley

Waiting for tests.

All points suggested by acbramley were addressed.

Added a small nit and a discussion on the function name.

Some grammatical suggestions, but that should not hold this back from being reviewed by others.

Applied @BramDriesen grammar suggestions.

1) Drupal\Tests\node\Kernel\NodeUserCancelTest::testCancelReassignToAnonymous
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'2'
+'0'
/builds/issue/drupal-2977362/vendor/phpunit/phpunit/src/Framework/Constraint/Equality/IsEqual.php:94
/builds/issue/drupal-2977362/core/modules/node/tests/src/Kernel/NodeUserCancelTest.php:77
/builds/issue/drupal-2977362/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
FAILURES!
Tests: 1, Assertions: 10, Failures: 1.

Verified the test coverage.

Went through the threads and left a checkmark on what's been addressed, which is everything.

Still verified the issue on 11.x and that the MR addresses it.

RTBC +1 for me.

Verified the fix on Drupal 11 setup as per the steps to reproduce.
-Created content using user 1.
-Edited the content using user 2.
-Deleted user 1 using user 3.
-Verified the revision of the content. Working as expected.

Needs work for @lleber's comment.

Believe all feedback has been addressed.

Added some feedback to the MR.

I merged the #45 and #57 patches, create a new patch for Drupal 10.1.4
it's work for me.

All work is being done in the MR, it's much easier for reviewers if we do not confuse things with additional patches.

Composer patches support local files, you do not need to upload them to an issue.

I tried to apply all the code modified by MR4713 in drupal 10.1.8 version and found that when selecting "Delete the account and make its content belong to the Anonymous user. This action cannot be undone. Reassign its groups to the super administrator." The created node by this user will also be deleted.Is there any new progress regarding this ticket?

I updated the patch from #116 to work with Drupal 10.3.0

See #117

Issue #3384789: Allow restriction of available User Cancellation Methods on deletion confirmation form could help to limit the options

Some rough conflicts in here that need resolving.

Reroled and refactored the fix.
Updated tests.
Also, provided patch files for different versions,

P.S. There is also a related bug described here #3338260: Skip changing 'revision translation affected' field if the old revision is syncing. Without that patch, a revision_translation_affected property might be changed, which can cause the issue when we see a revision of the other(unexpected) language in the revisions list.

Steps to reproduce:

• User A - created a node EN - revision 1
• User B - created a translation ES - revision 2
• User A - updated a translation EN - revsion 3
• Revision list:
  • EN - revisions 1 and 3
  • ES - revision 2
• Remove User B
• Revision list:
  • EN - revisions 1, 2, 3
  • ES - revision 2

We need to move these new methods somewhere else, NodeStorage is in the process of being deprecated. #3396062: Deprecate most methods on NodeStorage

We also should be using entity queries instead of direct db queries.

Hi @acbramley
I understand your point, but:
- That ticket is still in progress, so it doesn't make sense to move the functions elsewhere and refactor them for now, because it's still a big question which of the tasks will be solved first.
Since there will be a merge conflict here, the task that will merge last will fix it. So, we can ignore it for now.
- A similar opinion as above, since all class methods currently use db queries, it should be the same here. Their refactoring will be part of #3396062 or will happen here if that task is merged first.
Especially since we can't use entity queries to get these values. It will at least be select queries

Conclusion: Your comment cannot be a blocker for this ticket.

Going to agree with with 127, but can leave in review for others

so it doesn't make sense to move the functions elsewhere and refactor them for now

It doesn't make sense to add new methods to an interface/class that we know is being deprecated. We can postpone this issue on that one instead if you like.

since all class methods currently use db queries, it should be the same here

That code is ancient and is part of the reason why that class is being deprecated, it's not compatible with all db engines. We should do it properly first since we're introducing new functionality, not leave it to a follow up to refactor again.

@acbramley

We need to move these new methods somewhere else

It doesn't make sense to add new methods to an interface/class that we know is being deprecated. We can postpone this issue on that one instead if you like.

Where exactly to move?
My opinion is that new methods must be in the same place where revisionIds() is located, which is logical, but I'd be happy to hear another suggestion.
And I'm not sure it's right to postpone a critical issue because of a minor one.

That code is ancient and is part of the reason why that class is being deprecated, it's not compatible with all db engines. We should do it properly first since we're introducing new functionality, not leave it to a follow up to refactor again.

But it's not a deprecated class yet, we just have a ticket to make it so, and it's a big question when that ticket will be resolved. And which of the tickets will be solved first.
If this one is merged first, then that one will be refactored according to 11.x. If that one will be merged first, then this one will be refactored.
For example, we had to refactor this one after merging the #3442009: OOP hooks using attributes and event dispatcher

So I see the following ways:
1. Keep it as it is, cuz the other ("normal" priority) refactoring ticket cannot block the "critical" one, this issue breaks the content on the site and must be fixed asap. Later in the context of #3396062 the code will be refactored
2. To locate new methods in the same place as a revisionIds() method, we can make a separate branch as a combination of #2977362 and #3396062 + keep the current one. The two branches should always be up to date. But I don't think this is the right way to combine the two tickets, because it could delay the resolving of a critical issue, but this is a possible way.

I would like to hear other opinions. Maybe even in the form of a vote :)

Updated code to work with latest 11.x. There were conflicts due to #3511357: Clean up hook implementations in Node module and #3396062: Deprecate most methods on NodeStorage.

None of the remarks have been addressed. Now that #3396062: Deprecate most methods on NodeStorage is in it is expected that there will be deprecation warnings.

Addressed #127 and the remarks on the MR. Extended the test coverage to include a forward draft revision, which covers the change in ContentEntityBase.

This issue is also affecting contrib, e.g. Entity Reference Revisions: #3549453: Exception when cancelling user account.

Appears to have some open threads in the MR.

Rerolled for 11.3.x: adjusted for NodeBulkUpdate service and NodeUserHooks (node.admin.inc now deprecated wrapper);
avoided adding new methods to deprecated NodeStorageInterface by querying revision tables directly.
Added syncing flag flow to NodeBulkUpdate + wrapper and retained the same behavior.
Included ModerationStateUserTest. Patch applies cleanly on 11.3.2.

@jorgik can you please make the changes to the MR instead of posting them in a patch? Maintainers have little time and it is much easier to check changes in the MR instead of having to go through a patch line by line to see what is changed ;)

I encountered the same revision author issue in Drupal core version 10.3.1.
The patch referenced in the Drupal.org issue could not be applied directly due to differences in the core implementation for this version. Therefore, we created a custom patch aligned specifically with Drupal 10.3.1 after following the documented reproduction steps.

Steps to Reproduce (Verified)

1. User A creates a node
2. User B edits the same node (new revisions created)
3. User C (Admin) cancels User A’s account and selects “Reassign content to the anonymous user” (user_cancel_reassign)
4. Navigate to the Revisions tab

Issue Observed (Before Patch)
Before applying the patch, all revisions authored by User A and User B were still correctly stored in the database.

After User C (Admin) cancels User A’s account and selects “Reassign content to the anonymous user” (user_cancel_reassign)

After Patch apply

• A) Revision history integrity is preserved
• B) Only the canceled user’s revisions are reassigned
• C) Editorial audit trail remains accurate

Before Apply Pacth:
select * from node_revision where nid=134626;
+--------+--------+----------+--------------+--------------------+-----------------------------------------------------------------------------------------------+------------------+
| nid | vid | langcode | revision_uid | revision_timestamp | revision_log | revision_default |
+--------+--------+----------+--------------+--------------------+-----------------------------------------------------------------------------------------------+------------------+
| 134626 | 236476 | ja | 3656 | 1617180547 | NULL | 1 |
| 134626 | 236521 | ja | 3656 | 1617181953 | NULL | 0 |
| 134626 | 236531 | ja | 3656 | 1617182124 | NULL | 0 |
| 134626 | 236536 | ja | 3656 | 1617182149 | NULL | 1 |
| 134626 | 448176 | ja | 3656 | 1617182149 | NULL | 1 |
| 134626 | 638421 | ja | 4351 | 1715586369 | NULL | 0 |
| 134626 | 638426 | ja | 4351 | 1715586408 | NULL | 0 |
| 134626 | 638431 | ja | 4351 | 1715586451 | NULL | 0 |
| 134626 | 638436 | ja | 4351 | 1715586494 | NULL | 1 |
| 134626 | 638521 | ja | 4351 | 1715643866 | NULL | 1 |
+--------+--------+----------+--------------+--------------------+-----------------------------------------------------------------------------------------------+------------------+
10 rows in set (0.005 sec)

After Apply Patch
select * from node_revision where nid=134626;
+--------+--------+----------+--------------+--------------------+-----------------------------------------------------------------------------------------------+------------------+
| nid | vid | langcode | revision_uid | revision_timestamp | revision_log | revision_default |
+--------+--------+----------+--------------+--------------------+-----------------------------------------------------------------------------------------------+------------------+
| 134626 | 236476 | ja | 0 | 1617180547 | NULL | 1 |
| 134626 | 236521 | ja | 0 | 1617181953 | NULL | 0 |
| 134626 | 236531 | ja | 0 | 1617182124 | NULL | 0 |
| 134626 | 236536 | ja | 0 | 1617182149 | NULL | 1 |
| 134626 | 448176 | ja | 3656 | 1617182149 | NULL | 1 |
| 134626 | 638421 | ja | 4351 | 1715586369 | NULL | 0 |
| 134626 | 638426 | ja | 4351 | 1715586408 | NULL | 0 |
| 134626 | 638431 | ja | 4351 | 1715586451 | NULL | 0 |
| 134626 | 638436 | ja | 4351 | 1715586494 | NULL | 1 |
| 134626 | 638521 | ja | 4351 | 1715643866 | NULL | 1 |
+--------+--------+----------+--------------+--------------------+-----------------------------------------------------------------------------------------------+------------------+

I addressed all the open remarks on the MR and merged in latest changes from main. In addition to this:

• Reverted unrelated change in NodeStorage.
• Replaced deprecated call to user_cancel().

I had Claude Opus 5.5 have a look at what is left to do on this issue and it made an interesting suggestion:

Regarding the remaining task "Decide on alternative in case there are large number of revisions":

The update itself scales: NodeBulkUpdate::process() switches to a batch when there are more than 10 revisions. However, before the batch starts, NodeUserHooks::userRevisionIdsGroupedByLanguage() loads every revision owned by the cancelled user, in a single request, only to check which translations they own. For a user with thousands of revisions this is slow and memory intensive, and it happens outside the batch.

We don't need to load the revisions for this. An entity query can restrict a condition to a single language, so we can run one query per language and get the revision IDs directly:

$grouped = [];
foreach ($this->languageManager->getLanguages(LanguageInterface::STATE_ALL) as $langcode => $language) {
  $revisionIds = $this->entityTypeManager->getStorage('node')->getQuery()
    ->accessCheck(FALSE)
    ->allRevisions()
    ->condition('uid', $account->id(), '=', $langcode)
    ->sort('vid')
    ->execute();
  if ($revisionIds) {
    $grouped[$langcode] = array_keys($revisionIds);
  }
}

This runs one lightweight query per language instead of loading each revision. I tried this locally: ModerationStateUserTest and NodeUserCancelTest both pass, and removing the language argument from the condition makes ModerationStateUserTest fail, so the existing coverage protects the per-language behaviour. The snippet assumes LanguageManagerInterface is injected into NodeUserHooks.

This is probably worth a look since it seems like a significant performance gain.

Did some more work on the MR:

• Added a test that actually tests the exact scenario that was originally reported: cancelling the node author.
• Some small documentation fixes.
• Applied the performance improvement suggested in the previous comment.

I re-reviewed the full MR after merging in the latest changes on main. Everything was looking pretty good, but wanted to check that we weren't missing any test coverage.

I had Claude analyse the changes and find any gaps in testing. The full list of Gaps is attached as a text file. I went ahead and had it fix the most important ones:

1. The core storage change has no direct test. - Fixed in f03c4bdb
2. An existing core test hid this bug - UserCancelTest said it builds "a node with two revisions" but since it called get_object_vars it was actually creating 2 separate nodes. That hid the bug with the syncing change in ContentEntityStorageBase. Fixed in c01fdf02
3. Changes to changed and the revision count aren't checked - Since we're using the syncing swap, we weren't checking that the changed dates or revision ids weren't changing. Fixed in b8e71e99

I think this should be ready now.