Problem/Motivation

When users are canceled any files attached to that user are not updated and lead to orphaned files.

Steps to reproduce

  1. Create a test user
  2. Upload a file
  3. Cancel the test user selecting make content belong to anonymous
  4. Check the database and verify the file ownership is still to the ID of the deleted user

Proposed resolution

When a user is canceled make any files belong to anonymous.

Remaining tasks

Reroll patch
Review

User interface changes

NA

API changes

NA

Data model changes

NA

Release notes snippet

NA

Original Post

I think I said it in whole in the subject line.

I deleted two users but in the file_managed table the uids are still from the now deleted users. I would have thought the uids would be changed to zero (the anonymous user).

I'm guessing there needs to be a hook in the file module somewhere to pick up the "user reassign" action.

CommentFileSizeAuthor
#38 1461042-38.patch3.49 KBsmustgrave
#38 interdiff-25-38.txt5.64 KBsmustgrave
#25 1461042-25.patch3.08 KBaerozeppelin
#25 interdiff-1461042-17-25.txt2.47 KBaerozeppelin
#25 test-only-fail-1461042-25.patch2.47 KBaerozeppelin
#17 drupal-file-user-orphaned-1461042-15.patch683 byteseriknewby
#17 interdiff.txt796 byteseriknewby
#15 drupal-file-user-orphaned-1461042-15.patch680 byteseriknewby
#15 interdiff.txt439 byteseriknewby
#13 drupal-file-user-orphaned-1461042-13.patch681 byteseriknewby
#13 interdiff.txt808 byteseriknewby
#10 drupal-file-user-orphaned-1461042-10.patch937 bytessaltednut
#7 drupal-1461042-7.patch932 bytesallie micka
#6 drupal-1461042-6.patch968 bytesallie micka
#5 drupal-1461042-5.patch963 bytesallie micka

Issue fork drupal-1461042

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:

About issue forks

Comments

reg’s picture

reg commented

I came across this because it affected the media module, here's the issue: http://drupal.org/node/1459822

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Version: 7.10 » 8.x-dev
Issue tags: +Needs tests, +Needs backport to D7

Bugs have to be fixed in D8 and then backported to D7.

reg’s picture

reg commented

Ah, thanks for letting me know.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Issue tags: +sprint, +Media Initiative
allie micka’s picture

allie micka
she/her
commented
StatusFileSize
new drupal-1461042-5.patch963 bytes

This patch anonymizes files on user_cancel_reassign, and attempts to delete a user's files when the user is deleted. However, the call to entity_delete_multiple for files will not work until #1361226: Make the file entity a classed object is committed.

allie micka’s picture

allie micka
she/her
commented
Status: Active » Needs work
StatusFileSize
new drupal-1461042-6.patch968 bytes

Aaaand querying the correct table.

allie micka’s picture

allie micka
she/her
commented
StatusFileSize
new drupal-1461042-7.patch932 bytes

Errant case in file_user_cancel

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, drupal-1461042-7.patch, failed testing.

saltednut’s picture

saltednut
she/her
Primary language English
Location Chicago
commented
Status: Needs work » Needs review
StatusFileSize
new drupal-file-user-orphaned-1461042-10.patch937 bytes

Rerolled patch

eriknewby’s picture

eriknewby commented
Issue tags: -Needs tests, -Needs backport to D7, -sprint, -Media Initiative

#10: drupal-file-user-orphaned-1461042-10.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +Needs tests, +Needs backport to D7, +sprint, +Media Initiative

The last submitted patch, drupal-file-user-orphaned-1461042-10.patch, failed testing.

eriknewby’s picture

eriknewby commented
Assigned: Unassigned » eriknewby
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt808 bytes
new drupal-file-user-orphaned-1461042-13.patch681 bytes

After applying the patch from #10, I managed to find some things that needed changing in order to get it to work again.
See the interdiff for actual changes to the patch.
Notice I've removed the hook_user_predelete() call because that doesn't seem to fall into this particular issue that was filed. This issue is specifically about "anonymizing" file management. I think we can open a separate issue to deal with the deleting of file entities when a user account is deleted along with its associated content.

star-szr’s picture

star-szr
he/him
Primary language English
commented
Status: Needs review » Needs work

Nice work @eriknewby! The only thing I'm seeing is the indent level of the break statement doesn't match coding standards, see https://drupal.org/coding-standards#controlstruct.

Setting to needs work not so much for the indentation but because this will need test coverage as well.

eriknewby’s picture

eriknewby commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt439 bytes
new drupal-file-user-orphaned-1461042-15.patch680 bytes

updated indentation. Let me know if I missed anything else.

Status: Needs review » Needs work

The last submitted patch, drupal-file-user-orphaned-1461042-15.patch, failed testing.

eriknewby’s picture

eriknewby commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt796 bytes
new drupal-file-user-orphaned-1461042-15.patch683 bytes

argh. Left out a closing bracket. fixt!

star-szr’s picture

star-szr
he/him
Primary language English
commented
Status: Needs review » Needs work

Code standards look good. Setting to NW for the remaining test coverage.

mgifford’s picture

mgifford
he/him
Primary language English
commented
Assigned: eriknewby » Unassigned
Issue summary: View changes

Status: Needs work » Needs review

andypost queued 17: drupal-file-user-orphaned-1461042-15.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 17: drupal-file-user-orphaned-1461042-15.patch, failed testing.

slashrsm’s picture

slashrsm commented
Issue tags: -sprint +D8Media

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

aerozeppelin’s picture

aerozeppelin commented
Status: Needs work » Needs review
StatusFileSize
new test-only-fail-1461042-25.patch2.47 KB
new interdiff-1461042-17-25.txt2.47 KB
new 1461042-25.patch3.08 KB

Tests for patch #17.

The last submitted patch, 25: test-only-fail-1461042-25.patch, failed testing.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: -Needs tests +Needs Review Queue Initiative, +Needs issue summary update

This issue is being reviewed by the kind folks in Slack, #need-reveiw-queue. 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 require as a guide.

Confirmed this is still an issue in D10.1

Removing the needs tests tag as they were added in #25.

Looking at patch #25 it will need to be rerolled but also it's got a number of deprecation calls in it that will need to be replaced.
Looking at the test I think 

      $auth_user = $this->drupalCreateUser();
      $super_user = $this->drupalCreateUser([
          'cancel account',
          'select account cancellation method',
          'administer users',
        ]);

Can be removed and use the users already in the test.

To adhere to ticket templates tagging for IS update using the standard template with proposed solution explicitly written.

smustgrave’s picture

smustgrave commented
Version: 9.5.x-dev » 10.1.x-dev
Issue summary: View changes
Priority: Normal » Major
Status: Needs work » Needs review
Issue tags: -Needs issue summary update
StatusFileSize
new interdiff-25-38.txt5.64 KB
new 1461042-38.patch3.49 KB

Updated issue summary and rerolled patch replacing deprecated calls.

Think this could be major as this leads to orphaned files.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented
Status: Needs review » Reviewed & tested by the community

The approach was already validated before in #14 and I think this looks good as well, this has enough test coverage as well.

I was doubting if the hook was the right place or if we should do it where the user actually gets removed, but I think this is better so that it all sticks in the file module.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Bug Smash Initiative

No longer applies.

Fix looks good though

+++ b/core/modules/file/tests/src/Functional/FileListingTest.php
@@ -214,6 +217,57 @@ public function testFileListingUsageNoLink() {
+    // Create node entity and attach the created file.
+    $node = $this->drupalCreateNode(['type' => 'article', 'file' => $file]);
+    $node->save();

Why do we create a node and an entity to attach the file to?

Isn't one (the entity test entity) enough?

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

mohit_aghera made their first commit to this issue’s fork.

mohit_aghera opened merge request !10610

mohit_aghera’s picture

mohit_aghera commented
Status: Needs work » Needs review

I've addressed the feedback in #40
I think node tests aren't required as such.

Fixed a few more things in the PR
- convert it to use OOP hooks
- Fixed a few things in tests.
- Tests seems green now.

Hiding all the patches in favour of MR.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: -Needs backport to D7

Nice!

Left 1 item of feedback on the MR.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.