Allow resending messages with failed attachments (#2966)

* Mark message as failed if an attachment fails uploading

* Re-upload failed attachments when resending message

* When all attachments finish uploading, mark message as pending send

* Update CHANGELOG.md

* Revert "Mark message as failed if an attachment fails uploading"

This reverts commit f289378.

* Mark message as failed when attachment fails uploading in AttachmentQueueUploader

* Fix editing message with attachments re-sending the message again

* Remove unused prop in tests

* Fix editing message with local attachments sending the local temp file to remote server

* Fix comment typo

* Provide a comment to explain creating local payload

* Fix attachments save to attachment storage with the same id

* Fix file attachments retry button not doing anything

* Update CHANGELOG.md

* Fix vale CI

.github/workflows/vale-doc-lint.yml

@@ -16,6 +16,7 @@ jobs:
           reporter: github-pr-check
           fail_on_error: true
           files: docusaurus
+          version: "2.30.0"
         env:
           # Required, set by GitHub actions automatically:
           # https://docs.github.com/en/actions/security-guides/automatic-token-authentication#about-the-github_token-secret

CHANGELOG.md

@@ -3,7 +3,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 # Upcoming
 
-### 🔄 Changed
+## StreamChat
+### 🐞 Fixed
+- Fix not possible to resend messages with failed attachments [#2966](https://github.com/GetStream/stream-chat-swift/pull/2966)
+
+## StreamChatUI
+### 🐞 Fixed
+- Fix file attachments retry button not retrying upload [#2966](https://github.com/GetStream/stream-chat-swift/pull/2966)
 
 # [4.47.0](https://github.com/GetStream/stream-chat-swift/releases/tag/4.47.0)
 _January 09, 2024_

Sources/StreamChat/Models/Attachments/AnyAttachmentPayload.swift

@@ -190,19 +190,28 @@ extension AttachmentPayload {
     }
 }
 
+extension ChatMessageAttachment<Data> {
+    func toAnyAttachmentPayload() -> AnyAttachmentPayload? {
+        let types = ChatClient.attachmentTypesRegistry
+        guard let payloadType = types[type] else { return nil }
+        guard let payload = try? JSONDecoder.default.decode(
+            payloadType,
+            from: self.payload
+        ) else {
+            return nil
+        }
+
+        // If the attachment is local, we should create the payload as a local file
+        if let uploadingState = self.uploadingState, uploadingState.state != .uploaded {
+            return AnyAttachmentPayload(type: type, payload: payload, localFileURL: uploadingState.localFileURL)
+        }
+
+        return AnyAttachmentPayload(payload: payload)
+    }
+}
+
 public extension Array where Element == ChatMessageAttachment<Data> {
     func toAnyAttachmentPayload() -> [AnyAttachmentPayload] {
-        compactMap { attachment in
-            let types = ChatClient.attachmentTypesRegistry
-            guard let payloadType = types[attachment.type] else { return nil }
-            guard let payload = try? JSONDecoder.default.decode(
-                payloadType,
-                from: attachment.payload
-            ) else {
-                return nil
-            }
-
-            return AnyAttachmentPayload(payload: payload)
-        }
+        compactMap { $0.toAnyAttachmentPayload() }
     }
 }

Sources/StreamChat/Workers/Background/AttachmentQueueUploader.swift

@@ -109,7 +109,7 @@ class AttachmentQueueUploader: Worker {
 
             if let temporaryURL = attachment.localURL {
                 do {
-                    let localURL = try attachmentStorage.storeAttachment(at: temporaryURL)
+                    let localURL = try attachmentStorage.storeAttachment(id: id, temporaryURL: temporaryURL)
                     attachment.localURL = localURL
                 } catch {
                     log.error("Could not copy attachment to local storage: \(error.localizedDescription)", subsystems: .offlineSupport)
@@ -154,6 +154,23 @@ class AttachmentQueueUploader: Worker {
             // Update attachment local state.
             attachmentDTO.localState = newState
 
+            let message = attachmentDTO.message
+
+            // When all attachments finish uploading, mark message pending send
+            if newState == .uploaded {
+                let allAttachmentsAreUploaded = message.attachments.filter { $0.localState != .uploaded }.isEmpty
+                // We only want to make a message to be resent when it is in failed state
+                // If we did not check the state, when editing a message, it would resend an existing message
+                if allAttachmentsAreUploaded && message.localMessageState == .sendingFailed {
+                    message.localMessageState = .pendingSend
+                }
+            }
+
+            // If attachment failed upload, mark message as failed
+            if newState == .uploadingFailed {
+                message.localMessageState = .sendingFailed
+            }
+
             if var uploadedAttachment = uploadedAttachment {
                 self?.updateRemoteUrl(of: &uploadedAttachment)
                 if let processedAttachment = self?.attachmentPostProcessor?.process(uploadedAttachment: uploadedAttachment) {
@@ -243,9 +260,12 @@ private class AttachmentStorage {
     /// Since iOS 8, we cannot use absolute paths to access resources because the intermediate folders can change between sessions/app runs. The content of it, when
     /// using `.documentsDirectory`, is stable though.
     /// Because of that, if the file is already in our storage, the only thing we will do is to return a fresh and valid url to access it.
-    func storeAttachment(at temporaryURL: URL) throws -> URL {
-        let id = temporaryURL.lastPathComponent
-        let sandboxedURL = baseURL.appendingPathComponent(id)
+    func storeAttachment(id: AttachmentId, temporaryURL: URL) throws -> URL {
+        let fileExtension = temporaryURL.pathExtension
+        let fileName = temporaryURL.deletingPathExtension().lastPathComponent
+        let attachmentId = [id.cid.rawValue, id.messageId, String(id.index)].joined(separator: "-")
+        let fileId = "\(fileName)-\(attachmentId).\(fileExtension)"
+        let sandboxedURL = baseURL.appendingPathComponent(fileId)
 
         guard !fileExists(at: sandboxedURL) else {
             return sandboxedURL

Sources/StreamChat/Workers/MessageUpdater.swift

@@ -549,6 +549,11 @@ class MessageUpdater: Worker {
                     reason: "only failed or bounced messages can be resent."
                 )
             }
+
+            let failedAttachments = messageDTO.attachments.filter { $0.localState == .uploadingFailed }
+            failedAttachments.forEach {
+                $0.localState = .pendingUpload
+            }
 
             messageDTO.localMessageState = .pendingSend
         }, completion: completion)

Sources/StreamChatUI/Appearance+Images.swift

@@ -233,7 +233,8 @@ public extension Appearance {
         public var fileAttachmentActionIcons: [LocalAttachmentState?: UIImage] {
             get { _fileAttachmentActionIcons ??
                 [
-                    .uploaded: download,
+                    // Uncomment when download feature is done
+                    // .uploaded: download,
                     .uploadingFailed: restart,
                     nil: folder
                 ]

Sources/StreamChatUI/ChatMessageList/Attachments/File/ChatFileAttachmentListView+ItemView.swift

@@ -12,9 +12,12 @@ extension ChatMessageFileAttachmentListView {
             didSet { updateContentIfNeeded() }
         }
 
-        /// Closure what should happen on tapping the given attachment.
+        /// Closure which notifies when the user tapped the attachment.
         open var didTapOnAttachment: ((ChatMessageFileAttachment) -> Void)?
 
+        /// Closure which notifies when the user tapped an attachment action. (Ex: Retry)
+        open var didTapActionOnAttachment: ((ChatMessageFileAttachment) -> Void)?
+
         /// Label which shows name of the file, usually with extension (file.pdf)
         open private(set) lazy var fileNameLabel = UILabel()
             .withoutAutoresizingMaskConstraints
@@ -62,7 +65,11 @@ extension ChatMessageFileAttachmentListView {
             super.setUp()
 
             let tapRecognizer = UITapGestureRecognizer(target: self, action: #selector(didTapOnAttachment(_:)))
-            addGestureRecognizer(tapRecognizer)
+            mainContainerStackView.addGestureRecognizer(tapRecognizer)
+
+            let actionTapRecognizer = UITapGestureRecognizer(target: self, action: #selector(didTapActionOnAttachment(_:)))
+            actionIconImageView.addGestureRecognizer(actionTapRecognizer)
+            actionIconImageView.isUserInteractionEnabled = true
         }
 
         override open func setUpAppearance() {
@@ -140,6 +147,11 @@ extension ChatMessageFileAttachmentListView {
             didTapOnAttachment?(attachment)
         }
 
+        @objc open func didTapActionOnAttachment(_ recognizer: UITapGestureRecognizer) {
+            guard let attachment = content else { return }
+            didTapActionOnAttachment?(attachment)
+        }
+
         private var fileIcon: UIImage? {
             guard let file = content?.payload.file else { return nil }
 

Sources/StreamChatUI/ChatMessageList/Attachments/File/ChatMessageFileAttachmentListView.swift

@@ -12,9 +12,12 @@ open class ChatMessageFileAttachmentListView: _View, ComponentsProvider {
         didSet { updateContentIfNeeded() }
     }
 
-    /// Closure what should happen on tapping the given attachment.
+    /// Closure which notifies when the user tapped the attachment.
     open var didTapOnAttachment: ((ChatMessageFileAttachment) -> Void)?
 
+    /// Closure which notifies when the user tapped an attachment action. (Ex: Retry)
+    open var didTapActionOnAttachment: ((ChatMessageFileAttachment) -> Void)?
+
     /// Container which holds one or multiple attachment views in self.
     open private(set) lazy var containerStackView: ContainerStackView = ContainerStackView()
         .withoutAutoresizingMaskConstraints
@@ -35,6 +38,7 @@ open class ChatMessageFileAttachmentListView: _View, ComponentsProvider {
         content.forEach { attachment in
             let item = components.fileAttachmentView.init()
             item.didTapOnAttachment = { [weak self] in self?.didTapOnAttachment?($0) }
+            item.didTapActionOnAttachment = { [weak self] in self?.didTapActionOnAttachment?($0) }
             item.content = attachment
             containerStackView.addArrangedSubview(item)
         }

Sources/StreamChatUI/ChatMessageList/Attachments/File/FileAttachmentViewInjector.swift

@@ -7,8 +7,11 @@ import UIKit
 
 /// The delegate used `FileAttachmentViewInjector` to communicate user interactions.
 public protocol FileActionContentViewDelegate: ChatMessageContentViewDelegate {
-    /// Called when the user taps on attachment action
+    /// Called when the user taps on the attachment.
     func didTapOnAttachment(_ attachment: ChatMessageFileAttachment, at indexPath: IndexPath?)
+
+    /// Called when the user taps on the action of the attachment. (Ex: Retry)
+    func didTapActionOnAttachment(_ attachment: ChatMessageFileAttachment, at indexPath: IndexPath?)
 }
 
 public class FilesAttachmentViewInjector: AttachmentViewInjector {
@@ -25,6 +28,13 @@ public class FilesAttachmentViewInjector: AttachmentViewInjector {
             delegate.didTapOnAttachment(attachment, at: self?.contentView.indexPath?())
         }
 
+        attachmentListView.didTapActionOnAttachment = { [weak self] attachment in
+            guard
+                let delegate = self?.contentView.delegate as? FileActionContentViewDelegate
+            else { return }
+            delegate.didTapActionOnAttachment(attachment, at: self?.contentView.indexPath?())
+        }
+
         return attachmentListView.withoutAutoresizingMaskConstraints
     }()
 

Sources/StreamChatUI/ChatMessageList/ChatMessageListVC.swift

@@ -969,7 +969,7 @@ open class ChatMessageListVC: _ViewController,
         }
     }
 
-    // MARK: - Attachment Action Delegates
+    // MARK: - Link Action Delegates
 
     open func didTapOnLinkAttachment(
         _ attachment: ChatMessageLinkAttachment,
@@ -978,13 +978,26 @@ open class ChatMessageListVC: _ViewController,
         router.showLinkPreview(link: attachment.url)
     }
 
+    // MARK: - File Action Delegates
+
     open func didTapOnAttachment(
         _ attachment: ChatMessageFileAttachment,
         at indexPath: IndexPath?
     ) {
         router.showFilePreview(fileURL: attachment.assetURL)
     }
 
+    open func didTapActionOnAttachment(_ attachment: ChatMessageFileAttachment, at indexPath: IndexPath?) {
+        switch attachment.uploadingState?.state {
+        case .uploadingFailed:
+            client
+                .messageController(cid: attachment.id.cid, messageId: attachment.id.messageId)
+                .restartFailedAttachmentUploading(with: attachment.id)
+        default:
+            break
+        }
+    }
+
     /// Executes the provided action on the message
     open func didTapOnAttachmentAction(
         _ action: AttachmentAction,

StreamChat.xcodeproj/project.pbxproj

@@ -5654,7 +5654,6 @@
 				8800A26E258A04D5006D64C4 /* ChatMessageAttachmentPreviewVC.swift */,
 				88BA7F86258B97C9006CE0C5 /* UploadingOverlayView.swift */,
 				ADD4C0D92B30A71F00F230FF /* File */,
-				ADD4C0DD2B30A91900F230FF /* Unsupported */,
 				ADD4C0D82B30A6E100F230FF /* Giphy */,
 				ADD4C0DA2B30A78500F230FF /* VoiceRecording */,
 				ADD4C0DB2B30A7B400F230FF /* Gallery */,

Tests/StreamChatTests/Models/Attachments/AnyAttachmentPayload_Tests.swift

@@ -132,4 +132,40 @@ final class AnyAttachmentPayload_Tests: XCTestCase {
         XCTAssertEqual(sut.type, "custom")
         XCTAssertEqual(payload.calories, 20)
     }
+
+    func test_toAnyAttachmentPayload_whenRemoteAttachment_thenLocalFileShouldBeNil() throws {
+        let remoteAttachment = ChatMessageImageAttachment(
+            id: .unique,
+            type: .image,
+            payload: .init(title: nil, imageRemoteURL: .localYodaImage),
+            uploadingState: nil
+        ).asAnyAttachment
+
+        let anyAttachmentPayload = try XCTUnwrap(remoteAttachment.toAnyAttachmentPayload())
+        XCTAssertNil(anyAttachmentPayload.localFileURL)
+    }
+
+    func test_toAnyAttachmentPayload_whenLocalAttachment_whenUploaded_thenLocalFileShouldBeNil() throws {
+        let localAttachment = ChatMessageImageAttachment(
+            id: .unique,
+            type: .image,
+            payload: .init(title: nil, imageRemoteURL: .localYodaImage),
+            uploadingState: try .mock(localFileURL: .localYodaImage, state: .uploaded)
+        ).asAnyAttachment
+
+        let anyAttachmentPayload = try XCTUnwrap(localAttachment.toAnyAttachmentPayload())
+        XCTAssertNil(anyAttachmentPayload.localFileURL)
+    }
+
+    func test_toAnyAttachmentPayload_whenLocalAttachment_whenNotUploaded_thenLocalFileExists() throws {
+        let localAttachment = ChatMessageImageAttachment(
+            id: .unique,
+            type: .image,
+            payload: .init(title: nil, imageRemoteURL: .localYodaImage),
+            uploadingState: try .mock(localFileURL: .localYodaImage, state: .uploadingFailed)
+        ).asAnyAttachment
+
+        let anyAttachmentPayload = try XCTUnwrap(localAttachment.toAnyAttachmentPayload())
+        XCTAssertEqual(anyAttachmentPayload.localFileURL, .localYodaImage)
+    }
 }