Clean up mtu dialog

mullvad · May 15, 2024 · 6040fbc · 6040fbc
1 parent e23e73c
commit 6040fbc
Show file tree

Hide file tree

Showing 17 changed files with 138 additions and 88 deletions.
diff --git a/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/dialog/MtuDialogTest.kt b/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/dialog/MtuDialogTest.kt
@@ -13,6 +13,7 @@ import io.mockk.mockk
 import io.mockk.verify
 import net.mullvad.mullvadvpn.compose.createEdgeToEdgeComposeExtension
 import net.mullvad.mullvadvpn.compose.setContentWithTheme
+import net.mullvad.mullvadvpn.viewmodel.MtuDialogUiState
 import org.junit.jupiter.api.BeforeEach
 import org.junit.jupiter.api.Test
 import org.junit.jupiter.api.extension.RegisterExtension
@@ -31,13 +32,17 @@ class MtuDialogTest {
     @SuppressLint("ComposableNaming")
     @Composable
     private fun testMtuDialog(
-        mtuInitial: Int? = null,
-        onSaveMtu: (Int) -> Unit = { _ -> },
+        mtuInput: String = "",
+        isValidInput: Boolean = true,
+        showResetButton: Boolean = true,
+        onInputChanged: (String) -> Unit = { _ -> },
+        onSaveMtu: (String) -> Unit = { _ -> },
         onResetMtu: () -> Unit = {},
         onDismiss: () -> Unit = {},
     ) {
         MtuDialog(
-            mtuInitial = mtuInitial,
+            MtuDialogUiState(mtuInput, isValidInput, showResetButton),
+            onInputChanged = onInputChanged,
             onSaveMtu = onSaveMtu,
             onResetMtu = onResetMtu,
             onDismiss = onDismiss
@@ -60,12 +65,12 @@ class MtuDialogTest {
             // Arrange
             setContentWithTheme {
                 testMtuDialog(
-                    mtuInitial = VALID_DUMMY_MTU_VALUE,
+                    mtuInput = VALID_DUMMY_MTU_VALUE,
                 )
             }
 
             // Assert
-            onNodeWithText(VALID_DUMMY_MTU_VALUE.toString()).assertExists()
+            onNodeWithText(VALID_DUMMY_MTU_VALUE).assertExists()
         }
 
     @Test
@@ -74,22 +79,22 @@ class MtuDialogTest {
             // Arrange
             setContentWithTheme {
                 testMtuDialog(
-                    null,
+                    "",
                 )
             }
 
             // Act
-            onNodeWithText(EMPTY_STRING).performTextInput(VALID_DUMMY_MTU_VALUE.toString())
+            onNodeWithText(EMPTY_STRING).performTextInput(VALID_DUMMY_MTU_VALUE)
 
             // Assert
-            onNodeWithText(VALID_DUMMY_MTU_VALUE.toString()).assertExists()
+            onNodeWithText(VALID_DUMMY_MTU_VALUE).assertExists()
         }
 
     @Test
     fun testMtuDialogSubmitOfValidValue() =
         composeExtension.use {
             // Arrange
-            val mockedSubmitHandler: (Int) -> Unit = mockk(relaxed = true)
+            val mockedSubmitHandler: (String) -> Unit = mockk(relaxed = true)
             setContentWithTheme {
                 testMtuDialog(
                     VALID_DUMMY_MTU_VALUE,
@@ -125,7 +130,7 @@ class MtuDialogTest {
             val mockedClickHandler: () -> Unit = mockk(relaxed = true)
             setContentWithTheme {
                 testMtuDialog(
-                    mtuInitial = VALID_DUMMY_MTU_VALUE,
+                    mtuInput = VALID_DUMMY_MTU_VALUE,
                     onResetMtu = mockedClickHandler,
                 )
             }
@@ -157,7 +162,7 @@ class MtuDialogTest {
 
     companion object {
         private const val EMPTY_STRING = ""
-        private const val VALID_DUMMY_MTU_VALUE = 1337
-        private const val INVALID_DUMMY_MTU_VALUE = 1111
+        private const val VALID_DUMMY_MTU_VALUE = "1337"
+        private const val INVALID_DUMMY_MTU_VALUE = "1111"
     }
 }
diff --git a/...app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/VpnSettingsScreenTest.kt b/...app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/VpnSettingsScreenTest.kt
@@ -21,6 +21,7 @@ import net.mullvad.mullvadvpn.compose.test.LAZY_LIST_WIREGUARD_CUSTOM_PORT_NUMBE
 import net.mullvad.mullvadvpn.compose.test.LAZY_LIST_WIREGUARD_CUSTOM_PORT_TEXT_TEST_TAG
 import net.mullvad.mullvadvpn.compose.test.LAZY_LIST_WIREGUARD_PORT_ITEM_X_TEST_TAG
 import net.mullvad.mullvadvpn.model.Constraint
+import net.mullvad.mullvadvpn.model.Mtu
 import net.mullvad.mullvadvpn.model.Port
 import net.mullvad.mullvadvpn.model.PortRange
 import net.mullvad.mullvadvpn.model.QuantumResistantState
@@ -49,7 +50,7 @@ class VpnSettingsScreenTest {
                 )
             }
 
-            apply { onNodeWithText("Auto-connect").assertExists() }
+            onNodeWithText("Auto-connect").assertExists()
 
             onNodeWithTag(LAZY_LIST_TEST_TAG)
                 .performScrollToNode(hasTestTag(LAZY_LIST_LAST_ITEM_TEST_TAG))
@@ -67,7 +68,10 @@ class VpnSettingsScreenTest {
             // Arrange
             setContentWithTheme {
                 VpnSettingsScreen(
-                    state = VpnSettingsUiState.createDefault(mtu = VALID_DUMMY_MTU_VALUE),
+                    state =
+                        VpnSettingsUiState.createDefault(
+                            mtu = Mtu.fromString(VALID_DUMMY_MTU_VALUE).getOrNull()!!
+                        ),
                 )
             }
 
@@ -360,7 +364,7 @@ class VpnSettingsScreenTest {
     fun testMtuClick() =
         composeExtension.use {
             // Arrange
-            val mockedClickHandler: (Int?) -> Unit = mockk(relaxed = true)
+            val mockedClickHandler: (Mtu?) -> Unit = mockk(relaxed = true)
             setContentWithTheme {
                 VpnSettingsScreen(
                     state = VpnSettingsUiState.createDefault(),

diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/cell/MtuComposeCell.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/cell/MtuComposeCell.kt
@@ -13,16 +13,17 @@ import net.mullvad.mullvadvpn.R
 import net.mullvad.mullvadvpn.constant.MTU_MAX_VALUE
 import net.mullvad.mullvadvpn.constant.MTU_MIN_VALUE
 import net.mullvad.mullvadvpn.lib.theme.AppTheme
+import net.mullvad.mullvadvpn.model.Mtu
 
 @Preview
 @Composable
 private fun PreviewMtuComposeCell() {
-    AppTheme { MtuComposeCell(mtuValue = "1300", onEditMtu = {}) }
+    AppTheme { MtuComposeCell(mtuValue = Mtu(1300), onEditMtu = {}) }
 }
 
 @Composable
 fun MtuComposeCell(
-    mtuValue: String,
+    mtuValue: Mtu?,
     onEditMtu: () -> Unit,
 ) {
     val titleModifier = Modifier
@@ -45,10 +46,10 @@ private fun MtuTitle(modifier: Modifier) {
 }
 
 @Composable
-private fun MtuBodyView(mtuValue: String, modifier: Modifier) {
+private fun MtuBodyView(mtuValue: Mtu?, modifier: Modifier) {
     Row(modifier = modifier.wrapContentWidth1().wrapContentHeight()) {
         Text(
-            text = mtuValue.ifEmpty { stringResource(id = R.string.hint_default) },
+            text = mtuValue?.value?.toString() ?: stringResource(id = R.string.hint_default),
             color = MaterialTheme.colorScheme.onPrimary
         )
     }

diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/MtuDialog.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/MtuDialog.kt
@@ -8,8 +8,8 @@ import androidx.compose.material3.AlertDialog
 import androidx.compose.material3.MaterialTheme
 import androidx.compose.material3.Text
 import androidx.compose.runtime.Composable
-import androidx.compose.runtime.mutableStateOf
-import androidx.compose.runtime.remember
+import androidx.compose.runtime.collectAsState
+import androidx.compose.runtime.getValue
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.res.stringResource
 import androidx.compose.ui.tooling.preview.Preview
@@ -27,47 +27,48 @@ import net.mullvad.mullvadvpn.constant.MTU_MIN_VALUE
 import net.mullvad.mullvadvpn.lib.theme.AppTheme
 import net.mullvad.mullvadvpn.lib.theme.Dimens
 import net.mullvad.mullvadvpn.lib.theme.color.AlphaDescription
-import net.mullvad.mullvadvpn.util.isValidMtu
+import net.mullvad.mullvadvpn.model.Mtu
 import net.mullvad.mullvadvpn.viewmodel.MtuDialogSideEffect
+import net.mullvad.mullvadvpn.viewmodel.MtuDialogUiState
 import net.mullvad.mullvadvpn.viewmodel.MtuDialogViewModel
 import org.koin.androidx.compose.koinViewModel
+import org.koin.core.parameter.parametersOf
 
 @Preview
 @Composable
 private fun PreviewMtuDialog() {
-    AppTheme { MtuDialog(mtuInitial = 1234, EmptyResultBackNavigator()) }
+    AppTheme { MtuDialog(mtuInitial = Mtu(1234), EmptyResultBackNavigator()) }
 }
 
 @Destination(style = DestinationStyle.Dialog::class)
 @Composable
-fun MtuDialog(mtuInitial: Int?, navigator: ResultBackNavigator<Boolean>) {
-    val viewModel = koinViewModel<MtuDialogViewModel>()
+fun MtuDialog(mtuInitial: Mtu?, navigator: ResultBackNavigator<Boolean>) {
+    val viewModel = koinViewModel<MtuDialogViewModel>(parameters = { parametersOf(mtuInitial) })
 
+    val uiState by viewModel.uiState.collectAsState()
     LaunchedEffectCollect(viewModel.uiSideEffect) {
         when (it) {
             MtuDialogSideEffect.Complete -> navigator.navigateBack(result = true)
             MtuDialogSideEffect.Error -> navigator.navigateBack(result = false)
         }
     }
     MtuDialog(
-        mtuInitial = mtuInitial,
+        uiState,
+        onInputChanged = viewModel::onInputChanged,
         onSaveMtu = viewModel::onSaveClick,
         onResetMtu = viewModel::onRestoreClick,
-        onDismiss = navigator::navigateBack
+        onDismiss = { navigator.navigateBack(true) }
     )
 }
 
 @Composable
 fun MtuDialog(
-    mtuInitial: Int?,
-    onSaveMtu: (Int) -> Unit,
+    state: MtuDialogUiState,
+    onInputChanged: (String) -> Unit,
+    onSaveMtu: (String) -> Unit,
     onResetMtu: () -> Unit,
     onDismiss: () -> Unit,
 ) {
-
-    val mtu = remember { mutableStateOf(mtuInitial?.toString() ?: "") }
-    val isValidMtu = mtu.value.toIntOrNull()?.isValidMtu() == true
-
     AlertDialog(
         onDismissRequest = onDismiss,
         title = {
@@ -79,18 +80,13 @@ fun MtuDialog(
         text = {
             Column {
                 MtuTextField(
-                    value = mtu.value,
-                    onValueChanged = { newMtuValue -> mtu.value = newMtuValue },
-                    onSubmit = { newMtuValue ->
-                        val mtuInt = newMtuValue.toIntOrNull()
-                        if (mtuInt?.isValidMtu() == true) {
-                            onSaveMtu(mtuInt)
-                        }
-                    },
+                    value = state.mtuInput,
+                    onValueChanged = onInputChanged,
+                    onSubmit = onSaveMtu,
                     isEnabled = true,
                     placeholderText = stringResource(R.string.enter_value_placeholder),
                     maxCharLength = 4,
-                    isValidValue = isValidMtu,
+                    isValidValue = state.isValidInput,
                     modifier = Modifier.fillMaxWidth()
                 )
 
@@ -111,17 +107,12 @@ fun MtuDialog(
             Column(verticalArrangement = Arrangement.spacedBy(Dimens.buttonSpacing)) {
                 PrimaryButton(
                     modifier = Modifier.fillMaxWidth(),
-                    isEnabled = isValidMtu,
+                    isEnabled = state.isValidInput,
                     text = stringResource(R.string.submit_button),
-                    onClick = {
-                        val mtuInt = mtu.value.toIntOrNull()
-                        if (mtuInt?.isValidMtu() == true) {
-                            onSaveMtu(mtuInt)
-                        }
-                    }
+                    onClick = { onSaveMtu(state.mtuInput) }
                 )
 
-                if (mtuInitial != null) {
+                if (state.showResetToDefault) {
                     NegativeButton(
                         modifier = Modifier.fillMaxWidth(),
                         text = stringResource(R.string.reset_to_default_button),

diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/AccountScreen.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/AccountScreen.kt
@@ -19,7 +19,6 @@ import androidx.compose.runtime.getValue
 import androidx.compose.runtime.remember
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
-import androidx.compose.ui.platform.LocalContext
 import androidx.compose.ui.platform.LocalUriHandler
 import androidx.compose.ui.res.painterResource
 import androidx.compose.ui.res.stringResource

diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/VpnSettingsScreen.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/VpnSettingsScreen.kt
@@ -87,6 +87,7 @@ import net.mullvad.mullvadvpn.constant.WIREGUARD_PRESET_PORTS
 import net.mullvad.mullvadvpn.lib.theme.AppTheme
 import net.mullvad.mullvadvpn.lib.theme.Dimens
 import net.mullvad.mullvadvpn.model.Constraint
+import net.mullvad.mullvadvpn.model.Mtu
 import net.mullvad.mullvadvpn.model.Port
 import net.mullvad.mullvadvpn.model.PortRange
 import net.mullvad.mullvadvpn.model.QuantumResistantState
@@ -107,7 +108,7 @@ private fun PreviewVpnSettings() {
             state =
                 VpnSettingsUiState.createDefault(
                     isAutoConnectEnabled = true,
-                    mtu = "1337",
+                    mtu = Mtu(1337),
                     isCustomDnsEnabled = true,
                     customDnsItems = listOf(CustomDnsItem("0.0.0.0", false)),
                 ),
@@ -287,7 +288,7 @@ fun VpnSettingsScreen(
     onToggleBlockAdultContent: (Boolean) -> Unit = {},
     onToggleBlockGambling: (Boolean) -> Unit = {},
     onToggleBlockSocialMedia: (Boolean) -> Unit = {},
-    navigateToMtuDialog: (mtu: Int?) -> Unit = {},
+    navigateToMtuDialog: (mtu: Mtu?) -> Unit = {},
     navigateToDns: (index: Int?, address: String?) -> Unit = { _, _ -> },
     onToggleDnsClick: (Boolean) -> Unit = {},
     onBackClick: () -> Unit = {},
@@ -617,10 +618,7 @@ fun VpnSettingsScreen(
             }
 
             item {
-                MtuComposeCell(
-                    mtuValue = state.mtu,
-                    onEditMtu = { navigateToMtuDialog(state.mtu.toIntOrNull()) }
-                )
+                MtuComposeCell(mtuValue = state.mtu, onEditMtu = { navigateToMtuDialog(state.mtu) })
             }
             item {
                 MtuSubtitle(modifier = Modifier.testTag(LAZY_LIST_LAST_ITEM_TEST_TAG))

diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/VpnSettingsUiState.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/VpnSettingsUiState.kt
@@ -2,14 +2,15 @@ package net.mullvad.mullvadvpn.compose.state
 
 import net.mullvad.mullvadvpn.model.Constraint
 import net.mullvad.mullvadvpn.model.DefaultDnsOptions
+import net.mullvad.mullvadvpn.model.Mtu
 import net.mullvad.mullvadvpn.model.Port
 import net.mullvad.mullvadvpn.model.PortRange
 import net.mullvad.mullvadvpn.model.QuantumResistantState
 import net.mullvad.mullvadvpn.model.SelectedObfuscation
 import net.mullvad.mullvadvpn.viewmodel.CustomDnsItem
 
 data class VpnSettingsUiState(
-    val mtu: String,
+    val mtu: Mtu?,
     val isAutoConnectEnabled: Boolean,
     val isLocalNetworkSharingEnabled: Boolean,
     val isCustomDnsEnabled: Boolean,
@@ -25,7 +26,7 @@ data class VpnSettingsUiState(
 
     companion object {
         fun createDefault(
-            mtu: String = "",
+            mtu: Mtu? = null,
             isAutoConnectEnabled: Boolean = false,
             isLocalNetworkSharingEnabled: Boolean = false,
             isCustomDnsEnabled: Boolean = false,

diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/di/UiModule.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/di/UiModule.kt
@@ -175,7 +175,7 @@ val uiModule = module {
     }
     viewModel { parameters -> DeviceListViewModel(get(), parameters.get()) }
     viewModel { DeviceRevokedViewModel(get(), get()) }
-    viewModel { MtuDialogViewModel(get()) }
+    viewModel { parameters -> MtuDialogViewModel(get(), parameters.getOrNull()) }
     viewModel { parameters ->
         DnsDialogViewModel(get(), get(), parameters.getOrNull(), parameters.getOrNull())
     }
@@ -185,7 +185,7 @@ val uiModule = module {
     viewModel { SettingsViewModel(get(), get(), IS_PLAY_BUILD) }
     viewModel { SplashViewModel(get(), get()) }
     viewModel { VoucherDialogViewModel(get()) }
-    viewModel { VpnSettingsViewModel(get(), get(), get(), get()) }
+    viewModel { VpnSettingsViewModel(get(), get(), get()) }
     viewModel { WelcomeViewModel(get(), get(), get(), isPlayBuild = IS_PLAY_BUILD) }
     viewModel { ReportProblemViewModel(get(), get()) }
     viewModel { ViewLogsViewModel(get()) }

diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/repository/SettingsRepository.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/repository/SettingsRepository.kt
@@ -12,6 +12,7 @@ import net.mullvad.mullvadvpn.model.CustomDnsOptions
 import net.mullvad.mullvadvpn.model.DefaultDnsOptions
 import net.mullvad.mullvadvpn.model.DnsOptions
 import net.mullvad.mullvadvpn.model.DnsState
+import net.mullvad.mullvadvpn.model.Mtu
 import net.mullvad.mullvadvpn.model.ObfuscationSettings
 import net.mullvad.mullvadvpn.model.QuantumResistantState
 import net.mullvad.mullvadvpn.model.Settings
@@ -51,7 +52,7 @@ class SettingsRepository(
 
     suspend fun addCustomDns(address: InetAddress) = managementService.addCustomDns(address)
 
-    suspend fun setWireguardMtu(value: Int) = managementService.setWireguardMtu(value)
+    suspend fun setWireguardMtu(mtu: Mtu) = managementService.setWireguardMtu(mtu.value)
 
     suspend fun clearWireguardMtu() = managementService.setWireguardMtu(null)
 

diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/util/IntegerExtension.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/util/IntegerExtension.kt