Handle API changes from Rust. (#506)

There are some bad assumptions about profile changes in here. * Remove assumption FIXME's Profile changes that come from other members will be state event of None. * Bump SDK version.
element-hq · Jan 31, 2023 · 4f8cc08 · 4f8cc08
1 parent 17568a6
commit 4f8cc08
Show file tree

Hide file tree

Showing 13 changed files with 87 additions and 64 deletions.
diff --git a/ElementX.xcodeproj/project.pbxproj b/ElementX.xcodeproj/project.pbxproj
@@ -4008,7 +4008,7 @@
 			repositoryURL = "https://github.com/matrix-org/matrix-rust-components-swift";
 			requirement = {
 				kind = exactVersion;
-				version = "1.0.33-alpha";
+				version = "1.0.35-alpha";
 			};
 		};
 		96495DD8554E2F39D3954354 /* XCRemoteSwiftPackageReference "posthog-ios" */ = {

diff --git a/ElementX.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/ElementX.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
@@ -86,8 +86,8 @@
       "kind" : "remoteSourceControl",
       "location" : "https://github.com/matrix-org/matrix-rust-components-swift",
       "state" : {
-        "revision" : "2d702d0d52805e4f81924507b39ad81c8a74a63f",
-        "version" : "1.0.33-alpha"
+        "revision" : "f6b5ccd904da60ccf39f41161c7db19e87b09870",
+        "version" : "1.0.35-alpha"
       }
     },
     {

diff --git a/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemBubbledStylerView.swift b/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemBubbledStylerView.swift
@@ -46,8 +46,8 @@ struct TimelineItemBubbledStylerView<Content: View>: View {
                 .padding(timelineItem.isOutgoing ? .leading : .trailing, 40) // Extra padding to differentiate alignment.
             }
 
-            if timelineItem.isOutgoing {
-                TimelineDeliveryStatusView(deliveryStatus: timelineItem.properties.deliveryStatus)
+            if let deliveryStatus = timelineItem.properties.deliveryStatus {
+                TimelineDeliveryStatusView(deliveryStatus: deliveryStatus)
             }
         }
     }

diff --git a/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemPlainStylerView.swift b/ElementX/Sources/Screens/RoomScreen/View/Style/TimelineItemPlainStylerView.swift
@@ -33,8 +33,8 @@ struct TimelineItemPlainStylerView<Content: View>: View {
 
                     Spacer()
 
-                    if timelineItem.isOutgoing {
-                        TimelineDeliveryStatusView(deliveryStatus: timelineItem.properties.deliveryStatus)
+                    if let deliveryStatus = timelineItem.properties.deliveryStatus {
+                        TimelineDeliveryStatusView(deliveryStatus: deliveryStatus)
                     }
                 }
                 supplementaryViews

diff --git a/ElementX/Sources/Screens/RoomScreen/View/Supplementary/TimelineDeliveryStatusView.swift b/ElementX/Sources/Screens/RoomScreen/View/Supplementary/TimelineDeliveryStatusView.swift
@@ -22,23 +22,23 @@ struct TimelineDeliveryStatusView: View {
 
     private var systemImageName: String {
         switch deliveryStatus {
-        case .sending, .unknown:
+        case .sending:
             return "circle"
         case .sent:
             return "checkmark.circle"
+        case .sendingFailed:
+            return "exclamationmark.circle"
         }
     }
 
     init(deliveryStatus: TimelineItemDeliveryStatus) {
         self.deliveryStatus = deliveryStatus
 
         switch deliveryStatus {
-        case .sending:
+        case .sending, .sendingFailed:
             showDeliveryStatus = true
         case let .sent(elapsedTime: elapsedTime):
             showDeliveryStatus = elapsedTime < 3
-        case .unknown:
-            showDeliveryStatus = false
         }
     }
 

diff --git a/ElementX/Sources/Services/Room/RoomSummary/RoomEventStringBuilder.swift b/ElementX/Sources/Services/Room/RoomSummary/RoomEventStringBuilder.swift
@@ -61,10 +61,19 @@ struct RoomEventStringBuilder {
             return stateEventStringBuilder
                 .buildString(for: state, stateKey: stateKey, sender: sender, isOutgoing: isOutgoing)
                 .map(AttributedString.init)
-        case .roomMembership(userId: let userID, change: let change):
+        case .roomMembership(let userID, let change):
             return stateEventStringBuilder
                 .buildString(for: change, member: userID, sender: sender, isOutgoing: isOutgoing)
                 .map(AttributedString.init)
+        case .profileChange(let displayName, let prevDisplayName, let avatarUrl, let prevAvatarUrl):
+            return stateEventStringBuilder
+                .buildProfileChangeString(displayName: displayName,
+                                          previousDisplayName: prevDisplayName,
+                                          avatarURLString: avatarUrl,
+                                          previousAvatarURLString: prevAvatarUrl,
+                                          member: sender.id,
+                                          memberIsYou: isOutgoing)
+                .map(AttributedString.init)
         }
     }
 

diff --git a/ElementX/Sources/Services/Timeline/TimelineItemContent/MessageTimelineItem.swift b/ElementX/Sources/Services/Timeline/TimelineItemContent/MessageTimelineItem.swift
@@ -30,12 +30,7 @@ struct MessageTimelineItem<Content: MessageContentProtocol> {
     let content: Content
 
     var id: String {
-        switch item.key() {
-        case .transactionId(let txnID):
-            return txnID
-        case .eventId(let eventID):
-            return eventID
-        }
+        item.uniqueIdentifier()
     }
 
     var body: String {

diff --git a/ElementX/Sources/Services/Timeline/TimelineItemContent/RoomTimelineItemProperties.swift b/ElementX/Sources/Services/Timeline/TimelineItemContent/RoomTimelineItemProperties.swift
@@ -23,5 +23,5 @@ struct RoomTimelineItemProperties: Hashable {
     /// The aggregated reactions that have been sent for this item.
     var reactions: [AggregatedReaction] = []
     /// The delivery status for this item.
-    var deliveryStatus: TimelineItemDeliveryStatus = .unknown
+    var deliveryStatus: TimelineItemDeliveryStatus?
 }
diff --git a/ElementX/Sources/Services/Timeline/TimelineItemProxy.swift b/ElementX/Sources/Services/Timeline/TimelineItemProxy.swift
@@ -50,9 +50,9 @@ enum TimelineItemProxy {
 
 /// The delivery status for the item.
 enum TimelineItemDeliveryStatus: Hashable {
-    case unknown
     case sending
     case sent(elapsedTime: TimeInterval)
+    case sendingFailed
 }
 
 /// A light wrapper around event timeline items returned from Rust.
@@ -64,19 +64,18 @@ struct EventTimelineItemProxy: CustomDebugStringConvertible {
     }
 
     var id: String {
-        switch item.key() {
-        case .transactionId(let txnID):
-            return txnID
-        case .eventId(let eventID):
-            return eventID
-        }
+        item.uniqueIdentifier()
     }
 
-    var deliveryStatus: TimelineItemDeliveryStatus {
-        switch item.key() {
-        case .transactionId:
+    var deliveryStatus: TimelineItemDeliveryStatus? {
+        guard let localSendState = item.localSendState() else { return nil }
+
+        switch localSendState {
+        case .notSendYet:
             return .sending
-        case .eventId:
+        case .sendingFailed:
+            return .sendingFailed
+        case .sent:
             return .sent(elapsedTime: Date().timeIntervalSince1970 - timestamp.timeIntervalSince1970)
         }
     }
@@ -113,7 +112,7 @@ struct EventTimelineItemProxy: CustomDebugStringConvertible {
     }
 
     var reactions: [Reaction] {
-        item.reactions()
+        item.reactions() ?? []
     }
 
     var timestamp: Date {

diff --git a/ElementX/Sources/Services/Timeline/TimelineItems/RoomStateEventStringBuilder.swift b/ElementX/Sources/Services/Timeline/TimelineItems/RoomStateEventStringBuilder.swift
@@ -20,8 +20,13 @@ import UIKit
 struct RoomStateEventStringBuilder {
     let userID: String
 
-    // swiftlint:disable:next cyclomatic_complexity function_body_length
-    func buildString(for change: MembershipChange, member: String, sender: TimelineItemSender, isOutgoing: Bool) -> String? {
+    // swiftlint:disable:next cyclomatic_complexity
+    func buildString(for change: MembershipChange?, member: String, sender: TimelineItemSender, isOutgoing: Bool) -> String? {
+        guard let change else {
+            MXLog.verbose("Filtering timeline item for membership change that is nil")
+            return nil
+        }
+
         let senderName = sender.displayName ?? sender.id
         let senderIsYou = isOutgoing
         let memberIsYou = member == userID
@@ -65,22 +70,16 @@ struct RoomStateEventStringBuilder {
             } else {
                 return ElementL10n.noticeRoomKnockDenied(senderName, member)
             }
-        case .profileChanged(let displayName, let previousDisplayName, let avatarURLString, let previousAvatarURLString):
-            return profileChangedString(displayName: displayName, previousDisplayName: previousDisplayName,
-                                        avatarURLString: avatarURLString, previousAvatarURLString: previousAvatarURLString,
-                                        member: member, memberIsYou: memberIsYou,
-                                        sender: sender, senderIsYou: senderIsYou)
-        case .none, .error, .notImplemented, .unknown: // Not useful information for the user.
+        case .none, .error, .notImplemented: // Not useful information for the user.
             MXLog.verbose("Filtering timeline item for membership change: \(change)")
             return nil
         }
     }
 
     // swiftlint:disable:next cyclomatic_complexity function_parameter_count
-    private func profileChangedString(displayName: String?, previousDisplayName: String?,
-                                      avatarURLString: String?, previousAvatarURLString: String?,
-                                      member: String, memberIsYou: Bool,
-                                      sender: TimelineItemSender, senderIsYou: Bool) -> String {
+    func buildProfileChangeString(displayName: String?, previousDisplayName: String?,
+                                  avatarURLString: String?, previousAvatarURLString: String?,
+                                  member: String, memberIsYou: Bool) -> String? {
         let displayNameChanged = displayName != previousDisplayName
         let avatarChanged = avatarURLString != previousAvatarURLString
 
@@ -93,8 +92,8 @@ struct RoomStateEventStringBuilder {
             } else if let previousDisplayName {
                 return ElementL10n.noticeDisplayNameRemoved(member, previousDisplayName)
             } else {
-                MXLog.error("The display name changed from nil to nil, shouldn't be possible.")
-                return ElementL10n.noticeMemberNoChanges(member)
+                MXLog.error("The display name changed from nil to nil, filtering the item.")
+                return nil
             }
         case (false, true, false):
             return ElementL10n.noticeAvatarUrlChanged(displayName ?? member)
@@ -106,20 +105,20 @@ struct RoomStateEventStringBuilder {
             } else if let previousDisplayName {
                 return ElementL10n.noticeDisplayNameRemovedByYou(previousDisplayName)
             } else {
-                MXLog.error("The display name changed from nil to nil, shouldn't be possible.")
-                return ElementL10n.noticeMemberNoChangesByYou
+                MXLog.error("The display name changed from nil to nil, filtering the item.")
+                return nil
             }
         case (false, true, true):
             return ElementL10n.noticeAvatarUrlChangedByYou
         case (true, true, _):
             // When both have changed, get the string for the display name and tack on that the avatar changed too.
-            return profileChangedString(displayName: displayName, previousDisplayName: previousDisplayName,
-                                        avatarURLString: nil, previousAvatarURLString: nil,
-                                        member: member, memberIsYou: memberIsYou,
-                                        sender: sender, senderIsYou: senderIsYou) + "\n" + ElementL10n.noticeAvatarChangedToo
+            guard let string = buildProfileChangeString(displayName: displayName, previousDisplayName: previousDisplayName,
+                                                        avatarURLString: nil, previousAvatarURLString: nil,
+                                                        member: member, memberIsYou: memberIsYou) else { return nil }
+            return string + "\n" + ElementL10n.noticeAvatarChangedToo
         case (false, false, _):
-            MXLog.error("Nothing changed, shouldn't be possible.")
-            return ElementL10n.noticeMemberNoChangesByYou
+            MXLog.error("Nothing changed, shouldn't be possible. Filtering the item.")
+            return nil
         }
     }
 

diff --git a/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemFactory.swift b/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemFactory.swift
@@ -85,6 +85,8 @@ struct RoomTimelineItemFactory: RoomTimelineItemFactoryProtocol {
             return buildStateTimelineItemFor(eventItemProxy: eventItemProxy, state: content, stateKey: stateKey, isOutgoing: isOutgoing)
         case .roomMembership(userId: let userID, change: let change):
             return buildStateMembershipChangeTimelineItemFor(eventItemProxy: eventItemProxy, member: userID, membershipChange: change, isOutgoing: isOutgoing)
+        case .profileChange(let displayName, let prevDisplayName, let avatarUrl, let prevAvatarUrl):
+            return buildStateProfileChangeTimelineItemFor(eventItemProxy: eventItemProxy, displayName: displayName, previousDisplayName: prevDisplayName, avatarURLString: avatarUrl, previousAvatarURLString: prevAvatarUrl, isOutgoing: isOutgoing)
         }
     }
 
@@ -354,12 +356,28 @@ struct RoomTimelineItemFactory: RoomTimelineItemFactoryProtocol {
 
     private func buildStateMembershipChangeTimelineItemFor(eventItemProxy: EventTimelineItemProxy,
                                                            member: String,
-                                                           membershipChange: MembershipChange,
+                                                           membershipChange: MembershipChange?,
                                                            isOutgoing: Bool) -> RoomTimelineItemProtocol? {
         guard let text = stateEventStringBuilder.buildString(for: membershipChange, member: member, sender: eventItemProxy.sender, isOutgoing: isOutgoing) else { return nil }
         return buildStateTimelineItem(eventItemProxy: eventItemProxy, text: text, isOutgoing: isOutgoing)
     }
 
+    // swiftlint:disable:next function_parameter_count
+    private func buildStateProfileChangeTimelineItemFor(eventItemProxy: EventTimelineItemProxy,
+                                                        displayName: String?,
+                                                        previousDisplayName: String?,
+                                                        avatarURLString: String?,
+                                                        previousAvatarURLString: String?,
+                                                        isOutgoing: Bool) -> RoomTimelineItemProtocol? {
+        guard let text = stateEventStringBuilder.buildProfileChangeString(displayName: displayName,
+                                                                          previousDisplayName: previousDisplayName,
+                                                                          avatarURLString: avatarURLString,
+                                                                          previousAvatarURLString: previousAvatarURLString,
+                                                                          member: eventItemProxy.sender.id,
+                                                                          memberIsYou: isOutgoing) else { return nil }
+        return buildStateTimelineItem(eventItemProxy: eventItemProxy, text: text, isOutgoing: isOutgoing)
+    }
+
     private func buildStateTimelineItem(eventItemProxy: EventTimelineItemProxy, text: String, isOutgoing: Bool) -> RoomTimelineItemProtocol {
         StateRoomTimelineItem(id: eventItemProxy.id,
                               text: text,

diff --git a/UnitTests/Sources/RoomStateEventStringBuilderTests.swift b/UnitTests/Sources/RoomStateEventStringBuilderTests.swift
@@ -48,10 +48,12 @@ class RoomStateEventStringBuilderTests: XCTestCase {
 
     func validateDisplayNameChange(senderID: String, oldName: String?, newName: String?, expectedString: String) {
         let sender = TimelineItemSender(id: senderID, displayName: newName)
-        let change = MembershipChange.profileChanged(displayName: newName, prevDisplayName: oldName, avatarUrl: nil, prevAvatarUrl: nil)
-
-        let string = stringBuilder.buildString(for: change, member: sender.id, sender: sender, isOutgoing: sender.id == userID)
-
+        let string = stringBuilder.buildProfileChangeString(displayName: newName,
+                                                            previousDisplayName: oldName,
+                                                            avatarURLString: nil,
+                                                            previousAvatarURLString: nil,
+                                                            member: sender.id,
+                                                            memberIsYou: sender.id == userID)
         XCTAssertEqual(string, expectedString)
     }
 
@@ -79,11 +81,12 @@ class RoomStateEventStringBuilderTests: XCTestCase {
                               oldAvatarURL: String?, newAvatarURL: String?,
                               expectedString: String) {
         let sender = TimelineItemSender(id: senderID, displayName: senderName)
-        let change = MembershipChange.profileChanged(displayName: senderName, prevDisplayName: senderName,
-                                                     avatarUrl: oldAvatarURL, prevAvatarUrl: newAvatarURL)
-
-        let string = stringBuilder.buildString(for: change, member: sender.id, sender: sender, isOutgoing: sender.id == userID)
-
+        let string = stringBuilder.buildProfileChangeString(displayName: senderName,
+                                                            previousDisplayName: senderName,
+                                                            avatarURLString: newAvatarURL,
+                                                            previousAvatarURLString: oldAvatarURL,
+                                                            member: sender.id,
+                                                            memberIsYou: sender.id == userID)
         XCTAssertEqual(string, expectedString)
     }
 }
diff --git a/project.yml b/project.yml
@@ -40,7 +40,7 @@ include:
 packages:
   MatrixRustSDK:
     url: https://github.com/matrix-org/matrix-rust-components-swift
-    exactVersion: 1.0.33-alpha
+    exactVersion: 1.0.35-alpha
     # path: ../matrix-rust-sdk
   DesignKit:
     path: DesignKit