Merge branch 'add-voucher-error-message-droid-923'

Author: albin-mullvad

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/RedeemVoucherDialog.kt

@@ -274,10 +274,23 @@ private fun EnterVoucherBody(
             )
         }
     }
+    if (
+        state.voucherState is VoucherDialogState.Error &&
+            state.voucherState.error is RedeemVoucherError.EnteredAccountNumber
+    ) {
+        Text(
+            modifier = Modifier.padding(top = Dimens.smallPadding),
+            text = stringResource(id = R.string.voucher_is_account_number),
+            color = MaterialTheme.colorScheme.onPrimary,
+            style = MaterialTheme.typography.bodySmall
+        )
+    }
 }
 
 private fun RedeemVoucherError.message(): Int =
     when (this) {
+        RedeemVoucherError.TooShortVoucher,
+        RedeemVoucherError.EnteredAccountNumber,
         RedeemVoucherError.InvalidVoucher -> R.string.invalid_voucher
         RedeemVoucherError.VoucherAlreadyUsed -> R.string.voucher_already_used
         RedeemVoucherError.RpcError,

android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/VoucherDialogViewModel.kt

@@ -2,6 +2,7 @@ package net.mullvad.mullvadvpn.viewmodel
 
 import androidx.lifecycle.ViewModel
 import androidx.lifecycle.viewModelScope
+import arrow.core.raise.either
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.SharingStarted
 import kotlinx.coroutines.flow.combine
@@ -11,7 +12,9 @@ import kotlinx.coroutines.launch
 import net.mullvad.mullvadvpn.compose.state.VoucherDialogState
 import net.mullvad.mullvadvpn.compose.state.VoucherDialogUiState
 import net.mullvad.mullvadvpn.constant.VOUCHER_LENGTH
+import net.mullvad.mullvadvpn.lib.model.ParseVoucherCodeError
 import net.mullvad.mullvadvpn.lib.model.RedeemVoucherError
+import net.mullvad.mullvadvpn.lib.model.VoucherCode
 import net.mullvad.mullvadvpn.lib.shared.VoucherRepository
 import net.mullvad.mullvadvpn.util.VoucherRegexHelper
 
@@ -26,11 +29,23 @@ class VoucherDialogViewModel(private val voucherRepository: VoucherRepository) :
             }
             .stateIn(viewModelScope, SharingStarted.WhileSubscribed(), VoucherDialogUiState.INITIAL)
 
-    fun onRedeem(voucherCode: String) {
+    fun onRedeem(voucherInput: String) {
         vmState.update { VoucherDialogState.Verifying }
         viewModelScope.launch {
-            voucherRepository
-                .submitVoucher(voucherCode)
+            either {
+                    val voucherCode =
+                        VoucherCode.fromString(voucherInput)
+                            .mapLeft {
+                                when (it) {
+                                    is ParseVoucherCodeError.AllDigit ->
+                                        RedeemVoucherError.EnteredAccountNumber
+                                    is ParseVoucherCodeError.TooShort ->
+                                        RedeemVoucherError.TooShortVoucher
+                                }
+                            }
+                            .bind()
+                    voucherRepository.submitVoucher(voucherCode).bind()
+                }
                 .fold(
                     { error -> setError(error) },
                     { success -> handleAddedTime(success.timeAdded) }

android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/VoucherDialogViewModelTest.kt

@@ -16,6 +16,7 @@ import net.mullvad.mullvadvpn.compose.state.VoucherDialogState
 import net.mullvad.mullvadvpn.lib.common.test.TestCoroutineRule
 import net.mullvad.mullvadvpn.lib.model.RedeemVoucherError
 import net.mullvad.mullvadvpn.lib.model.RedeemVoucherSuccess
+import net.mullvad.mullvadvpn.lib.model.VoucherCode
 import net.mullvad.mullvadvpn.lib.shared.VoucherRepository
 import org.joda.time.DateTime
 import org.junit.jupiter.api.AfterEach
@@ -45,19 +46,20 @@ class VoucherDialogViewModelTest {
     @Test
     fun `ensure onRedeem invokes submit on VoucherRedeemer with same voucher code`() = runTest {
         val voucher = DUMMY_INVALID_VOUCHER
+        val parsedVoucher = VoucherCode.fromString(voucher).getOrNull()!!
 
         // Arrange
         val timeAdded = 0L
         val newExpiry = DateTime()
-        coEvery { mockVoucherRepository.submitVoucher(voucher) } returns
+        coEvery { mockVoucherRepository.submitVoucher(parsedVoucher) } returns
             RedeemVoucherSuccess(timeAdded, newExpiry).right()
 
         // Act
         assertIs<VoucherDialogState.Default>(viewModel.uiState.value.voucherState)
         viewModel.onRedeem(voucher)
 
         // Assert
-        coVerify(exactly = 1) { mockVoucherRepository.submitVoucher(voucher) }
+        coVerify(exactly = 1) { mockVoucherRepository.submitVoucher(parsedVoucher) }
     }
 
     @Test
@@ -66,8 +68,9 @@ class VoucherDialogViewModelTest {
 
         // Arrange
         every { mockVoucherSubmission.timeAdded } returns 0
-        coEvery { mockVoucherRepository.submitVoucher(voucher) } returns
-            RedeemVoucherError.InvalidVoucher.left()
+        coEvery {
+            mockVoucherRepository.submitVoucher(VoucherCode.fromString(voucher).getOrNull()!!)
+        } returns RedeemVoucherError.InvalidVoucher.left()
 
         // Act, Assert
         viewModel.uiState.test {
@@ -84,8 +87,9 @@ class VoucherDialogViewModelTest {
 
         // Arrange
         every { mockVoucherSubmission.timeAdded } returns 0
-        coEvery { mockVoucherRepository.submitVoucher(voucher) } returns
-            RedeemVoucherSuccess(0, DateTime()).right()
+        coEvery {
+            mockVoucherRepository.submitVoucher(VoucherCode.fromString(voucher).getOrNull()!!)
+        } returns RedeemVoucherSuccess(0, DateTime()).right()
 
         // Act, Assert
         viewModel.uiState.test {
@@ -102,8 +106,9 @@ class VoucherDialogViewModelTest {
 
         // Arrange
         every { mockVoucherSubmission.timeAdded } returns 0
-        coEvery { mockVoucherRepository.submitVoucher(voucher) } returns
-            RedeemVoucherError.VoucherAlreadyUsed.left()
+        coEvery {
+            mockVoucherRepository.submitVoucher(VoucherCode.fromString(voucher).getOrNull()!!)
+        } returns RedeemVoucherError.VoucherAlreadyUsed.left()
 
         // Act, Assert
         viewModel.uiState.test {

android/lib/daemon-grpc/src/main/kotlin/net/mullvad/mullvadvpn/lib/daemon/grpc/ManagementService.kt

@@ -116,6 +116,7 @@ import net.mullvad.mullvadvpn.lib.model.UnknownApiAccessMethodError
 import net.mullvad.mullvadvpn.lib.model.UnknownCustomListError
 import net.mullvad.mullvadvpn.lib.model.UpdateApiAccessMethodError
 import net.mullvad.mullvadvpn.lib.model.UpdateCustomListError
+import net.mullvad.mullvadvpn.lib.model.VoucherCode
 import net.mullvad.mullvadvpn.lib.model.WebsiteAuthToken
 import net.mullvad.mullvadvpn.lib.model.WireguardConstraints as ModelWireguardConstraints
 import net.mullvad.mullvadvpn.lib.model.WireguardEndpointData as ModelWireguardEndpointData
@@ -575,8 +576,10 @@ class ManagementService(
             .mapLeft(SetWireguardConstraintsError::Unknown)
             .mapEmpty()
 
-    suspend fun submitVoucher(voucher: String): Either<RedeemVoucherError, RedeemVoucherSuccess> =
-        Either.catch { grpc.submitVoucher(StringValue.of(voucher)).toDomain() }
+    suspend fun submitVoucher(
+        voucher: VoucherCode
+    ): Either<RedeemVoucherError, RedeemVoucherSuccess> =
+        Either.catch { grpc.submitVoucher(StringValue.of(voucher.value)).toDomain() }
             .mapLeftStatus {
                 when (it.status.code) {
                     Status.Code.INVALID_ARGUMENT,

android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/lib/model/RedeemVoucherError.kt

@@ -5,6 +5,10 @@ sealed class RedeemVoucherError {
 
     data object VoucherAlreadyUsed : RedeemVoucherError()
 
+    data object TooShortVoucher : RedeemVoucherError()
+
+    data object EnteredAccountNumber : RedeemVoucherError()
+
     data object RpcError : RedeemVoucherError()
 
     data class Unknown(val error: Throwable) : RedeemVoucherError()

android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/lib/model/VoucherCode.kt

@@ -0,0 +1,31 @@
+package net.mullvad.mullvadvpn.lib.model
+
+import arrow.core.Either
+import arrow.core.raise.either
+import arrow.core.raise.ensure
+
+@JvmInline
+value class VoucherCode private constructor(val value: String) {
+
+    companion object {
+        // Parsing reference:
+        // <services-repository>/services/docs/adr/0018-distinguish-voucher-codes-from-account-numbers.md
+        fun fromString(value: String): Either<ParseVoucherCodeError, VoucherCode> = either {
+            val trimmedValue = value.trim()
+            ensure(trimmedValue.length >= MIN_VOUCHER_LENGTH) {
+                ParseVoucherCodeError.TooShort(trimmedValue)
+            }
+            ensure(!value.all { it.isDigit() }) { ParseVoucherCodeError.AllDigit(trimmedValue) }
+            VoucherCode(trimmedValue)
+        }
+
+        const val MIN_VOUCHER_LENGTH = 16
+    }
+}
+
+sealed interface ParseVoucherCodeError {
+
+    data class AllDigit(val value: String) : ParseVoucherCodeError
+
+    data class TooShort(val value: String) : ParseVoucherCodeError
+}

android/lib/model/src/test/kotlin/net/mullvad/mullvadvpn/lib/model/VoucherCodeTest.kt

@@ -0,0 +1,34 @@
+package net.mullvad.mullvadvpn.lib.model
+
+import kotlin.test.assertEquals
+import kotlin.test.assertTrue
+import org.junit.jupiter.api.Test
+
+class VoucherCodeTest {
+    @Test
+    fun `parsing a too short voucher code should return TooShort`() {
+        val input = "mycode"
+        val result = VoucherCode.fromString(input)
+
+        assertTrue(result.isLeft())
+        assertEquals(ParseVoucherCodeError.TooShort(input), result.leftOrNull())
+    }
+
+    @Test
+    fun `numbers only should not be allowed`() {
+        val input = "1234123412341234"
+        val result = VoucherCode.fromString(input)
+
+        assertTrue(result.isLeft())
+        assertEquals(ParseVoucherCodeError.AllDigit(input), result.leftOrNull())
+    }
+
+    @Test
+    fun `number only input when too short should return TooShort`() {
+        val input = "123412341234"
+        val result = VoucherCode.fromString(input)
+
+        assertTrue(result.isLeft())
+        assertEquals(ParseVoucherCodeError.TooShort(input), result.leftOrNull())
+    }
+}

android/lib/resource/src/main/res/values/strings.xml

@@ -33,6 +33,7 @@
     <string name="redeem">Redeem</string>
     <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>
     <string name="error_occurred">An error occurred.</string>
     <string name="settings">Settings</string>
     <string name="no_internet_connection">No internet connection</string>

android/lib/shared/src/main/kotlin/net/mullvad/mullvadvpn/lib/shared/VoucherRepository.kt

@@ -1,12 +1,13 @@
 package net.mullvad.mullvadvpn.lib.shared
 
 import net.mullvad.mullvadvpn.lib.daemon.grpc.ManagementService
+import net.mullvad.mullvadvpn.lib.model.VoucherCode
 
 class VoucherRepository(
     private val managementService: ManagementService,
     private val accountRepository: AccountRepository
 ) {
-    suspend fun submitVoucher(voucher: String) =
+    suspend fun submitVoucher(voucher: VoucherCode) =
         managementService.submitVoucher(voucher).onRight {
             accountRepository.onVoucherRedeemed(it.newExpiryDate)
         }

gui/locales/messages.pot

@@ -2318,6 +2318,9 @@ msgstr ""
 msgid "Invalid or missing value \"%s\""
 msgstr ""
 
+msgid "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."
+msgstr ""
+
 msgid "Lets you select apps that should access the Internet directly without going through the VPN tunnel."
 msgstr ""