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

Make undo deletion work again #119

Closed
jancborchardt opened this issue Oct 28, 2012 · 62 comments
Closed

Make undo deletion work again #119

jancborchardt opened this issue Oct 28, 2012 · 62 comments

Comments

@jancborchardt
Copy link
Member

jancborchardt commented Oct 28, 2012

Short: We need »undo delete« for files again.


You have an ownCloud server and it syncs to your desktop.

Now when you delete a file via the web interface, it doesn’t really get deleted, because you get the opportunity to undo that (like Gmail, better than a modal asking you for confirmation, which no one reads and just confirms).

The problem here is that you expect it to be deleted, and thus also removed from your local synced folder. This doesn’t happen immediately though because the file is not removed in the backend.

@MTRichards and I discussed a solution for this: When you delete a file in the web interface, the deletion should take place after the 7 seconds undo time, or after performing the next navigation action (whatever comes first). The file should be visibly (but fake) removed from the list instantly, and instantly reappear on undo.

@jancborchardt
Copy link
Member Author

This seems related to #51 (Undo function makes sync experience suboptimal).

@DeepDiver1975
Copy link
Member

How do we want to handle external file systems in this case?

Moving the files to be deleted within the same filesystem is cheep.
Moving file from DropBox to ownCloud is expensive.

Can we ignore the undo feature for this case?

@MTRichards
Copy link
Contributor

@jancborchardt I think this is a UX question for you. Probably not, if possible...

@jancborchardt
Copy link
Member Author

Depends on what external file system we’re talking about I’d say:

  • Dropbox: the file will probably not be lost permanently anyway because Dropbox has their own versioning where files can be restored. As it is probably also synced to clients, we don’t want to have the problem of not actually deleting/moving the file and causing client »lag«. And it would be too expensive, so let’s just not do it.
  • FTP: doesn’t have any history and isn’t synced anywhere else probably, I’d say here use the current just notification-undo without moving the file

That as short example, more tomorrow.

@DeepDiver1975
Copy link
Member

How do we want the undo to work in general?

  1. Only allow undo of the last delete actions?
  2. Trash can feature and the user can undo whatever deletion

In addition:
How will this work together with version?
Do we want any integration of undo/trash can and versions?

@jancborchardt @karlitschek @schiesbn

@MTRichards
Copy link
Contributor

In general, it should only be the last action available to undo. The big one here is delete - and undo there moves it out of the trash back where it was. If you select a whole stack of files and folders and delete them, then undo undoes them all - undo usually means "oops, I take that back".

Other undo options would be things like rename and move as well. Undo here is lower priority, but would be nice to have. Undo would change names back to original names, and locations to original locations.

Versions...this is interesting. I would like them to work independently, as people may or may not have the two turned on. If they are both enabled, undo should track and undo changes to the appropriate versions too. Example: if I delete a file, versions go away too, but if I then undo the delete, then the versions should come back too, if the versions app is turned on. If I delete a folder and hit undo, versions for everything in the folder should come back too. If I move a folder, versions should move, and then undo would move them back as well - but again, these last two actions are less important than undelete.

@jlgg
Copy link

jlgg commented Dec 30, 2012

As I commented some hours ago in #199, I've implemeted a patch in files_versions in order to have a simple trash. In this issue you can find my proposal.

In my implementations, versions of files aren't deleted so, if you recover the deleted file, you keep all history.

As I commented, I'm working on the following TODO list:

  1. Include remove procedure in the cron for deleted files/dirs
  2. Remove or fix working in "All versions..." link
  3. Generate an "Empty trash" procedure (different than cron)
  4. Change history icon in directories

You can see this implementation on my fork simple_trash. I accept comments an suggestions

@DeepDiver1975
Copy link
Member

@jlgg some questions

  1. How do you handle external fs?
  2. Your trash app is an extention to files_version, right?

AFAIK files_version will be reimplemented. @schiesbn is in charge
And we need an undo solution which works independent of versions.

@jlgg
Copy link

jlgg commented Dec 31, 2012

@DeepDiver1975

  1. I've just edited files_versions hooks.php and versions.php (and js and ajax files) using the same methods used in this ones. So if versions supports external fs, then my undo method supports too.
  2. Yes

I agree with you that undo should be independent of versions but we can get a working version in just some hours using versions support.

@schiessle
Copy link
Contributor

pull request #1266 introduces a trash bin like we know it from most desktop systems. Once the pull request is merged the other undo function will be gone which will solve the issue discussed here.

@jancborchardt
Copy link
Member Author

As you see from #2101 we should definitely bring back proper undo for Files in ownCloud 6.

@jancborchardt
Copy link
Member Author

Folks, what’s the resolution on this? I want to grab one of you awesome PHP devs at the Berlin hackfest next week and make undo deletion work again. It will have a 5–10 second automatic timeout so there will be no syncing lag. It should also be implemented in a way that it can be used by other apps.

Who’s with me?

@DeepDiver1975
Copy link
Member

@jancborchardt @MTRichards @karlitschek OC7 from my point of view

@karlitschek
Copy link
Contributor

Yes. oC7

@jancborchardt
Copy link
Member Author

Yeah, oC7 but then please let’s do that for real then. :)

@PVince81
Copy link
Contributor

@jancborchardt the undo file deletion seems to be gone now.
Is this still an issue or should we reconsider this when adding back undo in #7058 ?

@jancborchardt
Copy link
Member Author

@PVince81 yes – of course we should have undo back for file deletion. This issue is open exactly because undo deletion is gone.

It’s cumbersome to delete a file accidentally and then have to go to the deleted files to retrieve it.

@PVince81
Copy link
Contributor

I was rather asking whether this ticket here is a duplicate of #7058.
Seems I've misread as the linked ticket is for moving files, not deleting.
So let's keep that ticket open.

@jancborchardt
Copy link
Member Author

Yup. Very similar but not the same. :) One for deletion (where we used to have undo but removed it because it was badly implemented), one for moving (where we didn’t have it).

@DeepDiver1975 DeepDiver1975 removed this from the 8.1-current milestone Mar 26, 2015
@DeepDiver1975 DeepDiver1975 modified the milestones: 8.3-next, 8.2-current Aug 12, 2015
@DeepDiver1975 DeepDiver1975 modified the milestones: 9.1-next, 9.0-current Nov 10, 2015
@jancborchardt
Copy link
Member Author

@MTRichards can you please set this on the roadmap of 9.1? This is quite an important function to prevent people either losing data or needing to dig into the deleted files, and it just keeps getting pushed back.

@MTRichards
Copy link
Contributor

Ok. We will evaluate it when we fill 9.1, but I see no reason at this time to push it back yet again.

@jancborchardt
Copy link
Member Author

Hey @watermark since you worked on the delete confirmation app, would you like to implement this undo delete feature here so we can have the proper functionality in core? :)

@watermark
Copy link

@jancborchardt My extra curricular time is pretty full, so I don't believe I'll have time to address this. For my uses, we have to disable the "deleted files" app anyway, so this undo functionality wouldn't be of use for any of my installs. For "deleted files" to be useful for us, it would have to move to some type of system where deleted files from external storage didn't count towards the quota of the user that deleted it. Additionally, last I checked, "deleted files" uses a "copy" instead of a "move", which also made it difficult to use.

@jancborchardt
Copy link
Member Author

@watermark just to be clear though: This has nothing to do with the »deleted files« app. It’s simply a mechanism like »undo send« of Gmail and »undo deletion« of Google Drive etc to defer deletion for a few seconds in case it was a mistake.

It’s a more elegant solution than a popup since popups are often clicked away. Also, the popup makes every real deletion an annoying 2-click process.

@PVince81
Copy link
Contributor

PVince81 commented Jun 1, 2016

We're past feature freeze for 9.1 and there wasn't enough time to work on this.

@MTRichards again something to be scheduled, maybe for 9.2

CC @cmonteroluque

@PVince81 PVince81 modified the milestones: 9.2-next, 9.1-current Jun 1, 2016
@MTRichards
Copy link
Contributor

check, ok.

@Shamim-Capgemini
Copy link

Shamim-Capgemini commented Jun 1, 2016

@jancborchardt I'm not sure why elegant is expected to be coincident with "more usable". Yes, a popup requires multiple clicks. But a popup used to make a non-desirable activity more difficult is not a bad choice per se. Should we remove seals/locks on ALL airplane doors because it makes it harder to open to go skydiivng?

Sometimes, the ELEGANT is the WRONG choice for the AUDIENCE but NOT the AUTHOR. I feel it is not responsible for a committee to foist one scenario or another when both are clearly possible. With "delete and allow undo for a few seconds" there are many edge cases not covered.. What happens if it is not caught in time? What happens if the user looks away? We are making way too many assumptions on WHAT the BEST solution is.

I strongly advocate giving the users a choice since it works differently for different groups.

For the record, since i IMPLEMENTED the delete popup by hand in v6 and v7 for my consituents (140+ enterprise users, hundreds of Gigs of data stored), I have not had a SINGLE instance of:

  1. Accidental deletes
  2. No complaints on the two step delete
  3. No requests to restore any files
  4. No questions on how to restore shares from deleted files

I would consider this ample evidence for a need for both. The "elegant" solution for those that have a simple scenario or users that pay attention, or a "delete popup" for those users that need that level of hand-holding.

Thank you.

@watermark I share your opinions on this one.

@Shamim-Capgemini
Copy link

Shamim-Capgemini commented Jun 1, 2016

@jancborchardt Take home message: please don't take our ability to CHOOSE if we want a popup away just because it's annoying to a particular audience that has very specific needs. This one group does not speak for all users everywhere and should not be brandished as the cause-celebre to remove the "click-twice to delete" option.

@watermark
Copy link

@Shamim-Capgemini You're preaching to the wrong guy. I've had a similar experience as you (100+ users), and I implemented a plugin to do the popup as well. You should probably discuss your point of view with @jancborchardt.

@Shamim-Capgemini
Copy link

@watermark You are right. We're in the same boat.

@jancborchardt Please see my comment above.

@hodyroff
Copy link
Contributor

@watermark Like the delete question app. Do you have a chance to update to 9.1 and soon to 10.0? Would be happy to recommend to customers once in a while ... Or would you just like to implement this for Core? Would happily take it as an Admin setting function. Please do a PR in that case and link to it here.

However this one original was about something else and doesn't seem to work easily with external file systems. Closing.

@lock
Copy link

lock bot commented Aug 3, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests