Translate location names

[Rawa]

Adds support for translations for different locations

---

This change is 

[linear]

DROID-432 Translate location names

[Pururun]

Reviewed 29 of 34 files at r1, 4 of 4 files at r2, all commit messages.
Reviewable status: 33 of 34 files reviewed, 1 unresolved discussion (waiting on @Rawa)

---

android/lib/resource/src/main/res/xml/relay_locations.xml line 152 at r2 (raw file):

    <string name="Valencia">Valencia</string>
    <string name="Vancouver">Vancouver</string>
    <string name="Vienna">Vienna</string>

Not sure if this is an error in the source data or in the translation script, but Vienna and Wien appear both, which is a bit weird.

[Rawa]

Reviewable status: 7 of 34 files reviewed, all discussions resolved (waiting on @Pururun)

---

android/lib/resource/src/main/res/xml/relay_locations.xml line 152 at r2 (raw file):

Previously, Pururun (Jonatan Rhodin) wrote…

Not sure if this is an error in the source data or in the translation script, but Vienna and Wien appear both, which is a bit weird.

The relay-locations.pot includes both, I've created DES-1122 to address this.

[Pururun]

We should also sort the relay items based on the translated name. Currently they are sorted based on the english name so for example "Österrike" / "Österreich" will be close to the tope of the list even though it should be at the bottom.

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

---

android/app/src/main/AndroidManifest.xml line 3 at r1 (raw file):

<manifest xmlns:android="http://schemas.android.com/apk/res/android"
        xmlns:tools="http://schemas.android.com/tools">

I think we should not have a lot of formatting in this file?

[Pururun]

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

---

android/lib/shared/src/main/kotlin/net/mullvad/mullvadvpn/lib/shared/RelayLocationTranslationRepository.kt line 53 at r3 (raw file):

                val translation = loadRelayTranslation(xml)

                translation.entries.associate { (id, name) -> defaultTranslation[id]!! to name }

Is there a way to get around this? I get that we have it because the keys of the default locale does not always match the string we get from the daemon, but it would be nice if did not need this step, feels like it is open for errors and crashes.

[Rawa]

Nice catch! Fixed!

Reviewed 1 of 2 files at r4.
Reviewable status: 32 of 35 files reviewed, 1 unresolved discussion (waiting on @Pururun)

---

android/app/src/main/AndroidManifest.xml line 3 at r1 (raw file):

Previously, Pururun (Jonatan Rhodin) wrote…

I think we should not have a lot of formatting in this file?

Done.

---

android/lib/shared/src/main/kotlin/net/mullvad/mullvadvpn/lib/shared/RelayLocationTranslationRepository.kt line 53 at r3 (raw file):

Previously, Pururun (Jonatan Rhodin) wrote…

Is there a way to get around this? I get that we have it because the keys of the default locale does not always match the string we get from the daemon, but it would be nice if did not need this step, feels like it is open for errors and crashes.

Talked and address, thanks!

[Pururun]

Reviewed 2 of 2 files at r5, all commit messages.
Reviewable status: 34 of 35 files reviewed, all discussions resolved (waiting on @Rawa)

[albin-mullvad]

Reviewed 1 of 34 files at r1, 1 of 4 files at r2, 6 of 29 files at r3, 1 of 2 files at r4, 2 of 2 files at r5, 2 of 2 files at r6, 2 of 2 files at r7, all commit messages.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @Rawa)

---

-- commits line 7 at r6:
nit: change to:

Add support for translated location names

Code quote:

Add support location translations

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/broadcastreceiver/LocaleChangedBroadcastReceiver.kt line 1 at r7 (raw file):

package net.mullvad.mullvadvpn.broadcastreceiver

nit: I suggest changing to just receiver since it's shorter and easier to read

Code quote:

broadcastreceiver

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/repository/RelayListRepository.kt line 43 at r7 (raw file):

            .stateIn(CoroutineScope(dispatcher), SharingStarted.WhileSubscribed(), emptyList())

    private fun List<RelayItem.Location.Country>.translateRelay(

I suggest changing to translateRelays or translateAllContainingLocationsAndRelays depending on how verbose we want to be

Code quote:

translateRelay

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/util/StringExtensions.kt line 18 at r7 (raw file):

fun List<String>.trimAll() = map { it.trim() }

fun <T> List<T>.sortedByName(comparator: (T) -> String) =

I believe this function name is a bit too specific for such a generic file (StringExtensions.kt). Maybe the function belongs somewhere else or can be renamed?

Code quote:

sortedByName

---

android/lib/shared/src/main/kotlin/net/mullvad/mullvadvpn/lib/shared/RelayLocationTranslationRepository.kt line 32 at r7 (raw file):

            .stateIn(externalScope, SharingStarted.Eagerly, emptyMap())

    private suspend fun loadTranslations(locale: Locale?): Translations =

Isn't it better the caller handle the nullable case?

Code quote:

Locale?

---

android/lib/shared/src/main/kotlin/net/mullvad/mullvadvpn/lib/shared/RelayLocationTranslationRepository.kt line 34 at r7 (raw file):

    private suspend fun loadTranslations(locale: Locale?): Translations =
        withContext(dispatcher) {
            Logger.d("Updating translations based $locale")

typo?

Code quote:

Updating translations based $locale

---

android/lib/shared/src/main/kotlin/net/mullvad/mullvadvpn/lib/shared/RelayLocationTranslationRepository.kt line 43 at r7 (raw file):

        }

    private fun loadRelayTranslation(xml: XmlResourceParser): Map<String, String> {

Seems like this can be made a bit more readable by letting this function be an extension function for XmlResourceParser?

[Pururun]

Reviewable status: 35 of 36 files reviewed, 6 unresolved discussions (waiting on @albin-mullvad and @Rawa)

---

-- commits line 7 at r6:

Previously, albin-mullvad wrote…

nit: change to:

Add support for translated location names

Done

[Pururun]

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

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/broadcastreceiver/LocaleChangedBroadcastReceiver.kt line 1 at r7 (raw file):

Previously, albin-mullvad wrote…

nit: I suggest changing to just receiver since it's shorter and easier to read

That is also what we use for the bootreceiver in the autostart branch so that is a good change for that reason as well.

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/repository/RelayListRepository.kt line 43 at r7 (raw file):

Previously, albin-mullvad wrote…

I suggest changing to translateRelays or translateAllContainingLocationsAndRelays depending on how verbose we want to be

Changed it to translateRelays

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/util/StringExtensions.kt line 18 at r7 (raw file):

Previously, albin-mullvad wrote…

I believe this function name is a bit too specific for such a generic file (StringExtensions.kt). Maybe the function belongs somewhere else or can be renamed?

We named it as such since that is the expected behaviour when sorting something by name.

Since it is only used for relay list I moved it to RelayListExtensions and made it more specific.

---

android/lib/shared/src/main/kotlin/net/mullvad/mullvadvpn/lib/shared/RelayLocationTranslationRepository.kt line 32 at r7 (raw file):

Previously, albin-mullvad wrote…

Isn't it better the caller handle the nullable case?

Done

---

android/lib/shared/src/main/kotlin/net/mullvad/mullvadvpn/lib/shared/RelayLocationTranslationRepository.kt line 34 at r7 (raw file):

Previously, albin-mullvad wrote…

typo?

Done

---

android/lib/shared/src/main/kotlin/net/mullvad/mullvadvpn/lib/shared/RelayLocationTranslationRepository.kt line 43 at r7 (raw file):

Previously, albin-mullvad wrote…

Seems like this can be made a bit more readable by letting this function be an extension function for XmlResourceParser?

Done

[albin-mullvad]

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

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/broadcastreceiver/LocaleChangedBroadcastReceiver.kt line 1 at r7 (raw file):

Previously, Pururun (Jonatan Rhodin) wrote…

That is also what we use for the bootreceiver in the autostart branch so that is a good change for that reason as well.

👍

[albin-mullvad]

I suggest asking for someone with better rust knowledge to review the translation-converter. The rest looks good!

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

[MarkusPettersson98]

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

---

android/translations-converter/src/main.rs line 230 at r11 (raw file):

        .expect("Failed to open root locale directory")
        .filter_map(|dir_entry_result| dir_entry_result.ok().map(|dir_entry| dir_entry.path()))
        .filter(|dir_entry_path| dir_entry_path.is_dir())

Nit This could be split up further by introducing yet another map

        .filter_map(|dir_entry_result| dir_entry_result.ok())
        .map(|dir_entry| dir_entry.path())
        .filter(|dir_entry_path| dir_entry_path.is_dir())

Code quote:

        .filter_map(|dir_entry_result| dir_entry_result.ok().map(|dir_entry| dir_entry.path()))
        .filter(|dir_entry_path| dir_entry_path.is_dir())

---

android/translations-converter/src/main.rs line 278 at r11 (raw file):

/// `values-en-rUS`.
fn android_xml_directory(locale: &str) -> String {
    let mut directory = String::from("xml-");

Nit It seems to me like the comment doesn't actually line up with the implementation. I think the comment should say ".. gets correctly mapped to the directory name xml-en-rUS, right? 😊

Code quote:

/// This just makes sure a locale such as `en-US' gets correctly mapped to the directory name
/// `values-en-rUS`.
fn android_xml_directory(locale: &str) -> String {
    let mut directory = String::from("xml-");

[Pururun]

Reviewable status: 36 of 37 files reviewed, all discussions resolved

---

android/translations-converter/src/main.rs line 230 at r11 (raw file):

Previously, MarkusPettersson98 (Markus Pettersson) wrote…

Nit This could be split up further by introducing yet another map

        .filter_map(|dir_entry_result| dir_entry_result.ok())
        .map(|dir_entry| dir_entry.path())
        .filter(|dir_entry_path| dir_entry_path.is_dir())

Done

---

android/translations-converter/src/main.rs line 278 at r11 (raw file):

Previously, MarkusPettersson98 (Markus Pettersson) wrote…

Nit It seems to me like the comment doesn't actually line up with the implementation. I think the comment should say ".. gets correctly mapped to the directory name xml-en-rUS, right? 😊

Yes I think that it was values-en-rUS first and then it as changed to xml-en-rUS later but the comment was not updated. Good find!

[MarkusPettersson98]

Reviewable status: 36 of 37 files reviewed, all discussions resolved (waiting on @Pururun)

[albin-mullvad]

Reviewed all commit messages.
Reviewable status: 36 of 37 files reviewed, all discussions resolved (waiting on @Pururun)

[Pururun]

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