Rename all properties in the library to match the API reference

[remi-stripe]

This should be the final PR to rework most of the library to:

• match the naming in the API
• split out resource and params structures
• remove magic such as AmountZero or CouponEmpty and move all parameters to pointers

We can't list all the changes one by one as those are extensive but here are the ones likely to impact most integrations:

• All properties with the word url in it now have the same case such as BusinessURL.
• All properties with the word four in it now use a digit such as DynamicLast4 or SSNLast4.
• Renamed confusing properties such as Account on BankAccount becoming AccountNumber to avoid confusing with its Connect counterpart which is now named Account instead of AccountID.
• Properties such as MonthAnchor or constants such as Month are now renamed to match the corresponding parameter/value in the API MonthlyAnchor and Monthly.
• Meta and Live have been renamed to Metadata and Livemode on all classes
• Values on List objects has been renamed to Data
• Some classes have been renamed.
  • Owner is now AdditionalOwner,
  • Transaction is now BalanceTransaction with all constants and corresponding classes renamed to match this
  • references to Tx were also replaced by BalanceTransaction in classes, properties and constants
  • Fee and FeeRefund have been renamed to ApplicationFee and ApplicationFeeRefund along with all related classes, properties and constants.
  • Sub has been renamed to Subscription along with all related classes, properties and constants.
• Card parameters have been renamed such as City becoming AddressCity and Month becoming ExpMonth.
• Errors codes have been prefixed with ErrorCode such as ErrorCodeCardDeclined or ErrorCodeIncorrectZip.
• Event properties are now named Object and PreviousAttributes and methods GetObjectValue() and GetPreviousValue().
• Pagination properties have been renamed to StartingAfter, EndingBefore and HasMore.
• AddMeta() has been renamed to AddMetadata(), Expand() to AddExpand()

This replaces #459 and #507

[remi-stripe]

Flagging that I seem to not be able to upload with FileReader if I don't explicitly set Filename too. Not sure if expected but I don't want to forget

[brandur-stripe]

Hm, that shouldn't be the case, but we should look at that more closely.

[remi-stripe]

Should this be a pointer to the array?

[remi-stripe]

Should this be a pointer?

[brandur-stripe]

Haha, holy cow this is a monster! Amazing work Remi!

Just to be explicit about it: is this ready for review?

[remi-stripe]

@brandur-stripe I think it is. The only todo left is to move/rename all constants to the upper package. This is less likely to break something and I will do it as one unique commit separately hopefully this week but we should not block on it.

Let me know if I can help with the review in any way. The main gotcha I know of would be for parameters passed in the URL to be incorrectly sent as pointers address instead of the value. I haven't figured out a way to ensure this does not happen without just carefully reading the code though.

[brandur-stripe]

Let me know if I can help with the review in any way. The main gotcha I know of would be for parameters passed in the URL to be incorrectly sent as pointers address instead of the value. I haven't figured out a way to ensure this does not happen without just carefully reading the code though.

We should write an "encode parameters" function that takes a splat and checks each value probably. I think we can push that to a different PR though. Maybe one that I could do for a change :)

[remi-stripe]

Ah yes we talked about this last week but that seemed like a lot of work and I was worried I might make things worse mixing this here. Do you think we should do this before merging? It does seem safer for sure.

If you were to start the PR/build the method/examples, I can happily port all the clients to it afterwards

[brandur-stripe]

Remi this is is just totally nuts! Alright, I left a few comments throughout the PR here. Most of them fall into three buckets:

1. Eliminating the type aliases like type Current string like we talked about in Slack. We should probably do this along with the big breaking change, but now that I look here, I see what you mean that there's a lot of them.
2. In the response objects, some embedded structs are pointers (*AdditionalOwner) and some are just structs (AdditionalOwner). I think we should skew all the way in one direction by just making them all pointers.
3. There's still quite a few constants embedded at the package level (e.g. in sub/client.go as opposed to at the top-level stripe package). See comments throughout, but we should probably hoist all of these up.

A few omissions with regards to Values not being renamed to Data in lists:

1. TopupList
2. SourceTransactionList
3. ExchangeRateList

A few omissions with regards to uint64 not being converted to int64:

1. All fields in the DOB struct.
2. Limit under ListParams.
3. SubmissionCount under EvidenceDetails.
4. Size under File (in dispute.go).

Very excited to see this land! Thanks for all the hard work.

[brandur-stripe]

Should we used BusinessVATID here (VAT = value added tax)? Admittedly, this is a pretty awkward name.

[brandur-stripe]

I notice that in other files, embedded structs tend to be points (e.g. []AdditionalOwner). Should we do that for all the fields in LegalEntity?

[brandur-stripe]

Where did we land on the type aliasing? Should we just remove Gender in favor of string?

[brandur-stripe]

Just flagging that the question around whether embedded structs should be pointers or not may apply to other structs in this file too.

[brandur-stripe]

Just flagging another type alias.

[brandur-stripe]

Maybe change this to "CVC".

[brandur-stripe]

And same here: "CVC".

[brandur-stripe]

Hm, that shouldn't be the case, but we should look at that more closely.

[brandur-stripe]

Make this a point for consistency.

[brandur-stripe]

Maybe hoist these up to stripe as well.

[remi-stripe]

@brandur-stripe Thank you so much for the thorough review. I just fixed everything you raised that was an omission!

Edit: Okay I had broken the build so I fixed it quickly. There's a small one that annoys me a bit which is HTTPStatusCode here. If we turn it in int64 you apparently can't do this:

	if res.StatusCode >= 400 {
		return s.ResponseToError(res, resBody)
	}

I can definitely do if res.StatusCode >= int64(400) but wanted to confirm with you it was the right approach

[remi-stripe]

@brandur-stripe Okay I just started removing the "fake" types that we alias to string in the Account resource. I had to change related resource to use the new way or have their own constants. For example, even though the values are the same for bank account holder types and legal entity, they definitely should be different constants.

Before I go ahead and change the entire library again, can you confirm that this is the right approach and looks cleaner? Also, what do you think about not adding comments for those? That seems a bit much and most names are self explanatory.

[remi-stripe]

Once we've done all constants, I'll then move to passing each embedded struct as a pointer too.

[brandur-stripe]

Edit: Okay I had broken the build so I fixed it quickly. There's a small one that annoys me a bit which is HTTPStatusCode here. If we turn it in int64 you apparently can't do this:

Hah, yeah I thought about calling out status code too, but decided not to. If you look throughout the [net/http](https://godoc.org/net/http#StatusText) package in the standard library, it's an int everywhere, and I think that we should keep it as is just to be consistent with that. It would be a bad interface to have to convert to int when comparing to any codes from there.

Before I go ahead and change the entire library again, can you confirm that this is the right approach and looks cleaner? Also, what do you think about not adding comments for those? That seems a bit much and most names are self explanatory.

Yep, that looks right. And +1 removing the comments.

Once we've done all constants, I'll then move to passing each embedded struct as a pointer too.

Sounds good! Thanks Remi.

[remi-stripe]

@brandur-stripe My understanding is that we want to remove Currency entirely. Do we also want to start passing "usd" which is likely what most people do or do we want to add CurrencyUSD and CurrencyEUR, etc.?

[brandur-stripe]

@brandur-stripe My understanding is that we want to remove Currency entirely. Do we also want to start passing "usd" which is likely what most people do or do we want to add CurrencyUSD and CurrencyEUR, etc.?

Yeah, good question.

I think that since we sort of already have the list of constants, we should probably keep them around (but make them strings, so that the interface is compatible with just passing "usd" if you want) — there's a large-ish class of developer (myself included) who like to use constants when they're available just for good hygiene.

The counterargument is that some of these lists can be prone to becoming outdated (as new currencies are added and such), but my intuition would be that currencies (or more specifically, supported currencies) should be stable enough that isn't isn't likely to be a huge problem.

[remi-stripe]

Did another big rebase for all changes in master since last attempt in April.

[brandur-stripe]

@remi-stripe Amazing! Sorry again for the snail's pace here :$ Let me know when all your changes are ready.

[remi-stripe]

@brandur-stripe I need to undo the last 2 commits first and redo that work by moving all constants to the top level + renaming the types/names but not using them on parameters. Hopefully this can be done in a couple hours, but will ping you once ready.

[remi-stripe]

@brandur-stripe Okay I think we're ~almost there. I'm not sure it's the final version yet but I would like some quick 👍 or 👎 on all the last few changes:

• Stripe types for most enum-like string properties and string for all parameters.
• Removal of the currency package and whether we want currency.go as a top-level file or if it should go in stripe.go

Also whether we want to also address one of your earlier comments in this PR too:

In the response objects, some embedded structs are pointers (*AdditionalOwner) and some are just structs (AdditionalOwner). I think we should skew all the way in one direction by just making them all pointers.

[brandur-stripe]

Nice @remi-stripe! See below:

Stripe types for most enum-like string properties and string for all parameters.

Did you change anything already? I think it'd be nice to have types where possible, but I don't think it matters that much. I'd suggest leaving the ones that exist in place, and probably just not adding anything new for now.

Removal of the currency package and whether we want currency.go as a top-level file or if it should go in stripe.go

I'd just put it in top-level currency.go. We want to kill the package, but it's fine to keep it organized in its own file.

Also whether we want to also address one of your earlier comments in this PR too:

Hm, did you get a feeling as to how much of a pain this is to fix? I still think this would be somewhat nice to have just so it's predictable (i.e., things area always pointers) when you're writing your integration code.

[remi-stripe]

Did you change anything already? I think it'd be nice to have types where possible, but I don't think it matters that much. I'd suggest leaving the ones that exist in place, and probably just not adding anything new for now.

Yes I did all the changes we agreed upon yesterday for all the packages. This means all params structs have *string but all resources have custom types for each property that is really an enum. This is done in this specific commit: 35258f8

This means that I undid all the changes I did last month and re-added each type (and manually added a few more that were missing) and made sure that all constants are now top-level and properly prefixed.

Can you confirm this is what we expected?

I'd just put it in top-level currency.go. We want to kill the package, but it's fine to keep it organized in its own file.

That's what I did in ffbc8bd can you confirm that was correct?

Hm, did you get a feeling as to how much of a pain this is to fix? I still think this would be somewhat nice to have just so it's predictable (i.e., things area always pointers) when you're writing your integration code.

I'm hopeful this is mostly a matter of reading each struct and replacing with a *Type instead of Type so it should be doable.

[brandur-stripe]

Yes I did all the changes we agreed upon yesterday for all the packages. This means all params structs have *string but all resources have custom types for each property that is really an enum. This is done in this specific commit: 35258f8

This means that I undid all the changes I did last month and re-added each type (and manually added a few more that were missing) and made sure that all constants are now top-level and properly prefixed.

Can you confirm this is what we expected?

Oh man, you are a machine! Yes — this is what I was thinking.

That's what I did in ffbc8bd can you confirm that was correct?

Yep, this also looks good to me.

I'm hopeful this is mostly a matter of reading each struct and replacing with a *Type instead of Type so it should be doable.

Okay then, yeah I think we may as well go for it given the number of other changes that we already have in here. Feel free to push back though if this will push you over the edge, haha.

[remi-stripe]

@brandur-stripe just rebased on the latest master.

[brandur-stripe]

HERE WE GO!

[ob-stripe]

[brandur-stripe]

LOL! Yep.

Released as 32.0.0!

[hiren-bacancy]

Hi Team, Can anybody help me to find the missing Key struct from the Account struct. I was using v13.6.0 and now I have updated it to the v70.15.0. But what I found is, it is missing Key structure which consists Secret and Publish fields till version 31.0.0.

Let me know if you need any additional data.
Thank you.

v13.6.0

type Account struct {
	ID                   string               `json:"id"`
	ChargesEnabled       bool                 `json:"charges_enabled"`
	Country              string               `json:"country"`
	DefaultCurrency      string               `json:"default_currency"`
	DetailsSubmitted     bool                 `json:"details_submitted"`
	TransfersEnabled     bool                 `json:"transfers_enabled"`
	Name                 string               `json:"display_name"`
	Email                string               `json:"email"`
	ExternalAccounts     *ExternalAccountList `json:"external_accounts"`
	Statement            string               `json:"statement_descriptor"`
	Timezone             string               `json:"timezone"`
	BusinessName         string               `json:"business_name"`
	BusinessPrimaryColor string               `json:"business_primary_color"`
	BusinessUrl          string               `json:"business_url"`
	SupportPhone         string               `json:"support_phone"`
	SupportEmail         string               `json:"support_email"`
	SupportUrl           string               `json:"support_url"`
	ProductDesc          string               `json:"product_description"`
	Managed              bool                 `json:"managed"`
	DebitNegativeBal     bool                 `json:"debit_negative_balances"`
	Keys                 *struct {
		Secret  string `json:"secret"`
		Publish string `json:"publishable"`
	} `json:"keys"`
	Verification *struct {
		Fields         []string `json:"fields_needed"`
		Due            *int64   `json:"due_by"`
		DisabledReason string   `json:"disabled_reason"`
	} `json:"verification"`
	LegalEntity      *LegalEntity      `json:"legal_entity"`
	TransferSchedule *TransferSchedule `json:"transfer_schedule"`
	TOSAcceptance    *struct {
		Date      int64  `json:"date"`
		IP        string `json:"ip"`
		UserAgent string `json:"user_agent"`
	} `json:"tos_acceptance"`
	SupportAddress *Address `json:"support_address"`
	Deleted        bool     `json:"deleted"`
}


v70.15.0

type Account struct {
	BusinessProfile  *AccountBusinessProfile `json:"business_profile"`
	BusinessType     AccountBusinessType     `json:"business_type"`
	Capabilities     *AccountCapabilities    `json:"capabilities"`
	ChargesEnabled   bool                    `json:"charges_enabled"`
	Company          *AccountCompany         `json:"company"`
	Country          string                  `json:"country"`
	Created          int64                   `json:"created"`
	DefaultCurrency  Currency                `json:"default_currency"`
	Deleted          bool                    `json:"deleted"`
	DetailsSubmitted bool                    `json:"details_submitted"`
	Email            string                  `json:"email"`
	ExternalAccounts *ExternalAccountList    `json:"external_accounts"`
	ID               string                  `json:"id"`
	Individual       *Person                 `json:"individual"`
	Metadata         map[string]string       `json:"metadata"`
	Object           string                  `json:"object"`
	PayoutsEnabled   bool                    `json:"payouts_enabled"`
	Requirements     *AccountRequirements    `json:"requirements"`
	Settings         *AccountSettings        `json:"settings"`
	TOSAcceptance    *AccountTOSAcceptance   `json:"tos_acceptance"`
	Type             AccountType             `json:"type"`
}

[remi-stripe]

@hiren-bacancy Please work with our support team for help on this instead moving forward, this is a close to 4 years old PR.

To unblock you quickly the keys property was removed back in 2019 as documented here

The keys property on Account creation has been removed. Platforms should now authenticate as their connected accounts with their own key via the Stripe-Account header.

Our support team can be contacted here https://support.stripe.com/contact for follow up questions