-
Notifications
You must be signed in to change notification settings - Fork 573
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
Add support for passing level III data when creating a charge #1216
Conversation
|
||
namespace Stripe.Infrastructure.Middleware | ||
{ | ||
internal class ChargeLevel3Plugin : IParserPlugin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We really should revisit how plugins work. It's all copy/pasta for the sake of adding one list of items. There has to be a better way
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. I took a stab at this in #1218.
|
||
namespace Stripe | ||
{ | ||
public class StripeChargeLevel3LineItemOptions : INestedOptions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still don't understand when something should be called Options versus Option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should always be Options
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that would work. I thought that we started using Option
for nested ones to differentiate them from the top-level ones.
{ | ||
public class StripeChargeLevel3Options : INestedOptions | ||
{ | ||
[JsonProperty("level3[merchant_reference]")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works but since the parent has JsonProperty(level3)
why isn't it passed to the lower level and you have to repeat it? The same happened to shipping.
/// </summary> | ||
// this will actually send `line_items`. this is to flag it for the middleware | ||
// to process it as a plugin | ||
[JsonProperty("charge_level3_line_items_array")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This hack makes me sad each time I have to use it (and grep to find it again).
changed to [WIP] as this requires changes to the Charge ressource too but those are behind a gate |
469c123
to
f8f80de
Compare
Okay pushed a new version that can handle the returned |
cbc76d2
to
a133aa3
Compare
r? @ob-stripe
cc @stripe/api-libraries
cc @fred-stripe