[PART 2] Networking Refactor - Unify REST & GraphQL networking implementation
#182
Conversation
…lt w/ purchase flow
…ding in POST body
* Remove Eligibility feature * remove two types for Eligibility feature
scannillo
changed the title
REST & GraphQL networking implementationREST & GraphQL networking implementation
scannillo
force-pushed
the
refactor-graphql
branch
from
3fde783 to
2d1bcc0
Compare
scannillo
force-pushed
the
refactor-graphql
branch
from
42b2a22 to
0695f72
Compare
| /// :nodoc: | ||
| public func fetch(request: GraphQLRequest) async throws -> HTTPResponse { | ||
| let url = try constructGraphQLURL(queryName: request.queryNameForURL) | ||
|
|
There was a problem hiding this comment.
not sure if we need this extra space but up to you:
There was a problem hiding this comment.
I did it intentionally to group postBody and postData together, but down to remove this newline if it's harder to read
| let postBody = GraphQLHTTPPostBody(query: request.query, variables: request.variables) | ||
| let postData = try JSONEncoder().encode(postBody) | ||
|
|
||
| let httpRequest = HTTPRequest( |
There was a problem hiding this comment.
If we single line this is it above our linter line length? May be nice to just group all of the properties together but not sure if this is more readable this way.
There was a problem hiding this comment.
It does not violate our line length, but I thought it was a little cleaner. Happy to go either way though!
There was a problem hiding this comment.
Either way is fine, was more just curious. If it makes more sense to have things grouped together a particular way that's fine too.
There was a problem hiding this comment.
Not sure what you mean by "group all of these properties together"? You mean single line it?
There was a problem hiding this comment.
Ok I single-lined both the HTTPRequest() definitions (see commit 7417e15)
There was a problem hiding this comment.
Not sure what you mean by "group all of these properties together"? You mean single line it?
Ah, sorry was referencing this comment
There was a problem hiding this comment.
That link takes me back to:
If we single line this is it above our linter line length? May be nice to just group all of the properties together but not sure if this is more readable this way.
Do you mean to remove the newlines between these variables?
There was a problem hiding this comment.
Weird, idk why GH is linking to that. This comment above is the one I was trying to reference:
I did it intentionally to group postBody and postData together, but down to remove this newline if it's harder to read
Not a blocker though, whatever you feel is best
Sources/CorePayments/Networking/GraphQL/GraphQLErrorResponse.swift
Outdated
Show resolved
Hide resolved
scannillo
force-pushed
the
refactor-graphql
branch
from
7417e15 to
ff3ed9b
Compare
| url: url, | ||
| body: request.body | ||
| ) | ||
| let httpRequest = HTTPRequest(headers: headers, method: request.method, url: url, body: request.body) | ||
|
|
| public func fetch(request: GraphQLRequest) async throws -> HTTPResponse { | ||
| let url = try constructGraphQLURL(queryName: request.queryNameForURL) | ||
|
|
||
| let postBody = GraphQLHTTPPostBody(query: request.query, variables: request.variables) |
Reason for changes
Follow up to #177.
This PR:
APIClient(which previously just passed requests along to HTTP) into a trueNetworkingCoordinatorthat transforms REST & GraphQL --> HTTP.Summary of changes
fetch(request: GraphQLRequest)method toAPIClientGraphQLClientand other no-longer-used GraphQL filesGraphQLQueryResponse-->GraphQLHTTPResponseNext
APIClienttoNetworkingClientVaultPaymentMethodTokensAPIclass (waiting on Victoria's Vault without Purchase #172 PR to merge)Note
Tests are expected to fail.
Checklist
Added a changelog entryAuthors
@scannillo