Category navigation missing links
| Project: | Category |
| Version: | 6.x-2.0-rc1 |
| Component: | Category_menu |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | fixed |
There are missing navigation links for children categories when toc is set to 0 depth for the container holding the categories.
The problem is in the (category/category_menu/category_menu.module) - category_menu_tree_all_data function line 1019:
category_cache_op('set', 0, $cache_key, $tree);
---
It cache the tree for the first selected category after the cache is flushed so it always use the tree retrieved for that category.
I managed to fix this and it works well for now but I'm not sure if it will cause side effects in future. I have not enough time to study the _category_menu_tree_all_data and category_menu_tree_all_data functions.
In "category/category_display/category_display.module" on line 546:
$tree = category_menu_tree_all_data($book_link_orig['menu_name'], $book_link, $container_info->toc_depth);
I make a call to _category_menu_tree_all_data instead so the tree is not cached
$tree = _category_menu_tree_all_data($book_link_orig['menu_name'], $book_link, NULL, $container_info->toc_depth);
I tested the both functions have the same return on all my tests so what is the difference between them?
Any advices are welcome!
#1
No time to look into code now, but if there's some additional context (like trees being different for various categories), it should be added to $cache_key (like
$cache_key .= ':'. $cidor similar), so that we get separate cache for each version. If that means a different tree for every single category, we might even use its nid instead of the general-data zero key. The idea of cache keys is to provide all necessary context, so that we always hit the right data in cache, and nothing else.I admit, that the tree building code is really difficult to analyze, so while I was confident about all the context being included to the cache key, I can't be 100% sure. We need to check this bit, but I can't promise anything now.
#2
I've found it - it was a stupid typo introduced by myself (my huge apologies!), during that night-of-utter-madness when I converted all the expensive category functions to the new caching pattern. Unfortunately, the problem never shows on my site (because I have TOC depth set to Infinite, so I have all the data in the tree all the time), and I didn't spot it during my manual testing either.
All the same, the problem was indeed with the cache key - it was supposed to include ID of the menu link, for which that particular cached tree was built, but due to that typo only an empty string was included, making all the cached trees "match" each other in the cache.
I believe (and I tested) that the attached patch fixes it.
#3
I also had an issue and generated the exact same patch in #598196: Incorrect cache_key generation results in incorrect active trails, so this is fine by me!
#4
Thanks, JirkaRybka. Committed to HEAD.