fixes all day events and adds getter for Event::getDtStart

[buggedcom]

All day events should always end the next day. Additionally the tzid should not be applied to all day events.

And added a missing getDtStart getter function to the Event component.

[markuspoerschke]

Thanks for the contribution!

[Seldaek]

Since we upgraded to 0.11.3 from 0.10.1 all the all-day events in our feeds now span one day more than they should.. I don't know what this bugfix was based on, but I think it's fine for all day events to start and end on the same day. The +1 day that's done in this PR seems to mess up the feeds.

I'd be curious to hear why it was done, and whether it can be reverted.

[markuspoerschke]

@Seldaek does that affect only "single-day" events or also "multi-day" events?

According to the RFC the dtend is "not-inclusive".

  The following is an example of the "VEVENT" calendar component
  used to represent an anniversary that will occur annually:

   BEGIN:VEVENT
   UID:19970901T130000Z-123403@example.com
   DTSTAMP:19970901T130000Z
   DTSTART;VALUE=DATE:19971102
   SUMMARY:Our Blissful Anniversary
   TRANSP:TRANSPARENT
   CLASS:CONFIDENTIAL
   CATEGORIES:ANNIVERSARY,PERSONAL,SPECIAL OCCASION
   RRULE:FREQ=YEARLY
   END:VEVENT

  The following is an example of the "VEVENT" calendar component
  used to represent a multi-day event scheduled from June 28th, 2007
  to July 8th, 2007 inclusively.  Note that the "DTEND" property is
  set to July 9th, 2007, since the "DTEND" property specifies the
  non-inclusive end of the event.

   BEGIN:VEVENT
   UID:20070423T123432Z-541111@example.com
   DTSTAMP:20070423T123432Z
   DTSTART;VALUE=DATE:20070628
   DTEND;VALUE=DATE:20070709
   SUMMARY:Festival International de Jazz de Montreal
   TRANSP:TRANSPARENT
   END:VEVENT

I will check that also for myself tomorrow.

[Seldaek]

Here is a sample of what we got after upgrading the lib (in pink) and importing the new feed, so yes all all-day events became one day longer. At least in outlook. I haven't checked other tools in depth.

I just find it kinda weird that this would not have been noticed as a problem before now if all-day events were exported to ical a day short. @buggedcom Did you do the fix because you hit an issue or was it just out of spec-compliance's sake?

[markuspoerschke]

@Seldaek hey, I checked it again with Calendar.

This was my testcase (based on example1.php):

<?php

// use composer autoloader
require_once __DIR__ . '/../vendor/autoload.php';

// set default timezone (PHP 5.4)
date_default_timezone_set('Europe/Berlin');

// 1. Create new calendar
$vCalendar = new \Eluceo\iCal\Component\Calendar('www.example.com');

// 2. Create an event
$vEvent = new \Eluceo\iCal\Component\Event();
$vEvent->setDtStart(new \DateTime('2017-12-24'));
$vEvent->setDtEnd(new \DateTime('2017-12-26'));
$vEvent->setNoTime(true);
$vEvent->setSummary('Christmas: 24., 25. and 26. of December');

// Adding Timezone (optional)
$vEvent->setUseTimezone(true);

// 3. Add event to calendar
$vCalendar->addComponent($vEvent);

// 4. Set headers
header('Content-Type: text/calendar; charset=utf-8');
header('Content-Disposition: attachment; filename="cal.ics"');

// 5. Output
echo $vCalendar->render();

That produces the following output:

BEGIN:VCALENDAR
VERSION:2.0
PRODID:www.example.com
X-PUBLISHED-TTL:P1W
BEGIN:VEVENT
UID:5921f93baf54a
DTSTART;VALUE=DATE:20171224
SEQUENCE:0
TRANSP:OPAQUE
DTEND;VALUE=DATE:20171227
SUMMARY:Christmas: 24.\, 25. and 26. of December
CLASS:PUBLIC
X-MICROSOFT-CDO-ALLDAYEVENT:TRUE
DTSTAMP:20170521T223155Z
END:VEVENT
END:VCALENDAR

Imported into calendar it looks fine (my intention was to create a multi-day event on 24th, 25th and 26th of december:

The other event in green was a test for a single-day event on 24th of december.

Can you confirm the same usage of the ->setDtEnd() method and can you check that piece of code in Outlook? (I have none.)

[Seldaek]

Oh well, now I feel silly but I looked at our code to see where we declare all day (noTime) events, and I see we do this:

            $formattedEvent->setUseUtc(true);
            $formattedEvent->setNoTime(true);
            $formattedEvent->getDtEnd()->add(new \DateInterval('P1D'));

So I guess that last line was added as a workaround for the bug, and obviously it causes issues now the bug is fixed.. Sorry about the noise, I wasn't aware of that. The fix looks good then :)

[markuspoerschke]

@Seldaek haha, no worries. Actually the API in this package failed because it is confusing. When I will work on the documentation I will add some propper explanation about the meaning of the methods.