From 6a27075d2bfcbf66efacb3a01c9ab175e62eb7d4 Mon Sep 17 00:00:00 2001 From: manuroe Date: Mon, 20 May 2019 19:40:52 +0200 Subject: [PATCH 1/3] Reactions: Remove the send reaction hack as it is now done in the SDK --- .../ReactionsMenuViewModel.swift | 51 +------------------ .../RoomContextualMenuPresenter.swift | 2 +- 2 files changed, 2 insertions(+), 51 deletions(-) diff --git a/Riot/Modules/Room/ContextualMenu/ReactionsMenu/ReactionsMenuViewModel.swift b/Riot/Modules/Room/ContextualMenu/ReactionsMenu/ReactionsMenuViewModel.swift index 593e2f79a..93ba3cb41 100644 --- a/Riot/Modules/Room/ContextualMenu/ReactionsMenu/ReactionsMenuViewModel.swift +++ b/Riot/Modules/Room/ContextualMenu/ReactionsMenu/ReactionsMenuViewModel.swift @@ -21,7 +21,6 @@ final class ReactionsMenuViewModel: ReactionsMenuViewModelType { // MARK: - Properties // MARK: Private - private let session: MXSession // TODO: To remove. Only required for reactUsingHack() private let aggregations: MXAggregations private let roomId: String private let eventId: String @@ -38,11 +37,10 @@ final class ReactionsMenuViewModel: ReactionsMenuViewModelType { // MARK: - Setup - init(aggregations: MXAggregations, roomId: String, eventId: String, session: MXSession) { + init(aggregations: MXAggregations, roomId: String, eventId: String) { self.aggregations = aggregations self.roomId = roomId self.eventId = eventId - self.session = session self.loadData() self.listenToDataUpdate() @@ -142,16 +140,6 @@ final class ReactionsMenuViewModel: ReactionsMenuViewModelType { return } - // The server does not support support reaction yet - // Use a fallback mechanism - // TODO: To remove once the feature has landed on matrix.org homeserver - if let mxError = MXError(nsError: error) { - if mxError.errcode == kMXErrCodeStringUnrecognized { - sself.reactUsingHack(withReaction: reaction) - return - } - } - sself.coordinatorDelegate?.reactionsMenuViewModel(sself, didReactionFailedWithError: error, reaction: reaction.rawValue, isAddReaction: true) }) } else { @@ -209,41 +197,4 @@ final class ReactionsMenuViewModel: ReactionsMenuViewModelType { self.react(withReaction: unreaction, selected: false) } } - - /// reactUsingHack directly sends a `m.reaction` room message instead of using the `/send_relation` api. - /// - /// TODO: To remove once the feature has landed on matrix.org homeserver - /// - /// - Parameter reaction: the reaction - private func reactUsingHack(withReaction reaction: ReactionsMenuReaction) { - guard let room = self.session.room(withRoomId: self.roomId) else { - print("[ReactionsMenuViewModel] reactUsingHack: Error: Unknown room: \(self.roomId)") - return - } - - print("[ReactionsMenuViewModel] reactUsingHack") - - let reactionContent = [ - "m.relates_to": [ - "rel_type": "m.annotation", - "event_id": self.eventId, - "key": reaction.rawValue] - ] - - var nilEvent: MXEvent? - room.sendEvent(.reaction, content: reactionContent, localEcho: &nilEvent, completion: { [weak self] ( completion) in - guard let sself = self else { - return - } - switch completion { - case .success: - print("[ReactionsMenuViewModel] reactUsingHack: Success") - sself.coordinatorDelegate?.reactionsMenuViewModel(sself, didReactionComplete: reaction.rawValue, isAddReaction: true) - - case.failure(let error): - print("[ReactionsMenuViewModel] reactUsingHack: Error: \(error)") - sself.coordinatorDelegate?.reactionsMenuViewModel(sself, didReactionFailedWithError: error, reaction: reaction.rawValue, isAddReaction: true) - } - }) - } } diff --git a/Riot/Modules/Room/ContextualMenu/RoomContextualMenuPresenter.swift b/Riot/Modules/Room/ContextualMenu/RoomContextualMenuPresenter.swift index 7bf3be01b..2a4c39a53 100644 --- a/Riot/Modules/Room/ContextualMenu/RoomContextualMenuPresenter.swift +++ b/Riot/Modules/Room/ContextualMenu/RoomContextualMenuPresenter.swift @@ -105,7 +105,7 @@ final class RoomContextualMenuPresenter: NSObject { func showReactionsMenu(forEvent eventId: String, inRoom roomId: String, session: MXSession, aroundFrame frame: CGRect) { - let reactionsMenuViewModel = ReactionsMenuViewModel(aggregations: session.aggregations, roomId: roomId, eventId: eventId, session: session) + let reactionsMenuViewModel = ReactionsMenuViewModel(aggregations: session.aggregations, roomId: roomId, eventId: eventId) self.roomContextualMenuViewController?.showReactionsMenu(withViewModel: reactionsMenuViewModel, aroundFrame: frame) } } From 7bdab43e1601a348e079ff65a591f452495c948f Mon Sep 17 00:00:00 2001 From: manuroe Date: Mon, 20 May 2019 19:48:29 +0200 Subject: [PATCH 2/3] Reactions menu: Do not notify delegate if reaction requires an unreaction This avoids to call the delegate twice. Note: In a short future, we will no more have those 3 state buttons --- .../ReactionsMenuViewModel.swift | 32 +++++++++++++------ 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/Riot/Modules/Room/ContextualMenu/ReactionsMenu/ReactionsMenuViewModel.swift b/Riot/Modules/Room/ContextualMenu/ReactionsMenu/ReactionsMenuViewModel.swift index 93ba3cb41..a4158f36f 100644 --- a/Riot/Modules/Room/ContextualMenu/ReactionsMenu/ReactionsMenuViewModel.swift +++ b/Riot/Modules/Room/ContextualMenu/ReactionsMenu/ReactionsMenuViewModel.swift @@ -123,7 +123,13 @@ final class ReactionsMenuViewModel: ReactionsMenuViewModelType { } } - private func react(withReaction reaction: ReactionsMenuReaction, selected: Bool) { + private func react(withReaction reaction: ReactionsMenuReaction, selected: Bool, notifyDelegate: Bool = true) { + + // If required, unreact first + if selected { + self.ensure3StateButtons(withReaction: reaction) + } + if selected { self.aggregations.sendReaction(reaction.rawValue, toEvent: self.eventId, inRoom: self.roomId, success: {[weak self] _ in @@ -131,7 +137,9 @@ final class ReactionsMenuViewModel: ReactionsMenuViewModelType { return } - sself.coordinatorDelegate?.reactionsMenuViewModel(sself, didReactionComplete: reaction.rawValue, isAddReaction: true) + if notifyDelegate { + sself.coordinatorDelegate?.reactionsMenuViewModel(sself, didReactionComplete: reaction.rawValue, isAddReaction: true) + } }, failure: {[weak self] (error) in print("[ReactionsMenuViewModel] react: Error: \(error)") @@ -140,7 +148,9 @@ final class ReactionsMenuViewModel: ReactionsMenuViewModelType { return } - sself.coordinatorDelegate?.reactionsMenuViewModel(sself, didReactionFailedWithError: error, reaction: reaction.rawValue, isAddReaction: true) + if notifyDelegate { + sself.coordinatorDelegate?.reactionsMenuViewModel(sself, didReactionFailedWithError: error, reaction: reaction.rawValue, isAddReaction: true) + } }) } else { @@ -150,7 +160,9 @@ final class ReactionsMenuViewModel: ReactionsMenuViewModelType { return } - sself.coordinatorDelegate?.reactionsMenuViewModel(sself, didReactionComplete: reaction.rawValue, isAddReaction: false) + if notifyDelegate { + sself.coordinatorDelegate?.reactionsMenuViewModel(sself, didReactionComplete: reaction.rawValue, isAddReaction: false) + } }, failure: {[weak self] (error) in print("[ReactionsMenuViewModel] react: Error: \(error)") @@ -159,14 +171,14 @@ final class ReactionsMenuViewModel: ReactionsMenuViewModelType { return } - sself.coordinatorDelegate?.reactionsMenuViewModel(sself, didReactionFailedWithError: error, reaction: reaction.rawValue, isAddReaction: false) + if notifyDelegate { + sself.coordinatorDelegate?.reactionsMenuViewModel(sself, didReactionFailedWithError: error, reaction: reaction.rawValue, isAddReaction: false) + } }) } - self.coordinatorDelegate?.reactionsMenuViewModel(self, didSendReaction: reaction.rawValue, isAddReaction: !selected) - - if selected { - self.ensure3StateButtons(withReaction: reaction) + if notifyDelegate { + self.coordinatorDelegate?.reactionsMenuViewModel(self, didSendReaction: reaction.rawValue, isAddReaction: !selected) } } @@ -194,7 +206,7 @@ final class ReactionsMenuViewModel: ReactionsMenuViewModelType { } if let unreaction = unreaction { - self.react(withReaction: unreaction, selected: false) + self.react(withReaction: unreaction, selected: false, notifyDelegate: false) } } } From a4b8192473051a50e4218a4f3d722c8468f2f88a Mon Sep 17 00:00:00 2001 From: manuroe Date: Mon, 20 May 2019 20:41:05 +0200 Subject: [PATCH 3/3] Reactions menu: Do not notify delegate if reaction requires an unreaction Fix Steve's remark --- .../ReactionsMenuViewModel.swift | 26 ++++++------------- 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/Riot/Modules/Room/ContextualMenu/ReactionsMenu/ReactionsMenuViewModel.swift b/Riot/Modules/Room/ContextualMenu/ReactionsMenu/ReactionsMenuViewModel.swift index a4158f36f..8fbf06106 100644 --- a/Riot/Modules/Room/ContextualMenu/ReactionsMenu/ReactionsMenuViewModel.swift +++ b/Riot/Modules/Room/ContextualMenu/ReactionsMenu/ReactionsMenuViewModel.swift @@ -72,7 +72,7 @@ final class ReactionsMenuViewModel: ReactionsMenuViewModelType { return } - self.react(withReaction: theReaction, selected: theNewState) + self.react(withReaction: theReaction, selected: theNewState, delegate: self.coordinatorDelegate) } // MARK: - Private @@ -123,7 +123,7 @@ final class ReactionsMenuViewModel: ReactionsMenuViewModelType { } } - private func react(withReaction reaction: ReactionsMenuReaction, selected: Bool, notifyDelegate: Bool = true) { + private func react(withReaction reaction: ReactionsMenuReaction, selected: Bool, delegate: ReactionsMenuViewModelCoordinatorDelegate? = nil) { // If required, unreact first if selected { @@ -137,9 +137,7 @@ final class ReactionsMenuViewModel: ReactionsMenuViewModelType { return } - if notifyDelegate { - sself.coordinatorDelegate?.reactionsMenuViewModel(sself, didReactionComplete: reaction.rawValue, isAddReaction: true) - } + delegate?.reactionsMenuViewModel(sself, didReactionComplete: reaction.rawValue, isAddReaction: true) }, failure: {[weak self] (error) in print("[ReactionsMenuViewModel] react: Error: \(error)") @@ -148,9 +146,7 @@ final class ReactionsMenuViewModel: ReactionsMenuViewModelType { return } - if notifyDelegate { - sself.coordinatorDelegate?.reactionsMenuViewModel(sself, didReactionFailedWithError: error, reaction: reaction.rawValue, isAddReaction: true) - } + delegate?.reactionsMenuViewModel(sself, didReactionFailedWithError: error, reaction: reaction.rawValue, isAddReaction: true) }) } else { @@ -160,9 +156,7 @@ final class ReactionsMenuViewModel: ReactionsMenuViewModelType { return } - if notifyDelegate { - sself.coordinatorDelegate?.reactionsMenuViewModel(sself, didReactionComplete: reaction.rawValue, isAddReaction: false) - } + delegate?.reactionsMenuViewModel(sself, didReactionComplete: reaction.rawValue, isAddReaction: false) }, failure: {[weak self] (error) in print("[ReactionsMenuViewModel] react: Error: \(error)") @@ -171,15 +165,11 @@ final class ReactionsMenuViewModel: ReactionsMenuViewModelType { return } - if notifyDelegate { - sself.coordinatorDelegate?.reactionsMenuViewModel(sself, didReactionFailedWithError: error, reaction: reaction.rawValue, isAddReaction: false) - } + delegate?.reactionsMenuViewModel(sself, didReactionFailedWithError: error, reaction: reaction.rawValue, isAddReaction: false) }) } - if notifyDelegate { - self.coordinatorDelegate?.reactionsMenuViewModel(self, didSendReaction: reaction.rawValue, isAddReaction: !selected) - } + delegate?.reactionsMenuViewModel(self, didSendReaction: reaction.rawValue, isAddReaction: !selected) } // We can like, dislike, be indifferent but we cannot like & dislike at the same time @@ -206,7 +196,7 @@ final class ReactionsMenuViewModel: ReactionsMenuViewModelType { } if let unreaction = unreaction { - self.react(withReaction: unreaction, selected: false, notifyDelegate: false) + self.react(withReaction: unreaction, selected: false) } } }