Don't merge numbers if they only differ after a semicolon

Bug:2432347

Change-Id: Ie62f36a29ae76287265f5ec19fc5507c0d368adf

diff --git a/src/com/android/contacts/ContactsUtils.java b/src/com/android/contacts/ContactsUtils.java
index ba1f4fa..94dabba 100644 (file)
--- a/src/com/android/contacts/ContactsUtils.java
+++ b/src/com/android/contacts/ContactsUtils.java
@@ -51,6 +51,7 @@ import java.util.ArrayList;
 
 public class ContactsUtils {
     private static final String TAG = "ContactsUtils";
+    private static final String WAIT_SYMBOL_AS_STRING = String.valueOf(PhoneNumberUtils.WAIT);
     /**
      * Build the display title for the {@link Data#CONTENT_URI} entry in the
      * provided cursor, assuming the given mimeType.
@@ -439,9 +440,10 @@ public class ContactsUtils {
 
     /**
      * Returns true if two data with mimetypes which represent values in contact entries are
-     * considered equal.
+     * considered equal for collapsing in the GUI. For caller-id, use
+     * {@link PhoneNumberUtils#compare(Context, String, String)} instead
      */
-    public static final boolean areDataEqual(Context context, CharSequence mimetype1,
+    public static final boolean shouldCollapse(Context context, CharSequence mimetype1,
             CharSequence data1, CharSequence mimetype2, CharSequence data2) {
         if (TextUtils.equals(Phone.CONTENT_ITEM_TYPE, mimetype1)
                 && TextUtils.equals(Phone.CONTENT_ITEM_TYPE, mimetype2)) {
@@ -451,7 +453,23 @@ public class ContactsUtils {
             if (data1 == null || data2 == null) {
                 return false;
             }
-            return PhoneNumberUtils.compare(context, data1.toString(), data2.toString());
+
+            // If the number contains semicolons, PhoneNumberUtils.compare
+            // only checks the substring before that (which is fine for caller-id usually)
+            // but not for collapsing numbers. so we check each segment indidually to be more strict
+            // TODO: This should be replaced once we have a more robust phonenumber-library
+            String[] dataParts1 = data1.toString().split(WAIT_SYMBOL_AS_STRING);
+            String[] dataParts2 = data2.toString().split(WAIT_SYMBOL_AS_STRING);
+            if (dataParts1.length != dataParts2.length) {
+                return false;
+            }
+            for (int i = 0; i < dataParts1.length; i++) {
+                if (!PhoneNumberUtils.compare(context, dataParts1[i], dataParts2[i])) {
+                    return false;
+                }
+            }
+
+            return true;
         } else {
             if (mimetype1 == mimetype2 && data1 == data2) {
                 return true;

diff --git a/src/com/android/contacts/ViewContactActivity.java b/src/com/android/contacts/ViewContactActivity.java
index 2e41d80..ec365dc 100644 (file)
--- a/src/com/android/contacts/ViewContactActivity.java
+++ b/src/com/android/contacts/ViewContactActivity.java
@@ -1123,7 +1123,8 @@ public class ViewContactActivity extends Activity
                 return false;
             }
 
-            if (!ContactsUtils.areDataEqual(context, mimetype, data, entry.mimetype, entry.data)) {
+            if (!ContactsUtils.shouldCollapse(context, mimetype, data, entry.mimetype,
+                    entry.data)) {
                 return false;
             }
 

diff --git a/src/com/android/contacts/ui/QuickContactWindow.java b/src/com/android/contacts/ui/QuickContactWindow.java
index 6d4ff9b..6fdcb01 100644 (file)
--- a/src/com/android/contacts/ui/QuickContactWindow.java
+++ b/src/com/android/contacts/ui/QuickContactWindow.java
@@ -912,7 +912,7 @@ public class QuickContactWindow implements Window.Callback,
             if (!ContactsUtils.areObjectsEqual(mKind, other.mKind)) {
                 return false;
             }
-            if (!ContactsUtils.areDataEqual(mContext, mMimeType, mBody, other.mMimeType,
+            if (!ContactsUtils.shouldCollapse(mContext, mMimeType, mBody, other.mMimeType,
                     other.mBody)) {
                 return false;
             }

diff --git a/tests/src/com/android/contacts/ContactsUtilsTests.java b/tests/src/com/android/contacts/ContactsUtilsTests.java
index d4d4e5b..8e18044 100644 (file)
--- a/tests/src/com/android/contacts/ContactsUtilsTests.java
+++ b/tests/src/com/android/contacts/ContactsUtilsTests.java
@@ -109,40 +109,75 @@ public class ContactsUtilsTests extends AndroidTestCase {
         assertFalse("1:2", ContactsUtils.areObjectsEqual(1, 2));
     }
 
-    public void testAreDataEqual() throws Exception {
-        checkAreDataEqual("1", true, null, null, null, null);
-        checkAreDataEqual("2", true, "a", "b", "a", "b");
-
-        checkAreDataEqual("11", false, "a", null, null, null);
-        checkAreDataEqual("12", false, null, "a", null, null);
-        checkAreDataEqual("13", false, null, null, "a", null);
-        checkAreDataEqual("14", false, null, null, null, "a");
-
-        checkAreDataEqual("21", false, "a", "b", null, null);
-        checkAreDataEqual("22", false, "a", "b", "a", null);
-        checkAreDataEqual("23", false, "a", "b", null, "b");
-        checkAreDataEqual("24", false, "a", "b", "a", "x");
-        checkAreDataEqual("25", false, "a", "b", "x", "b");
-
-        checkAreDataEqual("31", false, null, null, "a", "b");
-        checkAreDataEqual("32", false, "a", null, "a", "b");
-        checkAreDataEqual("33", false, null, "b", "a", "b");
-        checkAreDataEqual("34", false, "a", "x", "a", "b");
-        checkAreDataEqual("35", false, "x", "b", "a", "b");
-
-        checkAreDataEqual("41", true, Phone.CONTENT_ITEM_TYPE, null, Phone.CONTENT_ITEM_TYPE,
+    public void testShouldCollapse() throws Exception {
+        checkShouldCollapse("1", true, null, null, null, null);
+        checkShouldCollapse("2", true, "a", "b", "a", "b");
+
+        checkShouldCollapse("11", false, "a", null, null, null);
+        checkShouldCollapse("12", false, null, "a", null, null);
+        checkShouldCollapse("13", false, null, null, "a", null);
+        checkShouldCollapse("14", false, null, null, null, "a");
+
+        checkShouldCollapse("21", false, "a", "b", null, null);
+        checkShouldCollapse("22", false, "a", "b", "a", null);
+        checkShouldCollapse("23", false, "a", "b", null, "b");
+        checkShouldCollapse("24", false, "a", "b", "a", "x");
+        checkShouldCollapse("25", false, "a", "b", "x", "b");
+
+        checkShouldCollapse("31", false, null, null, "a", "b");
+        checkShouldCollapse("32", false, "a", null, "a", "b");
+        checkShouldCollapse("33", false, null, "b", "a", "b");
+        checkShouldCollapse("34", false, "a", "x", "a", "b");
+        checkShouldCollapse("35", false, "x", "b", "a", "b");
+
+        checkShouldCollapse("41", true, Phone.CONTENT_ITEM_TYPE, null, Phone.CONTENT_ITEM_TYPE,
                 null);
-        checkAreDataEqual("42", true, Phone.CONTENT_ITEM_TYPE, "1", Phone.CONTENT_ITEM_TYPE, "1");
+        checkShouldCollapse("42", true, Phone.CONTENT_ITEM_TYPE, "1", Phone.CONTENT_ITEM_TYPE, "1");
 
-        checkAreDataEqual("51", false, Phone.CONTENT_ITEM_TYPE, "1", Phone.CONTENT_ITEM_TYPE, "2");
-        checkAreDataEqual("52", false, Phone.CONTENT_ITEM_TYPE, "1", Phone.CONTENT_ITEM_TYPE, null);
-        checkAreDataEqual("53", false, Phone.CONTENT_ITEM_TYPE, null, Phone.CONTENT_ITEM_TYPE, "2");
+        checkShouldCollapse("51", false, Phone.CONTENT_ITEM_TYPE, "1", Phone.CONTENT_ITEM_TYPE,
+                "2");
+        checkShouldCollapse("52", false, Phone.CONTENT_ITEM_TYPE, "1", Phone.CONTENT_ITEM_TYPE,
+                null);
+        checkShouldCollapse("53", false, Phone.CONTENT_ITEM_TYPE, null, Phone.CONTENT_ITEM_TYPE,
+                "2");
+
+        // Test phone numbers
+        checkShouldCollapse("60", true,
+                Phone.CONTENT_ITEM_TYPE, "1234567",
+                Phone.CONTENT_ITEM_TYPE, "1234567");
+        checkShouldCollapse("61", false,
+                Phone.CONTENT_ITEM_TYPE, "1234567",
+                Phone.CONTENT_ITEM_TYPE, "1234568");
+        checkShouldCollapse("62", true,
+                Phone.CONTENT_ITEM_TYPE, "1234567;0",
+                Phone.CONTENT_ITEM_TYPE, "1234567;0");
+        checkShouldCollapse("63", false,
+                Phone.CONTENT_ITEM_TYPE, "1234567;89321",
+                Phone.CONTENT_ITEM_TYPE, "1234567;321");
+        checkShouldCollapse("64", true,
+                Phone.CONTENT_ITEM_TYPE, "1234567;89321",
+                Phone.CONTENT_ITEM_TYPE, "1234567;89321");
+        checkShouldCollapse("65", false,
+                Phone.CONTENT_ITEM_TYPE, "1234567;0111111111",
+                Phone.CONTENT_ITEM_TYPE, "1234567;");
+        checkShouldCollapse("66", false,
+                Phone.CONTENT_ITEM_TYPE, "12345675426;91970xxxxx",
+                Phone.CONTENT_ITEM_TYPE, "12345675426");
+        checkShouldCollapse("67", false,
+                Phone.CONTENT_ITEM_TYPE, "12345675426;23456xxxxx",
+                Phone.CONTENT_ITEM_TYPE, "12345675426;234567xxxx");
+        checkShouldCollapse("68", true,
+                Phone.CONTENT_ITEM_TYPE, "1234567;1234567;1234567",
+                Phone.CONTENT_ITEM_TYPE, "1234567;1234567;1234567");
+        checkShouldCollapse("69", false,
+                Phone.CONTENT_ITEM_TYPE, "1234567;1234567;1234567",
+                Phone.CONTENT_ITEM_TYPE, "1234567;1234567");
     }
 
-    private void checkAreDataEqual(String message, boolean expected, CharSequence mimetype1,
+    private void checkShouldCollapse(String message, boolean expected, CharSequence mimetype1,
             CharSequence data1, CharSequence mimetype2, CharSequence data2) {
         assertEquals(message, expected,
-                ContactsUtils.areDataEqual(mContext, mimetype1, data1, mimetype2, data2));
+                ContactsUtils.shouldCollapse(mContext, mimetype1, data1, mimetype2, data2));
     }
 
     public void testAreIntentActionEqual() throws Exception {