CCK datestamp fields totally broken and fields without timezone handling mostly broken

The trouble disabling signup-enabled nodes was already reported and fixed here: #335190: Disable signup on node leaves signup enabled and is mentioned in the "Known bugs" section of the 6.x-1.0-rc1 release notes.

The other part of your problem, where the signups are closed as soon as you edit, I can't reproduce at all, with either 6.x-1.0-rc1 nor the latest 6.x-1.x-dev of signup. So, I'm calling this a support request until we can confirm it's actually a bug, not a misconfiguration of some kind.

- Are you using event.module or CCK date?
- What exact version of those are you using? (e.g. the first line from the file, something like: // $Id: event.module,v 1.389 2008/11/11 12:05:26 killes Exp $
- Can you show exactly what the settings are like on your admin/content/node-type/event page?
- Do you have any other contrib modules enabled? If so, which ones and what version?
- Do your problems go away if you upgrade to the latest signup 6.x-1.x-dev release?

Cheers,
-Derek

I am seeing this behavior, as well. I'm using CVS checkout of 6.x-dev (updated about 5 mins ago). This configuration is using CCK Date, with a to-from date field as the defined "Date field to use with signup."

Still can't reproduce this with CCK date. @jrbeeman: Can you provide detailed steps and exact configuration you're using?

The date field has the following configuration:
- Name: "Start / end dates"
- Saved as Datestamp
- Required
- To date: required
- Granularity: All except 'second"
- Time zone handling: User's

The content type has the following config:
- Signup options: Enabled (on by default)
- Date field to use with signup: "Start / end dates"

In the test node, I have set the "Start / end dates" to:
- Start: November 27, 2008 08:00
- End: November 27, 2008 09:00

Relevant signup settings for the node are:
- Enabled
- Signup limit: 0

Upon node submission, the message "Course start time is already past the signup close-in-advance time, signups now closed." is presented. I can go to the "Signups" tab and re-open signups, but upon node submission, the same message is presented and signups are closed again.

Excellent, I can now reproduce this. ;)

Lemme see if I can narrow this down to exactly what's causing the trouble. I'll bet $20 it's the timezone handling.

I owe myself $20. ;) The problem is that it's a datestamp, as opposed to a "date" or "datetime" field. Pretty sure I see the bug now, stay tuned...

Try this on for size. It might still be broken with various timezone settings, but it's certainly closer. ;)

Fixed codestyle bug in the previous patch, and backported (I think) to DRUPAL-5--2 branch. Timezones still aren't tested.

Patch works for me against Drupal 6!

Also note: this bug effects nodes using a datestamp field for the signup start time that also have a signup limit. If you're at the limit, and either a user cancels or a signup admin increases the limit, signups still won't open, since signup thinks the node has already started...

Anyway, I just tried testing various timezone options here. Either I'm doing something wrong, this patch doesn't handle timezones how it should, I misunderstand the timezone-related options for date fields, there are bugs in date module and friends, or some combination of the above... ;)

Lots of debug printing later, it seems like the logic is working correctly here assuming that everything is stored in the DB as UTC time. I tried a datestamp field with every timezone option I could. In my testing, I found:

Site's timezone

Doesn't work. It looks like it actually stores the event via the user's TZ, not the currently configured site TZ. But, the timestamp is a UTC time, so the logic in the patch would be right, if it stored the right UTC time based on the site TZ, not the user's TZ. This seems like a date* bug, not a problem with this patch.

Date's timezone

Works as expected. The date is actually stored in UTC with a display offset. So, if you say "2:00am US/Pacific", it actually stores it as 10:00am UTC, with an offset. The display is right, and the logic in this patch is right.

User's timezone

Works as expected. The date is actually stored in UTC with a display offset. So, if your user's timezone is "America/Los Angeles" and create an event at 2:00am, it actually stores it as 10:00am UTC, with an offset. The display is right, and the logic in this patch is right.

UTC

Obviously, it stores and appears as UTC times, and the logic in the patch works.

No timezone conversion

Doesn't seem to work. It looks like it's stored as if the current time is UTC. Maybe that's what's intended with this setting. But, it breaks the logic in the patch, since we're using UTC for the current time, not the current local time (or whatever current time that was used when the event was created). Not sure how to handle this one.

Also, I found that all three of these return the same value on my test site:

time()
date('U')
gmdate('U')

Is that just because my machine is configured to think it's at UTC with a reasonable timezone offset? Can this always be assumed, or is it safer to use gmdate('U') to get the current UTC time, instead of assuming time() gives you that? The PHP docs for time() suggest it might be skewed by a timezone. Seems like date.module and date_api.module just use time() for this, so I'm assuming that's safe, but I wanted to ask just in case. ;)

@alex.k: Are you saying that if cron is running regularly on your site, you don't see it auto-close datestamp events that have already reached the "Close-in-advance" window? I.e. if the site is configured to auto-close 1 hour in advance (the default), and you have an event set start at 12pm UTC, and it's 11:15 UTC when cron runs, the event stays open? If so, that's another (different, mostly unrelated) bug.

Or, are you saying you're not seeing this bug (signups closing way too early) with cron + datestamp? If so, that's to be expected. There are different functions responsible for finding the nodes that should be autoclosed by cron (since we select them with specially crafted SQL in 1 big query) than the broken code fixed by this patch (which just tries to answer the question: "has this node already entered the period when signups should be closed?").

Heh, upon closer inspection, testing, and understanding of DateAPI, I see that datestamp fields are in fact completely broken with signup's current code. ;) Auto-close, reminder emails, *and* the stuff previously discussed in this patch are all busted for datestamp fields. I believe the attached patch solves it all. Testing and reviews (especially by KarenS) welcome. ;)

Argh. Many hours of testing, debugging, reading DateAPI code, etc later, and I've sort of got this working, but not quite. :( Dates and timezones are the work of the devil, no question...

I tried using the DateAPI as much as possible, and I think it's making things worse. :( Here are the current known bugs with these patches:

A) In date.5x-1.inc, auto-close, reminder, and _signup_date_node_completed() all appear to now be working, for both date and datestamp fields. The only known problem is that when dates are displayed on the signup administration page (admin/content/signup) or in the %node_start_time email token, they always appear to be rendered in UTC timezone. I tried, and failed, to understand how date 5.x-1.* handles timezones.

B) In date.5x-2.inc, _signup_date_node_completed() works for both date and datestamp fields. Auto-close is now working with datestamp fields, but I seem to have broken date fields. :( However, reminder emails are working for date fields, but broken for datestamp(!). Joy. ;) Inside DateAPI, there are bizzaro work-arounds for bugs in views that break things for other people trying to use the date_sql_handler class. Even trying to work-around the work-arounds, it's still not working. :( I think instead of using that class, I'd be better off going back to manually constructing the SQL myself, with proper care for date vs. datestamp fields and the rest of it. I'm going to assume that CCK date fields are always stored in the DB with UTC time, and then optionally with a TZ offset for how it should be displayed. Not sure if that assumption is always right, but it appears to be right given my testing.

C) In D6, both _signup_date_node_completed() and auto-close appear to work for both date and datestamp fields. However, reminder emails aren't being sent for either one.

It's late and I'm getting frustrated with this. I *really* need to talk to KarenS to make sense of this mess now.

With a bunch of help from KarenS, and a few hours put into making the CCK Date Testsite install profile, this patch is a lot closer. ;) _signup_date_node_completed() is working right for *ALL* configurations (the original bug reported here). Yay! Auto-close and reminder emails mostly work, with the following known bugs:

A) Date fields with no TZ handling ("date_tz_none" from the profile) don't send reminder emails.

B) Date fields with no TZ handling ("date_tz_none") auto-close events prematurely. From the profile, the 2 hours in advance node closed, but not the 2 days in advance. Probably some kind of timezone confusion.

C) Datestamp fields never send reminders at all, regardless of the timezone handling. :( I have a theory about this, but I need to confirm with KarenS about it.

D) Datestamp fields with no TZ handling ("datestamp_tz_none") auto-close some nodes prematurely. Again, 2 hours in advance closed, but the 2 day in advance was still open.

E) Datetime fields with no TZ handling ("datetime_tz_none") don't send reminder emails.

F) Datetime fields with no TZ handling ("datetime_tz_none") auto-close the 2 hour-in-advance nodes early.

So long as you configure *any* TZ handling at all, everything works except reminder emails for datestamp fields. No TZ handling on the fields breaks reminder emails and nodes tend to auto-close before they should.

It's possible that some of these combinations work if they're the *only* node type on the site. I think one of the bugs I found was how DateAPI is written to assume it's only got a single date_api_sql handler class talking to the DB at once, and given how cron works with signup, that's not always true. So, the next step would be to take the install profile mentioned above, and try a few different times where you remove content_types/*.inc except for a single node type you want to test (e.g. datetime_tz_none.inc) and see if it's still broken even if that's the only date field type on the site.

I also need to backport all this to D5, though luckily dateapi 5.x-2.* and dateapi 6.x-2.* are nearly identical (according to KarenS), so hopefully it'll be easy to do that. Signup's support for date 5.x-1.* is probably going to have to die at this point. I guess the latest patch sort of works for certain configurations, so I guess I can commit it, but I'm not going to spend any more time trying to make that stuff work. It's just not worth it, and date 5.x-1.* is no longer officially supported by KarenS, anyway...

FYI: I just tried reinstalling my test site with only date_tz_none configured and I had the same problems: No reminder emails sent, and the 2 hours-in-advance ("near") node auto-closed via cron when it shouldn't have. So, it appears that my theory about things being broken with multiple date field types on the same site isn't true, at least not in the way I thought it was. ;)

I've just committed some changes to the Date module, and with my changes and this patch I can get everything working correctly.

The problem dww was worried about was that the date handler was trying to force the database to different timezones, but it is only trying to force the database to use UTC to be sure no timezone adjustments get added in to things like FROM_UNIXTIME(). I was able to reproduce the problems noted above with datestamp fields (which use FROM_UNIXTIME) until I found a bug in the date handler that was not setting the database value for mysql databases, only mysqli databases. I also found some inconsistencies in the handling of the database timezone for dates that use no timezone. After fixing both of those issues, I was able to get the signups behaving correctly.

If dww was using mysql rather than mysqli, this should fix his problems. If he was using mysqli, I'm not sure if I solved the problem, but it's possible that my fixes for the no timezone handling also took care of the datestamp issues.

So this patch needs to be tested with the very latest -dev version of Date to confirm that it is working.

Yup, I've been using mysql not mysqli, so those fixes to date_api_sql.inc were the problem I was hitting. That perfectly explains what I was seeing (which was throwing me off) -- I had signup print out the final SQL it was using. That SQL failed to find any hits via cron, or interactively while my @@session.time_zone was set to 'SYSTEM'. Once I ran that query interactively to set @@session.time_zone to '+00:00', the raw SQL started working interactively. So, I assumed something was broken with how @@session.time_zone was getting set, but I was barking up the wrong tree. ;) Glad you figured it out.

Anyway, here's a slightly updated patch to D6 that just fixes a few comments, and a backport to D5 that includes all the latest DateAPI goodness, along with the evil workaround for ***SQLD***. Using these patches, and the latest DateAPI code from both branches (and my patch for #340356: date/date_elements.inc still contains date_timezone_convert()), everything is working flawlessly now for datestamp fields. Yay!

However, date and datetime fields with no TZ handling are still broken as described in comment #16 (points A, B, E, and F). :( So, it does seem that whatever changes you were talking about for TZ none either aren't working as expected, or something else is still going wrong...

p.s. @KarenS: seems like we're both online now -- wanna meet in #drupal-signup on IRC to see if we can quickly hash out the rest of this?

I also just re-tested _signup_date_node_completed() on all configurations using the latest patches and the latest Date API code from KarenS. The 'datestamp_tz_none now' node does not autoclose when you edit it, even though it should. This is true in both D6 and D5. I haven't debugged yet, just wanted to add a comment that all the other configurations are working except this one.

After another very fruitful IRC session with KarenS, we narrowed down some things.:

#20 (_signup_date_node_completed() being broken for datestamp_tz_none now) is a new bug in CCK date where if you edit a datestamp field with TZ none at all, weird things happen to the To/From date values. So, there's nothing wrong with _signup_date_node_completed(), it's just that datestamp + TZ none is currently broken in the end of DRUPAL-6--2 and DRUPAL-5--2 branches of date.module.

#19: We're pretty sure this isn't a bug in the signup code, either. After digging fairly deeply into it, the bug appears to be that in the case of tz_none fields, somewhere CCK thinks the timezone_db is UTC, when it should really be using the site TZ (in the case of the cckdatetest install profile, America/New_York). It's not yet clear where/why this is happening, but we're 90% sure the bug is not with these patches.

We both need to call it a night at this point, but Karen is planning to take a fresh look tomorrow morning and will hopefully post back here with her findings... Stay tuned.

Rename this issue to reflect the current problem.

I found more fixes needed for the handling of dates without timezones and have committed them. I haven't confirmed whether or not those fixes take care of the remaining signup problems. but if no one else gets to it, I'll try to test that later.

If the fixes work, this patch is OK, it's the Date module that needed changes.

@KarenS: I just tried updating my copy of date* to the latest from the DRUPAL-6--2 branch, reinstalled the cckdatetest profile, ran the following from the PHP block to signup root to all events (to test reminder emails):

$common = array(
  'uid' => 1,
  'signup_form_data' => array(
    'Name' => 'Root',
    'Phone' => '123-456-7890',
  ),
);
$results = db_query("SELECT nid FROM {signup} WHERE status = 1 AND nid > 0");
while ($node = db_fetch_array($results)) {
  $values = array_merge($common, $node);
  signup_sign_up_user($values);
}

and then ran cron. Sadly, once again, date(time)?_tz_none doesn't send any reminders at all, and the "near" event closed early. :( I'm still seeing the TZ offset in the SQL, for example:

SELECT n.title, n.nid, n.type, s.reminder_email, s.forwarding_email, field_date_tz_none_value
FROM node n INNER JOIN signup s ON s.nid = n.nid
LEFT JOIN content_type_date_tz_none ON content_type_date_tz_none.vid = n.vid
WHERE (s.send_reminder = 1) AND (n.type = 'date_tz_none')
AND ('2008-11-30 01:15:01' >= DATE_SUB(ADDTIME(STR_TO_DATE(field_date_tz_none_value, '%Y-%m-%%dT%T'), SEC_TO_TIME(-18000)), INTERVAL s.reminder_days_before DAY))
AND ('2008-11-30 01:15:01' <= DATE_ADD(ADDTIME(STR_TO_DATE(field_date_tz_none_value, '%Y-%m-%%dT%T'), SEC_TO_TIME(-18000)), INTERVAL 1 HOUR))

The handler class still thinks: db_timezone is UTC for date(time)?_tz_none fields. :( date_get_timezone_db() is still returning UTC for fields with TZ none... Seems like I should be writing this in an issue in the date issue queue, since it seems to be pretty clearly a date bug at this point. Let me know if there's anything else I can do to help with this. Thanks!

I found a bug in the date profile, it is creating the fields before setting up the site timezone. Those steps need to be reversed. Without that information, the field construction code will assume the site timezone is 'UTC'.

I'm getting ready to post a patch on that issue.

Hrm, weird. I just tried again with the latest date code in DRUPAL-6--2, I moved the call to cckdatetest_create_content_types() after the call to cckdatetest_configure_date(), cleared my DB and reinstalled the profile. I ran through the steps above in #24 and for date(time)?_tz_none, I'm still seeing the handler think that timezone_db is UTC and therefore, reminders and auto-close aren't working right. :(

(The good news is that the datestamp bugs mentioned in #20 and #21 are now gone with the latest date code from DRUPAL-6--2). ;)

A bug in date.inc in the signup code. We need the following change:

date_get_timezone_db($field)

should be

date_get_timezone_db($field['tz_handling'])

Argh, sorry I missed that! I'll try that now and see how it all goes... stay tuned.

Yay, that did it! Re-tested everything with D6 and D5, and it's all working. I also put in the following work-around code in _signup_date_sql_handler() for sites with datestamp fields using the latest official releases of Date and mysql:

function _signup_date_sql_handler($field, $compare_tz = NULL) {
  // Workaround for a bug in DateAPI upto 6.x-2.0-rc5.
  static $mysql_db_offset_set = FALSE;
  ...
  // Now that the handler is properly initialized, tell the DB what TZ to use.
  $handler->set_db_timezone();

  // Up to and including DateAPI 6.x-2.0-rc5, the previous call only worked
  // for mysqli and pgsql. It was a no-op on mysql itself, so in that case, we
  // have to do the work ourselves here or datestamp fields don't work.
  if ($GLOBALS['db_type'] == 'mysql' && !$mysql_db_offset_set && version_compare(db_version(), '4.1.3', '>=')) {
    db_query("SET @@session.time_zone = '+00:00'");
    $mysql_db_offset_set = TRUE;
  }

  return $handler;
}

So, using 6.x-2.0-rc5 or 5.x-2.4, the only configuration that's still broken for signup are datestamps with no TZ handling. If you use that configuration and you update to the end of DRUPAL-[56]--2 branch, everything is fine. Otherwise, you can use either the latest official releases of Date or the latest -dev code, and signup works great.

Given all that, I committed the following patches to HEAD and DRUPAL-5--2 of signup respectively. Yay!!! ;)

(Setting the title back to be more accurate about what this issue ended up fixing).

I am looking at #29 above as the fix.

hate to reopen, but I am using signup 2.6, Date 2.5 and Calendar 2.5. I am on Drupal 5.14.

I get the same problem of event signups closing on edit.

i allow users to determine the timezone of the event.

Chris

please let me know if I should open another issue although this is exactly the same.
While it is a D5 issue, I see from above that you provided D5 and D6 solution.
Thanks

@activelyOUT: Please stop reopening closed issues for known bugs with older versions of signup. Dude, upgrade to the latest version. You're wasting both of our time with this. RTF CHANGELOG -- it clearly states this bug was fixed in verison 5.x-2.7...