Problem/Motivation

Right now, wse_menu.active_trail decorates menu.active_trail. But, WseMenuActiveTrail extends MenuActiveTrail, meaning that the decoration breaks if any other decorator is added.

Steps to reproduce

Proposed resolution

Make WseMenuActiveTrail implement MenuActiveTrailInterface without extending the base class. This allows for proper decoration.

Final resolution

Stop decorating MenuActiveTrail and decorate cache.menu instead, since the only reason for dectorating menu.active_trail was to add the active workspace ID to the cache ID, which we can more easily do with cache.menu.

This has the side-effect of segmenting the cache for menu.tree_storage also, which seems to be the right thing to do there as well.

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork wse-3559780

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

alecsmrekar created an issue. See original summary.

alecsmrekar opened merge request !121

alecsmrekar’s picture

alecsmrekar commented

I posted a PR, and here are some test steps for it:

  1. Add this to your local settings $settings['cache']['bins']['menu'] = 'cache.backend.database';
  2. ddev drush si standard -y
  3. ddev drush uli
  4. create a basic page node, add check "Provide a menu link"
  5. ddev drush en -y wse_menu workspaces_ui
  6. add and activate a new WS
  7. visit the homepage and the new basic page
  8. ddev drush sqlq "select * from cache_menu cm where cid like '%:active%'"
  9. ensure you see a result, and the should have the MS UUID in front
alecsmrekar’s picture

alecsmrekar commented
Assigned: alecsmrekar » Unassigned
Issue summary: View changes
Status: Needs work » Needs review
alecsmrekar’s picture

alecsmrekar commented
Assigned: Unassigned » alecsmrekar
Status: Needs review » Needs work
alecsmrekar’s picture

alecsmrekar commented
Assigned: alecsmrekar » Unassigned
Status: Needs work » Needs review

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

gabesullice opened merge request !122

alecsmrekar closed merge request !121

alecsmrekar’s picture

alecsmrekar commented

This is a much better solution, thanks Gabe!

alecsmrekar’s picture

alecsmrekar commented
Status: Needs review » Reviewed & tested by the community
amateescu’s picture

amateescu commented
Title: Implement proper decoration for WseMenuActiveTrail » Decorate the cache.menu service instead of menu.active_trail
Component: Code » Workspace Menu

Updating title based on the new approach.

amateescu’s picture

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

Merged, thanks!

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

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

Maintainers, credit people who helped resolve this issue.

gabesullice’s picture

gabesullice
Primary language English
commented
Issue summary: View changes

Status: Fixed » Closed (fixed)

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