Clean up Section Associations when a Drupal user is deleted

I looked into this a bit, and the proper way to delete this is to load the SectionAssociation entity and then delete it.

So something like this:

$storage_handler = \Drupal::entityTypeManager()->getStorage('section_association');
$entities = $storage_handler->loadByProperties(['user_id' = $account->id()]);
$storage_handler->delete($entities);

This should load all sections that the user is assigned to and then delete them, since delete() here takes an array. The delete() also handles revisions.

See https://api.drupal.org/api/drupal/core%21includes%21entity.inc/function/...
See https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Entity%21...

Now, of course, we have to write a test.

Logging shouldn't be necessary, since the deletion of the user is logged.

Not sure about Cron. That might be worth some research. Could also be a cache issue, but the delete() should handle that.

We should probably associate two users, delete one, and ensure that the second one still exists.

I would also do a direct Equals comparison (or Empty) after the deletion. It might not be equal for unrelated reasons, so stricter checking is better.

Otherwise, the test looks solid.

I like this. It's thorough. The checks on individual users might be removed, but there is no harm in having extra assertions like that. What's missing is this bit, to run after the user deletion:

    //Set Expected User IDs
    $expected = [$editor2->id()];

    //Get assigned users
    $existing_users = $user_storage->getEditors($scheme, $base_link->getPluginId());

    //assert that these are the same
    $this->assertEquals($expected, array_keys($existing_users));

This is the piece that is critical to test: "We had two users and deleted one, now we should have one user."

This looks really good.

1. +++ b/tests/src/Functional/DeleteUserTest.php
   @@ -0,0 +1,87 @@
   +class DeleteUserTest extends BrowserTestBase {
   ...
   +    $admin = $this->setUpAdminUser([
   +      'create page content',
   +      'edit any page content',
   

   As you're not driving around a browser here, this should be a KernelTest (Extend from KernelTestBase, not BrowserTestBase) as it will be an order of magnitude faster. There are some examples in the module you can base it off.

2. +++ b/workbench_access.module
   @@ -188,3 +188,15 @@ function workbench_access_process_inline_entity_form(array &$element, FormStateI
   +    $storage_handler = \Drupal::entityTypeManager()->getStorage('section_association');
   +    $entities = $storage_handler->loadByProperties(['user_id' => $account->id()]);
   +    $storage_handler->delete($entities);
   

   nice!

Changing to "bug" since this breaks the list of assigned users on the editor assignment form.

To reproduce:

- set up an access scheme
- make a few users eligible for section assignment
- add the one with lowest UID to a section
- delete that user
- try to add other users to the section and they won't show up
- if you add users with lower UIDs than the deleted user, the entire list should show up as expected but the deleted user will be represented by a blank row

I haven't tried the patch here but will look into it now.

I've updated the patch. I don't think the previous implementation was correct to delete the entire association entity; this one just just unsets the relevant user_id "field" delta on the entity.

I've left the test as a BrowserTest since that has a bunch of needed traits for entity creation and user management that KernelTest does not.

I was able to reproduce that deleting a user breaks the section with the current dev release as mentioned in #18. You can no longer add users or view the existing users.

I am using:
- Drupal: 8.7.9
- Workbench Access: 1.x-dev
- Config Split: 1.4
- Palantir.net Drupal Skeleton: https://github.com/palantirnet/drupal-skeleton
- Drush: 8.1.18
- PHPUnit: 6.5.14

Steps to reproduce:
- Enable workbench_access
- Create three users: /admin/people/create
- Edit the permissions for authenticated users and allow, "Allow all members of this role to be assigned to Workbench Access sections"
- Create a Tags term, /admin/structure/taxonomy/manage/tags/overview
- Create a new schema for taxonomy "Tags" and content type "Article", /admin/config/workflow/workbench_access/access_scheme/add
- Add the two example users to the example term section at /admin/config/workflow/workbench_access/example_scheme/sections/1/users
- Delete the first example user, "Delete the account and its content."
- View the list of users in the section, /admin/config/workflow/workbench_access/example_scheme/sections/1/users
- They are all gone, but only the deleted user should be gone
- Try to add the third example user to the section, /admin/config/workflow/workbench_access/example_scheme/sections/1/users
- No users are shown

With the patch in #18, the above works as expected. The PHPUnit test also passed.

I did get a deprecation notice which is a separate issue. I think this is from a base class.

Remaining self deprecation notices (1)

  1x: Drupal\Component\Plugin\ConfigurablePluginInterface is deprecated in Drupal 8.7.0 and will be removed before Drupal 9.0.0. You should implement ConfigurableInterface and/or DependentPluginInterface directly as needed. If you implement ConfigurableInterface you may choose to implement ConfigurablePluginInterface in Drupal 8 as well for maximum compatibility, however this must be removed prior to Drupal 9. See https://www.drupal.org/node/2946161
    1x in DeleteUserTest::testUserDelete from Drupal\Tests\workbench_access\Functional

+++ b/tests/src/Functional/DeleteUserTest.php
@@ -0,0 +1,83 @@
+    user_delete($editor->id());

should we use $editor->delete() here? I assume user_delete is one of those functions like entity_load that will eventually be deprecated

We should. All functions that depend on https://api.drupal.org/api/drupal/core%21includes%21entity.inc/function/... will be deprecated.

Test-only patch.

Last patch was malformed. Tests are green locally.

We would need something similar for Roles, but that can go in another issue.

Committed.