From a517807f5881d94fe0b7b86896eafc0f966a8d0d Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Mon, 1 Jun 2026 12:51:26 -0400 Subject: [PATCH] fix(contacts): centralize e164 normalization via PhoneUtils.toE164 The inline normalizeToE164 in FullAccessContactReader and PickerContactReader used a fragile cleanNumber pipeline that could produce non-E.164 strings for certain country formats. Replace both with a single PhoneUtils.toE164 that delegates to libphonenumber parse/isValidNumber/format(E164) pipeline, which is authoritative. --- .../internal/FullAccessContactReader.kt | 15 +--- .../device/internal/PickerContactReader.kt | 15 +--- .../com/flipcash/app/phone/PhoneUtils.kt | 15 ++++ .../com/flipcash/app/phone/PhoneUtilsTest.kt | 73 +++++++++++++++++++ 4 files changed, 90 insertions(+), 28 deletions(-) diff --git a/apps/flipcash/shared/contacts/src/main/kotlin/com/flipcash/app/contacts/device/internal/FullAccessContactReader.kt b/apps/flipcash/shared/contacts/src/main/kotlin/com/flipcash/app/contacts/device/internal/FullAccessContactReader.kt index 89c804e64..4dc6ac08f 100644 --- a/apps/flipcash/shared/contacts/src/main/kotlin/com/flipcash/app/contacts/device/internal/FullAccessContactReader.kt +++ b/apps/flipcash/shared/contacts/src/main/kotlin/com/flipcash/app/contacts/device/internal/FullAccessContactReader.kt @@ -64,18 +64,5 @@ class FullAccessContactReader @Inject constructor( return Result.success(result) } - private fun normalizeToE164(rawNumber: String): String? { - val cleaned = rawNumber.filter { it.isDigit() || it == '+' } - if (cleaned.isBlank()) return null - - val withPlus = if (cleaned.startsWith("+")) cleaned else "+$cleaned" - if (!phoneUtils.isPhoneNumberValid(withPlus)) return null - - val countryCode = phoneUtils.getCountryCode(withPlus.removePrefix("+")) - val locale = phoneUtils.countryLocales.find { it.countryCode == countryCode } - ?: phoneUtils.defaultCountryLocale - - val result = phoneUtils.cleanNumber(withPlus.removePrefix("+${locale.phoneCode}"), locale) - return result.ifBlank { null } - } + private fun normalizeToE164(rawNumber: String): String? = phoneUtils.toE164(rawNumber) } diff --git a/apps/flipcash/shared/contacts/src/main/kotlin/com/flipcash/app/contacts/device/internal/PickerContactReader.kt b/apps/flipcash/shared/contacts/src/main/kotlin/com/flipcash/app/contacts/device/internal/PickerContactReader.kt index 6f8bce56f..1cc52f3aa 100644 --- a/apps/flipcash/shared/contacts/src/main/kotlin/com/flipcash/app/contacts/device/internal/PickerContactReader.kt +++ b/apps/flipcash/shared/contacts/src/main/kotlin/com/flipcash/app/contacts/device/internal/PickerContactReader.kt @@ -51,18 +51,5 @@ class PickerContactReader @Inject constructor( return Result.success(result) } - private fun normalizeToE164(rawNumber: String): String? { - val cleaned = rawNumber.filter { it.isDigit() || it == '+' } - if (cleaned.isBlank()) return null - - val withPlus = if (cleaned.startsWith("+")) cleaned else "+$cleaned" - if (!phoneUtils.isPhoneNumberValid(withPlus)) return null - - val countryCode = phoneUtils.getCountryCode(withPlus.removePrefix("+")) - val locale = phoneUtils.countryLocales.find { it.countryCode == countryCode } - ?: phoneUtils.defaultCountryLocale - - val result = phoneUtils.cleanNumber(withPlus.removePrefix("+${locale.phoneCode}"), locale) - return result.ifBlank { null } - } + private fun normalizeToE164(rawNumber: String): String? = phoneUtils.toE164(rawNumber) } diff --git a/apps/flipcash/shared/phone/src/main/kotlin/com/flipcash/app/phone/PhoneUtils.kt b/apps/flipcash/shared/phone/src/main/kotlin/com/flipcash/app/phone/PhoneUtils.kt index 04dabdb61..aaacb0f6a 100644 --- a/apps/flipcash/shared/phone/src/main/kotlin/com/flipcash/app/phone/PhoneUtils.kt +++ b/apps/flipcash/shared/phone/src/main/kotlin/com/flipcash/app/phone/PhoneUtils.kt @@ -129,6 +129,21 @@ class PhoneUtils @Inject constructor( } } + fun toE164(rawNumber: String): String? { + val cleaned = rawNumber.filter { it.isDigit() || it == '+' } + if (cleaned.isBlank()) return null + + return try { + val parsed = phoneNumberUtil.parse(cleaned, defaultCountryLocale.countryCode) + if (!phoneNumberUtil.isValidNumber(parsed)) return null + val type = phoneNumberUtil.getNumberType(parsed) + if (type == PhoneNumberUtil.PhoneNumberType.UNKNOWN) return null + phoneNumberUtil.format(parsed, PhoneNumberUtil.PhoneNumberFormat.E164) + } catch (_: NumberParseException) { + null + } + } + fun toFlagEmoji(country: String): String { // 1. It first checks if the string consists of only 2 characters: ISO 3166-1 alpha-2 two-letter country codes (https://en.wikipedia.org/wiki/Regional_Indicator_Symbol). if (country.length != 2) { diff --git a/apps/flipcash/shared/phone/src/test/kotlin/com/flipcash/app/phone/PhoneUtilsTest.kt b/apps/flipcash/shared/phone/src/test/kotlin/com/flipcash/app/phone/PhoneUtilsTest.kt index 48436f222..a9829e5b5 100644 --- a/apps/flipcash/shared/phone/src/test/kotlin/com/flipcash/app/phone/PhoneUtilsTest.kt +++ b/apps/flipcash/shared/phone/src/test/kotlin/com/flipcash/app/phone/PhoneUtilsTest.kt @@ -15,6 +15,7 @@ import org.robolectric.RuntimeEnvironment import org.robolectric.annotation.Config import kotlin.test.assertEquals import kotlin.test.assertFalse +import kotlin.test.assertNull import kotlin.test.assertTrue @RunWith(RobolectricTestRunner::class) @@ -138,6 +139,78 @@ class PhoneUtilsTest { // endregion + // region toE164 + + @Test + fun `toE164 normalizes international number with plus`() { + val mockNumber = mockk(relaxed = true) + every { mockPhoneNumberUtil.parse("+12025551234", "US") } returns mockNumber + every { mockPhoneNumberUtil.isValidNumber(mockNumber) } returns true + every { mockPhoneNumberUtil.getNumberType(mockNumber) } returns PhoneNumberUtil.PhoneNumberType.FIXED_LINE_OR_MOBILE + every { mockPhoneNumberUtil.format(mockNumber, PhoneNumberUtil.PhoneNumberFormat.E164) } returns "+12025551234" + + assertEquals("+12025551234", phoneUtils.toE164("+12025551234")) + } + + @Test + fun `toE164 normalizes national number without country code`() { + val mockNumber = mockk(relaxed = true) + every { mockPhoneNumberUtil.parse("2025551234", "US") } returns mockNumber + every { mockPhoneNumberUtil.isValidNumber(mockNumber) } returns true + every { mockPhoneNumberUtil.getNumberType(mockNumber) } returns PhoneNumberUtil.PhoneNumberType.FIXED_LINE_OR_MOBILE + every { mockPhoneNumberUtil.format(mockNumber, PhoneNumberUtil.PhoneNumberFormat.E164) } returns "+12025551234" + + assertEquals("+12025551234", phoneUtils.toE164("2025551234")) + } + + @Test + fun `toE164 strips formatting characters`() { + val mockNumber = mockk(relaxed = true) + every { mockPhoneNumberUtil.parse("+12025551234", "US") } returns mockNumber + every { mockPhoneNumberUtil.isValidNumber(mockNumber) } returns true + every { mockPhoneNumberUtil.getNumberType(mockNumber) } returns PhoneNumberUtil.PhoneNumberType.FIXED_LINE_OR_MOBILE + every { mockPhoneNumberUtil.format(mockNumber, PhoneNumberUtil.PhoneNumberFormat.E164) } returns "+12025551234" + + assertEquals("+12025551234", phoneUtils.toE164("+1 (202) 555-1234")) + } + + @Test + fun `toE164 returns null for blank input`() { + assertNull(phoneUtils.toE164("")) + assertNull(phoneUtils.toE164(" ")) + } + + @Test + fun `toE164 returns null for invalid number`() { + val mockNumber = mockk(relaxed = true) + every { mockPhoneNumberUtil.parse("123", "US") } returns mockNumber + every { mockPhoneNumberUtil.isValidNumber(mockNumber) } returns false + + assertNull(phoneUtils.toE164("123")) + } + + @Test + fun `toE164 returns null for unparseable number`() { + every { mockPhoneNumberUtil.parse("abc", "US") } throws io.michaelrocks.libphonenumber.android.NumberParseException( + io.michaelrocks.libphonenumber.android.NumberParseException.ErrorType.NOT_A_NUMBER, + "not a number" + ) + + assertNull(phoneUtils.toE164("abc")) + } + + @Test + fun `toE164 returns null for UNKNOWN number type`() { + val mockNumber = mockk(relaxed = true) + every { mockPhoneNumberUtil.parse("5551234567", "US") } returns mockNumber + every { mockPhoneNumberUtil.isValidNumber(mockNumber) } returns true + every { mockPhoneNumberUtil.getNumberType(mockNumber) } returns PhoneNumberUtil.PhoneNumberType.UNKNOWN + + assertNull(phoneUtils.toE164("5551234567")) + } + + // endregion + // region formatNumber @Test