ANR when asking to select the notification method #7675
Conversation
| import im.vector.app.test.fakes.FakePushersManager | ||
| import im.vector.app.test.fakes.FakeUnifiedPushHelper | ||
| import im.vector.app.test.fixtures.PusherFixture | ||
| import io.mockk.verify |
mnaturel
force-pushed
the
fix/mna/unified-push-selection
branch
from
e146b84 to
b30dda7
Compare
| // 'app should always check the device for a compatible Google Play services APK before accessing Google Play services features' | ||
| if (checkPlayServices(activity)) { | ||
| if (checkPlayServices(context)) { |
There was a problem hiding this comment.
I think checkPlayServices can display a dialog, and so needs an Activity. I will try running your code on an emulator without the PlayServices.
There was a problem hiding this comment.
From what I understood, we only use the checking method GoogleApiAvailability.isGooglePlayServicesAvailable(context). This method only requires a Context. In case of error, we display a toast which only requires a Context as well.
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Thanks for extracting logic from the Activity to the ViewModel. Maybe creating a new dedicated ViewModel may have been better, but it can be done later.
| @@ -23,5 +23,4 @@ import org.matrix.android.sdk.api.session.sync.SyncRequestState | |||
| data class HomeActivityViewState( | |||
| val syncRequestState: SyncRequestState = SyncRequestState.Idle, | |||
| val authenticationDescription: AuthenticationDescription? = null, | |||
| val areNotificationsSilenced: Boolean = false, | |||
There was a problem hiding this comment.
This is (or will be) handled elsewhere?
There was a problem hiding this comment.
I need to look into that, I am not sure it is needed. What is sure is that it should not have been added in the ViewState as it was not used for rendering UI but for domain logic. There will be 2 successive PRs following that current PR which will fix issues with both remote and local notifications toggling for the session.
mnaturel
force-pushed
the
fix/mna/unified-push-selection
branch
from
b30dda7 to
f8c59f6
Compare
|
SonarCloud Quality Gate failed. |
Type of change
Content
Refactoring most of the logic for registration of push notification distributor. Adding unit tests on the new code.
The previous code was a mix of UI and domain layer which lead to non necessary complexity and some bugs like ANR, crash or unexpected behavior.
Motivation and context
Closes #7653
Screenshots / GIFs
Tests
Both FDroid and Gplay version should be tested.
Tested devices
Checklist