Publication in the morning fails! (2nd patch)
AlexisWilke - October 5, 2008 - 19:58
| Project: | Announcements |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | closed |
Description
Hi Nancy,
I had a customer asking me why his announcements would not appear (and it took me some time because they did not work he deleted them...)
The reason was simple: he was publishing them in the morning. The mktime(12, ...) was the culprit. To not be hit by the difference between UTC and local time, use gmmktime() instead.
I'm attaching a patch, also I could not test properly since it is afternoon now for me...
And if you need someone to help maintain the code, I guess I could help a bit. 8-)
Thank you.
Alexis
#1
The actual patch is here.
#2
Thank you for this patch. I have to think for a few minutes on this one, but I do see where it is better than what is there. However, it would be based on the server's time zone, not the user's. This may be appropriate.
#3
Yes. I was thinking I should think about it too... 8-)
I do not think it is correct like that either because it is based on UTC only, not the local server. That is, announcements will be published at midnight UTC, not midnight to the website. And I would think that it would make more sense to get it updated at midnight local time for the owner of the site.
We could also have a setting in the admin screens to define the time at which it should be updated. Then we can add that time to time() and compare with that. This way, it is whatever time the user chooses.
Thank you.
#4
Hmm, all servers I have dealt with (including even my local Windows system) are set to GMT. So using "date_default_timezone" (the site timezone) is probably not needed. I think I may be getting myself confused for nothing.
#5
Fix committed on both branches.
#6
Nancy,
Okay, so I think that this assumption is correct:
1) On Edit we want to do: UTC -> User Timezone
2) On Submit we want to do: User Timezone -> UTC
If User Timezone not defined, use Site Timezone. If Site Timezone not defined, use 0 (i.e. as is.)
I'm attaching a patch for that feature. Notice that instead of date(), I'm using gmdate().
And I'm not 100% sure but it seems to me that the announcements_all() function should also use gmdate(). It may not make any difference since under Linux `date +%s` and `date -u +%s` return the same result.
Note that I took the code from: /includes/common.inc around line 3600 found in the format_date() function.
Thank you.
Alexis Wilke
#7
Committed on both branches. BTW, when you submit a patch, it is okay to set the "Assigned" field to yourself.
#8
Automatically closed -- issue fixed for two weeks with no activity.