ICS Export: export by componentType #568
Conversation
| @@ -200,7 +209,7 @@ function($item) use ($path) { | |||
| // Flattening the arrays | |||
| foreach($nodes as $node) { | |||
| if (isset($node[200][$calDataProp])) { | |||
| $blobs[] = $node[200][$calDataProp]; | |||
| $blobs[$node['href']] = $node[200][$calDataProp]; | |||
There was a problem hiding this comment.
To (at a later point) be able to add the object's uri to the output. That way, updates to the event could be sent without 'guessing' the uri (calendar.uri & object.uid) which might fail.
There was a problem hiding this comment.
So is this actually used anywhere? Not within sabre/dav right?
There was a problem hiding this comment.
Nope, at least not yet ;)
There was a problem hiding this comment.
Ok, so if the behavior changes, this change should also be documented in mergeObjects.
|
Needs a unittest |
| $componentType = 'VEVENT'; | ||
| } | ||
| if (isset($queryParams['componentType'])) { | ||
| if (!in_array($queryParams['componentType'], ['VEVENT', 'VTODO', 'VJOURNAL'])) { |
There was a problem hiding this comment.
shouldn't it be validated in generateResponse directly instead?
There was a problem hiding this comment.
Well, the other validation also happens here.
There was a problem hiding this comment.
since the method is only used once, it doesn't matter. is the generateResponse() method protected by design? or should it be better private?
There was a problem hiding this comment.
We actually subclass this in fruux, so we can add additional caching. I think that that was also the original use-case for splitting it into a second method in the first place.
There was a problem hiding this comment.
so your subclass may also benefit from moving the validation into generateRespone;)
There was a problem hiding this comment.
Actually.. no. It would make it more cumbersome as the validation would have to be repeated in our generateResponse.
There was a problem hiding this comment.
ah right.. in case you only override generateResponse its fine as is.
|
Added unit tests and also added a reference in mergeObjects. |
|
Currently unittests are failing! |
|
damnit, didn't run the tests after the latest changes, will check immediately |
This reverts commit 532a76d.
|
@armin-hackmann Just out of curiosity.. did you not get an email from travis? |
|
nope, I didn't. Commit is reversed, tests run successfully again. |
|
Do you have an account there or email settings? Could be useful :) |
ICS Export: export by componentType
No description provided.