Send notification actions
ClosedPublic

Authored by nicolasfella on Apr 17 2018, 6:47 PM.

Details

Summary

Store PendingIntents from notification actions. Send list of notifications to desktop and trigger Intent when matching packet arrives.
CCBUG: 366475

Test Plan

Create test notification, trigger package is received correctly. Whether intent.send() is actually successful is NOT yet tested.

Diff Detail

Repository
R225 KDE Connect - Android application
Branch
arcpatch-D12294
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 9353
Build 9371: arc lint + arc unit
nicolasfella created this revision.Apr 17 2018, 6:47 PM
Restricted Application added a project: KDE Connect. · View Herald TranscriptApr 17 2018, 6:47 PM
nicolasfella requested review of this revision.Apr 17 2018, 6:47 PM

I think we should also send the action icon. That might be hard at the moment though, because we can only send one payload.

src/org/kde/kdeconnect/Plugins/NotificationsPlugin/NotificationsPlugin.java
82

Maybe it's good to refactor this into Map<String, RemotelyAccessedNotification> or similar.

RemotelyAccessedNotification can then contain the id, the available actions and reply intent.

293

I think renaming to actionsJson is clearer.

296

Maybe if a single title is missing, don't send any actions? Otherwise, this can be confusing.

302

Double Log (with line 268), I'd remove at least one of them.

521

Something like Log.e(TAG, "Firing action for notification failed", e) is better in general (colors the stacktrace etc., actually labels it as an error).

I think we should also send the action icon. That might be hard at the moment though, because we can only send one payload.

For what would you use it?

I think we should also send the action icon. That might be hard at the moment though, because we can only send one payload.

For what would you use it?

For in the plasmoid, that looks better.

MatMaul added a subscriber: MatMaul.Jun 3 2018, 5:12 PM
Restricted Application added a subscriber: kdeconnect. · View Herald TranscriptJun 3 2018, 5:12 PM
apol added a subscriber: apol.Jul 26 2018, 5:46 PM

I think we should also send the action icon. That might be hard at the moment though, because we can only send one payload.

For what would you use it?

For in the plasmoid, that looks better.

Let's focus on making it possible and we can add the bells and whistles later?

nicolasfella edited the summary of this revision. (Show Details)Sep 26 2018, 11:38 PM
sredman accepted this revision.Mar 22 2019, 4:22 AM
sredman added a subscriber: sredman.

It works for me. Finally I am able to use my university's 2FA without actually touching my phone! :D

Since the previous reviewers seemed satisfied and since this has been sitting for awhile, I say go for it

This revision is now accepted and ready to land.Mar 22 2019, 4:22 AM
nicolasfella closed this revision.Mar 22 2019, 4:07 PM