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

deleting files on tab 'Auf dem Gerät' deletes also files on nextcloud instance #2718

Closed
computersalat opened this issue Jun 16, 2018 · 24 comments

Comments

@computersalat
Copy link

Actual behaviour

Having an 'auto-upload' folder defined which I have choosen to 'syncronize'. The result was, that the folder appeared on Auf dem Gerät on my smartphone. I didn't wanted to have those files twice or double ...

  • one in the original folder e.g. /storage/emulated/0/ProgramData (which is uploaded to /OnePlus2/ProgramData)
  • and the sync in /storage/emulated/0/nextcloud/OnePlus2/ProgramData

so I decided to delete the local sync OnePlus2 which is shown under tab Auf dem Gerät ... but also /OnePlus2 folder on my nextcloud instance was deleted

Expected behaviour

From my point of view only the local sync of files should be deleted when deleting files on the tab Auf dem Gerät on your smartphone and NOT the source on the nextcloud instance ...

Steps to reproduce

  1. sync files from nextcloud that they will be downloaded from your nextcloud instance to your device
  2. select 'Auf dem Gerät' to see the local synced of files
  3. delete the sysnced folder in the tab 'Auf dem Gerät'
    ... before you try enable the usage of Trash on your nextcloud that you can restore deleted files.

Environment data

Android version: 8.1.0

Device model: OnePlus 2

Stock or customized system: RR-O-v6.0.0-20180530-oneplus-Official

Nextcloud app version: 3.1.0

Nextcloud server version: 12.0.7

Logs

Web server error log

not needed cause wrong behaviour, not an error

Nextcloud log (data/nextcloud.log)

not needed cause wrong behaviour, not an error
@tobiasKaminsky
Copy link
Member

The "on device" tab is only a limited view of the complete file listing, so the behaviour is the same.
If you delete a file/folder you get this dialog:
2018-06-18-074835

yes: delete on server and local
local only: only on device, not on server
no: nothing deleted

--> Did you accidentally clicked "yes"?

@computersalat
Copy link
Author

I can't remember about that 'dialogue', but will try again to see if I 'accidentally' clicked "yes"...
anyway it is confusing. the 'on device' tab should only handle 'local' files...

@AndyScherzinger
Copy link
Member

anyway it is confusing. the 'on device' tab should only handle 'local' files...

This is likely also a translation issue since the original/English term we use here is available offline.

@tobiasKaminsky
Copy link
Member

It seems to be confusing for all of us ;-)

In drawer it is named "on device", which is showing all files that are downloaded (and also the parent folders):

2018-06-18-094004

"available offline" is for files, which then get synced if any side (local or remote) is changing.

anyway it is confusing. the 'on device' tab should only handle 'local' files...

Do we want then switch the dialog?
"Do you really want to delete welcome.txt from your device?
"yes": delete only local
"also on server": delete local & remote
"no": does nothing

@jancborchardt what do you think? Or is this bad to have the "same" action (delete) behaving different?

@jancborchardt
Copy link
Member

Delete should behave the same from wherever you do it. :)

(Which currently means showing this dialog. But yes – ideally it just deletes it off the server directly with a toast to undo. Because currently this extra modal is only useful for the case of only removing the cached version on the device, which is kind-of-edge-casey.)

@tobiasKaminsky
Copy link
Member

which is kind-of-edge-casey

well, this is what this issue is all about.
And as @computersalat pointed out, if you do not read correctly / assume that only the local file would be deleted, this can lead to "data loss".

@jancborchardt
Copy link
Member

It seems like it might stem a from a confusion about the word "On device". Because it’s just a filter on the list of files, the same actions should apply.

Maybe renaming it to "Available offline" might be good? Of course there’s also the explicit "available offline" functionality – but files from there show up there too, and de facto all of the files "On device" are "Available offline" in the sense of the word.

The point being that deleting a file which is labeled "on the device" it makes sense to assume that it would only be deleted off the device. But for a file which also happens to be "available offline", deleting it will actually delete it.

@tobiasKaminsky
Copy link
Member

Having two completely different things called "available offline" will confuse all.
If we want to change naming, then we should change
"on device" -> "available offline" (which will show all files/folders that are downloaded, independent if they are one-time downloaded or "available offline")

"available offline" (on files) -> "keep in sync" / permanently synced / 2way sync
(something that shows that a real sync is done. At least from my point of view, the main point is not "offline", but "sync" (as even a manually downloaded file is offline available, but will never get updated).

(we had a similar discussion ages ago and there "available offline" was chosen, but what was the previous name? I do not remember…)

@tobiasKaminsky
Copy link
Member

Ah, found, history is (from old to new):

  • keep file up to date (internally keep_in_sync)
  • favorite (internally keep_in_sync)
  • favorite (internally favorite)
  • set as available offline (internally favorite)

@tobiasKaminsky
Copy link
Member

<string name="favorite">Set as available offline</string>
<string name="unfavorite">Unset as available offline</string>
<string name="favorite_real">Add to favorites</string>
<string name="unset_favorite_real">Remove from favourites</string>

This is a real mess.
Once we have decided something, the internal names should reflect this.

I vote for "keep in sync".

@AndyScherzinger
Copy link
Member

AndyScherzinger commented Jun 28, 2018

Not sure if discussing the used term is the right first step. I'd rather would like to challenge the feature set here:

  • Download
  • Available Offline
  • Favorite

In my opinion having these 3 doesn't make much sense (from a user perspective) because:

  • If a file is physically stored on the device I'd argue it should always be kept in sync (who would want a legacy version of the file at hand?). So Downloaded and Available Offline should be just the same thing
    • Download == Available Offline
    • Rename it to Keep in sync
  • Favorite afaik also started to drift towards a Download/Available Offline thing (but not consistently/completely). So I think there are variations to be discussed:
    • Favorite == Bookmark
    • Favorite == Bookmark + Keep in sync

For a first rewrite I vote for:

  • Download == Available Offline
  • Rename it to Keep in sync
  • Favorite == Bookmark

...because else Favorites are a subset files kept in sync and in case a user favs a folder they might not want to get the whole thing mirrored to their device.

@tobiasKaminsky
Copy link
Member

Download == Available Offline

Available offline is a periodic scan that refreshes all "available offline" files.

Rename it to Keep in sync

If we want that every downloaded file is also up to date, then we do not need "keep in sync" at all?
My concern with this approach is:

  • a background scan that syncs all downloaded files when of wifi, is fine
  • what if on the road and entering a large folder, where 400mb has changed -> auto sync all of them is a nogo, so we would then also have to show a warning / outdated file.

Favorite == Bookmark

Should remain favorite as it is the same as on server.

@AndyScherzinger
Copy link
Member

If we want that every downloaded file is also up to date, then we do not need "keep in sync" at all?

We would still need it for folders (which we do not have at all atm)

My concern with this approach is

completely agree with your point here

Should remain favorite as it is the same as on server.

Which it kind of isn't at the moment. Marking a file as Favorite in the details screen will download the file and mark it available offline!

@tobiasKaminsky
Copy link
Member

tobiasKaminsky commented Jun 28, 2018

We would still need it for folders (which we do not have at all atm)

We then would have it only for folders:

  • every downloaded file is automatically "keep in sync", so content change is reflected, should also work if file is deleted on server
  • on folder: new files get downloaded / old files get removed

Sync would then be

  • background job, to work with doze
  • every x hours
  • only on unmetered wifi
  • sync every downloaded file: update content or delete file
  • sync every "keep in sync" folder: download new files (file update / deletion is done in step above)

When entering a "keep in sync" folder with wifi:
sync folder: download new files, update all files, delete old files

When entering a "keep in sync" folder without wifi:

  • get new file listing
  • show files with red cross as outdated, if they changed
  • delete old files
  • add new files, but do not download

When entering a regular folder (independent from wifi):

  • get new file listing
  • show downloaded files with red cross as outdated, if they changed
  • delete old files
  • add new files, but not downloaded

@AndyScherzinger
Copy link
Member

AndyScherzinger commented Jun 28, 2018

@tobiasKaminsky looks good to me, except for the last scenario

When entering a regular folder (independent from wifi)

When entering a regular folder (independent from wifi):

  • get new file listing
  • show files with red cross as outdated (if they have been downloaded before)
  • delete old files
  • add new files, but not downloaded

@tobiasKaminsky
Copy link
Member

👍 updated the 3 scenarios a bit

@jancborchardt
Copy link
Member

From the user perspective, I agree with @AndyScherzinger’s comment #2718 (comment), but would go even simpler:

  • Every downloaded file should of course be kept up to date, as we should never have outdated files.
  • Every favorited file should also be downloaded and kept up to date, since it seems to be so important that you favorited it.

That’s it.

@AndyScherzinger
Copy link
Member

Every favorited file should also be downloaded and kept up to date, since it seems to be so important that you favorited it.

I strongly suggest to not do this because keep-in-sync != favorite, because:

  • favorite-state is kept in sync with the server: marking a file as favorite would mean we auto download/kep-in-sync with the server, also all clients would then have to behave that way, furthermore if on the Android client a users sets a file to favorite all other clients would then also have to auto download/kept-in-sync
  • looking at the first argument (even though this feature hasn't even been started yet) when a user marks a folder as a favorite by @jancborchardt's description all files/folders within this folder would have to be downloaded/kept-in-sync which might be an issue (space-wise)
  • if and when should this happen as mentioned by @tobiasKaminsky: marking a lot of (large) files as favorites on the server should then be auto-downloaded to the client (on wifi only? on cellular? notification + user interaction as-in on demand?)

TD;DR I think this needs further discussion and is a quite complex feature also looking a #2783 where users might expect no physical copy at all when opening files from the server but downloading only on demand.

@tobiasKaminsky
Copy link
Member

Every favorited file should also be downloaded and kept up to date, since it seems to be so important that you favorited it.

For favorited files I would be fine with auto downloading it (on wifi).
But as @AndyScherzinger this should not be done automatically on folders, this is why we have "keep-in-sync".
Having two meanings of favorite (download on file / no download on folder) is strange and thus we should do no auto-downloading at all.

Another point for this:
Imagine you have 30 favorited images, where it is totally fine not to have a file downloaded, but you marked them only, that they stay on top of a folder.
--> you are forced to have a downloaded copy

So:
If an user is downloading a file, he is doing it on purpose and thus this file should be kept up to date.
Favorite is just a different and complete independent feature and should not mixed in here.

In general:
There seem to be different expectation what our app is doing (also from other issues)

  • cache
    • keep files only in cache, so no extensive space is used
    • no real access from other apps (yes this was somewhere wanted)
  • file access like desktop
    • sync folders
    • keep all files
    • 2 way sync

Supporting both is nearly impossible to manage, so we have to choose and I would like to go for 2.

@jancborchardt
Copy link
Member

Ok, good points @AndyScherzinger. So then favorites are not downloaded by default – except if you do download them.

But then I still think that keep-in-sync is not necessary because once you use "Download", downloaded files should absolutely all be kept in sync, also for complete folders.

Which in the end is what @AndyScherzinger proposed at #2718 (comment), right? :) So then I agree, good direction! 🚀

@AndyScherzinger
Copy link
Member

@jancborchardt Yes that is what I proposed 👍

@tobiasKaminsky
Copy link
Member

So, I implemented a sync job that syncs all downloaded files and removes them if they are no longer on the server.
This runs only when of unmetered wifi, and only one job at a time.
How often should this run? Currently it is 15min.

I therefore removed "keep in sync".

For folders we currently have a manual sync, which we (still) want to extend to have a real 2way sync later.

@AndyScherzinger
Copy link
Member

Every 15 minutes sounds fine to me and we could also improve upon this once it is on master to have some more logic for calculating the intervals based on the change frequency, if files change once a week would don't need to check every 15 minutes I guess :)

@stale
Copy link

stale bot commented May 10, 2019

This request did not receive an update in the last 4 weeks. Please take a look again and update the issue with new details, otherwise the issue will be automatically closed in 2 weeks. Thank you!

@stale stale bot added the stale label May 10, 2019
@stale stale bot closed this as completed May 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants