Fix Contact sorting problem
And also exact the comparator for search and Contact list to a
separate class.
Bug: 141192471
Test: Manually
Change-Id: I88ecadceb845bd9254a590a366c0e9d4829fed11
diff --git a/res/values/arrays.xml b/res/values/arrays.xml
index eadeb53..9646504 100644
--- a/res/values/arrays.xml
+++ b/res/values/arrays.xml
@@ -36,13 +36,13 @@
</string-array>
<string-array name="contact_order_entry_values">
- <item>given_name</item>
- <item>family_name</item>
+ <item>@string/given_name_first_key</item>
+ <item>@string/family_name_first_key</item>
</string-array>
<string-array name="contact_order_entries">
<!-- This array is mapped to contact_order_keys. -->
- <item>@string/give_name_first_title</item>
+ <item>@string/given_name_first_title</item>
<item>@string/family_name_first_title</item>
</string-array>
</resources>
diff --git a/res/values/strings.xml b/res/values/strings.xml
index 0035be2..23ecf12 100644
--- a/res/values/strings.xml
+++ b/res/values/strings.xml
@@ -167,9 +167,14 @@
<!-- Title of the settings to sort contact order [CHAR LIMIT=40]-->
<string name="sort_order_title">Contact order</string>
<string name="sort_order_key" translatable="false">contact_order</string>
- <!-- Title of the settings for sorting Contacts in different orders -->
+
+ <!-- Keys and titles of the settings for sorting Contacts in different orders -->
+ <!-- Key of given name -->
+ <string name="given_name_first_key" translatable="false">given_name</string>
+ <!-- Key of family name -->
+ <string name="family_name_first_key" translatable="false">family_name</string>
<!-- Title of given name [CHAR LIMIT=40]-->
- <string name="give_name_first_title">First name</string>
+ <string name="given_name_first_title">First name</string>
<!-- Title of family name [CHAR LIMIT=40]-->
<string name="family_name_first_title">Last name</string>
</resources>
diff --git a/src/com/android/car/dialer/ui/common/entity/ContactSortingInfo.java b/src/com/android/car/dialer/ui/common/entity/ContactSortingInfo.java
new file mode 100644
index 0000000..f59e9f7
--- /dev/null
+++ b/src/com/android/car/dialer/ui/common/entity/ContactSortingInfo.java
@@ -0,0 +1,82 @@
+/*
+ * Copyright (C) 2019 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.car.dialer.ui.common.entity;
+
+import android.content.Context;
+import android.util.Pair;
+
+import com.android.car.dialer.R;
+import com.android.car.dialer.livedata.SharedPreferencesLiveData;
+import com.android.car.telephony.common.Contact;
+
+import java.util.Comparator;
+
+/**
+ * Information about how Contacts are sorted.
+ */
+public class ContactSortingInfo {
+ public static final int SORT_BY_FIRST_NAME = 1;
+ public static final int SORT_BY_LAST_NAME = 2;
+ /**
+ * Sort by the default display order of a name. For western names it will be "Given Family".
+ * For unstructured names like east asian this will be the only order.
+ * Phone Dialer uses the same method for sorting given names.
+ *
+ * @see android.provider.ContactsContract.Contacts#DISPLAY_NAME_PRIMARY
+ */
+ private static final Comparator<Contact> sFirstNameComparator =
+ (o1, o2) -> o1.compareByDisplayName(o2);
+
+ /**
+ * Sort by the alternative display order of a name. For western names it will be "Family,
+ * Given". For unstructured names like east asian this order will be ignored and treated as
+ * primary.
+ * Phone Dialer uses the same method for sorting family names.
+ *
+ * @see android.provider.ContactsContract.Contacts#DISPLAY_NAME_ALTERNATIVE
+ */
+ private static final Comparator<Contact> sLastNameComparator =
+ (o1, o2) -> o1.compareByAltDisplayName(o2);
+
+ /**
+ * A static method that return how Contacts are sorted
+ * The first parameter is the comparator that is used for sorting Contacts
+ * The second parameter is a reference to keep track of the soring method.
+ */
+ public static Pair<Comparator<Contact>, Integer> getSortingInfo(Context context,
+ SharedPreferencesLiveData preferencesLiveData) {
+ String key = preferencesLiveData.getKey();
+ String defaultValue = context.getResources().getStringArray(
+ R.array.contact_order_entry_values)[0];
+ String firstNameSort = context.getResources().getString(
+ R.string.given_name_first_key);
+
+ Comparator<Contact> comparator;
+ Integer sortMethod;
+ if (preferencesLiveData.getValue() == null
+ || firstNameSort.equals(
+ preferencesLiveData.getValue().getString(key, defaultValue))) {
+ comparator = sFirstNameComparator;
+ sortMethod = SORT_BY_FIRST_NAME;
+ } else {
+ comparator = sLastNameComparator;
+ sortMethod = SORT_BY_LAST_NAME;
+ }
+
+ return new Pair<>(comparator, sortMethod);
+ }
+}
diff --git a/src/com/android/car/dialer/ui/contact/ContactListAdapter.java b/src/com/android/car/dialer/ui/contact/ContactListAdapter.java
index cdc1ccd..67b7242 100644
--- a/src/com/android/car/dialer/ui/contact/ContactListAdapter.java
+++ b/src/com/android/car/dialer/ui/contact/ContactListAdapter.java
@@ -26,6 +26,7 @@
import androidx.recyclerview.widget.RecyclerView;
import com.android.car.dialer.R;
+import com.android.car.dialer.ui.common.entity.ContactSortingInfo;
import com.android.car.telephony.common.Contact;
import java.util.ArrayList;
@@ -83,12 +84,12 @@
@Override
public int getItemCount() {
- return mContactList.size();
+ return mContactList.size();
}
private String getHeader(Contact contact) {
String label;
- if (mSortMethod.equals(ContactListViewModel.SORT_BY_LAST_NAME)) {
+ if (mSortMethod.equals(ContactSortingInfo.SORT_BY_LAST_NAME)) {
label = contact.getPhonebookLabelAlt();
} else {
label = contact.getPhonebookLabel();
diff --git a/src/com/android/car/dialer/ui/contact/ContactListViewModel.java b/src/com/android/car/dialer/ui/contact/ContactListViewModel.java
index d6dd8c6..b8df331 100644
--- a/src/com/android/car/dialer/ui/contact/ContactListViewModel.java
+++ b/src/com/android/car/dialer/ui/contact/ContactListViewModel.java
@@ -27,6 +27,7 @@
import com.android.car.dialer.R;
import com.android.car.dialer.livedata.SharedPreferencesLiveData;
+import com.android.car.dialer.ui.common.entity.ContactSortingInfo;
import com.android.car.dialer.widget.WorkerExecutor;
import com.android.car.telephony.common.Contact;
import com.android.car.telephony.common.InMemoryPhoneBook;
@@ -40,10 +41,6 @@
* View model for {@link ContactListFragment}.
*/
public class ContactListViewModel extends AndroidViewModel {
-
- public static final int SORT_BY_FIRST_NAME = 1;
- public static final int SORT_BY_LAST_NAME = 2;
-
private final Context mContext;
private final LiveData<Pair<Integer, List<Contact>>> mSortedContactListLiveData;
@@ -74,27 +71,6 @@
private Future<?> mRunnableFuture;
- /**
- * Sort by the default display order of a name. For western names it will be "Given Family".
- * For unstructured names like east asian this will be the only order.
- * Phone Dialer uses the same method for sorting given names.
- *
- * @see android.provider.ContactsContract.Contacts#DISPLAY_NAME_PRIMARY
- */
- private final Comparator<Contact> mFirstNameComparator =
- (o1, o2) -> o1.compareByDisplayName(o2);
-
- /**
- * Sort by the alternative display order of a name. For western names it will be "Family,
- * Given". For unstructured names like east asian this order will be ignored and treated as
- * primary.
- * Phone Dialer uses the same method for sorting family names.
- *
- * @see android.provider.ContactsContract.Contacts#DISPLAY_NAME_ALTERNATIVE
- */
- private final Comparator<Contact> mLastNameComparator =
- (o1, o2) -> o1.compareByAltDisplayName(o2);
-
private SortedContactListLiveData(Context context,
@NonNull LiveData<List<Contact>> contactListLiveData,
@NonNull SharedPreferencesLiveData sharedPreferencesLiveData) {
@@ -112,22 +88,11 @@
return;
}
- String key = mPreferencesLiveData.getKey();
- String firstNameSort = mContext.getResources().getString(
- R.string.give_name_first_title);
-
List<Contact> contactList = mContactListLiveData.getValue();
- Comparator<Contact> comparator;
- Integer sortMethod;
- if (mPreferencesLiveData.getValue() == null
- || firstNameSort.equals(
- mPreferencesLiveData.getValue().getString(key, firstNameSort))) {
- comparator = mFirstNameComparator;
- sortMethod = SORT_BY_FIRST_NAME;
- } else {
- comparator = mLastNameComparator;
- sortMethod = SORT_BY_LAST_NAME;
- }
+ Pair<Comparator<Contact>, Integer> contactSortingInfo = ContactSortingInfo
+ .getSortingInfo(mContext, mPreferencesLiveData);
+ Comparator<Contact> comparator = contactSortingInfo.first;
+ Integer sortMethod = contactSortingInfo.second;
// SingleThreadPoolExecutor is used here to avoid multiple threads sorting the list
// at the same time.
diff --git a/src/com/android/car/dialer/ui/search/ContactResultsViewModel.java b/src/com/android/car/dialer/ui/search/ContactResultsViewModel.java
index 8773791..b852ffb 100644
--- a/src/com/android/car/dialer/ui/search/ContactResultsViewModel.java
+++ b/src/com/android/car/dialer/ui/search/ContactResultsViewModel.java
@@ -33,6 +33,7 @@
import com.android.car.dialer.R;
import com.android.car.dialer.livedata.SharedPreferencesLiveData;
+import com.android.car.dialer.ui.common.entity.ContactSortingInfo;
import com.android.car.telephony.common.Contact;
import com.android.car.telephony.common.InMemoryPhoneBook;
import com.android.car.telephony.common.ObservableAsyncQuery;
@@ -40,7 +41,6 @@
import java.util.ArrayList;
import java.util.Collections;
-import java.util.Comparator;
import java.util.List;
/** {link AndroidViewModel} used for search functionality. */
@@ -86,10 +86,6 @@
private final LiveData<String> mSearchQueryLiveData;
private final LiveData<List<Contact>> mContactListLiveData;
private final SharedPreferencesLiveData mSharedPreferencesLiveData;
- private final Comparator<Contact> mFirstNameComparator =
- (o1, o2) -> o1.compareByDisplayName(o2);
- private final Comparator<Contact> mLastNameComparator =
- (o1, o2) -> o1.compareByAltDisplayName(o2);
ContactResultsLiveData(Context context,
LiveData<String> searchQueryLiveData,
@@ -134,7 +130,8 @@
List<Contact> contacts = new ArrayList<>();
contacts.addAll(getValue());
- Collections.sort(contacts, getComparator());
+ Collections.sort(contacts,
+ ContactSortingInfo.getSortingInfo(mContext, mSharedPreferencesLiveData).first);
setValue(contacts);
}
@@ -153,22 +150,11 @@
contacts.add(contact);
}
}
- Collections.sort(contacts, getComparator());
+ Collections.sort(contacts,
+ ContactSortingInfo.getSortingInfo(mContext, mSharedPreferencesLiveData).first);
setValue(contacts);
cursor.close();
}
-
- private Comparator<Contact> getComparator() {
- String firstNameSort = mContext.getResources().getString(
- R.string.give_name_first_title);
- String key = mSharedPreferencesLiveData.getKey();
- if (firstNameSort.equals(
- mSharedPreferencesLiveData.getValue().getString(key, firstNameSort))) {
- return mFirstNameComparator;
- } else {
- return mLastNameComparator;
- }
- }
}
private static class SearchQueryParamProvider implements QueryParam.Provider {
diff --git a/tests/robotests/src/com/android/car/dialer/ui/contact/ContactListFragmentTest.java b/tests/robotests/src/com/android/car/dialer/ui/contact/ContactListFragmentTest.java
index d8c87c1..9fc4898 100644
--- a/tests/robotests/src/com/android/car/dialer/ui/contact/ContactListFragmentTest.java
+++ b/tests/robotests/src/com/android/car/dialer/ui/contact/ContactListFragmentTest.java
@@ -37,6 +37,7 @@
import com.android.car.dialer.R;
import com.android.car.dialer.telecom.UiCallManager;
import com.android.car.dialer.testutils.ShadowAndroidViewModelFactory;
+import com.android.car.dialer.ui.common.entity.ContactSortingInfo;
import com.android.car.dialer.ui.favorite.FavoriteViewModel;
import com.android.car.telephony.common.Contact;
import com.android.car.telephony.common.PhoneNumber;
@@ -84,7 +85,7 @@
MockitoAnnotations.initMocks(this);
MutableLiveData<Pair<Integer, List<Contact>>> contactList = new MutableLiveData<>();
- contactList.setValue(new Pair<>(ContactListViewModel.SORT_BY_LAST_NAME,
+ contactList.setValue(new Pair<>(ContactSortingInfo.SORT_BY_LAST_NAME,
Arrays.asList(mMockContact1, mMockContact2, mMockContact3)));
ShadowAndroidViewModelFactory.add(ContactListViewModel.class, mMockContactListViewModel);
when(mMockContactListViewModel.getAllContacts()).thenReturn(contactList);
