Created on 25 September 2023, 9 months ago
Updated 24 January 2024, 5 months ago

Problem/Motivation

As per #3194052 β†’ to get stable release there needs to be tests. This includes

  • Manage toolbar has the correct menu links
  • That content admin and user admin role have the appropriate permission
  • Content admin views are created for each content type
πŸ“Œ Task
Status

Needs review

Version

1.0

Component

Code

Created by

πŸ‡¬πŸ‡§United Kingdom andybroomfield

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @andybroomfield
  • Merge request !2Issue #3389673: Add test suite β†’ (Open) created by andybroomfield
  • πŸ‡¬πŸ‡§United Kingdom andybroomfield

    This also builds on current dev work so should now install on Drupal 10.

  • Pipeline finished with Failed
    9 months ago
    #23559
  • Pipeline finished with Failed
    9 months ago
    #23567
  • Pipeline finished with Failed
    9 months ago
    #23674
  • Pipeline finished with Success
    9 months ago
    #23894
  • Pipeline finished with Success
    9 months ago
    #23900
  • Status changed to Needs review 9 months ago
  • πŸ‡¬πŸ‡§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.

  • Pipeline finished with Success
    5 months ago
    #81612
  • πŸ‡¬πŸ‡§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.

Production build 0.69.0 2024