-
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 /v1/topups endpoints #520
Conversation
r? @stripe/api-libraries |
topup/client_test.go
Outdated
assert.NotNil(t, topup) | ||
} | ||
|
||
func TestTopupGet(t *testing.T) { |
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.
Apologies in advance for the nit, but would you mind alphabetizing these tests (i.e., move the Get
to the top). We're still working on that in a lot of places in the library, but I think we have this decently consistent throughout tests now.
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.
No need to apologize! That's done.
Someone familiar with the new product being implemented should probably take a review pass as well, but LGTM. Thanks Jamu! |
topup.go
Outdated
CreatedRange *RangeQueryParams `form:"created"` | ||
} | ||
|
||
// TopupList is a list of charges as retrieved from a list endpoint. |
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.
Is charges
an intentional word here, vs using topups
?
Similarly for other references to charge
in the description?
Good catch, thanks! That’s copy pasta leftovers that didn’t come off in the
dishwasher. Will fix.
…On Fri, Feb 16, 2018 at 9:13 AM miguel-stripe ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In topup.go
<#520 (comment)>:
> +// returning an error for unsupported sources.
+func (p *TopupParams) SetSource(sp interface{}) error {
+ source, err := SourceParamsFor(sp)
+ p.Source = source
+ return err
+}
+
+// TopupListParams is the set of parameters that can be used when listing topups.
+// For more details see https://stripe.com/docs/api#list_topups.
+type TopupListParams struct {
+ ListParams `form:"*"`
+ Created int64 `form:"created"`
+ CreatedRange *RangeQueryParams `form:"created"`
+}
+
+// TopupList is a list of charges as retrieved from a list endpoint.
Is charges an intentional word here, vs using topups?
Similarly for other references to charge in the description?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#520 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AiNtgzIYuKfveB3pbVdhG5lfFy8tvR2yks5tVbdTgaJpZM4SHsBZ>
.
|
Let us know when this is finalized and we'll merge and cut a release. |
520634d
to
5b35dcb
Compare
r? @stripe/api-libraries are we ready to merge it? |
Yep, it's good to go. Thanks guys! |
Released as 29.1.0. |
* Adding basic data sanitizer for stripe field requirements * Sanitizer test
This add standard retrieve, create and update client support for the new
/v1/topups
endpoint.r? @apakulov-stripe @ccontinanza-stripe @chellman-stripe @kenneth-stripe @miguel-stripe