- Issue created by @andybroomfield
- 🇬🇧United Kingdom andybroomfield
This also builds on current dev work so should now install on Drupal 10.
- Status changed to Needs review
about 1 year ago 5:10pm 26 September 2023 - 🇬🇧United Kingdom andybroomfield
Tests are now passing on Drupal CI.
There are still linter and coding standards warnings, though I suggest that is handled in a new issue. - 🇬🇧United Kingdom jeni_dc
Thanks for your work on this, Andy! All the tests look good, but I'm not sure we should include the changes to the .install file as a part of this. In particular, this commit:
https://git.drupalcode.org/project/tasty_backend/-/merge_requests/2/diff...
Is there a specific reason these changes were included along with the tests? I'm aware of possible issues in the module in deployment versus initial install, but I think there's going to be other things that need addressing properly related to that and I don't want these changes lost in this issue for adding the tests.
In particular, this added section:
// Add role assign roles to user manager. $roles = \Drupal::entityTypeManager()->getStorage('user_role')->loadMultiple(); foreach ($roles as $role) { $role_id = $role->id(); if ($role_id == 'content_admin' || $role_id == 'user_admin') { user_role_grant_permissions('user_admin', [ 'assign ' . $role_id . ' role' ]); } }
It re-adds the 'assign content_admin role' permission to the user admin role that's already included in the user.role.user_admin.yml config file.
I had addressed an issue related to assigning the User Admin role the 'assign user_admin role' permission (not documented in this issue queue yet) in this commit, which removes that permission from the user.role.user_admin.yml file and re-adds it during a hook_ENTITY_TYPE_insert.
https://git.drupalcode.org/project/tasty_backend/-/commit/f254753f15b03f...
That issue is documented in the Role Delegation issue queue at 🐛 Adding non-existent permissions to a role is not allowed Active
So I think your addition to the hook_install here might duplicate what's already in place from my previous commit to make sure that permission gets added to the user admin role when the role is created. That needs to happen not only during module install but also deployment since IIRC having that permission in the role config during a deploy will trigger the same error. So we'll need a separate issue to manage that. If there's something specific related to the tests and that please let me know.
- 🇬🇧United Kingdom andybroomfield
I've pushed a rebased branch that removes the .install modifications.
The module still installs on Drupal 10 and tests are passing so this is ready for review again.