Skip to content

📲 Unifiedpush#2213

Closed
koo5 wants to merge 4 commits into
nextcloud:masterfrom
koo5:unifiedpush
Closed

📲 Unifiedpush#2213
koo5 wants to merge 4 commits into
nextcloud:masterfrom
koo5:unifiedpush

Conversation

@koo5

@koo5 koo5 commented Jul 13, 2022

Copy link
Copy Markdown

This PR adds a new build variant: "unifiedpush".

problems:

  • does not work with UP-NextPush yet... (Up-Example does not work with it either)
  • currently does not work when there are multiple distributors available - we think this is because UnifiedPush.registerAppWithDialog expects to be called from an activity..
  • another design issue is that unifiedpush.ChatAndCallMessagingService is largely a copy of firebase.ChatAndCallMessagingService - not sure where to create a common ancestor.
  • "ClosedInterface" was a strange name to begin with...

feedback, ideas and corrections appreciated!

koo5 added 4 commits July 13, 2022 05:04
Signed-off-by: Jindrich Kolman <kolman.jindrich@gmail.com>
Signed-off-by: Jindrich Kolman <kolman.jindrich@gmail.com>
Signed-off-by: Jindrich Kolman <kolman.jindrich@gmail.com>
Signed-off-by: Jindrich Kolman <kolman.jindrich@gmail.com>
@nextcloud-android-bot

Copy link
Copy Markdown
Collaborator

Lint

TypemasterPR
Warnings116117
Errors11

SpotBugs (new)

Warning Type Number
Bad practice Warnings 8
Correctness Warnings 35
Experimental Warnings 2
Internationalization Warnings 9
Malicious code vulnerability Warnings 15
Performance Warnings 22
Security Warnings 2
Dodgy code Warnings 61
Total 154

SpotBugs (master)

Warning Type Number
Bad practice Warnings 8
Correctness Warnings 35
Experimental Warnings 2
Internationalization Warnings 9
Malicious code vulnerability Warnings 15
Performance Warnings 22
Security Warnings 2
Dodgy code Warnings 61
Total 154

Lint increased!

@nickvergessen nickvergessen added this to the 15.0.0 milestone Aug 9, 2022
@nickvergessen nickvergessen changed the title WIP: Unifiedpush 🔔📨 Unifiedpush Aug 9, 2022
@nickvergessen nickvergessen changed the title 🔔📨 Unifiedpush 📲 Unifiedpush Aug 9, 2022
@AlvaroBrey

AlvaroBrey commented Aug 9, 2022

Copy link
Copy Markdown
Member

Hey @koo5, thank you very much for the time and effort gone into this. We would love to have this implemented in Talk, as it can help our users become more independent from Google infrastructure.

Can you write some instructions on how to test this (server + android setup)?

@AlvaroBrey

Copy link
Copy Markdown
Member
  • another design issue is that unifiedpush.ChatAndCallMessagingService is largely a copy of firebase.ChatAndCallMessagingService - not sure where to create a common ancestor.

You can create an ancestor class in the main sourceset; it doesn't do any harm as it will only be registered in the specific flavor's manifests.

@ASerbinski

Copy link
Copy Markdown
Contributor

Unified push is interesting, but I think that this is the wrong approach. There are more than one option for not-google push message delivery, another that is probably more helpful is https://gitlab.com/Nextcloud-Push which operates server side as a plugin to nextcloud itself, meaning that you install the proxy into your nextcloud instance, and then Nextcloud can push the messages directly to the client itself.

Another option would be to work somehow with Nextcloud Files HPB.

So the problem then becomes that this approach would lead to the need to publish multiple separate variants of Nextcloud Talk, whereas in my view, a more ideal solution would be to be able to support all of the different push message delivery mechanisms within the SAME variant.

Obviously a google variant has to be kept separately from an "everything else" variant in order to include on F-Droid, but in my opinion, it would be more proper for all of the additional delivery mechanisms to be available on BOTH variants.

The user should be asked which push delivery mechanism to use during initial setup.

@AndyScherzinger

AndyScherzinger commented Aug 9, 2022

Copy link
Copy Markdown
Member

Sorry for potentially widening the field of option, but linking to the discussion of the Android files app regarding supporting an alternative push solution, see also: nextcloud/android#5510 as well as nextcloud/android#8684

@mahibi mahibi removed this from the 15.0.0 milestone Sep 14, 2022
@nickvergessen

Copy link
Copy Markdown
Member

Implemented via #5883 in the meantime

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2. developing Work in progress enhancement New feature or request feature: notifications 🔔

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants