Posted by darrick on January 16, 2010 at 3:34am
| Project: | Date |
| Version: | 6.x-2.7 |
| Component: | Code |
| Category: | bug report |
| Priority: | major |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
I have a calendar view with a granularity of week. My expectation is going to my view url with no arguments (default set to current time) the proper week would show. However, the view is a week behind (http://dctv.davismedia.org/schedule).
I read the report "Add option to use ISO 8601 week numbers" at http://drupal.org/node/641344 but don't feel this issue is about whether the format is ISO 8601 or not. I tried to force the default argument to now() + 1wk but had no luck there. Any solutions or tips are appreciated. Thanks.
Comments
#1
I am having the exact same problem. It seems like it happened immediately after the new year or with a recent upgrade of the date module. How do we fix this!?
#2
Yep, happening to me, too... but it actually looks almost 2 weeks off.
#3
I think I see why it's happening - and I don't see where I should fix this... any suggestions on how to do so would be welcomed.
When I look in the View for my Calendar, you know it's got the various displays for Week, Month, Year, etc... the issue that most of them had "month" as the default value in Argument instead of the correct daypart is minor and easily fixed... but when I look at the Week display and preview said display... well, looking at the SQL generated I see why it's a week off.
The Week's SQL has this in it:
LEFT JOIN content_type_nodetype node_data_field_fieldname_date ON node.vid = node_data_field_fieldname_date.vid
WHERE ((node.status <> 0) AND (node.type in ('nodetype')))
AND ((ADDTIME(FROM_UNIXTIME(node.created), SEC_TO_TIME(-18000)) <= '2010-01-18 00:00:00' AND ADDTIME(FROM_UNIXTIME(node.created), SEC_TO_TIME(-18000)) >= '2010-01-11 00:00:00'))
ORDER BY node_data_field_fieldname_date_field_fieldname_date_value ASC
The 'nodetype' and "fieldname' bits are me changing the name of those items here... but today is the 18th and the sql is saying "display a weeks worth of events prior to the current week" (i have monday as the start day - if i chose sunday, it'd be two weeks prior)
I've been searching around in the code and am not finding where the sql statement to choose the "week value".
#4
I did find a fix. Which is a total hack but it works. This issue is related to http://drupal.org/node/641344 and is a bug in the date_api module. As stated in that issue the date module does not use ISO 8601 format for the date week format. However, the default argument in the week granularity view is in ISO 8601.
I've attached a patch and will cross-post to issue 641344.
#5
#4 seems to work ok with date browser and calendar week view as well... Thank you
#6
Same problem for me :(
#7
I fixed some issues with the last patch I submitted. Also discovered you need to set "First day of week:" to monday at admin/settings/date-time.
#8
Patch solves my issue. Thanks darrick!
#9
Where is the file that we are patching this to? I can't find it!
never mind... stupid question.
#10
I tried the patch (actually both of them), but my calendar still displays the previous week by default, any idea?
#11
I tried the patch in #7 at 9:45pm on Sun, Jan 31. It previously said "Jan 17" in the date changer, and now it says "Jan 24."
So I changed the +7 in the patch to a +14 and the date is correct.
I assume there are still some issues with this that will recur on other days.
However I do not want to change my start D.O.W. to a Monday.
#10 are you caching?
#12
I can confirm I was having the same issue. subscribing to wait for an official fix.
#13
subscribe
#14
subscribe
#15
subscribe
#16
Critical, because... well, my users live in the past since the beginning of the year, with one week late! lol
#17
This is not a proper answer to the problem, but it it a module-based work-around using the hook_views_query_alter().
Basically, every site that I work on needs some custom module work, and I put all of the odd and end functions into a misc module (providing that the task is not large enough to warrant a separate module). Here is what I included in my misc.module:
<?php
function misc_views_query_alter(&$view, &$query) {
$arg = arg(1);
if ($view->name == 'episodes_weekly' && empty($arg)) {
// we only want to overwrite the argument when there was not one passed in the url
$view->args[0] = _calculate_week(time());
}
}
function _calculate_week($timestamp) {
$today_array = getdate($timestamp);
$year_start_dow_timestamp = $timestamp - ($today_array['yday'] * 24 * 60 * 60);
$year_start_dow_index = date('w', $year_start_dow_timestamp);
// use a quick math trick to find out which week we are in.
$week_offset = ceil(($today_array['yday'] + $year_start_dow_index) / 7);
$year = $today_array['year'];
// we can't have a week # 1 if the year doesn't start on Sunday, so we need to use
// the year and week number of Dec. 31st of the previous year
if ($week_offset == 1 && $year_start_dow_index != 0) {
return _calculate_week($year_start_dow_timestamp - (24 * 60 * 60));
}
return $year .'-W'. $week_offset;
}
?>
Obviously, you would need to modify this somewhat to fit your needs. In my case, the view was at url
/shows/*argument here*, so it made sense to test for the presence of arg(1). My view was namedepisodes_weekly, so you would need to change that to the appropriate view name. Also, the argument in question just happened to be the first (and only) argument for the view, so it made sense to put the new argument value into$view->args[0]. Your implementation may need the argument placed in a different place in the array if your arguments are configured differently.This code assumes that Sunday is the first day of the week. I didn't want to make it any more flexible, since I assume that this is only a temporary solution until the correct patch makes it in. I just wanted to provide a simple, non-hacking alternative that will fix the problem until a proper solution can be implemented.
#18
subscribe
#19
The patch from darrick works for me. Thank you.
#20
@#17: your approach looked good to me, because I rather not patch too many files. Unfortunately, the code breaks when the path of the view has two or more elements. For example, it works fine if your view is located at www.example.com/calendar, but it breaks on www.example.com/events/calendar. In that case, arg(1) will return 'calendar' so the rest of the script is not executed. I could not find a way to determine if the argument has been created as a default argument, or if it's actually passed to the view in the url. Since changing the path is not an option in my case, I will use the patch from #7.
#21
The code doesn't break, but it does have to be modified for your use. It is custom code, after all. That's why I pointed out that I used
arg(1). If your url is different, then the argument would be in a different place (likefoo/bar/*argument here*would usearg(2)).in your case (
events/calendar), you would be testing for arg(2).If you used
foo/bar/baz, then you would use arg(3), and so on...#22
Of course I can modify code, that's not the point. I'm not criticizing you or your code, I just wanted to share why I ended up using the patch instead of the hook_views_query_alter solution. In my specific situation, having to modify the code is not good enough. I searched for a way to make the code "self-modifying" but I could not find it. I'm sorry if I didn't make myself clear in #20.
#23
Patch from Darrick works! Thanks
#24
"Your patch didn't work", "Yes, it did, you just entered it wrong", BLA BLA BLA!! People are just too sensitive on these forums! Let's not get too touchy on here. After all, it's nothing personal. Wow!
#25
subscribe
#26
I fixed this by applying the patch in #641344: Add option to use ISO 8601 week numbers. I know you are aware of that issue, and I know they are different, but still I just wanted to let you know.
#27
Subscribe
#28
This doesn't appear to be fixed in the new dev release... still defaulting to a week behind.
#29
subscribe
#30
Patch of #7 fixed this issue for me.
#31
subscribe
#32
I'm experiencing the same problem with current versions Calendar/DateAPI, I like the hook_alter, thanks coreyp_1!
Is #7 going to be committed?
Greetings Wappie
#33
This issue was not resolved in the latest release!
#34
Same issue. Using #7 as well. tried #17 first but is not working in my situation (deeper path level and i am using multiple displays on the same page).
Looking forward for a commit in the next release.
#35
Thank you darrick, patch on #7 worked for me as well (used in Calendar Views with an argument needing granularity of week). I'm subscribing in case the patch is committed to a future release.
#36
subscribe
#37
subscribe
#38
KarenS, what do you think of the issue / patch#7?
Greetings Wappie
#39
subscribe
#40
This may have been fixed by #641344: Add option to use ISO 8601 week numbers, which was recently applied.
Could someone confirm or deny that, please?
Make sure you use the -dev with the #641344 patch applied (or a direct CVS checkout).
That means you will need a -dev with an August 13, or later date.
&
Make sure that you configure your site's date settings to First week: Monday & enable the 'Use ISO-8601 week numbers' option.
#41
Personally, I am hesitant to go this route because I do not want my first day of week to be Monday. So even if the patch works, that requirement would still not make the whole issue "fixed."
I am still using my hack in #11 and it works for most situations, but I am hoping for the "Monday" requirement to be removed, if at all possible.
Thanks for your efforts.
#42
+1 MBroberg. I really can't display calendars starting on Monday because I am displaying full weeks, not work weeks. A calendar that starts Monday shows next week's Sunday...
#43
I can confirm that for me #641344: Add option to use ISO 8601 week numbers fixes this issue for me. Of course first day of the week has to be Monday, but for me this is not so bad.
#44
Has the patch in #7 been committed yet? It seems to be the best solution.
#45
#7 worked for me, but I had to change the 7:
<?php$arg = date($this->format(),time()+7*24*60*60);
?>
To 14:
<?php$arg = date($this->format(),time()+14*24*60*60);
?>
I'm not sure what is causing the 14 or 7 days offset. This could probably get included in Date API if there was some setting options for it at "/admin/settings/date-time". Having the first day of the week set to Monday just doesn't work in the US.
#46
@ #40, the fix for #641344: Add option to use ISO 8601 week numbers works, but is not the best solution if you need your weeks to start on Sunday.
#47
subscribing
#48
Has there been any work on this even towards a commit? This issue renders the module useless for several people.
-R
#49
#50
#641344: Add option to use ISO 8601 week numbers is insufficient to solve this problem. Currently, the best solution is in number 7... Is it ever going to be committed?
Thinking in regards to the other post (though the ISO standard states that Monday is the first day of the week)
Standards pushed aside, couldn't it be a cosmetic issue instead of a major standards violating one?
There's got to be a better way of dealing with this.
#51
#7 does not work if the "Use ISO-8601 week numbers" option is enabled at /admin/settings/date-time.
I am working on a patch to correct that.
#52
Patch that addresses #51 attached.
I'm not entirely convinced this is the right way to fix this. It is very hacky and sort of band-aids the real problem, IMO.
But, at least for the calendar view, it seems to work.
Also, this patch is actually applied to the Date module, so by rights, this is probably a Date issue.
But, the problem seems to manifest itself in Calendar, so I will leave this in the Calendar issue queue for now.
#53
Looks fine, but this doesn't fix the 14 vs. 7 days offset issue. See #45. Adding a form field to "/admin/settings/date-time" that sets a variable for the 14 or 7 days offset is one solution. I haven't looking into why I'm getting 14 days and not 7, but others are as well, see #11.
#54
Arg! I did not notice the 7 vs 14 day offset issues above.
I assumed the problem was that ISO week number could be different than the 'calendar' week number, and that the most these two different week numbers could be different is by 1 week (7 days), not 2 weeks (14 days).
Any idea why some sites require a 7 day offset correction while other require 14 days?
Do you have your timezone set at /admin/settings/date-time?
Does
dsm(date("Y-m-d g:ia", time()));return the correct (current) time for you?#55
subscribe
lets hope for a commit soon
at least I have a fix now though
#56
Funny thing, last night the site I've been working on was offsetting too far using 14 days. I switched it back to 7. However, when 14 was working, it was on a Sunday. I think there may be a problem on Sundays using the 7 days setting. I'll try and check what the site is doing this Sunday to verify.
#57
The reason is, in date_week_range() function, it's being:
<?php// move to the right week
date_modify($min_date, '+' . strval(7 * ($week - 1)) . ' days');
?>
And it should be:
<?php// move to the right week
date_modify($min_date, '+' . strval(7 * $week) . ' days');
?>
#58
Reporting back... It's Sunday the 12th and my week calendar is showing last week, 5th through 11th. The patch code is currently set to 7 days.
<?php$arg = date($this->format(),time()+7*24*60*60);
?>
Can anyone else confirm this?
#59
Mine is doing the same.
#60
#7 works for me as well. Also want to echo comments by nfd (#48/#50) and others.
#61
subscribing - i can't use #7 since my week starts on sunday. looking forward to a robust fix.
#62
subscribe
#63
Subscribing, my week starts on a sunday too...
#64
subscribe, patch works #52 thanks
#65
I reworked my patch. Tested with the "Use ISO-8601 week numbers" both on and off.
The patch is against the latest cvs. It also fixes the issue with the first week being off by a year.
#66
I must say I am tremendously disappointed that this was not fixed in the most recent release. It has been over a year and this has not gotten the attention it disserves. Thank you darrick for your patches, they have been a valueable resource!
-R
#67
Looking at the patch it is applied to the date module, has this issue been reported on the date module's issue queue? Perhaps we should move it?
#68
@nfd, I am not going to apply patches that are submitted saying 'this is a total hack', and I don't have time right now to find something that is not a hack. I had to get a release out because lots of important fixes had been made, just not this one. I can't wait until the issue queue is empty to do a release.
#69
I also see that there is a new patch that may not be a hack at #65, but it came in after I finished the release and it needs to be tested.
#70
#71
Can anyone verify that #65 handles Sundays properly. The previous patch gave you the prior week on Sundays, but come Monday, it would be the correct week.
#72
@KarenS. I appreciate that you don't have a lot of time to commit to every issue and thank you greatly for your hard work. It just feels though as if this issue (a whole year later) has not gotten the attention it deserves because it is indeed a function crippling issue. Perhaps enforcing or getting people to assist with enforcing the designation of what is critical and what is not may help with better prioritizing the issue queue. I am pleased though that now you have actually looked at/posted to this issue.
I find it odd that such a critical/popular module really only has you making contributions with the vast majority of commits. It is a shame that there isn’t anyone else as dedicated as you are to contribute to the Date and Calendar modules as much as you do. I wish I could help, but I barely can program a form that pushes data to PayPal, let alone a complex time calculating module such as this. Thanks for all of your hard work!
@bendiy I can confirm that it handles Sunday's properly. It was easy to install and works well. :)
#73
Committed a different fix. I found a non-hacky way to do this. We have a nice API to compute the week, we don't need to do it here.
#74
That's excellent news. Thanks Karen!
#75
Excellent! Thank you for fixing this. :)
#76
Karen, I notice that your commit fixed this issue through the Date module rather than the Calendar. Just marking it as such.
#77
Automatically closed -- issue fixed for 2 weeks with no activity.
#78
I am having this bug still because I am waiting on the recommended release. When do you think you will update the dev version to recommended status?
#79
@Kman2123 it should be in the latest release! :D
#80
@nfd The patch for this issue was only released in the dev version and not the stable release version. Do you know when the stable release will be updated to include the patch in the dev release?
#81
I've just moved from 6.x-2.7 recommended release to the 6.x-2.7-dev version of Nov 27, 2011 and it did fix the issue of showing the wrong week's date in the calendar week view.
I'm updating the status on this because the comment in #80 is correct. The patch that fixes this was applied only to the dev version and not the official release. I think its appropriate to keep this issue open until the official release contains the fix.
#82
Good to hear it has been fixed. Hopefully they will update the recommended version here soon!
#83
The last submitted patch, 686394-3.diff, failed testing.
#84
Fixes are never applied to the 'official' point releases. Once a point release is made, it is a snapshot that is never again changed.
All fixes are applied to the -dev versions. Later, once the module maintainer deems it a appropriate time, they create a new 'official' point release from the -dev that supersedes the previous 'official' release.
This issue has been fixed, and marked as such.
The next 'official' point release will include this fix.
#85
Automatically closed -- issue fixed for 2 weeks with no activity.