diff --git a/app/src/test/kotlin/com/android/messaging/data/contact/repository/ContactsRepositoryImplTest.kt b/app/src/test/kotlin/com/android/messaging/data/contact/repository/ContactsRepositoryImplTest.kt index 0c6706ef2..bf5d44dc2 100644 --- a/app/src/test/kotlin/com/android/messaging/data/contact/repository/ContactsRepositoryImplTest.kt +++ b/app/src/test/kotlin/com/android/messaging/data/contact/repository/ContactsRepositoryImplTest.kt @@ -363,6 +363,52 @@ internal class ContactsRepositoryImplTest { ) } + @Test + fun browseMergesPhoneAndEmailContactsInPersianAlphabetOrder() = runTest { + stubDefaultPhoneCursor( + rows = listOf( + phoneRow(contactId = 1L, sortKey = "بهرام", number = "+15550001"), + phoneRow(contactId = 2L, sortKey = "پرویز", number = "+15550002"), + phoneRow(contactId = 3L, sortKey = "تینا", number = "+15550003"), + phoneRow(contactId = 5L, sortKey = "ژاله", number = "+15550005"), + phoneRow(contactId = 6L, sortKey = "کامران", number = "+15550006"), + ), + ) + stubDefaultEmailCursor( + rows = listOf( + emailRow(contactId = 4L, sortKey = "چکاوک", address = "chakavak@example.com"), + emailRow(contactId = 7L, sortKey = "گلاره", address = "gelareh@example.com"), + emailRow(contactId = 8L, sortKey = "لیلا", address = "leila@example.com"), + ), + ) + + val repo = createRepository() + val page = repo.searchContacts(query = "", offset = 0).first() + + Assert.assertEquals((1L..8L).toList(), page.contacts.map(Contact::id)) + } + + @Test + fun searchMergesLowercaseAndAccentedNamesInAlphabetOrder() = runTest { + val phoneRows = listOf( + phoneRow(contactId = 1L, sortKey = "alice", number = "+15550001"), + phoneRow(contactId = 3L, sortKey = "Émile", number = "+15550003"), + ) + val emailRows = listOf( + emailRow(contactId = 2L, sortKey = "Bob", address = "bob@example.com"), + emailRow(contactId = 4L, sortKey = "Zoe", address = "zoe@example.com"), + ) + stubFilterPhoneCursor(query = "e", rows = phoneRows) + stubFilterEmailCursor(query = "e", rows = emailRows) + stubExpansionPhoneCursor(rows = phoneRows) + stubExpansionEmailCursor(rows = emailRows) + + val repo = createRepository() + val page = repo.searchContacts(query = "e", offset = 0).first() + + Assert.assertEquals(listOf(1L, 2L, 3L, 4L), page.contacts.map(Contact::id)) + } + @Test fun phoneAndEmailDestinationsForSameContactMerge() = runTest { stubFilterPhoneCursor( @@ -691,7 +737,7 @@ internal class ContactsRepositoryImplTest { contactId: Long, sortKey: String, number: String, - type: Int, + type: Int = ContactsContract.CommonDataKinds.Phone.TYPE_MOBILE, customLabel: String? = null, isPrimary: Boolean = false, isSuperPrimary: Boolean = false, @@ -716,7 +762,7 @@ internal class ContactsRepositoryImplTest { contactId: Long, sortKey: String, address: String, - type: Int, + type: Int = ContactsContract.CommonDataKinds.Email.TYPE_HOME, customLabel: String? = null, dataId: Long = nextDataId++, ): RawRow { diff --git a/app/src/test/kotlin/com/android/messaging/ui/recipientselection/component/RecipientContactSectionsTest.kt b/app/src/test/kotlin/com/android/messaging/ui/recipientselection/component/RecipientContactSectionsTest.kt new file mode 100644 index 000000000..3a0fb6849 --- /dev/null +++ b/app/src/test/kotlin/com/android/messaging/ui/recipientselection/component/RecipientContactSectionsTest.kt @@ -0,0 +1,47 @@ +package com.android.messaging.ui.recipientselection.component + +import com.android.messaging.ui.recipientselection.component.row.contactItem +import com.android.messaging.ui.recipientselection.model.section.RecipientContactListEntry +import kotlinx.collections.immutable.toImmutableList +import org.junit.Assert +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +@RunWith(RobolectricTestRunner::class) +internal class RecipientContactSectionsTest { + + @Test + fun sectionHeadersFollowAlphabetOrderWithNonLettersLast() { + val displayNames = listOf( + "123 Taxi", + "Zoe", + "Émile", + "Bob", + "تینا", + "پرویز", + "بهرام", + "گلاره", + "کامران", + "لیلا", + ) + val items = displayNames + .mapIndexed { index, displayName -> + contactItem(id = index + 1L, displayName = displayName) + } + .toImmutableList() + + val entries = recipientContactListEntries( + items = items, + showSectionHeaders = true, + ) + + val headers = entries + .filterIsInstance() + .map { header -> header.label } + Assert.assertEquals( + listOf("B", "É", "Z", "ب", "پ", "ت", "ک", "گ", "ل", "#"), + headers, + ) + } +} diff --git a/src/com/android/messaging/data/contact/repository/ContactsRepository.kt b/src/com/android/messaging/data/contact/repository/ContactsRepository.kt index ddc066ec8..fc824e0a0 100644 --- a/src/com/android/messaging/data/contact/repository/ContactsRepository.kt +++ b/src/com/android/messaging/data/contact/repository/ContactsRepository.kt @@ -2,6 +2,7 @@ package com.android.messaging.data.contact.repository import android.content.ContentResolver import android.database.Cursor +import android.icu.text.Collator import android.net.Uri import android.provider.ContactsContract import android.provider.ContactsContract.CommonDataKinds.Email @@ -71,7 +72,7 @@ internal class ContactsRepositoryImpl @Inject constructor( sortOrder = SORT_BY_SORT_KEY_PRIMARY_ASC, ) - val mergedRows = (phoneRows + emailRows).sortedBy { row -> row.sortKey } + val mergedRows = sortByLocalizedSortKey(rows = phoneRows + emailRows) val contactGroups = when { mergedRows.isEmpty() -> emptyList() @@ -97,7 +98,7 @@ internal class ContactsRepositoryImpl @Inject constructor( sortOrder = SORT_BY_SORT_KEY_PRIMARY_ASC, ) - val mergedMatches = (phoneMatches + emailMatches).sortedBy { it.sortKey } + val mergedMatches = sortByLocalizedSortKey(rows = phoneMatches + emailMatches) if (mergedMatches.isEmpty()) { val hasDigits = query.any { character -> character.isDigit() } @@ -134,13 +135,9 @@ internal class ContactsRepositoryImpl @Inject constructor( matchRows: List, offset: Int, ): ContactsPage { - val contactSortKeys = LinkedHashMap() - - matchRows.forEach { row -> - contactSortKeys.putIfAbsent(row.contactId, row.sortKey) - } - - val orderedContactIds = contactSortKeys.keys.toList() + val orderedContactIds = matchRows + .map(DestinationRow::contactId) + .distinct() val pageStart = offset.coerceAtMost(maximumValue = orderedContactIds.size) val pageEndExclusive = (pageStart + PAGE_SIZE) @@ -154,10 +151,7 @@ internal class ContactsRepositoryImpl @Inject constructor( val pageGroups = when { pageContactIds.isEmpty() -> emptyList() else -> { - loadAndGroupDestinationsForContacts( - contactIds = pageContactIds, - sortKeysByContactId = contactSortKeys, - ) + loadAndGroupDestinationsForContacts(contactIds = pageContactIds) } } @@ -176,7 +170,6 @@ internal class ContactsRepositoryImpl @Inject constructor( private fun loadAndGroupDestinationsForContacts( contactIds: List, - sortKeysByContactId: Map, ): List> { val phoneRows = readDestinationRowsForContacts( uri = Phone.CONTENT_URI, @@ -198,18 +191,12 @@ internal class ContactsRepositoryImpl @Inject constructor( rowsByContact[row.contactId]?.add(row) } - return rowsByContact.entries - .asSequence() - .filter { (_, rows) -> rows.isNotEmpty() } - .sortedWith( - compareBy>> { - sortKeysByContactId[it.key].orEmpty() - } - .thenBy { it.value.first().displayName } - .thenBy { it.key }, - ) - .map { it.value } - .toList() + return rowsByContact.values.filter { rows -> rows.isNotEmpty() } + } + + private fun sortByLocalizedSortKey(rows: List): List { + val collator = Collator.getInstance() + return rows.sortedWith(compareBy(collator) { row -> row.sortKey }) } private fun emptyContactsPage(): ContactsPage { diff --git a/src/com/android/messaging/ui/recipientselection/component/RecipientContactSections.kt b/src/com/android/messaging/ui/recipientselection/component/RecipientContactSections.kt index 851909f1f..9f01be94d 100644 --- a/src/com/android/messaging/ui/recipientselection/component/RecipientContactSections.kt +++ b/src/com/android/messaging/ui/recipientselection/component/RecipientContactSections.kt @@ -1,5 +1,6 @@ package com.android.messaging.ui.recipientselection.component +import android.icu.text.Collator import com.android.messaging.ui.recipientselection.model.picker.RecipientPickerListItem import com.android.messaging.ui.recipientselection.model.section.RecipientContactListEntry import kotlinx.collections.immutable.ImmutableList @@ -32,7 +33,7 @@ internal fun recipientContactListEntries( val entries = persistentListOf().builder() sections.keys - .sortedWith(SECTION_LABEL_COMPARATOR) + .sortedWith(sectionLabelComparator()) .forEach { label -> val sectionItems = sections.getValue(label) entries.add(RecipientContactListEntry.Header(label)) @@ -66,13 +67,7 @@ internal fun recipientContactSectionLabel(item: RecipientPickerListItem): String } } -private val SECTION_LABEL_COMPARATOR = Comparator { first, second -> - val firstIsNonLetter = first == NON_LETTER_SECTION_LABEL - val secondIsNonLetter = second == NON_LETTER_SECTION_LABEL - - when { - firstIsNonLetter == secondIsNonLetter -> first.compareTo(second) - firstIsNonLetter -> 1 - else -> -1 - } +private fun sectionLabelComparator(): Comparator { + return compareBy { label -> label == NON_LETTER_SECTION_LABEL } + .then(Collator.getInstance()) }