From 5722cfe7d012838d066c8a3f089fa34e41ecb975 Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Thu, 23 Jun 2022 15:51:19 +1000 Subject: [PATCH] Fixed a bunch of bugs Fixed a bug where call messages weren't getting migrated correctly Fixed a bug where the conversation screen would be dismissed when returning from the background Fixed a bug where the conversation screen wasn't starting focused on the first unread message Fixed a bug where contacts that were approved might not be approved after the migration (flags weren't stored correctly previously???) Fixed a bug where the closed group members might not be migrated correctly Fixed a bug where some legacy info messages could be mistakenly migrated as call messages instead of message request acceptance messages Fixed a bug where the last message wasn't showing it's "sent" status correctly Fixed a bug where the QuoteView wasn't laying out the same way it used to Removed some buggy animations when sending/receiving single messages --- .../ConversationVC+Interaction.swift | 14 +- Session/Conversations/ConversationVC.swift | 237 +++++++----------- .../Conversations/ConversationViewModel.swift | 27 +- .../Content Views/QuoteView.swift | 2 +- Session/Meta/AppDelegate.swift | 1 + .../Database/LegacyDatabase/SMKLegacy.swift | 34 ++- .../Migrations/_003_YDBToGRDBMigration.swift | 132 ++++++---- .../MessageReceiver+ClosedGroups.swift | 41 ++- .../MessageReceiver+VisibleMessages.swift | 13 +- .../MessageSender+ClosedGroups.swift | 38 +-- .../Sending & Receiving/MessageSender.swift | 2 - .../SessionThreadViewModel.swift | 24 +- 12 files changed, 296 insertions(+), 269 deletions(-) diff --git a/Session/Conversations/ConversationVC+Interaction.swift b/Session/Conversations/ConversationVC+Interaction.swift index c2e3a801d..bb76b60dc 100644 --- a/Session/Conversations/ConversationVC+Interaction.swift +++ b/Session/Conversations/ConversationVC+Interaction.swift @@ -360,6 +360,9 @@ extension ConversationVC: return } + // Let the viewModel know we are about to send a message + self?.viewModel.sentMessageBeforeUpdate = true + // Update the thread to be visible _ = try SessionThread .filter(id: threadId) @@ -391,11 +394,9 @@ extension ConversationVC: ) ).insert(db) } - - guard let interactionId: Int64 = interaction.id else { return } - + // If there is a Quote the insert it now - if let quoteModel: QuotedReplyModel = quoteModel { + if let interactionId: Int64 = interaction.id, let quoteModel: QuotedReplyModel = quoteModel { try Quote( interactionId: interactionId, authorId: quoteModel.authorId, @@ -412,7 +413,6 @@ extension ConversationVC: ) }, completion: { [weak self] _, _ in - self?.viewModel.sentMessageBeforeUpdate = true self?.handleMessageSent() } ) @@ -457,6 +457,9 @@ extension ConversationVC: return } + // Let the viewModel know we are about to send a message + self?.viewModel.sentMessageBeforeUpdate = true + // Update the thread to be visible _ = try SessionThread .filter(id: threadId) @@ -480,7 +483,6 @@ extension ConversationVC: ) }, completion: { [weak self] _, _ in - self?.viewModel.sentMessageBeforeUpdate = true self?.handleMessageSent() // Attachment successfully sent - dismiss the screen diff --git a/Session/Conversations/ConversationVC.swift b/Session/Conversations/ConversationVC.swift index 3ac99bbdb..660b29e7b 100644 --- a/Session/Conversations/ConversationVC.swift +++ b/Session/Conversations/ConversationVC.swift @@ -49,6 +49,7 @@ final class ConversationVC: BaseVC, OWSConversationSettingsViewDelegate, Convers // Scrolling & paging var isUserScrolling = false + var hasPerformedInitialScroll = false var didFinishInitialLayout = false var scrollDistanceToBottomBeforeUpdate: CGFloat? var baselineKeyboardHeight: CGFloat = 0 @@ -420,16 +421,8 @@ final class ConversationVC: BaseVC, OWSConversationSettingsViewDelegate, Convers override func viewDidAppear(_ animated: Bool) { super.viewDidAppear(animated) - // Perform the initial scroll and highlight if needed (if we started with a focused message - // this will have already been called to instantly snap to the destination but we don't - // trigger the highlight until after the screen has appeared to make it more obvious) - performInitialScrollIfNeeded() - // Flag that the initial layout has been completed (the flag blocks and unblocks a number // of different behaviours) - // - // Note: This MUST be set after the above 'performInitialScrollIfNeeded' is called as it - // won't run if this flag is set to true didFinishInitialLayout = true if delayFirstResponder || isShowingSearchUI { @@ -516,13 +509,16 @@ final class ConversationVC: BaseVC, OWSConversationSettingsViewDelegate, Convers // The default scheduler emits changes on the main thread self?.handleThreadUpdates(threadData) - self?.performInitialScrollIfNeeded() + + // Note: We want to load the interaction data into the UI after the initial thread data + // has loaded to prevent an issue where the conversation loads with the wrong offset + if self?.viewModel.onInteractionChange == nil { + self?.viewModel.onInteractionChange = { [weak self] updatedInteractionData in + self?.handleInteractionUpdates(updatedInteractionData) + } + } } ) - - self.viewModel.onInteractionChange = { [weak self] updatedInteractionData in - self?.handleInteractionUpdates(updatedInteractionData) - } } private func stopObservingChanges() { @@ -617,42 +613,42 @@ final class ConversationVC: BaseVC, OWSConversationSettingsViewDelegate, Convers return } - // Determine if we are inserting content at the top of the collectionView - struct ItemChangeInfo { - enum InsertLocation { - case top - case bottom - case other - case none - } + // Mark received messages as read + let didSendMessageBeforeUpdate: Bool = self.viewModel.sentMessageBeforeUpdate + self.viewModel.markAllAsRead() + self.viewModel.sentMessageBeforeUpdate = false + + // When sending a message we want to reload the UI instantly (with any form of animation the message + // sending feels somewhat unresponsive but an instant update feels snappy) + guard !didSendMessageBeforeUpdate else { + self.viewModel.updateInteractionData(updatedData) + self.tableView.reloadData() - let insertLocation: InsertLocation - let wasCloseToBottom: Bool - let sentMessageBeforeUpdate: Bool + // Note: The scroll button alpha won't get set correctly in this case so we forcibly set it to + // have an alpha of 0 to stop it appearing buggy + self.scrollToBottom(isAnimated: false) + self.scrollButton.alpha = 0 + self.unreadCountView.alpha = scrollButton.alpha + return + } + + // Reload the table content animating changes if they'll look good + struct ItemChangeInfo { + let isInsertAtTop: Bool let firstIndexIsVisible: Bool - let visibleInteractionId: Int64 let visibleIndexPath: IndexPath let oldVisibleIndexPath: IndexPath - let lastVisibleIndexPath: IndexPath init( - insertLocation: InsertLocation, - wasCloseToBottom: Bool, - sentMessageBeforeUpdate: Bool, + isInsertAtTop: Bool = false, firstIndexIsVisible: Bool = false, - visibleInteractionId: Int64 = -1, visibleIndexPath: IndexPath = IndexPath(row: 0, section: 0), - oldVisibleIndexPath: IndexPath = IndexPath(row: 0, section: 0), - lastVisibleIndexPath: IndexPath = IndexPath(row: 0, section: 0) + oldVisibleIndexPath: IndexPath = IndexPath(row: 0, section: 0) ) { - self.insertLocation = insertLocation - self.wasCloseToBottom = wasCloseToBottom - self.sentMessageBeforeUpdate = sentMessageBeforeUpdate + self.isInsertAtTop = isInsertAtTop self.firstIndexIsVisible = firstIndexIsVisible - self.visibleInteractionId = visibleInteractionId self.visibleIndexPath = visibleIndexPath self.oldVisibleIndexPath = oldVisibleIndexPath - self.lastVisibleIndexPath = lastVisibleIndexPath } } @@ -660,96 +656,79 @@ final class ConversationVC: BaseVC, OWSConversationSettingsViewDelegate, Convers source: viewModel.interactionData, target: updatedData ) + let isInsert: Bool = (changeset.map({ $0.elementInserted.count }).reduce(0, +) > 0) + let wasLoadingMore: Bool = self.isLoadingMore + let wasOffsetCloseToBottom: Bool = self.isCloseToBottom let numItemsInUpdatedData: [Int] = updatedData.map { $0.elements.count } - let itemChangeInfo: ItemChangeInfo = { + let itemChangeInfo: ItemChangeInfo? = { guard - changeset.map({ $0.elementInserted.count }).reduce(0, +) > 0, + isInsert, let oldSectionIndex: Int = self.viewModel.interactionData.firstIndex(where: { $0.model == .messages }), let newSectionIndex: Int = updatedData.firstIndex(where: { $0.model == .messages }), let newFirstItemIndex: Int = updatedData[newSectionIndex].elements .firstIndex(where: { item -> Bool in item.id == self.viewModel.interactionData[oldSectionIndex].elements.first?.id }), - let newLastItemIndex: Int = updatedData[newSectionIndex].elements - .lastIndex(where: { item -> Bool in - item.id == self.viewModel.interactionData[oldSectionIndex].elements.last?.id - }), let firstVisibleIndexPath: IndexPath = self.tableView.indexPathsForVisibleRows? .filter({ $0.section == oldSectionIndex }) .sorted() .first, - let lastVisibleIndexPath: IndexPath = self.tableView.indexPathsForVisibleRows? - .filter({ $0.section == oldSectionIndex }) - .sorted() - .last, let newVisibleIndex: Int = updatedData[newSectionIndex].elements .firstIndex(where: { item in item.id == self.viewModel.interactionData[oldSectionIndex] .elements[firstVisibleIndexPath.row] .id - }), - let newLastVisibleIndex: Int = updatedData[newSectionIndex].elements - .firstIndex(where: { item in - item.id == self.viewModel.interactionData[oldSectionIndex] - .elements[lastVisibleIndexPath.row] - .id }) - else { - return ItemChangeInfo( - insertLocation: .none, - wasCloseToBottom: isCloseToBottom, - sentMessageBeforeUpdate: self.viewModel.sentMessageBeforeUpdate - ) - } + else { return nil } return ItemChangeInfo( - insertLocation: { - let insertedAtTop: Bool = ( - newSectionIndex > oldSectionIndex || - newFirstItemIndex > 0 - ) - let insertedAtBot: Bool = ( - newSectionIndex < oldSectionIndex || - newLastItemIndex < (updatedData[newSectionIndex].elements.count - 1) - ) - - // If anything was inserted at the top then we need to maintain the current - // offset so always return a 'top' insert location - switch (insertedAtTop, insertedAtBot, isLoadingMore) { - case (true, _, true), (true, false, false): return .top - case (false, true, _): return .bottom - case (false, false, _), (true, true, false): return .other - } - }(), - wasCloseToBottom: isCloseToBottom, - sentMessageBeforeUpdate: self.viewModel.sentMessageBeforeUpdate, + isInsertAtTop: ( + newSectionIndex > oldSectionIndex || + newFirstItemIndex > 0 + ), firstIndexIsVisible: (firstVisibleIndexPath.row == 0), - visibleInteractionId: updatedData[newSectionIndex].elements[newVisibleIndex].id, visibleIndexPath: IndexPath(row: newVisibleIndex, section: newSectionIndex), - oldVisibleIndexPath: firstVisibleIndexPath, - lastVisibleIndexPath: IndexPath(row: newLastVisibleIndex, section: newSectionIndex) + oldVisibleIndexPath: firstVisibleIndexPath ) }() + guard !isInsert || wasLoadingMore || itemChangeInfo?.isInsertAtTop == true else { + self.viewModel.updateInteractionData(updatedData) + self.tableView.reloadData() + + // Animate to the target interaction (or the bottom) after a slightly delay to prevent buggy + // animation conflicts + if let focusedInteractionId: Int64 = self.focusedInteractionId { + // If we had a focusedInteractionId then scroll to it (and hide the search + // result bar loading indicator) + DispatchQueue.main.asyncAfter(deadline: .now() + .milliseconds(100)) { [weak self] in + self?.searchController.resultsBar.stopLoading() + self?.scrollToInteractionIfNeeded( + with: focusedInteractionId, + isAnimated: true, + highlight: (self?.shouldHighlightNextScrollToInteraction == true) + ) + } + } + else if wasOffsetCloseToBottom { + // Scroll to the bottom if an interaction was just inserted and we either + // just sent a message or are close enough to the bottom (wait a tiny fraction + // to avoid buggy animation behaviour) + DispatchQueue.main.asyncAfter(deadline: .now() + .milliseconds(100)) { [weak self] in + self?.scrollToBottom(isAnimated: true) + } + } + return + } + /// UITableView doesn't really support bottom-aligned content very well and as such jumps around a lot when inserting content but /// we want to maintain the current offset from before the data was inserted (except when adding at the bottom while the user is at /// the bottom, in which case we want to scroll down) /// /// Unfortunately the UITableView also does some weird things when updating (where it won't have updated it's internal data until /// after it performs the next layout); the below code checks a condition on layout and if it passes it calls a closure - if itemChangeInfo.insertLocation == .top { - let cellSorting: (MessageCell, MessageCell) -> Bool = { lhs, rhs -> Bool in - if !lhs.isHidden && rhs.isHidden { return true } - if lhs.isHidden && !rhs.isHidden { return false } - - return (lhs.frame.minY < rhs.frame.minY) - } - let oldRect: CGRect = (self.tableView.subviews - .compactMap { $0 as? MessageCell } - .sorted(by: cellSorting) - .first(where: { cell -> Bool in cell.viewModel?.id == itemChangeInfo.visibleInteractionId })? - .frame) - .defaulting(to: self.tableView.rectForRow(at: itemChangeInfo.oldVisibleIndexPath)) + if let itemChangeInfo: ItemChangeInfo = itemChangeInfo, (itemChangeInfo.isInsertAtTop || wasLoadingMore) { + let oldCellHeight: CGFloat = self.tableView.rectForRow(at: itemChangeInfo.oldVisibleIndexPath).height // The the user triggered the 'scrollToTop' animation (by tapping in the nav bar) then we // need to stop the animation before attempting to lock the offset (otherwise things break) @@ -778,8 +757,10 @@ final class ConversationVC: BaseVC, OWSConversationSettingsViewDelegate, Convers .height) .defaulting(to: 0) } - let newTargetRect: CGRect? = self?.tableView.rectForRow(at: itemChangeInfo.visibleIndexPath) - let heightDiff: CGFloat = (oldRect.height - (newTargetRect ?? oldRect).height) + let newTargetHeight: CGFloat? = self?.tableView + .rectForRow(at: itemChangeInfo.visibleIndexPath) + .height + let heightDiff: CGFloat = (oldCellHeight - (newTargetHeight ?? oldCellHeight)) self?.tableView.contentOffset.y += (calculatedRowHeights - heightDiff) } @@ -803,61 +784,31 @@ final class ConversationVC: BaseVC, OWSConversationSettingsViewDelegate, Convers } ) } - else if itemChangeInfo.insertLocation == .bottom || itemChangeInfo.insertLocation == .other { - CATransaction.begin() - CATransaction.setCompletionBlock { [weak self] in - if let focusedInteractionId: Int64 = self?.focusedInteractionId { - // If we had a focusedInteractionId then scroll to it (and hide the search - // result bar loading indicator) - self?.searchController.resultsBar.stopLoading() - self?.scrollToInteractionIfNeeded( - with: focusedInteractionId, - isAnimated: true, - highlight: (self?.shouldHighlightNextScrollToInteraction == true) - ) - } - else if itemChangeInfo.sentMessageBeforeUpdate || itemChangeInfo.wasCloseToBottom { - // Scroll to the bottom if an interaction was just inserted and we either - // just sent a message or are close enough to the bottom - self?.scrollToBottom(isAnimated: true) - } - } - } - // Reload the table content (animate changes if we aren't inserting at the top) self.tableView.reload( using: changeset, deleteSectionsAnimation: .none, insertSectionsAnimation: .none, reloadSectionsAnimation: .none, deleteRowsAnimation: .bottom, - insertRowsAnimation: .bottom, + insertRowsAnimation: .none, reloadRowsAnimation: .none, - interrupt: { itemChangeInfo.insertLocation == .top || $0.changeCount > ConversationViewModel.pageSize } + interrupt: { itemChangeInfo?.isInsertAtTop == true || $0.changeCount > ConversationViewModel.pageSize } ) { [weak self] updatedData in self?.viewModel.updateInteractionData(updatedData) } - - if itemChangeInfo.insertLocation == .bottom || itemChangeInfo.insertLocation == .other { - CATransaction.commit() - } - - // Mark received messages as read - viewModel.markAllAsRead() - viewModel.sentMessageBeforeUpdate = false } private func performInitialScrollIfNeeded() { - guard !didFinishInitialLayout && hasLoadedInitialThreadData && hasLoadedInitialInteractionData else { return } + guard !hasPerformedInitialScroll && hasLoadedInitialThreadData && hasLoadedInitialInteractionData else { + return + } // Scroll to the last unread message if possible; otherwise scroll to the bottom. // When the unread message count is more than the number of view items of a page, // the screen will scroll to the bottom instead of the first unread message if let focusedInteractionId: Int64 = self.viewModel.focusedInteractionId { self.scrollToInteractionIfNeeded(with: focusedInteractionId, isAnimated: false, highlight: true) - } - else if let firstUnreadInteractionId: Int64 = self.viewModel.threadData.threadFirstUnreadInteractionId { - self.scrollToInteractionIfNeeded(with: firstUnreadInteractionId, position: .top, isAnimated: false) self.unreadCountView.alpha = self.scrollButton.alpha } else { @@ -865,6 +816,7 @@ final class ConversationVC: BaseVC, OWSConversationSettingsViewDelegate, Convers } self.scrollButton.alpha = self.getScrollButtonOpacity() + self.hasPerformedInitialScroll = true // Now that the data has loaded we need to check if either of the "load more" sections are // visible and trigger them if so @@ -1187,7 +1139,7 @@ final class ConversationVC: BaseVC, OWSConversationSettingsViewDelegate, Convers } func tableView(_ tableView: UITableView, willDisplayHeaderView view: UIView, forSection section: Int) { - guard self.didFinishInitialLayout && !self.isLoadingMore else { return } + guard self.hasPerformedInitialScroll && !self.isLoadingMore else { return } let section: ConversationViewModel.SectionModel = self.viewModel.interactionData[section] @@ -1264,6 +1216,7 @@ final class ConversationVC: BaseVC, OWSConversationSettingsViewDelegate, Convers self.shouldHighlightNextScrollToInteraction else { self.focusedInteractionId = nil + self.shouldHighlightNextScrollToInteraction = false return } @@ -1439,16 +1392,16 @@ final class ConversationVC: BaseVC, OWSConversationSettingsViewDelegate, Convers animated: (self.didFinishInitialLayout && isAnimated) ) - // Don't clear these values if we have't done the initial layout (we will call this - // method a second time to trigger the highlight after the screen appears) - guard self.didFinishInitialLayout else { return } - - self.focusedInteractionId = nil - self.shouldHighlightNextScrollToInteraction = false - + // If we haven't finished the initial layout then we want to delay the highlight slightly + // so it doesn't look buggy with the push transition if highlight { - self.highlightCellIfNeeded(interactionId: interactionId) + DispatchQueue.main.asyncAfter(deadline: .now() + .milliseconds(self.didFinishInitialLayout ? 0 : 150)) { [weak self] in + self?.highlightCellIfNeeded(interactionId: interactionId) + } } + + self.shouldHighlightNextScrollToInteraction = false + self.focusedInteractionId = nil return } diff --git a/Session/Conversations/ConversationViewModel.swift b/Session/Conversations/ConversationViewModel.swift index 2248cb7cc..3881a4ccb 100644 --- a/Session/Conversations/ConversationViewModel.swift +++ b/Session/Conversations/ConversationViewModel.swift @@ -53,9 +53,27 @@ public class ConversationViewModel: OWSAudioPlayerDelegate { // MARK: - Initialization init(threadId: String, threadVariant: SessionThread.Variant, focusedInteractionId: Int64?) { + // If we have a specified 'focusedInteractionId' then use that, otherwise retrieve the oldest + // unread interaction and start focused around that one + let targetInteractionId: Int64? = { + if let focusedInteractionId: Int64 = focusedInteractionId { return focusedInteractionId } + + return GRDBStorage.shared.read { db in + let interaction: TypedTableAlias = TypedTableAlias() + + return try Interaction + .select(.id) + .filter(interaction[.wasRead] == false) + .filter(interaction[.threadId] == threadId) + .order(interaction[.timestampMs].asc) + .asRequest(of: Int64.self) + .fetchOne(db) + } + }() + self.threadId = threadId self.initialThreadVariant = threadVariant - self.focusedInteractionId = focusedInteractionId + self.focusedInteractionId = targetInteractionId self.pagedDataObserver = nil // Note: Since this references self we need to finish initializing before setting it, we @@ -68,7 +86,7 @@ public class ConversationViewModel: OWSAudioPlayerDelegate { DispatchQueue.global(qos: .default).async { [weak self] in // If we don't have a `initialFocusedId` then default to `.pageBefore` (it'll query // from a `0` offset) - guard let initialFocusedId: Int64 = focusedInteractionId else { + guard let initialFocusedId: Int64 = targetInteractionId else { self?.pagedDataObserver?.load(.pageBefore) return } @@ -216,8 +234,11 @@ public class ConversationViewModel: OWSAudioPlayerDelegate { prevModel: (index > 0 ? sortedData[index - 1] : nil), nextModel: (index < (sortedData.count - 1) ? sortedData[index + 1] : nil), isLast: ( + // The database query sorts by timestampMs descending so the "last" + // interaction will actually have a 'pageOffset' of '0' even though + // it's the last element in the 'sortedData' array index == (sortedData.count - 1) && - pageInfo.currentCount == pageInfo.totalCount + pageInfo.pageOffset == 0 ) ) } diff --git a/Session/Conversations/Message Cells/Content Views/QuoteView.swift b/Session/Conversations/Message Cells/Content Views/QuoteView.swift index b5db4cd45..d17e1dc24 100644 --- a/Session/Conversations/Message Cells/Content Views/QuoteView.swift +++ b/Session/Conversations/Message Cells/Content Views/QuoteView.swift @@ -84,7 +84,7 @@ final class QuoteView: UIView { // Subtract smallSpacing twice; once for the spacing in between the stack view elements and // once for the trailing margin. - if attachment != nil { + if attachment == nil { availableWidth = maxWidth - 2 * hInset - Values.accentLineThickness - 2 * smallSpacing } else { diff --git a/Session/Meta/AppDelegate.swift b/Session/Meta/AppDelegate.swift index cbe6f8f5e..8d79922a8 100644 --- a/Session/Meta/AppDelegate.swift +++ b/Session/Meta/AppDelegate.swift @@ -299,6 +299,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UNUserNotificationCenterD return } + self.hasInitialRootViewController = true self.window?.rootViewController = OWSNavigationController( rootViewController: (Identity.userExists() ? HomeVC() : diff --git a/SessionMessagingKit/Database/LegacyDatabase/SMKLegacy.swift b/SessionMessagingKit/Database/LegacyDatabase/SMKLegacy.swift index 25273ebfb..b27cc5e9e 100644 --- a/SessionMessagingKit/Database/LegacyDatabase/SMKLegacy.swift +++ b/SessionMessagingKit/Database/LegacyDatabase/SMKLegacy.swift @@ -1312,19 +1312,51 @@ public enum SMKLegacy { case call case messageRequestAccepted = 99 } + public enum _InfoMessageCallState: Int { + case incoming + case outgoing + case missed + case permissionDenied + case unknown + } public var wasRead: Bool public var messageType: _InfoMessageType + public var callState: _InfoMessageCallState public var customMessage: String? // MARK: NSCoder public required init(coder: NSCoder) { + let parsedMessageType: _InfoMessageType = _InfoMessageType(rawValue: (coder.decodeObject(forKey: "messageType") as! NSNumber).intValue)! + let rawCallState: Int? = (coder.decodeObject(forKey: "callState") as? NSNumber)?.intValue + self.wasRead = (coder.decodeObject(forKey: "read") as? Bool) // Note: 'read' is the correct key .defaulting(to: false) - self.messageType = _InfoMessageType(rawValue: (coder.decodeObject(forKey: "messageType") as! NSNumber).intValue)! self.customMessage = coder.decodeObject(forKey: "customMessage") as? String + switch (parsedMessageType, rawCallState) { + // Note: There was a period of time where the 'messageType' value for both 'call' and + // 'messageRequestAccepted' was the same, this code is here to handle any messages which + // might have been mistakenly identified as 'call' messages when they should be seen as + // 'messageRequestAccepted' messages (hard-coding a timestamp to be sure that any calls + // after the value was changed are correctly identified as 'unknown') + case (.call, .none): + guard (coder.decodeObject(forKey: "timestamp") as? UInt64 ?? 0) < 1647500000000 else { + fallthrough + } + + self.messageType = .messageRequestAccepted + self.callState = .unknown + + default: + self.messageType = parsedMessageType + self.callState = _InfoMessageCallState( + rawValue: (rawCallState ?? _InfoMessageCallState.unknown.rawValue) + ) + .defaulting(to: .unknown) + } + super.init(coder: coder) } } diff --git a/SessionMessagingKit/Database/Migrations/_003_YDBToGRDBMigration.swift b/SessionMessagingKit/Database/Migrations/_003_YDBToGRDBMigration.swift index 1faa74994..6c0005eb8 100644 --- a/SessionMessagingKit/Database/Migrations/_003_YDBToGRDBMigration.swift +++ b/SessionMessagingKit/Database/Migrations/_003_YDBToGRDBMigration.swift @@ -419,6 +419,15 @@ enum _003_YDBToGRDBMigration: Migration { legacyBlockedSessionIds.contains(legacyContact.sessionID) ) + /// Looks like there are some cases where conversations would be visible in the old version but wouldn't in the new version + /// it seems to be related to the `isApproved` and `didApproveMe` not being set correctly somehow, this logic is to + /// ensure the flags are set correctly based on sent/received messages + let interactionsForContact: [SMKLegacy._DBInteraction] = (interactions["\(SMKLegacy.contactThreadPrefix)\(legacyContact.sessionID)"] ?? []) + let shouldForceIsApproved: Bool = interactionsForContact + .contains(where: { $0 is SMKLegacy._DBOutgoingMessage }) + let shouldForceDidApproveMe: Bool = interactionsForContact + .contains(where: { $0 is SMKLegacy._DBIncomingMessage }) + // Determine if this contact is a "real" contact (don't want to create contacts for // every user in the new structure but still want profiles for every user) if @@ -430,7 +439,9 @@ enum _003_YDBToGRDBMigration: Migration { legacyContact.hasBeenBlocked || shouldForceTrustContact || shouldForceApproveContact || - shouldForceBlockContact + shouldForceBlockContact || + shouldForceIsApproved || + shouldForceDidApproveMe { // Create the contact try Contact( @@ -443,7 +454,8 @@ enum _003_YDBToGRDBMigration: Migration { isApproved: ( isCurrentUser || legacyContact.isApproved || - shouldForceApproveContact + shouldForceApproveContact || + shouldForceIsApproved ), isBlocked: ( !isCurrentUser && ( @@ -454,7 +466,8 @@ enum _003_YDBToGRDBMigration: Migration { didApproveMe: ( isCurrentUser || legacyContact.didApproveMe || - shouldForceApproveContact + shouldForceApproveContact || + shouldForceDidApproveMe ), hasBeenBlocked: (!isCurrentUser && (legacyContact.hasBeenBlocked || legacyContact.isBlocked)) ).insert(db) @@ -599,33 +612,30 @@ enum _003_YDBToGRDBMigration: Migration { ).insert(db) } - // Only create the 'GroupMember' models if the current user is actually a member - // of the group (if the user has left the group or been removed from it we now - // delete all of these records so want this to behave the same way) - if groupModel.groupMemberIds.contains(currentUserPublicKey) { - try groupModel.groupMemberIds.forEach { memberId in - try GroupMember( - groupId: threadId, - profileId: memberId, - role: .standard - ).insert(db) - } - - try groupModel.groupAdminIds.forEach { adminId in - try GroupMember( - groupId: threadId, - profileId: adminId, - role: .admin - ).insert(db) - } - - try (closedGroupZombieMemberIds[legacyThread.uniqueId] ?? []).forEach { zombieId in - try GroupMember( - groupId: threadId, - profileId: zombieId, - role: .zombie - ).insert(db) - } + // Create the 'GroupMember' models for the group (even if the current user is no longer + // a member as these objects are used to generate the group avatar icon) + try groupModel.groupMemberIds.forEach { memberId in + try GroupMember( + groupId: threadId, + profileId: memberId, + role: .standard + ).insert(db) + } + + try groupModel.groupAdminIds.forEach { adminId in + try GroupMember( + groupId: threadId, + profileId: adminId, + role: .admin + ).insert(db) + } + + try (closedGroupZombieMemberIds[legacyThread.uniqueId] ?? []).forEach { zombieId in + try GroupMember( + groupId: threadId, + profileId: zombieId, + role: .zombie + ).insert(db) } } @@ -746,27 +756,53 @@ enum _003_YDBToGRDBMigration: Migration { // way to determine who actually triggered the info message authorId = currentUserPublicKey body = { - // Note: The 'DisappearingConfigurationUpdateInfoMessage' stored additional info and constructed - // a string at display time so we want to continue that behaviour - guard - infoMessage.messageType == .disappearingMessagesUpdate, - let updateMessage: SMKLegacy._DisappearingConfigurationUpdateInfoMessage = infoMessage as? SMKLegacy._DisappearingConfigurationUpdateInfoMessage, - let infoMessageData: Data = try? JSONEncoder().encode( - DisappearingMessagesConfiguration.MessageInfo( - senderName: updateMessage.createdByRemoteName, - isEnabled: updateMessage.configurationIsEnabled, - durationSeconds: TimeInterval(updateMessage.configurationDurationSeconds) + // Note: Some message types stored additional info and constructed a string + // at display time, instead we encode the data into the body of the message + // as JSON so we want to continue that behaviour but not change the database + // structure for some edge cases + switch infoMessage.messageType { + case .disappearingMessagesUpdate: + guard + let updateMessage: SMKLegacy._DisappearingConfigurationUpdateInfoMessage = infoMessage as? SMKLegacy._DisappearingConfigurationUpdateInfoMessage, + let infoMessageData: Data = try? JSONEncoder().encode( + DisappearingMessagesConfiguration.MessageInfo( + senderName: updateMessage.createdByRemoteName, + isEnabled: updateMessage.configurationIsEnabled, + durationSeconds: TimeInterval(updateMessage.configurationDurationSeconds) + ) + ), + let infoMessageString: String = String(data: infoMessageData, encoding: .utf8) + else { break } + + return infoMessageString + + case .call: + let messageInfo: CallMessage.MessageInfo = CallMessage.MessageInfo( + state: { + switch infoMessage.callState { + case .incoming: return .incoming + case .outgoing: return .outgoing + case .missed: return .missed + case .permissionDenied: return .permissionDenied + case .unknown: return .unknown + } + }() ) - ), - let infoMessageString: String = String(data: infoMessageData, encoding: .utf8) - else { - return ((infoMessage.body ?? "").isEmpty ? - infoMessage.customMessage : - infoMessage.body - ) + + guard + let messageInfoData: Data = try? JSONEncoder().encode(messageInfo), + let messageInfoDataString: String = String(data: messageInfoData, encoding: .utf8) + else { break } + + return messageInfoDataString + + default: break } - return infoMessageString + return ((infoMessage.body ?? "").isEmpty ? + infoMessage.customMessage : + infoMessage.body + ) }() wasRead = infoMessage.wasRead expiresInSeconds = nil // Info messages don't expire diff --git a/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+ClosedGroups.swift b/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+ClosedGroups.swift index 764830c23..d398e4b47 100644 --- a/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+ClosedGroups.swift +++ b/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+ClosedGroups.swift @@ -280,8 +280,9 @@ extension MessageReceiver { } // Remove any 'zombie' versions of the added members (in case they were re-added) - _ = try closedGroup - .zombies + _ = try GroupMember + .filter(GroupMember.Columns.groupId == id) + .filter(GroupMember.Columns.role == GroupMember.Role.zombie) .filter(addedMembers.contains(GroupMember.Columns.profileId)) .deleteAll(db) @@ -333,6 +334,13 @@ extension MessageReceiver { return SNLog("Ignoring invalid closed group update.") } + // Delete the removed members + try GroupMember + .filter(GroupMember.Columns.groupId == id) + .filter(removedMembers.contains(GroupMember.Columns.profileId)) + .filter([ GroupMember.Role.standard, GroupMember.Role.zombie ].contains(GroupMember.Columns.role)) + .deleteAll(db) + // If the current user was removed: // • Stop polling for the group // • Remove the key pairs associated with the group @@ -343,10 +351,6 @@ extension MessageReceiver { if wasCurrentUserRemoved { ClosedGroupPoller.shared.stopPolling(for: id) - try closedGroup - .allMembers - .deleteAll(db) - _ = try closedGroup .keyPairs .deleteAll(db) @@ -357,15 +361,6 @@ extension MessageReceiver { publicKey: userPublicKey ) } - else { - // Remove the member from the group and it's zombies - try closedGroup.members - .filter(removedMembers.contains(GroupMember.Columns.profileId)) - .deleteAll(db) - try closedGroup.zombies - .filter(removedMembers.contains(GroupMember.Columns.profileId)) - .deleteAll(db) - } // Notify the user if needed guard members != Set(groupMembers.map { $0.profileId }) else { return } @@ -418,15 +413,16 @@ extension MessageReceiver { .asSet() .subtracting(membersToRemove.map { $0.profileId }) + // Delete the members to remove + try GroupMember + .filter(GroupMember.Columns.groupId == id) + .filter(updatedMemberIds.contains(GroupMember.Columns.profileId)) + .deleteAll(db) if didAdminLeave || sender == userPublicKey { // Remove the group from the database and unsubscribe from PNs ClosedGroupPoller.shared.stopPolling(for: id) - try closedGroup - .allMembers - .deleteAll(db) - _ = try closedGroup .keyPairs .deleteAll(db) @@ -438,12 +434,7 @@ extension MessageReceiver { ) } else { - // Delete all old user roles and re-add them as a zombie - try closedGroup - .allMembers - .filter(GroupMember.Columns.profileId == sender) - .deleteAll(db) - + // Re-add the removed member as a zombie try GroupMember( groupId: id, profileId: sender, diff --git a/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+VisibleMessages.swift b/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+VisibleMessages.swift index 0bd20d00a..97806cff0 100644 --- a/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+VisibleMessages.swift +++ b/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+VisibleMessages.swift @@ -125,8 +125,15 @@ extension MessageReceiver { linkPreviewUrl: (message.linkPreview?.url ?? message.openGroupInvitation?.url), // Keep track of the open group server message ID ↔ message ID relationship openGroupServerMessageId: message.openGroupServerMessageId.map { Int64($0) }, - openGroupWhisperMods: false, - openGroupWhisperTo: nil + openGroupWhisperMods: (message.recipient?.contains(".mods") == true), + openGroupWhisperTo: { + guard + let recipientParts: [String] = message.recipient?.components(separatedBy: "."), + recipientParts.count >= 3 // 'server.roomToken.whisperTo.whisperMods' + else { return nil } + + return recipientParts[2] + }() ).inserted(db) } catch { @@ -170,7 +177,7 @@ extension MessageReceiver { variant: variant, syncTarget: message.syncTarget ) - + // Parse & persist attachments let attachments: [Attachment] = try dataMessage.attachments .compactMap { proto -> Attachment? in diff --git a/SessionMessagingKit/Sending & Receiving/Message Handling/MessageSender+ClosedGroups.swift b/SessionMessagingKit/Sending & Receiving/Message Handling/MessageSender+ClosedGroups.swift index 374c13cec..d9332c814 100644 --- a/SessionMessagingKit/Sending & Receiving/Message Handling/MessageSender+ClosedGroups.swift +++ b/SessionMessagingKit/Sending & Receiving/Message Handling/MessageSender+ClosedGroups.swift @@ -409,19 +409,11 @@ extension MessageSender { .map { $0.profileId } let members: Set = Set(groupMemberIds).subtracting(removedMembers) - // Update zombie * member list - let profileIdsToRemove: Set = allGroupMembers - .filter { member in - removedMembers.contains(member.profileId) && ( - member.role == .standard || - member.role == .zombie - ) - } - .map { $0.profileId } - .asSet() + // Update zombie & member list try GroupMember .filter(GroupMember.Columns.groupId == thread.id) - .filter(profileIdsToRemove.contains(GroupMember.Columns.profileId)) + .filter(removedMembers.contains(GroupMember.Columns.profileId)) + .filter([ GroupMember.Role.standard, GroupMember.Role.zombie ].contains(GroupMember.Columns.role)) .deleteAll(db) let interactionId: Int64? @@ -522,7 +514,7 @@ extension MessageSender { ClosedGroupPoller.shared.stopPolling(for: groupPublicKey) GRDBStorage.shared.write { db in - _ = try closedGroup + try closedGroup .keyPairs .deleteAll(db) @@ -535,10 +527,24 @@ extension MessageSender { } .map { _ in } - // Update the group - _ = try closedGroup - .allMembers - .deleteAll(db) + // Update the group (if the admin leaves the group is disbanded) + let wasAdminUser: Bool = try GroupMember + .filter(GroupMember.Columns.groupId == thread.id) + .filter(GroupMember.Columns.profileId == userPublicKey) + .filter(GroupMember.Columns.role == GroupMember.Role.admin) + .isNotEmpty(db) + + if wasAdminUser { + try GroupMember + .filter(GroupMember.Columns.groupId == thread.id) + .deleteAll(db) + } + else { + try GroupMember + .filter(GroupMember.Columns.groupId == thread.id) + .filter(GroupMember.Columns.profileId == userPublicKey) + .deleteAll(db) + } // Return return promise diff --git a/SessionMessagingKit/Sending & Receiving/MessageSender.swift b/SessionMessagingKit/Sending & Receiving/MessageSender.swift index e870118be..ef3b3e3da 100644 --- a/SessionMessagingKit/Sending & Receiving/MessageSender.swift +++ b/SessionMessagingKit/Sending & Receiving/MessageSender.swift @@ -652,8 +652,6 @@ public final class MessageSender { mostRecentFailureText: error.localizedDescription ).save(db) } - - // Remove the message timestamps if it fails } // MARK: - Convenience diff --git a/SessionMessagingKit/Shared Models/SessionThreadViewModel.swift b/SessionMessagingKit/Shared Models/SessionThreadViewModel.swift index 5becf86be..8e0e49d4d 100644 --- a/SessionMessagingKit/Shared Models/SessionThreadViewModel.swift +++ b/SessionMessagingKit/Shared Models/SessionThreadViewModel.swift @@ -30,7 +30,6 @@ public struct SessionThreadViewModel: FetchableRecord, Decodable, Equatable, Has public static let threadContactIsTypingKey: SQL = SQL(stringLiteral: CodingKeys.threadContactIsTyping.stringValue) public static let threadUnreadCountKey: SQL = SQL(stringLiteral: CodingKeys.threadUnreadCount.stringValue) public static let threadUnreadMentionCountKey: SQL = SQL(stringLiteral: CodingKeys.threadUnreadMentionCount.stringValue) - public static let threadFirstUnreadInteractionIdKey: SQL = SQL(stringLiteral: CodingKeys.threadFirstUnreadInteractionId.stringValue) public static let contactProfileKey: SQL = SQL(stringLiteral: CodingKeys.contactProfile.stringValue) public static let closedGroupNameKey: SQL = SQL(stringLiteral: CodingKeys.closedGroupName.stringValue) public static let closedGroupUserCountKey: SQL = SQL(stringLiteral: CodingKeys.closedGroupUserCount.stringValue) @@ -57,7 +56,6 @@ public struct SessionThreadViewModel: FetchableRecord, Decodable, Equatable, Has public static let threadUnreadCountString: String = CodingKeys.threadUnreadCount.stringValue public static let threadUnreadMentionCountString: String = CodingKeys.threadUnreadMentionCount.stringValue - public static let threadFirstUnreadInteractionIdString: String = CodingKeys.threadFirstUnreadInteractionId.stringValue public static let closedGroupUserCountString: String = CodingKeys.closedGroupUserCount.stringValue public static let openGroupUserCountString: String = CodingKeys.openGroupUserCount.stringValue public static let contactProfileString: String = CodingKeys.contactProfile.stringValue @@ -86,7 +84,6 @@ public struct SessionThreadViewModel: FetchableRecord, Decodable, Equatable, Has public let threadContactIsTyping: Bool? public let threadUnreadCount: UInt? public let threadUnreadMentionCount: UInt? - public let threadFirstUnreadInteractionId: Int64? // Thread display info @@ -214,7 +211,6 @@ public extension SessionThreadViewModel { self.threadContactIsTyping = nil self.threadUnreadCount = unreadCount self.threadUnreadMentionCount = nil - self.threadFirstUnreadInteractionId = nil // Thread display info @@ -496,7 +492,6 @@ public extension SessionThreadViewModel { let openGroup: TypedTableAlias = TypedTableAlias() let interaction: TypedTableAlias = TypedTableAlias() - let firstUnreadInteractionTableLiteral: SQL = SQL(stringLiteral: "\(ViewModel.threadFirstUnreadInteractionIdString)_table") let interactionIdLiteral: SQL = SQL(stringLiteral: Interaction.Columns.id.name) let interactionThreadIdLiteral: SQL = SQL(stringLiteral: Interaction.Columns.threadId.name) let closedGroupUserCountTableLiteral: SQL = SQL(stringLiteral: "\(ViewModel.closedGroupUserCountString)_table") @@ -508,7 +503,7 @@ public extension SessionThreadViewModel { /// parse and might throw /// /// Explicitly set default values for the fields ignored for search results - let numColumnsBeforeProfiles: Int = 15 + let numColumnsBeforeProfiles: Int = 13 let request: SQLRequest = """ SELECT \(thread[.id]) AS \(ViewModel.threadIdKey), @@ -540,8 +535,6 @@ public extension SessionThreadViewModel { \(thread[.messageDraft]) AS \(ViewModel.threadMessageDraftKey), \(Interaction.self).\(ViewModel.threadUnreadCountKey), - \(Interaction.self).\(ViewModel.threadUnreadMentionCountKey), - \(firstUnreadInteractionTableLiteral).\(interactionIdLiteral) AS \(ViewModel.threadFirstUnreadInteractionIdKey), \(ViewModel.contactProfileKey).*, \(closedGroup[.name]) AS \(ViewModel.closedGroupNameKey), @@ -550,7 +543,6 @@ public extension SessionThreadViewModel { \(openGroup[.name]) AS \(ViewModel.openGroupNameKey), \(openGroup[.server]) AS \(ViewModel.openGroupServerKey), \(openGroup[.roomToken]) AS \(ViewModel.openGroupRoomTokenKey), - \(openGroup[.imageData]) AS \(ViewModel.openGroupProfilePictureDataKey), \(openGroup[.userCount]) AS \(ViewModel.openGroupUserCountKey), \(Interaction.self).\(ViewModel.interactionIdKey), @@ -566,23 +558,11 @@ public extension SessionThreadViewModel { \(interaction[.threadId]), MAX(\(interaction[.timestampMs])), - SUM(\(interaction[.wasRead]) = false) AS \(ViewModel.threadUnreadCountKey), - SUM(\(interaction[.wasRead]) = false AND \(interaction[.hasMention]) = true) AS \(ViewModel.threadUnreadMentionCountKey) + SUM(\(interaction[.wasRead]) = false) AS \(ViewModel.threadUnreadCountKey) FROM \(Interaction.self) GROUP BY \(interaction[.threadId]) ) AS \(Interaction.self) ON \(interaction[.threadId]) = \(thread[.id]) - LEFT JOIN ( - SELECT - \(interaction[.id]), - \(interaction[.threadId]), - MIN(\(interaction[.timestampMs])) - FROM \(Interaction.self) - WHERE ( - \(interaction[.wasRead]) = false AND - \(SQL("\(interaction[.threadId]) = \(threadId)")) - ) - ) AS \(firstUnreadInteractionTableLiteral) ON \(firstUnreadInteractionTableLiteral).\(interactionThreadIdLiteral) = \(thread[.id]) LEFT JOIN \(Profile.self) AS \(ViewModel.contactProfileKey) ON \(ViewModel.contactProfileKey).\(profileIdColumnLiteral) = \(thread[.id]) LEFT JOIN \(OpenGroup.self) ON \(openGroup[.threadId]) = \(thread[.id])