adding support to view/open closed events

after playing with this for a few more minutes, i decided i like the display/interface better with this patch (signup_open.1a.patch). i think this is a more elegant UI, so i'd prefer this patch to my previous posting. this version takes less space, and is nicer to look at...

thanks,
-derek

if we're going to put the open and close buttons with the node, they must go under the 'signups' tab, not on the main page, as it's an admin function and should go in the admin-related tab. this will also fix the permissions problem with your patch--namely that anybody at all can close and open signups--we only want people w/ admin privs to be able to do this.

that's exactly where i put the buttons. this all happens on the "signups" tab (node/X/signups), not on the main view. unless something got *seriously* messed up with the patch, this new table with the buttons shows up on the signups tab. furthermore, i don't know what you mean about priv problems... if i log out of the site, i don't get a signup tab and i therefore have no access to the buttons. if i log in as a regular user i can see the signups, but still don't have a signup tab and can't close/open events. only if i log in as an admin do i get the signups tab with the additional info/buttons.

the cron stuff in signup_open_signup is unnecessary

good to know, i was just following the signup_close_signup() code as closely as possible. i figured it'd be possible that someday we might have signups only *open* at a specific date, managed via cron...

the callbacks for opening and closing signups should not go under the signup_user_signup function--they would probably live best in the callback that generates the signups tab.

good point. again, i'm still just trying to get stuff working based on what i see, and trying to put things next to other things that already do something like what i need. i agree they should be moved, once i have a better understanding of how all this callback generation stuff really works (i gotta read more about the drupal api). if it's trivial for you to post a patch to my patch that moved them where you think they should go, that'd help me understand more about how you want the module laid out. mostly i get the principle: things should live near what they do functionally, not just near similar implementations. ;) i'll take another look at this aspect of it and see if i can find a more suitable location.

we really should make these buttons and not links

maybe it's confusing because i used a callback to implement the button, and there's some more simple way, but what i have definitely generates buttons, not links. if you apply the patch and play with it, you'll see what i mean. if you look at the part where i'm putting a value into the "ctrl_row" array, you'll see:

+    $ctrl_row[] = array( t('Signups <b>open</b> for this event'), 
+                         form(form_hidden('nid', $node->nid) .
+                            form_submit(t('Close Signups')), 'post',
+                              url('signup', drupal_get_destination())) );
+  }

it's definitely a "Close Signups" form_submit() button that posts the simple form... (with the hidden nid value so the callback knows what node this form corresponds to).

also, it would be great if you could provide a patch to put the same functionality in HEAD.

no offense, but i've got enough to worry about just trying to get my own custom version of signup working and coordinating my changes with one branch of your respository. ;) plus, i'm still working out all the other problems with other modules not quite doing what they should... my primary concern at the moment is getting my site up and running. for now, my site's running 4.6, so that's what i'm getting familiar with and actively developing. that said, signup is going to play such a central role in my site that i'm trying to share as much of my work on signup as possible... but, i can't make any promises about HEAD patches for any of the stuff i'm doing, until i end up getting ready to convert my entire site to 4.7. i'm still climbing the steep learning curve, so i'm trying to stay focused on what i most need to know. i'm sure you understand. when/if i get inspired to start looking at the HEAD/4.7 signup code, i'll see if i can crank out a similar patch, and if so, i'll definitely follow-up to this thread.

also, i wanted to get feedback on this patch before i spent more time on it, started trying to figure out how to add the new admin/signup/closed_events tab to view and re-open all closed signups, etc, etc...

thanks,
-derek

ok, i figured out what you meant about implementing the form callbacks for those buttons in the node/X/signups page itself. while i was at it, i also added a tab to admin/signups to view closed events, and the links on that page allow you to open each event. i noticed that while testing this new tab, i often had to toggle page caching on/off for changes i made to signup_menu() to be visible. :( is that normal?

anyway, i'll post a few more comments and patches with the individual changes i've made. at the end, i'll post a new, full patch that does everything (which applies cleanly to 1.1.2.24).

-derek

patch #2 -- adds the admin/signups/closed_events tab

cvs log:

added support for a "closed events" tab in /admin/signup to view all
events where the signups have been closed.  in this view of the table,
the "Operations" field contains a hotlink to "Open Event".  to share
the most possible code, i renamed "signup_admin_page()" to
"_signup_admin_helper()", made it take a $type arg (a string which is
either 'open' or 'closed'), and then signup_admin_page() and
signup_admin_closed() both call the same method with different args.
also, to make things look nicer, if the table would otherwise be
empty, instead i just print out "No (open|closed) events".

patch #3: fixes the "Open Signups" and "Close Signups" buttons to be implemented in the right place, and other cleanup of open/close methods, buttons and links to be more sane...

patch attached, cvs log follows...

fixed the confusion over the open + close signup buttons and where the
form callback was implemented.  now, in the node/X/settings page, the
buttons to open/close the event post a form back to the same page,
which a) makes more sense (thanks, chad), and b) avoids the trouble of
including a hidden form field with the nid.  however, b/c of confusion
over the signup_open_signup() method and the flow of control given the
administrative hotlinks trying to use the same methods, i split those
apart.  now, _signup_{open|close}_signup() are the underlying helpers
that do the actual work to open or close the event (the DB update).
signup_{open|close}_signup_link() are the callbacks for the admin
table hot links, and they know to send the caller back to the right
admin page.  the buttons from node/X/signups, now just call
_signup_{open|close}_signup() to do their work, and then reload the
same node/X/signups page (so the user sees the results).

here's the full, corrected (hopefully final) patch of everything i've mentioned in this thread. this takes a clean copy of 1.1.2.24 and adds all of the open signups functionality, including the buttons on node/X/signups and the new admin/signups/closed_events tab.

i guess the only other things i left out of the patch were to remove the TODO item in the comment at the top of the file (that list looks like it's fairly stale and could use some lovin', anyway), and to update the help text "Signups can be manually closed for any node at administer->signup" to mention these changes. i wasn't sure if there are any other places that documentation needs to be updated, so i figured i'd just leave all of that to you. ;) let me know if you want me to generate a patch to signup_help() and the TODO comment...

enjoy,
-derek

here's a new full patch that includes the fix to the SQL query mentioned in http://drupal.org/node/44914

whoops, the formatting you chose for the SQL fix in revision 1.1.2.25 was slightly different than what i was patching against. this patch will apply cleanly to 1.1.2.25...

clearly, signup isn't going away anytime soon, and this stuff would be useful. i'll take another look at this feature/patch, see if it still applies, and consider checking it in.

i'm not thrilled with the current implementation. for example, in admin/signup, it's silly to have "overview" and "open events" as separate tabs if they show the same thing. plus, my earlier patches added a button the node/X/signups tab to open/close signups on that node, which is a handy UI element. so, i'm still gonna dust this stuff off, and get some of these improvements into the code...

oh, and because the current implementation in admin/signup is broken: given the way the SQL is written, it will only let you open or close events that have had a signup already. :( this is a serious problem for the http://drupalcon.org site right now.

here's the patch for 4.6

patch for 4.7 and HEAD

applied to HEAD, 4.7 and 4.6.

it'd be nice if someone signup-aware could still review these patches. ;) but i know they're not any worse than the previous code (which wouldn't allow you to do anything in /admin/signups unless the node had at least 1 signup). but, it'd probably good for *someone* else to make sure they're ok. i've tested heavily, but i might be doing something stupid... ;)

thanks,
-derek