Skip to content

Commit 99bd279

Browse files
RawaPururun
Rawa
authored and
Pururun
committed
Remove one-to-one relationship between provider and ownership
1 parent 9246544 commit 99bd279

File tree

25 files changed

+322
-318
lines changed

25 files changed

+322
-318
lines changed

‎android/CHANGELOG.md

+3
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ Line wrap the file at 100 chars. Th
3333
- Update to DAITA v2. The main difference is that many different machines are provided by relays
3434
instead of a bundled list.
3535

36+
### Fixed
37+
- Fix a crash that would occur because a Provider would be listed twice in the filter screen.
38+
3639

3740
## [android/2024.9] - 2024-12-09
3841
### Changed

‎android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/data/DummyRelayItems.kt

+4-5
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import net.mullvad.mullvadvpn.lib.model.CustomListName
66
import net.mullvad.mullvadvpn.lib.model.GeoLocationId
77
import net.mullvad.mullvadvpn.lib.model.Ownership
88
import net.mullvad.mullvadvpn.lib.model.PortRange
9-
import net.mullvad.mullvadvpn.lib.model.Provider
109
import net.mullvad.mullvadvpn.lib.model.ProviderId
1110
import net.mullvad.mullvadvpn.lib.model.RelayItem
1211
import net.mullvad.mullvadvpn.lib.model.RelayList
@@ -20,8 +19,8 @@ private val DUMMY_RELAY_1 =
2019
"Relay host 1",
2120
),
2221
active = true,
23-
provider =
24-
Provider(providerId = ProviderId("PROVIDER RENTED"), ownership = Ownership.Rented),
22+
provider = ProviderId("PROVIDER RENTED"),
23+
ownership = Ownership.Rented,
2524
daita = false,
2625
)
2726
private val DUMMY_RELAY_2 =
@@ -32,8 +31,8 @@ private val DUMMY_RELAY_2 =
3231
"Relay host 2",
3332
),
3433
active = true,
35-
provider =
36-
Provider(providerId = ProviderId("PROVIDER OWNED"), ownership = Ownership.MullvadOwned),
34+
provider = ProviderId("PROVIDER OWNED"),
35+
ownership = Ownership.MullvadOwned,
3736
daita = false,
3837
)
3938
private val DUMMY_RELAY_CITY_1 =

‎android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/FilterScreenTest.kt

+46-48
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import net.mullvad.mullvadvpn.compose.createEdgeToEdgeComposeExtension
1111
import net.mullvad.mullvadvpn.compose.setContentWithTheme
1212
import net.mullvad.mullvadvpn.compose.state.RelayFilterUiState
1313
import net.mullvad.mullvadvpn.lib.model.Ownership
14-
import net.mullvad.mullvadvpn.lib.model.Provider
1514
import net.mullvad.mullvadvpn.lib.model.ProviderId
1615
import org.junit.jupiter.api.Test
1716
import org.junit.jupiter.api.extension.RegisterExtension
@@ -30,7 +29,7 @@ class FilterScreenTest {
3029
onApplyClick: () -> Unit = {},
3130
onSelectedOwnership: (ownership: Ownership?) -> Unit = {},
3231
onAllProviderCheckChange: (isChecked: Boolean) -> Unit = {},
33-
onSelectedProvider: (checked: Boolean, provider: Provider) -> Unit = { _, _ -> },
32+
onSelectedProvider: (checked: Boolean, provider: ProviderId) -> Unit = { _, _ -> },
3433
) {
3534
setContentWithTheme {
3635
FilterScreen(
@@ -50,7 +49,7 @@ class FilterScreenTest {
5049
initScreen(
5150
state =
5251
RelayFilterUiState(
53-
allProviders = DUMMY_RELAY_ALL_PROVIDERS,
52+
providerToOwnerships = DUMMY_RELAY_ALL_PROVIDERS,
5453
selectedOwnership = null,
5554
selectedProviders = DUMMY_SELECTED_PROVIDERS,
5655
)
@@ -65,7 +64,7 @@ class FilterScreenTest {
6564
initScreen(
6665
state =
6766
RelayFilterUiState(
68-
allProviders = DUMMY_RELAY_ALL_PROVIDERS,
67+
providerToOwnerships = DUMMY_RELAY_ALL_PROVIDERS,
6968
selectedOwnership = null,
7069
selectedProviders = DUMMY_SELECTED_PROVIDERS,
7170
)
@@ -80,7 +79,7 @@ class FilterScreenTest {
8079
initScreen(
8180
state =
8281
RelayFilterUiState(
83-
allProviders = DUMMY_RELAY_ALL_PROVIDERS,
82+
providerToOwnerships = DUMMY_RELAY_ALL_PROVIDERS,
8483
selectedOwnership = Ownership.MullvadOwned,
8584
selectedProviders = DUMMY_SELECTED_PROVIDERS,
8685
)
@@ -95,7 +94,7 @@ class FilterScreenTest {
9594
initScreen(
9695
state =
9796
RelayFilterUiState(
98-
allProviders = DUMMY_RELAY_ALL_PROVIDERS,
97+
providerToOwnerships = DUMMY_RELAY_ALL_PROVIDERS,
9998
selectedOwnership = Ownership.Rented,
10099
selectedProviders = DUMMY_SELECTED_PROVIDERS,
101100
)
@@ -110,7 +109,7 @@ class FilterScreenTest {
110109
initScreen(
111110
state =
112111
RelayFilterUiState(
113-
allProviders = DUMMY_RELAY_ALL_PROVIDERS,
112+
providerToOwnerships = DUMMY_RELAY_ALL_PROVIDERS,
114113
selectedOwnership = null,
115114
selectedProviders = DUMMY_SELECTED_PROVIDERS,
116115
)
@@ -128,58 +127,57 @@ class FilterScreenTest {
128127
initScreen(
129128
state =
130129
RelayFilterUiState(
131-
allProviders = listOf(),
130+
providerToOwnerships = DUMMY_RELAY_ALL_PROVIDERS,
132131
selectedOwnership = null,
133-
selectedProviders =
134-
listOf(Provider(ProviderId("31173"), Ownership.MullvadOwned)),
132+
selectedProviders = listOf(ProviderId("31173")),
135133
),
136134
onApplyClick = mockClickListener,
137135
)
138136
onNodeWithText("Apply").performClick()
139137
verify { mockClickListener() }
140138
}
141139

140+
@Test
141+
fun ensureSelectedProviderIsShowEvenThoughItIsNotInAllProviders() =
142+
composeExtension.use {
143+
// Arrange
144+
initScreen(
145+
state =
146+
RelayFilterUiState(
147+
providerToOwnerships = DUMMY_RELAY_ALL_PROVIDERS,
148+
selectedOwnership = null,
149+
selectedProviders = listOf(ProviderId("1RemovedProvider")),
150+
)
151+
)
152+
153+
// Act
154+
onNodeWithText("Providers").performClick()
155+
// Asset
156+
onNodeWithText("1RemovedProvider (removed)").assertExists()
157+
}
158+
142159
companion object {
143160
private val DUMMY_RELAY_ALL_PROVIDERS =
144-
listOf(
145-
Provider(ProviderId("31173"), Ownership.MullvadOwned),
146-
Provider(ProviderId("100TB"), Ownership.Rented),
147-
Provider(ProviderId("Blix"), Ownership.MullvadOwned),
148-
Provider(ProviderId("Creanova"), Ownership.MullvadOwned),
149-
Provider(ProviderId("DataPacket"), Ownership.Rented),
150-
Provider(ProviderId("HostRoyale"), Ownership.Rented),
151-
Provider(ProviderId("hostuniversal"), Ownership.Rented),
152-
Provider(ProviderId("iRegister"), Ownership.Rented),
153-
Provider(ProviderId("M247"), Ownership.Rented),
154-
Provider(ProviderId("Makonix"), Ownership.Rented),
155-
Provider(ProviderId("PrivateLayer"), Ownership.Rented),
156-
Provider(ProviderId("ptisp"), Ownership.Rented),
157-
Provider(ProviderId("Qnax"), Ownership.Rented),
158-
Provider(ProviderId("Quadranet"), Ownership.Rented),
159-
Provider(ProviderId("techfutures"), Ownership.Rented),
160-
Provider(ProviderId("Tzulo"), Ownership.Rented),
161-
Provider(ProviderId("xtom"), Ownership.Rented),
161+
mapOf(
162+
ProviderId("31173") to setOf(Ownership.MullvadOwned),
163+
ProviderId("100TB") to setOf(Ownership.Rented),
164+
ProviderId("Blix") to setOf(Ownership.MullvadOwned),
165+
ProviderId("Creanova") to setOf(Ownership.MullvadOwned),
166+
ProviderId("DataPacket") to setOf(Ownership.Rented),
167+
ProviderId("HostRoyale") to setOf(Ownership.Rented),
168+
ProviderId("hostuniversal") to setOf(Ownership.Rented),
169+
ProviderId("iRegister") to setOf(Ownership.Rented),
170+
ProviderId("M247") to setOf(Ownership.Rented),
171+
ProviderId("Makonix") to setOf(Ownership.Rented),
172+
ProviderId("PrivateLayer") to setOf(Ownership.Rented),
173+
ProviderId("ptisp") to setOf(Ownership.Rented),
174+
ProviderId("Qnax") to setOf(Ownership.Rented),
175+
ProviderId("Quadranet") to setOf(Ownership.Rented),
176+
ProviderId("techfutures") to setOf(Ownership.Rented),
177+
ProviderId("Tzulo") to setOf(Ownership.Rented),
178+
ProviderId("xtom") to setOf(Ownership.Rented),
162179
)
163180

164-
private val DUMMY_SELECTED_PROVIDERS =
165-
listOf(
166-
Provider(ProviderId("31173"), Ownership.MullvadOwned),
167-
Provider(ProviderId("100TB"), Ownership.Rented),
168-
Provider(ProviderId("Blix"), Ownership.MullvadOwned),
169-
Provider(ProviderId("Creanova"), Ownership.MullvadOwned),
170-
Provider(ProviderId("DataPacket"), Ownership.Rented),
171-
Provider(ProviderId("HostRoyale"), Ownership.Rented),
172-
Provider(ProviderId("hostuniversal"), Ownership.Rented),
173-
Provider(ProviderId("iRegister"), Ownership.Rented),
174-
Provider(ProviderId("M247"), Ownership.Rented),
175-
Provider(ProviderId("Makonix"), Ownership.Rented),
176-
Provider(ProviderId("PrivateLayer"), Ownership.Rented),
177-
Provider(ProviderId("ptisp"), Ownership.Rented),
178-
Provider(ProviderId("Qnax"), Ownership.Rented),
179-
Provider(ProviderId("Quadranet"), Ownership.Rented),
180-
Provider(ProviderId("techfutures"), Ownership.Rented),
181-
Provider(ProviderId("Tzulo"), Ownership.Rented),
182-
Provider(ProviderId("xtom"), Ownership.Rented),
183-
)
181+
private val DUMMY_SELECTED_PROVIDERS = DUMMY_RELAY_ALL_PROVIDERS.keys.toList()
184182
}
185183
}

‎android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/cell/CheckboxCell.kt

+2-1
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ internal fun CheckboxCell(
3131
modifier: Modifier = Modifier,
3232
title: String,
3333
checked: Boolean,
34+
enabled: Boolean = true,
3435
onCheckedChange: (Boolean) -> Unit,
3536
background: Color = MaterialTheme.colorScheme.surfaceContainerLow,
3637
startPadding: Dp = Dimens.mediumPadding,
@@ -41,7 +42,7 @@ internal fun CheckboxCell(
4142
verticalAlignment = Alignment.CenterVertically,
4243
modifier =
4344
modifier
44-
.clickable { onCheckedChange(!checked) }
45+
.clickable(enabled) { onCheckedChange(!checked) }
4546
.defaultMinSize(minHeight = minHeight)
4647
.fillMaxWidth()
4748
.background(background)

‎android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/preview/FilterUiStatePreviewParameterProvider.kt

+3-5
Original file line numberDiff line numberDiff line change
@@ -3,19 +3,17 @@ package net.mullvad.mullvadvpn.compose.preview
33
import androidx.compose.ui.tooling.preview.PreviewParameterProvider
44
import net.mullvad.mullvadvpn.compose.state.RelayFilterUiState
55
import net.mullvad.mullvadvpn.lib.model.Ownership
6-
import net.mullvad.mullvadvpn.lib.model.Provider
76
import net.mullvad.mullvadvpn.lib.model.ProviderId
87

9-
private val PROVIDER =
10-
Provider(providerId = ProviderId("provider1"), ownership = Ownership.MullvadOwned)
8+
private val PROVIDER_TO_OWNERSHIPS = mapOf(ProviderId("provider1") to setOf(Ownership.MullvadOwned))
119

1210
class FilterUiStatePreviewParameterProvider : PreviewParameterProvider<RelayFilterUiState> {
1311
override val values =
1412
sequenceOf(
1513
RelayFilterUiState(
14+
providerToOwnerships = PROVIDER_TO_OWNERSHIPS,
1615
selectedOwnership = Ownership.MullvadOwned,
17-
allProviders = listOf(PROVIDER),
18-
selectedProviders = listOf(PROVIDER),
16+
selectedProviders = PROVIDER_TO_OWNERSHIPS.keys.toList(),
1917
)
2018
)
2119
}

‎android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/preview/RelayItemPreviewData.kt

+2-2
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package net.mullvad.mullvadvpn.compose.preview
22

33
import net.mullvad.mullvadvpn.lib.model.GeoLocationId
44
import net.mullvad.mullvadvpn.lib.model.Ownership
5-
import net.mullvad.mullvadvpn.lib.model.Provider
65
import net.mullvad.mullvadvpn.lib.model.ProviderId
76
import net.mullvad.mullvadvpn.lib.model.RelayItem
87

@@ -56,7 +55,8 @@ private fun generateRelayItemRelay(
5655
RelayItem.Location.Relay(
5756
id = GeoLocationId.Hostname(city = cityCode, code = hostName),
5857
active = active,
59-
provider = Provider(ProviderId("Provider"), Ownership.MullvadOwned),
58+
provider = ProviderId("Provider"),
59+
ownership = Ownership.MullvadOwned,
6060
daita = daita,
6161
)
6262

‎android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/preview/SelectLocationsUiStatePreviewParameterProvider.kt

-2
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@ package net.mullvad.mullvadvpn.compose.preview
33
import androidx.compose.ui.tooling.preview.PreviewParameterProvider
44
import net.mullvad.mullvadvpn.compose.state.RelayListType
55
import net.mullvad.mullvadvpn.compose.state.SelectLocationUiState
6-
import net.mullvad.mullvadvpn.lib.model.Ownership
7-
import net.mullvad.mullvadvpn.lib.model.Provider
86
import net.mullvad.mullvadvpn.usecase.FilterChip
97
import net.mullvad.mullvadvpn.usecase.ModelOwnership
108

‎android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/FilterScreen.kt

+34-10
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ import net.mullvad.mullvadvpn.compose.state.RelayFilterUiState
4646
import net.mullvad.mullvadvpn.compose.transitions.SlideInFromRightTransition
4747
import net.mullvad.mullvadvpn.compose.util.CollectSideEffectWithLifecycle
4848
import net.mullvad.mullvadvpn.lib.model.Ownership
49-
import net.mullvad.mullvadvpn.lib.model.Provider
49+
import net.mullvad.mullvadvpn.lib.model.ProviderId
5050
import net.mullvad.mullvadvpn.lib.theme.AppTheme
5151
import net.mullvad.mullvadvpn.lib.theme.Dimens
5252
import net.mullvad.mullvadvpn.viewmodel.FilterScreenSideEffect
@@ -98,7 +98,7 @@ fun FilterScreen(
9898
onApplyClick: () -> Unit,
9999
onSelectedOwnership: (ownership: Ownership?) -> Unit,
100100
onAllProviderCheckChange: (isChecked: Boolean) -> Unit,
101-
onSelectedProvider: (checked: Boolean, provider: Provider) -> Unit,
101+
onSelectedProvider: (checked: Boolean, provider: ProviderId) -> Unit,
102102
) {
103103
var providerExpanded by rememberSaveable { mutableStateOf(false) }
104104
var ownershipExpanded by rememberSaveable { mutableStateOf(false) }
@@ -126,7 +126,7 @@ fun FilterScreen(
126126
itemsWithDivider(
127127
key = { it.name },
128128
contentType = { ContentType.ITEM },
129-
items = state.filteredOwnershipByProviders,
129+
items = state.selectableOwnerships,
130130
) { ownership ->
131131
Ownership(ownership, state, onSelectedOwnership)
132132
}
@@ -139,9 +139,17 @@ fun FilterScreen(
139139
AllProviders(state, onAllProviderCheckChange)
140140
}
141141
itemsWithDivider(
142-
key = { it.providerId.value },
142+
key = { it.value },
143143
contentType = { ContentType.ITEM },
144-
items = state.filteredProvidersByOwnership,
144+
items = state.removedProviders,
145+
) { provider ->
146+
RemovedProvider(provider, state, onSelectedProvider)
147+
}
148+
149+
itemsWithDivider(
150+
key = { it.value },
151+
contentType = { ContentType.ITEM },
152+
items = state.selectableProviders,
145153
) { provider ->
146154
Provider(provider, state, onSelectedProvider)
147155
}
@@ -216,14 +224,30 @@ private fun LazyItemScope.AllProviders(
216224

217225
@Composable
218226
private fun LazyItemScope.Provider(
219-
provider: Provider,
227+
providerId: ProviderId,
228+
state: RelayFilterUiState,
229+
onSelectedProvider: (checked: Boolean, providerId: ProviderId) -> Unit,
230+
) {
231+
CheckboxCell(
232+
title = providerId.value,
233+
checked = providerId in state.selectedProviders,
234+
onCheckedChange = { checked -> onSelectedProvider(checked, providerId) },
235+
modifier = Modifier.animateItem(),
236+
)
237+
}
238+
239+
@Composable
240+
private fun LazyItemScope.RemovedProvider(
241+
providerId: ProviderId,
220242
state: RelayFilterUiState,
221-
onSelectedProvider: (checked: Boolean, provider: Provider) -> Unit,
243+
onSelectedProvider: (checked: Boolean, providerId: ProviderId) -> Unit,
222244
) {
245+
val checked = providerId in state.selectedProviders
223246
CheckboxCell(
224-
title = provider.providerId.value,
225-
checked = provider in state.selectedProviders,
226-
onCheckedChange = { checked -> onSelectedProvider(checked, provider) },
247+
title = stringResource(R.string.removed_provider, providerId.value),
248+
checked = checked,
249+
enabled = checked,
250+
onCheckedChange = { checked -> onSelectedProvider(checked, providerId) },
227251
modifier = Modifier.animateItem(),
228252
)
229253
}

‎android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/FilterConstrainExtensions.kt

+5-8
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ package net.mullvad.mullvadvpn.compose.state
22

33
import net.mullvad.mullvadvpn.lib.model.Constraint
44
import net.mullvad.mullvadvpn.lib.model.Ownership
5-
import net.mullvad.mullvadvpn.lib.model.Provider
5+
import net.mullvad.mullvadvpn.lib.model.ProviderId
66
import net.mullvad.mullvadvpn.lib.model.Providers
77

88
fun Ownership?.toOwnershipConstraint(): Constraint<Ownership> =
@@ -11,18 +11,15 @@ fun Ownership?.toOwnershipConstraint(): Constraint<Ownership> =
1111
else -> Constraint.Only(this)
1212
}
1313

14-
fun Constraint<Providers>.toSelectedProviders(allProviders: List<Provider>): List<Provider> =
14+
fun Constraint<Providers>.toSelectedProviders(allProviders: List<ProviderId>): List<ProviderId> =
1515
when (this) {
1616
Constraint.Any -> allProviders
17-
is Constraint.Only ->
18-
value.providers.toList().mapNotNull { provider ->
19-
allProviders.firstOrNull { it.providerId == provider }
20-
}
17+
is Constraint.Only -> value.providers.toList()
2118
}
2219

23-
fun List<Provider>.toConstraintProviders(allProviders: List<Provider>): Constraint<Providers> =
20+
fun List<ProviderId>.toConstraintProviders(allProviders: List<ProviderId>): Constraint<Providers> =
2421
if (size == allProviders.size) {
2522
Constraint.Any
2623
} else {
27-
Constraint.Only(Providers(map { provider -> provider.providerId }.toHashSet()))
24+
Constraint.Only(Providers(toHashSet()))
2825
}

0 commit comments

Comments
 (0)