From 6c45490dd19034b407d07fcab60b5aff23074bf1 Mon Sep 17 00:00:00 2001 From: Onuray Sahin Date: Mon, 21 Nov 2022 18:44:45 +0300 Subject: [PATCH] Code review fixes. --- .../src/main/res/values/config-settings.xml | 1 + .../features/settings/VectorPreferences.kt | 10 ++++++ .../settings/devices/v2/DevicesViewModel.kt | 12 ++----- .../othersessions/OtherSessionsViewModel.kt | 12 ++----- .../v2/overview/SessionOverviewViewModel.kt | 12 ++----- .../main/res/layout/item_other_session.xml | 6 ++-- .../devices/v2/DevicesViewModelTest.kt | 33 ++++++++++++++++--- .../OtherSessionsViewModelTest.kt | 8 ++--- .../overview/SessionOverviewViewModelTest.kt | 8 ++--- .../app/test/fakes/FakeSharedPreferences.kt | 5 --- .../app/test/fakes/FakeVectorPreferences.kt | 4 +++ 11 files changed, 65 insertions(+), 46 deletions(-) diff --git a/vector-config/src/main/res/values/config-settings.xml b/vector-config/src/main/res/values/config-settings.xml index 504c587b8d..ef9695a080 100755 --- a/vector-config/src/main/res/values/config-settings.xml +++ b/vector-config/src/main/res/values/config-settings.xml @@ -51,6 +51,7 @@ false true false + false diff --git a/vector/src/main/java/im/vector/app/features/settings/VectorPreferences.kt b/vector/src/main/java/im/vector/app/features/settings/VectorPreferences.kt index 3424c2b54c..30b5ef9131 100755 --- a/vector/src/main/java/im/vector/app/features/settings/VectorPreferences.kt +++ b/vector/src/main/java/im/vector/app/features/settings/VectorPreferences.kt @@ -1231,4 +1231,14 @@ class VectorPreferences @Inject constructor( return vectorFeatures.isVoiceBroadcastEnabled() && defaultPrefs.getBoolean(SETTINGS_LABS_VOICE_BROADCAST_KEY, getDefault(R.bool.settings_labs_enable_voice_broadcast_default)) } + + fun showIpAddressInDeviceManagerScreens(): Boolean { + return defaultPrefs.getBoolean(SETTINGS_SESSION_MANAGER_SHOW_IP_ADDRESS, getDefault(R.bool.settings_device_manager_show_ip_address)) + } + + fun setIpAddressVisibilityInDeviceManagerScreens(isVisible: Boolean) { + defaultPrefs.edit { + putBoolean(VectorPreferences.SETTINGS_SESSION_MANAGER_SHOW_IP_ADDRESS, isVisible) + } + } } diff --git a/vector/src/main/java/im/vector/app/features/settings/devices/v2/DevicesViewModel.kt b/vector/src/main/java/im/vector/app/features/settings/devices/v2/DevicesViewModel.kt index b0ca32016f..971fb123f0 100644 --- a/vector/src/main/java/im/vector/app/features/settings/devices/v2/DevicesViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/settings/devices/v2/DevicesViewModel.kt @@ -16,15 +16,12 @@ package im.vector.app.features.settings.devices.v2 -import android.content.SharedPreferences -import androidx.core.content.edit import com.airbnb.mvrx.MavericksViewModelFactory import com.airbnb.mvrx.Success import dagger.assisted.Assisted import dagger.assisted.AssistedFactory import dagger.assisted.AssistedInject import im.vector.app.core.di.ActiveSessionHolder -import im.vector.app.core.di.DefaultPreferences import im.vector.app.core.di.MavericksAssistedViewModelFactory import im.vector.app.core.di.hiltMavericksViewModelFactory import im.vector.app.features.auth.PendingAuthHandler @@ -53,8 +50,7 @@ class DevicesViewModel @AssistedInject constructor( private val interceptSignoutFlowResponseUseCase: InterceptSignoutFlowResponseUseCase, private val pendingAuthHandler: PendingAuthHandler, refreshDevicesUseCase: RefreshDevicesUseCase, - @DefaultPreferences - private val sharedPreferences: SharedPreferences, + private val vectorPreferences: VectorPreferences, ) : VectorSessionsListViewModel(initialState, activeSessionHolder, refreshDevicesUseCase) { @AssistedFactory @@ -73,7 +69,7 @@ class DevicesViewModel @AssistedInject constructor( } private fun refreshIpAddressVisibility() { - val shouldShowIpAddress = sharedPreferences.getBoolean(VectorPreferences.SETTINGS_SESSION_MANAGER_SHOW_IP_ADDRESS, false) + val shouldShowIpAddress = vectorPreferences.showIpAddressInDeviceManagerScreens() setState { copy(isShowingIpAddress = shouldShowIpAddress) } @@ -135,9 +131,7 @@ class DevicesViewModel @AssistedInject constructor( setState { copy(isShowingIpAddress = !isShowingIpAddress) } - sharedPreferences.edit { - putBoolean(VectorPreferences.SETTINGS_SESSION_MANAGER_SHOW_IP_ADDRESS, !isShowingIpAddress) - } + vectorPreferences.setIpAddressVisibilityInDeviceManagerScreens(!isShowingIpAddress) } private fun handleVerifyCurrentSessionAction() { diff --git a/vector/src/main/java/im/vector/app/features/settings/devices/v2/othersessions/OtherSessionsViewModel.kt b/vector/src/main/java/im/vector/app/features/settings/devices/v2/othersessions/OtherSessionsViewModel.kt index fb20f0fd31..0f1dcca4cc 100644 --- a/vector/src/main/java/im/vector/app/features/settings/devices/v2/othersessions/OtherSessionsViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/settings/devices/v2/othersessions/OtherSessionsViewModel.kt @@ -16,15 +16,12 @@ package im.vector.app.features.settings.devices.v2.othersessions -import android.content.SharedPreferences -import androidx.core.content.edit import com.airbnb.mvrx.MavericksViewModelFactory import com.airbnb.mvrx.Success import dagger.assisted.Assisted import dagger.assisted.AssistedFactory import dagger.assisted.AssistedInject import im.vector.app.core.di.ActiveSessionHolder -import im.vector.app.core.di.DefaultPreferences import im.vector.app.core.di.MavericksAssistedViewModelFactory import im.vector.app.core.di.hiltMavericksViewModelFactory import im.vector.app.features.auth.PendingAuthHandler @@ -47,8 +44,7 @@ class OtherSessionsViewModel @AssistedInject constructor( private val signoutSessionsUseCase: SignoutSessionsUseCase, private val pendingAuthHandler: PendingAuthHandler, refreshDevicesUseCase: RefreshDevicesUseCase, - @DefaultPreferences - private val sharedPreferences: SharedPreferences, + private val vectorPreferences: VectorPreferences, ) : VectorSessionsListViewModel( initialState, activeSessionHolder, refreshDevicesUseCase ) { @@ -68,7 +64,7 @@ class OtherSessionsViewModel @AssistedInject constructor( } private fun refreshIpAddressVisibility() { - val shouldShowIpAddress = sharedPreferences.getBoolean(VectorPreferences.SETTINGS_SESSION_MANAGER_SHOW_IP_ADDRESS, false) + val shouldShowIpAddress = vectorPreferences.showIpAddressInDeviceManagerScreens() setState { copy(isShowingIpAddress = shouldShowIpAddress) } @@ -108,9 +104,7 @@ class OtherSessionsViewModel @AssistedInject constructor( setState { copy(isShowingIpAddress = !isShowingIpAddress) } - sharedPreferences.edit { - putBoolean(VectorPreferences.SETTINGS_SESSION_MANAGER_SHOW_IP_ADDRESS, !isShowingIpAddress) - } + vectorPreferences.setIpAddressVisibilityInDeviceManagerScreens(!isShowingIpAddress) } private fun handleFilterDevices(action: OtherSessionsAction.FilterDevices) { diff --git a/vector/src/main/java/im/vector/app/features/settings/devices/v2/overview/SessionOverviewViewModel.kt b/vector/src/main/java/im/vector/app/features/settings/devices/v2/overview/SessionOverviewViewModel.kt index a105c40e9c..6c159fd50e 100644 --- a/vector/src/main/java/im/vector/app/features/settings/devices/v2/overview/SessionOverviewViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/settings/devices/v2/overview/SessionOverviewViewModel.kt @@ -16,15 +16,12 @@ package im.vector.app.features.settings.devices.v2.overview -import android.content.SharedPreferences -import androidx.core.content.edit import com.airbnb.mvrx.MavericksViewModelFactory import com.airbnb.mvrx.Success import dagger.assisted.Assisted import dagger.assisted.AssistedFactory import dagger.assisted.AssistedInject import im.vector.app.core.di.ActiveSessionHolder -import im.vector.app.core.di.DefaultPreferences import im.vector.app.core.di.MavericksAssistedViewModelFactory import im.vector.app.core.di.hiltMavericksViewModelFactory import im.vector.app.features.auth.PendingAuthHandler @@ -58,8 +55,7 @@ class SessionOverviewViewModel @AssistedInject constructor( private val togglePushNotificationUseCase: TogglePushNotificationUseCase, private val getNotificationsStatusUseCase: GetNotificationsStatusUseCase, refreshDevicesUseCase: RefreshDevicesUseCase, - @DefaultPreferences - private val sharedPreferences: SharedPreferences, + private val vectorPreferences: VectorPreferences, ) : VectorSessionsListViewModel( initialState, activeSessionHolder, refreshDevicesUseCase ) { @@ -80,7 +76,7 @@ class SessionOverviewViewModel @AssistedInject constructor( } private fun refreshIpAddressVisibility() { - val shouldShowIpAddress = sharedPreferences.getBoolean(VectorPreferences.SETTINGS_SESSION_MANAGER_SHOW_IP_ADDRESS, false) + val shouldShowIpAddress = vectorPreferences.showIpAddressInDeviceManagerScreens() setState { copy(isShowingIpAddress = shouldShowIpAddress) } @@ -134,9 +130,7 @@ class SessionOverviewViewModel @AssistedInject constructor( setState { copy(isShowingIpAddress = !isShowingIpAddress) } - sharedPreferences.edit { - putBoolean(VectorPreferences.SETTINGS_SESSION_MANAGER_SHOW_IP_ADDRESS, !isShowingIpAddress) - } + vectorPreferences.setIpAddressVisibilityInDeviceManagerScreens(!isShowingIpAddress) } private fun handleVerifySessionAction() = withState { viewState -> diff --git a/vector/src/main/res/layout/item_other_session.xml b/vector/src/main/res/layout/item_other_session.xml index dee96d2b2f..a6205e7d50 100644 --- a/vector/src/main/res/layout/item_other_session.xml +++ b/vector/src/main/res/layout/item_other_session.xml @@ -13,7 +13,7 @@ android:layout_width="0dp" android:layout_height="0dp" android:background="@drawable/bg_other_session" - app:layout_constraintBottom_toBottomOf="@id/otherSessionVerificationStatusImageView" + app:layout_constraintBottom_toBottomOf="@id/otherSessionSeparator" app:layout_constraintEnd_toEndOf="parent" app:layout_constraintStart_toStartOf="parent" app:layout_constraintTop_toTopOf="parent" /> @@ -53,11 +53,12 @@ android:layout_height="wrap_content" android:layout_marginStart="16dp" android:layout_marginEnd="8dp" + android:layout_marginTop="8dp" android:ellipsize="end" android:lines="1" app:layout_constraintEnd_toEndOf="parent" app:layout_constraintStart_toEndOf="@id/otherSessionDeviceTypeImageView" - app:layout_constraintTop_toTopOf="@id/otherSessionDeviceTypeImageView" + app:layout_constraintTop_toTopOf="@id/otherSessionItemBackground" tools:text="Element Mobile: Android" /> () private val fakePendingAuthHandler = FakePendingAuthHandler() private val fakeRefreshDevicesUseCase = mockk(relaxUnitFun = true) - private val fakeSharedPreferences = FakeSharedPreferences() + private val fakeVectorPreferences = FakeVectorPreferences() private fun createViewModel(): DevicesViewModel { return DevicesViewModel( @@ -87,7 +89,7 @@ class DevicesViewModelTest { interceptSignoutFlowResponseUseCase = fakeInterceptSignoutFlowResponseUseCase, pendingAuthHandler = fakePendingAuthHandler.instance, refreshDevicesUseCase = fakeRefreshDevicesUseCase, - sharedPreferences = fakeSharedPreferences, + vectorPreferences = fakeVectorPreferences.instance, ) } @@ -100,7 +102,7 @@ class DevicesViewModelTest { givenVerificationService() givenCurrentSessionCrossSigningInfo() givenDeviceFullInfoList(deviceId1 = A_DEVICE_ID_1, deviceId2 = A_DEVICE_ID_2) - fakeSharedPreferences.givenSessionManagerShowIpAddress(false) + fakeVectorPreferences.givenSessionManagerShowIpAddress(false) } private fun givenVerificationService(): FakeVerificationService { @@ -347,6 +349,29 @@ class DevicesViewModelTest { } } + @Test + fun `given the viewModel when initializing it then view state of ip address visibility is false`() { + // When + val viewModelTest = createViewModel().test() + + // Then + viewModelTest.assertLatestState { it.isShowingIpAddress == false } + viewModelTest.finish() + } + + @Test + fun `given the viewModel when toggleIpAddressVisibility action is triggered then view state and preference change accordingly`() { + // When + val viewModel = createViewModel() + val viewModelTest = viewModel.test() + viewModel.handle(DevicesAction.ToggleIpAddressVisibility) + + // Then + viewModelTest.assertLatestState { it.isShowingIpAddress == true } + every { fakeVectorPreferences.instance.setIpAddressVisibilityInDeviceManagerScreens(true) } just runs + viewModelTest.finish() + } + private fun givenCurrentSessionCrossSigningInfo(): CurrentSessionCrossSigningInfo { val currentSessionCrossSigningInfo = mockk() every { currentSessionCrossSigningInfo.deviceId } returns A_CURRENT_DEVICE_ID diff --git a/vector/src/test/java/im/vector/app/features/settings/devices/v2/othersessions/OtherSessionsViewModelTest.kt b/vector/src/test/java/im/vector/app/features/settings/devices/v2/othersessions/OtherSessionsViewModelTest.kt index d6ed5a61a7..054369ec9f 100644 --- a/vector/src/test/java/im/vector/app/features/settings/devices/v2/othersessions/OtherSessionsViewModelTest.kt +++ b/vector/src/test/java/im/vector/app/features/settings/devices/v2/othersessions/OtherSessionsViewModelTest.kt @@ -25,8 +25,8 @@ import im.vector.app.features.settings.devices.v2.RefreshDevicesUseCase import im.vector.app.features.settings.devices.v2.filter.DeviceManagerFilterType import im.vector.app.test.fakes.FakeActiveSessionHolder import im.vector.app.test.fakes.FakePendingAuthHandler -import im.vector.app.test.fakes.FakeSharedPreferences import im.vector.app.test.fakes.FakeSignoutSessionsUseCase +import im.vector.app.test.fakes.FakeVectorPreferences import im.vector.app.test.fakes.FakeVerificationService import im.vector.app.test.fixtures.aDeviceFullInfo import im.vector.app.test.test @@ -67,7 +67,7 @@ class OtherSessionsViewModelTest { private val fakeRefreshDevicesUseCase = mockk(relaxed = true) private val fakeSignoutSessionsUseCase = FakeSignoutSessionsUseCase() private val fakePendingAuthHandler = FakePendingAuthHandler() - private val fakeSharedPreferences = FakeSharedPreferences() + private val fakeVectorPreferences = FakeVectorPreferences() private fun createViewModel(viewState: OtherSessionsViewState = OtherSessionsViewState(defaultArgs)) = OtherSessionsViewModel( @@ -77,7 +77,7 @@ class OtherSessionsViewModelTest { signoutSessionsUseCase = fakeSignoutSessionsUseCase.instance, pendingAuthHandler = fakePendingAuthHandler.instance, refreshDevicesUseCase = fakeRefreshDevicesUseCase, - sharedPreferences = fakeSharedPreferences, + vectorPreferences = fakeVectorPreferences.instance, ) @Before @@ -87,7 +87,7 @@ class OtherSessionsViewModelTest { every { SystemClock.elapsedRealtime() } returns 1234 givenVerificationService() - fakeSharedPreferences.givenSessionManagerShowIpAddress(false) + fakeVectorPreferences.givenSessionManagerShowIpAddress(false) } private fun givenVerificationService(): FakeVerificationService { diff --git a/vector/src/test/java/im/vector/app/features/settings/devices/v2/overview/SessionOverviewViewModelTest.kt b/vector/src/test/java/im/vector/app/features/settings/devices/v2/overview/SessionOverviewViewModelTest.kt index c4ab82b7e8..3f81abd483 100644 --- a/vector/src/test/java/im/vector/app/features/settings/devices/v2/overview/SessionOverviewViewModelTest.kt +++ b/vector/src/test/java/im/vector/app/features/settings/devices/v2/overview/SessionOverviewViewModelTest.kt @@ -28,9 +28,9 @@ import im.vector.app.features.settings.devices.v2.verification.CheckIfCurrentSes import im.vector.app.test.fakes.FakeActiveSessionHolder import im.vector.app.test.fakes.FakeGetNotificationsStatusUseCase import im.vector.app.test.fakes.FakePendingAuthHandler -import im.vector.app.test.fakes.FakeSharedPreferences import im.vector.app.test.fakes.FakeSignoutSessionsUseCase import im.vector.app.test.fakes.FakeTogglePushNotificationUseCase +import im.vector.app.test.fakes.FakeVectorPreferences import im.vector.app.test.fakes.FakeVerificationService import im.vector.app.test.test import im.vector.app.test.testDispatcher @@ -78,7 +78,7 @@ class SessionOverviewViewModelTest { private val togglePushNotificationUseCase = FakeTogglePushNotificationUseCase() private val fakeGetNotificationsStatusUseCase = FakeGetNotificationsStatusUseCase() private val notificationsStatus = NotificationsStatus.ENABLED - private val fakeSharedPreferences = FakeSharedPreferences() + private val fakeVectorPreferences = FakeVectorPreferences() private fun createViewModel() = SessionOverviewViewModel( initialState = SessionOverviewViewState(args), @@ -91,7 +91,7 @@ class SessionOverviewViewModelTest { refreshDevicesUseCase = refreshDevicesUseCase, togglePushNotificationUseCase = togglePushNotificationUseCase.instance, getNotificationsStatusUseCase = fakeGetNotificationsStatusUseCase.instance, - sharedPreferences = fakeSharedPreferences, + vectorPreferences = fakeVectorPreferences.instance, ) @Before @@ -106,7 +106,7 @@ class SessionOverviewViewModelTest { A_SESSION_ID_1, notificationsStatus ) - fakeSharedPreferences.givenSessionManagerShowIpAddress(false) + fakeVectorPreferences.givenSessionManagerShowIpAddress(false) } private fun givenVerificationService(): FakeVerificationService { diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeSharedPreferences.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeSharedPreferences.kt index 0242bfe148..f9d525fd13 100644 --- a/vector/src/test/java/im/vector/app/test/fakes/FakeSharedPreferences.kt +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeSharedPreferences.kt @@ -18,7 +18,6 @@ package im.vector.app.test.fakes import android.content.SharedPreferences import im.vector.app.features.settings.FontScaleValue -import im.vector.app.features.settings.VectorPreferences.Companion.SETTINGS_SESSION_MANAGER_SHOW_IP_ADDRESS import io.mockk.every import io.mockk.mockk @@ -33,8 +32,4 @@ class FakeSharedPreferences : SharedPreferences by mockk() { every { contains("APPLICATION_USE_SYSTEM_FONT_SCALE_KEY") } returns true every { getBoolean("APPLICATION_USE_SYSTEM_FONT_SCALE_KEY", any()) } returns useSystemScale } - - fun givenSessionManagerShowIpAddress(showIpAddress: Boolean) { - every { getBoolean(SETTINGS_SESSION_MANAGER_SHOW_IP_ADDRESS, any()) } returns showIpAddress - } } diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeVectorPreferences.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeVectorPreferences.kt index 4baa7e2b90..f05f5f1493 100644 --- a/vector/src/test/java/im/vector/app/test/fakes/FakeVectorPreferences.kt +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeVectorPreferences.kt @@ -52,4 +52,8 @@ class FakeVectorPreferences { fun verifySetNotificationEnabledForDevice(enabled: Boolean, inverse: Boolean = false) { verify(inverse = inverse) { instance.setNotificationEnabledForDevice(enabled) } } + + fun givenSessionManagerShowIpAddress(showIpAddress: Boolean) { + every { instance.showIpAddressInDeviceManagerScreens() } returns showIpAddress + } }