Fix a potential race condition when redacting a message. (#3061)

* Refactor the timeline item menu action provider. - Move it into its own struct. - Use an item, not an ID so it doesn't randomly change. - Move permissions into the room screen view model. * Use the stable ID when redacting/editing/forwarding a message. Just like we do when fetching the item in the actions menu.
2026-11-03 16:10:20 +00:00 · 2024-07-18 15:14:38 +01:00
parent 2fb7f65957
commit 6339fe2bf0
12 changed files with 337 additions and 275 deletions
@@ -21,14 +21,14 @@ import SwiftUI
 /// The contents of the context menu shown when right clicking an item in the timeline on a Mac
 struct TimelineItemMacContextMenu: View {
    let item: RoomTimelineItemProtocol
-    let actionProvider: (@MainActor (_ itemId: TimelineItemIdentifier) -> TimelineItemMenuActions?)?
+    let actionProvider: TimelineItemMenuActionProvider
    let send: (TimelineItemMenuAction) -> Void
    
    var body: some View {
        if ProcessInfo.processInfo.isiOSAppOnMac {
-            if let menuActions = actionProvider?(item.id) {
+            if let menuActions = actionProvider.makeActions() {
                Section {
-                    if item.isReactable {
+                    if !menuActions.reactions.isEmpty {
                        if #available(iOS 17.0, *) {
                            let reactions = (item as? EventBasedTimelineItemProtocol)?.properties.reactions ?? []
                            ControlGroup {
@@ -15,142 +15,8 @@
 //

 import Compound
-import SFSafeSymbols
 import SwiftUI

-struct TimelineItemMenuActions {
-    let reactions: [TimelineItemMenuReaction]
-    let actions: [TimelineItemMenuAction]
-    let debugActions: [TimelineItemMenuAction]
-    
-    init?(actions: [TimelineItemMenuAction], debugActions: [TimelineItemMenuAction]) {
-        if actions.isEmpty, debugActions.isEmpty {
-            return nil
-        }
-        
-        self.actions = actions
-        self.debugActions = debugActions
-        reactions = [
-            .init(key: "👍️", symbol: .handThumbsup),
-            .init(key: "👎️", symbol: .handThumbsdown),
-            .init(key: "🔥", symbol: .flame),
-            .init(key: "❤️", symbol: .heart),
-            .init(key: "👏", symbol: .handsClap)
-        ]
-    }
-    
-    var canReply: Bool {
-        for action in actions {
-            if case .reply = action {
-                return true
-            }
-        }
-        
-        return false
-    }
-}
-
-struct TimelineItemMenuReaction {
-    let key: String
-    let symbol: SFSymbol
-}
-
-enum TimelineItemMenuAction: Identifiable, Hashable {
-    case copy
-    case edit
-    case copyPermalink
-    case redact
-    case reply(isThread: Bool)
-    case forward(itemID: TimelineItemIdentifier)
-    case viewSource
-    case retryDecryption(sessionID: String)
-    case report
-    case react
-    case toggleReaction(key: String)
-    case endPoll(pollStartID: String)
-    
-    var id: Self { self }
-    
-    /// Whether the item should cancel a reply/edit occurring in the composer.
-    var switchToDefaultComposer: Bool {
-        switch self {
-        case .reply, .edit:
-            return false
-        default:
-            return true
-        }
-    }
-    
-    /// Whether the action should be shown for an item that failed to send.
-    var canAppearInFailedEcho: Bool {
-        switch self {
-        case .copy, .edit, .redact, .viewSource:
-            return true
-        default:
-            return false
-        }
-    }
-    
-    /// Whether the action should be shown for a redacted item.
-    var canAppearInRedacted: Bool {
-        switch self {
-        case .viewSource:
-            return true
-        default:
-            return false
-        }
-    }
-    
-    /// Whether or not the action is destructive.
-    var isDestructive: Bool {
-        switch self {
-        case .redact, .report:
-            return true
-        default:
-            return false
-        }
-    }
-    
-    /// The action's label.
-    @ViewBuilder
-    var label: some View {
-        switch self {
-        case .copy:
-            Label(L10n.actionCopy, icon: \.copy)
-        case .edit:
-            Label(L10n.actionEdit, icon: \.edit)
-        case .copyPermalink:
-            Label(L10n.actionCopyLinkToMessage, icon: \.link)
-        case .reply(let isThread):
-            Label(isThread ? L10n.actionReplyInThread : L10n.actionReply, icon: \.reply)
-        case .forward:
-            Label(L10n.actionForward, icon: \.forward)
-        case .redact:
-            Label(L10n.actionRemove, icon: \.delete)
-        case .viewSource:
-            Label(L10n.actionViewSource, icon: \.code)
-        case .retryDecryption:
-            Label(L10n.actionRetryDecryption, systemImage: "arrow.down.message")
-        case .report:
-            Label(L10n.actionReportContent, icon: \.chatProblem)
-        case .react:
-            Label(L10n.actionReact, icon: \.reactionAdd)
-        case .toggleReaction:
-            // Unused label - manually created in TimelineItemMacContextMenu.
-            Label(L10n.actionReact, icon: \.reactionAdd)
-        case .endPoll:
-            Label(L10n.actionEndPoll, icon: \.pollsEnd)
-        }
-    }
-}
-
-extension RoomTimelineItemProtocol {
-    var isReactable: Bool {
-        guard let eventItem = self as? EventBasedTimelineItemProtocol else { return false }
-        return !eventItem.isRedacted && !eventItem.hasFailedToSend && !eventItem.hasFailedDecryption
-    }
-}
-
 struct TimelineItemMenu: View {
    @EnvironmentObject private var context: RoomScreenViewModel.Context
    @Environment(\.dismiss) private var dismiss
@@ -171,7 +37,7 @@ struct TimelineItemMenu: View {
            
            ScrollView {
                VStack(alignment: .leading, spacing: 0.0) {
-                    if item.isReactable {
+                    if !actions.reactions.isEmpty {
                        reactionsSection
                            .padding(.top, 4.0)
                            .padding(.bottom, 8.0)
@@ -317,7 +183,7 @@ struct TimelineItemMenu_Previews: PreviewProvider, TestablePreview {
    @ViewBuilder
    static var testView: some View {
        if let item = RoomTimelineItemFixtures.singleMessageChunk.first as? EventBasedTimelineItemProtocol,
-           let actions = TimelineItemMenuActions(actions: [.copy, .edit, .reply(isThread: false), .redact], debugActions: [.viewSource]) {
+           let actions = TimelineItemMenuActions(isReactable: true, actions: [.copy, .edit, .reply(isThread: false), .redact], debugActions: [.viewSource]) {
            TimelineItemMenu(item: item, actions: actions)
                .environmentObject(viewModel.context)
        }
@@ -0,0 +1,138 @@
+//
+// Copyright 2024 New Vector Ltd
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+import SFSafeSymbols
+import SwiftUI
+
+struct TimelineItemMenuActions {
+    let reactions: [TimelineItemMenuReaction]
+    let actions: [TimelineItemMenuAction]
+    let debugActions: [TimelineItemMenuAction]
+    
+    init?(isReactable: Bool, actions: [TimelineItemMenuAction], debugActions: [TimelineItemMenuAction]) {
+        if !isReactable, actions.isEmpty, debugActions.isEmpty {
+            return nil
+        }
+        
+        self.actions = actions
+        self.debugActions = debugActions
+        reactions = if isReactable {
+            [
+                .init(key: "👍️", symbol: .handThumbsup),
+                .init(key: "👎️", symbol: .handThumbsdown),
+                .init(key: "🔥", symbol: .flame),
+                .init(key: "❤️", symbol: .heart),
+                .init(key: "👏", symbol: .handsClap)
+            ]
+        } else {
+            []
+        }
+    }
+}
+
+struct TimelineItemMenuReaction {
+    let key: String
+    let symbol: SFSymbol
+}
+
+enum TimelineItemMenuAction: Identifiable, Hashable {
+    case copy
+    case edit
+    case copyPermalink
+    case redact
+    case reply(isThread: Bool)
+    case forward(itemID: TimelineItemIdentifier)
+    case viewSource
+    case retryDecryption(sessionID: String)
+    case report
+    case react
+    case toggleReaction(key: String)
+    case endPoll(pollStartID: String)
+    
+    var id: Self { self }
+    
+    /// Whether the item should cancel a reply/edit occurring in the composer.
+    var switchToDefaultComposer: Bool {
+        switch self {
+        case .reply, .edit:
+            return false
+        default:
+            return true
+        }
+    }
+    
+    /// Whether the action should be shown for an item that failed to send.
+    var canAppearInFailedEcho: Bool {
+        switch self {
+        case .copy, .edit, .redact, .viewSource:
+            return true
+        default:
+            return false
+        }
+    }
+    
+    /// Whether the action should be shown for a redacted item.
+    var canAppearInRedacted: Bool {
+        switch self {
+        case .viewSource:
+            return true
+        default:
+            return false
+        }
+    }
+    
+    /// Whether or not the action is destructive.
+    var isDestructive: Bool {
+        switch self {
+        case .redact, .report:
+            return true
+        default:
+            return false
+        }
+    }
+    
+    /// The action's label.
+    @ViewBuilder
+    var label: some View {
+        switch self {
+        case .copy:
+            Label(L10n.actionCopy, icon: \.copy)
+        case .edit:
+            Label(L10n.actionEdit, icon: \.edit)
+        case .copyPermalink:
+            Label(L10n.actionCopyLinkToMessage, icon: \.link)
+        case .reply(let isThread):
+            Label(isThread ? L10n.actionReplyInThread : L10n.actionReply, icon: \.reply)
+        case .forward:
+            Label(L10n.actionForward, icon: \.forward)
+        case .redact:
+            Label(L10n.actionRemove, icon: \.delete)
+        case .viewSource:
+            Label(L10n.actionViewSource, icon: \.code)
+        case .retryDecryption:
+            Label(L10n.actionRetryDecryption, systemImage: "arrow.down.message")
+        case .report:
+            Label(L10n.actionReportContent, icon: \.chatProblem)
+        case .react:
+            Label(L10n.actionReact, icon: \.reactionAdd)
+        case .toggleReaction:
+            // Unused label - manually created in TimelineItemMacContextMenu.
+            Label(L10n.actionReact, icon: \.reactionAdd)
+        case .endPoll:
+            Label(L10n.actionEndPoll, icon: \.pollsEnd)
+        }
+    }
+}
@@ -0,0 +1,106 @@
+//
+// Copyright 2024 New Vector Ltd
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+import Foundation
+
+struct TimelineItemMenuActionProvider {
+    let timelineItem: RoomTimelineItemProtocol
+    let canCurrentUserRedactSelf: Bool
+    let canCurrentUserRedactOthers: Bool
+    let isDM: Bool
+    let isViewSourceEnabled: Bool
+    
+    // swiftlint:disable:next cyclomatic_complexity
+    func makeActions() -> TimelineItemMenuActions? {
+        guard let item = timelineItem as? EventBasedTimelineItemProtocol else {
+            // Don't show a context menu for non-event based items.
+            return nil
+        }
+
+        if timelineItem is StateRoomTimelineItem {
+            // Don't show a context menu for state events.
+            return nil
+        }
+
+        var debugActions: [TimelineItemMenuAction] = []
+        if isViewSourceEnabled {
+            debugActions.append(.viewSource)
+        }
+
+        if let encryptedItem = timelineItem as? EncryptedRoomTimelineItem {
+            switch encryptedItem.encryptionType {
+            case .megolmV1AesSha2(let sessionID, _):
+                debugActions.append(.retryDecryption(sessionID: sessionID))
+            default:
+                break
+            }
+            
+            return .init(isReactable: false, actions: [.copyPermalink], debugActions: debugActions)
+        }
+        
+        var actions: [TimelineItemMenuAction] = []
+
+        if item.canBeRepliedTo {
+            if let messageItem = item as? EventBasedMessageTimelineItemProtocol {
+                actions.append(.reply(isThread: messageItem.isThreaded))
+            } else {
+                actions.append(.reply(isThread: false))
+            }
+        }
+        
+        if item.isForwardable {
+            actions.append(.forward(itemID: item.id))
+        }
+
+        if item.isEditable {
+            actions.append(.edit)
+        }
+
+        if item.isCopyable {
+            actions.append(.copy)
+        }
+        
+        if item.isRemoteMessage {
+            actions.append(.copyPermalink)
+        }
+
+        if canRedactItem(item), let poll = item.pollIfAvailable, !poll.hasEnded, let eventID = item.id.eventID {
+            actions.append(.endPoll(pollStartID: eventID))
+        }
+        
+        if canRedactItem(item) {
+            actions.append(.redact)
+        }
+
+        if !item.isOutgoing {
+            actions.append(.report)
+        }
+
+        if item.hasFailedToSend {
+            actions = actions.filter(\.canAppearInFailedEcho)
+        }
+
+        if item.isRedacted {
+            actions = actions.filter(\.canAppearInRedacted)
+        }
+
+        return .init(isReactable: item.isReactable, actions: actions, debugActions: debugActions)
+    }
+    
+    private func canRedactItem(_ item: EventBasedTimelineItemProtocol) -> Bool {
+        item.isOutgoing ? canCurrentUserRedactSelf : canCurrentUserRedactOthers && !isDM
+    }
+}
@@ -57,7 +57,12 @@ struct RoomScreen: View {
            .alert(item: $context.alertInfo)
            .sheet(item: $context.debugInfo) { TimelineItemDebugView(info: $0) }
            .sheet(item: $context.actionMenuInfo) { info in
-                context.viewState.timelineItemMenuActionProvider?(info.item.id).map { actions in
+                let actions = TimelineItemMenuActionProvider(timelineItem: info.item,
+                                                             canCurrentUserRedactSelf: context.viewState.canCurrentUserRedactSelf,
+                                                             canCurrentUserRedactOthers: context.viewState.canCurrentUserRedactOthers,
+                                                             isDM: context.viewState.isEncryptedOneToOneRoom,
+                                                             isViewSourceEnabled: context.viewState.isViewSourceEnabled).makeActions()
+                if let actions {
                    TimelineItemMenu(item: info.item, actions: actions)
                        .environmentObject(context)
                }
@@ -137,14 +137,18 @@ struct TimelineItemBubbledStylerView<Content: View>: View {
            .swipeRightAction {
                SwipeToReplyView(timelineItem: timelineItem)
            } shouldStartAction: {
-                context.viewState.timelineItemMenuActionProvider?(timelineItem.id)?.canReply ?? false
+                timelineItem.canBeRepliedTo
            } action: {
                let isThread = (timelineItem as? EventBasedMessageTimelineItemProtocol)?.isThreaded ?? false
                context.send(viewAction: .handleTimelineItemMenuAction(itemID: timelineItem.id, action: .reply(isThread: isThread)))
            }
            .contextMenu {
-                TimelineItemMacContextMenu(item: timelineItem,
-                                           actionProvider: context.viewState.timelineItemMenuActionProvider) { action in
+                let provider = TimelineItemMenuActionProvider(timelineItem: timelineItem,
+                                                              canCurrentUserRedactSelf: context.viewState.canCurrentUserRedactSelf,
+                                                              canCurrentUserRedactOthers: context.viewState.canCurrentUserRedactOthers,
+                                                              isDM: context.viewState.isEncryptedOneToOneRoom,
+                                                              isViewSourceEnabled: context.viewState.isViewSourceEnabled)
+                TimelineItemMacContextMenu(item: timelineItem, actionProvider: provider) { action in
                    context.send(viewAction: .handleTimelineItemMenuAction(itemID: timelineItem.id, action: action))
                }
            }