feat(dav)!: add webhook compatibility for calendar object events

[edward-ly]

Summary

Adds webhook compatibility to event whenever a new calendar object (e.g. calendar event) is created via implementation of OCP\EventDispatcher\IWebhookCompatibleEvent.

TODO

• Add new events to webhook listeners manual page
• (possibly others, let me know if I'm missing anything)

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)

[ChristophWurst]

Is this going to be a public API?

The events are in OCA, not OCP. There are no compatibility or stability guarantees in private app namespaces.

[edward-ly]

Is this going to be a public API?

The events are in OCA, not OCP. There are no compatibility or stability guarantees in private app namespaces.

Ah right, I forgot about that. So if webhooks can only listen to events in the public namespace, would adding a new namespace such as OCP\Calendar\Events and creating new events that extend the private ones be a viable solution?

EDIT: actually, in the webhook listeners documentation, there are some webhook events that are still in the OCA namespace, so I'm not sure if the workaround is really necessary.

[ChristophWurst]

EDIT: actually, in the webhook listeners documentation, there are some webhook events that are still in the OCA namespace, so I'm not sure if the workaround is really necessary.

That is true and worrying to me, but on the other hand not my responsibility.

For caldav and carddav I will require a strict separation of private and public API, so we can continue to make easy modifications to internals, and have a solid, stable API for selected classes.

So yes, if these events are consumed by other apps, please move them to OCP. Do not extend private types from the public API. Just move them.

[ChristophWurst]

See above. The goal of this PR is to expose a public API, which internal events aren't.

Thanks you for you contribution

[AndyScherzinger]

/backport to stable31

[ChristophWurst]

@AndyScherzinger are we backporting new APIs for this?

[AndyScherzinger]

In this case yes, else the customer needs to wait until v32 enterprise.

[edward-ly]

I think I have everything setup now, but let me know if I'm missing anything.

[ChristophWurst]

Looks good 👍

[tcitworld]

My 2 cents: I get the customer specific need, but it would be nice to have all events (not just calendar object creation) at the same time targeting the same API level.

[ChristophWurst]

I'm not sure I get your message. Do you mean also moving/exposing the other calendar events?

[edward-ly]

Added the remaining events for the calendar objects now, will get to the documentation shortly.

[ChristophWurst]

Nice!

[st3iny]

We are gonna break some apps I think. But it's fine as the OCA stuff is not a public API.

[come-nc]

We are gonna break some apps I think. But it's fine as the OCA stuff is not a public API.

Still, make sure that it’s part of the ugrade guide for apps.

[tcitworld]

Seeing this remark on the above mentioned issue by @JonathanTreffler

it is a bit weird to have such generically named public events, that are actually fixed to the default calendar backend and therefore not as generic as the name would suggest

That's correct, OCP events should be able to be emitted by any app (not just DAV\CalendarBackend). To distinguish between those, maybe we should reference the calendar provider class inside events as well?

[come-nc]

Seeing this remark on the above mentioned issue by @JonathanTreffler

it is a bit weird to have such generically named public events, that are actually fixed to the default calendar backend and therefore not as generic as the name would suggest

Is that theoritical or are there other calendar backends out there?
Why would another calendar backend not fire the OCP events?

[ChristophWurst]

Is that theoritical or are there other calendar backends out there?

Not many, but https://github.com/nextcloud/deck/blob/0223dd9a3a50838a202f3c68ed714e80b00dd2ae/lib/DAV/CalendarPlugin.php#L16 for example.

[JonathanTreffler]

Is that theoritical or are there other calendar backends out there?

Currently not many, but I sure hope we some day have an equivalent to groupfolders for calendars (so basicaly calendars that are not owned by any user but are global to the system). (And unless somebody else starts that project before I get to it I plan on being the person who writes that)

[JonathanTreffler]

Why would another calendar backend not fire the OCP events?

Yes they could, but what are they going to put in as calendarId ? Unlike files where the fileIds are unique in the filecache globally no matter which app mounted it in the filesystem AFAIK calendars are only uniquely addressable over their principalUri + uri and the id is an implementation detail of the provider and not unique (please somebody correct me if I got that wrong 😆 ).

(In dav_push we stopped referencing calendars by principalUri + uri and instead now reference calendars by provider name + id, because those are the same no matter from which users perspective you look at it (for example a shared calendar has a different uri from the perspective of the user shared to, but the same id))

[AndyScherzinger]

Hi @JonathanTreffler not sure if it went unnoticed but due to #51331 this change will also become active in a 31 when merged and shipped, since I can't wait to for 32 due to customer needs.

While I can judge if there is a non-breaking way for Nc31? @ChristophWurst

[JonathanTreffler]

Hi @JonathanTreffler not sure if it went unnoticed but due to #51331 this change will also become active in a 31 when merged and shipped, since I can't wait to for 32 due to customer needs.

While I can judge if there is a non-breaking way for Nc31? @ChristophWurst

Thank you for making me aware of that, i totally missed the backport.

I am going to look into hooking into the the old or new events depending on what is available, but if that is not possible I think it would also be fine to require the latest release of 31 for dav_push.