From c8b9620f65d21da5521314cc71b9aadc4fecccee Mon Sep 17 00:00:00 2001 From: Sasha Weiss Date: Wed, 5 Apr 2023 13:47:32 -0700 Subject: [PATCH] Add confirmation to forward-from-media-page flow --- .../ForwardMessageViewController.swift | 57 +++++++---- .../MediaPageViewController.swift | 94 ++++++++++++------- .../translations/en.lproj/Localizable.strings | 6 ++ 3 files changed, 104 insertions(+), 53 deletions(-) diff --git a/Signal/src/ViewControllers/ForwardMessageViewController.swift b/Signal/src/ViewControllers/ForwardMessageViewController.swift index 61cf8947c3..25055d87e7 100644 --- a/Signal/src/ViewControllers/ForwardMessageViewController.swift +++ b/Signal/src/ViewControllers/ForwardMessageViewController.swift @@ -95,6 +95,34 @@ class ForwardMessageViewController: InteractiveSheetViewController { } } + public class func present( + forAttachmentStreams attachmentStreams: [TSAttachmentStream], + fromMessage message: TSMessage, + from fromViewController: UIViewController, + delegate: ForwardMessageDelegate + ) { + do { + let builder = Item.Builder(interaction: message) + + builder.attachments = try attachmentStreams.map { attachmentStream in + try attachmentStream.cloneAsSignalAttachment() + } + + let item: Item = builder.build() + + present( + content: .single(item: item), + from: fromViewController, + delegate: delegate + ) + } catch let error { + ForwardMessageViewController.showAlertForForwardError( + error: error, + forwardedInteractionCount: 1 + ) + } + } + public class func present( forStoryMessage storyMessage: StoryMessage, from fromViewController: UIViewController, @@ -128,13 +156,6 @@ class ForwardMessageViewController: InteractiveSheetViewController { present(content: .single(item: builder.build()), from: fromViewController, delegate: delegate) } - public class func present( - _ textAttachment: TextAttachment, - from fromViewController: UIViewController, - delegate: ForwardMessageDelegate - ) { - } - private class func present(content: Content, from fromViewController: UIViewController, delegate: ForwardMessageDelegate) { @@ -346,8 +367,6 @@ extension ForwardMessageViewController { private func send(item: Item, toOutgoingMessageRecipientThreads outgoingMessageRecipientThreads: [TSThread]) -> Promise { AssertIsOnMainThread() - let componentState = item.componentState - if let stickerMetadata = item.stickerMetadata { let stickerInfo = stickerMetadata.stickerInfo if StickerManager.isStickerInstalled(stickerInfo: stickerInfo) { @@ -355,7 +374,7 @@ extension ForwardMessageViewController { self.send(installedSticker: stickerInfo, thread: recipientThread) } } else { - guard let stickerAttachment = componentState?.stickerAttachment else { + guard let stickerAttachment = item.stickerAttachment else { return Promise(error: OWSAssertionError("Missing stickerAttachment.")) } do { @@ -633,52 +652,52 @@ public struct ForwardMessageItem { fileprivate typealias Item = ForwardMessageItem let interaction: TSInteraction? - let componentState: CVComponentState? let attachments: [SignalAttachment]? let contactShare: ContactShareViewModel? let messageBody: MessageBody? let linkPreviewDraft: OWSLinkPreviewDraft? let stickerMetadata: StickerMetadata? + let stickerAttachment: TSAttachmentStream? let textAttachment: TextAttachment? fileprivate class Builder { let interaction: TSInteraction? - let componentState: CVComponentState? var attachments: [SignalAttachment]? var contactShare: ContactShareViewModel? var messageBody: MessageBody? var linkPreviewDraft: OWSLinkPreviewDraft? var stickerMetadata: StickerMetadata? + var stickerAttachment: TSAttachmentStream? var textAttachment: TextAttachment? - init(interaction: TSInteraction? = nil, componentState: CVComponentState? = nil) { + init(interaction: TSInteraction? = nil) { self.interaction = interaction - self.componentState = componentState } func build() -> ForwardMessageItem { ForwardMessageItem( interaction: interaction, - componentState: componentState, attachments: attachments, contactShare: contactShare, messageBody: messageBody, linkPreviewDraft: linkPreviewDraft, stickerMetadata: stickerMetadata, + stickerAttachment: stickerAttachment, textAttachment: textAttachment ) } } fileprivate var asBuilder: Builder { - let builder = Builder(interaction: interaction, componentState: componentState) + let builder = Builder(interaction: interaction) builder.attachments = attachments builder.contactShare = contactShare builder.messageBody = messageBody builder.linkPreviewDraft = linkPreviewDraft builder.stickerMetadata = stickerMetadata + builder.stickerAttachment = stickerAttachment return builder } @@ -702,7 +721,7 @@ public struct ForwardMessageItem { transaction: SDSAnyReadTransaction ) throws -> Item { - let builder = Builder(interaction: interaction, componentState: componentState) + let builder = Builder(interaction: interaction) let shouldHaveText = (selectionType == .allContent || selectionType == .secondaryContent) @@ -748,6 +767,10 @@ public struct ForwardMessageItem { if let stickerMetadata = componentState.stickerMetadata { builder.stickerMetadata = stickerMetadata + + if let stickerAttachment = componentState.stickerAttachment { + builder.stickerAttachment = stickerAttachment + } } } diff --git a/Signal/src/ViewControllers/MediaGallery/MediaPageViewController.swift b/Signal/src/ViewControllers/MediaGallery/MediaPageViewController.swift index 6c69775596..6d8b43cb8d 100644 --- a/Signal/src/ViewControllers/MediaGallery/MediaPageViewController.swift +++ b/Signal/src/ViewControllers/MediaGallery/MediaPageViewController.swift @@ -546,29 +546,6 @@ class MediaPageViewController: UIPageViewController { return view } - private func buildRenderItem(forGalleryItem galleryItem: MediaGalleryItem) -> CVRenderItem? { - return databaseStorage.read { transaction in - let interactionId = galleryItem.message.uniqueId - guard let interaction = TSInteraction.anyFetch(uniqueId: interactionId, - transaction: transaction) else { - owsFailDebug("Missing interaction.") - return nil - } - guard let thread = TSThread.anyFetch(uniqueId: interaction.uniqueThreadId, - transaction: transaction) else { - owsFailDebug("Missing thread.") - return nil - } - let threadAssociatedData = ThreadAssociatedData.fetchOrDefault(for: thread, - transaction: transaction) - return CVLoader.buildStandaloneRenderItem(interaction: interaction, - thread: thread, - threadAssociatedData: threadAssociatedData, - containerView: self.view, - transaction: transaction) - } - } - private func dismissSelf(animated isAnimated: Bool, completion: (() -> Void)? = nil) { guard let currentViewController else { return } @@ -602,24 +579,69 @@ class MediaPageViewController: UIPageViewController { AttachmentSharing.showShareUI(forAttachment: attachmentStream, sender: sender) } + /// Forwards all media from the message containing the currently gallery + /// item. + /// + /// Skips any media that we do not have downloaded. @objc private func didPressForward(_ sender: Any) { - let galleryItem: MediaGalleryItem = currentItem + let messageForCurrentItem = currentItem.message - guard let renderItem = buildRenderItem(forGalleryItem: galleryItem) else { - owsFailDebug("viewItem was unexpectedly nil") - return + let mediaAttachments: [TSAttachment] = databaseStorage.read { transaction in + messageForCurrentItem.bodyAttachments(with: transaction.unwrapGrdbRead) } - // Only forward media. - let selectionType: CVSelectionType = .primaryContent - let selectionItem = CVSelectionItem(interactionId: renderItem.interaction.uniqueId, - interactionType: renderItem.interaction.interactionType, - isForwardable: true, - selectionType: selectionType) - ForwardMessageViewController.present(forSelectionItems: [selectionItem], - from: self, - delegate: self) + let mediaAttachmentStreams: [TSAttachmentStream] = mediaAttachments.compactMap { attachment in + guard let attachmentStream = attachment as? TSAttachmentStream else { + // Our current media item should always be an attachment + // stream (downloaded). However, we can't guarantee that the + // same is true for other media in the message to forward. For + // example, another piece of media in this message may have + // failed to download. + // + // If so, we should continue trying to forward the ones we can. + + Logger.warn("Skipping attachment that is not an attachment stream. Did this attachment fail to download?") + return nil + } + + return attachmentStream + } + + switch mediaAttachmentStreams.count { + case 0: + owsFail("We should always have at least one attachment stream, for the current item.") + case 1: + ForwardMessageViewController.present( + forAttachmentStreams: mediaAttachmentStreams, + fromMessage: messageForCurrentItem, + from: self, + delegate: self + ) + default: + // If we are forwarding multiple items, warn the user first. + + OWSActionSheets.showConfirmationAlert( + message: OWSLocalizedString( + "MEDIA_PAGE_FORWARD_MEDIA_CONFIRM_MESSAGE", + comment: "Text explaining that the user will forward all media from a message." + ), + proceedTitle: OWSLocalizedString( + "MEDIA_PAGE_FORWARD_MEDIA_CONFIRM_TITLE", + comment: "Text confirming the user wants to forward media." + ), + proceedAction: { [weak self] _ in + guard let self else { return } + + ForwardMessageViewController.present( + forAttachmentStreams: mediaAttachmentStreams, + fromMessage: messageForCurrentItem, + from: self, + delegate: self + ) + } + ) + } } private func deleteCurrentMedia() { diff --git a/Signal/translations/en.lproj/Localizable.strings b/Signal/translations/en.lproj/Localizable.strings index 59d09b01c2..6b9345dd06 100644 --- a/Signal/translations/en.lproj/Localizable.strings +++ b/Signal/translations/en.lproj/Localizable.strings @@ -3574,6 +3574,12 @@ /* Section header in media gallery collection view */ "MEDIA_GALLERY_THIS_MONTH_HEADER" = "This Month"; +/* Text explaining that the user will forward all media from a message. */ +"MEDIA_PAGE_FORWARD_MEDIA_CONFIRM_MESSAGE" = "All media in this message will be forwarded."; + +/* Text confirming the user wants to forward media. */ +"MEDIA_PAGE_FORWARD_MEDIA_CONFIRM_TITLE" = "Forward"; + /* Context menu item in media viewer. Refers to deleting currently displayed photo/video. */ "MEDIA_VIEWER_DELETE_MEDIA_ACTION" = "Delete";