From ad2c9ff5a68a874e97dd3599e643dcdc316f3dec Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Wed, 16 Oct 2024 17:08:55 +1100 Subject: [PATCH] Fixed a number of bugs found when checking the acceptance criteria MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit • Fixed some incorrect copy • Fixed a bug where being the only member in a group would result in messages always having the "sending" status • Fixed a bug where accepting a group invitation on a linked device wouldn't start the poller on the current device • Fixed a bug where there could be a duplicate 'you were invited to join the group' info message after accepting the group invite • Fixed a bug where the failed invitation toast could appear multiple times • Fixed a bug where the legacy groups banner was appearing for non-admins (apparently it shouldn't be) --- Session/Closed Groups/NewClosedGroupVC.swift | 37 +++- .../ConversationVC+Interaction.swift | 8 + Session/Conversations/ConversationVC.swift | 23 ++- .../MessageRequestFooterView.swift | 4 +- .../New Conversation/NewMessageScreen.swift | 2 +- .../Database/Models/Interaction.swift | 55 +---- .../Jobs/GroupInviteMemberJob.swift | 191 ++++++++++-------- .../LibSession+UserGroups.swift | 14 +- SessionUtilitiesKit/General/Feature.swift | 3 +- 9 files changed, 197 insertions(+), 140 deletions(-) diff --git a/Session/Closed Groups/NewClosedGroupVC.swift b/Session/Closed Groups/NewClosedGroupVC.swift index dcdaa1d27..78d711cf8 100644 --- a/Session/Closed Groups/NewClosedGroupVC.swift +++ b/Session/Closed Groups/NewClosedGroupVC.swift @@ -55,6 +55,30 @@ final class NewClosedGroupVC: BaseVC, UITableViewDataSource, UITableViewDelegate private static let textFieldHeight: CGFloat = 50 private static let searchBarHeight: CGFloat = (36 + (Values.mediumSpacing * 2)) + private let contentStackView: UIStackView = { + let result: UIStackView = UIStackView() + result.axis = .vertical + result.distribution = .fill + + return result + }() + + private lazy var minVersionBanner: InfoBanner = { + let result: InfoBanner = InfoBanner( + info: InfoBanner.Info( + font: .systemFont(ofSize: Values.verySmallFontSize), + message: "groupInviteVersion".localized(), + icon: .none, + tintColor: .black, + backgroundColor: .warning, + accessibility: Accessibility(label: "Version warning banner") + ) + ) + result.isHidden = dependencies[feature: .updatedGroups] + + return result + }() + private lazy var nameTextField: TextField = { let result = TextField( placeholder: "groupNameEnter".localized(), @@ -192,11 +216,14 @@ final class NewClosedGroupVC: BaseVC, UITableViewDataSource, UITableViewDelegate return } - view.addSubview(tableView) - tableView.pin(.top, to: .top, of: view) - tableView.pin(.leading, to: .leading, of: view) - tableView.pin(.trailing, to: .trailing, of: view) - tableView.pin(.bottom, to: .bottom, of: view) + view.addSubview(contentStackView) + contentStackView.pin(.top, to: .top, of: view) + contentStackView.pin(.leading, to: .leading, of: view) + contentStackView.pin(.trailing, to: .trailing, of: view) + contentStackView.pin(.bottom, to: .bottom, of: view) + + contentStackView.addArrangedSubview(minVersionBanner) + contentStackView.addArrangedSubview(tableView) view.addSubview(fadeView) fadeView.pin(.leading, to: .leading, of: view) diff --git a/Session/Conversations/ConversationVC+Interaction.swift b/Session/Conversations/ConversationVC+Interaction.swift index 93b44dc26..bec36c777 100644 --- a/Session/Conversations/ConversationVC+Interaction.swift +++ b/Session/Conversations/ConversationVC+Interaction.swift @@ -1270,6 +1270,7 @@ extension ConversationVC: confirmStyle: .danger, cancelTitle: "urlCopy".localized(), cancelStyle: .alert_text, + hasCloseButton: true, onConfirm: { [weak self] _ in UIApplication.shared.open(url, options: [:], completionHandler: nil) self?.showInputAccessoryView() @@ -2568,6 +2569,13 @@ extension ConversationVC { return viewModel.dependencies[singleton: .storage] .writePublisher { [dependencies = viewModel.dependencies] db -> AnyPublisher in + /// Remove any existing `infoGroupInfoInvited` interactions from the group (don't want to have a duplicate one from + /// inside the group history) + _ = try Interaction + .filter(Interaction.Columns.threadId == group.id) + .filter(Interaction.Columns.variant == Interaction.Variant.infoGroupInfoInvited) + .deleteAll(db) + /// Optimistically insert a `standard` member for the current user in this group (it'll be update to the correct /// one once we receive the first `GROUP_MEMBERS` config message but adding it here means the `canWrite` /// state of the group will continue to be `true` while we wait on the initial poll to get back) diff --git a/Session/Conversations/ConversationVC.swift b/Session/Conversations/ConversationVC.swift index a213f432f..85db2ba16 100644 --- a/Session/Conversations/ConversationVC.swift +++ b/Session/Conversations/ConversationVC.swift @@ -235,17 +235,21 @@ final class ConversationVC: BaseVC, LibSessionRespondingViewController, Conversa let result: InfoBanner = InfoBanner( info: InfoBanner.Info( font: .systemFont(ofSize: Values.miniFontSize), - message: "groupInviteVersion".localized(), + message: "groupLegacyBanner" + .put(key: "date", value: Features.legacyGroupDepricationDate.formattedForBanner) + .localized(), icon: .link, tintColor: .messageBubble_outgoingText, backgroundColor: .primary, accessibility: Accessibility(label: "Legacy group banner"), - labelAccessibility: Accessibility(label: "Legacy group banner text"), height: nil, - onTap: { [weak self] in self?.openUrl("https://getsession.org/faq") } + onTap: { [weak self] in self?.openUrl(Features.legacyGroupDepricationUrl) } ) ) - result.isHidden = (self.viewModel.threadData.threadVariant != .legacyGroup) + result.isHidden = ( + self.viewModel.threadData.threadVariant != .legacyGroup || + self.viewModel.threadData.currentUserIsClosedGroupAdmin != true + ) return result }() @@ -794,6 +798,17 @@ final class ConversationVC: BaseVC, LibSessionRespondingViewController, Conversa ) } + if + initialLoad || + viewModel.threadData.threadVariant != updatedThreadData.threadVariant || + viewModel.threadData.currentUserIsClosedGroupAdmin != updatedThreadData.currentUserIsClosedGroupAdmin + { + legacyGroupsBanner.isHidden = ( + updatedThreadData.threadVariant != .legacyGroup || + updatedThreadData.currentUserIsClosedGroupAdmin != true + ) + } + if initialLoad || viewModel.threadData.threadIsBlocked != updatedThreadData.threadIsBlocked { addOrRemoveBlockedBanner(threadIsBlocked: (updatedThreadData.threadIsBlocked == true)) } diff --git a/Session/Conversations/Views & Modals/MessageRequestFooterView.swift b/Session/Conversations/Views & Modals/MessageRequestFooterView.swift index e9fb20bbd..8dbfe3c1a 100644 --- a/Session/Conversations/Views & Modals/MessageRequestFooterView.swift +++ b/Session/Conversations/Views & Modals/MessageRequestFooterView.swift @@ -63,7 +63,7 @@ class MessageRequestFooterView: UIView { result.translatesAutoresizingMaskIntoConstraints = false result.clipsToBounds = true result.titleLabel?.font = UIFont.boldSystemFont(ofSize: 16) - result.setTitle("deleteAfterGroupPR1BlockUser".localized(), for: .normal) + result.setTitle("block".localized(), for: .normal) result.setThemeTitleColor(.danger, for: .normal) result.addTarget(self, action: #selector(block), for: .touchUpInside) @@ -86,7 +86,7 @@ class MessageRequestFooterView: UIView { result.accessibilityLabel = "Delete message request" result.isAccessibilityElement = true result.translatesAutoresizingMaskIntoConstraints = false - result.setTitle("decline".localized(), for: .normal) + result.setTitle("delete".localized(), for: .normal) result.addTarget(self, action: #selector(decline), for: .touchUpInside) return result diff --git a/Session/Home/New Conversation/NewMessageScreen.swift b/Session/Home/New Conversation/NewMessageScreen.swift index d0452ecd5..a88c0d75b 100644 --- a/Session/Home/New Conversation/NewMessageScreen.swift +++ b/Session/Home/New Conversation/NewMessageScreen.swift @@ -151,7 +151,7 @@ struct EnterAccountIdScreen: View { ) ) { ZStack { - Text("\("messageNewDescriptionMobile".localized())\(Image(systemName: "questionmark.circle"))") + Text("messageNewDescriptionMobile".localized().appending("\(Image(systemName: "questionmark.circle"))")) .font(.system(size: Values.verySmallFontSize)) .foregroundColor(themeColor: .textSecondary) .multilineTextAlignment(.center) diff --git a/SessionMessagingKit/Database/Models/Interaction.swift b/SessionMessagingKit/Database/Models/Interaction.swift index 4e7dbfa64..fe8f28ef6 100644 --- a/SessionMessagingKit/Database/Models/Interaction.swift +++ b/SessionMessagingKit/Database/Models/Interaction.swift @@ -446,52 +446,15 @@ public struct Interaction: Codable, Identifiable, Equatable, FetchableRecord, Mu switch variant { case .standardOutgoing: - // New outgoing messages should immediately determine their recipient list - // from current thread state - switch threadVariant { - case .contact: - try RecipientState( - interactionId: success.rowID, - recipientId: threadId, // Will be the contact id - state: .sending - ).insert(db) - - case .legacyGroup, .group: - let closedGroupMemberIds: Set = (try? GroupMember - .select(.profileId) - .filter(GroupMember.Columns.groupId == threadId) - .asRequest(of: String.self) - .fetchSet(db)) - .defaulting(to: []) - - guard !closedGroupMemberIds.isEmpty else { - SNLog("Inserted an interaction but couldn't find it's associated thread members") - return - } - - // Exclude the current user when creating recipient states (as they will never - // receive the message resulting in the message getting flagged as failed) - let currentUserSessionId: SessionId? = transientDependencies?.value[cache: .general].sessionId - try closedGroupMemberIds - .filter { memberId -> Bool in memberId != currentUserSessionId?.hexString } - .forEach { memberId in - try RecipientState( - interactionId: success.rowID, - recipientId: memberId, - state: .sending - ).insert(db) - } - - case .community: - // Since we use the 'RecipientState' type to manage the message state - // we need to ensure we have a state for all threads; so for open groups - // we just use the open group id as the 'recipientId' value - try RecipientState( - interactionId: success.rowID, - recipientId: threadId, // Will be the open group id - state: .sending - ).insert(db) - } + /// The `RecipientState` is used to manage the sending status of a message regardless of the type of conversation it's + /// sent to, in a `contact` conversation the "recipient" is a contact and, as such, can have a `Read` state if enabled but for + /// group and community conversations the "recipient" of the message is the conversation itself so there can be no `Read` + /// state, and we only need a single record + try RecipientState( + interactionId: success.rowID, + recipientId: threadId, + state: .sending + ).insert(db) default: break } diff --git a/SessionMessagingKit/Jobs/GroupInviteMemberJob.swift b/SessionMessagingKit/Jobs/GroupInviteMemberJob.swift index 32408ea21..041c47415 100644 --- a/SessionMessagingKit/Jobs/GroupInviteMemberJob.swift +++ b/SessionMessagingKit/Jobs/GroupInviteMemberJob.swift @@ -20,10 +20,6 @@ public enum GroupInviteMemberJob: JobExecutor { public static var requiresThreadId: Bool = true public static var requiresInteractionId: Bool = false - private static let notificationDebounceDuration: DispatchQueue.SchedulerTimeType.Stride = .milliseconds(1500) - private static var notifyFailurePublisher: AnyPublisher? - private static let notifyFailureTrigger: PassthroughSubject<(), Never> = PassthroughSubject() - public static func run( _ job: Job, queue: DispatchQueue, @@ -127,11 +123,9 @@ public enum GroupInviteMemberJob: JobExecutor { } // Notify about the failure - GroupInviteMemberJob.notifyOfFailure( - groupId: threadId, - memberId: details.memberSessionIdHexString, - using: dependencies - ) + dependencies.mutate(cache: .groupInviteMemberJob) { cache in + cache.addInviteFailure(groupId: threadId, memberId: details.memberSessionIdHexString) + } // Register the failure switch error { @@ -199,88 +193,122 @@ public enum GroupInviteMemberJob: JobExecutor { .localizedFormatted(baseFont: ToastController.font) } } +} + +// MARK: - GroupInviteMemberJob Cache + +public extension GroupInviteMemberJob { + struct InviteFailure: Hashable { + let groupId: String + let memberId: String + } - private static func notifyOfFailure(groupId: String, memberId: String, using dependencies: Dependencies) { - dependencies.mutate(cache: .groupInviteMemberJob) { cache in - cache.failedMemberIds.insert(memberId) + class Cache: GroupInviteMemberJobCacheType { + private static let notificationDebounceDuration: DispatchQueue.SchedulerTimeType.Stride = .milliseconds(3000) + + private let dependencies: Dependencies + private let inviteFailedNotificationTrigger: PassthroughSubject<(), Never> = PassthroughSubject() + private var disposables: Set = Set() + public private(set) var inviteFailures: Set = [] + + // MARK: - Initialiation + + init(using dependencies: Dependencies) { + self.dependencies = dependencies + + setupInviteFailureListener() } - /// This method can be triggered by each individual invitation failure so we want to throttle the updates to 250ms so that we can group failures - /// and show a single toast - if notifyFailurePublisher == nil { - notifyFailurePublisher = notifyFailureTrigger - .debounce(for: notificationDebounceDuration, scheduler: DispatchQueue.global(qos: .userInitiated)) - .handleEvents( - receiveOutput: { [dependencies] _ in - let failedIds: [String] = dependencies.mutate(cache: .groupInviteMemberJob) { cache in - let result: Set = cache.failedMemberIds - cache.failedMemberIds.removeAll() - return Array(result) - } - - // Don't do anything if there are no 'failedIds' values or we can't get a window - guard - !failedIds.isEmpty, - let mainWindow: UIWindow = dependencies[singleton: .appContext].mainWindow - else { return } - - typealias FetchedData = (groupName: String, profileInfo: [String: Profile]) - - let data: FetchedData = dependencies[singleton: .storage] - .read { db in - ( - try ClosedGroup - .filter(id: groupId) - .select(.name) - .asRequest(of: String.self) - .fetchOne(db), - try Profile.filter(ids: failedIds).fetchAll(db) - ) - } - .map { maybeName, profiles -> FetchedData in - ( - (maybeName ?? "groupUnknown".localized()), - profiles.reduce(into: [:]) { result, next in result[next.id] = next } - ) - } - .defaulting(to: ("groupUnknown".localized(), [:])) - let message: NSAttributedString = failureMessage( - groupName: data.groupName, - memberIds: failedIds, - profileInfo: data.profileInfo - ) + // MARK: - Functions + + public func addInviteFailure(groupId: String, memberId: String) { + print("[RAWR] Add failure for: \(memberId)") + inviteFailures.insert(InviteFailure(groupId: groupId, memberId: memberId)) + inviteFailedNotificationTrigger.send(()) + } + + public func clearPendingFailures(for groupId: String) { + inviteFailures = inviteFailures.filter { $0.groupId != groupId } + } + + // MARK: - Internal Functions + + private func setupInviteFailureListener() { + inviteFailedNotificationTrigger + .subscribe(on: DispatchQueue.global(qos: .userInitiated), using: dependencies) + .debounce( + for: Cache.notificationDebounceDuration, + scheduler: DispatchQueue.global(qos: .userInitiated) + ) + .map { [dependencies] _ -> (failedInvites: Set, groupId: String) in + dependencies.mutate(cache: .groupInviteMemberJob) { cache in + guard let targetGroupId: String = cache.inviteFailures.first?.groupId else { return ([], "") } - DispatchQueue.main.async { - let toastController: ToastController = ToastController( - text: message, - background: .backgroundSecondary - ) - toastController.presentToastView(fromBottomOfView: mainWindow, inset: Values.largeSpacing) + let result: Set = cache.inviteFailures.filter { $0.groupId == targetGroupId } + cache.clearPendingFailures(for: targetGroupId) + return (result, targetGroupId) + } + } + .filter { failedInvites, _ in !failedInvites.isEmpty } + .setFailureType(to: Error.self) + .flatMapStorageReadPublisher(using: dependencies, value: { db, data -> (maybeName: String?, failedMemberIds: [String], profiles: [Profile]) in + let failedMemberIds: [String] = data.failedInvites.map { $0.memberId } + + return ( + try ClosedGroup + .filter(id: data.groupId) + .select(.name) + .asRequest(of: String.self) + .fetchOne(db), + failedMemberIds, + try Profile.filter(ids: failedMemberIds).fetchAll(db) + ) + }) + .map { maybeName, failedMemberIds, profiles -> (groupName: String, failedIds: [String], profileMap: [String: Profile]) in + let profileMap: [String: Profile] = profiles.reduce(into: [:]) { result, next in + result[next.id] = next + } + let sortedFailedMemberIds: [String] = failedMemberIds.sorted { lhs, rhs in + // Sort by name, followed by id if names aren't present + switch (profileMap[lhs]?.displayName(for: .group), profileMap[rhs]?.displayName(for: .group)) { + case (.some(let lhsName), .some(let rhsName)): return lhsName < rhsName + case (.some, .none): return true + case (.none, .some): return false + case (.none, .none): return lhs < rhs } } - ) - .map { _ in () } - .eraseToAnyPublisher() - - notifyFailurePublisher?.sinkUntilComplete() + + return ( + (maybeName ?? "groupUnknown".localized()), + sortedFailedMemberIds, + profileMap + ) + } + .catch { _ in Just(("", [], [:])).eraseToAnyPublisher() } + .filter { _, failedIds, _ in !failedIds.isEmpty } + .receive(on: DispatchQueue.main, using: dependencies) + .sink(receiveValue: { [dependencies] groupName, failedIds, profileMap in + guard let mainWindow: UIWindow = dependencies[singleton: .appContext].mainWindow else { return } + + let toastController: ToastController = ToastController( + text: GroupInviteMemberJob.failureMessage( + groupName: groupName, + memberIds: failedIds, + profileInfo: profileMap + ), + background: .backgroundSecondary + ) + toastController.presentToastView(fromBottomOfView: mainWindow, inset: Values.largeSpacing) + }) + .store(in: &disposables) } - - notifyFailureTrigger.send(()) - } -} - -// MARK: - GroupInviteMemberJob Cache - -public extension GroupInviteMemberJob { - class Cache: GroupInviteMemberJobCacheType { - public var failedMemberIds: Set = [] } } public extension Cache { static let groupInviteMemberJob: CacheConfig = Dependencies.create( identifier: "groupInviteMemberJob", - createInstance: { _ in GroupInviteMemberJob.Cache() }, + createInstance: { dependencies in GroupInviteMemberJob.Cache(using: dependencies) }, mutableInstance: { $0 }, immutableInstance: { $0 } ) @@ -290,11 +318,14 @@ public extension Cache { /// This is a read-only version of the Cache designed to avoid unintentionally mutating the instance in a non-thread-safe way public protocol GroupInviteMemberJobImmutableCacheType: ImmutableCacheType { - var failedMemberIds: Set { get } + var inviteFailures: Set { get } } public protocol GroupInviteMemberJobCacheType: GroupInviteMemberJobImmutableCacheType, MutableCacheType { - var failedMemberIds: Set { get set } + var inviteFailures: Set { get } + + func addInviteFailure(groupId: String, memberId: String) + func clearPendingFailures(for groupId: String) } // MARK: - GroupInviteMemberJob.Details diff --git a/SessionMessagingKit/LibSession/Config Handling/LibSession+UserGroups.swift b/SessionMessagingKit/LibSession/Config Handling/LibSession+UserGroups.swift index 833800e1f..b9b0d5ae0 100644 --- a/SessionMessagingKit/LibSession/Config Handling/LibSession+UserGroups.swift +++ b/SessionMessagingKit/LibSession/Config Handling/LibSession+UserGroups.swift @@ -31,7 +31,8 @@ internal extension LibSession { static let columnsRelatedToUserGroups: [ColumnExpression] = [ ClosedGroup.Columns.name, ClosedGroup.Columns.authData, - ClosedGroup.Columns.groupIdentityPrivateKey + ClosedGroup.Columns.groupIdentityPrivateKey, + ClosedGroup.Columns.invited ] } @@ -515,6 +516,17 @@ internal extension LibSessionCacheType { calledFromConfig: .userGroups, using: dependencies ) + + // If the group changed to no longer be in the invited state then we need to trigger the + // group approval process + if !group.invited && existingGroup.invited != group.invited { + try ClosedGroup.approveGroup( + db, + group: existingGroup, + calledFromConfig: .userGroups, + using: dependencies + ) + } } } diff --git a/SessionUtilitiesKit/General/Feature.swift b/SessionUtilitiesKit/General/Feature.swift index bb0acd713..7c3f64105 100644 --- a/SessionUtilitiesKit/General/Feature.swift +++ b/SessionUtilitiesKit/General/Feature.swift @@ -5,7 +5,8 @@ import Foundation public final class Features { - public static let legacyGroupDepricationDate: Date = Date.distantFuture // TODO: Set this date + public static let legacyGroupDepricationDate: Date = Date.distantFuture // TODO: [GROUPS REBUILD] Set this date + public static let legacyGroupDepricationUrl: String = "https://getsession.org/faq" // TODO: [GROUPS REBUILD] Set this url } public extension FeatureStorage {