From 8bd2468fb6fa5975d4e84a1d4ef09ee72a238901 Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Tue, 15 Oct 2024 18:06:35 +1100 Subject: [PATCH] Profile info update change, RadioButton bugfix, unit test changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit • Update group member profile info on invite acceptance • Fixed a bug with the default RadioButton enabled state • Added a few more unit tests --- Session/Utilities/MentionUtilities.swift | 1 + .../LibSession+GroupMembers.swift | 42 ++++++++++++ .../MessageReceiver+Groups.swift | 19 ++++++ .../MessageReceiver+VisibleMessages.swift | 1 + .../MessageSender+Groups.swift | 2 + .../MessageReceiverGroupsSpec.swift | 31 +++++++-- .../ThreadSettingsViewModelSpec.swift | 2 +- SessionTests/Database/DatabaseSpec.swift | 66 +++++++++++++++++++ SessionUIKit/Components/RadioButton.swift | 2 +- SessionUtilitiesKit/Database/Storage.swift | 2 +- 10 files changed, 161 insertions(+), 7 deletions(-) diff --git a/Session/Utilities/MentionUtilities.swift b/Session/Utilities/MentionUtilities.swift index 6f0121322..3bd2123ec 100644 --- a/Session/Utilities/MentionUtilities.swift +++ b/Session/Utilities/MentionUtilities.swift @@ -84,6 +84,7 @@ public enum MentionUtilities { guard let targetString: String = { guard !isCurrentUser else { return "you".localized() } + // FIXME: This does a database query and is happening when populating UI - should try to refactor it somehow (ideally resolve a set of mentioned profiles as part of the database query) guard let displayName: String = Profile.displayNameNoFallback(id: sessionId, threadVariant: threadVariant, using: dependencies) else { lastMatchEnd = (match.range.location + match.range.length) return nil diff --git a/SessionMessagingKit/LibSession/Config Handling/LibSession+GroupMembers.swift b/SessionMessagingKit/LibSession/Config Handling/LibSession+GroupMembers.swift index 719563da1..8879cc4c2 100644 --- a/SessionMessagingKit/LibSession/Config Handling/LibSession+GroupMembers.swift +++ b/SessionMessagingKit/LibSession/Config Handling/LibSession+GroupMembers.swift @@ -229,11 +229,13 @@ internal extension LibSession { memberId: String, role: GroupMember.Role, status: GroupMember.RoleStatus, + profile: Profile?, using dependencies: Dependencies ) throws { try dependencies.mutate(cache: .libSession) { cache in try cache.performAndPushChange(db, for: .groupMembers, sessionId: groupSessionId) { config in try LibSession.updateMemberStatus(memberId: memberId, role: role, status: status, in: config) + try LibSession.updateMemberProfile(memberId: memberId, profile: profile, in: config) } } } @@ -266,6 +268,46 @@ internal extension LibSession { try LibSessionError.throwIfNeeded(conf) } + static func updateMemberProfile( + _ db: Database, + groupSessionId: SessionId, + memberId: String, + profile: Profile?, + using dependencies: Dependencies + ) throws { + try dependencies.mutate(cache: .libSession) { cache in + try cache.performAndPushChange(db, for: .groupMembers, sessionId: groupSessionId) { config in + try LibSession.updateMemberProfile(memberId: memberId, profile: profile, in: config) + } + } + } + + static func updateMemberProfile( + memberId: String, + profile: Profile?, + in config: Config? + ) throws { + guard let profile: Profile = profile else { return } + guard case .object(let conf) = config else { throw LibSessionError.invalidConfigObject } + + // Only update members if they already exist in the group + var cMemberId: [CChar] = try memberId.cString(using: .utf8) ?? { throw LibSessionError.invalidCConversion }() + var groupMember: config_group_member = config_group_member() + + // If the member doesn't exist then do nothing + guard groups_members_get(conf, &groupMember, &cMemberId) else { return } + + groupMember.set(\.name, to: profile.name) + + if profile.profilePictureUrl != nil && profile.profileEncryptionKey != nil { + groupMember.set(\.profile_pic.url, to: profile.profilePictureUrl) + groupMember.set(\.profile_pic.key, to: profile.profileEncryptionKey) + } + + groups_members_set(conf, &groupMember) + try? LibSessionError.throwIfNeeded(conf) + } + static func flagMembersForRemoval( _ db: Database, groupSessionId: SessionId, diff --git a/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+Groups.swift b/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+Groups.swift index 9eb19b8a8..35ac29676 100644 --- a/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+Groups.swift +++ b/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+Groups.swift @@ -567,6 +567,16 @@ extension MessageReceiver { db, senderSessionId: sender, groupSessionIdHexString: groupSessionId.hexString, + profile: message.profile.map { profile in + profile.displayName.map { + Profile( + id: sender, + name: $0, + profilePictureUrl: profile.profilePictureUrl, + profileEncryptionKey: profile.profileKey + ) + } + }, using: dependencies ) } @@ -976,6 +986,7 @@ extension MessageReceiver { _ db: Database, senderSessionId: String, groupSessionIdHexString: String?, + profile: Profile?, using dependencies: Dependencies ) throws { // Only group admins can update the member approval state @@ -1005,6 +1016,7 @@ extension MessageReceiver { memberId: senderSessionId, role: .standard, status: .accepted, + profile: profile, using: dependencies ) @@ -1020,6 +1032,13 @@ extension MessageReceiver { calledFromConfig: nil, using: dependencies ) + try LibSession.updateMemberProfile( + db, + groupSessionId: groupSessionId, + memberId: senderSessionId, + profile: profile, + using: dependencies + ) default: break // Invalid cases } diff --git a/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+VisibleMessages.swift b/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+VisibleMessages.swift index 129506b2b..ca3959449 100644 --- a/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+VisibleMessages.swift +++ b/SessionMessagingKit/Sending & Receiving/Message Handling/MessageReceiver+VisibleMessages.swift @@ -378,6 +378,7 @@ extension MessageReceiver { db, senderSessionId: sender, groupSessionIdHexString: thread.id, + profile: nil, // Don't update the profile in this case using: dependencies ) diff --git a/SessionMessagingKit/Sending & Receiving/Message Handling/MessageSender+Groups.swift b/SessionMessagingKit/Sending & Receiving/Message Handling/MessageSender+Groups.swift index 74c0909e9..d853ad016 100644 --- a/SessionMessagingKit/Sending & Receiving/Message Handling/MessageSender+Groups.swift +++ b/SessionMessagingKit/Sending & Receiving/Message Handling/MessageSender+Groups.swift @@ -620,6 +620,7 @@ extension MessageSender { memberId: memberId, role: .standard, status: .notSentYet, + profile: nil, using: dependencies ) @@ -786,6 +787,7 @@ extension MessageSender { memberId: memberId, role: .admin, status: .notSentYet, + profile: nil, using: dependencies ) diff --git a/SessionMessagingKitTests/Sending & Receiving/MessageReceiverGroupsSpec.swift b/SessionMessagingKitTests/Sending & Receiving/MessageReceiverGroupsSpec.swift index 7020db64a..7914770e1 100644 --- a/SessionMessagingKitTests/Sending & Receiving/MessageReceiverGroupsSpec.swift +++ b/SessionMessagingKitTests/Sending & Receiving/MessageReceiverGroupsSpec.swift @@ -1031,8 +1031,9 @@ class MessageReceiverGroupsSpec: QuickSpec { // MARK: ---- updates the GROUP_KEYS state correctly it("updates the GROUP_KEYS state correctly") { - // TODO: Should return a value???? - mockCrypto.when { $0.generate(.ed25519KeyPair(seed: .any)) }.thenReturn(nil) + mockCrypto + .when { $0.generate(.ed25519KeyPair(seed: .any)) } + .thenReturn(KeyPair(publicKey: [1, 2, 3], secretKey: [4, 5, 6])) mockStorage.write { db in try MessageReceiver.handleGroupUpdateMessage( @@ -1986,8 +1987,8 @@ class MessageReceiverGroupsSpec: QuickSpec { } } - // MARK: ---- updates the profile information if provided - it("updates the profile information if provided") { + // MARK: ---- updates the profile information in the database if provided + it("updates the profile information in the database if provided") { mockStorage.write { db in try MessageReceiver.handleGroupUpdateMessage( db, @@ -2129,6 +2130,28 @@ class MessageReceiverGroupsSpec: QuickSpec { expect(groups_members_get(groupMembersConf, &groupMember, &cMemberId)).to(beTrue()) expect(groupMember.invited).to(equal(0)) } + + // MARK: ---- updates the config member entry with profile information if provided + it("updates the config member entry with profile information if provided") { + mockStorage.write { db in + _ = try GroupMember.deleteAll(db) + } + + mockStorage.write { db in + try MessageReceiver.handleGroupUpdateMessage( + db, + threadId: groupId.hexString, + threadVariant: .group, + message: inviteResponseMessage, + using: dependencies + ) + } + + var cMemberId: [CChar] = "051111111111111111111111111111111111111111111111111111111111111112".cString(using: .utf8)! + var groupMember: config_group_member = config_group_member() + expect(groups_members_get(groupMembersConf, &groupMember, &cMemberId)).to(beTrue()) + expect(groupMember.get(\.name)).to(equal("TestOtherMember")) + } } } diff --git a/SessionTests/Conversations/Settings/ThreadSettingsViewModelSpec.swift b/SessionTests/Conversations/Settings/ThreadSettingsViewModelSpec.swift index 509a8ea98..d186be72e 100644 --- a/SessionTests/Conversations/Settings/ThreadSettingsViewModelSpec.swift +++ b/SessionTests/Conversations/Settings/ThreadSettingsViewModelSpec.swift @@ -347,7 +347,7 @@ class ThreadSettingsViewModelSpec: QuickSpec { let modal2: ConfirmationModal? = (screenTransitions.last?.destination as? ConfirmationModal) expect(modal2?.info.title).to(equal("theError".localized())) - expect(modal2?.info.body).to(equal(.text("displayNameErrorDescriptionShorter".localized()))) + expect(modal2?.info.body).to(equal(.text("nicknameErrorShorter".localized()))) expect(modal2?.info.confirmTitle).to(beNil()) expect(modal2?.info.cancelTitle).to(equal("okay".localized())) } diff --git a/SessionTests/Database/DatabaseSpec.swift b/SessionTests/Database/DatabaseSpec.swift index 967d45565..f641a52b1 100644 --- a/SessionTests/Database/DatabaseSpec.swift +++ b/SessionTests/Database/DatabaseSpec.swift @@ -63,6 +63,46 @@ class DatabaseSpec: QuickSpec { // MARK: - a Database describe("a Database") { + // MARK: -- triggers the migration requirements in the expected order + it("triggers the migration requirements in the expected order") { + var migrationRequirementsSet: [MigrationRequirement] = [] + mockStorage.perform( + migrationTargets: [ + TestMigratableTarget.self + ], + async: false, + onProgressUpdate: nil, + onMigrationRequirement: { [dependencies = dependencies!] db, requirement in + migrationRequirementsSet.append(requirement) + + MigrationTest.handleRequirements(db, requirement: requirement, using: dependencies) + }, + onComplete: { _, _ in } + ) + + expect(migrationRequirementsSet).to(equal([.sessionIdCached, .libSessionStateLoaded])) + } + + // MARK: -- triggers all prior migration requirements if only a latter one is called + it("triggers all prior migration requirements if only a latter one is called") { + var migrationRequirementsSet: [MigrationRequirement] = [] + mockStorage.perform( + migrationTargets: [ + SNMessagingKit.self + ], + async: false, + onProgressUpdate: nil, + onMigrationRequirement: { [dependencies = dependencies!] db, requirement in + migrationRequirementsSet.append(requirement) + + MigrationTest.handleRequirements(db, requirement: requirement, using: dependencies) + }, + onComplete: { _, _ in } + ) + + expect(migrationRequirementsSet).to(equal([.sessionIdCached, .libSessionStateLoaded])) + } + // MARK: -- can be created from an empty state it("can be created from an empty state") { mockStorage.perform( @@ -404,3 +444,29 @@ private class MigrationTest { } } } + +enum TestMigratableTarget: MigratableTarget { // Just to make the external API nice + public static func migrations() -> TargetMigrations { + return TargetMigrations( + identifier: .session, + migrations: [ + [ + TestRequiresLibSessionStateMigration.self + ] + ] + ) + } +} + +enum TestRequiresLibSessionStateMigration: Migration { + static let target: TargetMigrations.Identifier = .session + static let identifier: String = "test" // stringlint:disable + static let needsConfigSync: Bool = false + static let minExpectedRunDuration: TimeInterval = 0.1 + static var requirements: [MigrationRequirement] = [.libSessionStateLoaded] + static let fetchedTables: [(TableRecord & FetchableRecord).Type] = [] + static let createdOrAlteredTables: [(TableRecord & FetchableRecord).Type] = [] + static let droppedTables: [(TableRecord & FetchableRecord).Type] = [] + + static func migrate(_ db: Database, using dependencies: Dependencies) throws {} +} diff --git a/SessionUIKit/Components/RadioButton.swift b/SessionUIKit/Components/RadioButton.swift index 5cabf926e..597de9aac 100644 --- a/SessionUIKit/Components/RadioButton.swift +++ b/SessionUIKit/Components/RadioButton.swift @@ -36,7 +36,7 @@ public class RadioButton: UIView { set { titleLabel.text = newValue } } - public private(set) var isEnabled: Bool = false + public private(set) var isEnabled: Bool = true public private(set) var isSelected: Bool = false private let onSelected: ((RadioButton) -> ())? diff --git a/SessionUtilitiesKit/Database/Storage.swift b/SessionUtilitiesKit/Database/Storage.swift index 2e971662f..647ddaeb3 100644 --- a/SessionUtilitiesKit/Database/Storage.swift +++ b/SessionUtilitiesKit/Database/Storage.swift @@ -389,7 +389,7 @@ open class Storage { // Remove any processed requirements from the list (don't want to process them multiple times) unprocessedMigrationRequirements.mutate { - $0 = Array($0.asSet().subtracting(migration.requirements.asSet())) + $0 = Array($0.asSet().subtracting(unprocessedRequirements.asSet())) } }