Fix other group update description bugs and add tests.

app/src/main/java/org/thoughtcrime/securesms/database/model/GroupsV2UpdateMessageProducer.java

@@ -13,6 +13,7 @@ import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 import androidx.annotation.PluralsRes;
 import androidx.annotation.StringRes;
+import androidx.annotation.VisibleForTesting;
 
 import com.google.protobuf.ByteString;
 
@@ -41,9 +42,11 @@ import org.whispersystems.signalservice.api.util.UuidUtil;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
 import java.util.UUID;
@@ -865,14 +868,35 @@ final class GroupsV2UpdateMessageProducer {
     return args.toArray();
   }
 
-  private static @NonNull Spannable makeRecipientsClickable(@NonNull Context context, @NonNull String template, @NonNull List<RecipientId> recipientIds, @Nullable Consumer<RecipientId> clickHandler) {
+  @VisibleForTesting
+  static @NonNull Spannable makeRecipientsClickable(@NonNull Context context, @NonNull String template, @NonNull List<RecipientId> recipientIds, @Nullable Consumer<RecipientId> clickHandler) {
     SpannableStringBuilder builder = new SpannableStringBuilder();
     int startIndex = 0;
 
-    for (RecipientId recipientId : recipientIds) {
+    Map<String, RecipientId> idByPlaceholder = new HashMap<>();
-      String placeholder      = makePlaceholder(recipientId);
+    for (RecipientId id : recipientIds) {
-      int    placeHolderStart = template.indexOf(placeholder, startIndex);
+      idByPlaceholder.put(makePlaceholder(id), id);
-      String beforeChunk      = template.substring(startIndex, placeHolderStart);
+    }
+
+    while (startIndex < template.length()) {
+      Map.Entry<String, RecipientId> nearestEntry    = null;
+      int                            nearestPosition = Integer.MAX_VALUE;
+
+      for (Map.Entry<String, RecipientId> entry : idByPlaceholder.entrySet()) {
+        String placeholder      = entry.getKey();
+        int    placeholderStart = template.indexOf(placeholder, startIndex);
+
+        if (placeholderStart >= 0 && placeholderStart < nearestPosition) {
+          nearestPosition = placeholderStart;
+          nearestEntry    = entry;
+        }
+      }
+
+      if (nearestEntry != null) {
+        String      placeholder = nearestEntry.getKey();
+        RecipientId recipientId = nearestEntry.getValue();
+
+        String beforeChunk = template.substring(startIndex, nearestPosition);
 
         builder.append(beforeChunk);
         builder.append(SpanUtil.clickable(Recipient.resolved(recipientId).getDisplayName(context), 0, v -> {
@@ -881,17 +905,18 @@ final class GroupsV2UpdateMessageProducer {
           }
         }));
 
-      startIndex = placeHolderStart + placeholder.length();
+        startIndex = nearestPosition + placeholder.length();
-    }
+      } else {
-
-    if (startIndex < template.length()) {
         builder.append(template.substring(startIndex));
+        startIndex = template.length();
+      }
     }
 
     return builder;
   }
 
-  private static @NonNull String makePlaceholder(@NonNull RecipientId recipientId) {
+  @VisibleForTesting
+  static @NonNull String makePlaceholder(@NonNull RecipientId recipientId) {
     return "{{SPAN_PLACEHOLDER_" + recipientId + "}}";
   }
 }

app/src/test/java/org/thoughtcrime/securesms/database/model/GroupsV2UpdateMessageProducerTest.java

@@ -30,6 +30,7 @@ import org.whispersystems.signalservice.api.push.ServiceId;
 import org.whispersystems.signalservice.api.util.UuidUtil;
 
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 import java.util.UUID;
 import java.util.stream.Collectors;
@@ -1351,6 +1352,71 @@ public final class GroupsV2UpdateMessageProducerTest {
     assertThat(describeNewGroup(group), is("Group updated."));
   }
 
+  @Test
+  public void makeRecipientsClickable_onePlaceholder() {
+    RecipientId id = RecipientId.from(1);
+
+    Spannable result = GroupsV2UpdateMessageProducer.makeRecipientsClickable(
+        ApplicationProvider.getApplicationContext(),
+        GroupsV2UpdateMessageProducer.makePlaceholder(id),
+        Collections.singletonList(id),
+        null
+    );
+
+    assertEquals("Alice", result.toString());
+  }
+
+  @Test
+  public void makeRecipientsClickable_twoPlaceholders_sameRecipient() {
+    RecipientId id          = RecipientId.from(1);
+    String      placeholder = GroupsV2UpdateMessageProducer.makePlaceholder(id);
+
+    Spannable result = GroupsV2UpdateMessageProducer.makeRecipientsClickable(
+        ApplicationProvider.getApplicationContext(),
+        placeholder + " " + placeholder,
+        Collections.singletonList(id),
+        null
+    );
+
+    assertEquals("Alice Alice", result.toString());
+  }
+
+  @Test
+  public void makeRecipientsClickable_twoPlaceholders_differentRecipient() {
+    RecipientId id1 = RecipientId.from(1);
+    RecipientId id2 = RecipientId.from(2);
+
+    String placeholder1 = GroupsV2UpdateMessageProducer.makePlaceholder(id1);
+    String placeholder2 = GroupsV2UpdateMessageProducer.makePlaceholder(id2);
+
+    Spannable result = GroupsV2UpdateMessageProducer.makeRecipientsClickable(
+        ApplicationProvider.getApplicationContext(),
+        placeholder1 + " " + placeholder2,
+        Arrays.asList(id1, id2),
+        null
+    );
+
+    assertEquals("Alice Bob", result.toString());
+  }
+
+  @Test
+  public void makeRecipientsClickable_complicated() {
+    RecipientId id1 = RecipientId.from(1);
+    RecipientId id2 = RecipientId.from(2);
+
+    String placeholder1 = GroupsV2UpdateMessageProducer.makePlaceholder(id1);
+    String placeholder2 = GroupsV2UpdateMessageProducer.makePlaceholder(id2);
+
+    Spannable result = GroupsV2UpdateMessageProducer.makeRecipientsClickable(
+        ApplicationProvider.getApplicationContext(),
+        placeholder1 + " said hello to " + placeholder2 + ", and " + placeholder2 + " said hello back to " + placeholder1 + ".",
+        Arrays.asList(id1, id2),
+        null
+    );
+
+    assertEquals("Alice said hello to Bob, and Bob said hello back to Alice.", result.toString());
+  }
+
   private @NonNull List<String> describeChange(@NonNull DecryptedGroupChange change) {
     return describeChange(null, change);
   }