Skip to content

Commit

Permalink
Add support for more detailed location changed messages
Browse files Browse the repository at this point in the history
  • Loading branch information
Rawa authored and Pururun committed Jul 18, 2024
1 parent 2368bb8 commit 7224dfe
Show file tree
Hide file tree
Showing 6 changed files with 103 additions and 7 deletions.
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package net.mullvad.mullvadvpn.compose.communication

import android.os.Parcelable
import kotlinx.parcelize.IgnoredOnParcel
import kotlinx.parcelize.Parcelize
import net.mullvad.mullvadvpn.lib.model.CustomListId
import net.mullvad.mullvadvpn.lib.model.CustomListName
import net.mullvad.mullvadvpn.lib.model.GeoLocationId

sealed interface CustomListSuccess : Parcelable {
val undo: CustomListAction
Expand Down Expand Up @@ -31,6 +33,14 @@ data class Renamed(override val undo: CustomListAction.Rename) : CustomListSucce

@Parcelize
data class LocationsChanged(
val id: CustomListId,
val name: CustomListName,
val locations: List<GeoLocationId>,
val oldLocations: List<GeoLocationId>,
) : CustomListSuccess {
override val undo: CustomListAction.UpdateLocations
) : CustomListSuccess
get() = CustomListAction.UpdateLocations(id, oldLocations)

@IgnoredOnParcel val addedLocations = locations - oldLocations
@IgnoredOnParcel val removedLocations = oldLocations - locations
}
Original file line number Diff line number Diff line change
Expand Up @@ -849,7 +849,22 @@ private fun CustomListSuccess.message(context: Context): String =
} ?: context.getString(R.string.locations_were_changed_for, name)
is Deleted -> context.getString(R.string.delete_custom_list_message, name)
is Renamed -> context.getString(R.string.name_was_changed_to, name)
is LocationsChanged -> context.getString(R.string.locations_were_changed_for, name)
is LocationsChanged ->
when {
addedLocations.isNotEmpty() && removedLocations.isEmpty() ->
context.getString(
R.string.location_was_added_to_list,
addedLocations.first(),
name
)
removedLocations.isNotEmpty() && addedLocations.isEmpty() ->
context.getString(
R.string.location_was_removed_from_list,
removedLocations.first(),
name
)
else -> context.getString(R.string.locations_were_changed_for, name)
}
}

@Composable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,10 @@ class CustomListActionUseCase(
.mapLeft(UpdateLocationsError::UpdateLocations)
.bind()
LocationsChanged(
id = action.id,
name = customList.name,
undo = action.not(locations = customList.locations)
locations = action.locations,
oldLocations = customList.locations,
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,13 @@ class CustomListActionUseCaseTest {
val customList = CustomList(id = customListId, name = name, locations = oldLocations)
val action = CustomListAction.UpdateLocations(id = customListId, locations = newLocations)
val expectedResult =
LocationsChanged(name = name, undo = action.not(locations = oldLocations)).right()
LocationsChanged(
id = customListId,
name = name,
locations = newLocations,
oldLocations = oldLocations,
)
.right()
coEvery { mockCustomListsRepository.getCustomListById(customListId) } returns
customList.right()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -252,20 +252,37 @@ class SelectLocationViewModelTest {
@Test
fun `after adding a location to a list should emit location added side effect`() = runTest {
// Arrange
val expectedResult: LocationsChanged = mockk()
val customListId = CustomListId("1")
val addedLocationsId = GeoLocationId.Country("se")
val customListName = CustomListName.fromString("custom")
val location: RelayItem.Location.Country = mockk {
every { id } returns GeoLocationId.Country("se")
every { name } returns "Sweden"
every { descendants() } returns emptyList()
}
val customList =
RelayItem.CustomList(
id = CustomListId("1"),
customListName = CustomListName.fromString("custom"),
customListName = customListName,
locations = emptyList(),
expanded = false
)
val expectedResult =
LocationsChanged(
id = customListId,
name = customListName,
locations = listOf(addedLocationsId),
oldLocations = emptyList(),
)

coEvery { mockCustomListActionUseCase(any<CustomListAction.UpdateLocations>()) } returns
expectedResult.right()
LocationsChanged(
id = customListId,
name = customListName,
locations = listOf(addedLocationsId),
oldLocations = emptyList()
)
.right()

// Act, Assert
viewModel.uiSideEffect.test {
Expand All @@ -276,6 +293,51 @@ class SelectLocationViewModelTest {
}
}

@Test
fun `after removing a location from a list should emit location removed side effect`() =
runTest {
// Arrange
val locationName = "Sweden"
val customListId = CustomListId("1")
val removedLocationsId = GeoLocationId.Country("se")
val customListName = CustomListName.fromString("custom")
val location: RelayItem.Location.Country = mockk {
every { id } returns removedLocationsId
every { name } returns locationName
every { descendants() } returns emptyList()
}
val customList =
RelayItem.CustomList(
id = customListId,
customListName = customListName,
locations = emptyList(),
expanded = false
)
val expectedResult =
LocationsChanged(
id = customListId,
name = customListName,
locations = emptyList(),
oldLocations = listOf(removedLocationsId),
)
coEvery { mockCustomListActionUseCase(any<CustomListAction.UpdateLocations>()) } returns
LocationsChanged(
id = customListId,
name = customListName,
locations = emptyList(),
oldLocations = listOf(removedLocationsId),
)
.right()

// Act, Assert
viewModel.uiSideEffect.test {
viewModel.removeLocationFromList(item = location, customList = customList)
val sideEffect = awaitItem()
assertIs<SelectLocationSideEffect.LocationRemovedFromCustomList>(sideEffect)
assertEquals(expectedResult, sideEffect.result)
}
}

companion object {
private const val RELAY_LIST_EXTENSIONS =
"net.mullvad.mullvadvpn.relaylist.RelayListExtensionsKt"
Expand Down
1 change: 1 addition & 0 deletions android/lib/resource/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -383,4 +383,5 @@
<string name="delete_method_question">Delete method?</string>
<string name="failed_to_set_current_test_error">Failed to set to current - API not reachable</string>
<string name="failed_to_set_current_unknown_error">Failed to set to current - Unknown reason</string>
<string name="location_was_removed_from_list">%s was removed from \"%s\"</string>
</resources>

0 comments on commit 7224dfe

Please sign in to comment.