Show all locations for custom lists in edit locations and detail view

[Pururun]

This is to avoid unexpected behavior. The locations are not shown in the select location screen, to align ux with desktop.

---

This change is 

[linear]

DROID-903 Edit list view gets affected by filters

[albin-mullvad]

Reviewed 8 of 14 files at r1, all commit messages.
Reviewable status: 8 of 14 files reviewed, 1 unresolved discussion (waiting on @Pururun)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/CustomListLocationsViewModel.kt line 40 at r1 (raw file):

    val uiState =
        combine(relayListUseCase.relatListAll(), _searchTerm, _selectedLocations) {

Typo?

Code quote:

relat

[albin-mullvad]

Reviewed 1 of 14 files at r1.
Reviewable status: 9 of 14 files reviewed, 3 unresolved discussions (waiting on @Pururun)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/RelayListUseCase.kt line 61 at r1 (raw file):

    fun selectedRelayItem(): Flow<RelayItem?> = relayListWithSelection().map { it.selectedItem }

    fun relayListFiltered(): Flow<List<RelayItem.Country>> =

I suggest: filteredRelayList

Code quote:

relayListFiltered

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/RelayListUseCase.kt line 64 at r1 (raw file):

        relayListWithSelection().map { it.filteredCountries }

    fun relatListAll(): Flow<List<RelayItem.Country>> =

I suggest something like: unfilteredRelayList or fullRelayList

Code quote:

relatListAll

[albin-mullvad]

Reviewed 1 of 14 files at r1.
Reviewable status: 10 of 14 files reviewed, 3 unresolved discussions (waiting on @Pururun)

[Rawa]

Reviewed 10 of 14 files at r1, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @albin-mullvad and @Pururun)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/RelayListUseCase.kt line 64 at r1 (raw file):

Previously, albin-mullvad wrote…

I suggest something like: unfilteredRelayList or fullRelayList

I agree the naming can be better, personally I think relayList and filteredRelayList would be clear enough.

[albin-mullvad]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @Pururun)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/RelayListUseCase.kt line 64 at r1 (raw file):

Previously, Rawa (David Göransson) wrote…

I agree the naming can be better, personally I think relayList and filteredRelayList would be clear enough.

Yeah, that also works! Was thinking of suggesting that but got the impression that @Pururun might see value in being more explicit. I'm fine either way!

[albin-mullvad]

Reviewed 4 of 14 files at r1.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @Pururun)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayListExtensions.kt line 183 at r1 (raw file):

    providers: Constraint<Providers>
): List<RelayItem.Country> =
    this.filter {

nit: not needed, right?

Code quote:

this.

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayListExtensions.kt line 195 at r1 (raw file):

    (ownership.toNullableOwnership() == null || ownership.toNullableOwnership() == it.ownership) &&
        (providers is Constraint.Any ||
            (providers is Constraint.Only && providers.value.providers.contains(it.providerName)))

Can the readability be improved? Quite hard to follow. Maybe using some helper function or local variable 🤔

Code quote:

    (ownership.toNullableOwnership() == null || ownership.toNullableOwnership() == it.ownership) &&
        (providers is Constraint.Any ||
            (providers is Constraint.Only && providers.value.providers.contains(it.providerName)))

[Pururun]

Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @albin-mullvad and @Rawa)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayListExtensions.kt line 183 at r1 (raw file):

Previously, albin-mullvad wrote…

nit: not needed, right?

Yeah, redid a little bit though since I realized my implementation would include empty cities.

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayListExtensions.kt line 195 at r1 (raw file):

Previously, albin-mullvad wrote…

Can the readability be improved? Quite hard to follow. Maybe using some helper function or local variable 🤔

I made it like that to not filter twice, but the previous implementation did also filter twice and was much more readable so I just changed it to use that one instead.

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/RelayListUseCase.kt line 61 at r1 (raw file):

Previously, albin-mullvad wrote…

I suggest: filteredRelayList

It was not needed so I just removed it.

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/RelayListUseCase.kt line 64 at r1 (raw file):

Previously, albin-mullvad wrote…

Yeah, that also works! Was thinking of suggesting that but got the impression that @Pururun might see value in being more explicit. I'm fine either way!

I would say I prefer fullRelayList

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/CustomListLocationsViewModel.kt line 40 at r1 (raw file):

Previously, albin-mullvad wrote…

Typo?

Wonder what happened there 🤔 Fixed.

[Pururun]

Reviewable status: 7 of 14 files reviewed, 3 unresolved discussions (waiting on @albin-mullvad and @Rawa)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/SelectLocationViewModel.kt line 38 at r3 (raw file):

    private val _searchTerm = MutableStateFlow(EMPTY_SEARCH_TERM)

    @Suppress("DestructuringDeclarationWithTooManyEntries")

Added this since I think that it is better to use a destructuring declaration and we only add an entry here that we don't use anyway.

[Pururun]

Reviewable status: 7 of 14 files reviewed, 3 unresolved discussions (waiting on @albin-mullvad and @Rawa)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/SelectLocationViewModel.kt line 38 at r3 (raw file):

Previously, Pururun (Jonatan Rhodin) wrote…

Added this since I think that it is better to use a destructuring declaration and we only add an entry here that we don't use anyway.

It should be noted that this is completely rewritten in the gRPC branch so hopefully this will not be in the code base for long.

[albin-mullvad]

Reviewed 3 of 4 files at r2, 3 of 3 files at r3, all commit messages.
Reviewable status: 13 of 14 files reviewed, all discussions resolved (waiting on @Rawa)

[Rawa]

Reviewed 1 of 4 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Pururun)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayListExtensions.kt line 187 at r3 (raw file):

                    val relays =
                        city.relays.filterRelayByOwnershipAndProviders(ownership, providers).map {
                            it.copy()

Unnecessary copy?

[albin-mullvad]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Pururun)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayListExtensions.kt line 183 at r1 (raw file):

Previously, Pururun (Jonatan Rhodin) wrote…

Yeah, redid a little bit though since I realized my implementation would include empty cities.

Still has the this keyword though? But I guess that's fine if you believe that it improves readability

[Rawa]

Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved

[Rawa]

Reviewable status: complete! all files reviewed, all discussions resolved

[albin-mullvad]

Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved

[Rawa]

Reviewable status: complete! all files reviewed, all discussions resolved