fix: add VTIMEZONE to Appointments by miaulalala · Pull Request #5339 · nextcloud/calendar

miaulalala · 2023-06-27T13:02:17Z

fixes part of #5031

Tested it but this doesn't fix the diverging timezones. Still, it makes us RFC compliant which is always a good thing, and might fix Outlook, although I don't have an Outlook setup to test.

Before:

BEGIN:VCALENDAR
VERSION:2.0
PRODID:-//Sabre//Sabre VObject 4.5.3//EN
CALSCALE:GREGORIAN
BEGIN:VEVENT
UID:sabre-vobject-4d193331-955f-4041-a7b9-44b14809b030
DTSTAMP:20230914T191309Z
SUMMARY:Bob - Have Lunch With Me
STATUS:CONFIRMED
DTSTART;TZID=Europe/Vienna:20230918T120000
DTEND;TZID=Europe/Vienna:20230918T123000
DESCRIPTION:Testing
ORGANIZER;CN=Anna Admin;CUTYPE=INDIVIDUAL;PARTSTAT=ACCEPTED:mailto:xxxxx
 [email protected]
ATTENDEE;CN=Anna Admin;CUTYPE=INDIVIDUAL;RSVP=TRUE;ROLE=REQ-PARTICIPANT;PAR
 TSTAT=ACCEPTED:mailto:[email protected]
ATTENDEE;CN=Bob;CUTYPE=INDIVIDUAL;RSVP=TRUE;ROLE=REQ-PARTICIPANT;PARTSTAT=A
 CCEPTED:mailto:[email protected]
LOCATION:Abigail's Kitchen
X-NC-APPOINTMENT:iHFJtxxcrW8g
END:VEVENT
END:VCALENDAR

After:

BEGIN:VCALENDAR
VERSION:2.0
PRODID:-//Sabre//Sabre VObject 4.5.3//EN
CALSCALE:GREGORIAN
BEGIN:VTIMEZONE
TZID:Europe/Vienna
BEGIN:DAYLIGHT
DTSTART:20230326T010000
TZOFFSETFROM:+0100
TZOFFSETTO:+0200
TZNAME:CEST
END:DAYLIGHT
BEGIN:DAYLIGHT
DTSTART:20240331T010000
TZOFFSETFROM:+0100
TZOFFSETTO:+0200
TZNAME:CEST
END:DAYLIGHT
BEGIN:STANDARD
DTSTART:20231029T010000
TZOFFSETFROM:+0200
TZOFFSETTO:+0100
TZNAME:CET
END:STANDARD
END:VTIMEZONE
BEGIN:VEVENT
UID:sabre-vobject-27752eed-efcb-4972-8cd5-84f78b65c6ae
DTSTAMP:20231102T134259Z
SUMMARY:Test - Have Lunch With Me
STATUS:CONFIRMED
DTSTART;TZID=Europe/Vienna:20231106T120000
DTEND;TZID=Europe/Vienna:20231106T123000
DESCRIPTION:Testing
ORGANIZER;CN=Anna Admin;CUTYPE=INDIVIDUAL;PARTSTAT=ACCEPTED:mailto:[email protected]
ATTENDEE;CN=Anna Admin;CUTYPE=INDIVIDUAL;RSVP=TRUE;ROLE=REQ-PARTICIPANT;PAR
 TSTAT=ACCEPTED:mailto:[email protected]
ATTENDEE;CN=Test;CUTYPE=INDIVIDUAL;RSVP=TRUE;ROLE=REQ-PARTICIPANT;PARTSTAT=
 ACCEPTED:mailto:[email protected]
LOCATION:Abigail's Kitchen
X-NC-APPOINTMENT:iHFJtxxcrW8g
END:VEVENT
END:VCALENDAR

ChristophWurst

Is it sufficient to only have TZID or do we need the full tz spec?

codecov · 2023-06-27T13:05:42Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (33e236c) 24.43% compared to head (496896a) 24.67%.
Report is 2 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #5339      +/-   ##
============================================
+ Coverage     24.43%   24.67%   +0.24%     
- Complexity      388      406      +18     
============================================
  Files           240      241       +1     
  Lines         10643    10693      +50     
  Branches       1739     1739              
============================================
+ Hits           2601     2639      +38     
- Misses         8042     8054      +12

Flag	Coverage Δ
javascript	`15.67% <ø> (ø)`
php	`62.36% <ø> (+0.33%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

ChristophWurst · 2023-06-27T13:05:56Z

unassigning milestone because this will go into main

miaulalala · 2023-06-27T13:08:25Z

Is it sufficient to only have TZID or do we need the full tz spec?

I need to look into the sabre code more but having the full timezone info could be good to have.

miaulalala · 2023-06-29T10:02:55Z

With https://gist.github.com/thomascube/47ff7d530244c669825736b10877a200 it works fine, for timezones that have offests it also works well.

ChristophWurst

Could we have a test or two for the new timezone generation?

lib/Service/Appointments/BookingCalendarWriter.php

ChristophWurst · 2023-07-06T10:44:19Z

It could make sense to create a new class for the tz generation. That makes testing easier and removes complexity from the appointment code.

lib/Service/Appointments/TimezoneGenerator.php

ChristophWurst · 2023-07-21T08:02:19Z

The generator looks great! Let's add some unit tests :)

toabi · 2023-10-27T08:21:44Z

Hi, what's the status about this? We have lots of issues with managing events with Customers using outlook… they don't really read text and just trust their Outlook Events. Which are currently offset by 1/2 hours… probably because of this bug.

miaulalala · 2023-10-30T11:08:58Z

This needs some tests, then we're good to merge. I hope to get this in before the next release on thursday.

miaulalala · 2023-10-30T11:09:09Z

/backport to stable4.5

ChristophWurst

Code looks good. Extraneous classes will byte us when they change

lib/Service/Appointments/TimezoneGenerator.php

Signed-off-by: Anna Larch <[email protected]>

ChristophWurst · 2023-11-07T13:49:02Z

lib/Service/Appointments/TimezoneGenerator.php

+ *  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  * GNU AFFERO GENERAL PUBLIC LICENSE for more details.
+ *  *
+ *  * You should have received a copy of the GNU Affero General Public
+ *  * License along with this library.  If not, see <http://www.gnu.org/licenses/>.
+ *  *
+ *
+ */
+
+declare(strict_types=1);
+
+/*
+ * @copyright 2023 Anna Larch <[email protected]>
+ *
+ * @author 2023 Anna Larch <[email protected]>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as


is this what they call dual licensing?

miaulalala added 2. developing Work in progress bug labels Jun 27, 2023

miaulalala added this to the v4.4.2 milestone Jun 27, 2023

miaulalala self-assigned this Jun 27, 2023

ChristophWurst reviewed Jun 27, 2023

View reviewed changes

ChristophWurst removed this from the v4.4.2 milestone Jun 27, 2023

miaulalala mentioned this pull request Jun 29, 2023

Outlook invite with time one hour minus from events of timezone America/Sao_Paulo #4429

Open

miaulalala requested review from ChristophWurst and tcitworld June 29, 2023 10:20

miaulalala added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jul 4, 2023

ChristophWurst reviewed Jul 6, 2023

View reviewed changes

lib/Service/Appointments/BookingCalendarWriter.php Outdated Show resolved Hide resolved

miaulalala requested a review from ChristophWurst July 20, 2023 16:08

miaulalala marked this pull request as draft July 20, 2023 16:08

ChristophWurst reviewed Jul 21, 2023

View reviewed changes

lib/Service/Appointments/TimezoneGenerator.php Outdated Show resolved Hide resolved

lib/Service/Appointments/TimezoneGenerator.php Outdated Show resolved Hide resolved

backportbot-nextcloud bot added the backport-request A backport was requested for this pull request label Oct 30, 2023

miaulalala force-pushed the fix/add-tz-to-appointment-ics branch from 33994a5 to cf64714 Compare November 2, 2023 13:11

miaulalala marked this pull request as ready for review November 2, 2023 13:29

miaulalala requested review from ChristophWurst and kesselb November 2, 2023 13:29

ChristophWurst approved these changes Nov 2, 2023

View reviewed changes

lib/Service/Appointments/TimezoneGenerator.php Show resolved Hide resolved

miaulalala mentioned this pull request Nov 2, 2023

\OCA\Calendar\Service\Appointments\BookingCalendarWriter depends on Sabre #3728

Open

miaulalala force-pushed the fix/add-tz-to-appointment-ics branch from 0597477 to a4347de Compare November 2, 2023 13:49

fix: add VTIMEZONE to Appointments

496896a

Signed-off-by: Anna Larch <[email protected]>

miaulalala force-pushed the fix/add-tz-to-appointment-ics branch from d68f9b2 to 496896a Compare November 7, 2023 12:18

ChristophWurst approved these changes Nov 7, 2023

View reviewed changes

ChristophWurst merged commit 27c4402 into main Nov 7, 2023

ChristophWurst deleted the fix/add-tz-to-appointment-ics branch November 7, 2023 13:49

backportbot-nextcloud bot mentioned this pull request Nov 7, 2023

[stable4.5] fix: add VTIMEZONE to Appointments #5564

Merged

backportbot-nextcloud bot removed the backport-request A backport was requested for this pull request label Nov 7, 2023

ChristophWurst mentioned this pull request Dec 11, 2023

Create API/service to build VTIMEZONE components for events nextcloud/server#42145

Open

Comments

Conversation

miaulalala commented Jun 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ChristophWurst left a comment

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Jun 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ChristophWurst commented Jun 27, 2023

Uh oh!

miaulalala commented Jun 27, 2023

Uh oh!

miaulalala commented Jun 29, 2023

Uh oh!

ChristophWurst left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ChristophWurst commented Jul 6, 2023

Uh oh!

Uh oh!

Uh oh!

ChristophWurst commented Jul 21, 2023

Uh oh!

toabi commented Oct 27, 2023

Uh oh!

miaulalala commented Oct 30, 2023

Uh oh!

miaulalala commented Oct 30, 2023

Uh oh!

ChristophWurst left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ChristophWurst Nov 7, 2023

Choose a reason for hiding this comment

Uh oh!

kesselb Nov 7, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

miaulalala commented Jun 27, 2023 •

edited

Loading

codecov bot commented Jun 27, 2023 •

edited

Loading