Adding notification option to not show notifications for new media folders

[AndyScherzinger]

Resolve #3080

Signed-off-by: Andy Scherzinger [email protected]

Notification	Settings

The setting will only be visible in case the user chose to bloc media scan notifications earlier via the notification action. As soon as the user chooses to show the notifications again, the setting will vanish again.

@tobiasKaminsky for review and test ❤️

[codecov]

Codecov Report

Merging #3448 into master will decrease coverage by <.01%.
The diff coverage is 1.31%.

@@             Coverage Diff             @@
##             master   #3448      +/-   ##
===========================================
- Coverage       6.2%   6.19%   -0.01%     
  Complexity        1       1              
===========================================
  Files           314     314              
  Lines         30190   30235      +45     
  Branches       4328    4331       +3     
===========================================
  Hits           1874    1874              
- Misses        28034   28078      +44     
- Partials        282     283       +1

Impacted Files	Coverage Δ	Complexity Δ
.../com/owncloud/android/ui/activity/Preferences.java	0% <0%> (ø)	0 <0> (ø)	⬇️
...oud/android/ui/activity/SyncedFoldersActivity.java	0% <0%> (ø)	0 <0> (ø)	⬇️
...ava/com/owncloud/android/db/PreferenceManager.java	24.8% <0%> (-0.61%)	0 <0> (ø)
...wncloud/android/jobs/MediaFoldersDetectionJob.java	7.36% <1.81%> (-1.21%)	0 <0> (ø)
.../third_parties/daveKoeller/AlphanumComparator.java	80.95% <0%> (-1.2%)	0% <0%> (ø)

[codecov]

Codecov Report

Merging #3448 into master will decrease coverage by 0.01%.
The diff coverage is 1.42%.

@@             Coverage Diff             @@
##             master   #3448      +/-   ##
===========================================
- Coverage      6.19%   6.18%   -0.02%     
  Complexity        1       1              
===========================================
  Files           314     314              
  Lines         30262   30301      +39     
  Branches       4340    4343       +3     
===========================================
- Hits           1876    1875       -1     
- Misses        28104   28142      +38     
- Partials        282     284       +2

Impacted Files	Coverage Δ	Complexity Δ
.../com/owncloud/android/ui/activity/Preferences.java	0% <0%> (ø)	0 <0> (ø)	⬇️
...oud/android/ui/activity/SyncedFoldersActivity.java	0% <0%> (ø)	0 <0> (ø)	⬇️
...ava/com/owncloud/android/db/PreferenceManager.java	24.21% <0%> (-0.59%)	0 <0> (ø)
...wncloud/android/jobs/MediaFoldersDetectionJob.java	7.36% <1.81%> (-1.21%)	0 <0> (ø)
...in/java/com/owncloud/android/datamodel/OCFile.java	57.6% <0%> (-1.39%)	0% <0%> (ø)
.../third_parties/daveKoeller/AlphanumComparator.java	83.33% <0%> (+1.19%)	0% <0%> (ø)	⬇️

[tobiasKaminsky]

Code review: ❤️

Do you have an easy way for testing?
I tried to download an image do sdcard/downloads, but no notification is popping up (I re-installed the app previously to not have downloads in any list)

[AndyScherzinger]

Do you have an easy way for testing?

Yes incredibly easy ;)

Use a file manager, go to downloads and have an image in the download folder, then:

1. create a new folder "ABC" within downloads, copy the image into it and wait -> notification comes up -> click Disable
2. create a new folder "BCD" within downloads, copy the image into it and wait -> no notification coming up
3. go into the app's settings and toggle the notification hidding setting, then back in the file manager create a new folder "CDE" within downloads, copy the image into it and wait -> notification comes up

[tobiasKaminsky]

Works as expected, great stuff!

Only thing I stumbled about is, that it is a bit strange for me that the new setting has a toggle button, but if you click on it / enable it, it vanishes right away.

I would rather keep it all the time (so that you can also disable it without having a notification before)
or
change wording (and toggle button) to something like "click here to enable notification again". Then it would be fine with instantly vanishing and only showing it once notifications are disabled.

[AndyScherzinger]

@tobiasKaminsky how about not hidding it right away while still not showing it in case notifications haven't been enabled. We just want a way for the users to re-enable it again but it shouldn't be that prominent as to keep it in the settings all the time. So we should in my opinion just remove the hiding when toggled, so you can toggle it while it is there, re-opnening the settings afterwards and it will be hidden.

[AndyScherzinger]

Reason there is a very, very small group of people who want to disable it and they can, so always showing it in the settings is way to prominent ;)

[AndyScherzinger]

@tobiasKaminsky changed the behavior as described in my latest comments and also fixed the code review issue you found, so afaik should be fine now and also has the behavior I would prefer

[tobiasKaminsky]

👍
Fine with the change, great work 🎉

[tobiasKaminsky]

Two new FindBug warnings, one in "BadPractise" and one in "Security".
Can you have a look at them?
(at some point we should make this failing, like with lint)

[AndyScherzinger]

Can you have a look at them?

I'll have a look 👍

[AndyScherzinger]

at some point we should make this failing, like with lint

I created an issue for it: #3465

[nextcloud-android-bot]

Lint

Type	master	PR
Warnings	72	72
Errors	0	0

FindBugs (new)

Warning Type	Number
Bad practice Warnings	35
Correctness Warnings	155
Internationalization Warnings	15
Malicious code vulnerability Warnings	12
Multithreaded correctness Warnings	9
Performance Warnings	118
Security Warnings	66
Dodgy code Warnings	133
Total	543

FindBugs (master)

Warning Type	Number
Bad practice Warnings	35
Correctness Warnings	155
Internationalization Warnings	15
Malicious code vulnerability Warnings	12
Multithreaded correctness Warnings	9
Performance Warnings	118
Security Warnings	66
Dodgy code Warnings	133
Total	543

[dbielz]

Hey, how about turning off adding new folders automatically, not only the notification? Where's the logic behind that? I could understand, if new found subfolders in a synced folder would be added automatically, but syncing EVERY new found media folder, just because I once decided that I want to sync ONE media folder? WTF?
I mean, where's the use case for this behaviour?

BTW: on my android (8.0) I can disable this notification already for Nextcloud version 3.4.2 separately under Settings->Notifications->Nexcloud, so where's the point in this issue?

[dbielz]

Sorry, but this one annoys me already for quite a while, and now I found this discussion about turning off the notifications only, which apparently already is realizable in android 8.0.

[f1o1f]

has this been disabled again? I can't find it in version 3.26.0. Maybe the German translation is bad but none of the options are toggled 'on' and the notifications still pop up (multiple times, actually, but that is a different problem)

[AndyScherzinger]

The action should still be there, didn't get removed. However the action as any notification action only shows up when the notification has been expanded by the user