Closed (fixed)
Project:
Navigation
Version:
1.x-dev
Component:
Code
Priority:
Critical
Category:
Bug report
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
29 Mar 2024 at 08:02 UTC
Updated:
15 Apr 2024 at 13:24 UTC
Jump to comment: Most recent
Comments
Comment #2
plopescComment #4
plopescAs part of the MR creation, commented the line suggested by @m4olivei in #3426576: Add tests for UserNavigationBlock.
It lead to some broken tests.
Debugged it a it and found the issue in
NavigationBlockCacheTest::testCachePermissionsCache tags before the change (Test pass):
config:block_list config:navigation.navigation_block.administration_menu config:navigation.navigation_block.content_menu config:navigation.navigation_block.oxaw6wfu config:navigation.navigation_block.user config:navigation.settings config:system.menu.admin http_response navigation_block_view rendered user:2 user_viewCache tags after the change (Test fails):
config:block_list config:navigation.navigation_block.administration_menu config:navigation.navigation_block.content_menu config:navigation.navigation_block.user config:navigation.settings config:system.menu.admin http_response navigation_block_view rendered user:2 user_viewI'm missing the cache tag related to the test navigation block added during
setup().My feeling is that reason behind failing test is that navigation bar cache tags are not properly invalidated or refreshed when a new navigation block is added.
More digging in this one will be necessary. I'll try to continue with this next if anybody finds the root cause before.
Comment #5
plopescMR created that replaces the problematic user cache context with user.permissions to make the navigation bar cacheable.
As a secondary effect, we found that cache tags were not being implemented properly because navigation bar was not including automatically new navigation blocks added.
Root cause seemed to be that navigation block list cache tag (
config:navigation_block_list) was not included in the bar itself.Once the cache tag is added, tests are back to green and 'X-Drupal-Dynamic-Cache' header is not 'UNCACHEABLE' anymore.
I believe this is ready for review now!
Comment #6
m4oliveiGreat work!
That all makes logical sense to me. Ran through some tests on Tugboat, and it appears to be working well as well.
Comment #8
m4oliveiThanks again @plopesc.
Merged to 1.x