Add better Voucher Code validation

[Rawa]

---

This change is 

[linear]

DROID-923 Add voucher error message

[Rawa]

Reviewable status: 0 of 9 files reviewed, 2 unresolved discussions

---

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

        viewModelScope.launch {
            either {
                    val voucherCode =

There is two ways about this, right now we try and parse it as VoucherCode and if it returns AllDigit (which is not valid according to spec) we show the AccountNumber error message. Another way about it would be to try and parse it as an AccountNumber and if successful print the error message. What do you guys think?

---

android/lib/resource/src/main/res/values/strings.xml line 36 at r1 (raw file):

    <string name="invalid_voucher">Voucher code is invalid.</string>
    <string name="voucher_already_used">Voucher code has already been used.</string>
    <string name="voucher_is_account_number">Looks like you entered an account number</string>

Any ideas of what we want the error message to be?

[Rawa]

Reviewable status: 0 of 9 files reviewed, 3 unresolved discussions

---

android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/lib/model/VoucherCode.kt line 12 at r1 (raw file):

    companion object {
        // Parsing reference: docs/adr/0018-distinguish-voucher-codes-from-account-numbers.md
        fun fromString(value: String): Either<ParseVoucherCodeError, VoucherCode> = either {

We could make even stricter validation, e.g only certain characters are allowed or max length, but I opted against it if the schema is subject to change in the future. So what is implemented is the lower bounds. (min length, not only digits)

[Pururun]

Looks good! Just need to align on the error message.

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

---

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

Previously, Rawa (David Göransson) wrote…

There is two ways about this, right now we try and parse it as VoucherCode and if it returns AllDigit (which is not valid according to spec) we show the AccountNumber error message. Another way about it would be to try and parse it as an AccountNumber and if successful print the error message. What do you guys think?

In which cases would we fail to parse it as an account number but it would be only digits and over 16 characters? If it is too many? It feels fine to me as it is.

---

android/lib/resource/src/main/res/values/strings.xml line 36 at r1 (raw file):

Previously, Rawa (David Göransson) wrote…

Any ideas of what we want the error message to be?

I guess it should be based on what the normal use case is. Maybe something like "It looks like you entered an account number. If that is the case, log out and log in again using your account number" ?

[Rawa]

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

---

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

Previously, Pururun (Jonatan Rhodin) wrote…

In which cases would we fail to parse it as an account number but it would be only digits and over 16 characters? If it is too many? It feels fine to me as it is.

Well yeah it is a bit about interpretation of how when we logically should show the error.
A. When we are successfully parsing it as an account number
B. When we notice that the Voucher Code is consisting of only digits and is long enough.

Right now we have the B approach, we could technically miss cases were they enter a valid account number that is shorter than 16 characters and then would say it is too short rather than it is an account number. By doing the A approach we know it fulfills all our requirements for an AccountNumber (could be good and bad).

I agree, I think our current solution is fine for now, if it becomes a problem we can refine it later.

---

android/lib/resource/src/main/res/values/strings.xml line 36 at r1 (raw file):

Previously, Pururun (Jonatan Rhodin) wrote…

I guess it should be based on what the normal use case is. Maybe something like "It looks like you entered an account number. If that is the case, log out and log in again using your account number" ?

Yeah it is hard to tell. Also "Looks like you entered an account number" doesn't sound like an error message. Maybe it should say, "Voucher code is invalid, looks like you enter an account number, if you wish to change account please logout first", but it would be quite a long error message 🙈 . Maybe @albin-mullvad has some input?

[Rawa]

Reviewable status: 5 of 10 files reviewed, 2 unresolved discussions (waiting on @albin-mullvad and @Pururun)

---

android/lib/resource/src/main/res/values/strings.xml line 36 at r1 (raw file):

Previously, Rawa (David Göransson) wrote…

Yeah it is hard to tell. Also "Looks like you entered an account number" doesn't sound like an error message. Maybe it should say, "Voucher code is invalid, looks like you enter an account number, if you wish to change account please logout first", but it would be quite a long error message 🙈 . Maybe @albin-mullvad has some input?

Talked with matilda, strings have been aligned.

[Pururun]

Reviewed 1 of 1 files at r2, 4 of 4 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion

[albin-mullvad]

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

---

android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/lib/model/VoucherCode.kt line 11 at r3 (raw file):

    companion object {
        // Parsing reference: docs/adr/0018-distinguish-voucher-codes-from-account-numbers.md

Change to <services-repository>/services/docs/adr/...

Code quote:

docs/adr/0018-distinguish-voucher-codes-from-account-numbers.md

---

android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/lib/model/VoucherCode.kt line 17 at r3 (raw file):

                ParseVoucherCodeError.TooShort(trimmedValue)
            }
            ensure(!value.all { it.isDigit() }) { ParseVoucherCodeError.AllDigit(trimmedValue) }

Is this already guaranteed. E.g. for previously generated vouchers

Code quote:

!value.all { it.isDigit() }

---

android/lib/resource/src/main/res/values/strings.xml line 36 at r1 (raw file):

Previously, Rawa (David Göransson) wrote…

Talked with matilda, strings have been aligned.

Sounds good 👍

---

android/lib/resource/src/main/res/values/strings.xml line 36 at r3 (raw file):

    <string name="invalid_voucher">Voucher code is invalid.</string>
    <string name="voucher_already_used">Voucher code has already been used.</string>
    <string name="voucher_is_account_number">It looks like you\’ve entered an account number instead of a voucher code. If you would like to change the active account, please log out first.</string>

Do we need to backslash the apostrophe? 🤔 In that case we might want to plan aligning the other strings

Code quote:

you\’ve

[Pururun]

Reviewed 3 of 3 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion

---

android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/lib/model/VoucherCode.kt line 11 at r3 (raw file):

Previously, albin-mullvad wrote…

Change to <services-repository>/services/docs/adr/...

Done

---

android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/lib/model/VoucherCode.kt line 17 at r3 (raw file):

Previously, albin-mullvad wrote…

Is this already guaranteed. E.g. for previously generated vouchers

According to our information there are no vouchers with only numbers.

---

android/lib/resource/src/main/res/values/strings.xml line 36 at r3 (raw file):

Previously, albin-mullvad wrote…

Do we need to backslash the apostrophe? 🤔 In that case we might want to plan aligning the other strings

Not needed, removed.

[albin-mullvad]

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

---

android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/lib/model/VoucherCode.kt line 12 at r1 (raw file):

Previously, Rawa (David Göransson) wrote…

We could make even stricter validation, e.g only certain characters are allowed or max length, but I opted against it if the schema is subject to change in the future. So what is implemented is the lower bounds. (min length, not only digits)

Sounds good 👍

[albin-mullvad]

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

[albin-mullvad]

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

[albin-mullvad]

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