Closed (fixed)
Project:
Administration menu
Version:
7.x-3.x-dev
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
26 Sep 2009 at 06:01 UTC
Updated:
22 Mar 2012 at 14:40 UTC
Jump to comment: Most recent file
Comments
Comment #1
dave reidPatches for HEAD and DRUPAL-6--3 attached to add an update cache flush menu if update.module is enabled.
Comment #2
dave reidI think sun mentioned to me on IRC that modules have a way to alter these cache links, but I couldn't see how seeing as the callback switch code is hard-coded. Are modules able to alter this?
Comment #3
j0rd commentedI was told my issue is a dupe of this issue.
I would like the ability to add a "Clear APC Cache" child item into the "Flush All Caches" menu and have it called when "Flush All Caches" is pressed.
Now I can add the child menu item with hook_admin_menu and hook_menu , but I'm not able to get my stuff into the "hardcoded switch" as Dave as mentioned.
There is a way to "hack" it in using hook_flush_caches though. It's no ideal as hook_flush_caches() get's called too often by other modules, which means my APC cache is getting cleared much much more than I want. Ideally my hook would only get called manually after I update the files on my server.
Provided is my example hackish module which works to clear APC cache when it's menu item is pressed and when "Flush All Caches" is pressed.
Comment #4
sunIn light of #420816: On-demand loading of dynamic paths and local tasks #1, we want to do this a bit differently.
1) admin_menu should register the path admin-menu/flush-caches (which it currently does already)
2) In addition to that, it should register the path admin-menu/flush-caches/%
3) Implementations then use the new expand_map hook to populate the menu with further dynamic links.
Or is this bullshit? Yes, it is. Sorry.
It doesn't work, because we want to execute something different for each sub-item.
Well, then in light of #631550: Stale + improper logic for MENU_VISIBLE_IN_TREE and MENU_VISIBLE_IN_BREADCRUMB, we may want to register local tasks instead, and make them execute the actual cache clearing function directly?
meh. That wouldn't execute the final redirect in admin_menu_flush_cache().
So perhaps back to a semi-dynamic, but local tasks approach? I.e. make the page callback execute admin_menu_flush_cache(), but assign a certain callback function as first page argument?
Whatever we do here, we want to (partially or not) revamp the existing implementation to remove most of the hard-coded logic.
Comment #5
mstrelan commentedIs this still on the cards? It's been almost 2.5 years since the last update and it would make #1471312: Add "Flush LESS files" to admin_menu's "flush all caches" submenu a lot nicer.
Comment #7
sunYes, this feature request still makes a lot of sense.
We merely need to think through the problem space a bit more in-depth to find the proper solution.
However, that would not only pollute the router system; it would also require each module to define the full router item, possibly re-using admin_menu's access definition (or not). Each module would have to copy the execution logic of admin_menu_flush_cache(), which involves proper CSRF token handling as well as the final redirect...
What do you think of attached patch?
Comment #8
sunApparently, 'update' and 'admin_menu' only invoke one specific callback, so we can register that callback directly, which is a nice clean-up and simplifies things.
Thus, the _admin_menu_flush_cache() helper is only used for core caches. yay :)
Comment #9
mstrelan commentedLove it! But you missed the case for "Flush all caches". Updated patch attached.
Comment #10
mstrelan commentedAnother thought, should admin_menu provide an optional callback message and put that through drupal_set_message()? For the LESS module I am writing a patch and would prefer to just call less_flush_caches(), which intentionally doesn't set a message, but at the moment if I want to set a message I have to create a new callback function which calls less_flush_caches() and drupal_set_message().
Alternatively admin_menu could just set a default message which includes the name of the cache that was flushed. That would probably be sufficient.
Comment #11
sunComment #12
sunAttached patch incorporates #10, which required to change some of the labels to make them work in the generic dsm().
I'm happy with this patch.
Comment #13
sunThanks for reporting, reviewing, and testing! Committed attached patch to all branches.
A new development snapshot will be available within the next 12 hours. This improvement will be available in the next official release.