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

Add "upload existing"-option to autoupload #2812

Closed
saibotk opened this issue Jul 8, 2018 · 20 comments
Closed

Add "upload existing"-option to autoupload #2812

saibotk opened this issue Jul 8, 2018 · 20 comments

Comments

@saibotk
Copy link

saibotk commented Jul 8, 2018

Hello guys,

I wondered if there is any reason, why the auto upload is just uploading new files, which from my perspective does not fulfill a good UX. As an example, i recently showed NC to my parents and the first expected behaviour was, that it would automaticially upload the whole folder and new files.
So that is, why i'd like to suggest to either add an info to the screen, stating that only new files will be uploaded, or add a "upload existing"-checkbox in the autoupload UI.

I think this could improve the UX on this significantly, because uploading a folder manually is not as easy as the autoupload interface, which seems to be more intuitive.

Just a suggestion, id like to hear your opinions on this,
Keep up the good work 👍

Greetings

@harrisonland
Copy link

I'd agree with this, tbh. It's one of the things that struck me as a bit odd when I first set up Nextcloud..

@AndyScherzinger
Copy link
Member

Hi @saibotk, hi @harrisonland - This is on the roadmap, see #285 but nobody had the time to implement it just yet and being a open source project contributions are always welcome ❤️

@harrisonland
Copy link

Thanks for response, @AndyScherzinger . Frankly, I wish I had more time to learn some programming and be able to contribute, but my wife would kill me! ;-)

Be assured that your efforts are really appreciated.

@saibotk
Copy link
Author

saibotk commented Jul 18, 2018

Thank you, I wish i had more time and less concurrent projects too, but i'll see...

@YounoYouno
Copy link

This would be the only feature i'd ever wish for in the NC app, thanks for getting this out there.

@javaisbetterthanpython
Copy link

javaisbetterthanpython commented Aug 5, 2018

@AndyScherzinger

What determines the prioritization of these feature requests? Seems like this is a high requested feature.

I looked at how this would be implemented, and my guess is basically, in the function to add a new directory to the auto-upload watchlist, these two steps would simply need to be taken:

  • Perform a directory listing of files in the target directory
  • Iterate through the list and add a new job for each existing file

I'm thinking this could be added either to SyncedFoldersActivity.class or actually even earlier in the MainApp.java between 538 and 539, just after dialog.dismiss() before context.startActivity(folderSyncIntent).

Would this approach make sense or am I missing something?

@AndyScherzinger
Copy link
Member

@javaisbetterthanpython yes something like that while @mario is the expert on this matter and I am sure if you would be happy to take on this task he'll sure be around to support you. Triggering this initial part within the SyncedFolderActivity seems fine while I can't say if it shouldn't be done within the Job itself directly since large, existing folders might be an issue if you do this within the activity...

As for prioritization that is hard to tell, @tobiasKaminsky and @mario both work for Nextcloud as a company so their priorities are a mix of community feedback and customer feedback. I am on the othr and do this in my spare time besides my day job so for me priority is rather looking at what I can get done within this limited timebox.

@javaisbetterthanpython
Copy link

Thanks for the reply.

@mario do you have any thoughts about the best approach to implement this capability? Does what I outlined seem reasonable, or should we take a different approach?

I looked at the code more today and am thinking that a "FilesSyncJob" needs to be created for existing files in the directory when it is first added to the "auto-upload" list in "SyncedFoldersActivity"? Is that right?

@AndyScherzinger
Copy link
Member

@tobiasKaminsky due to time constraints and work load, can you help out @javaisbetterthanpython? :)

@tobiasKaminsky
Copy link
Member

Is "upload existing" meant to be a one time action? E.g. a button in the settings of the synced folder?

Overall the main problem is if there are already images in the target folder, to distinguish if they need to re-upload or not:

  • We could simply compare them by name, but this will not work on devices which name their image img_001, img_002.
  • we could simply upload everything, leading to have duplicated files, which is a bad UX

An idea, which is not perfect, is to use some kind of hash:

  • first check file name
  • if file name already exists, check hash
  • if hash is the same do nothing, else upload, but append (2)

@AndyScherzinger
Copy link
Member

Imho it should be a one time action and could be triggered whenever an auto upload is configured for a folder the very first time (can be detected since we don't have a database record yet) and then simply push all existing files to the server.
We could of course add a button so users can manually trigger such a thing at a later point in time while this will make it a bit more complicated since we then definitely need the file checks you described @tobiasKaminsky .

@javaisbetterthanpython
Copy link

javaisbetterthanpython commented Aug 7, 2018

Agree with @AndyScherzinger - Also, I think the complexity to manually trigger this at-will would be unnecessary.

From my perspective, just getting a one time action to trigger whenever a new auto-upload is configured for a folder for the first time would be sufficient.

@tobiasKaminsky Does it make sense to include that one-time action in the updateToAutoUpload() function in MainApp.java? Or is it better off in the SyncedFoldersActivity.java?

I've only spent a couple hours reading the source and am starting to get an idea of how things work, but any thoughts would be appreciated.

@AndyScherzinger
Copy link
Member

Afaik it should be fine to include it in the SyncedFoldersActivity for a one-time action. If I remember correctly anything else is about restarts so if the auto upload queue gets filled with the existing files then we are all set.

@tobiasKaminsky
Copy link
Member

SyncedFoldersActivity is good.
We still need some sort of file check, as it might that in the target folder on server are already files named the same.
(on fresh installations chances are not that high, but on re-installations of the app this might happen)

@AndyScherzinger
Copy link
Member

We still need some sort of file check, as it might that in the target folder on server are already files named the same.
(on fresh installations chances are not that high, but on re-installations of the app this might happen)

Definitely especially since this also will happen on new devices if you had a device before that used the same naming schema for images.

@javaisbetterthanpython
Copy link

What about requiring the destination folder to be empty? Would that be too much negative impact on the user experience?

@mario
Copy link
Contributor

mario commented Aug 12, 2018

Honestly, I always thought either that OR requiring a Checksum plugin on the server (to generate file hash) in order to see if files are the same. But you still lack other things - e.g. timestamps, etc etc.

@AndyScherzinger
Copy link
Member

Checking if the folder exists/is-empty might be a first step and maybe then if this is not the case renaming the files appending a (1) for the filename might work since a non-empty folder is probably happening quite often, for example when you have a new phone and activate the auto upload for the camera folder.

@Redsandro
Copy link

I've just started using the Android app, and this is a bit of an UX fail. You get to visually confirm the folder you choose to upload. Your brain automatically thinks: "So these are the files that will be syned huh."

Then you click the cloud icon, and nothing happens. So you click the options menu. No option to sync existing files.

The whole reason for syncing is so you can easily access your files and/or remove them from your phone, right? That's why I installed this app, but it doesn't sync anything, because the files pre-existed on my phone. 😅 I need to find another way to copy the media folders off my phone, because this app only works if I had installed the app as the absolute first thing when I bought the phone. 😂

That's why I'm happy with #2873 👍

@AndyScherzinger
Copy link
Member

to be released with 3.11. via #4788

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

8 participants