desktop icon indicating copy to clipboard operation
desktop copied to clipboard

Unsynced folders are lost, if you uncheck

Open aronpapp opened this issue 5 years ago • 71 comments

Expected behaviour

  • Warn user of possible data loss
  • Ask in dialog box, if the user really wants to erase data or inform user, that unsynced folders were not removed from the filesystem

Actual behaviour

Erases (unsynced) data silently.

Steps to reproduce

  1. Move or copy a folder to your sync folder using a filemamanger
  2. Open nextCloud client
  3. Do not wait to sync the new folder to the server
  4. Uncheck checkbox for the new folder
  5. Click Apply

Client configuration

Client version: Version 2.5.1final (build 20181204)

Operating system: Debian 9.8

OS language: en

Qt version used by client package (Linux only, see also Settings dialog): I guess it uses bundled Qt.

Client package (From Nextcloud or distro) (Linux only): Nextcloud-2.5.1-x86_64.AppImage

Installation path of client: No need to install

Server configuration

Issue is not related to the server.

aronpapp avatar May 08 '19 07:05 aronpapp

What does it mean 4. Remove checkbox before new folder ?

Germano0 avatar May 08 '19 08:05 Germano0

What does it mean 4. Remove checkbox before new folder ?

Sorry, I mean uncheck the checkbox (also fixed in the initial post)

aronpapp avatar May 08 '19 10:05 aronpapp

You mean you uncheck the folder so it doesn't get synced, correct?

camilasan avatar May 08 '19 18:05 camilasan

You mean you uncheck the folder so it doesn't get synced, correct?

Yes, before it gets get synced, gets deleted from the filesystem, so data is completly flushed.

aronpapp avatar May 09 '19 06:05 aronpapp

That sounds like a very important problem.

freechelmi avatar Dec 05 '19 15:12 freechelmi

There's a large discussion about this behaviour in the owncloud repo: https://github.com/owncloud/client/issues/2502

Includes more failure cases and a couple of proposed UI fixes.

b12f avatar Jan 23 '20 14:01 b12f

That is really critical - we have exactly that problem. It dramatically lowers down the tool acceptance and. As the desktop app is available as source code: has someone tried to disable the method that deletes the local files? Any indication on the source code file where we can turn it off?

salagula avatar Oct 22 '20 04:10 salagula

I guess there's something I don't get regarding that one, but here unchecking something leads to this warning: Screenshot_20201022_125320

And it only removes if you confirm it. To me, it's pretty clear it's going to kill data in there.

er-vin avatar Oct 22 '20 10:10 er-vin

The problem is that it also removes files that aren't synced yet. So let's say you added a big directory to nextcloud, but want to not sync it (for now, or not after all), it removes the data completely. When would this happen? Mostly during installation; you select a folder in which the data is that you want to sync, but then find out there's a couple of large files in there that you do not want to sync after all. Poof, data gone.

Edit: Alternatively, you have to wait for the complete sync to finish before you can unsync, or you have to reinstall nextcloud. In any case, the banner is not enough, and having this behaviour as default is bad. Both the discussion here and in the owncloud repository show that there are enough people who - even though they are technically versed - still get caught by surprise and lose data.

From a UX perspective, the warning is not enough imo. This behaviour can lead to unsynchronized data being lost forever, so this should be a big fat red banner if there are unsynchronized files to be deleted. Even better, it should be possible to unsynchronize without deletion.

b12f avatar Oct 22 '20 11:10 b12f

I agree to b12f - it is not about the message, it is about the fact that it deletes files before they are synced. Again my question: can someone point me to the code segment where this is triggered? I would like to comment the section out (if possible)

salagula avatar Oct 22 '20 15:10 salagula

I agree to b12f - it is not about the message, it is about the fact that it deletes files before they are synced. Again my question: can someone point me to the code segment where this is triggered? I would like to comment the section out (if possible)

It's not that simple, it's really just about putting those folders in the selective sync exclude list and then the sync engine "doing its thing". It's not like there's a function where those folders are removed, this is deeply buried in the sync engine behavior AFAICT.

er-vin avatar Oct 26 '20 13:10 er-vin

I guess there's something I don't get regarding that one, but here unchecking something leads to this warning: Screenshot_20201022_125320

And it only removes if you confirm it. To me, it's pretty clear it's going to kill data in there.

I'd like to vote for a dialogue that rather than cancel / apply gives three options:

  1. remove folder from sync and delete local data (= the current behaviour)
  2. remove folder from sync but keep local data (= my preferred behaviour in most cases)
  3. cancel

irisv avatar Nov 25 '20 10:11 irisv

  1. remove folder from sync and delete local data (= the current behaviour)
  2. remove folder from sync but keep local data (= my preferred behaviour in most cases)
  3. cancel

Honestly, there is a big risk of doing no. 2 based on my own experience. I had cases where I removed a folder like this (might have be in older owncloud times) and I kept the folder in there but it was not synced anymore. After around x days I forgot about it and changed some important data on one machine in that folder and I changed files on the other machine some days later as well. It took me some further days to figure out, I was working on different instances and need to do manual sync work now.

So the rule should be, if it is in your locally synced nextcloud folder, then it's synced and if it is not synced it is not in there - store your data somewhere else, but don't confuse yourself with folders being in sync and some not.

So no. 1 and no. 3 are the only realistic options for not messing up with your personal data controlling, so I disagree and would be surprised you describe a usecase where no. 2 is your "most preferred behaviour".

Where I agree is that there could be an additional warning, if data is going to be deleted, which was not synced yet. Use case could be you move a bigger folder from anywhere to your local nextcloud folder to get it synced and expect the sync to be finished after a while, but due to low internet speed the folder was only half synced yet. If you then just try to remove the hook to get the folder deleted (maybe to save some local space) you have to be warned. There it makes sense for not loosing unsynced data.

andreaslink-de avatar Nov 25 '20 10:11 andreaslink-de

Will a fix be included in the next nextcloud release? Or is there something planned for a future release?

sojer avatar Nov 25 '20 11:11 sojer

So no. 1 and no. 3 are the only realistic options for not messing up with your personal data controlling, so I disagree and would be surprised you describe a usecase where no. 2 is your "most preferred behaviour".

Ok, then let me explain my usage to you. I have a set of core folders that need to be synced and kept up to date continuously. Those will never be deleted from the client, so those don't come into this discussion. When I delete a folder from the client, it is -by definition- a folder I've only had use for temporary and now no longer need. So your scenario where "around x days [you] forgot about it and changed some important data on one machine (...)" will never apply to these folders, because these are not files that are going to be changed. These are folders that I only needed to use for a limited period of time, that might have been handy to sync while they're useful but don't need to be synced any more once I'm done with whatever task they were created for. Still, sometimes I want to hang on to them locally, if only for a couple of minutes, because, well, deleting a whole folder in one go can be scary. What I do now is rename/move the folder, then remove it from the client and then figure out myself what want to delete and what needs to be archived & where. You might say that's the 'wrong' way of using Nextcloud, but I'm a belt-and-braces kind of person.

But from this discussion (and the post that lead me here today) I conclude there are people out there who are not taking that extra precaution and are then surprised that files have 'disappeared'. Disappearing files - even if in reality its merely the impression to some users that files have disappeared (even though they should have know that their actions would have this result), is just about the worst thing that can happen with a data storage solution. So something to be avoided at all cost, I'd think.

So more importantly than my specific way of using the client of my data management routines in general, is that forcing users to choose between deleting the local files or keeping them, is a way of hammering home the point that local files will be deleted. And if they're not yet on the server, that means they're gone for good. That's basically why I thought it'd be a good idea to introduce that additional option. Sometimes it's useful to force people to make a choice. Yes, people should know that already, but when you have people asking whether this will be 'fixed' in future releases, you can conclude that this isn't behaviour that is intuitively understood by all users. So you can be a purist about it and say that people should have read the warnings and read the manual and if they don't, whatever happens is their own fault, or you can nudge them in the right direction. So my suggestion was a way of giving those people that extra nudge they need.

Plus, in my usage case, it'd save me that extra precautionary step.

irisv avatar Nov 25 '20 11:11 irisv

Will a fix be included in the next nextcloud release? Or is there something planned for a future release?

Since there has not been reached a consensus about how the current behavior should be altered, this is very unlikely.


That said, I would like to suggest an alternative to the proposals that have been made so far: Add a third option to move the folder out of the sync folder.

If the users choose this option, the client would fire up a "choose directory" dialog and then move the not-yet-synced-folder out of the synced folder to the folder chosen by the client.

dirdi avatar Nov 25 '20 12:11 dirdi

That said, I would like to suggest an alternative to the proposals that have been made so far: Add a third option to move the folder out of the sync folder.

If the users choose this option, the client would fire up a "choose directory" dialog and then move the not-yet-synced-folder out of the synced folder to the folder chosen by the client.

That'd also work for me. That's basically what I do now manually.

irisv avatar Nov 25 '20 12:11 irisv

I’d add a 4th option that seems more intuitive to me: sync anything put in a Nextcloud folder BEFORE deleting the local copy that is unchecked for syncing. That way the cloud copy exists but the local copy does not.

For those who wish to keep a static local copy of some or all of the files — they shouldn’t be in Nextcloud folder to start with.

robballan avatar Nov 25 '20 13:11 robballan

Maybe it should just forbid unchecking folders that are not yet synced to server with some kind of explanation why this checkbox is grayed out in disabled state and suggestion to move this folder out of sync folder manually in file manager.

The warning about removing synced folder from local file system is NOT the same as removing non-synced folder from local file system. In the 1-st case deleting data is safe. And in the 2-nd case deleting data is NOT safe. I think that removing local unsynced data is not a valid option, application should not do it in any case and should not suggest it. If user really wants to lose his data, he can just remove it in file manager manually.

sadr0b0t avatar Nov 25 '20 14:11 sadr0b0t

You should NOT be able to uncheck a folder for syncing UNTIL it has been synced upstream. That would solve it.

On Nov 25, 2020, at 9:13 AM, Anton [email protected] wrote:

Maybe it should just forbid unchecking folders that are not yet synced to server with some kind of explanation why this checkbox is grayed out in disabled state and suggestion to move this folder out of sync folder manually in file manager.

The warning about removing synced folder from local file system is NOT the same as removing non-synced folder from local file system. In the 1-st case deleting data is safe. And in the 2-nd case deleting data is NOT safe. I think that removing local unsynced data is not a valid option, application should not do it in any case and should not suggest it. If user really wants to lose his data, he can just remove it in file manager manually.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.


raa

robballan avatar Nov 25 '20 15:11 robballan

You should NOT be able to uncheck a folder for syncing UNTIL it has been synced upstream. That would solve it.

@b12f gives a very good case where this would not work: the initial setup of Nextcloud. The sync starts, you realize that there is a 200 GB subfolder you do not want to synchronize, uncheck it and the data is gone.

Not removing unsynced data is a solution that is safer.

wsw70 avatar Nov 26 '20 10:11 wsw70

The sync starts, you realize that there is a 200 GB subfolder you do not want to synchronize, uncheck it and the data is gone.

You can put sync on pause, go to file manager, move this folder somewhere else, continue to sync.

sadr0b0t avatar Nov 26 '20 12:11 sadr0b0t

You can put sync on pause, go to file manager, move this folder somewhere else, continue to sync.

That's like telling someone they should reboot their pc if nextcloud crashes instead of fixing the cause of the crash. This is at its core a UX problem, and the solution you're offering is still hugely bad UX, especially because it would still mean people lose valuable data if they think they can change syncing options in the nextcloud UI, and then don't understand the current warning (which a lot of people don't)

b12f avatar Nov 26 '20 12:11 b12f

especially because it would still mean people lose valuable data if they think they can change syncing options in the nextcloud UI, and then don't understand the current warning (which a lot of people don't)

My suggestion is to forbid removing unsynced data from nextcloud ui. The user should not be able to uncheck folder which not synced.

sadr0b0t avatar Nov 26 '20 12:11 sadr0b0t

You can put sync on pause, go to file manager, move this folder somewhere else, continue to sync.

I install Nextcloud, start the sync, realize that what is being synced is not what I want to sync, uncheck - read the warning that states that the files in the unchecked folder will be removed and not synced anymore - but hey, they are not synced yet, because I just started the first sync after installation.

Crap, they managed to get synced finally... Good thing I have a backup /s

What I am trying to say is that once the sync is done then maybe the warning is fine (I could live with that, the idea being that I d not want anymore these files to be in Nextcloud) - but not at the install stage while the sync is going on.

As for the prompt itself, it should leave the choice for the files to be removed or stay as they are (not synced anymore, with some consequences down the road such as diffs between files if they are re-synced.)

wsw70 avatar Nov 26 '20 12:11 wsw70

My suggestion is to forbid removing unsynced data from nextcloud ui. The user should not be able to uncheck folder which not synced.

This is an interesting suggestion - fair point. I just wonder how it could work for selective sync of files within a folder

wsw70 avatar Nov 26 '20 12:11 wsw70

You can put sync on pause, go to file manager, move this folder somewhere else, continue to sync.

You're imagining a user with perfect knowledge like yourself. But this usage case involves people who have just installed Nextcloud and added a folder they didn't mean to add. The UI should also be safe to use for them.

My suggestion is to forbid removing unsynced data from nextcloud ui. The user should not be able to uncheck folder which not synced.

That would solve the issue of lost data, but if if I understand you correctly then in this scenario where a user unwittingly added a 200 GB subfolder they do not want to synchronize, that would involve uploading that 200 GB subfolder first, then the user would uncheck the box, not understanding the full implications of the warning, resulting in that 200 GB of local data being deleted. Then I imagine that user has a minor panic attack, until they realise the data is safe on the server. But then they have to download that 200 GB again and remove it from the server, just to get back to where they started. So that's 400 GB of unnecessary and unwanted traffic and one very irritated new user.

irisv avatar Nov 26 '20 12:11 irisv

I just wonder how it could work for selective sync of files within a folder

NextCloud just does have partial syncing feature as far as I know. Unchecked folder in NextCloud UI just means that this folder exists on server but the client does not want to download it as local copy. It is not about partial syncing. Current UI allows to lose data by accident and I also think this is a bug.

For having partial sync for some data in enabled folders - I think this could be discussed as a separate feature (for example, this can be implemented as a king of .gitignore file in git).

sadr0b0t avatar Nov 26 '20 12:11 sadr0b0t

then the user would uncheck the box, not understanding the full implications of the warning, resulting in that 200 GB of local data being deleted

No, the user should not be able to uncheck the box for the folder which is not synced - it should be drawn in disabled state with some kind of (optional) explanation why it is disabled.

sadr0b0t avatar Nov 26 '20 12:11 sadr0b0t

image

Isn't it possible to change this dialog in such a way?

  • "Disable sync but untouch locally" Bevor the action starts, this folder/file is automatically added to the ignored files

  • "Disable sync and remove locally" Proceed as currently

  • "Cancel" Just stop

sojer avatar Nov 26 '20 12:11 sojer