Add option to use ISO 8601 week numbers

Actually, I think Calendar is using method/nomenclature common to UK, Australia, Canada, New Zealand, & USA, according to wikipedia here: http://en.wikipedia.org/wiki/Week_number#Week_number

This differs from the ISO standard...

Looks like MoSaG got the desired results with a hack, but I think for 90+% of the installs week numbering is as desired.
So, I am going to mark this as won't fix for now, but if someone wants to change the behavior, go ahead and reopen.

To the best of my knowledge, the ISO numbers are used throughout continental Europe and much of Asia. Certainly any install in China, Japan, Germany, Belgium or The Netherlands is now displaying what is locally considered to be an incorrect week number. And at least in continental Europe, week numbers are critical in business.

To my users, a calendar with the wrong week numbers is worse than useless, so despite my reservations about hacking modules, I am going to patch the date module source as an emergency repair to what is locally a critical issue. We do all our scheduling by week numbers, so we cannot operate if our week numbers do not correspond to those of our customers.

I understand that American users probably would prefer to stick with the present non-standard-but-common-Anglosaxon definition. I would be happy to help make this configurable, but would appreciate some guidance on where and how that should be done -- it's not my module after all.

The patch by MoSaG solves the week numbers being displayed non-ISO. However, the links to weeks in the calendar still use the non-standard semantics (i.e. calendar/2010-W2 will link to the week starting Jan 4th, rather than Jan 11th). The additional patch below will solve this:

  $last_week_of_year=date("W",strtotime(($year-1)."-"."12-31"));
  $last_day_of_year=date("w",strtotime(($year-1)."-"."12-31"));
  if ($last_day_of_year > 3) {
    if ($week == $last_week_of_year) { $week=1; }
    else { $week++; }
  }

Insert this immediately after the opening of function date_week_range() in date/date_api.module.

This still leaves the Prev/Next links in the week view in error. To fix that, comment out the following code in function date_week() in date/date_api.module:

#  if ($date_year > intval($parts[0])) {
#    $last_date = drupal_clone($date);
#    date_modify($last_date, '-7 days');
#    $week = date_format($last_date, 'W') + 1;
#  } else if ($date_year < intval($parts[0])) {
#    $week = 0;
#  }
#  if ($year_week != 1) $week++;

I think that gets everything into ISO mode without ruining anything else.

1.) Please create a patch file for review
2.) This should be a configurable setting. IMO, the setting should be under/ "First Day of Week" at "admin/settings/date-time"

OK, willdo.

I'm slightly flooded at this moment though, so it might be the weekend before I get around to this.

Note: I marked #677816: Date browser shows wrong week start in the first week of the year as a duplicate of this issue.
That issue is actually against the Date module, but I think both can & should be addressed in this issue.

It does bring up a good point, though:
Attention ALSO needs to be paid to the Date Browser to make sure it's week displays are correct.
I am not sure if the code above does this. (it might, as #6 above is modifying the date module).

subscribe

Yes, the current behavior is by design, using ISO week numbers in the US would be totally wrong. I don't know if there is an easy way to make this configurable and have it work correctly both ways, but I'll look at a patch to do that if someone creates one.

The Date browser uses the same underlying code, so that will need the same fix. Actually, the Calendar uses the Date browser code, but either way, they both should work the same.

subscribe

I quote myself from another issue regarding this subject:

http://en.wikipedia.org/wiki/ISO_week_date says that "The ISO week date system is a leap week calendar system that is part of the ISO 8601 date and time standard." but the date_week

// remove the leap week if it's present
if ($date_year > intval($parts[0])) {

That can't be right??

What about adding a check box "Use ISO week numbering" in admin/settings/date-time as #7 arlinsandbulte says? Then in the http://drupalapi.org/api/function/date_week add an

if(box not checked) {
  // remove the leap week if it's present
  if ($date_year > intval($parts[0])) {
  ...
  }
}
else {
 // Do nothing since leap week is present in ISO week numbering
}

To me it seems as the error is in the http://drupalapi.org/api/function/date_week function and not the calendar. Therefore thats where it should be fixed. But I might be wrong...

I've attached some test code which checks for week number errors from calendar.inc. Insert code and visit the calendar page.

Patch attached.

Patch description:
1) Introduce a new boolean Drupal variable, date_api_are_weeknumbers_iso8601, default value FALSE.
2) Make the changes to date_week_range() and date_week() mentioned in #6 above conditional on this variable.
3) Alter form system_date_time_settings to have an additional element in the fieldset 'locale' to alter the value of this variable.

Patch QA:
- Patch is against date-6.x-2.x-dev downloaded 2010-01-10 16:08 UTC. According to the CVS browser, this is up to date with HEAD.
- Patch has been tested to
-- Not change calendar and date browser behavior by default.
-- Change to ISO 8601 in both calendar en date browser if the variable is set.
-- Allow setting the variable from the admin/settings/date-time page.
- Patch has only been tested on a MySQL install, but affects a function that uses database-specific code (function date_week()).
- Patch assumes that it is safe to alter the definition of week number on a running system. This is true for the calendar and date browser functionality, which recompute week numbers on the fly; this would be false in the presence of a hypothetical module that stores week numbers.

Surprising: I see the wrong week in Date browser, but not in the calendar week view. (I haven't tested the #14 patch yet).

Great job, Rob. Works well in my testing.
However, I found one potential problem: For ISO date to work properly, the First day of week must be set to Monday.

(For 2010) With the "Use ISO-8601 week numbers" check box set & "First day of week" set to Sunday, week #54 incorrectly shows on the January calendar page.
See attached screenshots.

I really don't think this is a show stopper, though.
I propose 2 options to fix this:

Option #1 (EASY):
Just add a note or warning after the checkbox to explain that "First day of week" must be set to Monday when this option is checked. I modified the patch to do that and attached it.

Option #2 (More user friendly, but more difficult and complex).:
Automate the ISO settings. For example, disable the "First day of week" setting when the checkbox is checked and force "First day of week" to Monday.

I am OK with either of these fixes. For now, I am going to mark the attached patch (Option #1) as RTBC.
If someone wants to tackle Option #2, which might be preferable, it could be done after this is committed (or before if you are quick enough).

Better (more standard way) to do Option #1 above.
Patch attached.

Actually, this is a Date issue.

No, I do not want this code changing the first day of the week, that could impact other modules and I don't want to be responsible for making changes the end user may not have expected. A warning is fine and is plenty, so long as the user can continue to see the warning after they make the change so they can tell when they edit this why it may not be displaying the way they want (I don't want a one-time warning that disappears).

I haven't had time to try this out yet.

I posted the following issue at http://drupal.org/node/686394 "Default of current time is off by one week for granularity of week". I have no opinion on whether or not Date uses ISO 8601 for weeks or not. However, the default current time returned in the get_default_argument function of date_api_argument_handler.inc is in ISO 8601. This causes the week view to be one week in the past. I've attached a patch to fix this at the above issue. It's a hack but works for me at the moment.

#19: what would you suggest? The bug breaks the functionality of date browser. I cannot launch the site where the "current week" is actually "past week". It's going to be critical for me. Thank you in advance for help...

#21: Karen was referring to option #2 that I outlined in #16. She just does not want the "First day of week" to automatically be set to "Monday" when enabling the "Use ISO-8601 week numbers" option.

I think Karen is fine with my Option #1, which is what the patch in #17 does.

#22 Thank you for explanation (and for the patch :-)), I'll try it.

I tried to apply the patch (#17) from terminal (Ubuntu), with the result: "patch unexpectedly ends in middle of line" (to tell the truth, I don't know, what does it exactly mean).

However, I applied the patch manually and it solved my problem perfectly. Thank you!

Unfortenately, my optimism was too strong. The patch #17 broke the functionality of calendar week view (and its week browser). With this patch, the default calendar week is wrong (one week shifted to future) and the week browsing does not work well.

subscribe

#20 seems to work well, thanks

subbity sub subscribe

For my purposes, which was getting the month view to display ISO weeks, #17 worked well.
Thanks.

I think patch (#17) addresses the issue in this thread.
The off-by-one week error when setting the calendar default argument granularity to week is a separate, but possibly related issue. It should be addressed in #686394: Default of current time is off by one week for granularity of week. I confirmed this issue both with and without the #17 patch applied.

I suggest keeping this issue RTBC and it be committed to -dev.

+1

Subscribing

I confirm the #17 works, and it actually also fixes the issue in #686394: Default of current time is off by one week for granularity of week. I believe it's ready to be comitted.

I think, however, it'd be a much better solution to have the date module choose ISO configuration from the chosen time zone. I believe it's a more generic solution, and it would allow users with different time zone choices to have views shown according to their preference. Of course, this solution probably requieres changing a lot of code, so I do understand KarenS's position.

Marcus

subscribe :)

subscribe

To sum up: The way the calendar module displays week numbers is simply wrong for some countries. We have a patch (#17) which introduces a new configuration option to specify how the week numbers should be calculated. Several users reported this patch to be working.

So, is there anything left that needs to be done to get this committed?

I reported that patch #17 solved my problem with the weekly date browser, but broke the week calendar view (shifted it to the future, I don't understand why). So I'm not sure wheather it is a general solution.

Could you describe your settings for the Date and Calendar modules as well as what version of each you are using?

@38:
Date 6.x-2.4 (patched #17), Calendar 6.x-2.2
After patching and enabling ISO weeks, the Date browser with weekly granularity works fine, but the calendar week view is shifted one week to the future. I attach exports of both views and patched module file (date_api.module, saved as date_api_module.txt because of suffix restrictions).

Your date_api.module seems to be incompletely patched. In your file there's a block of code starting on line 1108, ending on line 1116 (both inclusive) that is executed unconditionally. This block should only be executed if the week numbers are NOT meant to be ISO 8601. To that end, the patch #17 encloses this block in

if (!variable_get('date_api_are_weeknumbers_iso8601', FALSE)) {

The patch has this code at somewhat different line numbers, namely

@@ -1093,15 +1105,19 @@ function date_week($date) {

Perhaps you missed this bit when you were forced to patch manually?

If I remember correctly from when I was researching this back in January, you're being tripped by the $week++ in the final line of this block.

Could you correct the patching and report whether that solves the problem?

@Rob Carriere #40 - Thank you very much for your help, I confirm that this was the reason... and that the patch #17, applied correctly :-), works fine. I apologize for my mistake very much.

@#41: You're welcome, glad to hear your problem's solved.

Ok, looks like the following is still correct: We have a working and tested(!!) patch in #17.

Can we get this committed? If not, what needs to be done?

subscribe - just discovered I'm having the same/related issue where if the first day of the week is a Sunday, the days are all off by one, e.g. Thursday 13th May 2010 appears as a Friday.

I tested #17, and it works fine!

And seriously, fixing this bug is critical, if anything. I'm sure it's affecting much more than 10% of Drupal users, probably more than 50% even. UK, Australia, Canada, New Zealand, & USA are probably in the minority compared to the rest of the (Drupalized) world. Not having this fixed makes me, my employer and Drupal itself look really bad.

(I would actually need the patch ported to Drupal 5 aswell for one project, but at least it works for drupal 6..)

Let's get this committed, pretty please?

@bibo
+1

This is indeed a major problem to many Drupal users as it is essentially breaking some key parts of the calendar module. If you are living in a timzone using ISO 8601, calendar module is showing wrong week numbers. To those people it does not matter that the way calendar handles this is correct for other timezones. So, we obviously need a setting to fix this.

Subscribing...

I am not using Calendar at all, I didn't even enable it -- just Views and Date.

Two variations I've tried for my View's arguments: (Summary, sorted ascending)
-- Node: Created week
-- Date: Date (node), Node: Post date

In both cases, if I have my Date & Time settings set at Sunday as the first day of the week, the week numbers are off by 2; they're off by 1 if I'm using Monday as the first day of the week.

Just like what others have described, the link text and URL path elements are the parts that are off (by 1 or 2 weeks, depending), compared to the page title (and view results). For example, an item in the "Summary, sorted ascending" list would be:
May 10 2010 (19) --> URL linking to /path/to/page-display/2010-W19
...and the page you end up on has the following title/result set (I grabbed bits from the query):
Sunday's Message: May 3 2010 (18) -- node.created <= '2010-05-10 00:00:00' AND >= '2010-05-03 00:00:00'

Anyway, like everyone else, I really need a fix for this!! Here are some other details, in case they're helpful:
-- Drupal 6.14
-- Date 6.x-2.4
-- Views 6.x-2.6

Thank you!

**EDIT** Never mind what I said about both argument variations — "Node: Created date" works properly (although it ignores your first-day-of-the-week setting and uses Monday regardless).

quick follow-up -- #17 worked exactly as expected for me. thank you, arlinsandbulte!!

#17 works for me, thanks!

#17 works for me, thanks!

It's not fixed until it's committed into the module. Setting status back to RTBC.

For Karen's benefit, here is a summary:

To reproduce the problem & verify the patch:

1. Fresh install of Drupal 6.19, CCK 6.x-2.8, Views 6.x-2.11, Date6.x-2.x-dev (August 12, 2010 - 18:06), & Calendar 6.x-2.2.
   Note: Calendar not really needed, but makes it easy to verify the patch works, as well as checking that Date changes don't mess up Calendar.
2. Enable following Modules: Content, Calendar, Date, Date API, Date Timezone, Views, & Views UI.
3. Enable Calendar & Date Browser default views.
4. Set site's default timezone at /admin/settings/date-time (I used America/Chicago)
5. Set User-configurable time zones: Enabled (this should be the default)
6. Set First day of week to: Sunday (this should be the default)
7. Visit /calendar/2010-01. Week #1 is Jan 1 & 2. This is correct according to USA & Canada week # convention (which is not ISO 86011)
8. Visit /calendar/2009-12. Week #53 is Dec 27-31. This is correct according to USA & Canada week # convention (which is not ISO 86011)
9. Set First day of week to: Monday at /admin/settings/date-time.
10. Visit /calendar/2010-01. Week #1 is Jan 1-3. This may be correct according to USA & Canada week # convention, but is completely wrong according to the ISO 8601 standard. According to ISO 8601, Jan 1-3, 2010 is actually Week # 53 of 2009. Week #1 of 2010 should be Jan 4-10, 2010.
11. Visit /calendar/2009-12. Week #53 is Dec 28-31. This may be correct according to USA & Canada week # convention, and it just so happens that this is correct for the ISO standard too. That is why this error was not noticed until 2010.
12. Visit /calendar/2010-12. Week #53 is Dec 27-31. This may be correct according to USA & Canada convention, but is completely wrong according to ISO 8601. For ISO 8601 compliance, the last week of the year should be week #52 & includes Dec 27-31.
13. Apply patch from #17
14. Visit /admin/settings/date-time and enable 'Use ISO-8601 week numbers'
15. Visit /calendar/2010-01. Week #1 is Jan 4-10. This is correct for ISO 8601 compliance (wrong for the USA/Canada convention).
16. Visit /calendar/2009-12. Week #53 is Dec 28-31. This is correct for ISO 8601 compliance, (and might be ok for USA/Canada too, although weeks usually start on Sunday there instead).
17. Visit /calendar/2010-12. Week #52 is Dec 27-31. This is correct for ISO 8601 compliance (wrong for the USA/Canada convention).
18. Visit /admin/settings/date-time and set First day of week to 'Sunday' and disable 'Use ISO-8601 week numbers.'
19. Visit /calendar/2010-01. Week #1 is Jan 1&2. This is correct for the USA/Canada convention (but wrong for ISO).

I used calendar above to check the problem & verify the solution. I also took a look at the date-browser, and everything seemed good there too. I have done some browsing and clicking and did not find anything this patch broke. Most of my testing took place with the 'Use ISO-8601 week numbers' option unchecked.

To summarize:

(Reference here for details of week number standards: http://en.wikipedia.org/wiki/Seven-day_week#Week_numbering)

• Without the patch applied
  • Week numbers conform to the USA/Canada & "Much of the Middle East" convention.
  • Week numbers DO NOT conform to the ISO 8601 standard.
• With the patch applied
  • Week numbers conform to the USA/Canada & "Much of the Middle East" convention, but ONLY when the 'Use ISO-8601 week numbers' option is unchecked. (Note: First day of week should be set to Sunday for USA/Canada & Saturday for "Much of the Middle East")
  • Week numbers adhere to the ISO 8601 standard, but ONLY when the 'Use ISO-8601 week numbers' option is checked AND the first day of week is set to Monday.

Further Justification for this patch to be RTBC & committed:
Looks like when this new option is unchecked, the code logic simply reverts to EXACTLY the way it worked before the patch due to the added if statements.
So, if there is anything wrong with this patch, it will only change the date module behavior if the new option is checked. One can easily 'revert' this patch by simply unchecking the 'Use ISO-8601 week numbers' option. So we are not worse off than before the patch.

Hope that helps!
Arlin

Thanks for all this Arlin :)

Here are my comments (some of them just because I know Arlin is trying to learn the best way to make patches like this for the future). I went ahead and committed this, with my changes:

1) I added a validation handler to the form where you choose to use ISO dates to prevent you from choosing that option without also changing the first day of the week to Monday. It will still be under their full control (no changing it without them knowing), but it will keep them from selecting an option that will not work.

2) I simplified the name of the variable a bit -- it was kind of long.

3) I added the variable to the list of variables to be deleted in hook_uninstall().

4) I created a separate function for date_iso_week_range() instead of munging the logic for both methods together. This is just to make maintenance easier. I added a switch at the beginning of date_week_range() that checks the variable and uses the alternate function.

5) You used date() and strtotime() in your calculations. We have to avoid those functions because they incorporate timezone adjustments. They adjust to the server timezone, which may or may not be right, depending on how the system is set up and what timezone you are trying to use. Instead we always have to use date_make_date(), date_format() and date_modify() to manipulate dates in a way where we can have total control over the timezone adjustments. Also those older functions only work for the years 1970-2038 in PHP4, and a wider, but still limited date range in PHP5, so we can't rely on them.

6) ISO weeks are actually much easier to work with than calendar weeks. The PHP date functions use those normally. So we can simplify some of the logic when we want ISO weeks.

Thanks Karen, good info to know. I checked out this commit and everything works as expected.

Looking at the diff, I was initially confused by the "if (variable_get('date_api_use_iso8601', FALSE))" statements. From my untrained eye, it looked like this logic was backwards until I read the variable_get API reference (http://api.drupal.org/api/function/variable_get/6). First is looked like that statement was testing "if 'date_api_use_iso8601 == False"
Still a little confused, but I think I get it.

Thanks again!

Thanks for your work on this Karen.

I feel though, even though the standard states that Monday is the first day of the week, couldn't making Sunday the first day of a week merely a cosmetic issue as opposed to a major standards violating issue? (Just curious.)

There is another issue #686394: Default of current time is off by one week for granularity of week that is related to this and it has a solution but the solution has not yet been committed. (See item number 7 of #686394: Default of current time is off by one week for granularity of week.) This fix was cited as being a solution to this problem, but alas it is not for those of us who use Sundays as the first day of the week.

Thanks for all of your hard work. Do you have a donation form available for us to make contributions?

Sorry for reopening this issue, but in 2011 it does not work for me, if I try to set the date browser with a week granularity. The browser shows the week starting on December 26, 2011 (!) as a current date (current week).

I use PHP 5.2.10., ISO week checked, first day set to Monday.

The same problem for both 6.x-2.6 and 6.x.-2.x-dev version of the module.

Exported view attached. Thank you in advance for help.

#59
Your issue might be related to this one instead: #686394: Default of current time is off by one week for granularity of week

I don't think so (but even so, I follow both threads). However, in this (reported) issue, I don't use the Calendar module. The date_browser view is provided by the Date module itself.

#686394: Default of current time is off by one week for granularity of week is actually an issue with the date_browser not the calendar module. My view is showing a default of December 18 2011. I was really hoping the issue would go away in the new year.

Please don't re-open this issue to report a bug, this is a fixed feature request. If there are bugs after the feature is added they are separate issues. I committed a number of fixes to the ISO week functionality this morning. To get my fixes you need to try the very latest -dev code. If you don't use a cvs checkout and use the dev tarball you have to wait for tomorrow to get a tarball that has today's fixes.