Allow both event and CCK date to work simultaneously

This is only lightly tested, and there are some parts that still aren't handled right, but I wanted to post the work-in-progress for initial feedback on the approach. To summarize:

- Adopted "scheduler" as the terminology for the "event backend", to avoid using "event" to mean anything other than "event module"

- Added a new includes/scheduler.inc file for code related to deciding which scheduler to use for a given node or type, and for all the "switchboard" functions.

- Replaced signup_node_has_date() with _signup_get_node_scheduler(), and signup_node_type_has_date() with _signup_get_node_type_scheduler(). Both of the new functions return 'none', 'date', or 'event', so you know not just if it's TRUE/FALSE, but if it's TRUE, which scheduler to use for that node type.

- Moved signup_site_has_dates() into includes/scheduler.inc, so we no longer should need includes/event.none.inc at all.

- Prefixed all the scheduler backend functions in includes/(event|date).* to use _signup_(event|date) so that there's no function conflicts if you include more than one scheduler backend at once.

TODO:

The crazy query builder stuff doesn't actually all work with both date and event enabled. ;) I think the scheduler-specific queries for event are written such that they assume {event} is the only table in town.

A) For autoclose and reminder email, I think it might be cleaner to just build and execute separate queries for each node type, instead of trying to munge it all together into a single query.

B) The user signup listing at user/N (if you don't use views) hasn't been ported to the new system at all.

C) Not sure what to do about the signup admin overview page (admin/content/signup). Currently it gets confused about what value to put in that first column, and everything is coming out [Untimed].

New patch:

A) Mostly fixed -- we now just do separate queries for each content type. However, I'm getting duplicates in my testing.

B) Haven't started this.

C) Mostly fixed. See #328295: event_join() ignores $type argument ;) The only bummer is that since the start time column could now potentially be coming from many different tables, I don't know how to get tablesort_sql() to sort on that. We had this problem with the date support in the first place. So, this is a minor regression for sites using event, since they'd no longer be able to sort by start time. Not sure what to do about this, if anything.

While I was at it, I fixed up the API to the scheduling backends to just do everything based on node type, and CCK can do the somewhat funky field stuff it wanted. Previously, it was a weird mix of passing around signup date fields or node types, and that led to some bugs/confusion. I think it's a lot cleaner this way.

I just had a few more minutes to think about this between other commitments. New patch fixes (C) for sites that don't have date at all by knowing that if date isn't enabled, we can sort on e.event_start on the admin page. We could probably be smarter here and even allow sorting in the date case if there's only 1 signup-enabled content type with date fields, but that's for another day. ;) So, I think (C) is done.

All that remains is debugging the duplicate reminder + autoclose stuff (probably an easy SQL bug or something) and figuring out what, if anything, to do about (B).

Just got back and spent a little more time on this. The current patch fixes the last of the known problems with autoclose and reminder emails. Since we're doing separate queries for each signup enabled node type, I rewrote the queries a bit so that they always include a WHERE n.type = '%s' and added the type name as a query argument. Also made sure we always select n.type in our queries, so that _signup_get_node_scheduler() always gets something with a 'type' member, even if it's not a fully loaded node object.

So, all that remains is (B). I'm not sure it's worth spending much time on, since that code only appears if you're not using views, and I'm planning to make views a requirement in the near future anyway. But, I think I'll spend a few minutes right now and see if I can come up with something relatively quick and easy. Stay tuned.

p.s. I might as well assign myself to this issue, I'm clearly working on it. ;)

To solve (B), I decided to massively simplify the code by introducing a minor regression in functionality for the poor fools who aren't running views: the nasty hard-coded signup schedule on the user profile is no longer sorted by the start time on event or CCK date nodes. Instead, we just sort on the nid, which is probably a pretty close approximation to sorting by start time, anyway, and that requires no help from the scheduler backend(s). Similar to the signup admin form, I'm not even sure *how* you'd sort across multiple node types with start times from different tables. Yuck.

Also, the start time is no longer displayed at the signup schedule on user/N/signups. There are 2 possible solutions for sites that want this data:

1) Turn on views module and customize the default views for this however you want.

2) Implement your own version of theme_signup_user_schedule() and display whatever you want (since on that page, each node is already fully loaded).

I think this is acceptable, and it really simplifies things (removes a few functions from the scheduler API, etc). Note that the existing code isn't fully implemented, anyway (some of the functions were missing from event.5x-2.inc for example).

Therefore, all the known problems from this issue have been addressed, so this is now ready for review. Note, the patch is huge, but I don't see any way to split it up any smaller than this. Furthermore, it actually rips out more code (481 lines) than it introduces (385 lines), and IMHO the code makes more sense and is easier to understand, especially for the scheduler backend stuff.

Whoops, I left a switchboard version of signup_list_user_signups_sql() in includes/scheduler.inc in the last patch. Now gone.

Fixed an unfinished code comment.

@shane_jordan: Thanks for looking at the code. However, I didn't (intend to) write the code how you describe. Don't be confused by the return value from _signup_get_node_scheduler() and _signup_get_node_type_scheduler(). Those indeed return 'event' vs. 'date' (or 'none'), but if you look at how they're implemented, you'll see that they handle CCK node types called 'event' that have CCK date fields and are signup-enabled...

That said, it's possible I missed a spot. Can you post any snippets from the patch that makes you think it works the way you described in your comment?

Thanks,
-Derek

Right, you're reading it wrong. ;) _signup_get_node_type_scheduler() tells you the scheduler to use for a given node type:

/**
 * Determine which scheduler backend to use for the given node type.
 *
 * @param $type
 *   Machine-readable name of the node type to test.
 *
 * @return
 *   'event' to use the event.module, 'date' for the date.module, or 'none' if
 *   the node type is untimed.
 */
function _signup_get_node_type_scheduler($type) {
  if (module_exists('event') && variable_get('event_nodeapi_'. $type, 'never') != 'never') {
    return 'event';
  }
  if (module_exists('date')) {
    $date_field = variable_get('signup_date_field_'. $type, 0);
    if (!empty($date_field) && $date_field != 'none') {
      return 'date';
    }
  }
  return 'none';
}

In english: "which backend scheduler system should I use for a node type called 'event'?", the answer can be 'event' (event.module), 'date' (date.module), or 'none' (this node doesn't have time, at least not as far as signup.module is concerned).

It does mean that if you have event.module on your site, and you have a node type that has both event fields and CCK date fields, signup.module assumes you're using the event fields for the purpose of signup. I think that's a reasonable assumption in the interest of trying not to overly complicate the code or the UI.

Decided no one's actually going to carefully review this entire patch. ;) So, I committed to HEAD. If interested parties could test HEAD as much as possible in various combinations of event vs. date vs. nothing and views or not, it'd be lovely. If you find a problem, please just open a new issue about it. Thanks.

"I am one of those people using a node called 'event' for my dates ... So there are logical reasons for that."

Absolutely. The last thing I would do is hard-code anything based on the name of the node type. ;) See #328647: Purge "event" from the UI (and code) unless we're talking about event.module in fact, about fixing places in the user-facing text where the code used to use 'event' and now uses the human-readable node type name.

Whoops, this introduced a bug I just noticed. If you visit admin/content/signup and set the signup status filter to either 'Open' or 'Closed', you get a bunch of PHP errors from $where not being defined. Committed the fix to HEAD and DRUPAL-5--2:
http://drupal.org/cvs?commit=151224
http://drupal.org/cvs?commit=151225