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 support for simple arrays and objects in resource generation #988

Merged
merged 8 commits into from
Oct 21, 2022

Conversation

tomelm
Copy link
Contributor

@tomelm tomelm commented Oct 15, 2022

Reviewers

r? @etsai-stripe @mnorth-stripe
cc @stripe/developer-products

Summary

In this PR, I added:

  1. support for simple array* parameters as CLI flags
  2. support for nested objects* parameters as CLI flags through dot notation
  3. simple types on the --help flag for resource operations.

Simple array parameters (that is, an array of primitive types like strings or ints) are easy to support in the CLI. In the PR, to add data as an array the user will submit the same flag multiple times:

$ ./stripe payment_intents create \
  --payment-method-types=oxxo \ 
  --payment-method-types=card \
  --amount=200 \
  --currency=usd

This results in the following POST body:

{
  "payment_method_types": [
    "oxxo",
    "card"
  ],
  "currency": "usd",
  "amount": "200"
}

Next, added flags for nested objects through dot notation. Dots look like they have the most compatibility across shells (brackets unfortunately just don't work). This command works:

$ ./stripe payment_intents create \
  --payment-method-types=card \
  --amount=200 \
  --currency=usd \
  --shipping.address.country=us \
  --shipping.name=foo

Which results in the following body:

{
  "payment_method_types": [
    "card"
  ],
  "currency": "usd",
  "amount": "200",
  "shipping": {
    "address": {
      "country": "us"
    },
    "name": "foo"
  }
}

I also re-enabled a little bit of the --help text on generated resources so that people had a better sense for what are arrays or not:

$ ./stripe payment_intents create --help
Usage:
  stripe payment_intents create [--param=value] [-d "nested[param]=value"]

Request Parameters:
      --amount string
      --application-fee-amount string
      --automatic-payment-methods.enabled string
      --capture-method string
      --confirm string
      --confirmation-method string
      --currency string
      --customer string
      --description string
      --error-on-requires-action string
      --mandate string
      --mandate-data.customer-acceptance.accepted-at string
      --mandate-data.customer-acceptance.online.ip-address string
      --mandate-data.customer-acceptance.online.user-agent string
      --mandate-data.customer-acceptance.type string
      --off-session string
      --on-behalf-of string
      --payment-method string
      --payment-method-data.acss-debit.account-number string
      --payment-method-data.acss-debit.institution-number string
      --payment-method-data.acss-debit.transit-number string
      --payment-method-data.au-becs-debit.account-number string
      --payment-method-data.au-becs-debit.bsb-number string
      --payment-method-data.bacs-debit.account-number string
      --payment-method-data.bacs-debit.sort-code string
      --payment-method-data.billing-details.email string
      --payment-method-data.billing-details.name string
      --payment-method-data.billing-details.phone string
      --payment-method-data.boleto.tax-id string
      --payment-method-data.eps.bank string
      --payment-method-data.fpx.account-holder-type string
      --payment-method-data.fpx.bank string
      --payment-method-data.ideal.bank string
      --payment-method-data.klarna.dob.day string
      --payment-method-data.klarna.dob.month string
      --payment-method-data.klarna.dob.year string
      --payment-method-data.p24.bank string
      --payment-method-data.radar-options.session string
      --payment-method-data.sepa-debit.iban string
      --payment-method-data.sofort.country string
      --payment-method-data.type string
      --payment-method-data.us-bank-account.account-holder-type string
      --payment-method-data.us-bank-account.account-number string
      --payment-method-data.us-bank-account.account-type string
      --payment-method-data.us-bank-account.financial-connections-account string
      --payment-method-data.us-bank-account.routing-number string
      --payment-method-types stringArray
      --radar-options.session string
      --receipt-email string
      --return-url string
      --setup-future-usage string
      --shipping.address.city string
      --shipping.address.country string
      --shipping.address.line1 string
      --shipping.address.line2 string
      --shipping.address.postal-code string
      --shipping.address.state string
      --shipping.carrier string
      --shipping.name string
      --shipping.phone string
      --shipping.tracking-number string
      --statement-descriptor string
      --statement-descriptor-suffix string
      --transfer-data.amount string
      --transfer-data.destination string
      --transfer-group string
      --use-stripe-sdk string

Note: all types are specifically for the Stripe CLI itself, not the Stripe API. The CLI handles
transforming types to what the API expects.

Few things to note on the --help:

  1. The list is now quite long. The best way to do this seems to be to treat nested objects as graphs and fully expand the graph to the leaf node and treat that as a parameter. The best option here would be to do similar as the API ref docs and split the params list into the most common params and other params (see https://stripe.com/docs/api/payment_intents/create).
  2. Everything is a string or stringArray. This is because all inputs in the terminal are strings (and technically since we use form data, all requests to Stripe are strings). I added the caveat at the very end as a small clarification
  3. I tried to add required and description but because of how we pass the Open API data into the resource generation right now this is unfortunately a bit more complicated and may require a little more rewiring. This'll be for another time.

* The one part that is still not implemented here is for an array of objects. Because brackets don't work and the flag data is inherently unordered (or at least, order is not guaranteed) there's not a good way to run flags on arrays of objects and make sure the right indexes are mapped.

For the dot notation, I tested this on a few shells and OS's and looks like it works generally okay (but not Windows, I don't have access to that at the moment). We're explicitly not following POSIX guidance which we tried to do for a while to maintain maximum compatibility but adoption of WSL (especially WSL2) should make this more bearable for Windows users (whereas previously, Powershell caused problems here).

@tomelm tomelm requested a review from a team as a code owner October 15, 2022 22:22
pkg/cmd/templates.go Outdated Show resolved Hide resolved
Copy link

@mike-north mike-north left a comment

Choose a reason for hiding this comment

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

This looks great! Still need to review resource_utils.go

pkg/cmd/resource/operation.go Show resolved Hide resolved
pkg/cmd/resources_cmds.go Show resolved Hide resolved
pkg/cmd/templates.go Outdated Show resolved Hide resolved
Copy link
Contributor

@bernerd-stripe bernerd-stripe left a comment

Choose a reason for hiding this comment

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

Fantastic! The translation of simple array arguments looks particularly smooth.

pkg/cmd/templates.go Outdated Show resolved Hide resolved
Copy link

@mnorth-stripe mnorth-stripe left a comment

Choose a reason for hiding this comment

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

This is great! Thanks for working on this

@tomer-stripe tomer-stripe merged commit 2b00f2f into stripe:master Oct 21, 2022
@tomelm tomelm deleted the tomer/improve-resource-commands branch October 22, 2022 02:37
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

6 participants