timezone column in users table should have 0 by default, otherwize the following problem occurs:

All users who didn't visit their accounts to specify a time zone have NULL in timezone column. This cause format_date() function to work incorrectly in such call:

$output .= form_item(t('Local time'), format_date(date('U'), 'large', '', $user->timezone));

where $user->timezone is a timezone of the specified user. I'm using such call for a custom profile page to display user's local time. When $user->timezone is null format_date uses time zone of the current (active) user instead of 0.

Of course, this can be overcome by the following call:

$output .= form_item(t('Local time'), format_date(date('U'), 'large', '', (empty($user->timezone)) ? 0 : $user->timezone));

but I though it is better to fix it in the Drupal SQL script.

Comments

magico’s picture

Version: 4.6.6 » 4.6.9

I couldn't find the refered line of code.

magico’s picture

Status: Active » Closed (fixed)
ardas’s picture

You didn't understand me. There is no such call in the code. This is an example of format_date() usage. The problem is in the database - time zone column is NULL without DEFAULT clause which is bad. I just suggest to set DEFAULT 0 for this column to avoid the problem I described.

Thanks.

magico’s picture

Status: Closed (fixed) » Active
ardas’s picture

Thank you for reanimating this issue.
The only fix required is a little change in database script to add DEFAULT 0 for timezone column to prevent the situation when a new user didn't set up his timezone and NULL is in his timezone field which is treated as a time zone of logged in user which is wrong.

Thanks.

magico’s picture

Version: 4.6.9 » 5.x-dev
StatusFileSize
new861 bytes

The column still has default NULL in 5.x-dev
Anyone accepts this patch?

magico’s picture

StatusFileSize
new865 bytes

Forget the previous patch. This is now correct!

morbus iff’s picture

Status: Active » Needs work

A database change like this needs a matching system_update_#### for upgraders.

ardas’s picture

This is exactly what I was talking about. Thank you guys for supporting this suggestion.

magico’s picture

Status: Needs work » Needs review
StatusFileSize
new1.78 KB

Hope this is now correct.

ardas’s picture

Yeah, it looks so.

dmitrig01’s picture

Version: 5.x-dev » 6.x-dev

New features go into 6.x-dev

stevenpatz’s picture

Status: Needs review » Reviewed & tested by the community
ChrisKennedy’s picture

Status: Reviewed & tested by the community » Needs work

The system_update number should be in the 2000 block.

As long as we're fixing this, shouldn't we also update the user table and set timezone to 0 if it's NULL?

pancho’s picture

Status: Needs work » Needs review
StatusFileSize
new1.42 KB

I rerolled the patch using Schema API and adding the missing 'not null' property. Also I added updating the user table as suggested by Chris.

I'm not really sure about the consequences though. What if the system-wide default timezone differs from 0? A user timezone of 0 would mean a deviance from default rather than default then...

dpearcefl’s picture

Does this still need to be fixed in current d6?

Status: Needs review » Needs work

The last submitted patch, timezone_add_default.patch, failed testing.

Status: Needs work » Closed (outdated)

Automatically closed because Drupal 6 is no longer supported. If the issue verifiably applies to later versions, please reopen with details and update the version.