Show info messages for push

[tobiasKaminsky]

Show warnings if

• no push possible (fdroid)
• push possible, but old login

I hope my idea is correct to use the push url / push return from google server.

Do we use polling, e.g. 1x / minute for fdroid, if this is possible? If not, then I will open up a new issue for 2.1.

resolves #1519

Old login:

Fdroid (thanks to Björn for helping with wording ;-) ):

[AndyScherzinger]

Do we use polling, e.g. 1x / minute for fdroid, if this is possible? If not, then I will open up a new issue for 2.1.

afaik we don't have polling implemented yet but would recommend to have a 10-15 minutes interval to not drain the battery. As a second step we could then of course go for a "smarter" polling looking at the frequency of notifications coming in.

[AndyScherzinger]

I'd say we can shorten this to "Push notifications disabled due to dependencies on proprietary Google play services."

[AndyScherzinger]

I hope my idea is correct to use the push url / push return from google server.

Yes that should work just fine

[AndyScherzinger]

👍 nice solution. One optimization though would be to add an action like "dismiss" or "OK" to hide the snackbar again. Future improvements could then be to have an action which navigates you to the account management screen in case of the old login scenario.

[mario]

"due to old login method" means nothing for the majority of people.

Also, this shouldn't go in 2.0. We really have bugfixes to do for 2.0, and things like this can go maybe in 2.0.1.

You have to use AlarmManager for F-droid as that is the only thing that currently guarantees the device will wake up. The interval can be decided I guess, but in order to know which notifications to show we'll have to store notifications in the DB and show only ones we haven't shown etc etc.

As for other issue I see with this PR: it could happen that even with the proper login (webview) the first time you open notifications Push registration didn't finish yet. Or it failed. In this case a "wrong" error message would be shown that would confuse people.

Also "Google play" -> "Google Play"

[AndyScherzinger]

@mario would you leave out the messages for 2.0.0 as well or just the polling because yes the polling is post 2.0.0 like Tobias mentioned in the description so I'd say we would be fine adding the messages (especially the Google Play one for the f-droid users). As for your scenario is there a way to detect that the push registration is not finished so we could then show yet another variant of the message?

[mario]

@AndyScherzinger there is currently no way to distinguish between a "just failed this time" and "failed because you use old login", but I guess I/you/whoever could add support for this by adding a status column to PushConfigurationState and handling return codes from the server.

If you want the messages as a first step, sure - but please change the typo/capitalization and do the shortening.

[AndyScherzinger]

but please change the typo/capitalization and do the shortening.

sure! As for the shortening do you mean the one I proposed?

[mario]

@AndyScherzinger yup. Plus find a way to change the "old login method" wording to something else. cc @tobiasKaminsky

[tobiasKaminsky]

We decided at the conf to show only warnings as a first step for 2.0.

there is currently no way to distinguish between a "just failed this time" and "failed because you use old login", but I guess I/you/whoever could add support for this by adding a status column to PushConfigurationState and handling return codes from the server.

Is there a way to trigger "failing one time" so that I can catch the correct result type?
In https://github.com/nextcloud/android-library/blob/master/src/com/owncloud/android/lib/resources/notifications/RegisterAccountDeviceForProxyOperation.java#L87 is the status set, so we already can use it, but I would need the error codes to distinguish.

Regarding the wordings:
Fdroid: Push notification disabled due to dependencies on proprietary Google Play services.
Old login: Push notifications require auth token based login. Please consider to re-create your account.

[mario]

You actually need to check the return of the registration done with Nc server, not the proxy server.

https://github.com/nextcloud/notifications/blob/master/docs/push-v2.md

If you get a 400 when posting to POST /ocs/v2.php/apps/notifications/api/v2/push, and the Body contains INVALID_SESSION_TOKEN, then you're using a password instead of the app token and you could show this message.

Also "Please consider to recreating your account." maybe?

[tobiasKaminsky]

I added a third case:

• pushUrl is set -> push should theoretically work
• "new login" (no error on registration)
• BUT pushValue provided by google is empty/null

--> temporary failure

I hope this is now correct :-)

For discussion of polling I'll open now a new issue: #1561

[tobiasKaminsky]

Regarding Drone/Lint: the one new warning is "obsolete dependencies" with targets the library.
After testing and before merging this needs to be changed to a stable lib release, and then drone is satisfied.

Rebase done

[AndyScherzinger]

👍 please rebase 😁 cc @mario for review

[tobiasKaminsky]

Rebase done

[AndyScherzinger]

Sweet, then it can be merged after @mario approved it via 👍

[mario]

Fine for now :) 👍