date_week_range and date_iso_week_range both returns 1 day too much.

The function works as intended, your change will break everything in Date and Calendar that is using it. It is finding the outside parameters of the week. And I don't know what you mean by saying the testing suite seems deprecated, the tests are working.

If you see a place where the Date and Calendar code create the wrong results because of a bad value here, please reopen, but you haven't indicated anything is broken, just that the function doesn't produce the results you expected. Since making this change would require changing dozens of other lines of code, I won't apply it unless there is some way that Date and Calendar are broken without this.

This is in no way a critical issue, leaving aside the question about whether it is even a bug.

This may or may not be related, but I get this after applying the patch for #1227350: Date summary views broken in latest dev.

The weeks are off by two in summary view. I have three nodes with dates in August: 2011-08-01, 2011-08-08, and 2011-08-15. All the times are 10:00 - 12:00 (GMT-5), so midnight is not the issue. When the summary view is set to week granularity, I get

July 24 2011 (29) (1)
July 31 2011 (30) (1)
August 7 2011 (31) (1)

when it should be

August 7 2011 (31) (1)
August 14 2011 (32) (1)
August 21 2011 (33) (1)

However, the argument 2011-W32 returns the node for 2011-08-01. This is only one week off as 2011-08-01 is in week 31. Clicking on the "August 7 2011 (31) (1)" link in the summary returns no results.

Incidentally, I think listing the first day of the week in the summary instead of the last day would be more intuitive for users.

Hi, I have same problem as described in original post. My view (basic table, not a calendar) takes week as an parameter in this format: "2013-W11".

2013-W11 should be from 4.3.2013 to 10.3.2013 (d.m.Y) but view is also showing 1 extra post (I have one post per day.) with date 11.03.2013.

"Use ISO-8601 week numbers" settings have no effects on the problem.

Can some server side settings affect this behavior? Is there any aditional info I can provide?

KarenS, you say

> The function works as intended, your change will break everything in Date and Calendar that is using it.
No so. To see this bug (whether it is in Calendar or Date) create a view with a calendar display and a week-date pager; you would expect to see 7 days for each paged week, but you get 8.

I applied the fix suggested here so now I only get the conventional 7 days per week, all else seems to be working as expected.

In addition the comment above the function says "@return A numeric array containing the start and end dates of a week." but as is it returns the start dates of the week and the following week.

It's a bit disappointing that after 3 years I have a new release of Date and have to re-apply this fix. What's the problem?

I agree with hughworm, this doesn't work as intended and it's quite illogical, I'm making a view with a weekly timetable for a school and I get also the next Monday. Changing the "+7 days" to "+6 days" doesn't seem to break anything.

She replied three years ago, so maybe in the meantime she changed her mind :)

Anyway I checked where this function could influence other parts of date/calendar. If we go up the function calls, we can see that the fixed functions are influencing two parts od the date module: the date pager and the theming, so I really don't see any big problems that could arise after this simple fix. Anyway, to be sure, I'm doing some tests and everything works fine for now.

I suppose that if we're hoping in a fix & commit, we should provide a patch, shouldn't we? ;)

Checking the tests, they're IMHO not correct:

Test calendar date_week_range(5, 2008): should be 2008-01-27 to 2008-02-03, found 2008-01-27 to 2008-02-02.
Test calendar date_week_range(5, 2009): should be 2009-01-25 to 2009-02-01, found 2009-01-25 to 2009-01-31.
Test ISO date_week_range(5, 2008): should be 2008-01-28 to 2008-02-04, found 2008-01-28 to 2008-02-03.
Test ISO date_week_range(5, 2009): should be 2009-01-26 to 2009-02-02, found 2009-01-26 to 2009-02-01.

The weeks in the tests have eight days.

It's necessary to open a new issue for the tests or everything could be done here?

Thanks for your time on this schifazl.
I guess we need to patch the tests, so I attach a revised patch file.
It now passes the revised date_api test on my environment...

I did that too, but my question is another:
the tests shoud be fixed in another patch?

In another module's issue queue a patch was failing tests for some errors in the tests. The module's owner first created and committed a patch for the tests, and only after that he re-tested and finally committed the patch.

That's why I have this doubt :)

No one knows? Well, let's see what happens now...

Test passed! We need some more feedback to set status to RTBC I think (and hope that this patch won't be rejected).

We must cross our fingers now ;)

Hi,

I think that with this patch, the week is not complete. I think we need to change:

date_modify($max_date, '+6 days');

with:

date_modify($max_date, '+6 days +23 hours +59 minutes +59 seconds');

Otherwise the last day is missing.

Thanks

Hi,
Sorry you are right, I have to give some Information. Sorry about that.
The problem that I have is with a Week View. The View Query was looking like this:

WHERE (( (DATE_FORMAT(ADDTIME(field_data_field_datetime.field_datetime_value, SEC_TO_TIME(7200)), '%Y-%m-%d %H:%i:%s') >= '2015-06-21 00:00:00' AND DATE_FORMAT(ADDTIME(field_data_field_datetime.field_datetime_value, SEC_TO_TIME(7200)), '%Y-%m-%d %H:%i:%s') <= '2015-06-27 00:00:00') )

This is not a week, end should be 23:59:59. That is why I did the patch.

I agree with #30.

The original implementation, date_modify($max_date, '+7 days');, creates a period that is a tiny bit too long, for example it would start at 00:00 on Sunday and finish at 00:00 the following sunday, so Sunday is included twice. That would be ok if a less than operator were used, but since a less than or equal to operator is used in the SQL, it does not exclude the beginning of the second Sunday.

But the change date_modify($max_date, '+6 days') means the period runs from 00:00 on say Sunday, to 00:00 on the 6th day, eg Saturday, meaning that the whole of Saturday is excluded.

Either using date_modify($max_date, '+6 days +23 hours +59 minutes +59 seconds'), or changing the SQL condition to use a less than or equal to should solve this.

Hi,

@graham.roberts Whats is wrong with my patch #27? I am using it, and works great for me.

Thanks!

Patch looks great.

Only problem is dat the title matches the original issue and not the current follow-up issue.

This follow-up makes total sense.

Hi,

To summarize (and check I got this right):

Assuming the week starts at Monday, it is quite reasonable to say that the "last day of the week" is "Sunday"; after all, we are talking about "a day", and the specific point in time that refers to it is "Sunday, 00:00:00h".

But, if we are talking about a "week range", it is quite reasonable to say we are interested in the time interval between the beginning of the first day of the week and the end of the last day of the week; therefore, a "week range" fits more naturally, at least for me, with the idea of "Sunday, 23:59:59h".

If I understand things correctly, the original code would yield an almost correct "week range" (from Monday 00:00:00 to the next Monday 00:00:00) but an incorrect "last week day" (Monday instead of Sunday). But the Calendar module assumed that and things mostly worked ok, because when you ask the API for a range you usually will not worry much about the "last day", only the range itself.

The patch in #16 fixes the "last day" problem but worsens the "wrong range" problem, excluding a whole day from the range. This was fixed by the patch proposed in #27.

The problem is that #16 was included in release 7.x-2.9 but #27 was not. This created a bug in calendar view: https://www.drupal.org/node/1779142 . I believe this is a really serious bug, as it broke a very basic functionality in calendar that worked for ages and affects basically anyone with events in a calendar that are not "all day" events. Given that and the fact that a patch is already available, I believe it warrants a new release.

Thanks for the summary, @Nelson.

I can confirm that the Date API module release that includes the patch from #16 did cause results from the seventh day of my Calendar Views week displays to be excluded.

I also confirm that the patch from #27 fixed the problem for me.

Without #27, the week-display queries for my Calendar Views clearly show that results must start before the beginning of the last day to be included, effectively leaving off results from the entire last day.

I can also confirm that patch #27 solves our issues with Date 7.x-2.9.

With 7.x-2.8, the WHERE clause of the Views query looks like the following on a Week calendar:

WHERE (( (DATE_FORMAT(ADDTIME(field_data_field_date.field_date_value2, SEC_TO_TIME(-21600)), '%Y-%m-%d %H:%i:%s') >= '2015-12-06 00:00:00' AND DATE_FORMAT(ADDTIME(field_data_field_date.field_date_value, SEC_TO_TIME(-21600)), '%Y-%m-%d %H:%i:%s') <= '2015-12-13 00:00:00') )

While the WHERE clause with 7.x-2.9 generates as follows:

WHERE (( (DATE_FORMAT(ADDTIME(field_data_field_date.field_date_value2, SEC_TO_TIME(-21600)), '%Y-%m-%d %H:%i:%s') >= '2015-12-06 00:00:00' AND DATE_FORMAT(ADDTIME(field_data_field_date.field_date_value, SEC_TO_TIME(-21600)), '%Y-%m-%d %H:%i:%s') <= '2015-12-12 00:00:00') )

In 7.x-2.9, one less day is being retrieved (Dec 12 vs Dec 13). This is causing no entries to be returned for the last day of the week. With the patch applied, we now see the following for the WHERE clause:

WHERE (( (DATE_FORMAT(ADDTIME(field_data_field_date.field_date_value2, SEC_TO_TIME(-21600)), '%Y-%m-%d %H:%i:%s') >= '2015-12-06 00:00:00' AND DATE_FORMAT(ADDTIME(field_data_field_date.field_date_value, SEC_TO_TIME(-21600)), '%Y-%m-%d %H:%i:%s') <= '2015-12-12 23:59:59') )

This is a better result, since a given day should be "less than or equal to 23:59:59", and not "00:00:00" as it was previously.

#27 fixes problem for me as well on 2.9.

We have found that patch #27 causes an unintended issue with the Week Calendar template. Please review the Calendar patch I've created, and let me know if there are any questions or feedback. Thanks.

https://www.drupal.org/node/2673192

according to #42

patch # 27 works well for me

@podarok, why is this "needs work"? Per my comment in #42, it's simply necessary to include the patch in Issue #2673192. Patch #27 works fine as long as the other patch is committed too.

patch #27 works great for me.

Thank you very much!!

Greetings,

I applied the patch (#27) to the Date module 7.x-2.9.

Result : in the week view, an event on Sunday now appears.

BUT multi-column presentation (events lasting many days) stop at Saturday.
( in Views, multi-column is an option, you can switch it to show the event title in every column if you prefer, which would probably bypass this issue remaining after patch #27. )

I much prefer the multi-column display of long events.

@Michael_Lessard_micles.biz, please apply the patch linked in #42. I believe that will solve the issue you're describing: https://www.drupal.org/node/2673192

Both Issue #1137062 and #2673192 need to be committed together.

+1 I applied both patches (the one here and the one offered by ron_s above).

Both worked seamlessly (no errors).

Calendar is now fully fixed. Thank you.

Patch in #27 works for me.

Thanks !

Patch #27 worked for me as well. Cheers

We had a calendar with a week display, and the events on the 7th day weren't showing. Applying patch #27 fixed it for us.

Patch #27 worked for me!

This really should be committed.

Seems like this one is ready to be included in the next release?

Re-uploading to run it through the latest tests.

Bumping to 2.11.

@DamienMcKenna, just to reconfirm per comment #42, this patch will cause a problem in the Calendar module if the patch for Issue #2673192 is not released at the same time.

@ron_s: Thanks for the clarification. I don't know if any of the Calendar maintainers are overly active..

FYI: Latest patch can be applied to the latest stable release of the date module and it still working, thanks alot!

Running the latest patch from here and the Calendar module as recommended and still getting an error.

Seems to work for everything except All Day events on the 7th day of the week in the week view.

@catapipper, please read the thread. This has been commented on several times by me.

@ron_s, sorry turns out the issue we are having is loosely related to this issue but is specific enough to have been solved by this patch alone.

@catapipper, re-read comments #42, #45, #48 and #57. There is a separate patch to resolve the issue to which you are referring.

It has nothing to do with the Date module. It is a Calendar module problem.

Here's an updated patch that applies to latest 7.x-2.x. Setting back to 'Needs review' to run tests.

Committed. Thank you all!

Just to avoid confusion:
I can confirm that the current dev version of the date module has the suggested patch, but it is not included in the latest release version 2.10. To use date 2.10 with calendar 3.5, I had to apply the two patches
https://www.drupal.org/files/issues/2018-08-21/date-n1137062-65.patch
https://www.drupal.org/files/issues/calendar-7thdayeventdisplay-1779142-...
to make items appear on day 7 in the week view.

patch #27 worked for me.