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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

davery created an issue. See original summary.

davery’s picture

Priority: Normal » Critical

Marking 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:

  if(!empty($current_day->errors)) {
    return FALSE;
  }

line 575:
$date_in->errors[] = TRUE;

full diff:

date/date_repeat# git diff ./date_repeat_calc.inc 
diff --git a/date/date_repeat/date_repeat_calc.inc b/date/date_repeat/date_repeat_calc.inc
index d840df6..0ee66be 100644
--- a/date/date_repeat/date_repeat_calc.inc
+++ b/date/date_repeat/date_repeat_calc.inc
@@ -280,6 +280,7 @@ function _date_repeat_calc($rrule, $start, $end, $exceptions, $timezone, $additi
         foreach ($direction_days as $day) {
           // Find the BYDAY date in the current month.
           if ($rrule['FREQ'] == 'MONTHLY') {
+            $current_day->errors = array();
             $current_day = date_repeat_set_month_day($current_day, $day['day'], $day['direction_count'], $day['direction'], $timezone, $modify_time);
           }
           else {
@@ -499,6 +500,9 @@ function date_repeat_add_dates(&$days, $current_day, $start_date, $end_date, $ex
       return FALSE;
     }
   }
+  if(!empty($current_day->errors)) {
+    return FALSE;
+  }
   // Don't add a day if it is already saved so we don't throw the count off.
   if (in_array($formatted, $days)) {
     return TRUE;
@@ -568,6 +572,7 @@ function date_repeat_set_month_day($date_in, $day, $count = 1, $direction = '+',
       return $date;
     }
   }
+  $date_in->errors[] = TRUE;
   return $date_in;
 }
davery’s picture

davery’s picture

Will this test automatically?

davery’s picture

Title: Repeat date problem with 5th weekday in month » Repeat date problem with 5th weekday in february
davery’s picture

davery’s picture

davery’s picture

Status: Active » Reviewed & tested by the community

Difference 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.

DamienMcKenna’s picture

Version: 7.x-2.10 » 7.x-2.x-dev
Status: Reviewed & tested by the community » Needs review

@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.

DamienMcKenna’s picture

Priority: Critical » Major
Status: Needs review » Needs work
Issue tags: +Needs tests

Changing 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.