u01jmg3 / ics-parser

Parser for iCalendar Events • PHP 8+, 7 (≥ 7.4), 5 (≥ 5.6)
MIT License
448 stars 144 forks source link

Support the DTSTART of an event also being an EXDATE #246

Closed s0600204 closed 4 years ago

s0600204 commented 4 years ago

Potential fix for #240.

u01jmg3 commented 4 years ago

Thanks for this - don't seem to get the tests to pass:

1) RecurrencesTest::testStartDateIsExdateUsingUntil
Exception: DateTime::__construct(): Failed to parse time string (20191023T095000EXDATE;TZID=Europe/London:20191009T095000EXDATE;TZID=Europe/London:20190925T095000EXDATE;TZID=Europe/London:20190911T095000) at position 15 (E): The timezone could not be found in the database

C:\ics-parser\vendor\nesbot\carbon\src\Carbon\Carbon.php:555
C:\ics-parser\src\ICal\ICal.php:2169
C:\ics-parser\src\ICal\ICal.php:1278
C:\ics-parser\src\ICal\ICal.php:709
C:\ics-parser\src\ICal\ICal.php:541
C:\ics-parser\tests\RecurrencesTest.php:270
C:\ics-parser\tests\RecurrencesTest.php:208

2) RecurrencesTest::testStartDateIsExdateUsingCount
Exception: DateTime::__construct(): Failed to parse time string (20191023T095000EXDATE;TZID=Europe/London:20191009T095000EXDATE;TZID=Europe/London:20190925T095000EXDATE;TZID=Europe/London:20190911T095000) at position 15 (E): The timezone could not be found in the database

C:\ics-parser\vendor\nesbot\carbon\src\Carbon\Carbon.php:555
C:\ics-parser\src\ICal\ICal.php:2169
C:\ics-parser\src\ICal\ICal.php:1278
C:\ics-parser\src\ICal\ICal.php:709
C:\ics-parser\src\ICal\ICal.php:541
C:\ics-parser\tests\RecurrencesTest.php:270
C:\ics-parser\tests\RecurrencesTest.php:229
s0600204 commented 4 years ago

Hmm. Seems to be a whitespace issue stemming from trying to include multiple ical lines in one argument to RecurrencesTest.php's version of formatIcalEvent().

247 should make adding EXDATEs to tests in this file viable. Once that's been considered, I'll update this PR.

u01jmg3 commented 4 years ago

Testing now passes ✔️

However, there are a couple of differences between the example events that are produced before and after this PR.

Notably the Paris Timezone Test no longer appears. There are also slight differences in the output of the BYMONTHDAY Test (22-09-2016 13:00) event and the Parent Recurrence Event (13-08-2017 19:00). Is this expected?

s0600204 commented 4 years ago

Should be resolved, now.

The Paris Timezone Test was not being included due to it having an invalid RRULE (touched on in #236) and me forgetting to add a line to retain the event in this case.

The other two differences were interlinked. It appears that the code processing events with a RECURRENCE-ID is reliant on the entries of $this->cal['EVENTS'] array remaining at the same (numerical and otherwise arbitrary) index after recurrence-determination as before. If not, then the wrong event gets replaced.

Anyhow, $this->cal['EVENTS'] is now modified instead of replaced, which should solve both problems.

u01jmg3 commented 4 years ago

The Paris Timezone Test was not being included due to it having an invalid RRULE (touched on in #236) and me forgetting to add a line to retain the event in this case.

Will change to RRULE:FREQ=WEEKLY;BYDAY=WE

It appears that the code processing events with a RECURRENCE-ID is reliant on the entries of $this->cal['EVENTS'] array remaining at the same (numerical and otherwise arbitrary) index after recurrence-determination as before.

🤢