Remove user notification for autoupload edge cases #12413

JonasMayerDev · 2024-01-26T17:21:02Z

File Conflicts

This PR should finally fix #11974.
When ever an upload conflict occurs, the app checks if size, creation and modification date for the local file and remote file are the same. If so, the app assumes it is the same file and skips upload and adds the upload to the uploaded successfully tab with a bold text to inform the user the upload is skipped. The user is not directly notified when such a "same file conflict" occurs.

Local file removed

The same "silent" behavior is used when a local file is not present anymore to fix error notifications where users would delete a file directly after creating it (e.g. making a photo and deleting it) in auto upload as suggested by @tobiasKaminsky.

Tests written, or not not needed

tobiasKaminsky · 2024-01-29T11:10:52Z

/backport to stable-3.28

ZetaTom

During my brief testing everything worked as expected.

app/src/main/java/com/owncloud/android/ui/adapter/UploadListAdapter.java

ZetaTom · 2024-01-30T08:03:45Z

app/src/main/java/com/owncloud/android/ui/adapter/UploadListAdapter.java

+        if (item.getUploadStatus() == UploadStatus.UPLOAD_SUCCEEDED &&
+        item.getLastResult() != UploadResult.UPLOADED){
+            itemViewHolder.binding.uploadStatus.setVisibility(View.VISIBLE);
+            itemViewHolder.binding.uploadStatus.setTypeface(null, Typeface.BOLD);


A bold message looks a little out of place, in my opinion, as all other error messages are displayed using the normal typeface.

@ZetaTom This is intended as the "same file conflict" will be displayed under the category uploaded where it should stand out to show the user that something special was done to this file, and it was not uploaded but skipped... What do you think, is this ok for you?

@ZetaTom, FYI I talked with Tobias, and he also said that it should not be bold but we for now just merged and i change that in a separate PR

lckarssen · 2024-01-31T14:05:43Z

I have been using this QA APK for the last four days, since this PR was mentioned in #11974 (comment) (alongside the regular Nextcloud app, v3.27.0).

Interestingly, I haven't had any conflicts since. From neither app, so maybe the conflict resolution of the QA app also prevents the conflicts from happening in the regular version of the app.

Moreover, when looking at the battery usage over the past few days (#12141, #11983), I see that the regular app has used 7.1% of battery, compared to 0.6% for the QA version of this PR. So the increased battery usage may indeed have something to do with the conflicts.

JonasMayerDev · 2024-01-31T16:01:54Z

@lckarssen I think the improved battery consumption has to do with this PR: #12337 that should also be included in future releases. It's good to hear that the conflicts are gone, but the changes in the QA app should not affect the normal app...

Signed-off-by: Jonas Mayer <[email protected]>

…apter.java Co-authored-by: Tom <[email protected]> Signed-off-by: Jonas Mayer <[email protected]>

github-actions · 2024-02-07T14:43:55Z

blue-Light-Screenshot test failed, but no output was generated. Maybe a preliminary stage failed.

github-actions · 2024-02-07T15:49:13Z

Codacy

Lint

Type	master	PR
Warnings	69	69
Errors	3	3

SpotBugs

Category	Base	New
Bad practice	68	68
Correctness	73	73
Dodgy code	359	359
Experimental	2	2
Internationalization	7	7
Malicious code vulnerability	2	2
Multithreaded correctness	6	6
Performance	56	56
Security	18	18
Total	591	591

github-actions · 2024-02-07T15:58:04Z

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/12413.apk

To test this change/fix you can simply download above APK file and install and test it in parallel to your existing Nextcloud app.

lckarssen · 2024-02-07T16:04:11Z

@lckarssen I think the improved battery consumption has to do with this PR: #12337 that should also be included in future releases. It's good to hear that the conflicts are gone, but the changes in the QA app should not affect the normal app...

@JonasMayerDev You were right about the fact that the QA app shouldn't affect the "old" app. I recently had a case again where some of the uploads in the old app had conflicts, whereas the QA uploads went fine. So, another 👍 for this PR.

JonasMayerDev · 2024-02-07T19:42:38Z

@lckarssen That's really good to hear! Thank you for testing this PR :)

tovine · 2024-02-08T14:10:50Z

I've tried this build over the last week and saw this new message "Same file found on remote, skipping upload" several times, but there were also quite a few false duplicates that were not caught.
So there seems to be some work left before it's 100%, but a definite improvement over how it used to be so I think it's a very good start!

Hopefully you'll revisit the idea of supporting server side file checksums to get a more robust check in the future, but in the meantime I'd like to say thank you for the work you put in! 🎉

JonasMayerDev · 2024-02-09T21:12:52Z

I've tried this build over the last week and saw this new message "Same file found on remote, skipping upload" several times, but there were also quite a few false duplicates that were not caught. So there seems to be some work left before it's 100%, but a definite improvement over how it used to be so I think it's a very good start!

Hopefully you'll revisit the idea of supporting server side file checksums to get a more robust check in the future, but in the meantime I'd like to say thank you for the work you put in! 🎉

@tovine Do you use a multi account setup? If there are still conflicts, I have absolutely no idea where they come from, except if you use a multi account setup. Then I would guess the conflicts come from files that are uploaded to the wrong account and collide with files from the wrong account as documented here #12516?
If you don't use a multi account setup, I would suggest that we wait until more people tested the latest releases to see if it is still a problem for a large amount of people.

tovine · 2024-02-10T15:36:16Z

No, I don't (at least not in the Nextcloud QA app).
I have however reconfigured the app with media folders to be synced that I've already synced with the app before, and selected to sync existing files as well, so in that sense I guess it's a "worst case scenario".

Yethal · 2024-02-13T14:17:28Z

@AndyScherzinger would it be possible to fit this change in 3.28.0 release somehow? It's a major usability improvement for Android users.

JonasMayerDev · 2024-03-11T15:06:22Z

@Yethal The changes should actually be included in the latest 3.28.0 release using this back port: #12452
Do you still experience the Issue with the latest release?

Yethal · 2024-03-12T11:30:29Z

I don't everything has been smooth sailing for the past few days.

JonasMayerDev requested review from tobiasKaminsky and ZetaTom January 26, 2024 17:21

JonasMayerDev linked an issue Jan 26, 2024 that may be closed by this pull request

Excessive file upload conflicts after update to 3.26.0 (not before) #11974

Closed

4 tasks

JonasMayerDev force-pushed the 11974-excessive-file-upload-conflicts-after-update-to-3260-not-before branch 2 times, most recently from 7d73d45 to 69ca33b Compare January 27, 2024 11:46

JonasMayerDev mentioned this pull request Jan 27, 2024

Excessive file upload conflicts after update to 3.26.0 (not before) #11974

Closed

4 tasks

backportbot bot added the backport-request label Jan 29, 2024

JonasMayerDev force-pushed the 11974-excessive-file-upload-conflicts-after-update-to-3260-not-before branch 3 times, most recently from 0421e7d to 283c875 Compare January 29, 2024 14:52

JonasMayerDev removed the backport-request label Jan 29, 2024

JonasMayerDev mentioned this pull request Jan 29, 2024

[stable-3.28] Remove user notification for autoupload edge cases #12452

Merged

1 task

JonasMayerDev force-pushed the 11974-excessive-file-upload-conflicts-after-update-to-3260-not-before branch 3 times, most recently from 16264db to 8bdd91a Compare January 29, 2024 19:43

tobiasKaminsky approved these changes Jan 30, 2024

View reviewed changes

ZetaTom reviewed Jan 30, 2024

View reviewed changes

JonasMayerDev force-pushed the 11974-excessive-file-upload-conflicts-after-update-to-3260-not-before branch from 8bdd91a to 323195d Compare January 30, 2024 11:31

JonasMayerDev force-pushed the 11974-excessive-file-upload-conflicts-after-update-to-3260-not-before branch 3 times, most recently from c618848 to e46ff29 Compare February 6, 2024 09:07

JonasMayerDev and others added 2 commits February 7, 2024 15:05

Redo changes, manuel rebase

1048fb3

Signed-off-by: Jonas Mayer <[email protected]>

Update app/src/main/java/com/owncloud/android/ui/adapter/UploadListAd…

1329a88

…apter.java Co-authored-by: Tom <[email protected]> Signed-off-by: Jonas Mayer <[email protected]>

JonasMayerDev force-pushed the 11974-excessive-file-upload-conflicts-after-update-to-3260-not-before branch from e46ff29 to 1329a88 Compare February 7, 2024 14:05

tobiasKaminsky merged commit d499675 into master Feb 8, 2024
19 of 20 checks passed

delete-merged-branch bot deleted the 11974-excessive-file-upload-conflicts-after-update-to-3260-not-before branch February 8, 2024 06:10

AndyScherzinger added this to the Nextcloud App 3.29.0 milestone Feb 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove user notification for autoupload edge cases #12413

Remove user notification for autoupload edge cases #12413

JonasMayerDev commented Jan 26, 2024

tobiasKaminsky commented Jan 29, 2024

ZetaTom left a comment

ZetaTom Jan 30, 2024

JonasMayerDev Jan 30, 2024

JonasMayerDev Feb 8, 2024

lckarssen commented Jan 31, 2024 •

edited

Loading

JonasMayerDev commented Jan 31, 2024

github-actions bot commented Feb 7, 2024

github-actions bot commented Feb 7, 2024

github-actions bot commented Feb 7, 2024

lckarssen commented Feb 7, 2024

JonasMayerDev commented Feb 7, 2024

tovine commented Feb 8, 2024 •

edited

Loading

JonasMayerDev commented Feb 9, 2024

tovine commented Feb 10, 2024

Yethal commented Feb 13, 2024

JonasMayerDev commented Mar 11, 2024

Yethal commented Mar 12, 2024

Remove user notification for autoupload edge cases #12413

Remove user notification for autoupload edge cases #12413

Conversation

JonasMayerDev commented Jan 26, 2024

File Conflicts

Local file removed

tobiasKaminsky commented Jan 29, 2024

ZetaTom left a comment

Choose a reason for hiding this comment

ZetaTom Jan 30, 2024

Choose a reason for hiding this comment

JonasMayerDev Jan 30, 2024

Choose a reason for hiding this comment

JonasMayerDev Feb 8, 2024

Choose a reason for hiding this comment

lckarssen commented Jan 31, 2024 • edited Loading

JonasMayerDev commented Jan 31, 2024

github-actions bot commented Feb 7, 2024

github-actions bot commented Feb 7, 2024

Codacy

Lint

SpotBugs

github-actions bot commented Feb 7, 2024

lckarssen commented Feb 7, 2024

JonasMayerDev commented Feb 7, 2024

tovine commented Feb 8, 2024 • edited Loading

JonasMayerDev commented Feb 9, 2024

tovine commented Feb 10, 2024

Yethal commented Feb 13, 2024

JonasMayerDev commented Mar 11, 2024

Yethal commented Mar 12, 2024

lckarssen commented Jan 31, 2024 •

edited

Loading

tovine commented Feb 8, 2024 •

edited

Loading