Merge branch 'be-honest-with-connection-state-droid-1690'

5 files changed, 35 insertions, 29 deletions

diff --git a/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/ConnectScreenTest.kt b/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/ConnectScreenTest.kt
index be998a3728..59357d0527 100644
--- a/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/ConnectScreenTest.kt
+++ b/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/ConnectScreenTest.kt
@@ -168,7 +168,7 @@ class ConnectScreenTest {
             )
 
             // Assert
-            onNodeWithText("DISCONNECTED").assertExists()
+            onNodeWithText("DISCONNECTING...").assertExists()
             onNodeWithText(mockLocationName).assertExists()
             onNodeWithText("Disconnect").assertExists()
         }
@@ -310,7 +310,7 @@ class ConnectScreenTest {
             )
 
             // Assert
-            onNodeWithText("CONNECTED").assertExists()
+            onNodeWithText("BLOCKING...").assertExists()
             onNodeWithText(mockLocationName).assertExists()
             onNodeWithText("Disconnect").assertExists()
             onNodeWithText("BLOCKING INTERNET").assertExists()
diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/ConnectionStatusText.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/ConnectionStatusText.kt
index 535dcf2bb8..a5b1cff000 100644
--- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/ConnectionStatusText.kt
+++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/ConnectionStatusText.kt
@@ -47,8 +47,8 @@ private fun TunnelState.text() =
         is TunnelState.Disconnected -> textResource(id = R.string.disconnected)
         is TunnelState.Disconnecting ->
             when (actionAfterDisconnect) {
-                ActionAfterDisconnect.Nothing -> textResource(id = R.string.disconnected)
-                ActionAfterDisconnect.Block -> textResource(id = R.string.connected)
+                ActionAfterDisconnect.Nothing -> textResource(id = R.string.disconnecting)
+                ActionAfterDisconnect.Block -> textResource(id = R.string.blocking)
                 ActionAfterDisconnect.Reconnect -> textResource(id = R.string.connecting)
             }
         is TunnelState.Error ->
diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/util/FlowUtils.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/util/FlowUtils.kt
index 0c88598923..e0fa1d29b4 100644
--- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/util/FlowUtils.kt
+++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/util/FlowUtils.kt
@@ -5,6 +5,7 @@ package net.mullvad.mullvadvpn.util
 import kotlinx.coroutines.Deferred
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.flow.Flow
+import kotlinx.coroutines.flow.flow
 
 inline fun <T1, T2, T3, T4, T5, T6, R> combine(
     flow: Flow<T1>,
@@ -61,3 +62,11 @@ fun <T> Deferred<T>.getOrDefault(default: T) =
     } catch (e: IllegalStateException) {
         default
     }
+
+fun <T> Flow<T>.withPrev(): Flow<Pair<T, T?>> = flow {
+    var prev: T? = null
+    collect { curr ->
+        emit(curr to prev)
+        prev = curr
+    }
+}
diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/ConnectViewModel.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/ConnectViewModel.kt
index 42838d75d6..5fb08bcc48 100644
--- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/ConnectViewModel.kt
+++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/ConnectViewModel.kt
@@ -8,7 +8,6 @@ import kotlinx.coroutines.FlowPreview
 import kotlinx.coroutines.channels.Channel
 import kotlinx.coroutines.flow.SharingStarted
 import kotlinx.coroutines.flow.StateFlow
-import kotlinx.coroutines.flow.debounce
 import kotlinx.coroutines.flow.filter
 import kotlinx.coroutines.flow.filterIsInstance
 import kotlinx.coroutines.flow.map
@@ -36,6 +35,7 @@ import net.mullvad.mullvadvpn.usecase.SelectedLocationTitleUseCase
 import net.mullvad.mullvadvpn.util.combine
 import net.mullvad.mullvadvpn.util.daysFromNow
 import net.mullvad.mullvadvpn.util.isSuccess
+import net.mullvad.mullvadvpn.util.withPrev
 
 @Suppress("LongParameterList")
 class ConnectViewModel(
@@ -62,14 +62,14 @@ class ConnectViewModel(
         combine(
                 selectedLocationTitleUseCase(),
                 inAppNotificationController.notifications,
-                connectionProxy.tunnelState,
+                connectionProxy.tunnelState.withPrev(),
                 lastKnownLocationUseCase.lastKnownDisconnectedLocation,
                 accountRepository.accountData,
                 deviceRepository.deviceState.map { it?.displayName() },
             ) {
                 selectedRelayItemTitle,
                 notifications,
-                tunnelState,
+                (tunnelState, prevTunnelState),
                 lastKnownDisconnectedLocation,
                 accountData,
                 deviceName ->
@@ -80,14 +80,22 @@ class ConnectViewModel(
                                 tunnelState.location ?: lastKnownDisconnectedLocation
                             is TunnelState.Connecting -> tunnelState.location
                             is TunnelState.Connected -> tunnelState.location
-                            is TunnelState.Disconnecting -> lastKnownDisconnectedLocation
+                            is TunnelState.Disconnecting ->
+                                when (tunnelState.actionAfterDisconnect) {
+                                    ActionAfterDisconnect.Nothing -> lastKnownDisconnectedLocation
+                                    ActionAfterDisconnect.Block -> lastKnownDisconnectedLocation
+                                    // Keep the previous connected location when reconnecting, after
+                                    // this state we will reach Connecting with the new relay
+                                    // location
+                                    ActionAfterDisconnect.Reconnect -> prevTunnelState?.location()
+                                }
                             is TunnelState.Error -> lastKnownDisconnectedLocation
                         },
                     selectedRelayItemTitle = selectedRelayItemTitle,
                     tunnelState = tunnelState,
                     showLocation =
                         when (tunnelState) {
-                            is TunnelState.Disconnected -> true
+                            is TunnelState.Disconnected -> tunnelState.location != null
                             is TunnelState.Disconnecting -> {
                                 when (tunnelState.actionAfterDisconnect) {
                                     ActionAfterDisconnect.Nothing -> false
@@ -105,7 +113,6 @@ class ConnectViewModel(
                     isPlayBuild = isPlayBuild,
                 )
             }
-            .debounce(UI_STATE_DEBOUNCE_DURATION_MILLIS)
             .stateIn(viewModelScope, SharingStarted.WhileSubscribed(), ConnectUiState.INITIAL)
 
     init {
diff --git a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/ConnectViewModelTest.kt b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/ConnectViewModelTest.kt
index 0d61b3e300..1dab9a4565 100644
--- a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/ConnectViewModelTest.kt
+++ b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/ConnectViewModelTest.kt
@@ -171,12 +171,11 @@ class ConnectViewModelTest {
     fun `given RelayListUseCase returns new selectedRelayItem uiState should emit new selectedRelayItem`() =
         runTest {
             val selectedRelayItemTitle = "Item"
-            selectedRelayItemFlow.value = selectedRelayItemTitle
-
             viewModel.uiState.test {
                 assertEquals(ConnectUiState.INITIAL, awaitItem())
-                val result = awaitItem()
-                assertEquals(selectedRelayItemTitle, result.selectedRelayItemTitle)
+
+                selectedRelayItemFlow.value = selectedRelayItemTitle
+                assertEquals(selectedRelayItemTitle, awaitItem().selectedRelayItemTitle)
             }
         }
 
@@ -196,7 +195,6 @@ class ConnectViewModelTest {
 
         // Act, Assert
         viewModel.uiState.test {
-            assertEquals(ConnectUiState.INITIAL, awaitItem())
             tunnelState.emit(TunnelState.Disconnected(null))
 
             // Start of with no location
@@ -215,12 +213,7 @@ class ConnectViewModelTest {
             val locationTestItem = null
 
             // Act, Assert
-            viewModel.uiState.test {
-                assertEquals(ConnectUiState.INITIAL, awaitItem())
-                expectNoEvents()
-                val result = awaitItem()
-                assertEquals(locationTestItem, result.location)
-            }
+            viewModel.uiState.test { assertEquals(locationTestItem, awaitItem().location) }
         }
 
     @Test
@@ -278,15 +271,12 @@ class ConnectViewModelTest {
             val mockErrorState: ErrorState = mockk()
             val expectedConnectNotificationState =
                 InAppNotification.TunnelStateError(mockErrorState)
-            val tunnelStateError = TunnelState.Error(mockErrorState)
-            notifications.value = listOf(expectedConnectNotificationState)
 
             // Act, Assert
             viewModel.uiState.test {
                 assertEquals(ConnectUiState.INITIAL, awaitItem())
-                tunnelState.emit(tunnelStateError)
-                val result = awaitItem()
-                assertEquals(expectedConnectNotificationState, result.inAppNotification)
+                notifications.value = listOf(expectedConnectNotificationState)
+                assertEquals(expectedConnectNotificationState, awaitItem().inAppNotification)
             }
         }
 
@@ -315,7 +305,6 @@ class ConnectViewModelTest {
         viewModel.uiState.test {
             awaitItem()
             outOfTimeViewFlow.value = true
-            awaitItem()
         }
 
         // Assert
@@ -328,12 +317,13 @@ class ConnectViewModelTest {
             // Arrange
             val tunnel = TunnelState.Error(mockk(relaxed = true))
             val lastKnownLocation: GeoIpLocation = mockk(relaxed = true)
-            lastKnownLocationFlow.emit(lastKnownLocation)
-            tunnelState.emit(tunnel)
 
             // Act, Assert
             viewModel.uiState.test {
                 assertEquals(ConnectUiState.INITIAL, awaitItem())
+                lastKnownLocationFlow.emit(lastKnownLocation)
+                tunnelState.emit(tunnel)
+                awaitItem()
                 val result = awaitItem()
                 assertEquals(lastKnownLocation, result.location)
             }