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

[buteo-sync-plugin-caldav] Fix upsync of imported exceptions. #20

Merged
merged 1 commit into from Mar 31, 2023

Conversation

dcaliste
Copy link
Contributor

The sync code relies on the fact that added
exceptions on device inherit the URI and ETAG
of the parent, when the dissociation code is
called.

This assumption is not true when importing
an exception, like a cancel event received
by email for instance.

This commit ensures that such an imported
event is actually seen as a local addition
and that the ETAG used to push the event to
the server is using the ETAG of the parent.

@pvuorela, this is fixing the issue for me, where previously the lack of URI and ETAG in the imported exception was making the sync code to treat it as a non fully upsync event, not modified on device, and was thus scratched by downloading the server version to get the ETAG. URI and ETAG are custom properties saved by the sync plugin to get track of upsynced incidences.

This issue may impact other sync plugin, those relying on custom properties copied from the parent to the exception when created on device, I don't know. Another way of fixing the issue would be to modify the import code in nemo-qml-plugin-calendar in a way, that when importing an exception, the event is not added directly to the calendar, but follow the same route as created exceptions:

  • dissociate an occurence from the parent at the given date,
  • merge the imported exception into the occurrence (copy everything but keep existing custom properties)
  • add the occurrence to the calendar.

The problem is that this "merge" action does not exist in KCalendarCore API and is a bit ugly to add.

What's your opinion ?

The sync code relies on the fact that added
exceptions on device inherit the URI and ETAG
of the parent, when the dissociation code is
called.

This assumption is not true when importing
an exception, like a cancel event received
by email for instance.

This commit ensures that such an imported
event is actually seen as a local addition
and that the ETAG used to push the event to
the server is using the ETAG of the parent.
@dcaliste
Copy link
Contributor Author

I've added a test in notebooksyncagent one to "validate" the proposition and ensure it won't get lost by a later modification.

@pvuorela
Copy link
Contributor

Importing a cancel event from an email sounds quite much a corner case. Not that fond of adding complexity on importing events and this is quite a small change thinking we could just go with this.

@pvuorela pvuorela merged commit e6382df into sailfishos:master Mar 31, 2023
@dcaliste
Copy link
Contributor Author

dcaliste commented Apr 1, 2023

This is indeed a corner case since I only notice it after years of using CalDAV sync... But looking at how I got it, is not that uncommon : a friend of mine set a weekly meeting up in her Google agenda and invited me to it. So I received an email with the ICS data attached. I imported it to my calendar, fine. Later on, we had to cancel one rendez-vous. She noted it as cancelled in her agenda and Google send me a cancellation email with the attached ICS data, containing just an exception event with STATUS:CANCELLED. That's how I got an exception as an email attachment. So the pattern is not that uncommon I think. Changing the time of one of the occurrence will be sent as email also and would trigger the same bug if not patched. So thanks for accepting it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants