Use an interface for PaymentIntentSourceAction #1329
Conversation
| else | ||
| { | ||
| sourceAction.Type = PaymentIntentSourceActionType.Unknown; | ||
| sourceAction.Value = Mapper<PaymentIntentSourceActionAuthorizeWithUrl>.MapFromJson(incoming.SelectToken("value")?.ToString()); |
There was a problem hiding this comment.
OOC can't you do the same logic as the other one where you map based on type but use value for the deserialization? Or is it just that we have one example so it's cleaner this way for now?
There was a problem hiding this comment.
Yeah, I think if we wanted to have an abstract version of this converter, we'd need to add 3 additional interfaces (for the type attribute, the value attribute, and the encompassing class). Seems overkill for now.
I did rework the converter to make it more similar to the existing ones for objects though.
| var sourceAction = new PaymentIntentSourceAction() | ||
| { | ||
| Type = PaymentIntentSourceActionType.Unknown, | ||
| Value = default(IPaymentIntentSourceActionValue), |
There was a problem hiding this comment.
What does default mean here? Shouldn't Value be null?
There was a problem hiding this comment.
default would return null here, but in any case this was unnecessary and I removed it.
| Assert.Equal("https://stripe.com", intent.NextSourceAction.AuthorizeWithUrl.Url); | ||
| var authorizeWithUrl = intent.NextSourceAction.Value as PaymentIntentSourceActionAuthorizeWithUrl; | ||
| Assert.NotNull(authorizeWithUrl); | ||
| Assert.Equal("https://stripe.com", authorizeWithUrl.Url); |
There was a problem hiding this comment.
Should we test with None?
There was a problem hiding this comment.
Yep! Added two more tests, one where next_source_action is null and one where it's a type that the converter cannot handle.
ob-stripe
force-pushed
the
ob-interface-next-source-action
branch
from
9d1e517 to
dc4d5f1
Compare
r? @remi-stripe
cc @stripe/api-libraries
Similar to #1320 but for
PaymentIntentSourceAction. The logic is slightly different because we can't deserialize the polymorphic attribute (value) based on its contents (like we can for Stripe objects, by looking at theobjectattribute). Instead, we need to look at the value of thetypeattribute that's at the same nesting level asvalue, so the converter has to be associated with the class that encompasses both attributes.I've also removed
Noneas a possible value in the enum -- instead, the entireNextSourceActionattribute will be set tonull(like in the API).