Fix unregistering pusher local error #2901
Conversation
|
|
📱 Scan the QR code below to install the build (arm64 only) for this PR. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## feature/bma/unitTestPush #2901 +/- ##
===========================================================
Coverage ? 75.26%
===========================================================
Files ? 1550
Lines ? 36927
Branches ? 7149
===========================================================
Hits ? 27794
Misses ? 5398
Partials ? 3735 ☔ View full report in Codecov by Sentry. |
| val userPushStore = userPushStoreFactory.getOrCreate(matrixClient.sessionId) | ||
| val currentPushProviderName = userPushStore.getPushProviderName() | ||
| val currentPushProvider = pushProviders.find { it.name == currentPushProviderName } | ||
| val currentDistributorValue = currentPushProvider?.getCurrentDistributor(matrixClient)?.value | ||
| if (currentPushProviderName != pushProvider.name || currentDistributorValue != distributor.value) { | ||
| // Unregister previous one if any | ||
| currentPushProvider?.unregister(matrixClient) | ||
| currentPushProvider | ||
| ?.also { Timber.d("Unregistering previous push provider $currentPushProviderName/$currentDistributorValue") } |
There was a problem hiding this comment.
Nit: this could be logged before this call and not need an .also {}.
There was a problem hiding this comment.
Yeah, not really since here currentPushProvider can still be null, so here ?. for the also is useful.
I agree that the code is a bit hard to follow, I tried to add some if(x (!= null) statement but it's getting worse.
Type of change
Content
Fixes #2895 by ignoring local error when unregistering a pusher (Firebase and UnifiedPush cases)
Base on the
unitTestPushbranch to be able to update the test, but the other PR will be merged first.Motivation and context
Fixes #2895
Screenshots / GIFs
NA
Tests
Maybe @Xiretza may want to test the patch.
Tested devices
Checklist