Fix unified push unregister #3877
Conversation
|
📱 Scan the QR code below to install the build (arm64 only) for this PR. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3877 +/- ##
===========================================
+ Coverage 82.87% 82.90% +0.02%
===========================================
Files 1784 1784
Lines 45090 45095 +5
Branches 5324 5326 +2
===========================================
+ Hits 37369 37386 +17
+ Misses 5857 5843 -14
- Partials 1864 1866 +2 ☔ View full report in Codecov by Sentry. |
| override fun cleanup(clientSecret: String) { | ||
| unifiedPushStore.storeUpEndpoint(clientSecret, null) | ||
| unifiedPushStore.storePushGateway(clientSecret, null) | ||
| UnifiedPush.unregisterApp(context, clientSecret) |
There was a problem hiding this comment.
Fix 1: we need to provide clientSecret here to correctly un-register the app.
| val currentPushProviderName = userPushStore.getPushProviderName() | ||
| val currentPushProvider = pushProviders.find { it.name == currentPushProviderName } | ||
| // Cleanup the current push provider. They may need the client secret, so delete the secret after. | ||
| currentPushProvider?.onSessionDeleted(sessionId) |
There was a problem hiding this comment.
Fix 2: let the push provider do any necessary clean up when a session is removed.
Only after cleanup the stores related to the removed session.
|
bmarty
added
the
Run-Maestro
github-actions
bot
removed
the
Run-Maestro
Content
There was 2 user stories where the UnifiedPush subject was not properly removed:
The video below shows that after those 2 use cases, the ntfy app shows an empty list of topics. This was not the case before.
Motivation and context
Cleanup data and avoid ghost subscribed topic.
Screenshots / GIFs
UnifiedPushUnregister.mp4
Tests
Tested devices
Checklist