-
Notifications
You must be signed in to change notification settings - Fork 130
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Dealing with EXDATES in ITip\Broker & fix for #126 #205
Dealing with EXDATES in ITip\Broker & fix for #126 #205
Conversation
probably related to sabre-io#126
@@ -855,7 +855,10 @@ protected function parseEventInfo(VCalendar $calendar = null) { | |||
$sequence = $vevent->SEQUENCE->getValue(); | |||
} | |||
if (isset($vevent->EXDATE)) { | |||
$exdate = $vevent->EXDATE->getParts(); | |||
foreach ($vevent->select('EXDATE') as $val) { | |||
$exdate = array_merge($exdate, $val->getParts()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we consider the PRIORITY
property here to sort EXDATE
before merging them? Because order is important when merging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed via Slack yesterday you: there is no relation between PRIORITY and EXDATE. And the exdates get sorted (so we have earliest to latest in our variable) after merging values of all EXDATE-properties.
Did I miss something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, you are good 👍
anything missing for you, @evert? :) |
…ificantChange-check Dealing with EXDATES in ITip\Broker & fix for #126
Sorry for the delay armin. Just noticed this on my todo list from a few days ago :S |
This PR solves two issues:
EXDATE
present in an objectEXDATE
-values in a PUT, the$significantChangeHash
would imply a real change of the event happenedAlso see the initial issue & the thread in this commit.