From ff2d96e0d5febe92feee8753fd000fbd25b2db34 Mon Sep 17 00:00:00 2001
From: Morgan Pretty <morgan.t.pretty@gmail.com>
Date: Mon, 27 Jun 2022 17:57:46 +1000
Subject: [PATCH] Fixed a handful more bugs
Fixed an issue where I'd shifted push notification logic to a background thread resulting in crashes
Fixed a bug where the status indicator view on the FullConversationCell was incorrectly showing for incoming messages
Fixed a bug where outgoing messages to closed groups would all be flagged as failed to send
Fixed a bug with the "autoLoadNextPageIfNeeded" on the conversation screen
Fixed a bug where the input view on a closed group wouldn't appear correctly based on whether the user was a member or not
Added the "autoLoadNextPageIfNeeded" logic to the home screen
---
Session/Conversations/ConversationVC.swift | 9 ++---
.../Conversations/ConversationViewModel.swift | 15 +++++++-
Session/Home/HomeVC.swift | 34 ++++++++++++++++++-
Session/Home/HomeViewModel.swift | 2 +-
Session/Notifications/SyncPushTokensJob.swift | 27 ++-------------
Session/Shared/FullConversationCell.swift | 2 +-
.../Database/Models/Interaction.swift | 19 +++++++----
.../SessionThreadViewModel.swift | 15 +++++---
8 files changed, 79 insertions(+), 44 deletions(-)
diff --git a/Session/Conversations/ConversationVC.swift b/Session/Conversations/ConversationVC.swift
index 660b29e7b..c3c767ae2 100644
--- a/Session/Conversations/ConversationVC.swift
+++ b/Session/Conversations/ConversationVC.swift
@@ -591,12 +591,13 @@ final class ConversationVC: BaseVC, OWSConversationSettingsViewDelegate, Convers
snInputView.text = draft
}
+ // Now we have done all the needed diffs, update the viewModel with the latest data
+ self.viewModel.updateThreadData(updatedThreadData)
+
+ /// **Note:** This needs to happen **after** we have update the viewModel's thread data
if viewModel.threadData.currentUserIsClosedGroupMember != updatedThreadData.currentUserIsClosedGroupMember {
reloadInputViews()
}
-
- // Now we have done all the needed diffs, update the viewModel with the latest data
- self.viewModel.updateThreadData(updatedThreadData)
}
private func handleInteractionUpdates(_ updatedData: [ConversationViewModel.SectionModel], initialLoad: Bool = false) {
@@ -837,7 +838,7 @@ final class ConversationVC: BaseVC, OWSConversationSettingsViewDelegate, Convers
// Note: We sort the headers as we want to prioritise loading newer pages over older ones
let sections: [(ConversationViewModel.Section, CGRect)] = (self?.viewModel.interactionData
.enumerated()
- .map { index, section in (section.model, (self?.tableView.rectForHeader(inSection: 0) ?? .zero)) })
+ .map { index, section in (section.model, (self?.tableView.rectForHeader(inSection: index) ?? .zero)) })
.defaulting(to: [])
let shouldLoadOlder: Bool = sections
.contains { section, headerRect in
diff --git a/Session/Conversations/ConversationViewModel.swift b/Session/Conversations/ConversationViewModel.swift
index c5fb9aca5..d6e3aa314 100644
--- a/Session/Conversations/ConversationViewModel.swift
+++ b/Session/Conversations/ConversationViewModel.swift
@@ -98,7 +98,20 @@ public class ConversationViewModel: OWSAudioPlayerDelegate {
// MARK: - Thread Data
/// This value is the current state of the view
- public private(set) var threadData: SessionThreadViewModel = SessionThreadViewModel()
+ public private(set) lazy var threadData: SessionThreadViewModel = SessionThreadViewModel(
+ threadId: self.threadId,
+ threadVariant: self.initialThreadVariant,
+ currentUserIsClosedGroupMember: (self.initialThreadVariant != .closedGroup ?
+ nil :
+ GRDBStorage.shared.read { db in
+ try GroupMember
+ .filter(GroupMember.Columns.groupId == self.threadId)
+ .filter(GroupMember.Columns.profileId == getUserHexEncodedPublicKey(db))
+ .filter(GroupMember.Columns.role == GroupMember.Role.standard)
+ .isNotEmpty(db)
+ }
+ )
+ )
/// This is all the data the screen needs to populate itself, please see the following link for tips to help optimise
/// performance https://github.com/groue/GRDB.swift#valueobservation-performance
diff --git a/Session/Home/HomeVC.swift b/Session/Home/HomeVC.swift
index ca1d002b5..67c08c973 100644
--- a/Session/Home/HomeVC.swift
+++ b/Session/Home/HomeVC.swift
@@ -15,6 +15,7 @@ final class HomeVC: BaseVC, UITableViewDataSource, UITableViewDelegate, NewConve
private var hasLoadedInitialStateData: Bool = false
private var hasLoadedInitialThreadData: Bool = false
private var isLoadingMore: Bool = false
+ private var isAutoLoadingNextPage: Bool = false
// MARK: - Intialization
@@ -319,6 +320,7 @@ final class HomeVC: BaseVC, UITableViewDataSource, UITableViewDelegate, NewConve
CATransaction.setCompletionBlock { [weak self] in
// Complete page loading
self?.isLoadingMore = false
+ self?.autoLoadNextPageIfNeeded()
}
// Reload the table content (animate changes after the first load)
@@ -328,7 +330,7 @@ final class HomeVC: BaseVC, UITableViewDataSource, UITableViewDelegate, NewConve
insertSectionsAnimation: .none,
reloadSectionsAnimation: .none,
deleteRowsAnimation: .bottom,
- insertRowsAnimation: .top,
+ insertRowsAnimation: .none,
reloadRowsAnimation: .none,
interrupt: { $0.changeCount > 100 } // Prevent too many changes from causing performance issues
) { [weak self] updatedData in
@@ -338,6 +340,36 @@ final class HomeVC: BaseVC, UITableViewDataSource, UITableViewDelegate, NewConve
CATransaction.commit()
}
+ private func autoLoadNextPageIfNeeded() {
+ guard !self.isAutoLoadingNextPage && !self.isLoadingMore else { return }
+
+ self.isAutoLoadingNextPage = true
+
+ DispatchQueue.main.asyncAfter(deadline: .now() + PagedData.autoLoadNextPageDelay) { [weak self] in
+ self?.isAutoLoadingNextPage = false
+
+ // Note: We sort the headers as we want to prioritise loading newer pages over older ones
+ let sections: [(HomeViewModel.Section, CGRect)] = (self?.viewModel.threadData
+ .enumerated()
+ .map { index, section in (section.model, (self?.tableView.rectForHeader(inSection: index) ?? .zero)) })
+ .defaulting(to: [])
+ let shouldLoadMore: Bool = sections
+ .contains { section, headerRect in
+ section == .loadMore &&
+ headerRect != .zero &&
+ (self?.tableView.bounds.contains(headerRect) == true)
+ }
+
+ guard shouldLoadMore else { return }
+
+ self?.isLoadingMore = true
+
+ DispatchQueue.global(qos: .default).async { [weak self] in
+ self?.viewModel.pagedDataObserver?.load(.pageAfter)
+ }
+ }
+ }
+
private func updateNavBarButtons() {
// Profile picture view
let profilePictureSize = Values.verySmallProfilePictureSize
diff --git a/Session/Home/HomeViewModel.swift b/Session/Home/HomeViewModel.swift
index 543830292..a1ff37bfd 100644
--- a/Session/Home/HomeViewModel.swift
+++ b/Session/Home/HomeViewModel.swift
@@ -19,7 +19,7 @@ public class HomeViewModel {
// MARK: - Variables
- public static let pageSize: Int = 14
+ public static let pageSize: Int = 15
public struct State: Equatable {
let showViewedSeedBanner: Bool
diff --git a/Session/Notifications/SyncPushTokensJob.swift b/Session/Notifications/SyncPushTokensJob.swift
index 1ce1a1220..a5234e826 100644
--- a/Session/Notifications/SyncPushTokensJob.swift
+++ b/Session/Notifications/SyncPushTokensJob.swift
@@ -33,31 +33,8 @@ public enum SyncPushTokensJob: JobExecutor {
}
return
}
- let isRegisteredForRemoteNotifications: Bool = UIApplication.shared.isRegisteredForRemoteNotifications
- // Swap back to the correct queue before continuing (don't want to inadvertantly do stuff on the main
- // thread that could block the user)
- queue.async {
- SyncPushTokensJob.internalRun(
- job,
- queue: queue,
- isRegisteredForRemoteNotifications: isRegisteredForRemoteNotifications,
- success: success,
- failure: failure,
- deferred: deferred
- )
- }
- }
-
- private static func internalRun(
- _ job: Job,
- queue: DispatchQueue,
- isRegisteredForRemoteNotifications: Bool,
- success: @escaping (Job, Bool) -> (),
- failure: @escaping (Job, Error?, Bool) -> (),
- deferred: @escaping (Job) -> ()
- ) {
- guard !isRegisteredForRemoteNotifications else {
+ guard !UIApplication.shared.isRegisteredForRemoteNotifications else {
deferred(job) // Don't need to do anything if push notifications are already registered
return
}
@@ -97,7 +74,7 @@ public enum SyncPushTokensJob: JobExecutor {
)
return promise
- .done { _ in
+ .done(on: queue) { _ in
Logger.warn("Recording push tokens locally. pushToken: \(redact(pushToken)), voipToken: \(redact(voipToken))")
GRDBStorage.shared.write { db in
diff --git a/Session/Shared/FullConversationCell.swift b/Session/Shared/FullConversationCell.swift
index b131b808a..fa9973805 100644
--- a/Session/Shared/FullConversationCell.swift
+++ b/Session/Shared/FullConversationCell.swift
@@ -374,7 +374,7 @@ public final class FullConversationCell: UITableViewCell {
statusIndicatorView.isHidden = false
default:
- statusIndicatorView.isHidden = false
+ statusIndicatorView.isHidden = true
}
}
diff --git a/SessionMessagingKit/Database/Models/Interaction.swift b/SessionMessagingKit/Database/Models/Interaction.swift
index 82ea89024..d082e108d 100644
--- a/SessionMessagingKit/Database/Models/Interaction.swift
+++ b/SessionMessagingKit/Database/Models/Interaction.swift
@@ -356,13 +356,18 @@ public struct Interaction: Codable, Identifiable, Equatable, FetchableRecord, Mu
return
}
- try members.forEach { member in
- try RecipientState(
- interactionId: interactionId,
- recipientId: member.profileId,
- state: .sending
- ).insert(db)
- }
+ // Exclude the current user when creating recipient states (as they will never
+ // receive the message resulting in the message getting flagged as failed)
+ let userPublicKey: String = getUserHexEncodedPublicKey(db)
+ try members
+ .filter { member -> Bool in member.profileId != userPublicKey }
+ .forEach { member in
+ try RecipientState(
+ interactionId: interactionId,
+ recipientId: member.profileId,
+ state: .sending
+ ).insert(db)
+ }
case .openGroup:
// Since we use the 'RecipientState' type to manage the message state
diff --git a/SessionMessagingKit/Shared Models/SessionThreadViewModel.swift b/SessionMessagingKit/Shared Models/SessionThreadViewModel.swift
index a4401df7f..d4f7083d3 100644
--- a/SessionMessagingKit/Shared Models/SessionThreadViewModel.swift
+++ b/SessionMessagingKit/Shared Models/SessionThreadViewModel.swift
@@ -194,11 +194,18 @@ public struct SessionThreadViewModel: FetchableRecordWithRowId, Decodable, Equat
// MARK: - Convenience Initialization
public extension SessionThreadViewModel {
+ static let invalidId: String = "INVALID_THREAD_ID"
+
// Note: This init method is only used system-created cells or empty states
- init(unreadCount: UInt = 0) {
+ init(
+ threadId: String? = nil,
+ threadVariant: SessionThread.Variant? = nil,
+ currentUserIsClosedGroupMember: Bool? = nil,
+ unreadCount: UInt = 0
+ ) {
self.rowId = -1
- self.threadId = "INVALID_THREAD_ID"
- self.threadVariant = .contact
+ self.threadId = (threadId ?? SessionThreadViewModel.invalidId)
+ self.threadVariant = (threadVariant ?? .contact)
self.threadCreationDateTimestamp = 0
self.threadMemberNames = nil
@@ -224,7 +231,7 @@ public extension SessionThreadViewModel {
self.closedGroupProfileBackFallback = nil
self.closedGroupName = nil
self.closedGroupUserCount = nil
- self.currentUserIsClosedGroupMember = nil
+ self.currentUserIsClosedGroupMember = currentUserIsClosedGroupMember
self.currentUserIsClosedGroupAdmin = nil
self.openGroupName = nil
self.openGroupServer = nil