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

Upcoming invoices now have proper integer-index-encoded params #475

Conversation

enugent-stripe
Copy link
Contributor

r? @brandur (But brandur already approved internally, so I'm going to go ahead and self-approve)

This ended up being a bit tricky to figure out where the problem was. It appears the upcoming invoices resource at some point took two URL params, customerID and invoiceOptions, where invoiceOptions was an object with a bunch of parameters. At some point we scrapped invoiceOptions and promoted all of its options to be optional top-level fields in the request.

So now we handle requests in one of two ways:

  1. If <=2 parameters, [customerID, {other_stuff}]: customerID and other_stuff appear directly in the URL and are index-encoded properly for array things.
  2. If >2 parameters, [customerID, one_field, other_stuff]: customerID and one_field appear directly in the URL. other_stuff is in the request body. subscription_items were not getting integer-index-encoded in the request body for requests with three parameters like: [customerID, one_field, subscription_items]. This fixes that by generalizing the existing encode method used by subscriptions.

I didn't touch the param-handling in case 1 (for backwards-compatibility, also because it works). And I guess we probably want to continue including the subscription_items in the request body in case 2 to skirt URL length limits.

@enugent-stripe enugent-stripe force-pushed the enugent-dx-1545-index-encode-invoices-for-subscriptions branch from db38fb4 to ae049ad Compare July 2, 2018 16:28
@brandur-stripe
Copy link
Contributor

Hey @enugent-stripe, we have a release script that'll automatically give you the opportunity to update the CHANGELOG, so we usually don't bother to put the individual lines in the PRs.

If you search the wiki for "releasing language bindings" you'll find a page that gives you instructions on getting an environment configured for released. I can walk you through the process a bit later too if you'd like.

@brandur-stripe
Copy link
Contributor

LGTM.

@enugent-stripe enugent-stripe merged commit ae049ad into master Jul 3, 2018
@ob-stripe ob-stripe deleted the enugent-dx-1545-index-encode-invoices-for-subscriptions branch October 29, 2019 15:10
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.

2 participants