Skip to content

Commit

Permalink
Handle API changes from Rust.
Browse files Browse the repository at this point in the history
There are some bad assumptions about profile changes in here.
  • Loading branch information
pixlwave committed Jan 31, 2023
1 parent d51a9b3 commit 97280d2
Show file tree
Hide file tree
Showing 10 changed files with 83 additions and 60 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,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)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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, // FIXME: This is a bad assumption
memberIsYou: isOutgoing) // FIXME: This is a bad assumption
.map(AttributedString.init)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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?
}
23 changes: 11 additions & 12 deletions ElementX/Sources/Services/Timeline/TimelineItemProxy.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)
}
}
Expand Down Expand Up @@ -113,7 +112,7 @@ struct EventTimelineItemProxy: CustomDebugStringConvertible {
}

var reactions: [Reaction] {
item.reactions()
item.reactions() ?? []
}

var timestamp: Date {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand All @@ -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)
Expand All @@ -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
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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, // FIXME: This is a bad assumption
memberIsYou: isOutgoing) else { return nil } // FIXME: This is a bad assumption
return buildStateTimelineItem(eventItemProxy: eventItemProxy, text: text, isOutgoing: isOutgoing)
}

private func buildStateTimelineItem(eventItemProxy: EventTimelineItemProxy, text: String, isOutgoing: Bool) -> RoomTimelineItemProtocol {
StateRoomTimelineItem(id: eventItemProxy.id,
text: text,
Expand Down
21 changes: 12 additions & 9 deletions UnitTests/Sources/RoomStateEventStringBuilderTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}
}

0 comments on commit 97280d2

Please sign in to comment.