Magic Mage | Stripe Developers | Page 1

versed roverBOT Apr 4, 2023, 11:00 PM

#

severe halo Apr 4, 2023, 11:00 PM

#

@candid timber let's chat here!

candid timber Apr 4, 2023, 11:00 PM

#

Ok

#

Something seems to happen when I use these query params inside of SubscriptionService.CancelAsync()

#

perhaps it does not like those strings being empty?

severe halo Apr 4, 2023, 11:08 PM

#

could you copy/paste that subscription ID so I don't mess up any of the characters when searching on my end?

#

@candid timber just making sure you saw my message since you appear to be offline

candid timber Apr 4, 2023, 11:11 PM

#

ok

#

I am always set to invis to avoid people trying to scam on discord.

#

sub_1Mt7uCIGajkrXKj9DKCghRwm

severe halo Apr 4, 2023, 11:19 PM

#

ah, looks like an error is being returned because of the blank/empty feedback. I recommend omitting the cancellation_details hash altogether if you're not passing any values for comment and feedback

candid timber Apr 4, 2023, 11:27 PM

#

ah you can do that? I thought I could not because of the lack of an ? on the signature on the cancel options class.

#

#

So that might need to add the little ? to that details part.

severe halo Apr 4, 2023, 11:33 PM

#

Yep, it's possible. Technically, all parameters for the subscription cancellation request are optional. The only bit that's required is the subscription ID

candid timber Apr 4, 2023, 11:37 PM

#

ok

candid timber Apr 4, 2023, 11:38 PM

#

severe halo Yep, it's possible. Technically, all parameters for the subscription cancellatio...

do you happen to know if it's possible to have the ai update that file in stripe.net to add the ? to the above type?

#

I would hate to need to make the change manually 😅.

severe halo Apr 4, 2023, 11:42 PM

#

Not quite sure I follow your question.

candid timber Apr 4, 2023, 11:43 PM

#

Basically can it be made to have the bot that opens pull requests to stripe.net to update that file to make CancelationDetails get the nullable annotation ??

#

(this is because to me when I see a property not marked as that I assume it's required, this is because for 99.9% of cases it actually is)

versed roverBOT Apr 4, 2023, 11:59 PM

#

proper lotus Apr 5, 2023, 12:04 AM

#

strings are always optional so we never add the ? for it right?

#

ultimately this API Request should absolutely not 5xx

candid timber Apr 5, 2023, 12:08 AM

#

proper lotus strings are always optional so we never add the `?` for it right?

strings are a special type in .NET to where they can be:

null
empty
not empty (or has only whitespace)

However the class those are stored in is not always the same case though hence why I feel that annotating that type used there on that property as nullable is a good idea to let the developer know that it's optional.

proper lotus Apr 5, 2023, 12:09 AM

#

the point I am making is that, AFAIK, we never say anywhere ? for string parameters in stripe-dotnet

#

and likely never will

candid timber Apr 5, 2023, 12:10 AM

#

Yeah

#

I was referring to make the SubscriptionCancellationDetailsOptions property in SubscriptionCancelOptions nullable though.

proper lotus Apr 5, 2023, 12:11 AM

#

I don't disagree it'd be good. We added it for other scalar types like DateTime and decimal but we won't for strings

#

and really same, we never set ? for "hashes"

#

like https://github.com/stripe/stripe-dotnet/blob/master/src/Stripe.net/Services/Subscriptions/SubscriptionCreateOptions.cs#L61
this is optional and there's no ?, it's implicit because it's a hash

candid timber Apr 5, 2023, 12:11 AM

#

oh

#

Would this be a good change I made earlier though (since it seems the ai skipped it on stripe-dotnet currently)? https://github.com/stripe/stripe-dotnet/pull/2673

proper lotus Apr 5, 2023, 12:14 AM

#

yeah we didn't really skip anything per se. Just that file is not automatically generated

candid timber Apr 5, 2023, 12:14 AM

#

oh

proper lotus Apr 5, 2023, 12:14 AM

#

there's likely a jira from me from 3.5 years ago saying "hey we should fix this" bumped every quarter

#

😹

candid timber Apr 5, 2023, 12:15 AM

#

Well at least I did that task for you today 😅.

proper lotus Apr 5, 2023, 12:15 AM

#

Files automatically generated by our "AI" have

``` at the top

#

But really going to go and yell a bit, the fact that you hit this and had to do the PR yourself is a bit "ugh" :p

candid timber Apr 5, 2023, 12:15 AM

#

I wonder why that file is not auto generated.

candid timber Apr 5, 2023, 12:16 AM

#

proper lotus Files automatically generated by our "AI" have ```// File generated from our Ope...

it's ok though, at least it was noticed sooner rather than later.

#

I actually wonder though who does the reviews and merges though, and if the pr I opened can be ported to the beta as well (or if it should just start there) 😅.

proper lotus Apr 5, 2023, 12:19 AM

#

humans review and merge right now though we mostly trust the code generated (it's reviewed earlier in our process internally as part of real code changes to our API)

#

and your change is about a public feature so it definitely should go in the public library

candid timber Apr 5, 2023, 12:22 AM

#

ok

#

So basically it will be in a new nuget.org release soon?

proper lotus Apr 5, 2023, 12:24 AM

#

yes, when a human has reviewed and then merged/released. Might take a couple of days

candid timber Apr 5, 2023, 12:24 AM

#

ok

#Magic Mage