Problem

If you try to delete a user that is already deleted (via steps below), the page just reloads with no indication of what happened.

Steps to reproduce:

  1. Create a user
  2. Go to admin/people
  3. Edit the user in a new tab
  4. Cancel the user account in the new tab, delete the account and its content
  5. Go back to the admin/people tab where the deleted user is still listed
  6. Select the user and cancel it using the bulk upload actions
  7. See the page reloads without the deleted user but there is no indication of what happened
CommentFileSizeAuthor
#6 cancel_user.mov13.29 MBsiddhant.bhosale
#3 3105973-3.patch2.79 KBhardik_patel_12
#2 3105973-2.patch681 byteshardik_patel_12
people_delete_error.mov19.61 MBhardik_patel_12

Comments

Hardik_Patel_12 created an issue. See original summary.

hardik_patel_12’s picture

hardik_patel_12 commented
StatusFileSize
new 3105973-2.patch681 bytes

Kindly review a patch

hardik_patel_12’s picture

hardik_patel_12 commented
StatusFileSize
new 3105973-3.patch2.79 KB

Kindly review a new patch.

hardik_patel_12’s picture

hardik_patel_12 commented
Assigned: hardik_patel_12 » Unassigned
Status: Needs work » Needs review
siddhant.bhosale’s picture

siddhant.bhosale commented
Assigned: Unassigned » siddhant.bhosale
siddhant.bhosale’s picture

siddhant.bhosale commented
StatusFileSize
new cancel_user.mov13.29 MB

I have tested the patch and works for me. looks good to be merged.

Adding the screen-recording while testing the patch.

siddhant.bhosale’s picture

siddhant.bhosale commented
Assigned: siddhant.bhosale » Unassigned
Status: Needs review » Reviewed & tested by the community
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Priority: Major » Normal

This seems to silently ignore the error. Shouldn't we warn the user that whatever they are trying to do couldn't be done because the entity no longer exists?

Also, this doesn't seem to fit "major" priority under our criteria.

hardik_patel_12’s picture

hardik_patel_12 commented

Absolutely @longwave you are right that user can't do anything with non existing entity, and it is normal priority as we have to handle fatal error if user user try do unusual scenario such as mentioned above.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

This could use test coverage.

Given we're just handling a race condition here (entities deleted in between selecting and submitting the form) I think silently not doing anything is probably OK.

xjm’s picture

xjm
she/her
Primary language English
commented
Issue tags: +Needs issue summary update

Since we should fix this bug in D8 too, I'm filing it against 8.8.x (which is the current bugfix support branch). Patches can be tested against other branches as needed when they're uploaded without changing the issue's version selector.

The issue will be automatically updated to 8.9.x after the last 8.8.x bugfix release. Thanks!

Also, we should write an actual issue summary.

Version: 9.0.x-dev » 9.1.x-dev

Drupal 9.0.10 was released on December 3, 2020 and is the final full bugfix release for the Drupal 9.0.x series. Drupal 9.0.x will not receive any further development aside from security fixes. Sites should update to Drupal 9.1.0 to continue receiving regular bugfixes.

Drupal-9-only bug reports should be targeted for the 9.1.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.2.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.1.x-dev » 9.3.x-dev

Drupal 9.1.10 (June 4, 2021) and Drupal 9.2.10 (November 24, 2021) were the last bugfix releases of those minor version series. Drupal 9 bug reports should be targeted for the 9.3.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.4.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.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.

Version: 9.5.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. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

pameeela’s picture

pameeela commented
Title: Error: Call to a member function access() on null in Drupal\user\Plugin\Action\CancelUser->access() (line 90 of /Applications/MAMP/htdocs/drupal-9-dev/core/modules/user/src/Plugin/Action/CancelUser.php) » Error when trying to delete a user that is already deleted
Issue summary: View changes
Status: Needs work » Postponed (maintainer needs more info)
Issue tags: -Needs issue summary update +Bug Smash Initiative

I've tried to reproduce this, but I'm not getting the error that was originally reported. In the screencast provided, there is no error visible but I'm not seeing an error in the logs either. It does just fail silently but I don't see why this is a use case to support? Can we close this if there is no error?

pameeela’s picture

pameeela commented
Title: Error when trying to delete a user that is already deleted » Unexpected behaviour when trying to delete a user that is already deleted

Updating title to reflect that there is no error from what I can see. Not sure if I have missed something.

catch’s picture

catch
he/him
Primary language English
commented

The silent failure accounts for a potential real race condition.

e.g. if two users are trying to delete a user at the same time, and one deletes the user individually, and the other users the bulk actions from admin permissions, and the bulk action runs last, then there is just nothing to do because the user is already deleted.

However it's also not particularly helpful to show a message because e.g. if you want to delete 10 users, and one of them gets deleted between selecting them and submitting the form, and then after the form was submitted, all 10 are deleted (even if one was deleted by someone else), what should be reported? That a user couldn't be deleted because they were already deleted?

Probably we could log something like that, but it doesn't seem useful to show to users- they could think something has gone wrong when it hasn't.

pameeela’s picture

pameeela commented
Status: Postponed (maintainer needs more info) » Closed (works as designed)

Thanks for confirming, I'll close this then since there was no further comment after it was marked as postponed.