Code review fixes.

vector-config/src/main/res/values/config-settings.xml

@@ -51,6 +51,7 @@
     <bool name="settings_labs_rich_text_editor_default">false</bool>
     <bool name="settings_labs_enable_voice_broadcast_visible">true</bool>
     <bool name="settings_labs_enable_voice_broadcast_default">false</bool>
+    <bool name="settings_device_manager_show_ip_address">false</bool>
     <!-- Level 1: Advanced settings -->
 
     <!-- Level 1: Help and about -->

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)
+        }
+    }
 }

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 vectorPreferences: VectorPreferences,
-        private val sharedPreferences: SharedPreferences,
 ) : VectorSessionsListViewModel<DevicesViewState, DevicesAction, DevicesViewEvent>(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 {
+        vectorPreferences.setIpAddressVisibilityInDeviceManagerScreens(!isShowingIpAddress)
-            putBoolean(VectorPreferences.SETTINGS_SESSION_MANAGER_SHOW_IP_ADDRESS, !isShowingIpAddress)
-        }
     }
 
     private fun handleVerifyCurrentSessionAction() {

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 vectorPreferences: VectorPreferences,
-        private val sharedPreferences: SharedPreferences,
 ) : VectorSessionsListViewModel<OtherSessionsViewState, OtherSessionsAction, OtherSessionsViewEvents>(
         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 {
+        vectorPreferences.setIpAddressVisibilityInDeviceManagerScreens(!isShowingIpAddress)
-            putBoolean(VectorPreferences.SETTINGS_SESSION_MANAGER_SHOW_IP_ADDRESS, !isShowingIpAddress)
-        }
     }
 
     private fun handleFilterDevices(action: OtherSessionsAction.FilterDevices) {

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 vectorPreferences: VectorPreferences,
-        private val sharedPreferences: SharedPreferences,
 ) : VectorSessionsListViewModel<SessionOverviewViewState, SessionOverviewAction, SessionOverviewViewEvent>(
         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 {
+        vectorPreferences.setIpAddressVisibilityInDeviceManagerScreens(!isShowingIpAddress)
-            putBoolean(VectorPreferences.SETTINGS_SESSION_MANAGER_SHOW_IP_ADDRESS, !isShowingIpAddress)
-        }
     }
 
     private fun handleVerifySessionAction() = withState { viewState ->

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" />
 
     <TextView
@@ -85,6 +86,7 @@
         tools:text="0.0.0.0" />
 
     <View
+        android:id="@+id/otherSessionSeparator"
         android:layout_width="0dp"
         android:layout_height="1dp"
         android:layout_marginTop="8dp"

vector/src/test/java/im/vector/app/features/settings/devices/v2/DevicesViewModelTest.kt

@@ -28,17 +28,19 @@ import im.vector.app.features.settings.devices.v2.verification.CurrentSessionCro
 import im.vector.app.features.settings.devices.v2.verification.GetCurrentSessionCrossSigningInfoUseCase
 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.test
 import im.vector.app.test.testDispatcher
 import io.mockk.coEvery
 import io.mockk.coVerify
 import io.mockk.every
+import io.mockk.just
 import io.mockk.justRun
 import io.mockk.mockk
 import io.mockk.mockkStatic
+import io.mockk.runs
 import io.mockk.unmockkAll
 import io.mockk.verify
 import io.mockk.verifyAll
@@ -73,7 +75,7 @@ class DevicesViewModelTest {
     private val fakeInterceptSignoutFlowResponseUseCase = mockk<InterceptSignoutFlowResponseUseCase>()
     private val fakePendingAuthHandler = FakePendingAuthHandler()
     private val fakeRefreshDevicesUseCase = mockk<RefreshDevicesUseCase>(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<CurrentSessionCrossSigningInfo>()
         every { currentSessionCrossSigningInfo.deviceId } returns A_CURRENT_DEVICE_ID

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<RefreshDevicesUseCase>(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 {

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 {

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
-    }
 }

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
+    }
 }