sabre-io / vobject

:date: The VObject library for PHP allows you to easily parse and manipulate iCalendar and vCard objects
http://sabre.io/vobject/
BSD 3-Clause "New" or "Revised" License
570 stars 125 forks source link

jCal does not support `time` property type #185

Closed Hywan closed 8 years ago

Hywan commented 9 years ago

Try the following code:

VObject\Reader::readJson('["vcalendar", ["foo", {}, "time", "12:00:00"]]');

It should fail.

I can fix this in the following days.

evert commented 9 years ago

Why should it fail? it looks ok to me :)

ahackschmitz commented 9 years ago

just comparing to the data we use in our webapp, it should be:

VObject\Reader::readJson('["vcalendar", [["foo", {}, "time", "12:00:00"]]]');

With an additional Property:

VObject\Reader::readJson('["vcalendar", [["foo", {}, "time", "12:00:00"], ["bar", {}, "time", "13:00:00"]]]');
evert commented 9 years ago

Right.. it's missing that extra bracket.

evert commented 9 years ago

But I doubt the missing brackets is what @Hywan was talking about, no?

Hywan commented 9 years ago

@evert Humm, in fact, @armin-hackmann is right and I feel dumb on this one :-D. Consider I was tired. This is the official story.

Hywan commented 9 years ago

Well, the initial bug I was trying to detect is here though.

$component = VObject\Reader::readJson('["vcalendar", [["foo", {}, "time", "12:00:00"]]]');
$this->assertEquals(
    'BEGIN:VCALENDAR' . CRLF .
    'FOO;VALUE=TIME:120000' . CRLF .
    'END:VCALENDAR' . CRLF,
    VObject\Writer::write($component)
);

We have FOO;VALUE=TIME:12:00:00 instead of FOO;VALUE=TIME:120000. I can fix this bug I think. I encounter the same bug with xCal. There is no test for the time property type with jCal so far.

evert commented 9 years ago

That makes sense.. so basically the time property needs a new getJsonValue() / setJsonValue()

Hywan commented 9 years ago

@evert Exact! I can do that but after my xCal patch… or I can start a new branch. I will give it a shot today.

evert commented 9 years ago

I'm assuming this didn't happen yet, so I'll add a fix to the 3.3 branch as well for this :)

evert commented 9 years ago

Ok .. there's more going on here :) The property actually has a bunch of 'json' behavior. Specifically, it has the 'getJsonValue' fully implemented for vCards (not iCalendar).

I suspect that the time property in vCards is different than iCalendar. I also suspect that the extra colons are 100% legal in vCard, but probably not in iCalendar!

So what needs to happen is that we need to move this Time property from Sabre\VObject\Property\Time to Sabre\VObject\Property\VCard\Time and create a new Sabre\VObject\Property\ICalendar\Time, if what I'm saying is actually correct.

Hywan commented 9 years ago

@evert Will check Monday :-). Difficult weekend here :-p.