Problem/Motivation
While working on #3427046: Shortcuts toolbar links are not updated automatically when default shortcut set is changed, we found out that shortcut set related caches for toolbar were not working as smooth as they could.
Biggest concerns were:
- Shortcut set is using user context, that generates a cache entry per user, even if multiple users share the same shortcut set.
- When a specific user's shortcut set is modified or unassigned, it was necessary to invalidate the shortcut set cache, forcing to recreate all cache entries mentioned above.
Steps to reproduce
- Install a clean drupal standard profile
- Add multiple users with access shortcuts and access toolbar permissions
- Login with all the user created above
- Confirm that cache_render table creates an entry whose cid start with
shortcut_set_toolbar_linksper user - Create a second shortcut set and assign it to one of the users
- Confirm that a new cache has been created for the user with the new links
- Confirm that cache entries have been invalidated unnecessarily for users whose shortcut set has not been modified
Proposed resolution
Provide a new user.shortcut_set cache context that would solve the problematic scenario defined above.
Only one cache entry would be created and shared by all the users who have the same shortcut set
When a specific user changes their shortcut set, either a new cache entry would be created, or would reuse an existing one
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Issue fork drupal-3437576
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:
Comments
Comment #3
plopescComment #4
plopescMR created to try to address this shortcut caching improvement.
Comment #6
smustgrave commentedAdded some simple typehints nothing major.
I did purposely not add one to getLabel as the docs say it returns a string but translatable markup seems more correct. But definitely out of scope.
Comment #7
catchTwo questions on the MR.
Comment #8
catchI opened #3439016: Replace the shortcut_set_users table with a base field on users, if we had that, we wouldn't need the extra query, we could look up the shortcut set from the user object which we'll have anyway. I think this probably needs to be postponed on that issue.
Comment #10
quietone commentedThe Shortcut Module was approved for removal in #3476880: [Policy] Move Shortcut module to contrib.
This is Postponed. The status is set according to two policies. The Remove a core extension and move it to a contributed project and the Extensions approved for removal policies.
The deprecation work is in #3569117: [meta] Tasks to deprecate the Shortcut module and the removal work in #3569121: [meta] Tasks to remove the Shortcut module.
Shortcut will be moved to a contributed project before Drupal 12.0.0 is released.
Comment #11
smustgrave commented