-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Move calendar objects between calendars #30120
Conversation
Not sure I have enough meta knowledge. But are URIs unique per calendar or globally? So could there be an error when moving a to be duplicate-URI element to another calendar which already has this URI? |
Lots of potential issues spotted by psalm. |
f81fdd7
to
c605aa0
Compare
@miaulalala: I don't want to discredit this implementation, but to me it sounds as if it only catches a corner case. I guess this PR won't help in cases where an event is moved from one calendar managed in Nextcloud to another calendar managed by some other server, and back again? Nextcloud server will still have to provide a solution for this case in order not to break other CalDAV clients. How about - probably in addition to this feature implemented here - allowing the Nextcloud Calender to "resurrect" and update items in the trashbin transparently if they are re-added by any client? This would also solve the "multiple servers" issue, and also additional ones (basically getting the CalDAV interface's semantics - to my knowledge - back in line with the RFCs). |
I guess it depends on your definiton of "edge case" ;) but you're right, this does indeed not help with the problem of deletion conflicts. I'll address this in a different PR. |
Failing drone tests seem to be related (e.g. |
a0c8088
to
747a4bc
Compare
Drone Failure unrelated (Samba) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a nitpick
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
As a (future) enhancement this is also a good example of an operation we should do atomically. Either the calendar gets moved and the changes are tracked in the sync table, or everything is rolled back. Ref #31205.
…ating them Signed-off-by: Anna Larch <anna@nextcloud.com>
1facde6
to
0745fc5
Compare
This comment was marked as off-topic.
This comment was marked as off-topic.
…ddressBook This allows to just UPDATE the card row instead of deleting it and reinsert it. It's very similar to #30120 for calendars. As we need the addressbookid exposed, this introduces OCA\DAV\CardDAV\Card that extends Sabre's. I chose specifically NOT to auto-inject LoggerInterface in Addressbook like in #30120 because the chain of DI is huge just for ONE simple call and it would break an existing dirty call (OCA\Contacts calling OCA\DAV) of ContactsManager in Contacts: nextcloud/contacts#1722 (in SocialApiService), but this is debatable. Signed-off-by: Thomas Citharel <tcit@tcit.fr>
…ddressBook This allows to just UPDATE the card row instead of deleting it and reinsert it. It's very similar to #30120 for calendars. As we need the addressbookid exposed, this introduces OCA\DAV\CardDAV\Card that extends Sabre's. I chose specifically NOT to auto-inject LoggerInterface in Addressbook like in #30120 because the chain of DI is huge just for ONE simple call and it would break an existing dirty call (OCA\Contacts calling OCA\DAV) of ContactsManager in Contacts: nextcloud/contacts#1722 (in SocialApiService), but this is debatable. Signed-off-by: Thomas Citharel <tcit@tcit.fr>
…ddressBook This allows to just UPDATE the card row instead of deleting it and reinsert it. It's very similar to #30120 for calendars. As we need the addressbookid exposed, this introduces OCA\DAV\CardDAV\Card that extends Sabre's. I chose specifically NOT to auto-inject LoggerInterface in Addressbook like in #30120 because the chain of DI is huge just for ONE simple call and it would break an existing dirty call (OCA\Contacts calling OCA\DAV) of ContactsManager in Contacts: nextcloud/contacts#1722 (in SocialApiService), but this is debatable. Signed-off-by: Thomas Citharel <tcit@tcit.fr>
…ddressBook This allows to just UPDATE the card row instead of deleting it and reinsert it. It's very similar to #30120 for calendars. As we need the addressbookid exposed, this introduces OCA\DAV\CardDAV\Card that extends Sabre's. I chose specifically NOT to auto-inject LoggerInterface in Addressbook like in #30120 because the chain of DI is huge just for ONE simple call and it would break an existing dirty call (OCA\Contacts calling OCA\DAV) of ContactsManager in Contacts: nextcloud/contacts#1722 (in SocialApiService), but this is debatable. Signed-off-by: Thomas Citharel <tcit@tcit.fr>
…ddressBook This allows to just UPDATE the card row instead of deleting it and reinsert it. It's very similar to #30120 for calendars. As we need the addressbookid exposed, this introduces OCA\DAV\CardDAV\Card that extends Sabre's. I chose specifically NOT to auto-inject LoggerInterface in Addressbook like in #30120 because the chain of DI is huge just for ONE simple call and it would break an existing dirty call (OCA\Contacts calling OCA\DAV) of ContactsManager in Contacts: nextcloud/contacts#1722 (in SocialApiService), but this is debatable. Signed-off-by: Thomas Citharel <tcit@tcit.fr>
…ddressBook This allows to just UPDATE the card row instead of deleting it and reinsert it. It's very similar to #30120 for calendars. As we need the addressbookid exposed, this introduces OCA\DAV\CardDAV\Card that extends Sabre's. I chose specifically NOT to auto-inject LoggerInterface in Addressbook like in #30120 because the chain of DI is huge just for ONE simple call and it would break an existing dirty call (OCA\Contacts calling OCA\DAV) of ContactsManager in Contacts: nextcloud/contacts#1722 (in SocialApiService), but this is debatable. Signed-off-by: Thomas Citharel <tcit@tcit.fr>
…ddressBook This allows to just UPDATE the card row instead of deleting it and reinsert it. It's very similar to #30120 for calendars. As we need the addressbookid exposed, this introduces OCA\DAV\CardDAV\Card that extends Sabre's. I chose specifically NOT to auto-inject LoggerInterface in Addressbook like in #30120 because the chain of DI is huge just for ONE simple call and it would break an existing dirty call (OCA\Contacts calling OCA\DAV) of ContactsManager in Contacts: nextcloud/contacts#1722 (in SocialApiService), but this is debatable. Signed-off-by: Thomas Citharel <tcit@tcit.fr>
…ddressBook This allows to just UPDATE the card row instead of deleting it and reinsert it. It's very similar to #30120 for calendars. As we need the addressbookid exposed, this introduces OCA\DAV\CardDAV\Card that extends Sabre's. I chose specifically NOT to auto-inject LoggerInterface in Addressbook like in #30120 because the chain of DI is huge just for ONE simple call and it would break an existing dirty call (OCA\Contacts calling OCA\DAV) of ContactsManager in Contacts: nextcloud/contacts#1722 (in SocialApiService), but this is debatable. Signed-off-by: Thomas Citharel <tcit@tcit.fr>
When moving a calendar object from calendar A to calendar B, move the object instead of copying it to the new calendar and deleting the old object.
Fixes nextcloud/calendar#3325