New hook to allow modules to add node and user toggle options
| Project: | Fasttoggle |
| Version: | 5.x-1.x-dev |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
Konstantin, excellent work on this module. You've shown how to solve elegantly some tricky security and usability issues.
I'm writing a module where I'd like to use fasttoggle to introduce new toggleable options for nodes, e.g., 'block content from [username]'. This differs a bit from the implementation in fasttoggle, in that I won't be toggling a core node setting, but it can work the same. That is, e.g., we set $node->author_is_blocked to 0 or 1 and then respond to that node property on node save (called by fasttoggle_node_option()).
What I'd love to see is a way to tap into fasttoggle to allow other modules to add options. Hence the attached patch. I've introduced a new hook_fasttoggle_options(). Thinking ahead to comments, it seems to make sense to have a single hook for all object types (nodes, users, comments), rather than separate ones for each. So I've taken the time to rework the user implementation to load data in the same way as nodes do, so we have a parallel approach that can be easily extended to comments (or other object types).
This has the potential benefit of opening the way for other toggleable user options as well as node ones. Is it useful to be able to add togglable links to users? We don't currently have a way to display these - there is no 'user' type in hook_link() - but I suppose one could be introduced, e.g., as links on the user profile page.
| Attachment | Size |
|---|---|
| fasttoggle-hook.patch | 5.6 KB |
#1
Hmm, the node toggling menu callback currently requires 'administer nodes' permission, which would make my patch above fairly pointless. The idea is to be able to add general toggleable options, and limiting them to users with 'administrate nodes' permisions would pretty much defeat the purpose.
Here's a revised patch. We test permissions for each toggle option we add. It seems to me that this doesn't open new security holes, but some review is needed. Is this the right approach? Are there better ones?
#2
I'd forgotten to update the
hook_link()implementation. We need to apply the same logic as elsewhere--test for available node toggle options, then iterate through them. That way all available toggle links will appear.#3
Thank you! That really makes sense and will simplify integration of other modules with fasttoggle. I reviewed the patch, fixed some quirks (omitted break; resulted in strange behavior for users without "administer nodes" permission, ...) and added some docs. Committed to DRUPAL-5.
#4
Nice! Your refinements to the patch make good sense. It's great to see as well (in http://drupal.org/node/112779) the new support for comments and nuanced permissions. This module is going places.
I have further small suggested change. We need a way to limit the nodes (or comments) that a link attaches to. All nodes have a 'status' property, but this may not be true for custom properties we wish to toggle. For example, in the module I'm trying to use fasttoggle for, I want to be able to add a toggleable setting only to a certain node type. At present, the any options returned by
hook_fasttoggle_options()will attach to all nodes (or all comments).The solution is simple: before adding a link to a node (or comment), we test to ensure the object has the applicable property. This gives us pretty full control over attaching links to nodes (or comments). Don't want the link? Easy, just don't set the property.
Patch attached.
#5
When I went to bed yesterday, I also thought about that problem, however I came up with a different solution (not yet implemented): We could call
fasttoggle_get_options()with the actual object as a parameter (we already load it infasttoggle_menu()anyway) so that the module can decide directly in the hook if it wants to add the link.This would also mean that the results from
fasttoggle_get_options()can’t be cached without some further thought. We would need a way to either a) pass around the resulting array so that other methods don’t have to call it, b) invent some other tricky caching mechanism or c) don’t cache it at all.#6
Yes, your proposed approach sounds better than what I did and has potential added advantages, e.g., by passing an object to
hook_fasttoggle_options()we modules could customize the toggle messages, e.g., for nodes, it could be'block content by '. $obj->name. But, yes, it's hard to see how we would cache the resulting options, as they could differ from node to node.Here's a draft patch that makes the change and drops caching.
#7
Changing status.
#8
I posted an intial version of Content Blocker module today. It uses Fast Toggle to enable users to selectively block content, e.g., by user. (Link below node says "block content by userx"). I needed to be able to limit the blocking/filtering to given node types so I've written it to be dependent on the patch above at http://drupal.org/node/136183#comment-229431.
#9
Committed to DRUPAL-5! Thanks for the patch. I converted the
fasttoggle_get_options()to take an arbitrary amount of parameters instead of just one.#10