Problem/Motivation

During the Private Shortcut module development, I found that when an user account is deleted, its assigned shortcut set remains mapped in the shortcut_set_users table.

Steps to reproduce

In a fresh standard D9.4 installation:

  1. Create a new user account, their expected UID is 2
  2. Explicitly assign the default shortcut set at /user/2/shortcuts
  3. Remove the user account

The shortcut assignment will remain at the shortcut_set_users table:

> SELECT * FROM shortcut_set_users;
+-----+----------+
| uid | set_name |
+-----+----------+
|   2 | default  |
+-----+----------+

Proposed resolution

Clean up assigned shortcut set when a user is deleted.

Remaining tasks

Patch, test, review.

User interface changes

None.

API changes

N/A

Data model changes

N/A

Release notes snippet

Not required.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

manuel.adan created an issue. See original summary.

manuel.adan’s picture

manuel.adan’s picture

Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 2: 3262384-2-TEST-ONLY-FAIL.patch, failed testing. View results

manuel.adan’s picture

Status: Needs work » Needs review

Test only patch failed as expected.

manuel.adan’s picture

Issue summary: View changes
manuel.adan’s picture

Status: Needs review » Reviewed & tested by the community

I dare to move it as RTBC to gain attention since it is easy to review.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 2: 3262384-2-TEST-ONLY-FAIL.patch, failed testing. View results

manuel.adan’s picture

Status: Needs work » Reviewed & tested by the community

Again TEST-ONLY-FAIL failed as expected. Fixed full patch/fail files weights.

catch’s picture

Status: Reviewed & tested by the community » Needs review
Issue tags: +Bug Smash Initiative

This makes sense. After this patch is committed, we'll still have the stale data in people's databases from before the bug was fixed. However, I think it's better to commit the fix to stop new bad data being created, and then have a follow-up to clean the old data up, so I've opened #3265660: Clean up stale shortcut set records for that. But, please don't set your own patches to RTBC - we only commit core issues with at least two, ideally three including the committer involved.

Also wondering if the test coverage should actually try to load the shortcut set we requested from the database, rather than checking whether the now-deleted user gets the default shortcut set.

larowlan’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me

The last submitted patch, 2: 3262384-2-TEST-ONLY-FAIL.patch, failed testing. View results

The last submitted patch, 2: 3262384-2-TEST-ONLY-FAIL.patch, failed testing. View results

The last submitted patch, 2: 3262384-2-TEST-ONLY-FAIL.patch, failed testing. View results

  • catch committed a34d0d6 on 10.0.x
    Issue #3262384 by manuel.adan: Assigned shortcut set is not cleaned on...
  • catch committed 44d1ac5 on 9.3.x
    Issue #3262384 by manuel.adan: Assigned shortcut set is not cleaned on...
  • catch committed c20ea2a on 9.4.x
    Issue #3262384 by manuel.adan: Assigned shortcut set is not cleaned on...
catch’s picture

Version: 9.4.x-dev » 9.3.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 10.0.x, cherry-picked to 9.4.x and 9.3.x, thanks!

Status: Fixed » Closed (fixed)

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