Add payment_settings to invoice #1247
Conversation
invoice.go
Outdated
| // redirected to. | ||
| type InvoicePaymentSettingsPaymentMethodOptionsBancontactPreferredLanguage string | ||
|
|
||
| // List of values that InvoicePaymentSettingsPaymentMethodOptionsBancontactPreferredLanguage can take. |
There was a problem hiding this comment.
Not sure about this -- the general rule is "define enums for resources, but not for params" but we don't seem to be explicit about "language" enums anywhere else.
There was a problem hiding this comment.
Yeah I'd recommend punting on constants for languages today. I don't know who we'll handle once we codegen but until then keeping those as strings is easier (because there could be hundreds of constants otherwise, it's like state or country)
There was a problem hiding this comment.
Hmm, IMO we should probably just make it a string because (1) it's documented as string as opposed to enum in the API ref, (2) this could potentially have a lot of different values, (3) if the product team added a new value without the library I'm not sure that we'd be able to handle it that well, and (4) it looks like it's a string on payment intent, setup attempt, and charge.
Thoughts?
| InvoicePaymentSettingsPaymentMethodTypeGiropay InvoicePaymentSettingsPaymentMethodType = "giropay" | ||
| InvoicePaymentSettingsPaymentMethodTypeIdeal InvoicePaymentSettingsPaymentMethodType = "ideal" | ||
| InvoicePaymentSettingsPaymentMethodTypeSepaCreditTransfer InvoicePaymentSettingsPaymentMethodType = "sepa_credit_transfer" | ||
| InvoicePaymentSettingsPaymentMethodTypeSepaDebit InvoicePaymentSettingsPaymentMethodType = "sepa_debit" |
There was a problem hiding this comment.
It's SEPA in account.go, but Sepa in charge.go and payment_intent.go I think. I figure it's better to be consistent with the "payment" resources than account.
There was a problem hiding this comment.
That's fair, though that will increase the number of breaking changes once we codegen and fix this right? tbh I have no idea how we'll handle codegen-ing specific acronyms having to be in upper case 😓
There was a problem hiding this comment.
Arg lol. Yeah, that seems okay.
|
|
||
| // List of values that InvoicePaymentSettingsPaymentMethodType can take. | ||
| const ( | ||
| InvoicePaymentSettingsPaymentMethodTypeAchCreditTransfer InvoicePaymentSettingsPaymentMethodType = "ach_credit_transfer" |
There was a problem hiding this comment.
Should be ACH, but it's Ach in other places and ACH does not exist anywhere.
invoice.go
Outdated
| // redirected to. | ||
| type InvoicePaymentSettingsPaymentMethodOptionsBancontactPreferredLanguage string | ||
|
|
||
| // List of values that InvoicePaymentSettingsPaymentMethodOptionsBancontactPreferredLanguage can take. |
There was a problem hiding this comment.
Yeah I'd recommend punting on constants for languages today. I don't know who we'll handle once we codegen but until then keeping those as strings is easier (because there could be hundreds of constants otherwise, it's like state or country)
| InvoicePaymentSettingsPaymentMethodTypeGiropay InvoicePaymentSettingsPaymentMethodType = "giropay" | ||
| InvoicePaymentSettingsPaymentMethodTypeIdeal InvoicePaymentSettingsPaymentMethodType = "ideal" | ||
| InvoicePaymentSettingsPaymentMethodTypeSepaCreditTransfer InvoicePaymentSettingsPaymentMethodType = "sepa_credit_transfer" | ||
| InvoicePaymentSettingsPaymentMethodTypeSepaDebit InvoicePaymentSettingsPaymentMethodType = "sepa_debit" |
There was a problem hiding this comment.
That's fair, though that will increase the number of breaking changes once we codegen and fix this right? tbh I have no idea how we'll handle codegen-ing specific acronyms having to be in upper case 😓
invoice.go
Outdated
| PreferredLanguage *string `form:"preferred_language"` | ||
| } | ||
|
|
||
| // InvoicePaymentSettingsParams is the set of parameters allowed for card on payment_method_options on |
There was a problem hiding this comment.
Comment references the wrong type
There was a problem hiding this comment.
This is a bit nitpicky too, but I'd probably leave out things like payment_method_options just because the user will never directly interact with those API parameter names (i.e. they use Go symbols like PaymentMethodOptions instead).
I usually just keep the docs on these structs as simple as plausibly possible like "is the set of parameters for invoice payment settings". Not super useful I know, but pretty much no documentation will be unless we go for all-the-way API ref style docs (which might be possible with codegen).
invoice.go
Outdated
| RequestThreeDSecure *string `form:"request_three_d_secure"` | ||
| } | ||
|
|
||
| // InvoicePaymentSettingsParams is the set of parameters allowed for the payment_method_options on ther |
There was a problem hiding this comment.
Comment references the wrong type
| Card *InvoicePaymentSettingsPaymentMethodOptionsCardParams `form:"card"` | ||
| } | ||
|
|
||
| // InvoicePaymentSettingsParams is the set of parameters allowed for the payment_settings on an invoice. |
There was a problem hiding this comment.
Comment references the wrong type
There was a problem hiding this comment.
Seems right to me
|
A couple minor comments (and from Remi as well), but looks fine to me! ptal @richardm-stripe |
|
r? @brandur-stripe
|
r? @brandur-stripe
cc @stripe/api-libraries
Adds support for
payment_settingson theInvoiceresource, from a recent API Release. (Skipping the "capability" part of that release for now, until that feature is fully released)