From 71f5ef5940ed98feb647ed12147e56d7d2119d08 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 22 Nov 2017 10:39:38 -0500 Subject: [PATCH] Improve handling of unread indicator edge cases. --- .../ConversationViewController.m | 33 ++++++++++++++---- Signal/src/util/ThreadUtil.m | 34 +++++++++++++++---- 2 files changed, 55 insertions(+), 12 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m index 9bf9b050d..be6533d9b 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m @@ -2859,6 +2859,7 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) { // We need to reload any modified interactions _before_ we call // reloadViewItems. + BOOL hasDeletions = NO; for (YapDatabaseViewRowChange *rowChange in rowChanges) { switch (rowChange.type) { case YapDatabaseViewChangeUpdate: { @@ -2877,6 +2878,7 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) { if (collectionKey.key) { [self.viewItemMap removeObjectForKey:collectionKey.key]; } + hasDeletions = YES; break; } default: @@ -2974,6 +2976,9 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) { [self.collectionView reloadData]; }]; [self updateLastVisibleTimestamp]; + if (hasDeletions) { + [self cleanUpUnreadIndicatorIfNecessary]; + } } else { BOOL shouldAnimateUpdates = [self shouldAnimateRowUpdates:rowChanges oldViewItemCount:oldViewItemCount]; void (^batchUpdatesCompletion)(BOOL) = ^(BOOL finished) { @@ -2988,6 +2993,9 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) { if (scrollToBottom) { [self scrollToBottomAnimated:shouldAnimateScrollToBottom && shouldAnimateUpdates]; } + if (hasDeletions) { + [self cleanUpUnreadIndicatorIfNecessary]; + } }; if (shouldAnimateUpdates) { [self.collectionView performBatchUpdates:batchUpdates completion:batchUpdatesCompletion]; @@ -3413,12 +3421,6 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) { if (lastVisibleViewItem) { uint64_t lastVisibleTimestamp = lastVisibleViewItem.interaction.timestampForSorting; self.lastVisibleTimestamp = MAX(self.lastVisibleTimestamp, lastVisibleTimestamp); - - // If we delete the last unread message (manually or due to disappearing messages) - // we may need to clean up an obsolete unread indicator. - if (lastVisibleViewItem.interaction.interactionType == OWSInteractionType_UnreadIndicator) { - [self ensureDynamicInteractions]; - } } [self ensureScrollDownButton]; @@ -3431,6 +3433,25 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) { self.hasUnreadMessages = numberOfUnreadMessages > 0; } +- (void)cleanUpUnreadIndicatorIfNecessary +{ + BOOL hasUnreadIndicator = self.dynamicInteractions.unreadIndicatorPosition != nil; + if (!hasUnreadIndicator) { + return; + } + __block BOOL hasUnseenInteractions = NO; + [self.uiDatabaseConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) { + hasUnseenInteractions = + [[transaction ext:TSUnreadDatabaseViewExtensionName] numberOfItemsInGroup:self.thread.uniqueId] > 0; + }]; + if (hasUnseenInteractions) { + return; + } + // If the last unread message was deleted (manually or due to disappearing messages) + // we may need to clean up an obsolete unread indicator. + [self ensureDynamicInteractions]; +} + - (void)updateLastVisibleTimestamp:(uint64_t)timestamp { OWSAssert(timestamp > 0); diff --git a/Signal/src/util/ThreadUtil.m b/Signal/src/util/ThreadUtil.m index 2ee2e5206..26beeb448 100644 --- a/Signal/src/util/ThreadUtil.m +++ b/Signal/src/util/ThreadUtil.m @@ -239,18 +239,40 @@ NS_ASSUME_NONNULL_BEGIN // Due to disappearing messages or manual deletion, // firstUnseenInteractionTimestampParameter may refer to an obsolete // interaction in which case we want to discard it. - TSInteraction *_Nullable lastInteraction = - [[transaction ext:TSMessageDatabaseViewExtensionName] lastObjectInGroup:thread.uniqueId]; - if (!lastInteraction - || lastInteraction.timestampForSorting - < firstUnseenInteractionTimestampParameter.unsignedLongLongValue) { + // + // Therefore, we should discard the existing unread indicator + // position if there are no "unreadable" messages after it. + __block TSInteraction *lastCallOrMessage = nil; + [[transaction ext:TSMessageDatabaseViewExtensionName] + enumerateRowsInGroup:thread.uniqueId + withOptions:NSEnumerationReverse + usingBlock:^(NSString *collection, + NSString *key, + id object, + id metadata, + NSUInteger index, + BOOL *stop) { + + OWSAssert([object isKindOfClass:[TSInteraction class]]); + + if ([object isKindOfClass:[TSIncomingMessage class]] || + [object isKindOfClass:[TSOutgoingMessage class]] || + [object isKindOfClass:[TSCall class]]) { + lastCallOrMessage = object; + *stop = YES; + } + if ([object isKindOfClass:[TSUnreadIndicatorInteraction class]]) { + *stop = YES; + } + }]; + if (!lastCallOrMessage) { firstUnseenInteractionTimestampParameter = nil; } } if (firstUnseenInteractionTimestampParameter) { result.firstUnseenInteractionTimestamp = firstUnseenInteractionTimestampParameter; } else { - TSInteraction *firstUnseenInteraction = + TSInteraction *_Nullable firstUnseenInteraction = [[TSDatabaseView unseenDatabaseViewExtension:transaction] firstObjectInGroup:thread.uniqueId]; if (firstUnseenInteraction) { result.firstUnseenInteractionTimestamp = @(firstUnseenInteraction.timestampForSorting);