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

Removing a suffix-virtual file does not trigger a remote deletion #6875

Closed
jnweiger opened this issue Nov 13, 2018 · 20 comments
Closed

Removing a suffix-virtual file does not trigger a remote deletion #6875

jnweiger opened this issue Nov 13, 2018 · 20 comments
Assignees
Labels
Design & UX Discussion feature:vfs native virtual files and placeholder implementation

Comments

@jnweiger
Copy link
Contributor

jnweiger commented Nov 13, 2018

tested with testpilotcloud 2.5.1 on Ubuntu 18.04

Enabled virtual files experimental feature on the connection wizard
Connected to demo.owncloud.org
Amongst others, a file 'ownCloud Manual.pdf.owncloud'
is downloaded.
when I remove the file on the client, it is recreated within a few seconds.

Expected behaviour: The remove propagates to the server, the file gets removed and remains removed.

Maybe related: #6718 -- renaming a virtal file works as expected in 2.5.1 but in 2.5.0 it showed a similar behaviour: automatic reset to the previous state.

@jnweiger
Copy link
Contributor Author

jnweiger commented Nov 13, 2018

@lazawan can you reproduce this?

@guruz guruz assigned ckamm and ogoffart and unassigned guruz Nov 13, 2018
@guruz guruz added this to the 2.5.x milestone Nov 13, 2018
@ogoffart
Copy link
Contributor

This is currently working as designed. Removing a placeholder does not remove the file of the server, for we are afraid of data loss.

This could be changed if another behaviour is preffered. @ckamm, @michaelstingl: what do you think?

@ckamm ckamm modified the milestones: 2.5.x, 2.6.0 Nov 13, 2018
@ckamm
Copy link
Contributor

ckamm commented Nov 13, 2018

Yes, intentional for now. Users might not expect that removing "a.owncloud" removes "a" on the server.

@jnweiger
Copy link
Contributor Author

Users might not expect that they have to download a move in order to make delete work.

@michaelstingl
Copy link
Contributor

Users might not expect that they have to download a move in order to make delete work.

This is probably true. Needs some concept work, that’s why I assigned myself.

@michaelstingl
Copy link
Contributor

Dropbox and iCloud for example have similar "virtual" concepts, and both have warning when a file gets moved out or gets deletes.

Dropbox: delete Dropbox: move iCloud: move
dropbox-delete dropbox-move icloud-move
Warning on top of all windows Warning on top of all windows Warning sliding in from top in current Finder window

I'm not sure if I'm a fan of this warning, but after confirmation of such a warning, we could delete the the remote file.

@ckamm ckamm added the feature:vfs native virtual files and placeholder implementation label Jan 15, 2019
@ckamm ckamm assigned michaelstingl and unassigned ckamm and ogoffart Jan 15, 2019
@ckamm ckamm changed the title Removing a virtual file does not work Removing a suffix-virtual file does not work Jan 15, 2019
@guruz
Copy link
Contributor

guruz commented Apr 15, 2019

@ckamm How is this with wincfapi?

@ckamm
Copy link
Contributor

ckamm commented Apr 15, 2019

There deletions of local dehydrated files synchronize as remote deletions. In my opinion that makes sense since it's clear it's the same file.

@ckamm ckamm changed the title Removing a suffix-virtual file does not work Removing a suffix-virtual file does not trigger a remote deletion Apr 17, 2019
@ckamm ckamm added the p3-medium Normal priority label Jun 4, 2019
@ckamm
Copy link
Contributor

ckamm commented Jun 4, 2019

This is blocked on a decision: Should a local placeholder deletion trigger a remote deletion or not?

@guruz
Copy link
Contributor

guruz commented Jun 4, 2019

@hodyroff @michaelstingl for discussion ^^
Safest would be to have the warning message like @michaelstingl screenshotted above, but this might be not so easy to implement with current architecture.

Is there a way now to have the file be moved to the server side trashbin? That could be an idea.. maybe this can be done with a MOVE operation?

@guruz
Copy link
Contributor

guruz commented Jun 4, 2019

@DeepDiver1975 just said that moving files to server side trashbin is currently not possible via WebDAV.

@ckamm
Copy link
Contributor

ckamm commented Jun 4, 2019

I suggest we leave it as-is and instead focus efforts on nicer vfs solutions, like wincfapi and equivalents for other platforms.

@guruz guruz removed this from the 2.6.0 milestone Jun 4, 2019
@guruz guruz removed the p3-medium Normal priority label Jun 4, 2019
@guruz
Copy link
Contributor

guruz commented Jun 4, 2019

Moving out of milestone and keeping as discussion topic

@michaelstingl
Copy link
Contributor

@hodyroff @michaelstingl for discussion ^^
Safest would be to have the warning message like @michaelstingl screenshotted above, but this might be not so easy to implement with current architecture.

Client detects deletion, but shouldn't send DELETE to the oC server. Instead it should open the warning dialogue. In case, users confirm the deletion, we can send the DELETE to the oC server. In the other case where the user cancels the deletion, it should re-download the file to the local sync folder. (similar handling like deletions in read-only folders)

I suggest we leave it as-is and instead focus efforts on nicer vfs solutions, like wincfapi and equivalents for other platforms.

I think we'll need this popup also for the newer fancy implementations where we don't have the real data in the local trashbin.

@ckamm
Copy link
Contributor

ckamm commented Jun 13, 2019

A warning dialog has complications:

  1. Ideally it pops up immediately when the deletion happens: but that only works if file watchers are reliable and the client is running. To be entirely safe it'd need to pop up during the sync run but that has huge disadvantages: It'd happen much too late and I very much want to avoid user interaction during sync runs (how would that work with owncloudcmd?).

  2. Users will ask for a "always delete" checkbox.

So: I imagine a file-watcher triggered dialog with this checkbox. If the user deletes placeholders while the client isn't looking the current no-delete behavior persists.

About other vfs implementations: Do you really think so? If the file is not distinguishable from the real file, why would deleting it not trigger a remote deletion?

@michaelstingl
Copy link
Contributor

About other vfs implementations: Do you really think so? If the file is not distinguishable from the real file, why would deleting it not trigger a remote deletion?

Yeah, it triggers the remote deletion. But also triggers a popup.

@HanaGemela
Copy link
Contributor

There is inconsistency between deleting a file and deleting a folder. File is not deleted but folder is.
There should be some warning that it will delete the folder on the server as well or it should just be recreated as is a file after deletion

@michaelstingl
Copy link
Contributor

There should be some warning that it will delete the folder on the server as well or it should just be recreated as is a file after deletion

👍

@jnweiger
Copy link
Contributor Author

jnweiger commented Aug 1, 2019

Contrary to the original post, I have also seen the opposite behaviour coming as a surprise: Some users seem to expect, they can remove local placeholders, without any effect on the server. That is a misconception. Requesters like thise in Michael's screenshot should help to educate users about the semantics of virtual files.

@ckamm
Copy link
Contributor

ckamm commented Aug 1, 2019

I think we have consensus on the warning, I've created #7360 to track the implementation.

@ckamm ckamm closed this as completed Aug 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design & UX Discussion feature:vfs native virtual files and placeholder implementation
Projects
None yet
Development

No branches or pull requests

6 participants