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

fix(CalDav): add support for Microsoft time zones #49459

Merged
merged 2 commits into from
Dec 9, 2024

Conversation

SebastianKrupinski
Copy link
Contributor

@SebastianKrupinski SebastianKrupinski commented Nov 25, 2024

Summary

  • Adds handling of Microsoft time zones, when generating iMip emails
  • Corrects some spelling

Checklist

@SebastianKrupinski SebastianKrupinski added the 3. to review Waiting for reviews label Nov 25, 2024
@SebastianKrupinski SebastianKrupinski self-assigned this Nov 25, 2024
@SebastianKrupinski SebastianKrupinski force-pushed the fix/issue-48079-windows-time-zones branch from b6b5030 to 8425ae8 Compare November 25, 2024 01:28
@SebastianKrupinski
Copy link
Contributor Author

/backport to stable30

Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

Very nice! Thank you

apps/dav/lib/CalDAV/TimeZoneFactory.php Show resolved Hide resolved
apps/dav/lib/CalDAV/TimeZoneFactory.php Outdated Show resolved Hide resolved
apps/dav/lib/CalDAV/TimeZoneFactory.php Outdated Show resolved Hide resolved
apps/dav/lib/CalDAV/TimeZoneFactory.php Show resolved Hide resolved
apps/dav/lib/CalDAV/TimeZoneFactory.php Outdated Show resolved Hide resolved
apps/dav/lib/CalDAV/TimeZoneFactory.php Outdated Show resolved Hide resolved
private $vCalendar2;
/** @var VCalendar */
private $vCalendar3;
private VCalendar $vCalendar1a;
Copy link
Member

Choose a reason for hiding this comment

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

nitpick: try to avoid this in bug fixes, because it may make the backport process more complicated. I suggest to either have a separate commit for refactorings, which can be left out of the backport, or do this in a separate PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I normally don't, I only did it in this fix because this class was only introduced in version 30 and there are no differences.

@ChristophWurst ChristophWurst added the pending documentation This pull request needs an associated documentation update label Nov 25, 2024
@ChristophWurst
Copy link
Member

Checklist

* Code is [properly formatted](https://docs.nextcloud.com/server/latest/developer_manual/digging_deeper/continuous_integration.html#linting)

* [Sign-off message](https://github.com/src-d/guide/blob/master/developer-community/fix-DCO.md) is added to all commits

*

@SebastianKrupinski please keep the original PR template from https://github.com/nextcloud/server/blob/master/.github/pull_request_template.md. Writing docs is very relevant here. Don't drop this.

Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

Looks good otherwise

apps/dav/lib/CalDAV/TimeZoneFactory.php Outdated Show resolved Hide resolved
@@ -0,0 +1,49 @@
<?php
Copy link
Member

Choose a reason for hiding this comment

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

strict types

@antondollmaier
Copy link

As written in #48079, thank you. This MR successfully resolves our issue!

@SebastianKrupinski SebastianKrupinski force-pushed the fix/issue-48079-windows-time-zones branch from 4fad4d8 to a925478 Compare December 9, 2024 17:40
@SebastianKrupinski
Copy link
Contributor Author

As written in #48079, thank you. This MR successfully resolves our issue!

Thanks for testing, you should see the fix, included in the next release of NC30

@solracsf solracsf added this to the Nextcloud 31 milestone Dec 9, 2024
@SebastianKrupinski SebastianKrupinski removed the pending documentation This pull request needs an associated documentation update label Dec 9, 2024
@SebastianKrupinski SebastianKrupinski merged commit 0c4794b into master Dec 9, 2024
188 checks passed
@SebastianKrupinski SebastianKrupinski deleted the fix/issue-48079-windows-time-zones branch December 9, 2024 18:41
@ChristophWurst
Copy link
Member

/backport to stable29

Copy link

backportbot bot commented Dec 13, 2024

The backport to stable30 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable30
git pull origin stable30

# Create the new backport branch
git checkout -b backport/49459/stable30

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts, resolve them
git cherry-pick a9254788 58b04fa7

# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/49459/stable30

Error: No changes found in backport branch


Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.

@solracsf
Copy link
Member

/backport to stable29

Copy link

backportbot bot commented Dec 13, 2024

The backport to stable29 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable29
git pull origin stable29

# Create the new backport branch
git checkout -b backport/49459/stable29

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts, resolve them
git cherry-pick a9254788 58b04fa7

# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/49459/stable29

Error: No changes found in backport branch


Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.

@SebastianKrupinski
Copy link
Contributor Author

/backport to stable29

This does not exist in 29, 30+ only

@skjnldsv skjnldsv mentioned this pull request Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

[Bug]: DateTimeZone::__construct(): Unknown or bad timezone (W. Europe Standard Time)
5 participants