Merge branch 'custom-list-name-is-not-being-trimmed-droid-902'

mullvad · Apr 16, 2024 · b9a5907 · b9a5907
2 parents c1d3d10 + a8f5a90
commit b9a5907
Show file tree

Hide file tree

Showing 15 changed files with 105 additions and 46 deletions.
diff --git a/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/data/DummyRelayItems.kt b/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/data/DummyRelayItems.kt
@@ -1,6 +1,7 @@
 package net.mullvad.mullvadvpn.compose.data
 
 import net.mullvad.mullvadvpn.model.Constraint
+import net.mullvad.mullvadvpn.model.CustomListName
 import net.mullvad.mullvadvpn.model.PortRange
 import net.mullvad.mullvadvpn.model.RelayEndpointData
 import net.mullvad.mullvadvpn.model.RelayList
@@ -46,6 +47,16 @@ val DUMMY_RELAY_COUNTRIES =
 
 val DUMMY_CUSTOM_LISTS =
     listOf(
-        RelayItem.CustomList("First list", false, "1", locations = DUMMY_RELAY_COUNTRIES),
-        RelayItem.CustomList("Empty list", expanded = false, "2", locations = emptyList())
+        RelayItem.CustomList(
+            CustomListName.fromString("First list"),
+            false,
+            "1",
+            locations = DUMMY_RELAY_COUNTRIES
+        ),
+        RelayItem.CustomList(
+            CustomListName.fromString("Empty list"),
+            expanded = false,
+            "2",
+            locations = emptyList()
+        )
     )
diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/communication/CustomListAction.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/communication/CustomListAction.kt
@@ -2,22 +2,26 @@ package net.mullvad.mullvadvpn.compose.communication
 
 import android.os.Parcelable
 import kotlinx.parcelize.Parcelize
+import net.mullvad.mullvadvpn.model.CustomListName
 
 sealed interface CustomListAction : Parcelable {
 
     @Parcelize
-    data class Rename(val customListId: String, val name: String, val newName: String) :
-        CustomListAction {
+    data class Rename(
+        val customListId: String,
+        val name: CustomListName,
+        val newName: CustomListName
+    ) : CustomListAction {
         fun not() = this.copy(name = newName, newName = name)
     }
 
     @Parcelize
     data class Delete(val customListId: String) : CustomListAction {
-        fun not(name: String, locations: List<String>) = Create(name, locations)
+        fun not(name: CustomListName, locations: List<String>) = Create(name, locations)
     }
 
     @Parcelize
-    data class Create(val name: String = "", val locations: List<String> = emptyList()) :
+    data class Create(val name: CustomListName, val locations: List<String> = emptyList()) :
         CustomListAction, Parcelable {
         fun not(customListId: String) = Delete(customListId)
     }

diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/communication/CustomListResult.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/communication/CustomListResult.kt
@@ -2,33 +2,34 @@ package net.mullvad.mullvadvpn.compose.communication
 
 import android.os.Parcelable
 import kotlinx.parcelize.Parcelize
+import net.mullvad.mullvadvpn.model.CustomListName
 
 sealed interface CustomListResult : Parcelable {
     val undo: CustomListAction
 
     @Parcelize
     data class Created(
         val id: String,
-        val name: String,
+        val name: CustomListName,
         val locationName: String?,
         override val undo: CustomListAction.Delete
     ) : CustomListResult
 
     @Parcelize
     data class Deleted(override val undo: CustomListAction.Create) : CustomListResult {
-        val name
+        val name: CustomListName
             get() = undo.name
     }
 
     @Parcelize
     data class Renamed(override val undo: CustomListAction.Rename) : CustomListResult {
-        val name: String
+        val name: CustomListName
             get() = undo.name
     }
 
     @Parcelize
     data class LocationsChanged(
-        val name: String,
+        val name: CustomListName,
         override val undo: CustomListAction.UpdateLocations
     ) : CustomListResult
 }
diff --git a/...d/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/CustomListNameTextField.kt b/...d/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/CustomListNameTextField.kt
@@ -14,6 +14,7 @@ import androidx.compose.ui.res.stringResource
 import androidx.compose.ui.text.input.KeyboardType
 import net.mullvad.mullvadvpn.R
 import net.mullvad.mullvadvpn.compose.textfield.CustomTextField
+import net.mullvad.mullvadvpn.model.CustomListName
 import net.mullvad.mullvadvpn.model.CustomListsError
 
 @Composable
@@ -41,6 +42,7 @@ fun CustomListNameTextField(
         placeholderText = null,
         isValidValue = error == null,
         isDigitsOnlyAllowed = false,
+        maxCharLength = CustomListName.MAX_LENGTH,
         supportingText =
             error?.let {
                 {

diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/CustomListExtensions.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/CustomListExtensions.kt
@@ -1,13 +1,14 @@
 package net.mullvad.mullvadvpn.relaylist
 
 import net.mullvad.mullvadvpn.model.CustomList
+import net.mullvad.mullvadvpn.model.CustomListName
 
 private fun CustomList.toRelayItemCustomList(
     relayCountries: List<RelayItem.Country>
 ): RelayItem.CustomList =
     RelayItem.CustomList(
         id = this.id,
-        name = this.name,
+        customListName = CustomListName.fromString(name),
         expanded = false,
         locations =
             this.locations.mapNotNull {

diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayItem.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayItem.kt
@@ -1,5 +1,6 @@
 package net.mullvad.mullvadvpn.relaylist
 
+import net.mullvad.mullvadvpn.model.CustomListName
 import net.mullvad.mullvadvpn.model.GeoIpLocation
 import net.mullvad.mullvadvpn.model.GeographicLocationConstraint
 
@@ -15,11 +16,12 @@ sealed interface RelayItem {
     val expanded: Boolean
 
     data class CustomList(
-        override val name: String,
+        val customListName: CustomListName,
         override val expanded: Boolean,
         val id: String,
         val locations: List<RelayItem>,
     ) : RelayItem {
+        override val name: String = customListName.value
         override val active
             get() = locations.any { location -> location.active }
 

diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/repository/CustomListsRepository.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/repository/CustomListsRepository.kt
@@ -8,6 +8,7 @@ import net.mullvad.mullvadvpn.lib.ipc.Request
 import net.mullvad.mullvadvpn.lib.ipc.events
 import net.mullvad.mullvadvpn.model.CreateCustomListResult
 import net.mullvad.mullvadvpn.model.CustomList
+import net.mullvad.mullvadvpn.model.CustomListName
 import net.mullvad.mullvadvpn.model.CustomListsError
 import net.mullvad.mullvadvpn.model.GeographicLocationConstraint
 import net.mullvad.mullvadvpn.model.UpdateCustomListResult
@@ -20,8 +21,8 @@ class CustomListsRepository(
     private val settingsRepository: SettingsRepository,
     private val relayListListener: RelayListListener
 ) {
-    suspend fun createCustomList(name: String): CreateCustomListResult {
-        val result = messageHandler.trySendRequest(Request.CreateCustomList(name))
+    suspend fun createCustomList(name: CustomListName): CreateCustomListResult {
+        val result = messageHandler.trySendRequest(Request.CreateCustomList(name.value))
 
         return if (result) {
             messageHandler.events<Event.CreateCustomListResultEvent>().first().result
@@ -52,8 +53,8 @@ class CustomListsRepository(
                 ArrayList(locationCodes.mapNotNull { getGeographicLocationConstraintByCode(it) })
         )
 
-    suspend fun updateCustomListName(id: String, name: String): UpdateCustomListResult =
-        getCustomListById(id)?.let { updateCustomList(it.copy(name = name)) }
+    suspend fun updateCustomListName(id: String, name: CustomListName): UpdateCustomListResult =
+        getCustomListById(id)?.let { updateCustomList(it.copy(name = name.value)) }
             ?: UpdateCustomListResult.Error(CustomListsError.OtherError)
 
     private suspend fun updateCustomListLocations(

diff --git a/...app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/customlists/CustomListActionUseCase.kt b/...app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/customlists/CustomListActionUseCase.kt
@@ -5,6 +5,7 @@ import net.mullvad.mullvadvpn.compose.communication.CustomListAction
 import net.mullvad.mullvadvpn.compose.communication.CustomListResult
 import net.mullvad.mullvadvpn.model.CreateCustomListResult
 import net.mullvad.mullvadvpn.model.CustomList
+import net.mullvad.mullvadvpn.model.CustomListName
 import net.mullvad.mullvadvpn.model.GeographicLocationConstraint
 import net.mullvad.mullvadvpn.model.UpdateCustomListResult
 import net.mullvad.mullvadvpn.relaylist.getRelayItemsByCodes
@@ -79,9 +80,9 @@ class CustomListActionUseCase(
         }
 
     fun performAction(action: CustomListAction.Delete): Result<CustomListResult.Deleted> {
-        val customList: CustomList? = customListsRepository.getCustomListById(action.customListId)
+        val customList: CustomList = customListsRepository.getCustomListById(action.customListId)!!
         val oldLocations = customList.locations()
-        val name = customList?.name ?: ""
+        val name = CustomListName.fromString(customList.name)
         customListsRepository.deleteCustomList(action.customListId)
         return Result.success(
             CustomListResult.Deleted(undo = action.not(locations = oldLocations, name = name))
@@ -91,9 +92,9 @@ class CustomListActionUseCase(
     suspend fun performAction(
         action: CustomListAction.UpdateLocations
     ): Result<CustomListResult.LocationsChanged> {
-        val customList: CustomList? = customListsRepository.getCustomListById(action.customListId)
+        val customList = customListsRepository.getCustomListById(action.customListId)!!
         val oldLocations = customList.locations()
-        val name = customList?.name ?: ""
+        val name = CustomListName.fromString(customList.name)
         customListsRepository.updateCustomListLocationsFromCodes(
             action.customListId,
             action.locations

diff --git a/...d/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/CreateCustomListDialogViewModel.kt b/...d/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/CreateCustomListDialogViewModel.kt
@@ -13,6 +13,7 @@ import kotlinx.coroutines.launch
 import net.mullvad.mullvadvpn.compose.communication.CustomListAction
 import net.mullvad.mullvadvpn.compose.communication.CustomListResult
 import net.mullvad.mullvadvpn.compose.state.CreateCustomListUiState
+import net.mullvad.mullvadvpn.model.CustomListName
 import net.mullvad.mullvadvpn.model.CustomListsError
 import net.mullvad.mullvadvpn.usecase.customlists.CustomListActionUseCase
 import net.mullvad.mullvadvpn.usecase.customlists.CustomListsException
@@ -38,7 +39,7 @@ class CreateCustomListDialogViewModel(
             customListActionUseCase
                 .performAction(
                     CustomListAction.Create(
-                        name,
+                        CustomListName.fromString(name),
                         if (locationCode.isNotEmpty()) {
                             listOf(locationCode)
                         } else {

diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/CustomListLocationsViewModel.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/CustomListLocationsViewModel.kt
@@ -28,8 +28,6 @@ class CustomListLocationsViewModel(
     private val relayListUseCase: RelayListUseCase,
     private val customListActionUseCase: CustomListActionUseCase
 ) : ViewModel() {
-    private var customListName: String = ""
-
     private val _uiSideEffect =
         MutableSharedFlow<CustomListLocationsSideEffect>(replay = 1, extraBufferCapacity = 1)
     val uiSideEffect: SharedFlow<CustomListLocationsSideEffect> = _uiSideEffect
@@ -195,11 +193,9 @@ class CustomListLocationsViewModel(
 
     private suspend fun fetchInitialSelectedLocations() {
         _selectedLocations.value =
-            awaitCustomListById(customListId)
-                ?.apply { customListName = name }
-                ?.locations
-                ?.selectChildren()
-                .apply { _initialLocations.value = this ?: emptySet() }
+            awaitCustomListById(customListId)?.locations?.selectChildren().apply {
+                _initialLocations.value = this ?: emptySet()
+            }
     }
 
     companion object {

diff --git a/...app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/EditCustomListNameDialogViewModel.kt b/...app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/EditCustomListNameDialogViewModel.kt
@@ -13,6 +13,7 @@ import kotlinx.coroutines.launch
 import net.mullvad.mullvadvpn.compose.communication.CustomListAction
 import net.mullvad.mullvadvpn.compose.communication.CustomListResult
 import net.mullvad.mullvadvpn.compose.state.UpdateCustomListUiState
+import net.mullvad.mullvadvpn.model.CustomListName
 import net.mullvad.mullvadvpn.model.CustomListsError
 import net.mullvad.mullvadvpn.usecase.customlists.CustomListActionUseCase
 import net.mullvad.mullvadvpn.usecase.customlists.CustomListsException
@@ -44,8 +45,8 @@ class EditCustomListNameDialogViewModel(
                 .performAction(
                     CustomListAction.Rename(
                         customListId = customListId,
-                        name = initialName,
-                        newName = name
+                        name = CustomListName.fromString(initialName),
+                        newName = CustomListName.fromString(name)
                     )
                 )
                 .fold(

diff --git a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/repository/CustomListsRepositoryTest.kt b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/repository/CustomListsRepositoryTest.kt
@@ -13,6 +13,7 @@ import net.mullvad.mullvadvpn.lib.ipc.Request
 import net.mullvad.mullvadvpn.lib.ipc.events
 import net.mullvad.mullvadvpn.model.CreateCustomListResult
 import net.mullvad.mullvadvpn.model.CustomList
+import net.mullvad.mullvadvpn.model.CustomListName
 import net.mullvad.mullvadvpn.model.CustomListsError
 import net.mullvad.mullvadvpn.model.GeographicLocationConstraint
 import net.mullvad.mullvadvpn.model.RelayList
@@ -94,7 +95,8 @@ class CustomListsRepositoryTest {
             flowOf(Event.CreateCustomListResultEvent(expectedResult))
 
         // Act
-        val result = customListsRepository.createCustomList(customListName)
+        val result =
+            customListsRepository.createCustomList(CustomListName.fromString(customListName))
 
         // Assert
         assertEquals(expectedResult, result)
@@ -113,7 +115,8 @@ class CustomListsRepositoryTest {
                 flowOf(Event.CreateCustomListResultEvent(expectedResult))
 
             // Act
-            val result = customListsRepository.createCustomList(customListName)
+            val result =
+                customListsRepository.createCustomList(CustomListName.fromString(customListName))
 
             // Assert
             assertEquals(expectedResult, result)
@@ -139,7 +142,11 @@ class CustomListsRepositoryTest {
         every { mockSettings.customLists.customLists } returns arrayListOf(mockCustomList)
 
         // Act
-        val result = customListsRepository.updateCustomListName(customListId, customListName)
+        val result =
+            customListsRepository.updateCustomListName(
+                customListId,
+                CustomListName.fromString(customListName)
+            )
 
         // Assert
         assertEquals(expectedResult, result)
@@ -167,7 +174,11 @@ class CustomListsRepositoryTest {
             every { mockSettings.customLists.customLists } returns arrayListOf(mockCustomList)
 
             // Act
-            val result = customListsRepository.updateCustomListName(customListId, customListName)
+            val result =
+                customListsRepository.updateCustomListName(
+                    customListId,
+                    CustomListName.fromString(customListName)
+                )
 
             // Assert
             assertEquals(expectedResult, result)