Reviewed & tested by the community
Project:
Signup Status
Version:
6.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
26 Jun 2009 at 19:10 UTC
Updated:
30 Jul 2015 at 19:40 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
dwwAt first glance, I thought this was duplicate with #406414: not being able to adminster signup statuses, but you're talking about the interface for an admin to edit another user's signup. Yeah, in that case, we should probably include all the possible status values -- I don't think we want a new permission for this. We should know that's the version of the signup form we're altering, so it should be fairly easy to somehow figure that out inside signup_status_form_signup_edit_form_alter() and then add a second optional parameter to _signup_status_status_form_element() which would default to FALSE, but if set to TRUE, include all status codes in the form element. Feel free to roll a patch if you're up for it, though I'll be offline for the next 2 weeks so I won't be able to review or commit it until after that. However, other folks know about and care for this module, so maybe someone else can step in...
Comment #2
ln282 commentedI'll try to roll a patch later, but here is how I fixed this:
I added a new status form element to the signup_status.module
and then changed the function signup_status_form_signup_edit_form_alter so that on line 269 it calls the new edit form element instead of the one for the main signup form.
$status_element = _signup_status_edit_status_form_element($signup->status);I haven't done a done a lot of testing, but it looks like it works so far.
Comment #3
dww@ln282: Thanks. However, please don't cut and paste 95% of that code into a separate function. It'd be much better to follow the path I laid out in comment #1 where there's just an argument to this function that controls if it should use the full list of status options or the list restricted by the "show on form" checkboxes.
Comment #4
ln282 commentedOk-- I actually came up with my fix before searching the issue queue, and that's why I didn't do it the way you described initially. Using an argument in the form element function for this is a whole lot better.
Anyhow, here's a patch that I think does what you had in mind.
Comment #5
dwwGreat, thanks, that's a lot better. ;) A few minor problems:
A) You want
||(logical OR) not|(bitwise OR)B) $editform is a bit wonky as a name for this argument. Something like $show_all or $show_all_options would be better.
C) You need to document the new argument in the PHPDoc comment for this function.
D) The callsite doesn't need this:
This would be fine:
Thanks!
-Derek
Comment #6
ln282 commentedAnother try at it. :)
Comment #7
stg11 commentedThis patch needs one more tweak. It needs to check for "manage signup status codes" permission before allowing all statuses to be shown to a user. That way, normal users could change their email address, phone number, etc., but not statuses set to "not shown on form".
Rather than passing TRUE in the function parameter, I just changed the line on or about line 246 from
to
This allows it to show for admins and not for "regular" users. I'm not sure this is the most efficient way to do it but it works.
Comment #8
stg11 commentedFollowup: actually what I posted in #7 doesn't quite work because the status will revert if they go in and save their signup information. For instance, if it is set to Pending when they sign up and the admin sets it to Confirmed, only Pending shows and the Pending status will show, and it will revert to Pending when they save it. So what it really needs to do is just show only the current status if they don't have "manage signup status codes" permission. To make it generally applicable it would be appropriate to have another permission "manage own signup status codes" for websites that want the module to work as it does now without the patch.
Comment #9
edmund.kwok commentedAttached patch shows all statuses for those with the SIGNUP_STATUS_MANAGE_PERMISSION permission. Users will also be able to see statuses that are not visible in the form, but has been set by admin, addressing #8.
Comment #10
jim kirkpatrick commented#9 Works for me!
Comment #11
penyaskitoI did's saw this issue when I hit the problem :(
I have another patch with a different approach, just added the permission, maybe it's worth a look.
I don't change the RTBC status, but take care that the currently reviewed patch is at #9.
Comment #12
penyaskitoIs this going to be commited?
Comment #13
tannis00 commentedWas anything ever done with this patch? I'd like to use the status to mark users who have paid for an event as paid, but I need the application of the paid status to be limited to administrators. This patch sounds like exactly what I need, but it doesn't appear to be part of even the dev version yet.