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): disable both iTip and iMip messages #49503

Merged
merged 1 commit into from
Dec 10, 2024

Conversation

SebastianKrupinski
Copy link
Contributor

@SebastianKrupinski SebastianKrupinski commented Nov 27, 2024

Summary

Adjusted logic to disable both iTip and iMip messages

Checklist

@SebastianKrupinski SebastianKrupinski added the 3. to review Waiting for reviews label Nov 27, 2024
@SebastianKrupinski SebastianKrupinski self-assigned this Nov 27, 2024
@SebastianKrupinski SebastianKrupinski force-pushed the fix/issue-48528-disable-itip-and-imip-messages-2 branch from dbb9365 to a8a3b24 Compare November 27, 2024 01:11
@SebastianKrupinski SebastianKrupinski marked this pull request as ready for review November 29, 2024 14:16
@SebastianKrupinski SebastianKrupinski force-pushed the fix/issue-48528-disable-itip-and-imip-messages-2 branch from a8a3b24 to d23f95e Compare November 29, 2024 14:55
Comment on lines 174 to 173
if ($vCal->VEVENT?->{'X-NC-DISABLE-SCHEDULING'}?->getValue() === 'true') {
$vCal->VEVENT->remove('X-NC-DISABLE-SCHEDULING');
$modified = true;
return;
}
Copy link
Member

Choose a reason for hiding this comment

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

The one-off property is weird to be honest. Let's try to move this into a custom header that's set by the client that pushes the event and doesn't want itip/imip to happen. That feels a bit more idiomatic to the dav world.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@kesselb
Copy link
Contributor

kesselb commented Dec 6, 2024

Works as expected.

I wonder why we are using a custom header instead of the Schedule-Reply header1?
It's implemented just in the line below ;)

Footnotes

  1. https://datatracker.ietf.org/doc/html/rfc6638#section-8.1

@SebastianKrupinski
Copy link
Contributor Author

Works as expected.

I wonder why we are using a custom header instead of the Schedule-Reply header1? It's implemented just in the line below ;)

Footnotes

1. [datatracker.ietf.org/doc/html/rfc6638#section-8.1](https://datatracker.ietf.org/doc/html/rfc6638#section-8.1) [↩](#user-content-fnref-1-825565710bf1bb8af44042a40c5fd8b0)

I agree, we could reuse this header property but according to the RFC it serves a different purpose...

The Schedule-Reply request header is used by a client to indicate to
   a server whether or not a scheduling operation ought to occur when an
   "Attendee" deletes a scheduling object resource.

So I don't know if using it for custom functionality on event creation is a good idea.

@ChristophWurst
Copy link
Member

Agree that it serves a different purpose and may lead to confusion. A custom header for this off-standard behavior sounds better IMO.

@ChristophWurst
Copy link
Member

/backport to stable30

@ChristophWurst
Copy link
Member

/backport to stable29

@kesselb kesselb added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Dec 10, 2024
@kesselb
Copy link
Contributor

kesselb commented Dec 10, 2024

@SebastianKrupinski please rebase, squash the fixup commits and hope for a friendly cypress ;)

@kesselb kesselb enabled auto-merge December 10, 2024 15:49
@SebastianKrupinski SebastianKrupinski force-pushed the fix/issue-48528-disable-itip-and-imip-messages-2 branch from fc48fc1 to 04cb122 Compare December 10, 2024 17:40
@SebastianKrupinski
Copy link
Contributor Author

/backport to stable28

@SebastianKrupinski
Copy link
Contributor Author

@SebastianKrupinski please rebase, squash the fixup commits and hope for a friendly cypress ;)

Lol. C'mon you know cypress is never friendly with anyone.

@kesselb kesselb merged commit 85f4f4f into master Dec 10, 2024
187 checks passed
@kesselb kesselb deleted the fix/issue-48528-disable-itip-and-imip-messages-2 branch December 10, 2024 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish
Projects
Development

Successfully merging this pull request may close these issues.

3 participants