Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add response iteration and conversion #129

Merged
merged 1 commit into from Nov 20, 2016

Conversation

joshsmith
Copy link
Contributor

This PR reworks Stripe response handling to iterate over the response and convert necessary values.

This does not yet handle lists from Stripe nor take in options to push to Stripe.

Reduces scope of certain struct keys as a result.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.9%) to 13.75% when pulling 3a0c76a on add-response-iteration-and-conversion into 9600d28 on 2.0.

Returns the Stripe response mapping of keys to types.
"""
@spec response_mapping :: Keyword.t
def response_mapping, do: @response_mapping
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just FYI, I think coveralls skips over single-line defs for coverage analysis

```
{:ok, resp} = Stripe.Connect.Oauth.token(code)

IO.inspect resp
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably don't want this in released code 💣

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DavidAntaramian this is just a code example.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless what you're saying is that the above renders the below? It might be better to indicate with an

iex> {:ok, resp} = Stripe.Connect.OAuth.token(code)
...> IO.inspect resp
%Stripe.Connect.OAuth.TokenResponse{...}

example

fingerprint: :string,
funding: :string,
last4: :string,
metadata: :metadata,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would just specify this as :map

Copy link
Contributor Author

@joshsmith joshsmith Nov 20, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason I chose :metadata instead of map was to be explicit that it was just structureless data (from Stripe's perspective), and because convert_value has a when is_map(format) clause, which might make that confusing.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.9%) to 13.75% when pulling 62b2416 on add-response-iteration-and-conversion into 9600d28 on 2.0.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.8%) to 13.836% when pulling 21b49b7 on add-response-iteration-and-conversion into 9600d28 on 2.0.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.4%) to 14.286% when pulling 84e1986 on add-response-iteration-and-conversion into 9600d28 on 2.0.

@joshsmith joshsmith force-pushed the add-response-iteration-and-conversion branch from 84e1986 to c8ab995 Compare November 20, 2016 04:03
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.4%) to 14.286% when pulling c8ab995 on add-response-iteration-and-conversion into 9600d28 on 2.0.

@joshsmith joshsmith merged commit ef505fd into 2.0 Nov 20, 2016
@joshsmith joshsmith deleted the add-response-iteration-and-conversion branch November 20, 2016 04:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants