-
Notifications
You must be signed in to change notification settings - Fork 460
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 listing source transactions #488
Conversation
d469f2b
to
fdc6346
Compare
@ob-stripe Can you double check the test code? Even if it's not run, it's still compiled, and I think the new test has a couple problems:
|
sourcetransaction.go
Outdated
Live bool `json:"livemode"` | ||
Source string `json:"source"` | ||
Type string `json:"type"` | ||
TypeData map[string]string `form:"-"` |
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.
We shouldn't be mixing json
and form
annotations (this might have been a bad copy + paste). It also looks like you'll have to run a gofmt
on this.
sourcetransaction/client.go
Outdated
} | ||
|
||
return &Iter{stripe.GetIter(lp, body, func(b *form.Values) ([]interface{}, stripe.ListMeta, error) { | ||
list := &stripe.SourceTransactionList{} |
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.
It's probably not convention elsewhere, but it might be worth failing fast here by returning an error if params.Source
wasn't set. Currently I believe it'll fail with kind of a confusing couldn't find URL /sources//source_transactions
404 (or something of the sort).
@ob-stripe Left a couple comments above, but thanks! |
7cf85cd
to
d3636cc
Compare
d3636cc
to
9365d16
Compare
Not sure how I managed to commit a file in such a borked state, but it should be all fixed now. I also added a check for |
Happens to the best of us :) Thanks! Release cut as 28.5.0. |
r? @brandur-stripe
cc @stripe/api-libraries @stan-stripe
Adds support for the
/v1/sources/src_.../source_transactions
endpoint.I wrote a test, but as stripe-mock doesn't support this endpoint yet the test is skipped for the time being. Not great, but it doesn't seem like we have an easy way to stubbing requests in stripe-go tests.