Skip to content

Conversation

@bmarty
Copy link
Member

@bmarty bmarty commented Nov 13, 2025

Content

Ensure pusher (and topic in case of usage of UnifiedPush) are unregistered if the user disables the notifications. This will ensure that no Push will be sent (and ignored) if the user disables the notifications.

If the user has configured UnifiedPush, let EX be robust against manual topic deletion by creating a new endpoint and registering it.

Motivation and context

Robust notifications

Screenshots / GIFs

Screen_recording_20251113_175739.mp4

Tests

  • From the notification settings screen, disable the notification for the session
  • The pusher should be unregistered. If using ntfy, the topic should vanish from the list
  • enable back the notification
  • the pusher should be registered again and a new topic should appear.

From ntfy application

  • delete the topic
  • EX should detect that and register a new pusher with a new topic.
  • A new topic is visible to the list
  • In case of error, a notification will be displayed to alert the user that notification will not work anymore.

Tested devices

  • Physical
  • Emulator
  • OS version(s):

Checklist

  • Changes have been tested on an Android device or Android emulator with API 24
  • 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 title will be used in the release note, it clearly define what will change for the user
  • Pull request includes screenshots or videos if containing UI changes
  • You've made a self review of your PR

…)` because it may scale the Bitmap on versions before API 27. Starting in API 27, the framework does this automatically.
Note that I did not manage to have the method `onRegistrationFailed` invoked. If the network is not available for instance, unregistering the previous pusher will fail first.
@bmarty bmarty added the PR-Bugfix For bug fix label Nov 13, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Nov 13, 2025

📱 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/hLuBRW

@codecov
Copy link

codecov bot commented Nov 13, 2025

Codecov Report

❌ Patch coverage is 91.30435% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.67%. Comparing base (8638c06) to head (f1e12c1).

Files with missing lines Patch % Lines
...mpl/notifications/factories/NotificationCreator.kt 83.33% 3 Missing and 2 partials ⚠️
...s/push/impl/notifications/NotificationDisplayer.kt 0.00% 4 Missing ⚠️
.../android/libraries/push/impl/DefaultPushService.kt 91.89% 2 Missing and 1 partial ⚠️
...mpl/notifications/NotificationSettingsPresenter.kt 80.00% 0 Missing and 1 partial ⚠️
...viders/unifiedpush/UnregisterUnifiedPushUseCase.kt 80.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #5726      +/-   ##
===========================================
+ Coverage    79.65%   79.67%   +0.02%     
===========================================
  Files         2433     2435       +2     
  Lines        65282    65380      +98     
  Branches      8329     8344      +15     
===========================================
+ Hits         52000    52094      +94     
- Misses       10302    10307       +5     
+ Partials      2980     2979       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@bmarty bmarty changed the title Notification cleanup - WIP Notification robustness Nov 13, 2025
@bmarty bmarty marked this pull request as ready for review November 13, 2025 17:00
@bmarty bmarty requested a review from a team as a code owner November 13, 2025 17:00
@bmarty bmarty requested review from jmartinesp and removed request for a team November 13, 2025 17:00
currentSlidingSyncVersion == SlidingSyncVersion.Proxy
}

private suspend fun ensurePusherIsRegistered(pusherRegistrationState: MutableState<AsyncData<Unit>>) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code moved to DefaultPushService so that can be reused by the notification settings screen

@sonarqubecloud
Copy link

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

Labels

PR-Bugfix For bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants