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

codegen: 14 more files #1312

Merged
merged 10 commits into from Jul 12, 2021
Merged

codegen: 14 more files #1312

merged 10 commits into from Jul 12, 2021

Conversation

richardm-stripe
Copy link
Contributor

@richardm-stripe richardm-stripe commented Jul 12, 2021

Notify

r? @stripe/api-library-reviewers

Summary

Makes 14 more files eligible for codegen. This includes adding a number of missing fields (see below).

The most involved thing here is the type alias LineItemItem introduced in checkout/session/client.go, the type alias ensures that this will not be a breaking change.

There is a go vet error that I will fix in a follow-up, please don't block on that.

Changelog

  • Add support for BillingAddressCollection to CheckoutSession
  • Add support for NetworkReasonCode to DisputeReason
  • Add support for Object to EphemeralKey, ApplicationFee, and DisputeReason
  • Add support for Description to Refund
  • Add const definition for value blocked on enum IssuingCardholderStatus
  • Bugfix: add support for Rate on CheckoutSessionTotalDetailsBreakdownTax -- the existing field TaxRate has the wrong json annotation and should be deprecated.

func (i *Iter) CheckoutSessionList() *stripe.CheckoutSessionList {
return i.List().(*stripe.CheckoutSessionList)
}
type LineItemIter lineitem.Iter
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this make it a breaking change on existing integrations?
Asking as there's an Iter in lineitem.go and some broken tests at the bottom

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, this is breaking as is -- we need to do type LineItemIter = lineitem.Iter to make it a transparent type alias and that will fix the test. I thought I had written the correct override for codegen to do this, need to debug.

@@ -429,7 +444,8 @@ type CheckoutSessionTotalDetailsBreakdownDiscount struct {
// CheckoutSessionTotalDetailsBreakdownTax represent the details of tax rate applied to a session.
type CheckoutSessionTotalDetailsBreakdownTax struct {
Amount int64 `json:"amount"`
TaxRate *TaxRate `json:"tax_rate"`
Rate *TaxRate `json:"rate"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we have Rate appear here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah great question, need to call this out in the changelog and should add a comment. This is a bugfix.

The json field is tax_rate, not rate, so TaxRate will never be populated and will always be nil.

https://github.com/stripe/stripe-node/blob/master/types/2020-08-27/Checkout/Sessions.d.ts#L724

Copy link
Contributor

@dcr-stripe dcr-stripe left a comment

Choose a reason for hiding this comment

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

LGTM overall - small nits.

@@ -99,18 +103,13 @@ type Dispute struct {
IsChargeRefundable bool `json:"is_charge_refundable"`
Livemode bool `json:"livemode"`
Metadata map[string]string `json:"metadata"`
NetworkReasonCode string `json:"network_reason_code"`
Object string `json:"object"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Missing the support for this in the changelog.

func Update(id string, params *stripe.DisputeParams) (*stripe.Dispute, error) {
return getC().Update(id, params)
}

// Update updates a dispute.
// Update updates a dispute's properties.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Double "update".

@richardm-stripe richardm-stripe merged commit a4b1efb into master Jul 12, 2021
@richardm-stripe richardm-stripe deleted the richardm-go-codegen-1 branch July 12, 2021 22:03
@dcr-stripe dcr-stripe mentioned this pull request Jul 14, 2021
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

3 participants