Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trim custom list name and limit to 30 characters #6134

Merged
merged 1 commit into from
Apr 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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()
)
)
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -41,6 +42,7 @@ fun CustomListNameTextField(
placeholderText = null,
isValidValue = error == null,
isDigitsOnlyAllowed = false,
maxCharLength = CustomListName.MAX_LENGTH,
supportingText =
error?.let {
{
Expand Down
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down
Original file line number Diff line number Diff line change
@@ -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

Expand All @@ -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 }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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))
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -38,7 +39,7 @@ class CreateCustomListDialogViewModel(
customListActionUseCase
.performAction(
CustomListAction.Create(
name,
CustomListName.fromString(name),
if (locationCode.isNotEmpty()) {
listOf(locationCode)
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -44,8 +45,8 @@ class EditCustomListNameDialogViewModel(
.performAction(
CustomListAction.Rename(
customListId = customListId,
name = initialName,
newName = name
name = CustomListName.fromString(initialName),
newName = CustomListName.fromString(name)
)
)
.fold(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
Loading
Loading