Fix issues 197 & 208 #228
Fix issues 197 & 208 #228
Conversation
`DateTime->setTimezone` does not modify the underlying value of a DateTime. It changes how values passed to `DateTime->setDate`, `DateTime->setISODate`, and `DateTime->setTime` should be interpreted; and also what timezone to export a date in when using `DateTime->format`.
|
Thanks for this.
|
|
Okay (lengthy post, sorry): Adding the following code to the top of echo "Calendar timezone: " . $ical->calendarTimeZone() . "\n";
foreach ($ical->sortEventsWithOrder($ical->events()) as $event) {
if (!isset($event->dtstart_array[0]['TZID'])) {
continue;
}
echo $event->uid . ", " . $event->dtstart_array[0]['TZID'] . ", " . $event->dtstart_array[1] . ", " . $event->dtstart_tz . "\n";
}
die();Doing this twice, once without the patch and once with, will give results for before and after. Which, once checked to make sure we're lining up the correct events with each other, results in the following table (UIDs have been omitted, otherwise this table would be twice as wide as it currently is):
From this, we should be able to go down the list and check ouput sanity. The first event in the list is pre-1970, and in Los Angeles over in the U.S.. According to this online resource, on that date Los Angeles was eight-hours behind GMT/UTC, and thus the generated The next batch of events, all occurrences of the same starting event, have a CLDR timezone that mentions a -5 hr offset from UTC. Thus, transposing from local time to UTC, I would have expected the UTC time to be five hours ahead of Local time. However it isn't; instead being only four. Looking closer at how the parser handles this, the parser maps the given timezone to the IANA timezone Taking that into account, and assuming a UTC-4 offset (instead of the UTC-5 expected from the name of the timezone), the resultant dates look correct.
And finally, |
|
Extremely thorough analysis and just what I was looking for. 👍 To confirm, your patch has not negatively affected the 24 events listed and in some cases has corrected the time zone output? |
|
I believe so, yes. |
This changeset should resolve both issues #197 and #208 (which are the same problem).
I have not added a test to
composer test, as the files there are hard-coded to examine the contents ofDTSTART_array, and modifying the test scripts to be able to test any (other) parameter is beyond the scope of this PR.Instead, running the ICS fragments posted with the issues in question should be enough, with the following notes:
Issue #197:
DTSTART_tzshould be "20181204T180000".$defaultTimezonehas no effect on this, as the ics-parser assumes that that the singleVTIMEZONEgiven in the ics fragment is the calendar's time zone.VTIMEZONEstatement (or remove it) so that the parser determines the calendar's time zone is "Europe/Copenhagen", then the expected value ifDTSTART_tzwould be "20181205T000000".Issue #208:
DTSTART_tzof the remaining events should be: '20190322T144500', '20190313T093000', '20190314T133000', '20190314T140000', '20190313T113000', '20190313T153000', '20190313T100000' in that order.The last chunk in this patch isn't necessary to this fix, but is related cleanup.