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): iTipBroker message generation and testing #50843

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

SebastianKrupinski
Copy link
Contributor

@SebastianKrupinski SebastianKrupinski commented Feb 17, 2025

Summary

Refactored iTip broker message generation to improve logic and RFC compatibility

TODO

  • Add phpdoc to function
  • Add phpdoc parameter definitions for arrays

Checklist

Copy link
Member

@st3iny st3iny left a comment

Choose a reason for hiding this comment

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

The following case did not cause a reaction email being sent to the organizer:

  1. Organizer invites attendee (recurring, daily, 5 times).
  2. Attendee declines the second to last instance.

In general, no messages seem to get sent if an attendee accepts or declines a single instance.

Is this intended?

@SebastianKrupinski
Copy link
Contributor Author

Is this intended?

I don't think this has anything to do with this PR.

  1. Organizer invites attendee (recurring, daily, 5 times).

This should work just like any other scenario, I tested this on my end and the iTip message are being generated properly.

Example,

Screenshot 2025-02-24 140336

Screenshot 2025-02-24 140413

  1. Attendee declines the second to last instance.

This PR does not touch the Attendee portion of the iTipBroker, that is done in the parseEventForAttendee function and this only overloads the parseEventForOrganizer function which generates messages for organizer changes.

In general, no messages seem to get sent if an attendee accepts or declines a single instance.

I had the same problem, this is most likely a issue with your DEV instance, when our server tests run they empty out the appdata_* folder, this is the place where the contents of a sent message is saved and if the "appdata_/mail/mail_user" folder is missing the send process fails.

Screenshot 2025-02-24 140655

Screenshot 2025-02-24 140949

Screenshot 2025-02-24 141349

@st3iny
Copy link
Member

st3iny commented Feb 25, 2025

/backport to stable31

@st3iny
Copy link
Member

st3iny commented Feb 25, 2025

/backport to stable30

@susnux susnux added this to the Nextcloud 32 milestone Mar 2, 2025
Copy link
Member

@st3iny st3iny left a comment

Choose a reason for hiding this comment

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

Does not fix cancelling a single instance.

Consider two accounts: organizer and user.

  1. Organizer: Create recurring event, daily, 3 times and invite another user.
  2. User: Accept whole series.
  3. Organizer: Cancels an instance, e.g. the second one.

Expected: User will only see the second instance being cancelled.
Actual: The whole series is cancelled from user's POV.

I exported the events and was able to confirm that the organizer's event has a new instance with a cancelled state. The user's event however, does not contain a separate instance. Instead, the base event was cancelled.

@SebastianKrupinski
Copy link
Contributor Author

Does not fix cancelling a single instance.

Consider two accounts: organizer and user.

1. Organizer: Create recurring event, daily, 3 times and invite another user.

2. User: Accept whole series.

3. Organizer: Cancels an instance, e.g. the second one.

Expected: User will only see the second instance being cancelled. Actual: The whole series is cancelled from user's POV.

I exported the events and was able to confirm that the organizer's event has a new instance with a cancelled state. The user's event however, does not contain a separate instance. Instead, the base event was cancelled.

Okay, having a look. Disregard the last message, I tested the wrong thing

@SebastianKrupinski
Copy link
Contributor Author

Does not fix cancelling a single instance.

Consider two accounts: organizer and user.

1. Organizer: Create recurring event, daily, 3 times and invite another user.

2. User: Accept whole series.

3. Organizer: Cancels an instance, e.g. the second one.

Expected: User will only see the second instance being cancelled. Actual: The whole series is cancelled from user's POV.

I exported the events and was able to confirm that the organizer's event has a new instance with a cancelled state. The user's event however, does not contain a separate instance. Instead, the base event was cancelled.

LMAO. So I found the issue... You've found another bug that I didn't know existed...

So the iTipBroker is now generating proper messages for instances, but its not processing Cancellation instances properly...

The method at fault... Sabre\VObject\ITip\Broker::processMessageCancel()

    protected function processMessageCancel(Message $itipMessage, ?VCalendar $existingObject = null)
    {
        if (!$existingObject) {
            // The event didn't exist in the first place, so we're just
            // ignoring this message.
        } else {
            foreach ($existingObject->VEVENT as $vevent) {
                $vevent->STATUS = 'CANCELLED';
                $vevent->SEQUENCE = $itipMessage->sequence;
            }
        }

        return $existingObject;
    }

As you can see it does not check the iTipMessage for a RECURRANCE-ID, it just applies the cancellation to every instance...

@SebastianKrupinski SebastianKrupinski changed the title fix: iTipBroker message generation and testing fix(CalDAV): iTipBroker message generation and testing Mar 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants