Setting to also upload existing files #4788
Conversation
AndyScherzinger
added
2. developing
needs info
|
In the original PR it was suggested that this should not be a setting but rather a one time action with a prompt when exiting the settings view. However, this would mean that when enabling/disabling the synced folder through the button from the synced folder list (and not the full parameters view) would skip files created while disabled, and not resync them when reenabled. Maybe this setting should be on by default ? This might also mean needing to warn the user that files with the same name at destination will be overridden while there is not conflict detection/management. |
Fully agree and so would probably @jancborchardt and @nextcloud/designers and @jospoortvliet. The user shouldn't have to think about it. It even say when a db entry is initially created the user could be asked if they want to upload existing files. So I'd rather see it an an initial opt-out thing. If initially the user decided to not upload existing files then they are left out, else the users simply want to always have them uploaded (which is likely the default: Backup anything and don't bother me). |
|
When enabling (don't matter how), this then takes all images from existing folder and uploads it? We could do it this way:
|
|
@tobiasKaminsky In my opinion
Is the best option in combination with a warning that already existing files with the same name don't get overwritten. Then a user has the choice to choose a different upload folder (or delete files in this folder before uploading) if he/her wants to upload everything (and he/her isn't sure about whether there are already files with the same name in this upload folder). Best thing is, that this initial upload then can be triggered more then one time and only uploads files that are (based on the name) not already uploaded into this upload folder. |
|
FWIW #2873 (comment) the APK works well for me |
|
Sadly, #4788 (comment) doesn't seem to work. I look forward to this feature. |
|
@cpw Sadly, please be more helpful/mindful, "doesnt seem to work" does really not help anyone to improve the issue, how exactly? please fill in a complete error report, device type, what you tried, messages you saw (paste the text as it is exactly), version number of client, android and nextcloud server, connection type etc |
|
Nevermind. I reinstalled this morning, and it seems to be trying to sync existing files this time. 🤷♀️ |
|
@Excel1 [edit: you referenced a comment from another PR while quoting the newest one] Also keep in mind that this PR modifies the database which means in order to test in realistic conditions, you should perform a full manual uninstall of the previous test versions before installing this new APK. Also this PR is still currently in discussion and development. |
|
@ArisuOngaku thats absolutely right. I only wanna give feedback :) #4788 (comment) i recently tested this apk. Same issues: (Note 8 - Android 9) After clicking the cloud icon below the tab autoupload the app restart. |
|
@Excel1 Thanks for that! I can't reproduce this crash. Would you mind sharing your logs? (settings -> logs -> 3 dots up right corner -> send by email, and you can actually upload this as a file if you select the nextcloud app itself) |
|
@ArisuOngaku sure. here it is. (i censored sensitive data) |
|
@Excel1 did you censor this line? the URL looks really broken to me |
|
@Excel1 Thanks! I see no stack trace though if I'm not mistaken, this is the part where you experience the crash: My best guess is that the local database was not wiped, did you fully uninstall the Nextcloud QA app before installing the new APK? If yes, then try that again but clean the app's cache before. What indicates that the database was correctly wiped is that you need to login again when you start the app. |
|
@ArisuOngaku Yes i did. And now i did it again (and deleted cache and app data). Should i also uninstall the original Nextcloud App ? |
|
@Excel1 No, I see no reason the original Nextcloud app would interfere. When I uninstall (manually) then install again the same APK (on emulator), my cache and logs get deleted. By any chance, are you correctly opening the |
|
@ArisuOngaku Okay. I definitely uninstalled the right version (QA) and deleted the right cache (QA). Here are the log (definitely from the QA version) 🤔 |
|
@Excel1 Thanks a lot for your patience. I looked deeper in your logs and it looks like your cache actually gets properly deleted. Are you familiar with android debug tools (adb) and the logcat utility? Recording the logs from there while reproducing the crash would give us a more precise and definitive answer to what's happening. |
|
@ArisuOngaku No problem, i have to say thank you - for helping me. |
|
@AndyScherzinger I rebased this PR on the latest master to fix conflicts (and also removed the typo commit and integrated its changes in the first one by koying), am I allowed to force push and work on this PR? |
@ArisuOngaku absolutely yes! ❤️ 👍 Only when you label it |
ashpieboop
force-pushed
the
instantupload_all
branch
2 times, most recently
from
44f75ec to
0b1c6e1
Compare
Codacy360Lint
SpotBugs (new)
SpotBugs (master)
|
|
APK file: https://www.kaminsky.me/nc-dev/android-artifacts/12728.apk |
Codacy360Lint
SpotBugs (new)
SpotBugs (master)
|
|
Screenshot test failed, but no output was generated. Maybe a preliminary stage failed. |
|
You're welcome! :) |
|
@ArisuOngaku Congrats for pushing this to the end :) |
3f62afa Merge remote-tracking branch 'origin/master' into dev 5d428fe Merge pull request #4788 from nextcloud/instantupload_all 49f3a67 ConflictsResolveActivity: code style and readability improvements 6540e3e Upload list conflicts: replace trash icon with menu and add resolve action a4ff1f1 Customize upload sync conflict notification strings e77ffa0 Fix synced folder layout form control widgets alignment 91ef307 Fix database migration of field forceOverwrite to NameCollisionPolicy 52d089b FileUploader: fix codacy issues and SpotBugs ec27f84 FileUploader: code cleanup 7713a28 FileUploader: require explicit NameCollisionPolicy and change default used value 6783e8a Make file uploads ask the user what to do when the file already exists on remote 213002f Make newly created synced folders auto upload existing files by default 1b7bd72 ADD: [instantupload] setting to also upload existing files c243453 Merge pull request #5465 from nextcloud/fix/create-folder-crash-autosync d178cde daily dev 20200218
|
I am using version 3.14 of the App and wonder why this feature is not included although it was already part of the 3.11 milestone? |
|
@bastian-src just checked and the feature is there! Depending on your screen resolution you might have to scroll down in the auto upload config dialog (item is at the bottom) how to handle existing files in such a case and the 3rd checkbox from the top should say (upload existing). So can you post a screenshot of the auto upload config dialog of your device? |
|
@AndyScherzinger sorry, my fault. The option is in the menu but I thought there would be another option/action necessary in order to trigger the upload of existing files. So, I think the appropriated behaviour would be that it uploads the existing files right after clicking on Save? |
|
Hmmm, the upload should happen within 15 minutes |
|
After 1h, the upload suddenly started. Thanks for your immediate response! |
|
Sweet and glad to hear :) |
successor for #2873
Originally created by @koying
Please be kind, first time I put my nose into the code.
This adds a setting to the instant uploads to also upload files already present on the device at the time instant upload is setup.
Ref: https://help.nextcloud.com/t/why-doesnt-android-client-upload-existing-pictures/10365
95% of the PR is for handling of the setting itself.
It might be worth wondering whether this deserves a setting at all, really, ie whether not uploading existing files is worthwhile, as it's a bit counter-intuitive, imo