Show only media folders that exist

[tobiasKaminsky]

Fix #3555

Signed-off-by: tobiasKaminsky [email protected]

[codecov]

Codecov Report

Merging #3587 into master will decrease coverage by <.01%.
The diff coverage is 0%.

@@             Coverage Diff             @@
##             master   #3587      +/-   ##
===========================================
- Coverage      6.05%   6.05%   -0.01%     
  Complexity        1       1              
===========================================
  Files           313     313              
  Lines         30761   30773      +12     
  Branches       4424    4427       +3     
===========================================
- Hits           1863    1862       -1     
- Misses        28621   28633      +12     
- Partials        277     278       +1

Impacted Files	Coverage Δ	Complexity Δ
...oud/android/ui/activity/SyncedFoldersActivity.java	0% <0%> (ø)	0 <0> (ø)	⬇️
.../com/owncloud/android/datamodel/MediaProvider.java	9.63% <0%> (ø)	0 <0> (ø)	⬇️
.../third_parties/daveKoeller/AlphanumComparator.java	82.14% <0%> (-1.2%)	0% <0%> (ø)
...ncloud/android/ui/activity/UploadListActivity.java	0% <0%> (ø)	0% <0%> (ø)	⬇️
...owncloud/android/ui/adapter/UploadListAdapter.java	0% <0%> (ø)	0% <0%> (ø)	⬇️

[AndyScherzinger]

@tobiasKaminsky would have to test this but I always thought that our notification trigger for a newly found folder will also write a db entry into our synced folders table which will then lead to the folder still being shown even after the deletion! So I think this fix won't help but I might be mistaken... :/

[tobiasKaminsky]

You are partly right:
For non-enabled folders this fix is working.

But:

• setup a folder as AutoUpload
• delete it
• see

Shall we silently remove them? What if this is just a folder on a removable sdcard and will re-appear later?
I would rather keep it but just show a proper thumbnail/default thumbnail?

[tobiasKaminsky]

As this is persisted in database, this also survives restarts, @AndyScherzinger

[AndyScherzinger]

Shall we silently remove them? What if this is just a folder on a removable sdcard and will re-appear later?
I would rather keep it but just show a proper thumbnail/default thumbnail?

I guess to be discussed with @nextcloud/designers. From a technical point of view it definitely needs to be removed for several reasons: It clutters the ist over time since it'll have more and more dead/non-existing folders over time so they shouldn't be shown when not present (anymore). Talking about deleting the entries complpetely: you need to do a file check per db entries if still present on the file system every time you load this list and that is procecssing time not well spent and will also grow over time. So to me it leaves us with the decision on how to handle such entries for SD cards where you still can have the same scenario: folders that don't exist anymore. So even there I am fine with deleting such db entries. Entries that shouldn't be deleted (right away) are the one that are active (even though the folder doesn't ecist anymore). The would then be deleted after de-activation and reentrering the auto upload screen at a later point in time.

Just my 2 cents :)

[tobiasKaminsky]

So to summarize:

• delete non-enabled folders
• keep enabled folders

even if the local folder is currently not available/deleted on purpose.

[tobiasKaminsky]

Added :)

[AndyScherzinger]

Correct while I'd say we still have an edge case :/

A folder with images that get created and deleted regularly will now work fine from a auto upload screen perspective but will now lead to a new folder notification in case the auto upload screen has been opened in between.
So maybe we need to keep the DB entries but flag them to then prevent the notifications for that folder? Other than that I think the notification implementation is flaws since it should maintain its own list of folder for which the app launches a notification so that these two things operate indipendently...

[tobiasKaminsky]

As discussed on IRC, we currently keep this approach, but if problems arises, we will use the new approach suggested by @AndyScherzinger

[nextcloud-android-bot]

Lint

Type	master	PR
Warnings	69	69
Errors	0	0

FindBugs (new)

Warning Type	Number
Bad practice Warnings	33
Correctness Warnings	155
Internationalization Warnings	15
Malicious code vulnerability Warnings	12
Multithreaded correctness Warnings	9
Performance Warnings	119
Security Warnings	58
Dodgy code Warnings	125
Total	526

FindBugs (master)

Warning Type	Number
Bad practice Warnings	33
Correctness Warnings	155
Internationalization Warnings	15
Malicious code vulnerability Warnings	12
Multithreaded correctness Warnings	9
Performance Warnings	119
Security Warnings	58
Dodgy code Warnings	125
Total	526