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

Opening relevant app on notification click #10

Open
penguin86 opened this issue Aug 7, 2021 · 6 comments
Open

Opening relevant app on notification click #10

penguin86 opened this issue Aug 7, 2021 · 6 comments
Assignees
Labels
enhancement New feature or request

Comments

@penguin86
Copy link

Hello @Andrewerr, I'm wrapping my head around implementing a feature that opens the relevant app on notification click, and I'd like to know your opinion.

The simplest way would be to use the url in the notification as intent (like when you visit a youtube.com/something url and your device opens YouTube app), but this cannot work with Nextcloud, as every instance has a different base url.

So I completed a very basic implementation that opens the relevant app using its package name (see penguin86@9214895 ).
This work for all the applications for which the app knows the package name (there is an app->package name mapping in nextcloud_package_names.xml and every new app developed needs to be added there). If the app is not installed, the notification link will be opened in browser.
This is fine to me, but I didn't issue a pull request because this isn't a complete solution: it works fine opening the corresponding app, but it opens the default activity or, if the app is running, brings on top the currently open activity. I.e., for Talk, you may click on a notification by John and see the Jack's chat, because it was the last you interacted with.

Now I'm trying to open the right activity, but this is definitely more challenging. There isn't a generic solution working for all apps.
I.e., Talk receives a GCM (now FCM) push message, retrieves the notification from notification API (as we do) and generates its own activity-opening intent. We could mock the message GCM sends to Talk and call it a day, but this wouldn't work with the F-Droid flavor, as the GCM code is present only on the Gplay flavor.
A solution could be bringing into NextcloudServices the entire notification parsing and intent generation code from Talk ( https://github.com/nextcloud/talk-android/blob/master/app/src/main/java/com/nextcloud/talk/jobs/NotificationWorker.java etc...), but I think this is bad for a couple of reasons:

  • it isn't generic: it works for Talk only and should be done ad hoc for any app we want to support
  • it relies on an "internal" intent, that can be changed from Talk developers at any time
    Any ideas on how this could be implemented cleaner?

Anyway, what do you think? Should we implement a per-app solution or is it sufficient to genericly open the right app, at least for now?

@0xf104a
Copy link
Owner

0xf104a commented Aug 9, 2021

Hello.
So first of all approach with opening url is very app dependant. In fact it may work for some apps and fail to another, because it requires app developers to implement handling by url, so they can open only urls having pattern say */index.php/apps/news/*, but from their side it may lead to url collision(so app will try to open urls which are not nextcloud instances), so we should not expect that such a method of opening an activity would be available.
As for approach with opening app by the name: it very good approach, but has some disadvantages: first of all as you have already pointed out: is is very hard to open right activity, but also there is another problem: if there is several apps providing handling for same types of notifications which one we should open?(e.g. there are few nextcloud news readers and so we will need to register them all and also there maybe situation when user for some reason have two of nextcloud news readers).
And FCM has a different way of delivering notifications rather then this app: at first app itself subscribes to a FCM(via Google Play services), then when messag is delivered Play services call(probably using Parcel API) an app which parses it(in fact message can be anything: for example Telegram used FCM to deliver proxy lists ) and calls procdure of handling.
So as we see there is no way to deliver notifcation in generic way(except UnfifiedPush which was mentioned in issue #11, but using it again requires an implementation by Nextcloud app developers).
Thus lets take Talk app as an example. I have read their Manifest and found an activity which probably may handle a particular chat:

<intent-filter>
    <action android:name="android.intent.action.VIEW" />
    <category android:name="android.intent.category.DEFAULT" />
    <data android:mimeType="vnd.android.cursor.item/vnd.com.nextcloud.talk2.chat" />
</intent-filter>

As it can be seen from this snippet Nextcloud Talk expects intent with specific mime-type which would porbably(have not checked it yet) will open right app.
So in my opinion the best solution is as follows:

  • We have a generic class OnClickProccessor which has:
    • Priority constant from 0 to INT_MAX
    • Boolean function apply(by default virtual,updates a notification to do something on click if notifcation meets some criterion, e.g. notification is from Nextcloud talk)
    • Method onNotificationClicked which is called when notification is clicked and does opening URL/app/etc.
    • Internal variables
  • When we want to support some app we implement an inherited class from OnClickProcessor which overrides apply and onNotificationClicked. In apply method we parse data received from Nextcloud and check if we should update notification(e.g. app is installed). If everything well we replace notification onClick method(don't remember API precisely) with our overrided onNotificationClicked. Of course in apply method we also update our internal varibales(e.g. chat id, activity to call, etc.). If we "applied" our processor succesfully then we return true otherwise we return false.
  • By default we have to processors
    • Processor which open url of notification and having priority 2
    • Processor which open app if it is installed and have priority 1
    • All other processors should have priority 0
  • When a notification is received we create priority queue with all processors and iterate over it applyin each to our notification. Also we store last processor which returned true(we should not delete it until it is called), other processors can be safely freed from memory.

Such an implementation would be generic enough and so we can definetely implement processors for opening url and opening app now and then in future implement app-specific processors. Also I am not sure whether we really need those priorities, but it definetely safer having them :)

@0xf104a 0xf104a added the enhancement New feature or request label Aug 9, 2021
@0xf104a 0xf104a self-assigned this Aug 15, 2021
0xf104a pushed a commit that referenced this issue Oct 10, 2021
@Gene-W
Copy link

Gene-W commented Jan 28, 2022

Hello, I've been trying several different settings to get notifications from Nextcloud Talk and then saw this conversation. I'm not very knowledgable in this area, so is this saying that notifications from Talk can't open in the Talk app just yet?

@0xf104a
Copy link
Owner

0xf104a commented Jan 30, 2022

Hello @Gene-W. Yes opening app relevant to notification(e.g. Talk on spreed notifications) is not yet implemented. Currently app only opens web version of relevant app. In future I am going to implement this feature

@Gene-W
Copy link

Gene-W commented Jan 30, 2022

Hello @Gene-W. Yes opening app relevant to notification(e.g. Talk on spreed notifications) is not yet implemented. Currently app only opens web version of relevant app. In future I am going to implement this feature

Excellent, thank you for clarifying. And thank you for your work on this project!

@ahcheing
Copy link

ahcheing commented Jun 6, 2024

I hate to be that annoying user who bothers foss devs with pings of "any progress?" but since deck app dev seems reluctant to implement built-in notifications for his app and since I dont have the skills to understand what this means I'm not sure what else to do other then ask if there has been any updates to this feature.

This is a pretty important feature for the deck app. Right now If I get a notification from deck that I need to reach out to alice, I cant do anything with it, because I cant click on the notification to open the deck app and see that its a reminder to talk with her about bob.

@0xf104a
Copy link
Owner

0xf104a commented Jun 7, 2024

@ahcheing Hello. Unfortunately I can not implement this solely by myself. It is possible for me to tell for example Deck that something happend(e.g. notification), though from other side @stefan-niedermann would need to write a code handling the notifications from my app

@0xf104a 0xf104a mentioned this issue Jun 7, 2024
3 tasks
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

No branches or pull requests

4 participants