letro/letro-ios
1
0
Fork 0
Code Issues Pull Requests Actions 5 Packages Projects Releases 2 Wiki Activity

Fix a bug where the image upload screen was unintentionally dismissed for some failures. (#4414)

Browse Source
This commit is contained in:
Doug
2025-08-14 18:19:57 +01:00
committed by GitHub
parent 1e5a5b36b2
commit de0a7ab30a
2 changed files with 108 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
ElementX/Sources/Screens/MediaUploadPreviewScreen/MediaUploadPreviewScreenViewModel.swift
View File

@@ -62,7 +62,6 @@ class MediaUploadPreviewScreenViewModel: MediaUploadPreviewScreenViewModelType,
Task {
defer { stopLoading() }
var shouldDismissOnCompletion = true
switch await processingTask.value {
case .success(let mediaInfos):
for mediaInfo in mediaInfos {
@@ -70,23 +69,20 @@ class MediaUploadPreviewScreenViewModel: MediaUploadPreviewScreenViewModelType,
case .success:
caption = nil // Set the caption only on the first uploaded file.
case .failure(let error):
MXLog.error("Failed processing media to upload with error: \(error)")
showError(label: L10n.screenMediaUploadPreviewErrorFailedProcessing)
MXLog.error("Failed sending media with error: \(error)")
showError(label: L10n.screenMediaUploadPreviewErrorFailedSending)
}
}
actionsSubject.send(.dismiss)
case .failure(.maxUploadSizeUnknown):
showAlert(.maxUploadSizeUnknown)
shouldDismissOnCompletion = false
case .failure(.maxUploadSizeExceeded(let limit)):
showAlert(.maxUploadSizeExceeded(limit: limit))
case .failure(let error):
MXLog.error("Failed processing media to upload with error: \(error)")
showError(label: L10n.screenMediaUploadPreviewErrorFailedProcessing)
}
if shouldDismissOnCompletion {
actionsSubject.send(.dismiss)
}
}
case .cancel:
requestHandle?.cancel()

119
UnitTests/Sources/MediaUploadPreviewScreenViewModelTests.swift
View File

@@ -13,6 +13,8 @@ import XCTest
class MediaUploadPreviewScreenViewModelTests: XCTestCase {
var timelineProxy: TimelineProxyMock!
var clientProxy: ClientProxyMock!
var userIndicatorController: UserIndicatorControllerMock!
var viewModel: MediaUploadPreviewScreenViewModel!
var context: MediaUploadPreviewScreenViewModel.Context { viewModel.context }
@@ -33,77 +35,97 @@ class MediaUploadPreviewScreenViewModelTests: XCTestCase {
}
func testImageUploadWithoutCaption() async throws {
setUpViewModel(url: imageURL, expectedCaption: nil)
setUpViewModel(urls: [imageURL], expectedCaption: nil)
context.caption = .init("")
try await send()
}
func testImageUploadWithBlankCaption() async throws {
setUpViewModel(url: imageURL, expectedCaption: nil)
setUpViewModel(urls: [imageURL], expectedCaption: nil)
context.caption = .init(" ")
try await send()
}
func testImageUploadWithCaption() async throws {
let caption = "This is a really great image!"
setUpViewModel(url: imageURL, expectedCaption: caption)
setUpViewModel(urls: [imageURL], expectedCaption: caption)
context.caption = .init(string: caption)
try await send()
}
func testVideoUploadWithoutCaption() async throws {
setUpViewModel(url: videoURL, expectedCaption: nil)
setUpViewModel(urls: [videoURL], expectedCaption: nil)
context.caption = .init("")
try await send()
}
func testVideoUploadWithCaption() async throws {
let caption = "Check out this video!"
setUpViewModel(url: videoURL, expectedCaption: caption)
setUpViewModel(urls: [videoURL], expectedCaption: caption)
context.caption = .init(string: caption)
try await send()
}
func testAudioUploadWithoutCaption() async throws {
setUpViewModel(url: audioURL, expectedCaption: nil)
setUpViewModel(urls: [audioURL], expectedCaption: nil)
context.caption = .init("")
try await send()
}
func testAudioUploadWithCaption() async throws {
let caption = "Listen to this!"
setUpViewModel(url: audioURL, expectedCaption: caption)
setUpViewModel(urls: [audioURL], expectedCaption: caption)
context.caption = .init(string: caption)
try await send()
}
func testFileUploadWithoutCaption() async throws {
setUpViewModel(url: fileURL, expectedCaption: nil)
setUpViewModel(urls: [fileURL], expectedCaption: nil)
context.caption = .init("")
try await send()
}
func testFileUploadWithCaption() async throws {
let caption = "Please will you check my article."
setUpViewModel(url: fileURL, expectedCaption: caption)
setUpViewModel(urls: [fileURL], expectedCaption: caption)
context.caption = .init(string: caption)
try await send()
}
func testProcessingFailure() async throws {
// Given an upload screen for a non-existent file.
setUpViewModel(urls: [badImageURL], expectedCaption: nil)
XCTAssertFalse(context.viewState.shouldDisableInteraction)
XCTAssertEqual(userIndicatorController.submitIndicatorDelayCallsCount, 0)
// When attempting to send the file
let deferredFailure = deferFailure(viewModel.actions, timeout: 1, message: "The screen should remain visible.") { $0 == .dismiss }
context.send(viewAction: .send)
XCTAssertTrue(context.viewState.shouldDisableInteraction, "The interaction should be disabled while sending.")
XCTAssertEqual(userIndicatorController.submitIndicatorDelayCallsCount, 1) // Loading indicator
// Then the failure should occur preventing the screen from being dismissed.
try await deferredFailure.fulfill()
XCTAssertFalse(context.viewState.shouldDisableInteraction)
XCTAssertEqual(userIndicatorController.submitIndicatorDelayCallsCount, 2, "An error indicator should be shown.")
}
func testUploadWithUnknownMaxUploadSize() async throws {
// Given an upload screen that is unable to fetch the max upload size.
setUpViewModel(url: imageURL, expectedCaption: nil, maxUploadSizeResult: .failure(.sdkError(ClientProxyMockError.generic)))
setUpViewModel(urls: [imageURL], expectedCaption: nil, maxUploadSizeResult: .failure(.sdkError(ClientProxyMockError.generic)))
XCTAssertFalse(context.viewState.shouldDisableInteraction)
XCTAssertNil(context.alertInfo)
// When attempting to send the media.
let deferredAlert = deferFulfillment(context.observe(\.viewState.bindings.alertInfo)) { $0 != nil }
let deferredFailure = deferFailure(viewModel.actions, timeout: 1, message: "The screen should remain visible.") { $0 == .dismiss }
context.send(viewAction: .send)
XCTAssertTrue(context.viewState.shouldDisableInteraction, "The interaction should be disabled while sending.")
// Then alert should be shown to tell the user it failed.
try await deferredAlert.fulfill()
try await deferredFailure.fulfill()
XCTAssertFalse(context.viewState.shouldDisableInteraction)
XCTAssertEqual(context.alertInfo?.id, .maxUploadSizeUnknown)
@@ -121,29 +143,90 @@ class MediaUploadPreviewScreenViewModelTests: XCTestCase {
func testUploadExceedingMaxUploadSize() async throws {
// Given an upload screen with a really small max upload size.
setUpViewModel(url: imageURL, expectedCaption: nil, maxUploadSizeResult: .success(100))
setUpViewModel(urls: [imageURL], expectedCaption: nil, maxUploadSizeResult: .success(100))
XCTAssertFalse(context.viewState.shouldDisableInteraction)
XCTAssertNil(context.alertInfo)
// When attempting to send an image that is larger the limit.
let deferredAlert = deferFulfillment(context.observe(\.viewState.bindings.alertInfo)) { $0 != nil }
let deferredFailure = deferFailure(viewModel.actions, timeout: 1, message: "The screen should remain visible.") { $0 == .dismiss }
context.send(viewAction: .send)
XCTAssertTrue(context.viewState.shouldDisableInteraction, "The interaction should be disabled while sending.")
// Then an alert should be shown to inform the user of the max upload size.
try await deferredAlert.fulfill()
try await deferredFailure.fulfill()
XCTAssertFalse(context.viewState.shouldDisableInteraction)
XCTAssertEqual(context.alertInfo?.id, .maxUploadSizeExceeded(limit: 100))
}
func testMultipleFiles() async throws {
// Given an upload screen with multiple media files.
setUpViewModel(urls: [fileURL, imageURL, fileURL], expectedCaption: nil)
XCTAssertFalse(context.viewState.shouldDisableInteraction)
XCTAssertEqual(userIndicatorController.submitIndicatorDelayCallsCount, 0)
// When attempting to send the files.
let deferredDismiss = deferFulfillment(viewModel.actions) { $0 == .dismiss }
context.send(viewAction: .send)
XCTAssertTrue(context.viewState.shouldDisableInteraction, "The interaction should be disabled while sending.")
XCTAssertEqual(userIndicatorController.submitIndicatorDelayCallsCount, 1) // Loading indicator
// Then the screen should be dismissed once all of the files have been sent.
try await deferredDismiss.fulfill()
XCTAssertEqual(timelineProxy.sendImageUrlThumbnailURLImageInfoCaptionRequestHandleCallsCount, 1)
XCTAssertEqual(timelineProxy.sendFileUrlFileInfoCaptionRequestHandleCallsCount, 2)
XCTAssertEqual(userIndicatorController.submitIndicatorDelayCallsCount, 1, "Only a loading indicator should be shown.")
}
func testMultipleFilesWithProcessingFailure() async throws {
// Given an upload screen for a non-existent file.
setUpViewModel(urls: [imageURL, fileURL, badImageURL], expectedCaption: nil)
XCTAssertFalse(context.viewState.shouldDisableInteraction)
XCTAssertEqual(userIndicatorController.submitIndicatorDelayCallsCount, 0)
// When attempting to send the file
let deferredFailure = deferFailure(viewModel.actions, timeout: 1, message: "The screen should remain visible.") { $0 == .dismiss }
context.send(viewAction: .send)
XCTAssertTrue(context.viewState.shouldDisableInteraction, "The interaction should be disabled while sending.")
XCTAssertEqual(userIndicatorController.submitIndicatorDelayCallsCount, 1) // Loading indicator
// Then the failure should occur preventing the screen from being dismissed.
try await deferredFailure.fulfill()
XCTAssertFalse(context.viewState.shouldDisableInteraction)
XCTAssertEqual(userIndicatorController.submitIndicatorDelayCallsCount, 2, "An error indicator should be shown.")
}
func testMultipleFilesWithSendFailure() async throws {
// Given an upload screen with multiple media files where one of the files will fail to send.
setUpViewModel(urls: [fileURL, imageURL, imageURL, fileURL], expectedCaption: nil, simulateImageSendFailures: true)
XCTAssertFalse(context.viewState.shouldDisableInteraction)
XCTAssertEqual(userIndicatorController.submitIndicatorDelayCallsCount, 0)
// When attempting to send the files.
let deferredDismiss = deferFulfillment(viewModel.actions) { $0 == .dismiss }
context.send(viewAction: .send)
XCTAssertTrue(context.viewState.shouldDisableInteraction, "The interaction should be disabled while sending.")
XCTAssertEqual(userIndicatorController.submitIndicatorDelayCallsCount, 1) // Loading indicator
// Then the screen should be dismissed so the user can see which files made it into the timeline.
try await deferredDismiss.fulfill()
XCTAssertEqual(timelineProxy.sendImageUrlThumbnailURLImageInfoCaptionRequestHandleCallsCount, 2)
XCTAssertEqual(timelineProxy.sendFileUrlFileInfoCaptionRequestHandleCallsCount, 2)
XCTAssertEqual(userIndicatorController.submitIndicatorDelayCallsCount, 3, "Error indicators for each failure should be shown.")
}
// MARK: - Helpers
private var audioURL: URL { assertResourceURL(filename: "test_audio.mp3") }
private var fileURL: URL { assertResourceURL(filename: "test_pdf.pdf") }
private var imageURL: URL { assertResourceURL(filename: "test_animated_image.gif") }
private var videoURL: URL { assertResourceURL(filename: "landscape_test_video.mov") }
private var badImageURL = URL(filePath: "/home/user/this_file_doesn't_exist.jpg")
private func assertResourceURL(filename: String) -> URL {
guard let url = Bundle(for: Self.self).url(forResource: filename, withExtension: nil) else {
@@ -153,7 +236,10 @@ class MediaUploadPreviewScreenViewModelTests: XCTestCase {
return url
}
private func setUpViewModel(url: URL, expectedCaption: String?, maxUploadSizeResult: Result<UInt, ClientProxyError>? = nil) {
private func setUpViewModel(urls: [URL],
expectedCaption: String?,
maxUploadSizeResult: Result<UInt, ClientProxyError>? = nil,
simulateImageSendFailures: Bool = false) {
timelineProxy = TimelineProxyMock(.init())
timelineProxy.sendAudioUrlAudioInfoCaptionRequestHandleClosure = { [weak self] _, _, caption, _ in
self?.verifyCaption(caption, expectedCaption: expectedCaption) ?? .failure(.sdkError(TestError.unknown))
@@ -162,7 +248,8 @@ class MediaUploadPreviewScreenViewModelTests: XCTestCase {
self?.verifyCaption(caption, expectedCaption: expectedCaption) ?? .failure(.sdkError(TestError.unknown))
}
timelineProxy.sendImageUrlThumbnailURLImageInfoCaptionRequestHandleClosure = { [weak self] _, _, _, caption, _ in
self?.verifyCaption(caption, expectedCaption: expectedCaption) ?? .failure(.sdkError(TestError.unknown))
guard !simulateImageSendFailures else { return .failure(.sdkError(TestError.unknown)) }
return self?.verifyCaption(caption, expectedCaption: expectedCaption) ?? .failure(.sdkError(TestError.unknown))
}
timelineProxy.sendVideoUrlThumbnailURLVideoInfoCaptionRequestHandleClosure = { [weak self] _, _, _, caption, _ in
self?.verifyCaption(caption, expectedCaption: expectedCaption) ?? .failure(.sdkError(TestError.unknown))
@@ -173,14 +260,16 @@ class MediaUploadPreviewScreenViewModelTests: XCTestCase {
clientProxy.underlyingMaxMediaUploadSize = maxUploadSizeResult
}
viewModel = MediaUploadPreviewScreenViewModel(mediaURLs: [url],
userIndicatorController = UserIndicatorControllerMock()
viewModel = MediaUploadPreviewScreenViewModel(mediaURLs: urls,
title: "Some File",
isRoomEncrypted: true,
shouldShowCaptionWarning: true,
mediaUploadingPreprocessor: MediaUploadingPreprocessor(appSettings: ServiceLocator.shared.settings),
timelineController: MockTimelineController(timelineProxy: timelineProxy),
clientProxy: clientProxy,
userIndicatorController: UserIndicatorControllerMock())
userIndicatorController: userIndicatorController)
}
private func verifyCaption(_ caption: String?, expectedCaption: String?) -> Result<Void, TimelineProxyError> {