-
Notifications
You must be signed in to change notification settings - Fork 470
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 new invoice methods and fixes to the Issuing Cardholder resource #707
Conversation
Flagging that this is a breaking change and we'll need the new API version too before we can release. |
4007edd
to
1ad0a49
Compare
8aca394
to
e751aa0
Compare
e751aa0
to
8073509
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple minor comments, but largely LGTM.
ptal @remi-stripe
@@ -47,11 +60,13 @@ type InvoiceUpcomingInvoiceItemParams struct { | |||
// For more details see https://stripe.com/docs/api#create_invoice, https://stripe.com/docs/api#update_invoice. | |||
type InvoiceParams struct { | |||
Params `form:"*"` | |||
AutoAdvance *bool `form:"auto_advance"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think auto_advance
is just a "finalize" parameter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where did you see that one? auto_advance
is the new world and has been planned for months. Just confirmed with the team internally that it's auto_advance
and finalize
does not exist as a parameter (but is a method)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Spoke with Remi offline: this seems to be a documentation problem in that auto_advance
isn't showing up in some places at the moment. Fine by me if we know it's supposed to be here.)
ApplicationFee *int64 `form:"application_fee"` | ||
Billing *string `form:"billing"` | ||
Closed *bool `form:"closed"` | ||
Customer *string `form:"customer"` | ||
DaysUntilDue *int64 `form:"days_until_due"` | ||
DefaultSource *string `form:"default_source"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I notice that DefaultSource
on invoice and subscription are marked as expandable. Should we support that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch, fixed! PTAL
Changes to the Issuing Cardholder resource - add a pending status - name is not updatable anymore so switched the test to use metadata Fix tests for Order Return as stripe-mock does not handle `items`. Add support for new methods and properties on the Invoice resource. Moved from API version 2018-09-24 to 2018-11-08
8073509
to
933e774
Compare
LGTM. |
* remove rubocop todo * Sentry error fixes * sf dev tip
This PR ships multiple breaking changes
2018-11-08
as the new API versiondefault_source
on Subscription.cc @stripe/api-libraries