Disconnect provider and ownership

[Rawa]

This PR changes the one-to-one relationship to one-to-many between provider and ownership.

---

This change is 

[linear]

DROID-1704 Multiple ownership for providers

[Pururun]

⭐

Reviewed 12 of 12 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Rawa)

---

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

                            Ownership.entries
                        } else {
                            Ownership.entries.filter { ownership ->

I think this could be simplified and be more like this:

allProviders.filter { it.providerId in selectedProviders }
                     .flatMap { it.ownership }
                     .distinct()

Could probably improve on the flatmap as well.

---

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

    fun onApplyButtonClicked() {
        val newSelectedOwnership = selectedOwnership.value.toOwnershipConstraint()
        // TODO should be all providers?!

Could do something like
availableProvidersUseCase.invoke().first()
maybe?

[Rawa]

Reviewable status: 5 of 16 files reviewed, 2 unresolved discussions (waiting on @Pururun)

---

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

Previously, Pururun (Jonatan Rhodin) wrote…

I think this could be simplified and be more like this:

allProviders.filter { it.providerId in selectedProviders }
                     .flatMap { it.ownership }
                     .distinct()

Could probably improve on the flatmap as well.

Fixed by reworking data.

[Rawa]

Reviewable status: 5 of 16 files reviewed, 2 unresolved discussions (waiting on @Pururun)

---

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

Previously, Pururun (Jonatan Rhodin) wrote…

Could do something like
availableProvidersUseCase.invoke().first()
maybe?

Simplified in new version.

[Pururun]

Reviewed 3 of 11 files at r2, 15 of 15 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Rawa)

---

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

Previously, Rawa (David Göransson) wrote…

Simplified in new version.

Do we actually need allProviders in the ui state?

[Rawa]

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

---

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

Previously, Pururun (Jonatan Rhodin) wrote…

Do we actually need allProviders in the ui state?

We need it to see if all providers are selected.

    val isAllProvidersChecked = allProviders.size == selectedProviders.size

We could use the providersToOwnershipUseCase here to get the full list of providers, but we would still need it in the UI state.

[Pururun]

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

---

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

Previously, Rawa (David Göransson) wrote…

We need it to see if all providers are selected.

    val isAllProvidersChecked = allProviders.size == selectedProviders.size

We could use the providersToOwnershipUseCase here to get the full list of providers, but we would still need it in the UI state.

Right, then all good!

[albin-mullvad]

Reviewed 3 of 12 files at r1, 3 of 11 files at r2, 15 of 15 files at r3, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Rawa)

---

android/app/src/test/kotlin/net/mullvad/mullvadvpn/usecase/FilterChipUseCaseTest.kt line 25 at r3 (raw file):

    private val mockRelayListFilterRepository: RelayListFilterRepository = mockk()
    private val mockProvidersOwnershipUseCase: ProviderToOwnershipsUseCase = mockk()

nit: mockProviderToOwnershipsUseCase

Code quote:

ProvidersOwnership

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/FilterChipUseCase.kt line 65 at r3 (raw file):

                                true
                            } else {
                                providerOwnershipRelationship[providerId]!!.contains(

Is this safe? E.g. what if a provider is used as a constraint is later removed from the relay list?

Code quote:

!!

[albin-mullvad]

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

---

a discussion (no related file):
Don't forget to update the changelog!

[Pururun]

Reviewed 6 of 6 files at r4, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @Rawa)

[Rawa]

Reviewable status: 24 of 25 files reviewed, 1 unresolved discussion (waiting on @albin-mullvad)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/FilterChipUseCase.kt line 65 at r3 (raw file):

Previously, albin-mullvad wrote…

Is this safe? E.g. what if a provider is used as a constraint is later removed from the relay list?

Addressed. Was not safe.

[Pururun]

Reviewed 2 of 2 files at r5, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @albin-mullvad)

[albin-mullvad]

Reviewed 6 of 6 files at r4, 2 of 2 files at r5, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Rawa)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/FilterChipUseCase.kt line 65 at r3 (raw file):

Previously, Rawa (David Göransson) wrote…

Addressed. Was not safe.

Nice! 👍

Can we add some unit test to somehow cover this case as well? Basically something like:

• Set a provider in the provider filter
• Change the underlying relay list
• Ensure that the provider is part of the removed relays

Might even be easier ways to achieve this 🤔

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/FilterChipUseCase.kt line 69 at r5 (raw file):

                                // so it is visible for the user. Because we won't know what
                                // ownerships it
                                // had

nit

Code quote:

                                // ownerships it
                                // had

[albin-mullvad]

Reviewed 2 of 2 files at r6, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Rawa)

[Rawa]

Reviewable status: 24 of 25 files reviewed, all discussions resolved (waiting on @albin-mullvad and @Pururun)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/FilterChipUseCase.kt line 65 at r3 (raw file):

Previously, albin-mullvad wrote…

Nice! 👍

Can we add some unit test to somehow cover this case as well? Basically something like:

• Set a provider in the provider filter
• Change the underlying relay list
• Ensure that the provider is part of the removed relays

Might even be easier ways to achieve this 🤔

I believe @Pururun fixed tests

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/FilterChipUseCase.kt line 69 at r5 (raw file):

Previously, albin-mullvad wrote…

nit

Addressed

[albin-mullvad]

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