Handle silent delete/delete-all push notifications #3969

tobiasKaminsky · 2019-05-07T10:16:19Z

TODO

test silent push notification on older version: how is this handled?

If a notification is now dismissed, the server sends a silent push notification to all devices and removes the notification in status bar.

This will be in NC17 on server side, but can already merged as this then just does nothing…
Ref: nextcloud/server#15040, nextcloud/notifications#318

Signed-off-by: tobiasKaminsky [email protected]

nickvergessen · 2019-05-07T11:03:55Z

src/main/java/com/owncloud/android/jobs/NotificationJob.java

+                        } else if (decryptedPushMessage.deleteAll) {
+                            notificationManager.cancelAll();
+                        } else if (!APP_SPREED.equals(decryptedPushMessage.getApp())) {
+                            // We ignore Spreed messages for now


Please don't!

The server app is cleverly enough. You only get them when there is no Talk app installed for the user. In this case a "linking to the web" notification in the files app is better than nothing.

Hm. This is code done by Mario.
I did not touched it.
I'll test what happens when I remove it.

tobiasKaminsky · 2019-05-07T11:56:18Z

This is what I do get if I do not have Talk app installed.

nickvergessen · 2019-05-07T12:20:23Z

Jep, thats the expected result

codecov · 2019-05-09T13:05:01Z

Codecov Report

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

@@             Coverage Diff             @@
##             master   #3969      +/-   ##
===========================================
- Coverage      6.58%   6.57%   -0.01%     
  Complexity        1       1              
===========================================
  Files           322     322              
  Lines         30908   30928      +20     
  Branches       4408    4410       +2     
===========================================
- Hits           2036    2035       -1     
- Misses        28576   28597      +21     
  Partials        296     296

Impacted Files	Coverage Δ	Complexity Δ
...ncloud/android/datamodel/DecryptedPushMessage.java	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
...ava/com/owncloud/android/jobs/NotificationJob.java	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
...ncloud/android/authentication/PassCodeManager.java	`13.72% <0%> (-2.19%)`	`0% <0%> (ø)`
.../third_parties/daveKoeller/AlphanumComparator.java	`80.95% <0%> (-1.2%)`	`0% <0%> (ø)`
.../android/authentication/AuthenticatorActivity.java	`1.88% <0%> (-0.01%)`	`0% <0%> (ø)`
...owncloud/android/ui/activity/PassCodeActivity.java	`0% <0%> (ø)`	`0% <0%> (ø)`	⬇️
...droid/ui/fragment/OCFileListBottomSheetDialog.java	`0% <0%> (ø)`	`0% <0%> (ø)`	⬇️
...cloud/android/ui/activity/FileDisplayActivity.java	`0% <0%> (ø)`	`0% <0%> (ø)`	⬇️
...ndroid/ui/activity/RequestCredentialsActivity.java	`0% <0%> (ø)`	`0% <0%> (ø)`	⬇️
...m/owncloud/android/ui/activity/DrawerActivity.java	`0% <0%> (ø)`	`0% <0%> (ø)`	⬇️
... and 2 more

codecov · 2019-05-09T13:05:01Z

Codecov Report

Merging #3969 into master will decrease coverage by 0.05%.
The diff coverage is 0%.

@@             Coverage Diff             @@
##             master   #3969      +/-   ##
===========================================
- Coverage     14.35%   14.3%   -0.06%     
  Complexity        1       1              
===========================================
  Files           329     329              
  Lines         31017   31023       +6     
  Branches       4427    4428       +1     
===========================================
- Hits           4453    4438      -15     
- Misses        25777   25801      +24     
+ Partials        787     784       -3

Impacted Files	Coverage Δ	Complexity Δ
...ncloud/android/datamodel/DecryptedPushMessage.java	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
...ava/com/owncloud/android/jobs/NotificationJob.java	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
...owncloud/android/ui/adapter/DiskLruImageCache.java	`31.16% <0%> (-7.8%)`	`0% <0%> (ø)`
.../owncloud/android/ui/activity/ToolbarActivity.java	`56.52% <0%> (-2.9%)`	`0% <0%> (ø)`
.../third_parties/daveKoeller/AlphanumComparator.java	`80.95% <0%> (-1.2%)`	`0% <0%> (ø)`
...loud/android/datamodel/ThumbnailsCacheManager.java	`33.39% <0%> (-0.93%)`	`0% <0%> (ø)`
...cloud/android/ui/activity/FileDisplayActivity.java	`19.29% <0%> (-0.09%)`	`0% <0%> (ø)`
...owncloud/android/ui/adapter/OCFileListAdapter.java	`28.27% <0%> (ø)`	`0% <0%> (ø)`	⬇️

AndyScherzinger · 2019-05-09T20:50:54Z

Rebase might help with spot bugs?

AndyScherzinger · 2019-05-10T21:18:17Z

Nope,increased scoring...

tobiasKaminsky · 2019-05-13T13:32:39Z


Method com.owncloud.android.datamodel.DecryptedPushMessage$$Parcelable.write(DecryptedPushMessage, Parcel, int, IdentityCollection) excessively uses methods of another class
--
  | Bug type CE_CLASS_ENVY (click for details) In class com.owncloud.android.datamodel.DecryptedPushMessage$$ParcelableIn method com.owncloud.android.datamodel.DecryptedPushMessage$$Parcelable.write(DecryptedPushMessage, Parcel, int, IdentityCollection)At DecryptedPushMessage$$Parcelable.java:[lines 48-61]Value com.owncloud.android.datamodel.DecryptedPushMessage

This is the reason. But this is because Parceler, so we cannot do much here.
Except of fiddling around with spotbugs to hide these false alarms.

nextcloud-android-bot · 2019-05-13T14:49:51Z

IT test failed: https://www.kaminsky.me/nc-dev/android-integrationTests/9153

AndyScherzinger · 2019-05-13T17:12:44Z

This is the reason. But this is because Parceler, so we cannot do much here.
Except of fiddling around with spotbugs to hide these false alarms.

@tobiasKaminsky filter didn't seem to work. So raise the score? (I guess)

AndyScherzinger · 2019-05-23T07:41:52Z

@tobiasKaminsky spotbugs filter change doesn't seem to work...

tobiasKaminsky · 2019-05-23T07:44:05Z

@tobiasKaminsky spotbugs filter change doesn't seem to work...

Yes, I am on it, but somehow all regex I am trying just do not work… :/

AndyScherzinger · 2019-05-23T07:53:40Z

not a regex pro but shouldn't ~.*?Parcelable.* work?

do not ignore talk messages (will only be sent if no talk app is installed) exclude generated Parcelable Signed-off-by: tobiasKaminsky <[email protected]>

tobiasKaminsky · 2019-05-23T08:43:23Z

Finally I hope I got it.
But I do not get why "~" is needed:

Escaping $ is ok
.* is also needed, as this just means any or zero character
but ~ is included in "."…

… [skip ci] Signed-off-by: nextcloud-android-bot <[email protected]>

nextcloud-android-bot · 2019-05-23T11:32:50Z

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/9405.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.

nextcloud-android-bot · 2019-05-23T11:38:21Z

Codacy

297

Lint

Type	master	PR
Warnings	58	58
Errors	0	0

SpotBugs (new)

Warning Type	Number
Bad practice Warnings	25
Correctness Warnings	68
Internationalization Warnings	12
Malicious code vulnerability Warnings	4
Multithreaded correctness Warnings	9
Performance Warnings	122
Security Warnings	47
Dodgy code Warnings	137
Total	424

SpotBugs (master)

Warning Type	Number
Bad practice Warnings	25
Correctness Warnings	74
Internationalization Warnings	12
Malicious code vulnerability Warnings	4
Multithreaded correctness Warnings	9
Performance Warnings	122
Security Warnings	47
Dodgy code Warnings	141
Total	434

3b31841 Merge pull request #3969 from nextcloud/receivePushDelete 6074c93 Drone: update FindBugs results to reflect reduced error/warning count [skip ci] f28007e OCFileListFragment: Only scroll to top if directory changed (#4058) b061e4d Merge commit '51ee4d88fd0555a945a24b1ab3beba2ffdc79c06' 51ee4d8 handle silent delete/delete-all push notifications do not ignore talk messages (will only be sent if no talk app is installed) exclude generated Parcelable fb6b11f Use product name on rich document, if available (#3971) 4197d3b use proper qa build lib dependency 8c06097 Only compare directories if necessary 403a3e0 Use product name on rich document, if available use placeholder for string reset to master-snapshot fix analysis warning use new translatable string use var vor qa flavor too 9b0a789 Merge pull request #4040 from nextcloud/ezaquarii/remove-findbugs d6b4dee Remove legacy FindBugs tasks from build script 53c9c05 daily dev 20190523

tobiasKaminsky added the 3. to review label May 7, 2019

tobiasKaminsky mentioned this pull request May 7, 2019

Send a push message when a notification was deleted nextcloud/notifications#318

Merged

nickvergessen reviewed May 7, 2019

View reviewed changes

tobiasKaminsky force-pushed the receivePushDelete branch from 2022cf9 to d6a73ef Compare May 7, 2019 12:26

nextcloud deleted a comment May 7, 2019

AndyScherzinger approved these changes May 8, 2019

View reviewed changes

AndyScherzinger approved these changes May 9, 2019

View reviewed changes

tobiasKaminsky force-pushed the receivePushDelete branch from ad2ad09 to 3078213 Compare May 10, 2019 11:04

nextcloud deleted a comment May 13, 2019

tobiasKaminsky force-pushed the receivePushDelete branch from 7e9dd59 to dc799c2 Compare May 22, 2019 11:17

nextcloud deleted a comment May 22, 2019

tobiasKaminsky mentioned this pull request May 23, 2019

Sync (Push) notifications nextcloud/desktop#8697

Closed

12 tasks

tobiasKaminsky force-pushed the receivePushDelete branch from dc799c2 to 5ee6693 Compare May 23, 2019 06:57

handle silent delete/delete-all push notifications

51ee4d8

do not ignore talk messages (will only be sent if no talk app is installed) exclude generated Parcelable Signed-off-by: tobiasKaminsky <[email protected]>

tobiasKaminsky force-pushed the receivePushDelete branch from 5ee6693 to 51ee4d8 Compare May 23, 2019 08:42

Merge commit '51ee4d88fd0555a945a24b1ab3beba2ffdc79c06'

b061e4d

nextcloud deleted a comment May 23, 2019

Drone: update FindBugs results to reflect reduced error/warning count…

6074c93

… [skip ci] Signed-off-by: nextcloud-android-bot <[email protected]>

nextcloud deleted a comment May 23, 2019

AndyScherzinger added this to the Nextcloud App 3.7.0 milestone May 23, 2019

AndyScherzinger merged commit 3b31841 into master May 23, 2019

AndyScherzinger deleted the receivePushDelete branch May 23, 2019 12:37

Uh oh!

Handle silent delete/delete-all push notifications #3969

Handle silent delete/delete-all push notifications #3969

Uh oh!

Conversation

tobiasKaminsky commented May 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nickvergessen May 7, 2019

Choose a reason for hiding this comment

Uh oh!

tobiasKaminsky May 7, 2019

Choose a reason for hiding this comment

Uh oh!

tobiasKaminsky commented May 7, 2019

Uh oh!

nickvergessen commented May 7, 2019

Uh oh!

codecov bot commented May 9, 2019

Codecov Report

Uh oh!

codecov bot commented May 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

AndyScherzinger commented May 9, 2019

Uh oh!

AndyScherzinger commented May 10, 2019

Uh oh!

tobiasKaminsky commented May 13, 2019

Uh oh!

nextcloud-android-bot commented May 13, 2019

Uh oh!

AndyScherzinger commented May 13, 2019

Uh oh!

AndyScherzinger commented May 23, 2019

Uh oh!

tobiasKaminsky commented May 23, 2019

Uh oh!

AndyScherzinger commented May 23, 2019

Uh oh!

tobiasKaminsky commented May 23, 2019

Uh oh!

nextcloud-android-bot commented May 23, 2019

Uh oh!

nextcloud-android-bot commented May 23, 2019

Codacy

Lint

SpotBugs (new)

SpotBugs (master)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

tobiasKaminsky commented May 7, 2019 •

edited

Loading

codecov bot commented May 9, 2019 •

edited

Loading