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

  1. Install a clean drupal standard profile
  2. Add multiple users with access shortcuts and access toolbar permissions
  3. Login with all the user created above
  4. Confirm that cache_render table creates an entry whose cid start with shortcut_set_toolbar_links per user
  5. Create a second shortcut set and assign it to one of the users
  6. Confirm that a new cache has been created for the user with the new links
  7. 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

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:

Comments

plopesc created an issue. See original summary.

plopesc’s picture

plopesc’s picture

Issue summary: View changes
Status: Active » Needs review

MR created to try to address this shortcut caching improvement.

smustgrave made their first commit to this issue’s fork.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

Added 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.

catch’s picture

Status: Reviewed & tested by the community » Needs work

Two questions on the MR.

catch’s picture

I 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.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

quietone’s picture

Status: Needs work » Postponed

The 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.

smustgrave’s picture

Project: Drupal core » Shortcut (from core)
Version: main » 2.x-dev
Component: shortcut.module » Code
Status: Postponed » Needs work