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

Represent enums on method params #1679

Open
samber opened this issue Jun 19, 2023 · 3 comments
Open

Represent enums on method params #1679

samber opened this issue Jun 19, 2023 · 3 comments

Comments

@samber
Copy link

samber commented Jun 19, 2023

Is your feature request related to a problem? Please describe.

No response

Describe the solution you'd like

In a previous stripe-go version, we had the following enum:

// SubscriptionPaymentBehavior lets you control the behavior of subscription creation in case of
// a failed payment.
type SubscriptionPaymentBehavior string

// List of values that SubscriptionPaymentBehavior can take.
const (
	SubscriptionPaymentBehaviorAllowIncomplete     SubscriptionPaymentBehavior = "allow_incomplete"
	SubscriptionPaymentBehaviorErrorIfIncomplete   SubscriptionPaymentBehavior = "error_if_incomplete"
	SubscriptionPaymentBehaviorPendingIfIncomplete SubscriptionPaymentBehavior = "pending_if_incomplete"
)

First, the default_incomplete value was missing.

Second, it was not used in the Subscription type.

I wonder why it disappeared. Using an enum would prevent any malformed value.

Describe alternatives you've considered

No response

Additional context

No response

@anniel-stripe
Copy link
Contributor

anniel-stripe commented Jun 20, 2023

Hi, thanks for submitting this issue! This is a good catch - it looks like we mistakenly removed this enum in v73 (also SubscriptionProrationBehavior). Since re-introducing it is a breaking change, we'll try to bring it back with the correct values in the next major version, which should take place some time in the next couple weeks.

Update: So this is expected behavior of our code generator unfortunately. The enum you mentioned was kept around for historical purposes along with others, but we removed them to make generation consistent / less error-prone. I certainly agree that an enum would make this method easier to use, but right now our generator renders enums on method params as strings. Although we could add a one-off fix to make payment_behavior an enum, I'd prefer to fix this for all applicable method params for consistency.

@anniel-stripe
Copy link
Contributor

anniel-stripe commented Jun 26, 2023

As a follow-up to my previous response, we won't be bringing back this enum yet - representing request-side enums as string pointers was a conscious design decision we made back when this library moved to pointer-based structures for params (see this RFC for more context). Fixing SubscriptionParams.PaymentBehavior specifically doesn't make sense unless we update the type for every request-side enum, which we cannot do because we'd also have to maintain value-to-pointer helper functions for each enum type. [0]

I'm going to keep this issue open and repurpose it as a broader request to represent request-side enum params as pointers to custom enum types, if that's all right. We have a potential workaround using generics [1] to fix this for all request-side enums, but it would require dropping Go versions <1.18 before we can implement it.

[0] Because this library represents params as pointer-based structs, we had to add helper functions like stripe.String("xxxxx") to coerce real values into pointers. The issue with this is that Go didn't have an easy way at the time to do this for extra types. For example if we have PaymentBehavior *SubscriptionPaymentBehavior, we'd need a stripe.PaymentBehavior(stripe.SubscriptionPaymentBehaviorAllowIncomplete) to turn that constant/enum into a pointer - and repeat for every single custom enum type. All these helper methods would become impossible to maintain and use, so we made the trade-off to limit ourselves to basic scalar types and represent enums in request params as string pointers.

[1] The good news is Go has evolved since that decision was made - the team tinkered around with using generics which would allow us to define the helper as

func String[T ~string](v T) *T {
	return &v
}

so you can call stripe.String(stripe.SubscriptionPaymentBehaviorAllowIncomplete) without issue, since the underlying type is a string. Unfortunately, this language feature was introduced in Go 1.18, so we'll need to drop support for <1.18 first.

@anniel-stripe anniel-stripe changed the title SubscriptionParams.PaymentBehavior should be an enum Represent enums on method params Jun 26, 2023
@felix-roehrich
Copy link

Maybe it makes sense to replace stripe.String (stripe.StringValue), stripe.Int64 (stripe.Int64Value), etc. with one generic like stripe.Ref (stripe.Val). At least the naming stripe.String feels confusing to me if it would return something that is not a string.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants