Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
I can create this issue as follows:
Set up a date field on a content type with repeats.
Choose start date of 8/31/2017 at, say, 10:00pm
Choose to: date 9/01/2017 at 12:00am
Click repeat, repeats monthly, on the fifth thursday of (leave them all blank). Check stop repeating after 4 occurrences.
One of the dates it adds will be february 1 & 2.
Comment | File | Size | Author |
---|---|---|---|
#7 | date_repeat_1.patch | 1.05 KB | davery |
| |||
#6 | date_repeat_1.patch | 1.05 KB | davery |
| |||
#3 | date_repeat.patch | 1.15 KB | davery |
Comments
Comment #2
davery CreditAttribution: davery commentedMarking as critical because it duplicates for repeats of whatever day falls on february 1st of a non leap year. So for instance 5th friday leading into 2019, 5th thursday into 2018, etc.
A fix is as follows, in date_repeat_calc.inc
line 283:
$current_day->errors = array();
line 503:
line 575:
$date_in->errors[] = TRUE;
full diff:
Comment #3
davery CreditAttribution: davery commentedComment #4
davery CreditAttribution: davery commentedWill this test automatically?
Comment #5
davery CreditAttribution: davery commentedComment #6
davery CreditAttribution: davery commentedComment #7
davery CreditAttribution: davery commentedComment #8
davery CreditAttribution: davery commentedDifference between patch 1 & 2 was diff path.
Difference between 2 & 3 is a space to make it match coding standards.
It passed... I think reclassifying to "tested ..." is the appropriate step after tests run. Please commit or tell me the next steps.
Comment #9
DamienMcKenna@davery: Thank you for digging through this bug and providing a patch, it is much appreciated!
FYI the issue status you were looking for is "needs review", that will trigger the testbots and let others know there is a patch to look at; "Reviewed and tested by the community" is for when there's a patch that others have reviewed and collectively believe it solves the issue at hand.
I'll try to review this shortly.
Comment #10
DamienMcKennaChanging this to "major" as it doesn't break data, it's just inconvenient.
I also think it'd be useful to add tests to make sure the change works as intended.