Skip to content
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

Add TZ in iTip REPLY iTip messages #465

Merged

Conversation

renaudboyer
Copy link
Contributor

@renaudboyer renaudboyer commented Jul 11, 2019

Add VTIMEZONE objects in iTip REPLY if there were present in the original message.

Allows to have well-formatted iTip REPLY messages

@codecov
Copy link

codecov bot commented Jul 11, 2019

Codecov Report

Merging #465 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #465      +/-   ##
============================================
+ Coverage     98.86%   98.87%   +0.01%     
  Complexity     1815     1815              
============================================
  Files            65       65              
  Lines          5179     5178       -1     
============================================
  Hits           5120     5120              
+ Misses           59       58       -1
Impacted Files Coverage Δ Complexity Δ
lib/ITip/Broker.php 99.54% <100%> (+0.22%) 154 <0> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 26673bb...7642fd7. Read the comment docs.

@Julian1998
Copy link

@DeepDiver1975 Can you merge this one?

@DeepDiver1975
Copy link
Member

Looks good to me - I'd like to have a second pair of eyes - @staabm maybe? THX

@staabm staabm requested a review from evert December 9, 2019 12:38
@staabm
Copy link
Member

staabm commented Dec 9, 2019

@evert should have a look. I am not an expert in this area

@m-a-v
Copy link

m-a-v commented Dec 9, 2019

I still haven't managed to get correct REPLY for Microsoft Outlook or Google invitations (if sent as attendee). Should this work with this fix?

@Julian1998
Copy link

Julian1998 commented Dec 9, 2019

@m-a-v Can you validate your ics and tell what happened wrong by using https://icalendar.org/validator.html

Not sure what you mean by getting a correct REPLY in this context. But this is a fix for several clients which fail when rendering an event with a TZID set (and the VTIMEZONE object misses ofc)

@renaudboyer
Copy link
Contributor Author

We were working on Microsoft/Google compatibility issues when we've done this fix. I don't remember which brand was rejecting the reply messages because of invalid format.
If you're using TZ, it should fix your replies.

@renaudboyer renaudboyer force-pushed the add-broker-itip-process-tz-tests branch from 806a527 to 7642fd7 Compare January 3, 2020 11:09
@DeepDiver1975
Copy link
Member

@evert should have a look. I am not an expert in this area

@evert any chance to have a look at this? THX a lot

Copy link
Member

@evert evert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eying this, this looks good. Awesome that compatibility is still improving

@DeepDiver1975 DeepDiver1975 merged commit a26a998 into sabre-io:master Jan 8, 2020
@phil-davis phil-davis mentioned this pull request Jan 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants