From 5fd4dae71a00f784e62e7a1d30e5b2ac0bb0d614 Mon Sep 17 00:00:00 2001 From: Amy Date: Mon, 4 Dec 2023 14:48:31 -0500 Subject: [PATCH 1/3] Remove defunct // TODO comment --- Kickstarter-iOS/SharedViews/LoadingBarButtonItem.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/Kickstarter-iOS/SharedViews/LoadingBarButtonItem.swift b/Kickstarter-iOS/SharedViews/LoadingBarButtonItem.swift index 83ea1f54de..e5e89d51dd 100644 --- a/Kickstarter-iOS/SharedViews/LoadingBarButtonItem.swift +++ b/Kickstarter-iOS/SharedViews/LoadingBarButtonItem.swift @@ -1,7 +1,6 @@ import Library import SwiftUI -// TODO(MBL-1039) - Refactor this so that saveTriggered takes a closure, not a binding struct LoadingBarButtonItem: View { @Binding var saveEnabled: Bool @Binding var showLoading: Bool From 94ad9c86206a4f62917abcca457a26e822dc0e51 Mon Sep 17 00:00:00 2001 From: Amy Date: Mon, 4 Dec 2023 16:54:54 -0500 Subject: [PATCH 2/3] MBL-1016: Add custom operator for mapping fetch results with Combine --- Kickstarter.xcodeproj/project.pbxproj | 4 ++++ KsApi/Publisher+Service.swift | 25 +++++++++++++++++++++++++ KsApi/Service.swift | 19 ++----------------- 3 files changed, 31 insertions(+), 17 deletions(-) create mode 100644 KsApi/Publisher+Service.swift diff --git a/Kickstarter.xcodeproj/project.pbxproj b/Kickstarter.xcodeproj/project.pbxproj index 4495c467ab..8fbaee413a 100644 --- a/Kickstarter.xcodeproj/project.pbxproj +++ b/Kickstarter.xcodeproj/project.pbxproj @@ -1498,6 +1498,7 @@ E1A149242ACE02B300F49709 /* FetchProjectsEnvelope+FetchBackerProjectsQueryDataTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = E1A149232ACE02B300F49709 /* FetchProjectsEnvelope+FetchBackerProjectsQueryDataTests.swift */; }; E1A149272ACE063400F49709 /* FetchBackerProjectsQueryDataTemplate.swift in Sources */ = {isa = PBXBuildFile; fileRef = E1A149262ACE063400F49709 /* FetchBackerProjectsQueryDataTemplate.swift */; }; E1AA8ABF2AEABBB100AC98BF /* Signal+Combine.swift in Sources */ = {isa = PBXBuildFile; fileRef = E1EA34EE2AE1B28400942A04 /* Signal+Combine.swift */; }; + E1BB25642B1E81AA000BD2D6 /* Publisher+Service.swift in Sources */ = {isa = PBXBuildFile; fileRef = E1BB25632B1E81AA000BD2D6 /* Publisher+Service.swift */; }; E1FDB1E82AEAAC6100285F93 /* CombineTestObserver.swift in Sources */ = {isa = PBXBuildFile; fileRef = E1FDB1E72AEAAC6100285F93 /* CombineTestObserver.swift */; }; /* End PBXBuildFile section */ @@ -3072,6 +3073,7 @@ E1A149212ACE013100F49709 /* FetchProjectsEnvelope.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = FetchProjectsEnvelope.swift; sourceTree = ""; }; E1A149232ACE02B300F49709 /* FetchProjectsEnvelope+FetchBackerProjectsQueryDataTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "FetchProjectsEnvelope+FetchBackerProjectsQueryDataTests.swift"; sourceTree = ""; }; E1A149262ACE063400F49709 /* FetchBackerProjectsQueryDataTemplate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FetchBackerProjectsQueryDataTemplate.swift; sourceTree = ""; }; + E1BB25632B1E81AA000BD2D6 /* Publisher+Service.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Publisher+Service.swift"; sourceTree = ""; }; E1EA34EE2AE1B28400942A04 /* Signal+Combine.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Signal+Combine.swift"; sourceTree = ""; }; E1FDB1E72AEAAC6100285F93 /* CombineTestObserver.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CombineTestObserver.swift; sourceTree = ""; }; /* End PBXFileReference section */ @@ -6524,6 +6526,7 @@ D01587761EEB2ED6006E7684 /* MockService.swift */, D01588261EEB2ED7006E7684 /* ServerConfig.swift */, D01588271EEB2ED7006E7684 /* Service.swift */, + E1BB25632B1E81AA000BD2D6 /* Publisher+Service.swift */, 8479CF2B2530A5D700FD13F1 /* Service+DecodeHelpers.swift */, 775DFAD4215EB2AB00620CED /* Service+RequestHelpers.swift */, D01588281EEB2ED7006E7684 /* ServiceTests.swift */, @@ -8631,6 +8634,7 @@ D67B6CD6221F468100B63A6B /* Location+Encode.swift in Sources */, 6093098D2A6054CB004297AF /* GraphAPI.TriggerThirdPartyEventInput+TriggerThirdPartyEventInput.swift in Sources */, 8AC3E105269F4D1C00168BF8 /* GraphAPI.ApplePay+ApplePayParams.swift in Sources */, + E1BB25642B1E81AA000BD2D6 /* Publisher+Service.swift in Sources */, D01588331EEB2ED7006E7684 /* Decodable.swift in Sources */, 06232D3E2795EC2C00A81755 /* Element+Helpers.swift in Sources */, 8ACF36E22627481C0026E74D /* ApiDateProtocol.swift in Sources */, diff --git a/KsApi/Publisher+Service.swift b/KsApi/Publisher+Service.swift new file mode 100644 index 0000000000..17d47c6f2a --- /dev/null +++ b/KsApi/Publisher+Service.swift @@ -0,0 +1,25 @@ +import Combine +import Foundation + +extension Publisher { + /// A convenience method for mapping the results of your fetch to another data type. Any unknown errors are returned in the error as `ErrorEnvelope.couldNotParseJSON`. + func mapFetchResults(_ convertData: @escaping ((Output) -> NewOutputType?)) + -> AnyPublisher { + return self.tryMap { (data: Output) -> NewOutputType in + guard let envelope = convertData(data) else { + throw ErrorEnvelope.couldNotParseJSON + } + + return envelope + } + .mapError { rawError in + + if let error = rawError as? ErrorEnvelope { + return error + } + + return ErrorEnvelope.couldNotParseJSON + } + .eraseToAnyPublisher() + } +} diff --git a/KsApi/Service.swift b/KsApi/Service.swift index fad620f275..e965fbaddb 100644 --- a/KsApi/Service.swift +++ b/KsApi/Service.swift @@ -367,24 +367,9 @@ public struct Service: ServiceType { -> AnyPublisher, ErrorEnvelope> { GraphQL.shared.client .fetch(query: GraphAPI.FetchUserEmailQuery()) - // TODO: make this a custom extension, we'll want to reuse this pattern - .tryMap { (data: GraphAPI.FetchUserEmailQuery.Data) -> UserEnvelope in - guard let envelope = UserEnvelope.userEnvelope(from: data) else { - throw ErrorEnvelope.couldNotParseJSON - } - - return envelope - } - .mapError { rawError in - - if let error = rawError as? ErrorEnvelope { - return error - } - - return ErrorEnvelope.couldNotParseJSON + .mapFetchResults { (data: GraphAPI.FetchUserEmailQuery.Data) -> UserEnvelope? in + UserEnvelope.userEnvelope(from: data) } - - .eraseToAnyPublisher() } public func fetchGraphUserSelf() From bd238765ba63ac24745dbac3ec53c63424ec89d5 Mon Sep 17 00:00:00 2001 From: Amy Date: Tue, 5 Dec 2023 11:03:37 -0500 Subject: [PATCH 3/3] MBL-1016: Add custom operator for handling API failures with Combine --- KsApi/Publisher+Service.swift | 23 ++++++++++++++++ .../ReportProjectFormViewModel.swift | 26 +++++++------------ 2 files changed, 32 insertions(+), 17 deletions(-) diff --git a/KsApi/Publisher+Service.swift b/KsApi/Publisher+Service.swift index 17d47c6f2a..b5d0393c83 100644 --- a/KsApi/Publisher+Service.swift +++ b/KsApi/Publisher+Service.swift @@ -22,4 +22,27 @@ extension Publisher { } .eraseToAnyPublisher() } + + /// A convenience method for gracefully catching API failures. + /// If you handle your API failure in receiveCompletion:, that will actually cancel the entire pipeline, which means the failed request can't be retried. + /// This is a wrapper around the .catch operator, which just makes it a bit easier to read. + /// + /// An example: + /// ``` + /// self.somethingHappened + /// .flatMap() { _ in + /// self.doAnAPIRequest + /// .handleFailureAndAllowRetry() { e in + /// showTheError(e) + /// } + /// } + + public func handleFailureAndAllowRetry(_ onFailure: @escaping (Self.Failure) -> Void) + -> AnyPublisher { + return self.catch { e in + onFailure(e) + return Empty() + } + .eraseToAnyPublisher() + } } diff --git a/Library/ViewModels/ReportProjectFormViewModel.swift b/Library/ViewModels/ReportProjectFormViewModel.swift index 52d3251d00..2a1372f9ad 100644 --- a/Library/ViewModels/ReportProjectFormViewModel.swift +++ b/Library/ViewModels/ReportProjectFormViewModel.swift @@ -57,24 +57,16 @@ public final class ReportProjectFormViewModel: ReportProjectFormViewModelType, self.saveTriggeredSubject .compactMap { [weak self] _ in self?.createFlaggingInput() + .handleFailureAndAllowRetry { _ in + self?.bannerMessage = MessageBannerViewViewModel(( + type: .error, + message: Strings.Something_went_wrong_please_try_again() + )) + self?.saveButtonEnabled = true + self?.saveButtonLoading = false + } } - .flatMap { [weak self] createFlaggingInput in - createFlaggingInput.catch { _ in - // An API error happens. - // We need to catch this up here in flatMap, instead of in sink, - // because we don't want an API failure to cancel this pipeline. - // If the pipeline gets canceled, you can't re-submit after a failure. - - self?.bannerMessage = MessageBannerViewViewModel(( - type: .error, - message: Strings.Something_went_wrong_please_try_again() - )) - self?.saveButtonEnabled = true - self?.saveButtonLoading = false - - return Empty() - } - } + .flatMap { $0 } .sink(receiveValue: { [weak self] _ in // Submitted successfully