Problem/Motivation

This issue is a follow-up to #3550604: Automated Tests: Add Functional tests for classes ExtraLinks and SearchLinks where this bug was identified when testing the Admin Toolbar Tools module.

Creating, deleting or updating a user role or a view does not get reflected immediately in the Admin Toolbar's extra links.

Steps to reproduce

[Insert]
User roles:
1 - Enable the Admin Toolbar Tools module.
2 - Add a user role at admin/people/roles/add
3 - Confirm the added role does not display in the menu links.

Views:
1 - Enable the Admin Toolbar Tools and Views modules.
2 - Add a view at admin/structure/views/add
3 - Confirm the added view does not display in the menu links.

[Delete]
Repeat the same steps with deleting a user role and a view and confirm the corresponding link still appears in menu links.

[Update]
Repeat the same steps with updating a user role and a view name/label and confirm the corresponding role and view still appear with the label before it was updated.

Proposed resolution

Add the 'user_role' and 'view' entity types to the ones currently triggering a menu link rebuild on entity operations, see:
https://git.drupalcode.org/project/admin_toolbar/-/blob/3.6.2/admin_tool...

This is already the case for the 'menu' config entity type: Whatever operation is carried on menus (create, delete or update), the changes will always immediately be reflected in the Admin Toolbar's extra menu links.
 

Feel free to let us know if you have any questions or concerns on any aspects of this issue or the project in general, we would surely be glad to hear your feedback.
Thanks in advance!

Issue fork admin_toolbar-3552172

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

dydave created an issue. See original summary.

dydave opened merge request !180

dydave’s picture

dydave commented
Status: Active » Needs review

Quick follow-up on this issue:

I think it is better to isolate these changes so they could really be focused on testing the entity operations for user roles and views.

All the changes detailed in the issue summary have been implemented and described in the merge request MR !180 above at #2.

There are no tests in this merge request, since they are added in related issue #3550604: Automated Tests: Add Functional tests for classes ExtraLinks and SearchLinks.

Since all the tests and jobs still seem to be passing 🟢, moving issue to Needs review as an attempt to get more testing feedback and reviews.

Feel free to let us know if you have any comments, questions or concerns on any aspects of this issue or the suggested changes in the merge request, we would surely be glad to help.
Thanks in advance!

ressa’s picture

ressa
he/him
commented
Status: Needs review » Reviewed & tested by the community

Thanks @dydave, another step on the path of steady improvements 🙂
And that IS an epic patch, in the other issue, where this bug was found..

I could reproduce most of the missing updates, after creating and renaming Views and Roles, though some I couldn't.

For example, updating a Role label seemed to get reflected immediately. Either way, after applying the patch all Views and Role changes (Create, Update, Delete) were immediately visible in the Admin Toolbar menu items, after clicking "Save".

As a side effect, the order or Roles in Admin Toolbar is also immediately updated now, nice!

  • dydave committed abbe6b35 on 3.x 
    Issue #3552172 by dydave: Trigger a menu link rebuild for user roles and...
dydave’s picture

dydave commented
Status: Reviewed & tested by the community » Fixed

Let's go @ressa! 🥳

Thanks a lot for your feedback on all the issues, that's a really great help! 🙏

Following your confirmation above at #4, I went ahead and merged the changes above at #5 👍

I'm glad the changes worked as expected and are helping standardizing module's behavior.

Basically, this issue was immediately encountered with the admin_toolbar_tools integration Functional tests:
#3550604: Automated Tests: Add Functional tests for classes ExtraLinks and SearchLinks
The tests create 10 Views in the setUp and with the first assertions, right after logging in as an admin user, the expected links to the created Views could not be found.... failing the tests.
So while I was developing the tests, I started investigating why the links would not display... and why would the 'menu' entity links would be displayed .... which brought me to this piece of code 😅
I looked at the HTML output of the tests and indeed, the Views links were not there, so I tried reproducing manually and again.... the same bug appeared 😅
Without these integration tests, testing the integration with all the modules implementing a custom logic with Admin Toolbar Tools, it would have been very difficult to actually catch these bugs.

I've added a bunch of other checks for the delete and update operations, see:
https://git.drupalcode.org/project/admin_toolbar/-/blob/c5fc0f14545808fe...
Which should cover these functions (insert, delete, update) for all supported entities now 😎
('comment_type', 'node_type', 'media_type', 'user_role', 'view', etc...)

The idea of this ticket is to try to fix any issues with the entity types that are currently supported.
Down the road we could think of improving this logic, trying to find a more generic method for supporting menu rebuilds with entity types operations.

Marking this issue as Fixed, for now.

Let's keep working of getting the rest of the issues to land. 👌
Thanks again very much for your great help @ressa!

Now that this issue is closed, please review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, please credit people who helped resolve this issue.

ressa’s picture

ressa
he/him
commented

Fantastic @dydave, and good decision to include all supported entities, the more coverage the better!

It sometimes requires a bit of detective work to understand why "something strange" happens, but so good that you finally got to the bottom of it. Tests and step-by-step walk-throughs really are a great help.

And yes, let's continue towards completing the last of the selected issues for 3.7 🙂

Status: Fixed » Closed (fixed)

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