Add more detailed snackbars when adding and removing a location in a custom list

[Pururun]

---

This change is 

[linear]

DROID-972 Custom lists add country snackbar/toast text wrong

[Rawa]

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

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/communication/CustomListSuccess.kt line 36 at r2 (raw file):

    val name: CustomListName,
    val locationNamesAdded: List<String>? = null,
    val locationNamesRemoved: List<String>? = null,

Doesn't make sense to have nullables.

Also should we have LocationsRemoved & LocationsAdded? Or if we add the full new set of locations we could get the added/removed by doing set operations.

[Rawa]

Reviewed 3 of 7 files at r3, 21 of 21 files at r5, all commit messages.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @Pururun)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/communication/CustomListActionResultData.kt line 8 at r5 (raw file):

import net.mullvad.mullvadvpn.lib.model.CustomListName

sealed interface CustomListActionResultData : Parcelable {

Introduce "Success" to split Error and Success in the result type.

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/SelectLocationScreen.kt line 849 at r5 (raw file):

            if (locationNames.size == 1) {
                context.getString(
                    R.string.location_was_added_to_list,

Should we have a separate string? Now it will say the same thing as if you added a item to a list.
E.g
Added 'Stockholm' to 'my_list' doesn't really say list got created.
maybe should say along the lines of:
Created list 'my_list' with location 'Stockholm'

---

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

                        { result ->
                            val resultData =
                                if (navArgs.newList) {

Maybe break out this logic into another function and return the CustomListActionResult?

---

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

                                                customListName = result.name,
                                                relayListRepository
                                                    .find(result.removedLocations.first())!!

Could be worth looking into if we should use either effect here. Then we can propagate the GenericError thingy.

---

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

        CustomListLocationsSideEffect

    data object Error : CustomListLocationsSideEffect

Not sure if we have other errors that occur or if it is enough with the generic error we already have for CustomListActionResultData.

---

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

    // data class LocationAddedToCustomList(val result: LocationsChanged) : SelectLocationSideEffect

    // class LocationRemovedFromCustomList(val result: LocationsChanged) : SelectLocationSideEffect

We should remove these if they are not to be used.

[Pururun]

Reviewable status: 16 of 24 files reviewed, 6 unresolved discussions (waiting on @Rawa)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/communication/CustomListActionResultData.kt line 8 at r5 (raw file):

Previously, Rawa (David Göransson) wrote…

Introduce "Success" to split Error and Success in the result type.

Done.

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/SelectLocationScreen.kt line 849 at r5 (raw file):

Previously, Rawa (David Göransson) wrote…

Should we have a separate string? Now it will say the same thing as if you added a item to a list.
E.g
Added 'Stockholm' to 'my_list' doesn't really say list got created.
maybe should say along the lines of:
Created list 'my_list' with location 'Stockholm'

I agree, however I am not sure that I want to change existing copy with this PR. Maybe create a linear issue for this?

---

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

Previously, Rawa (David Göransson) wrote…

Not sure if we have other errors that occur or if it is enough with the generic error we already have for CustomListActionResultData.

The difference is that one navigate back to the SelectLocationScreen and the other one does not and only show a toast on this screen. But I don't think they need to be different though, so will remove this.

[Pururun]

Reviewable status: 7 of 24 files reviewed, 6 unresolved discussions (waiting on @Rawa)

---

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

Previously, Rawa (David Göransson) wrote…

Maybe break out this logic into another function and return the CustomListActionResult?

Done.

---

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

Previously, Rawa (David Göransson) wrote…

Could be worth looking into if we should use either effect here. Then we can propagate the GenericError thingy.

Done.

[Pururun]

Reviewable status: 7 of 24 files reviewed, 6 unresolved discussions (waiting on @Rawa)

---

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

Previously, Rawa (David Göransson) wrote…

We should remove these if they are not to be used.

Done.

[Rawa]

Reviewed 1 of 8 files at r6, 10 of 14 files at r7, 8 of 8 files at r8, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/SelectLocationScreen.kt line 849 at r5 (raw file):

Previously, Pururun (Jonatan Rhodin) wrote…

I agree, however I am not sure that I want to change existing copy with this PR. Maybe create a linear issue for this?

Sounds good to me

[Rawa]

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