From 41142a3d4d3a003e1d43e44e7f209e5c896dc8bb Mon Sep 17 00:00:00 2001 From: Colin Rinke Date: Tue, 28 Apr 2026 20:55:13 +0200 Subject: [PATCH] [PM-35352] [PM-21264] Group card numbers in vault item display (#6810) --- .../vault/feature/item/VaultItemViewModel.kt | 3 +- .../feature/item/util/CipherViewExtensions.kt | 3 +- .../ui/vault/util/CardNumberUtils.kt | 61 +++++++++++++++ .../feature/item/VaultItemViewModelTest.kt | 15 +++- .../item/util/CipherViewExtensionsTest.kt | 33 ++++++++ .../ui/vault/util/CardNumberUtilsTest.kt | 75 +++++++++++++++++++ 6 files changed, 187 insertions(+), 3 deletions(-) diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModel.kt b/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModel.kt index 71f055a319..e89707a5bc 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModel.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModel.kt @@ -828,8 +828,9 @@ class VaultItemViewModel @Inject constructor( private fun handleCopyNumberClick() { onCardContent { _, card -> + val cardNumber = requireNotNull(card.number).number clipboardManager.setText( - text = requireNotNull(card.number).number, + text = cardNumber.filter { it.isDigit() }, toastDescriptorOverride = BitwardenString.number.asText(), ) } diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/item/util/CipherViewExtensions.kt b/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/item/util/CipherViewExtensions.kt index 0e56612dc8..043e5a9644 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/item/util/CipherViewExtensions.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/item/util/CipherViewExtensions.kt @@ -27,6 +27,7 @@ import com.x8bit.bitwarden.ui.vault.feature.vault.util.toLoginIconData import com.x8bit.bitwarden.ui.vault.model.VaultCardBrand import com.x8bit.bitwarden.ui.vault.model.VaultLinkedFieldType import com.x8bit.bitwarden.ui.vault.model.findVaultCardBrandWithNameOrNull +import com.x8bit.bitwarden.ui.vault.util.formatCardNumber import kotlinx.collections.immutable.ImmutableList import kotlinx.collections.immutable.toImmutableList import java.time.Clock @@ -162,7 +163,7 @@ fun CipherView.toViewState( cardholderName = card?.cardholderName, number = card?.number?.let { VaultItemState.ViewState.Content.ItemType.Card.NumberData( - number = it, + number = it.formatCardNumber(), isVisible = (previousState?.type as? VaultItemState.ViewState.Content.ItemType.Card) ?.number diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/util/CardNumberUtils.kt b/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/util/CardNumberUtils.kt index f61997f33e..860eb2a412 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/util/CardNumberUtils.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/util/CardNumberUtils.kt @@ -1,8 +1,69 @@ +@file:Suppress("TooManyFunctions") + package com.x8bit.bitwarden.ui.vault.util import com.bitwarden.ui.platform.feature.cardscanner.util.sanitizeCardNumber import com.x8bit.bitwarden.ui.vault.model.VaultCardBrand +/** + * Formats a card number using brand-specific spacing rules. + * + * The input string is first sanitized to remove non-digit characters, then the card brand is + * detected based on the digit patterns. Finally, the digits are grouped into blocks according to + * the brand's formatting rules, and spaces are inserted between the blocks for improved + * readability. + * + * @return The formatted card number. + */ +fun String.formatCardNumber(): String { + val digits = sanitizeCardNumber() + if (digits.isEmpty()) return this + val blocks = digits.detectCardBrand().formattingBlocks(digitCount = digits.length) + return digits.chunkByBlocks(blocks).joinToString(separator = " ") +} + +/** + * Returns the digit group sizes used to format a card number for a specific brand. + * + * @param digitCount The total number of sanitized digits available for formatting. + * @return A list of block sizes that defines how the card number should be grouped. + */ +@Suppress("MagicNumber") +private fun VaultCardBrand.formattingBlocks(digitCount: Int): List { + val default = listOf(4, 4, 4, 4) + return when (this) { + VaultCardBrand.AMEX -> listOf(4, 6, 5) + VaultCardBrand.DINERS_CLUB -> if (digitCount == 14) listOf(4, 6, 4) else default + VaultCardBrand.MAESTRO -> when (digitCount) { + 13 -> listOf(4, 4, 5) + 15 -> listOf(4, 6, 5) + 19 -> listOf(4, 4, 4, 4, 3) + else -> default + } + VaultCardBrand.UNIONPAY -> if (digitCount == 19) listOf(6, 13) else default + else -> default + } +} + +/** + * Splits the string into blocks of specified sizes. + * + * If the total of the block sizes is less than the string length, the remaining characters are + * included as an additional block at the end of the list. + * + * @param blocks A list of integers specifying the size of each block. + * @return A list of string blocks based on the specified sizes. + */ +private fun String.chunkByBlocks(blocks: List): List = buildList { + var remaining = this@chunkByBlocks + for (size in blocks) { + if (remaining.isEmpty()) return@buildList + add(remaining.take(size)) + remaining = remaining.drop(size) + } + if (remaining.isNotEmpty()) add(remaining) +} + /** * Detects the card brand based on the card number prefix. * diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModelTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModelTest.kt index 840cbe8e04..fe2c456ecd 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModelTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModelTest.kt @@ -2102,6 +2102,17 @@ class VaultItemViewModelTest : BaseViewModelTest() { @Test fun `on CopyNumberClick should call setText on the ClipboardManager`() = runTest { + val cardTypeWithFormattedNumber = DEFAULT_CARD_TYPE.copy( + number = VaultItemState.ViewState.Content.ItemType.Card.NumberData( + number = "1234 5436", + isVisible = false, + ), + ) + viewModel = createViewModel( + state = DEFAULT_STATE.copy( + viewState = createViewState(type = cardTypeWithFormattedNumber), + ), + ) every { mockCipherView.toViewState( previousState = null, @@ -2116,7 +2127,7 @@ class VaultItemViewModelTest : BaseViewModelTest() { relatedLocations = persistentListOf(), hasOrganizations = true, ) - } returns createViewState(type = DEFAULT_CARD_TYPE) + } returns createViewState(type = cardTypeWithFormattedNumber) mutableVaultItemFlow.value = DataState.Loaded(data = mockCipherView) mutableAuthCodeItemFlow.value = DataState.Loaded(data = null) mutableCollectionsStateFlow.value = DataState.Loaded(emptyList()) @@ -2129,6 +2140,8 @@ class VaultItemViewModelTest : BaseViewModelTest() { text = "12345436", toastDescriptorOverride = BitwardenString.number.asText(), ) + } + verify(atLeast = 1) { mockCipherView.toViewState( previousState = null, isPremiumUser = true, diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/item/util/CipherViewExtensionsTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/item/util/CipherViewExtensionsTest.kt index a88c420003..98bc9872e0 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/item/util/CipherViewExtensionsTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/item/util/CipherViewExtensionsTest.kt @@ -513,6 +513,39 @@ class CipherViewExtensionsTest { } } + @Test + fun `toViewState should format card number when transforming card content`() { + val cipherView = createCipherView(type = CipherType.CARD, isEmpty = false) + .copy( + card = createMockCardView( + number = 1, + cardNumber = "4111111111111111", + brand = VaultCardBrand.VISA.name, + ), + ) + val viewState = cipherView.toViewState( + previousState = null, + isPremiumUser = true, + totpCodeItemData = null, + clock = fixedClock, + canDelete = true, + canRestore = true, + canAssignToCollections = true, + canEdit = true, + baseIconUrl = "https://example.com/", + isIconLoadingDisabled = true, + relatedLocations = persistentListOf(), + hasOrganizations = true, + ) + + assertEquals( + "4111 1111 1111 1111", + (viewState.asContentOrNull()?.type as? VaultItemState.ViewState.Content.ItemType.Card) + ?.number + ?.number, + ) + } + private fun setupMockUri() { mockkStatic(Uri::class) val uriMock = mockk() diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/util/CardNumberUtilsTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/util/CardNumberUtilsTest.kt index b828338f32..299bdd7958 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/util/CardNumberUtilsTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/util/CardNumberUtilsTest.kt @@ -104,4 +104,79 @@ class CardNumberUtilsTest { ) assertEquals(VaultCardBrand.OTHER, "".detectCardBrand()) } + + @Test + fun `formatCardNumber should format Amex correctly`() { + assertEquals("3782 822463 10005", "378282246310005".formatCardNumber()) + assertEquals("3411 111111 11111", "341111111111111".formatCardNumber()) + } + + @Test + fun `formatCardNumber should format Diners Club 14 digits correctly`() { + assertEquals("3056 930902 5904", "30569309025904".formatCardNumber()) + } + + @Test + fun `formatCardNumber should format Diners Club with non 14 digits as default`() { + assertEquals("3600 0000 0000 0084", "3600000000000084".formatCardNumber()) + } + + @Test + fun `formatCardNumber should format Maestro 13 digits correctly`() { + assertEquals("5018 5753 94858", "5018575394858".formatCardNumber()) + } + + @Test + fun `formatCardNumber should format Maestro 15 digits correctly`() { + assertEquals("5018 575394 85843", "501857539485843".formatCardNumber()) + } + + @Test + fun `formatCardNumber should format Maestro 19 digits correctly`() { + assertEquals("5018 5753 9485 8437 306", "5018575394858437306".formatCardNumber()) + } + + @Test + fun `formatCardNumber should format Maestro other digits as default`() { + assertEquals("5018 5753 9485 8437", "5018575394858437".formatCardNumber()) + } + + @Test + fun `formatCardNumber should format UnionPay 19 digits correctly`() { + assertEquals("622795 5237950556428", "6227955237950556428".formatCardNumber()) + } + + @Test + fun `formatCardNumber should format UnionPay with non 19 digits as default`() { + assertEquals("6227 9552 3795 0556", "6227955237950556".formatCardNumber()) + } + + @Test + fun `formatCardNumber should format standard card numbers in groups of 4`() { + assertEquals("4111 1111 1111 1111", "4111111111111111".formatCardNumber()) + assertEquals("5500 0000 0000 0004", "5500000000000004".formatCardNumber()) + assertEquals("6011 1111 1111 1117", "6011111111111117".formatCardNumber()) + } + + @Test + fun `formatCardNumber should format card numbers in 4 groups of 4 and append remainder`() { + assertEquals("4111 1111 1111 1111 1", "41111111111111111".formatCardNumber()) + assertEquals("5500 0000 0000 0004 0000", "55000000000000040000".formatCardNumber()) + assertEquals("6011 1111 1111 1117 11111", "601111111111111711111".formatCardNumber()) + } + + @Test + fun `formatCardNumber should keep short standard card numbers unchanged`() { + assertEquals("123", "123".formatCardNumber()) + } + + @Test + fun `formatCardNumber should return empty string for empty input`() { + assertEquals("", "".formatCardNumber()) + } + + @Test + fun `formatCardNumber should sanitize and return empty when input has no digits`() { + assertEquals("----", "----".formatCardNumber()) + } }