Search for subscriptions, customers and credit card verifications. #43
Conversation
| @@ -81,6 +81,30 @@ case Customer.create(%{company: "Whale Corp"}) do | |||
| end | |||
| ``` | |||
|
|
|||
| Here is how to use the search endpoints: | |||
There was a problem hiding this comment.
Please add some space between the paragraph and the code block.
lib/credit_card_verification.ex
Outdated
| @doc """ | ||
| To search for credit card verifications, pass a map of search parameters. | ||
|
|
||
| See README for examples. |
There was a problem hiding this comment.
I'd prefer to have the examples in the docs, not just the README.
lib/credit_card_verification.ex
Outdated
|
|
||
| See README for examples. | ||
|
|
||
| Example: |
lib/credit_card_verification.ex
Outdated
|
|
||
| Example: | ||
|
|
||
| {:ok, verifications} = CreditCardVerification.search(%{customer: %{id: %{is: "1231231"}}}) |
There was a problem hiding this comment.
This needs to be indented four spaces, it also requires the full module name (Braintree.CreditCardVerification)
lib/credit_card_verification.ex
Outdated
| """ | ||
| @spec new(Map.t | [Map.t]) :: t | [t] | ||
| def new(%{"credit_card_verification" => map}), | ||
| do: new(map) |
There was a problem hiding this comment.
We don't use the newline + do: style anywhere else, so I'd prefer to be consistent and use the do/end style. Some day soon auto-formatting will make this a non issue, but we're not there yet!
lib/search.ex
Outdated
|
|
||
| # Credit card verification is an odd case because path to endpoints is different | ||
| # from the object name in the XML | ||
| defp fetch_ids_chunk(ids, "verifications"=resource, initializer, opts) when is_list(ids) do |
There was a problem hiding this comment.
I don't think it's necessary to bind to resource here, instead just post directly to "verifications/advanced_search".
lib/subscription.ex
Outdated
| @@ -164,6 +164,20 @@ defmodule Braintree.Subscription do | |||
| end | |||
|
|
|||
| @doc """ | |||
| To search for subscriptions, pass a map of search parameters. | |||
|
|
|||
| See README for examples. | |||
There was a problem hiding this comment.
Again, better to put one example in here. The "Example" needs to be fixed up as above too.
lib/xml/decoder.ex
Outdated
| @@ -40,6 +40,7 @@ defmodule Braintree.XML.Decoder do | |||
|
|
|||
| case attributes do | |||
| [type: "array"] -> %{name => transform({attributes, values})} | |||
| [type: "collection"] -> %{name => transform({attributes, values})} | |||
| _ -> %{name => transform(values)} | |||
There was a problem hiding this comment.
The _ indention here is even funnier now. As you're adding the new case it would be nice to pull it back.
lib/xml/decoder.ex
Outdated
| @@ -72,6 +73,10 @@ defmodule Braintree.XML.Decoder do | |||
| defp transform({[type: "array"], elements}), | |||
| do: Enum.map(without_nil(elements), &(elem(transform(&1), 1))) | |||
|
|
|||
| defp transform({[type: "collection"], elements}) do | |||
There was a problem hiding this comment.
Apparently we do use do: some places. Sorry about the inconsistency. Better to keep the do/end as you have it here.
There was a problem hiding this comment.
I do believe that in some cases, do: is cleaner. I'll let you decide.
lib/xml/decoder.ex
Outdated
| defp only_collection(elements), | ||
| do: elements | ||
| |> without_nil() | ||
| |> Enum.reject(fn e -> elem(e, 1) != [] end) |
There was a problem hiding this comment.
Let's avoid single character variables. This could use pattern matching rather than the elem function too: fn {_, value} -> value != [] end)
lib/http.ex
Outdated
| def resolve_unprocessable_entity_error(%{"api_error_response" => api_error_response}), | ||
| do: Error.new(api_error_response) | ||
| def resolve_unprocessable_entity_error(%{"unprocessable_entity" => api_response}), | ||
| do: :unprocessable_entity |
There was a problem hiding this comment.
This is the API response:
<unprocessable-entity />
My thinking was that there is no valuable data in there thus this approach. Let me know what you think.
lib/search.ex
Outdated
| end | ||
| end | ||
|
|
||
| defp ids(payload, resource, initializer, opts) when is_map(payload) do |
There was a problem hiding this comment.
Braintree's search is confusing by itself, but yes I agree. I am renaming this.
lib/xml/decoder.ex
Outdated
| @@ -72,6 +73,10 @@ defmodule Braintree.XML.Decoder do | |||
| defp transform({[type: "array"], elements}), | |||
| do: Enum.map(without_nil(elements), &(elem(transform(&1), 1))) | |||
|
|
|||
| defp transform({[type: "collection"], elements}) do | |||
There was a problem hiding this comment.
I do believe that in some cases, do: is cleaner. I'll let you decide.
lib/search.ex
Outdated
| @@ -0,0 +1,55 @@ | |||
| defmodule Braintree.Search do | |||
| @moduledoc """ | |||
| This module performs advanced search on a resource. | |||
There was a problem hiding this comment.
There shouldn't be any indentation within the moduledoc.
lib/http.ex
Outdated
| defp resolve_unprocessable_entity_error(%{"api_error_response" => api_error_response}), | ||
| do: Error.new(api_error_response) | ||
| defp resolve_unprocessable_entity_error(%{"unprocessable_entity" => _}), | ||
| do: :unprocessable_entity |
There was a problem hiding this comment.
Really this is multiple error types, it could simply be resolve_error_response.
Let's go with Error.new(%{message: "Unprocessable Entity"}) to return something consistent. Also, please use do/end here.
|
This has been merged locally (so I could squash the various fixup commits). Thanks again for all your work! |
No description provided.