Attach ICS to calendar update notifications

[dantefromhell]

The calendar module now has ICS file attachments in notification emails (#436 and #439) but these only trigger for newly created events, not when an event was updated.

My first primitive tests adjusting #436 to https://github.com/humhub/calendar/blob/master/notifications/EventUpdated.php in combination with a ProtonMail & Google hosted calendar resulted in duplicate events, instead of an updated event.

According to this Google Calendar Help article the SEQ field must be increased in the updated ICS to trigger updating an event.

In my tests the SEQ field has been 0 for all notifications sent.

If I understand interfaces/VCalendar.php#L183 correctly the SEQUENCE field is only added to the ICS field if getSequence actually returns a number, but this is where I've been getting lost in the code.

I'm happy to either make this a bug report or with some support figure out what the correct PR needs to look like.

Please advise on what the best solution is.

[luke-]

@yurabakhtin Can you please take a look into this?

[yurabakhtin]

According to this Google Calendar Help article the SEQ field must be increased in the updated ICS to trigger updating an event.

@dantefromhell Do you mean we should increase the column calendar_entry.sequence on each updating of the Calendar Entry?
Maybe the field UID with value like humhub-calendar_event-b23ed8b8-f26a-4950-842d-c45aba009898 should be used for update event.

[dantefromhell]

@yurabakhtin

Maybe the field UID with value like humhub-calendar_event-b23ed8b8-f26a-4950-842d-c45aba009898 should be used for update event.

I was trying to find the sentence in RFC5545 which talks about this, but somehow wasn't able to.

TL;DR from reading more (non-official e.g. [1]) documentation:
The UID field must remain fixed throughout the lifetime of a calendar event.

Do you mean we should increase the column calendar_entry.sequence on each updating of the Calendar Entry?

RFC5545 is pretty precise how the SEQ field is supposed to work:
https://datatracker.ietf.org/doc/html/rfc5545#section-3.8.7.4

TL;DR: It starts with SEQ=0 and will be incremented on every change to the event.
From my incomplete understanding of the code, yes this should be implemented as calendar_entry.sequence + 1

[1] https://devguide.calconnect.org/Data-Model/Simple-Event/#:~:text=An%20ics%20file%20is%20a,contains%20one%20or%20more%20events.&text=Then%20BEGIN%20and%20END%20of,one%20VCALENDAR%20object%20per%20file.

FYI I found a nice ICS validator tool in case that is helpful https://icalendar.org/validator.html#results

[yurabakhtin]

RFC5545 is pretty precise how the SEQ field is supposed to work: https://datatracker.ietf.org/doc/html/rfc5545#section-3.8.7.4

TL;DR: It starts with SEQ=0 and will be incremented on every change to the event. From my incomplete understanding of the code, yes this should be implemented as calendar_entry.sequence + 1

@dantefromhell Thank you for the info, yes, you are right.

I have found and tested this code https://github.com/humhub/calendar/blob/master/models/forms/CalendarEntryForm.php#L515-L519:

$incrementSequence = $this->original->getStartDateTime() != $this->entry->getStartDateTime();
$incrementSequence = $incrementSequence || $this->original->getEndDateTime() != $this->entry->getEndDateTime();
$incrementSequence = $incrementSequence || $this->original->getRrule() !== $this->entry->getRrule();
$incrementSequence = $incrementSequence || $this->original->getExdate() !== $this->entry->getExdate();
$incrementSequence = $incrementSequence || $this->original->getEventStatus() !== $this->entry->getEventStatus();

It means the column sequence is increased only when one of the field is updated.

Do you want to extend the list, for example, should we increase the sequence when a title or a description or some other field?

[dantefromhell]

Do you want to extend the list, for example, should we increase the sequence when a title or a description or some other field?

Reading the RFCs my answer is yes. I propose that end-user calendars (e.g. Google) should be able to detect any event changes correctly, which I guess translates into updating SEQ for any change to a field that gets exported in the ICS file.

Or maybe it's easier to just update the SEQ field internally on any event changes without needing to list fields explicitly? 🤔

[yurabakhtin]

@dantefromhell Ok, thanks, I have extened the list of fields.
@luke- PR #449.

[luke-]

@yurabakhtin @dantefromhell Thanks, I've just merged the PR