SAK-47091 handle spaces in names when sorting (sakaiproject#10402)

davidjnevin · Apr 11, 2022 · fcc9590 · fcc9590
1 parent f47ad2a
commit fcc9590
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 10 deletions.
diff --git a/...el/kernel-util/src/main/java/org/sakaiproject/util/comparator/UserSortNameComparator.java b/...el/kernel-util/src/main/java/org/sakaiproject/util/comparator/UserSortNameComparator.java
@@ -22,6 +22,7 @@
 
 import lombok.extern.slf4j.Slf4j;
 
+import org.apache.commons.lang3.StringUtils;
 import org.sakaiproject.user.api.User;
 import org.springframework.util.comparator.NullSafeComparator;
 
@@ -36,14 +37,7 @@ public class UserSortNameComparator implements Comparator<User> {
 
     public UserSortNameComparator() {
         collator = Collator.getInstance();
-        try {
-            String oldRules = ((RuleBasedCollator) collator).getRules();
-            String newRules = oldRules.replaceAll("<'\u005f'", "<' '<'\u005f'");
-            collator = new RuleBasedCollator(newRules);
-        } catch (ParseException e) {
-            log.warn("Can't create custom collator instead using the default collator, {}", e.toString());
-        }
-        collator.setStrength(Collator.SECONDARY); // ignore case
+        collator.setStrength(Collator.SECONDARY); // ignore case but do differentiate on accents
     }
 
     public UserSortNameComparator(boolean nullsLow) {
@@ -56,6 +50,19 @@ public int compare(User u1, User u2) {
         if (u1 == null) return (nullsLow ? -1 : 1);
         if (u2 == null) return (nullsLow ? 1 : -1);
 
-        return new NullSafeComparator<>(collator, nullsLow).compare(u1.getSortName(), u2.getSortName());
+        Comparator c = new NullSafeComparator<>(collator, nullsLow);
+
+        // Replace spaces to handle sorting scenarios where surname has space
+        String prop1 = StringUtils.replace(u1.getSortName(), " ", "+");
+        String prop2 = StringUtils.replace(u2.getSortName(), " ", "+");
+
+        // Secondary comparison on display name if full name is identical
+        // E.g., John Smith (smithj1) and John Smith (smithj2)
+        if (c.compare(prop1, prop2) == 0) {
+            prop1 = u1.getDisplayId();
+            prop2 = u2.getDisplayId();
+        }
+
+        return c.compare(prop1, prop2);
     }
 }
diff --git a/...ernel-util/src/test/java/org/sakaiproject/util/comparator/UserSortNameComparatorTest.java b/...ernel-util/src/test/java/org/sakaiproject/util/comparator/UserSortNameComparatorTest.java
@@ -99,7 +99,42 @@ public void fullnamesWithSpacesCompare() {
         when(userA.getSortName()).thenReturn("Martinez Torcal, Apple");
         User userB = Mockito.mock(User.class);
         when(userB.getSortName()).thenReturn("Martin Troncoso, X");
-
         assertEquals(1, comparator.compare(userA, userB));
+
+        User userC = Mockito.mock(User.class);
+        when(userC.getSortName()).thenReturn("Will, Xavier Raoul Beñat Edorta");
+        User userD = Mockito.mock(User.class);
+        when(userD.getSortName()).thenReturn("Williams, Abigail");
+        assertEquals(-1, comparator.compare(userC, userD));
+
+        User userE = Mockito.mock(User.class);
+        when(userE.getSortName()).thenReturn("Zong, Chen Guanting");
+        User userF = Mockito.mock(User.class);
+        when(userF.getSortName()).thenReturn("Zong Bi, A");
+        assertEquals(-1, comparator.compare(userE, userF));
+
+        User userG = Mockito.mock(User.class);
+        when(userG.getSortName()).thenReturn("Fredriksson, Maximilian Isak");
+        User userH = Mockito.mock(User.class);
+        when(userH.getSortName()).thenReturn("Fredriksson, Max Teo");
+        assertEquals(1, comparator.compare(userG, userH));
+    }
+
+    // We expect the eid to be used as a secondary comparison when names are identical
+    @Test
+    public void identicalNameCompare() {
+        UserSortNameComparator comparator = new UserSortNameComparator();
+        User userA = Mockito.mock(User.class);
+        when(userA.getSortName()).thenReturn("Smith, John");
+        when(userA.getDisplayId()).thenReturn(("jsmith"));
+        User userB = Mockito.mock(User.class);
+        when(userB.getSortName()).thenReturn("Smith, John");
+        when(userB.getDisplayId()).thenReturn(("smithj"));
+        User userC = Mockito.mock(User.class);
+        when(userC.getSortName()).thenReturn("Smith, John");
+        when(userC.getDisplayId()).thenReturn(("smithj1"));
+        assertEquals(-1, comparator.compare(userA, userB));
+        assertEquals(-1, comparator.compare(userA, userC));
+        assertEquals(-1, comparator.compare(userB, userC));
     }
 }