Fixed a number of bugs found when checking the acceptance criteria

• 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)
pull/894/head
Morgan Pretty 7 months ago
parent 208d7aa22c
commit ad2c9ff5a6

@ -55,6 +55,30 @@ final class NewClosedGroupVC: BaseVC, UITableViewDataSource, UITableViewDelegate
private static let textFieldHeight: CGFloat = 50 private static let textFieldHeight: CGFloat = 50
private static let searchBarHeight: CGFloat = (36 + (Values.mediumSpacing * 2)) 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 = { private lazy var nameTextField: TextField = {
let result = TextField( let result = TextField(
placeholder: "groupNameEnter".localized(), placeholder: "groupNameEnter".localized(),
@ -192,11 +216,14 @@ final class NewClosedGroupVC: BaseVC, UITableViewDataSource, UITableViewDelegate
return return
} }
view.addSubview(tableView) view.addSubview(contentStackView)
tableView.pin(.top, to: .top, of: view) contentStackView.pin(.top, to: .top, of: view)
tableView.pin(.leading, to: .leading, of: view) contentStackView.pin(.leading, to: .leading, of: view)
tableView.pin(.trailing, to: .trailing, of: view) contentStackView.pin(.trailing, to: .trailing, of: view)
tableView.pin(.bottom, to: .bottom, of: view) contentStackView.pin(.bottom, to: .bottom, of: view)
contentStackView.addArrangedSubview(minVersionBanner)
contentStackView.addArrangedSubview(tableView)
view.addSubview(fadeView) view.addSubview(fadeView)
fadeView.pin(.leading, to: .leading, of: view) fadeView.pin(.leading, to: .leading, of: view)

@ -1270,6 +1270,7 @@ extension ConversationVC:
confirmStyle: .danger, confirmStyle: .danger,
cancelTitle: "urlCopy".localized(), cancelTitle: "urlCopy".localized(),
cancelStyle: .alert_text, cancelStyle: .alert_text,
hasCloseButton: true,
onConfirm: { [weak self] _ in onConfirm: { [weak self] _ in
UIApplication.shared.open(url, options: [:], completionHandler: nil) UIApplication.shared.open(url, options: [:], completionHandler: nil)
self?.showInputAccessoryView() self?.showInputAccessoryView()
@ -2568,6 +2569,13 @@ extension ConversationVC {
return viewModel.dependencies[singleton: .storage] return viewModel.dependencies[singleton: .storage]
.writePublisher { [dependencies = viewModel.dependencies] db -> AnyPublisher<Void, Never> in .writePublisher { [dependencies = viewModel.dependencies] db -> AnyPublisher<Void, Never> 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 /// 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` /// 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) /// state of the group will continue to be `true` while we wait on the initial poll to get back)

@ -235,17 +235,21 @@ final class ConversationVC: BaseVC, LibSessionRespondingViewController, Conversa
let result: InfoBanner = InfoBanner( let result: InfoBanner = InfoBanner(
info: InfoBanner.Info( info: InfoBanner.Info(
font: .systemFont(ofSize: Values.miniFontSize), font: .systemFont(ofSize: Values.miniFontSize),
message: "groupInviteVersion".localized(), message: "groupLegacyBanner"
.put(key: "date", value: Features.legacyGroupDepricationDate.formattedForBanner)
.localized(),
icon: .link, icon: .link,
tintColor: .messageBubble_outgoingText, tintColor: .messageBubble_outgoingText,
backgroundColor: .primary, backgroundColor: .primary,
accessibility: Accessibility(label: "Legacy group banner"), accessibility: Accessibility(label: "Legacy group banner"),
labelAccessibility: Accessibility(label: "Legacy group banner text"),
height: nil, 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 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 { if initialLoad || viewModel.threadData.threadIsBlocked != updatedThreadData.threadIsBlocked {
addOrRemoveBlockedBanner(threadIsBlocked: (updatedThreadData.threadIsBlocked == true)) addOrRemoveBlockedBanner(threadIsBlocked: (updatedThreadData.threadIsBlocked == true))
} }

@ -63,7 +63,7 @@ class MessageRequestFooterView: UIView {
result.translatesAutoresizingMaskIntoConstraints = false result.translatesAutoresizingMaskIntoConstraints = false
result.clipsToBounds = true result.clipsToBounds = true
result.titleLabel?.font = UIFont.boldSystemFont(ofSize: 16) 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.setThemeTitleColor(.danger, for: .normal)
result.addTarget(self, action: #selector(block), for: .touchUpInside) result.addTarget(self, action: #selector(block), for: .touchUpInside)
@ -86,7 +86,7 @@ class MessageRequestFooterView: UIView {
result.accessibilityLabel = "Delete message request" result.accessibilityLabel = "Delete message request"
result.isAccessibilityElement = true result.isAccessibilityElement = true
result.translatesAutoresizingMaskIntoConstraints = false result.translatesAutoresizingMaskIntoConstraints = false
result.setTitle("decline".localized(), for: .normal) result.setTitle("delete".localized(), for: .normal)
result.addTarget(self, action: #selector(decline), for: .touchUpInside) result.addTarget(self, action: #selector(decline), for: .touchUpInside)
return result return result

@ -151,7 +151,7 @@ struct EnterAccountIdScreen: View {
) )
) { ) {
ZStack { ZStack {
Text("\("messageNewDescriptionMobile".localized())\(Image(systemName: "questionmark.circle"))") Text("messageNewDescriptionMobile".localized().appending("\(Image(systemName: "questionmark.circle"))"))
.font(.system(size: Values.verySmallFontSize)) .font(.system(size: Values.verySmallFontSize))
.foregroundColor(themeColor: .textSecondary) .foregroundColor(themeColor: .textSecondary)
.multilineTextAlignment(.center) .multilineTextAlignment(.center)

@ -446,52 +446,15 @@ public struct Interaction: Codable, Identifiable, Equatable, FetchableRecord, Mu
switch variant { switch variant {
case .standardOutgoing: case .standardOutgoing:
// New outgoing messages should immediately determine their recipient list /// The `RecipientState` is used to manage the sending status of a message regardless of the type of conversation it's
// from current thread state /// sent to, in a `contact` conversation the "recipient" is a contact and, as such, can have a `Read` state if enabled but for
switch threadVariant { /// group and community conversations the "recipient" of the message is the conversation itself so there can be no `Read`
case .contact: /// state, and we only need a single record
try RecipientState( try RecipientState(
interactionId: success.rowID, interactionId: success.rowID,
recipientId: threadId, // Will be the contact id recipientId: threadId,
state: .sending state: .sending
).insert(db) ).insert(db)
case .legacyGroup, .group:
let closedGroupMemberIds: Set<String> = (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)
}
default: break default: break
} }

@ -20,10 +20,6 @@ public enum GroupInviteMemberJob: JobExecutor {
public static var requiresThreadId: Bool = true public static var requiresThreadId: Bool = true
public static var requiresInteractionId: Bool = false public static var requiresInteractionId: Bool = false
private static let notificationDebounceDuration: DispatchQueue.SchedulerTimeType.Stride = .milliseconds(1500)
private static var notifyFailurePublisher: AnyPublisher<Void, Never>?
private static let notifyFailureTrigger: PassthroughSubject<(), Never> = PassthroughSubject()
public static func run( public static func run(
_ job: Job, _ job: Job,
queue: DispatchQueue, queue: DispatchQueue,
@ -127,11 +123,9 @@ public enum GroupInviteMemberJob: JobExecutor {
} }
// Notify about the failure // Notify about the failure
GroupInviteMemberJob.notifyOfFailure( dependencies.mutate(cache: .groupInviteMemberJob) { cache in
groupId: threadId, cache.addInviteFailure(groupId: threadId, memberId: details.memberSessionIdHexString)
memberId: details.memberSessionIdHexString, }
using: dependencies
)
// Register the failure // Register the failure
switch error { switch error {
@ -199,88 +193,122 @@ public enum GroupInviteMemberJob: JobExecutor {
.localizedFormatted(baseFont: ToastController.font) .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) { class Cache: GroupInviteMemberJobCacheType {
dependencies.mutate(cache: .groupInviteMemberJob) { cache in private static let notificationDebounceDuration: DispatchQueue.SchedulerTimeType.Stride = .milliseconds(3000)
cache.failedMemberIds.insert(memberId)
private let dependencies: Dependencies
private let inviteFailedNotificationTrigger: PassthroughSubject<(), Never> = PassthroughSubject()
private var disposables: Set<AnyCancellable> = Set()
public private(set) var inviteFailures: Set<InviteFailure> = []
// 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 // MARK: - Functions
/// and show a single toast
if notifyFailurePublisher == nil { public func addInviteFailure(groupId: String, memberId: String) {
notifyFailurePublisher = notifyFailureTrigger print("[RAWR] Add failure for: \(memberId)")
.debounce(for: notificationDebounceDuration, scheduler: DispatchQueue.global(qos: .userInitiated)) inviteFailures.insert(InviteFailure(groupId: groupId, memberId: memberId))
.handleEvents( inviteFailedNotificationTrigger.send(())
receiveOutput: { [dependencies] _ in }
let failedIds: [String] = dependencies.mutate(cache: .groupInviteMemberJob) { cache in
let result: Set<String> = cache.failedMemberIds public func clearPendingFailures(for groupId: String) {
cache.failedMemberIds.removeAll() inviteFailures = inviteFailures.filter { $0.groupId != groupId }
return Array(result) }
}
// MARK: - Internal Functions
// Don't do anything if there are no 'failedIds' values or we can't get a window
guard private func setupInviteFailureListener() {
!failedIds.isEmpty, inviteFailedNotificationTrigger
let mainWindow: UIWindow = dependencies[singleton: .appContext].mainWindow .subscribe(on: DispatchQueue.global(qos: .userInitiated), using: dependencies)
else { return } .debounce(
for: Cache.notificationDebounceDuration,
typealias FetchedData = (groupName: String, profileInfo: [String: Profile]) scheduler: DispatchQueue.global(qos: .userInitiated)
)
let data: FetchedData = dependencies[singleton: .storage] .map { [dependencies] _ -> (failedInvites: Set<InviteFailure>, groupId: String) in
.read { db in dependencies.mutate(cache: .groupInviteMemberJob) { cache in
( guard let targetGroupId: String = cache.inviteFailures.first?.groupId else { return ([], "") }
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
)
DispatchQueue.main.async { let result: Set<InviteFailure> = cache.inviteFailures.filter { $0.groupId == targetGroupId }
let toastController: ToastController = ToastController( cache.clearPendingFailures(for: targetGroupId)
text: message, return (result, targetGroupId)
background: .backgroundSecondary }
) }
toastController.presentToastView(fromBottomOfView: mainWindow, inset: Values.largeSpacing) .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 () } return (
.eraseToAnyPublisher() (maybeName ?? "groupUnknown".localized()),
sortedFailedMemberIds,
notifyFailurePublisher?.sinkUntilComplete() 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<String> = []
} }
} }
public extension Cache { public extension Cache {
static let groupInviteMemberJob: CacheConfig<GroupInviteMemberJobCacheType, GroupInviteMemberJobImmutableCacheType> = Dependencies.create( static let groupInviteMemberJob: CacheConfig<GroupInviteMemberJobCacheType, GroupInviteMemberJobImmutableCacheType> = Dependencies.create(
identifier: "groupInviteMemberJob", identifier: "groupInviteMemberJob",
createInstance: { _ in GroupInviteMemberJob.Cache() }, createInstance: { dependencies in GroupInviteMemberJob.Cache(using: dependencies) },
mutableInstance: { $0 }, mutableInstance: { $0 },
immutableInstance: { $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 /// 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 { public protocol GroupInviteMemberJobImmutableCacheType: ImmutableCacheType {
var failedMemberIds: Set<String> { get } var inviteFailures: Set<GroupInviteMemberJob.InviteFailure> { get }
} }
public protocol GroupInviteMemberJobCacheType: GroupInviteMemberJobImmutableCacheType, MutableCacheType { public protocol GroupInviteMemberJobCacheType: GroupInviteMemberJobImmutableCacheType, MutableCacheType {
var failedMemberIds: Set<String> { get set } var inviteFailures: Set<GroupInviteMemberJob.InviteFailure> { get }
func addInviteFailure(groupId: String, memberId: String)
func clearPendingFailures(for groupId: String)
} }
// MARK: - GroupInviteMemberJob.Details // MARK: - GroupInviteMemberJob.Details

@ -31,7 +31,8 @@ internal extension LibSession {
static let columnsRelatedToUserGroups: [ColumnExpression] = [ static let columnsRelatedToUserGroups: [ColumnExpression] = [
ClosedGroup.Columns.name, ClosedGroup.Columns.name,
ClosedGroup.Columns.authData, ClosedGroup.Columns.authData,
ClosedGroup.Columns.groupIdentityPrivateKey ClosedGroup.Columns.groupIdentityPrivateKey,
ClosedGroup.Columns.invited
] ]
} }
@ -515,6 +516,17 @@ internal extension LibSessionCacheType {
calledFromConfig: .userGroups, calledFromConfig: .userGroups,
using: dependencies 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
)
}
} }
} }

@ -5,7 +5,8 @@
import Foundation import Foundation
public final class Features { 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 { public extension FeatureStorage {

Loading…
Cancel
Save