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

Calendar and Contacts Trashbin #1662

Open
1 of 3 tasks
georgehrke opened this issue Oct 7, 2016 · 58 comments
Open
1 of 3 tasks

Calendar and Contacts Trashbin #1662

georgehrke opened this issue Oct 7, 2016 · 58 comments
Labels
1. to develop Accepted and waiting to be taken care of enhancement feature: caldav Related to CalDAV internals feature: dav feature: trashbin spec

Comments

@georgehrke
Copy link
Member

georgehrke commented Oct 7, 2016

We already provide a trash bin for files, that allows users to restore deleted files.
I'd like to see something similar for calendars and contacts.

This also helps users who accidentally deleted events/contacts or an entire calendar/addressbook in their client.

Todo

  • Calendar and event trash bin
  • Calendar subscription trash bin
  • Address book and contact card trash bin

cc @jancborchardt @tcitworld @schiessle

@georgehrke georgehrke added enhancement 1. to develop Accepted and waiting to be taken care of feature: dav spec labels Oct 7, 2016
@georgehrke
Copy link
Member Author

There are a few questions that need a prior discussion:

  • Which app shall implement the features logic?
    • dav
    • a new dav_trashbin
    • calendar respectively contacts

I'd not like to see it in the latter, because you can use CalDAV and CardDAV without the Calendar and Contacts app. Therefore the trash bin should always work, no matter whether the web gui is enabled.

  • Which app shall implement the GUI?
    • dav respectively dav_trashbin in the personal settings area
    • calendar respectively contacts

I'd actually go with calendar and contacts here. We can't do this with plain CalDAV/CardDAV. Therefore it's acceptable to having enable calendar respectively contacts before restoring imo.

I'd suggest to provide the trash bin via remote.php/dav/calendars_trashbin/ respectively remote.php/dav/addressbooks_trashbin/.

@schiessle I wasn't able to find the trash bin via remote.php/dav/. Does files_trashbin already use WebDAV for talking to the server?

@georgehrke
Copy link
Member Author

georgehrke commented Oct 7, 2016

However it would be of no use if the calendar/contacts app aren't here since you wouldn't be able to restore deleted calendars/contacts.

Yes, but there are the following scenarios:

Dav app takes care:
You delete a calendar by accident -> the calendar is moved to a trash bin by the dav app -> you enable the calendar app -> you restore the calendar

Calendar app takes care:
You delete a calendar by accident -> ¯_(ツ)_/¯

@tcitworld
Copy link
Member

Totally agree with all of these. If I understand correctly, putting an item in the bin would mean moving the data from the oc_calendars table to oc_calendar_bin, while not touching oc_calendar_objects. Do the data from oc_dav_shares must remain too ?

@nickvergessen
Copy link
Member

Couldn't we just create a flag, and some sort of hidden calendars/events which are not exposed, unless you propfind with a the special property? I think that would be very few lines to implement.

When you delete a calendar/event and it does not have the flag yet, we set the flag. If the flag already exists it's deleted from the system.

@georgehrke
Copy link
Member Author

georgehrke commented Oct 10, 2016

That would indeed be a good solution.
Instead of the flag I'd probably go for a timestamp, which is null by default.
This allows us to easily delete deleted calendars/events older than n days.

@georgehrke georgehrke added this to the Nextcloud 11.0 milestone Oct 10, 2016
@georgehrke georgehrke self-assigned this Oct 10, 2016
@georgehrke
Copy link
Member Author

But there is actually one issues I'd like to discuss before implementing this:

When a calendar/addressbook (but also event/contact) is deleted, the uid should be available again.
What to do about this? Replace the uid (or uri for event/contact) with a random string?

Keep this random string when replacing or tell the client to send a new uid along the request?

The dav_shares table uses the calendar id and not the calendar uri, we don't need to change anything there.
The behavior should be the same as in the files app. Restoring a calendar/addressbook also restores all the shares.

@georgehrke
Copy link
Member Author

Reminder for myself:
Do not return deleted calendars on allprop request

@nickvergessen
Copy link
Member

When a calendar/addressbook (but also event/contact) is deleted, the uid should be available again.
What to do about this? Replace the uid (or uri for event/contact) with a random string?

As long as there are no duplicates on restoring, fine by me.

@georgehrke
Copy link
Member Author

Yes, the db won't allow duplicate URIs.
https://github.com/nextcloud/server/blob/master/apps/dav/appinfo/database.xml#L375

@jancborchardt
Copy link
Member

Regarding where to store it – definitely directly in the Calendar respectively the Contacts app. cc @Henni @irgendwie

@georgehrke
Copy link
Member Author

Do you mean store or restore?

Please excuse my brevity and typos.
Sent from my mobile

On Oct 11, 2016, at 9:36 AM, Jan-Christoph Borchardt notifications@github.com wrote:

Regarding where to store it – definitely directly in the Calendar respectively the Contacts app. cc @Henni @irgendwie


You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub, or mute the thread.

@schiessle

This comment has been minimized.

@jancborchardt

This comment has been minimized.

@georgehrke

This comment has been minimized.

@Henni

This comment has been minimized.

@georgehrke

This comment has been minimized.

@jancborchardt

This comment has been minimized.

@spoorun

This comment has been minimized.

@nickvergessen

This comment has been minimized.

@raimund-schluessler

This comment has been minimized.

@ChristophWurst ChristophWurst added the 2. developing Work in progress label Apr 13, 2021
@ChristophWurst ChristophWurst added 1. to develop Accepted and waiting to be taken care of and removed 2. developing Work in progress labels Jun 4, 2021
@ChristophWurst ChristophWurst removed their assignment Jun 4, 2021
@ChristophWurst
Copy link
Member

The caldav part is done. Starting with Nextcloud 22 deletions of events and whole calendars result in a move into a trash bin instead of an instant and irreversible data deletion. This works with any connected client. Nextcloud Calendar then comes with a UI to restore from this trash bin. Until there is a standard for trash bins on caldav this trash bin won't be usable for connected clients.

@azrdev

This comment has been minimized.

@ChristophWurst

This comment has been minimized.

@cornzy
Copy link

cornzy commented Dec 1, 2021

Thank you for the trash bin :) Is there an option to change the default 30 days of keeping the trash?

@tcitworld
Copy link
Member

Yes, php occ config:app:set dav calendarRetentionObligation --value=%value% where %value% must be replaced by a number of seconds. 0 equals to disabled.

Will add it to the docs nextcloud/documentation#7696

@cornzy
Copy link

cornzy commented Dec 3, 2021

I just tried this configuration and set the value to 365. Reply:

Config value calendarRetentionObligation for app dav set to 365

Ok. But now when I go to the trash bin in the web frontend, under all entries it says:

Elements in the trash bin are deleted after 1 day

Is this just a bug in the UI? I can wait until tomorrow to tell you if all entries are deleted or not :)

@raimund-schluessler
Copy link
Member

I just tried this configuration and set the value to 365. Reply:

Config value calendarRetentionObligation for app dav set to 365

The value needs to be set in seconds. I assume you want to set it to 365 days, so you need to put 365*24*60*60 which equals 31536000 seconds.

@gohrner
Copy link

gohrner commented Dec 4, 2021

Is there any way to completely disable this feature? Unfortunately, it breaks things.

Besides even side-effects within Nextcloud itself (as reported in Tasks Issue 1685 or Calendar Issue 3325), it also breaks a custom calendar sync tool we use.

It generates events with computed UIDs to properly sync them to an external source, and might remove an event temporarily, but later has to re-add it.

(I strongly suggest that adding an event which is in the trash bin should not cause a conflict, but recover the event from the trash bin - but I'll add a separate request for that.)

I tried setting the calendarRetentionObligation to 1 second, but it doesn't seem to help, not even with running the background cron job manually before re-running our sync tool. Only manually expunging the calendar trash bin helps.

Any solutions? Do I really have to adjust my sync tool? But how to deal with other CalDAV clients which perform similar operations, like moving a calendar entry from one calendar to another and back using an CalDAV client - it will also try to recreate a "deleted" calendar entry with the same UID, I'd assume...

Sidenode: The calendar sync tool uses a role user account which has write access to calendars belonging to other users, where it syns the stuff to. The deleted calendars end up in the trash bin's of the users to whom the calendar belongs, not in the trash bin of the role account who actually deleted the event... Is this intended?

@gohrner
Copy link

gohrner commented Dec 4, 2021

(I strongly suggest that adding an event which is in the trash bin should not cause a conflict, but recover the event from the trash bin - but I'll add a separate request for that.)

=> #30096

@tcitworld
Copy link
Member

Is there any way to completely disable this feature? Unfortunately, it breaks things.

I tried setting the calendarRetentionObligation to 1 second, but it doesn't seem to help, not even with running the background cron job manually before re-running our sync tool. Only manually expunging the calendar trash bin helps.

Set it to 0 instead.

@gohrner
Copy link

gohrner commented Dec 4, 2021

Set it to 0 instead.

Ahh - I understood your comment above incorrectly. I somehow read it as "0 disables trashbin expiration completely" while what you meant was "0 disables trashbin completely".

Ok, thanks, I'll try that!

@zeaglee
Copy link

zeaglee commented Dec 8, 2021

Yes, php occ config:app:set dav calendarRetentionObligation --value=%value% where %value% must be replaced by a number of seconds. 0 equals to disabled.

Thank you for this. This update broke my synchronization with iCal Import/Export. I use it to copy an external calendar to a nextcloud calendar then sync the changes on a regular basis. Immediately after the update I started getting errors that an event's uuid existed in the trash.

@ChristophWurst ChristophWurst added the feature: caldav Related to CalDAV internals label Dec 17, 2021
@pgy866
Copy link

pgy866 commented Feb 24, 2022

Great PR, is it being discarded now?

@PhilippSchlesinger
Copy link

Is the carddav trashbin feature tracked in a separate issue that is not yet linked here by accident?

@gbakeman
Copy link

gbakeman commented Jun 14, 2023

I was just recently made another victim of an errant DAV client trying to helpfully "sync" old data which, between it and Nextcloud, seemed to mean deleting newer contact data. Unfortunately it looks like I'm going to be spending the afternoon learning how to recover from a mysql database. Would be very helpful if DAV changes are at least logged, and deletions are captured in this recycle bin feature we've been discussing here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. to develop Accepted and waiting to be taken care of enhancement feature: caldav Related to CalDAV internals feature: dav feature: trashbin spec
Projects
None yet
Development

No branches or pull requests