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

List methods with parameters are broken #56

Closed
wezm opened this issue Dec 27, 2022 · 1 comment · Fixed by #58
Closed

List methods with parameters are broken #56

wezm opened this issue Dec 27, 2022 · 1 comment · Fixed by #58

Comments

@wezm
Copy link

wezm commented Dec 27, 2022

I think #40 broke list methods. The code generated prior to this change looked like this (for Price in this case):

builder.add("active", active) unless active.nil?
builder.add("product", product) unless product.nil?
⋮
builder.add("ending_before", ending_before) unless ending_before.nil?

after the changes in #40 the generated code looks like this:

def self.list(active : Bool | ::Nil = nil, product : String | ::Nil = nil, currency : String | ::Nil = nil, type : String | ::Nil = nil, lookup_keys : Array(String) | ::Nil = nil, recurring : Hash(String, String | Int32 | Nil) | ::Nil = nil, limit : Int32 | ::Nil = nil, created : Hash(String, Int32) | ::Nil = nil, starting_after : String | ::Nil = nil, ending_before : String | ::Nil = nil, expand : Array(String) | ::Nil = nil) : List(Stripe::Price)
  io = IO::Memory.new
  builder = ParamsBuilder.new(io)

  builder.add(active, active) unless active.nil?
  builder.add(product, product) unless product.nil?
  ⋮
  builder.add(ending_before, ending_before) unless ending_before.nil?

  response = Stripe.client.get("/v1/#{"price"}s", form: io.to_s)

  if response.status_code == 200
    List(Stripe::Price).from_json(response.body)
  else
    raise Error.from_json(response.body, "error")
  end
end

Note that the keys are no longer quoted. This results in parameters like this being generated:

"%5B%22price_1MJ8JLGRhOlPd2IWrLbjKJgs%22%5D%5B0%5D=price_1MJ8JLGRhOlPd2IWrLbjKJgs&%5B%22data.product%22%5D%5B0%5D=data.product"

which URL decoded is:

["price_1MJ8JLGRhOlPd2IWrLbjKJgs"][0]=price_1MJ8JLGRhOlPd2IWrLbjKJgs&["data.product"][0]=data.product

note how the value is being used as the key and value.

The following test added to price_spec.cr illustrates the problem:

  it "listing prices with params" do
    WebMock.stub(:get, "https://api.stripe.com/v1/prices")
      .with(body: "currency=AUD")
      .to_return(status: 200, body: File.read("spec/support/list_prices.json"), headers: {"Content-Type" => "application/json"})

    prices = Stripe::Price.list(currency: "AUD")
    prices.first.id.should eq("price_1IqjDxJN5FrkuvKhKExUK1B2")
  end

With resulting error:

       Real HTTP connections are disabled. Unregistered request: GET https://api.stripe.com/v1/prices with body "AUD=AUD" with headers {"Content-Length" => "7", "Content-Type" => "application/x-www-form-urlencoded", "Host" => "api.stripe.com", "Authorization" => "Bearer test"}

Note that the body has the value twice.

The solution appears to be a call to stringify in the add_list_method macro, which makes the above test pass:

  {% for x in arguments.map &.var.id %}
    builder.add({{x.stringify}}, {{x.id}}) unless {{x.id}}.nil?
  {% end %}
@confact
Copy link
Owner

confact commented Jan 8, 2023

Thanks @wezm, added test and verified it was a bug. Fixed it in #58.

wezm added a commit to wezm/stripe.cr that referenced this issue Jan 31, 2023
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 a pull request may close this issue.

2 participants