Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Recurrence rule: last event missing #238

Closed
pikim opened this issue Aug 15, 2019 · 15 comments
Closed

Recurrence rule: last event missing #238

pikim opened this issue Aug 15, 2019 · 15 comments

Comments

@pikim
Copy link

pikim commented Aug 15, 2019

  • PHP Version: 7.2.10
  • PHP Default timezone: UTC
    • PHP date.timezone: no value
  • ICS Parser Version: 2.1.13
  • Linux

Description of the Issue:

The last event of a recurrence rule is missing. In the attached file is an recurring event with

RRULE:FREQ=DAILY;UNTIL=20191111T180000Z
DTSTART;TZID=Europe/Berlin:20191105T190000
DTEND;TZID=Europe/Berlin:20191105T220000

This should result in 7 events with the last one on November 11th. The parser returns 6 events with the last one on November 10th.

Steps to Reproduce:

Rename test.txt to *.ics and load it. Dump the returned events.

@u01jmg3 u01jmg3 self-assigned this Aug 15, 2019
@britweb-tim
Copy link

I'm seeing a similar issue here with an iCal feed. The last event in this series on Sunday 1st September is missing.

DTSTART;TZID=Europe/London:20190818T103000
DTEND;TZID=Europe/London:20190818T111500
RRULE:FREQ=WEEKLY;WKST=SU;UNTIL=20190902;BYDAY=SU

@u01jmg3
Copy link
Owner

u01jmg3 commented Aug 29, 2019

@s0600204: looks like there is an issue with the code for computing WEEKLY $matchingDays.

foreach ($matchingDays as $day) {
    $clonedDateTime = clone $frequencyRecurringDateTime;

    var_dump($clonedDateTime->format('c'));
    // 2019-08-18T10:30:00+01:00

    $candidateDateTimes[] = $clonedDateTime->setISODate(
        $frequencyRecurringDateTime->format('Y'),
        $frequencyRecurringDateTime->format('W'),
        $day
    );

    var_dump(end($candidateDateTimes)->format('c'));
    // 2019-08-11T10:30:00+01:00
}

$frequencyRecurringDateTime->format('W') + 1 fixes the issue but why would the week number be out by 1? This causes the final event to be incorrectly omitted.

@s0600204
Copy link
Collaborator

Hmm, yes, I see it.

It seems I need to rethink how to map different week starts onto a method (setISODate()) that expects weeks to start on a Monday. 🤔

The week number is correct - it's $day that's potentially incorrect: in the above example it will be 0 (which within setISODate() equates to the last day of the week previous to the one identified by the passed week number).

I should have time to look into it this weekend.

@u01jmg3
Copy link
Owner

u01jmg3 commented Aug 29, 2019

The week number is correct - it's $day that's potentially incorrect

Almost!

I should have time to look into it this weekend.

Perfect

u01jmg3 added a commit that referenced this issue Sep 2, 2019
Proposed fix for #238
@u01jmg3 u01jmg3 closed this as completed Sep 2, 2019
@pikim
Copy link
Author

pikim commented Sep 2, 2019

The original issue is not fixed, yet. Weekly rules may work now, but s0600204 didn't change anything in the code for daily rules.

@u01jmg3
Copy link
Owner

u01jmg3 commented Sep 2, 2019

I see 7 events from your iCal snippet as expected:

2019-09-02-12-46-localhost

@u01jmg3 u01jmg3 reopened this Sep 2, 2019
@pikim
Copy link
Author

pikim commented Sep 2, 2019

I only see 6 events.

My code

        // open and parse iCalendar files
        $ical = new ICal(
            "https://github.com/u01jmg3/ics-parser/files/2640806/test.txt",
            array(
                'defaultSpan'                 => 2,     // Default value
                'defaultTimeZone'             => 'UTC',
                'defaultWeekStart'            => 'MO',  // Default value
                'disableCharacterReplacement' => false, // Default value
                'filterDaysAfter'             => null,  // Default value
                'filterDaysBefore'            => null,  // Default value
                'replaceWindowsTimeZoneIds'   => false, // Default value
                'skipRecurrence'              => false, // Default value
                'useTimeZoneWithRRules'       => false, // Default value
            )
        );

        // get events sorted by date
        $events = $ical->sortEventsWithOrder($ical->events());
        dump($events);

gives me
6events

For details see: 6events.txt

@s0600204
Copy link
Collaborator

s0600204 commented Sep 2, 2019

May we ask what version of the parser you're currently running?

@u01jmg3
Copy link
Owner

u01jmg3 commented Sep 2, 2019

I am still getting 7 events as expected:

2019-09-02-14-29-localhost


Your code is almost identical to the defaults so nothing obviously wrong there. To note replaceWindowsTimeZoneIds and useTimeZoneWithRRules options are now obsolete and can be removed.

@pikim
Copy link
Author

pikim commented Sep 2, 2019

May we ask what version of the parser you're currently running?

I updated to v2.1.14 via Composer and replaced ICal.php manually with the one from e0e1b83 just to create the results from my last post. I was on v2.1.13 before.

@u01jmg3
Copy link
Owner

u01jmg3 commented Sep 2, 2019

To clarify, has dev-master (which is effectively what you are using) fixed your problem?

@pikim
Copy link
Author

pikim commented Sep 2, 2019

I took my base project (https://github.com/pikim/grav-plugin-events) updated the dependencies and replaced the ICal.php manually. You can see the output of this hack in #238 (comment), so the problem still exists.

I didn't try it your way. I'll try to find out how I have to put the lines from your link into my project. I'm not too familiar with Composer, yet. But is that supposed to change anything?

@s0600204
Copy link
Collaborator

s0600204 commented Sep 4, 2019

When I try it locally, I too get 7 events.


I know nothing about - let alone ever used - Grav CMS; but I thought I'd try setting up a basic install to see if perhaps there was some incompatibility with either Grav or @pikim's events plugin for it.

So, I acquired a copy of the current stable release of Grav (v1.6.15), along with a copy of the master branch of the events plugin, and installed them on an online webhost. I also manually replaced the ICal.php file within the plugin with the version from e0e1b83.

I had to make the following changes to classes/iCalendarProcessor.php of the events plugin to get it to parse the file...

@@ -159,11 +159,14 @@
         $location = $event->location;
         $description = $event->description;
         $last_modified = strtotime($event->last_modified);
-        $recurrence_id = strtotime($event->recurrence_id);
+        if (isset($event->recurrence_id)) {
+            $recurrence_id = strtotime($event->recurrence_id);
+        }
         $start = strtotime($event->dtstart);
         $end = strtotime($event->dtend);
 
         // split if element exists
+        $categories = array();
         if( isset($event->categories) ) {
             $categories = explode(',', $event->categories);
         }

...but made no other changes or altered any configuration from the defaults beyond providing the path to the remote file that I wished it to process (the same one as in the original post).

After selecting "Save" in the Admin layout, the events in the file were output across 7 files (one for each event): grav

@pikim have you made any changes to your plugin that you haven't made public?

@u01jmg3
Copy link
Owner

u01jmg3 commented Sep 10, 2019

@s0600204: thanks for checking things out further!

@pikim: closing this issue for now but happy to re-open if required

@u01jmg3 u01jmg3 closed this as completed Sep 10, 2019
@u01jmg3 u01jmg3 removed their assignment Sep 10, 2019
@u01jmg3 u01jmg3 added this to the v2.x.x milestone Sep 10, 2019
pikim added a commit to pikim/grav-plugin-events that referenced this issue Oct 5, 2019
@pikim
Copy link
Author

pikim commented Oct 5, 2019

This is strange. I only get 6 events from the input file, never 7. May this be caused by any timezone setting in PHP or any other setting from my hosting provider?

My result from the latest Github code without any modifications looks like this:
grafik

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants