fix(CalDAV): iTipBroker message generation and testing

[SebastianKrupinski]

Summary

Resolves: nextcloud/calendar#7057
Refactored iTip broker message generation to improve logic and RFC compatibility

Testing

How to test, open calendar app, and test the following

🔵 Single Event Operations

Basic Lifecycle

• Create new event with attendees
• Update event
• Delete event
• Cancel event (STATUS=CANCELLED)

Attendee Management

• Add new attendee to existing event
• Remove attendee from event
• Remove all attendees (convert to non-scheduling)

🟣 Recurring Event Operations

Master Instance

• Create recurring event (daily/weekly/monthly)
• Update master event
• Cancel master instance (entire series)
• Delete master instance
• Add EXDATE to master
• Remove EXDATE from master
• Convert recurring event to non-scheduling

Exception Instances

• Create exception (modify single occurrence time)
• Create exception (modify single occurrence summary)
• Update existing exception
• Cancel single occurrence (create cancelled exception)
• Create new exception that is already cancelled

🟠 Partial Attendee Lists (EXDATE Logic)

Critical EXDATE Scenarios

• Remove attendee from single occurrence (verify EXDATE in their REQUEST)
• Add attendee to single occurrence only (verify they get only exception)
• Different attendees on different exceptions:
  • Master: A + B + C
  • Exception 1: A + B only
  • Exception 2: A + C only
  • Verify A gets master + both exceptions
  • Verify B gets master + exception 1 + EXDATE for exception 2
  • Verify C gets master + exception 2 + EXDATE for exception 1

🔴 Edge Cases

Organizer & Empty States

• Organizer is also an attendee (verify organizer doesn't receive message)
• Create event with no attendees
• Remove all attendees from event

✅ Verification Per Test

For each test above, verify:

Message Method

• REQUEST for updates/additions
• CANCEL for removals/cancellations

Recipients

• All appropriate attendees receive messages
• No duplicate messages sent
• Organizer doesn't receive self-invites
• SCHEDULE-AGENT=CLIENT attendees skipped

Message Content

• All properties correctly reflected
• EXDATE properly added for partial attendee lists

Instance Selection

• Master included when appropriate
• Exceptions included when appropriate
• Cancelled instances not in REQUEST (only in CANCEL)

TODO

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

Checklist

• Code is properly formatted
• Sign-off message is added to all commits
• Tests (unit, integration, api and/or acceptance) are included
• Screenshots before/after for front-end changes
• Documentation (manuals or wiki) has been updated or is not required
• Backports requested where applicable (ex: critical bugfixes)

[st3iny]

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]

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,

2. 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.

[st3iny]

/backport to stable31

[st3iny]

/backport to stable30

[st3iny]

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]

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]

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...

[ChristophWurst]

Remove EXDATE from master

How do I do this from the Calendar UI?

[SebastianKrupinski]

so, feel free to create new issues out of them or tell me which ones need a new issue, so I don't create duplicates. I'm happy to create them so we can fix that in the future.

I'm sorry if that way of testing / documenting creates any kind of frustration or confusion on your side, that wasn't my intention.

I will retest this, this was working originally, but I modified some code when I was testing all the scenarios.

Is this now fixed with the changes you pushed right after the comment?

If you received a invitation then your test was successful.

Alright, I did. So, test successful.

As for the 2 events with the same UID this is correct behavior as per RFC, this is also how the current code works, event modification are stored inside the same event, as a modification.

Ahh, I see. It was just my first best guess without further digging into the RFC. This point might be out-of-scope anyway, but that's something that works with Google Calendars. So, not broken in general with Thunderbird (although Google seem to have handled that the same way we do in the past). When changing an event of a series (or any other changes), Thunderbird shows the accept/tentative/decline selection instead of just showing the "already processed" message. But it looks like they handle it differently as the change ics only contains the VEVENT with SEQUENCE:1? Maybe TB only looks for the first found VEVENT inside the ics file to decide if is already being processed or not? I don't know, it's up to you. I just want to mention it for housekeeping. Let me know if I should create a new issue for that.
First invitation to event series via Google Calendar

Invitation to a changed event within the series via Google Calendar

This is out of scope of this PR. This sounds like a UI issue in Calendar. Also I was not able to reproduce this, are you working with a up to date version of calendar?

I've used the main branch and built the UI locally before the test with node v20. Might be something wrong with my local build if you're unable to reproduce.

[SebastianKrupinski]

@DerDreschner

Looks like all this testing has found a bug in some unrelated code.

• ⛔ Attendees that are being removed from an event get no notification at all.

This is being caused buy a additional comparison in the email sending function that does not check for attendee changes properly

It's technically not related to this PR, you can see by the image above that the iTip message is generated properly as a cancel (that is what this PR addresses), the email sending plugin then ignore the itip message.

[DerDreschner]

Alright, then I'll give it a go! 👍

[SebastianKrupinski]

@DerDreschner Thank you for testing this! I know it was a lot! But you have finally brought a years long PR to a close

[DerDreschner]

@SebastianKrupinski : Yeah, finally 🎉 Sorry for being a bit aggressive with reporting bugs. I just have to get used to test against a stable release before writing it down. 😅

[SebastianKrupinski]

/backport to stable33