Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle push notifications #208

Closed
nickvergessen opened this issue Nov 9, 2019 · 17 comments · Fixed by #364
Closed

Handle push notifications #208

nickvergessen opened this issue Nov 9, 2019 · 17 comments · Fixed by #364
Labels
enhancement New feature or request

Comments

@nickvergessen
Copy link

nickvergessen commented Nov 9, 2019

Hi there,

this is just a general issue to discuss whether this all makes sense.

For the talk mobile apps we added the following behaviour:

  1. The register for push notifications just like the normal file apps
  2. When a deck app is registered for notifications for this user, the push notifications for deck only go to the deck app. At the same time the deck app gets no other notifications.

This would allow users that only have the deck app installed, to be notified about card deadlines, etc.

I could look into the necessary changes in the notifications app. From the mobile apps it would require the following things:

  1. The user agent has to match a pattern which we ship in the notifications app/push server, so it should not change afterwards anymore, suggestion: /^Mozilla\/5\.0 \(Android\) Nextcloud\-Deck v.*$/
  2. Implement the API ( https://github.com/nextcloud/notifications/blob/master/docs/push-v2.md ) - You might to look into the talk and/or files app for example code
  3. The related developer certificate needs to be passed to "us".

cc @juliushaertl

@nickvergessen nickvergessen added the 🐞 bug Something isn't working label Nov 9, 2019
@stefan-niedermann
Copy link
Owner

This would allow users that only have the deck app installed

The deck app requires the files app to be installed because it only allows login via Single-Sign-On.

@stefan-niedermann
Copy link
Owner

A solution i thought about was: The deck app doesn't handle notifications at all, but registers a URI handler. The files app could check whether or not a handler is registered and redirects the notification-click-action to the deck app....?

@violoncelloCH
Copy link

does the deck app even really need push notifications? I mean afaik it's just reminders which are time based... a calender app doesn't need push either for it's notifications generated on data which is synchronized and stored locally...

@desperateCoder
Copy link
Collaborator

@violoncelloCH has a good point there... What else could be a use case for push notifications?

@putt1ck
Copy link

putt1ck commented Nov 10, 2019

Comments. It's the single most important aspect of a connected Kanban system. Comments provide clarity and iteration to the original card; when someone posts a comment it is often either a question seeking better understanding or an answer providing it. If you're using Deck as a collaborative project management system, then seeing that a comment has been made is quite important.

@juliushaertl
Copy link
Collaborator

Use cases from my POV:

  • Mentioned in a comment
  • Card assigned
  • Card reached due date

@stefan-niedermann stefan-niedermann added enhancement New feature or request and removed 🐞 bug Something isn't working labels Feb 26, 2020
@stefan-niedermann
Copy link
Owner

@tobiasKaminsky can you please have a look at this comment and tell me whether this is technically possible and if this is something you would consider to implement at your end? (of course i would be happy to help you at a shared PR)

@tobiasKaminsky
Copy link
Collaborator

Problem is that each 3rd-party app (via SSO) needs an independent app token and currently we cannot create a new app-token from an app-token.
As far as I know @rullzer managed to do this.
I will talk with him, if and when this will be on server, and then I can start implementing it in SSO and thus any 3rd-party can get an app-token and thus register for push itself.

Would this be good?

(I do not want to have any logic in Files, beside SSO redirecting everything)

@stefan-niedermann
Copy link
Owner

My idea has nothing to do with SSO - It's just about Android stuff. The files app already handles push notifications very well and displays an Android notification when something happens. Clicking on this android notification currently opens the web interface of Nextcloud.

My idea is that the files app checks whether or not there is a handler for a special kind of notification, and then starts the Deck android app (if installed) and falls back to opening the webinterface (if not installed), like it is today.

The pro would be that no 3rd-party app would have to deal with Android nor Nextcloud push stuff (it's a pain in the ass and i am happy you already got it working so good).

@tobiasKaminsky
Copy link
Collaborator

Might be working.
But push does not have many infos in it. Mainly it is is the id of the notification.

@stefan-niedermann
Copy link
Owner

stefan-niedermann commented Apr 4, 2020

Yep, that's where @desperateCoder comes into the game.

The files app receives a notification with those fields: https://github.com/nextcloud/android/blob/01b1e719ffdf0546acf54d2e450aca81edee59be/src/main/java/com/owncloud/android/datamodel/DecryptedPushMessage.java#L32-L39

If the field app says, that the notification comes from the Deck server app, the files app will start an Intent and call the Deck android app and pushes all the other fields + the affected account to it.

At this point we are in the Deck android app and we will need the nid-field to fetch more information about the actual stuff that's going on. Since we recently received a push notification, we are online and can make a request to OCS.

The documentation of the push notification data says about the nid-field:

Numeric identifier of the notification in order to get more information via the OCS API

So, at this point we will call the correspondent OCS-endpoint and hope, that we can get the needed data (boardId, listId, cardId) to open the matching card in the Deck android app. I think in the worst case we will need to parse it from link in the action-object, but let's test what exact information we are able to fetch this way (or @juliushaertl can tell us directly).

Deck push notifications

Edit Thanks to my gf we now have the architectural graphic in *nice* 😉

@desperateCoder
Copy link
Collaborator

Yep, no problem at all. Sounds good, as long as the ocs service is implemented and allows me to get the data. The important point is: I need to know for which account, since we could have multiple accounts with conflicting remote-IDs of boards, stacks and cards.

@stefan-niedermann
Copy link
Owner

Yep, this is technically no problem, i already told @tobiasKaminsky that he needs to pass the account. I guess we will get the account like user@example.com because he cannot know the internal accountIds of course 😉

@stefan-niedermann stefan-niedermann pinned this issue Apr 4, 2020
@desperateCoder

This comment has been minimized.

@stefan-niedermann

This comment has been minimized.

stefan-niedermann added a commit that referenced this issue Apr 7, 2020
Added dummy activity

Signed-off-by: Stefan Niedermann <info@niedermann.it>
stefan-niedermann added a commit that referenced this issue Apr 7, 2020
Remove parentActivityName from PushNotificationActivity

Signed-off-by: Stefan Niedermann <info@niedermann.it>
stefan-niedermann added a commit that referenced this issue Apr 7, 2020
Add intent-filter

Signed-off-by: Stefan Niedermann <info@niedermann.it>
stefan-niedermann added a commit that referenced this issue Apr 7, 2020
Added dummy activity

Signed-off-by: Stefan Niedermann <info@niedermann.it>
stefan-niedermann added a commit that referenced this issue Apr 7, 2020
Remove parentActivityName from PushNotificationActivity

Signed-off-by: Stefan Niedermann <info@niedermann.it>
stefan-niedermann added a commit that referenced this issue Apr 7, 2020
Add intent-filter

Signed-off-by: Stefan Niedermann <info@niedermann.it>
stefan-niedermann added a commit that referenced this issue Apr 7, 2020
Added dummy activity

Signed-off-by: Stefan Niedermann <info@niedermann.it>
stefan-niedermann added a commit that referenced this issue Apr 7, 2020
Remove parentActivityName from PushNotificationActivity

Signed-off-by: Stefan Niedermann <info@niedermann.it>
stefan-niedermann added a commit that referenced this issue Apr 7, 2020
Add intent-filter

Signed-off-by: Stefan Niedermann <info@niedermann.it>
@stefan-niedermann stefan-niedermann linked a pull request Apr 7, 2020 that will close this issue
2 tasks
stefan-niedermann added a commit to nextcloud/android that referenced this issue Apr 16, 2020
stefan-niedermann added a commit to nextcloud/android that referenced this issue Apr 16, 2020
stefan-niedermann added a commit to nextcloud/android that referenced this issue Apr 16, 2020
stefan-niedermann added a commit to nextcloud/android that referenced this issue Apr 16, 2020
stefan-niedermann added a commit to nextcloud/android that referenced this issue Apr 17, 2020
stefan-niedermann added a commit to nextcloud/android that referenced this issue Apr 17, 2020
tobiasKaminsky pushed a commit to nextcloud/android that referenced this issue May 12, 2020
tobiasKaminsky pushed a commit to nextcloud/android that referenced this issue May 12, 2020
tobiasKaminsky pushed a commit to nextcloud/android that referenced this issue May 12, 2020
tobiasKaminsky pushed a commit to nextcloud/android that referenced this issue May 12, 2020
tobiasKaminsky pushed a commit to nextcloud/android that referenced this issue May 12, 2020
tobiasKaminsky pushed a commit to nextcloud/android that referenced this issue May 12, 2020
tobiasKaminsky pushed a commit to nextcloud/android that referenced this issue May 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants