Respond to CR.

Signal/src/ViewControllers/Utils/MessageRecipientStatusUtils.swift

@@ -16,35 +16,6 @@ import SignalMessaging
     case skipped
 }
 
-// Our per-recipient status messages are "biased towards success"
-// and reflect the most successful known state for that recipient.
-//
-// Our per-message status messages are "biased towards failure"
-// and reflect the least successful known state for that message.
-//
-// Why?
-//
-// When showing the per-recipient status, we want to show the message
-// as "read" even if delivery failed to another recipient of the same
-// message.
-//
-// When showing the per-message status, we want to show the message
-// as "failed" if delivery failed to any recipient, even if another 
-// receipient has read the message.
-// 
-// Note also that for legacy reasons we have redundant and possibly
-// conflicting state.  Examples:
-//
-// * We could have an entry in the recipientReadMap for a message
-//   that has no entries in its recipientDeliveryMap.
-// * We could have an entry in the recipientReadMap or recipientDeliveryMap
-//   for a message whose status is "attempting out" or "unsent".
-// * We could have a message whose wasDelivered property is false but
-//   which has entries in its recipientDeliveryMap or recipientReadMap.
-// * Etc.
-//
-// To resolve this ambiguity, we apply a "bias" towards success or
-// failure.
 class MessageRecipientStatusUtils: NSObject {
     // MARK: Initializers
 

SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.h

@@ -156,7 +156,7 @@ typedef NS_ENUM(NSInteger, TSGroupMetaMessage) {
 // All recipients of this message.
 - (NSArray<NSString *> *)recipientIds;
 
-// All recipients of this message who we are currently trying to send to (queued, uploading or doing send).
+// All recipients of this message who we are currently trying to send to (queued, uploading or during send).
 - (NSArray<NSString *> *)sendingRecipientIds;
 
 // All recipients of this message to whom it has been sent and delivered.

SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m

@@ -124,6 +124,7 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec
     }
     NSString *_Nullable singleGroupRecipient = [coder decodeObjectForKey:@"singleGroupRecipient"];
     if (singleGroupRecipient) {
+        OWSFail(@"%@ unexpected single group recipient message.", self.logTag);
         // If this is a "single group recipient message", treat it as such.
         recipientIds = @[
             singleGroupRecipient,
@@ -334,22 +335,14 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec
     [super saveWithTransaction:transaction];
 }
 
-- (OWSOutgoingMessageRecipientState)maxMessageState
+- (BOOL)hasSentToAnyRecipient
 {
-    OWSOutgoingMessageRecipientState result = OWSOutgoingMessageRecipientStateMin;
     for (TSOutgoingMessageRecipientState *recipientState in self.recipientStateMap.allValues) {
-        result = MAX(recipientState.state, result);
+        if (recipientState.state == OWSOutgoingMessageRecipientStateSent) {
+            return YES;
         }
-    return result;
-}
-
-- (OWSOutgoingMessageRecipientState)minMessageState
-{
-    OWSOutgoingMessageRecipientState result = OWSOutgoingMessageRecipientStateMax;
-    for (TSOutgoingMessageRecipientState *recipientState in self.recipientStateMap.allValues) {
-        result = MIN(recipientState.state, result);
     }
-    return result;
+    return NO;
 }
 
 - (BOOL)shouldStartExpireTimer:(YapDatabaseReadTransaction *)transaction
@@ -366,7 +359,7 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec
     } else if (self.recipientStateMap.count < 1) {
         return YES;
     } else {
-        return self.maxMessageState >= OWSOutgoingMessageRecipientStateSent;
+        return self.hasSentToAnyRecipient;
     }
 }
 
@@ -382,7 +375,7 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec
 
 - (NSArray<NSString *> *)recipientIds
 {
-    return [self.recipientStateMap.allKeys copy];
+    return self.recipientStateMap.allKeys;
 }
 
 - (NSArray<NSString *> *)sendingRecipientIds

SignalServiceKit/src/Messages/OWSMessageSender.m

@@ -607,6 +607,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException";
           failure:(RetryableFailureHandler)failureHandler
 {
     [self saveGroupMessage:message inThread:thread];
+
     NSMutableArray<TOCFuture *> *futures = [NSMutableArray array];
 
     for (SignalRecipient *recipient in recipients) {
@@ -616,12 +617,6 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException";
         if ([recipientId isEqualToString:[TSAccountManager localNumber]]) {
             continue;
         }
-        if (![message.sendingRecipientIds containsObject:recipientId]) {
-            // Skip recipients we have already sent this message to (on an
-            // earlier retry, perhaps).
-            DDLogInfo(@"%@ Skipping group message recipient; already sent: %@", self.logTag, recipient.uniqueId);
-            continue;
-        }
 
         // ...otherwise we send.
         [futures addObject:[self sendMessageFuture:message recipient:recipient thread:thread]];