Closed (fixed)
Project:
Signup
Version:
6.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Reporter:
Created:
24 Jul 2009 at 04:29 UTC
Updated:
8 Aug 2009 at 21:30 UTC
Jump to comment: Most recent file
A client wanted an easy way to conditionally override access to various signup node tabs. Although it'd sort of be possible to do this just with hook_menu_alter(), there's a problem in that various spots in the signup code invoke the _signup_menu_access() helper outside of the menu system to check access and to ensure proper logic of what's rendered and how. So really, the simplest, most complete approach is to invoke a hook inside _signup_menu_access(), sort of like node.module does inside node_access(). If the hook returns a value, use it. Otherwise, fall through and do the regular, hard-coded logic.
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | 529478-5.hook_signup_menu_access.patch | 7.21 KB | dww |
| #3 | 529478-3.hook_signup_menu_access.patch | 3.68 KB | dww |
| #1 | 529478-hook_signup_menu_access.patch | 3.76 KB | dww |
Comments
Comment #1
dwwComment #2
mlsamuelson commentedThe patch applies cleanly.
I wrote a test implementation of the hook and it functioned as expected, allowing me to override access to the signup node tab.
I think I'm going to love this feature.
Comment #3
dwwIn IRC, I asked greggles if this logical AND behavior was the right approach. It's certainly more secure. However, it's inconsistent with core, which (for better or worse), handles stuff like this via logical OR. So, here's a new patch that uses OR logic. There might be a smarter/faster way to OR an array than
in_array(TRUE, $foo)but I'm not sure what it is. ;) Even though generally I think FALSE should take precedence, I agree that consistency with core is a worthy goal.Another thought -- should I add an optional 3rd argument to _signup_menu_access() that controls if the hook is invoked? I was wondering if a hook implementation might want to be able to see what signup would return on its own when deciding what value the hook implementation should return. Can anyone think of a use-case for that? Or, would that just be bloat?
Thanks,
-Derek
Comment #4
dwwp.s. In testing, I found one slightly annoying case with this hook. If you're attempting to override the 'list' value, and you're using the built-in signup listing, there's a spot where we still just manually test if you've got admin access or if you've got the 'view all signup' permissions. So, if you have a hook that returns TRUE for 'list', you get the tab, but it's empty. :( Do we care?
Comment #5
dwwPatch that fixes #4. Pretty sure this is now golden. Any final reviews/complaints?
Comment #6
dwwCommitted to HEAD and DRUPAL-6--1. This will be out in RC4.