element-x-android icon indicating copy to clipboard operation
element-x-android copied to clipboard

Push provider switch

Open bmarty opened this issue 1 year ago • 4 comments

Type of change

  • [x] Feature
  • [ ] Bugfix
  • [ ] Technical
  • [ ] Other :

Content

Add an advanced setting entry to switch between available push providers: Firebase, and available UnifiedPush distributors. Also properly unregister existing pushers and update the codebase for everything to work as expected.

Please note that matrix.org user may have issue getting push from UnifiedPush since there is some rate limiting. This will be handled separately.

Motivation and context

Screenshots / GIFs

PushProviderSelector2

Tests

  • Install the application
  • Install the application ntfy from https://f-droid.org/fr/packages/io.heckel.ntfy/ for instance
  • Use the application and ensure that push is received
  • Go to settings / advanced settings and switch the push provider to ntfy.
  • The push should still be received (with the limitation on matrix.org)
  • You can also check on the notification troubleshoot screen that Push are received (even for matrix.org users)
  • It's also possible to check the registered pushers by using Element Android on the same account: go to setting / advanced settings / Notification targets to see the list of active pushers.

Tested devices

  • [ ] Physical
  • [x] Emulator
  • OS version(s):

Checklist

  • [ ] Changes have been tested on an Android device or Android emulator with API 23
  • [ ] UI change has been tested on both light and dark themes
  • [ ] Accessibility has been taken into account. See https://github.com/element-hq/element-x-android/blob/develop/CONTRIBUTING.md#accessibility
  • [ ] Pull request is based on the develop branch
  • [ ] Pull request includes a new file under ./changelog.d. See https://github.com/element-hq/element-x-android/blob/develop/CONTRIBUTING.md#changelog
  • [ ] Pull request includes screenshots or videos if containing UI changes
  • [ ] Pull request includes a sign off
  • [ ] You've made a self review of your PR

bmarty avatar May 17 '24 16:05 bmarty

Warnings
:warning:

gradle/libs.versions.toml#L6 - A newer version of com.android.tools.build:gradle than 8.4.0 is available: 8.4.1

:warning:

gradle/libs.versions.toml#L12 - A newer version of androidx.core:core-ktx than 1.13.0 is available: 1.13.1

:warning:

gradle/libs.versions.toml#L18 - A newer version of androidx.datastore:datastore-preferences than 1.0.0 is available: 1.1.1

:warning:

gradle/libs.versions.toml#L21 - A newer version of androidx.lifecycle:lifecycle-runtime-ktx than 2.7.0 is available: 2.8.0

:warning:

gradle/libs.versions.toml#L22 - A newer version of androidx.activity:activity-compose than 1.8.2 is available: 1.9.0

Messages
:book:

gradle/libs.versions.toml#L136 - There are multiple dependencies junit:junit but with different version

:book:

gradle/libs.versions.toml#L138 - There are multiple dependencies androidx.test.ext:junit but with different version

:book:

gradle/libs.versions.toml#L207 - There are multiple dependencies junit:junit but with different version

:book:

gradle/libs.versions.toml#L208 - There are multiple dependencies androidx.test.ext:junit but with different version

Generated by :no_entry_sign: dangerJS against 4ab0202f8a94c63f65774bfde6d79e83f63f8274

ElementBot avatar May 17 '24 16:05 ElementBot

:iphone: Scan the QR code below to install the build (arm64 only) for this PR. QR code If you can't scan the QR code you can install the build via this link: https://i.diawi.com/ZpaH9f

github-actions[bot] avatar May 17 '24 16:05 github-actions[bot]

Is there a specific reason to add the toggle to the "Advanced Settings" menu and not the "Notifications" menu? The latter would make more sense to me

frebib avatar May 20 '24 11:05 frebib

Is there a specific reason to add the toggle to the "Advanced Settings" menu and not the "Notifications" menu? The latter would make more sense to me

We discussed internally about it, and concluded that this feature is quite an advanced feature and we do not want to confuse users when they want to change their notification settings.

bmarty avatar May 21 '24 07:05 bmarty

Codecov Report

Attention: Patch coverage is 29.26829% with 145 lines in your changes are missing coverage. Please review.

Project coverage is 74.24%. Comparing base (15e7a9d) to head (4ab0202). Report is 5 commits behind head on develop.

Files Patch % Lines
.../unifiedpush/VectorUnifiedPushMessagingReceiver.kt 0.00% 20 Missing :warning:
...ies/pushproviders/firebase/FirebasePushProvider.kt 0.00% 16 Missing :warning:
...ment/android/libraries/push/impl/PushersManager.kt 0.00% 15 Missing :warning:
...roviders/unifiedpush/RegisterUnifiedPushUseCase.kt 0.00% 14 Missing :warning:
.../pushproviders/firebase/FirebaseNewTokenHandler.kt 0.00% 13 Missing :warning:
.../android/libraries/push/impl/DefaultPushService.kt 0.00% 11 Missing :warning:
...viders/unifiedpush/UnifiedPushNewGatewayHandler.kt 0.00% 10 Missing :warning:
...viders/unifiedpush/UnregisterUnifiedPushUseCase.kt 0.00% 10 Missing :warning:
.../preferences/impl/advanced/AdvancedSettingsView.kt 65.21% 0 Missing and 8 partials :warning:
...edpush/registration/EndpointRegistrationHandler.kt 0.00% 7 Missing :warning:
... and 9 more
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2873      +/-   ##
===========================================
- Coverage    74.34%   74.24%   -0.10%     
===========================================
  Files         1536     1538       +2     
  Lines        36731    36866     +135     
  Branches      7123     7145      +22     
===========================================
+ Hits         27306    27371      +65     
- Misses        5700     5763      +63     
- Partials      3725     3732       +7     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar May 21 '24 08:05 codecov[bot]

@frebib FTR we have changed our mind and will move the setting to the Notifications settings screen. Tracked by #2912.

The switch will be displayed only if several providers are detected, to not confuse regular users about this.

bmarty avatar May 27 '24 08:05 bmarty