User picture not deleted after cancelling user's account

I was able to reproduce this on a fresh 7.10 install.

1. Create new user via admin account.
2. Add a picture to the user's account via admin account or the user account itself.
3. Cancel the user's account using the "Delete the account and its content." option.

The user picture remains in the file system and in the file_managed and file_usage tables.

Possibly related to http://drupal.org/node/1378092.

I've tested this issue after applying the patch from http://drupal.org/node/1378092 and it does not resolve the issue - the difference is that this is via the cancellation form which calls user_delete whereas 1378092 is on the user_save call when saving user profile.

I'd be interested in opinions on a couple of approaches to this:

1. Modify user.module user_delete function to directly call file_usage_delete and file_delete if there is a profile picture attached to the user account (which would solve this individual issue)

2. As there would appear to be the possibility that any files which belong to the user may not be deleted when the user is deleted via user_delete_multiple, would a better approach be to implement hook_user_delete in the file module (file_user_delete) so that the file module will respond to the user delete by deleting any files associated with them?

As an aside - and might need a seperate issue, as far as I can see the code for user_cancel applies the same to "Delete user and all their content" and "Delete user and assign their content to anonymous". Do we need to cover this scenario too?

It makes the most sense that file deletion would be handled in file.module by a hook_user_predelete(), since user_delete() concerns itself only with deleting users from the {users*} tables (and {authmap}), whereas other modules are responsible for handling the deletion of their user data (e.g. node_user_predelete() and comment_user_predelete()).

Since a picture can't be added to the anonymous user (and thus can't be deleted), it doesn't make sense to transfer the picture over to the anonymous user if the ""Delete user and assign their content to anonymous" option is used.

Attached is a patch that deletes the picture when a user account is deleted, as well as adds a test for testing if a picture actually does get deleted when a user account is deleted.

This should probably be backported to 7.x too, no?

Patch looks sensible, and better than my idea!

I've tested in my dev environment and confirmed that the file is deleted from the file system and from the database.

Huh. I totally forgot there was a hook_user_predelete(). That seems like a sane fix to me.

Looks like this needs a re-roll, however.

Thanks webchick! Rerolled patch against current HEAD.

#7: user-1398616-7.patch queued for re-testing.

d7 version would be nice here indeed...

+++ b/core/modules/user/user.test
@@ -1075,6 +1075,45 @@ class UserPictureTestCase extends DrupalWebTestCase {
+      $this->drupalGet('user/' . $this->user->uid . '/edit');
+      $this->drupalPost(NULL, NULL, t('Cancel account'));

Minor: but the drupalGet() is unnecessary if you put the path in drupalPost().

Don't know if it is worth holding this up on that, since it is really aesthetic.
If not, this is RTBC.

Oops, wanted to leave at needs review.

+++ b/core/modules/user/user.testundefined
@@ -1075,6 +1075,45 @@ class UserPictureTestCase extends DrupalWebTestCase {
+   * Test that picture is deleted when user account is deleted.

Also, on the next reroll "Test" needs to be changed to "Tests"

I needed this for D7. Rerolled using hook_user_delete since hook_user_predelete doesn't seem to be available in D7.

Also include suggestions from comment #10 and #12. The test passes here.

Duplicates #1378092: User pictures are not removed properly I guess. @wedge?

@barraponto Not really a duplicate, and since that issue is closed I guess we can continue here? See explanation in comment #3.

#7: user-1398616-7.patch queued for re-testing.

Here's a patch rerolled against head that makes the changes in #10 and #12. file_usage_delete has also been removed from core, so made the necessary changes over to the new api.

Whoops, forgot to convert the variable_set to the new config system. Tests should pass now.

Fixing this might be a solution for #1568098: Delete Pictures on Feeds delete. And possibly #1433188: Add support for User Picture mapping field.

Currently a blocker for inclusion of 'feeds_user_picture' in Feeds.
I will be testing this patch soon I hope.

change Description

@alansaviolobo - I can only speak for myself but I'd ask that you please stop re-testing all these old issues, a bot could go through all these issues and click re-test on the last patch so I don't think it's very productive. A better use of your time might be for example to test this issue to see if it still applies to the latest 8.0.x, and do some triage.

Contributor task: Verify a reported issue

User picture is a field now, so this only applies to D7 now.