MBL-1016: Add custom operators for common API patterns #1900
Conversation
| @@ -1,7 +1,6 @@ | |||
| import Library | |||
| import SwiftUI | |||
|
|
|||
| // TODO(MBL-1039) - Refactor this so that saveTriggered takes a closure, not a binding | |||
There was a problem hiding this comment.
Did this a while ago but forgot the //TODO.
| /// } | ||
| /// } | ||
|
|
||
| public func handleFailureAndAllowRetry(_ onFailure: @escaping (Self.Failure) -> Void) |
There was a problem hiding this comment.
I had an "interesting" learning experience writing this operator. I spent about two hours trying to wrangle the types (with generics) for this function, and got completely stuck. I ended up changing things around a bit so that the types were simpler, which worked - but I think it's worth noting that any custom operators that handle complex type transformations (say, with a flatMap) are likely to be beastly to write.
|
|
||
| return ErrorEnvelope.couldNotParseJSON | ||
| .mapFetchResults { (data: GraphAPI.FetchUserEmailQuery.Data) -> UserEnvelope<GraphUserEmail>? in | ||
| UserEnvelope<GraphUserEmail>.userEnvelope(from: data) |
There was a problem hiding this comment.
Much cleaner!
| @@ -57,24 +57,16 @@ public final class ReportProjectFormViewModel: ReportProjectFormViewModelType, | |||
| self.saveTriggeredSubject | |||
| .compactMap { [weak self] _ in | |||
| self?.createFlaggingInput() | |||
| .handleFailureAndAllowRetry { _ in | |||
There was a problem hiding this comment.
Behold, more readable!
| -> AnyPublisher<NewOutputType, ErrorEnvelope> { | ||
| return self.tryMap { (data: Output) -> NewOutputType in | ||
| guard let envelope = convertData(data) else { | ||
| throw ErrorEnvelope.couldNotParseJSON |
There was a problem hiding this comment.
We could consider a different generic error here, if we wished, but I think this is fine.
amy-at-kickstarter
force-pushed
the
feat/adyer/mbl-1016/graphql-combine-patterns
branch
from
10fb839
to
ec68b06
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1900 +/- ##
==========================================
- Coverage 83.72% 83.72% -0.01%
==========================================
Files 1226 1227 +1
Lines 111788 111790 +2
Branches 29723 29727 +4
==========================================
Hits 93596 93596
- Misses 17175 17176 +1
- Partials 1017 1018 +1 ☔ View full report in Codecov by Sentry. |
amy-at-kickstarter
force-pushed
the
feat/adyer/mbl-1016/graphql-combine-patterns
branch
from
ec68b06
to
bd23876
Compare
amy-at-kickstarter
deleted the
feat/adyer/mbl-1016/graphql-combine-patterns
branch
📲 What
Add two custom operators for API patterns that (I expect) will be fairly common in Combine.
🤔 Why
As part of the SwiftUI migration work, we want to establish some repeatable patterns for the future. When I ported
ReportProjectFormViewModelto be completely in Combine, I left two somewhat confusing operations - a way to map fetch results, and a way to handle API failures without cancelling the pipeline. I wanted to make a cleaner interface for these two patterns, since I assume they'll pop up again.