Allowing Signup to use Date.module part 1

Would be nice to get a unified diff. see diffandpatch

ugh, yeah, that patch was rather tricky to use: not only did it not specify the file to patch, it was actually reversed from what you intended... here's a new version that is proper (see http://drupal.org/patch) for more, as moshe suggested). haven't tested this or looked closely yet, but wanted to put a real patch up here in case anyone else wants to play along at home.

1) what about sites that have both event and date enabled?

2) i don't like how this patch duplicates a bunch of code between the event and date cases. it looks like the split in the code is at the wrong place. instead of a completely separate else if around the whole thing, we need a much different approach: a split to conditionally iterate over the nodes that need a reminder email based on what date-aware module(s) are installed, but inside the loops, we call a shared function to actually generate all the reminder emails for a given event. similarly, we might need N different queries to find all the nodes that are closed, but in each case, we should call a shared helper method to actually do all the work. in fact, that method should just be signup_close_signup(), which should be fixed to remove the silly special-cases for cron, and let that single method do the watchdog() logging and invoking hook_signup_close(), etc.

3) what about node types that have more than 1 date field? how do you know which one to use for the purposes of closing the event? i've always thought we needed another setting on the node type admin form for this. if a node type has any CCK date fields, you have to select which one you want to use for the purposes of signup-related functionality. as it is now, your patch loops over *all* possible date fields for every possible node, and if *any* of them point to a time in the past, the event is closed. imagine a site that added a CCK date field for the date the gig was booked, the start time of the gig itself, and the end time. your code would close signups for all gigs as soon as they were booked. :(

4) even though you moved it out of the case for event.module, your date.module case still duplicates the code to initialize $closing_time.

5) the patch has some formatting/code-style/white-space issues. please see http://drupal.org/coding-standards

FYI: see killes's efforts for this over at http://drupal.org/node/154580

I just committed a patch over at http://drupal.org/node/154580 that lays an excellent foundation for date.module support. In fact, now it'll be easier to just implement complete date.module support all at once by adding the right signup_date.inc file. So, there's no reason to keep this issue separate from http://drupal.org/node/86462 anymore.

Hope you're interested in working on signup_date.inc over at http://drupal.org/node/86462.

Thanks!
-Derek