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

Add missing attributes on StripeThreeDSecure #1279

Merged
merged 1 commit into from
Sep 11, 2018
Merged

Conversation

ob-stripe
Copy link
Contributor

r? @remi-stripe
cc @stripe/api-libraries @pradeepannepu

Adds all missing attributes on the StripeThreeDSecure model class (including gated ones).

Replaces #1274.

public string Funding { get; set; }

[JsonProperty("google_reference")]
public string GoogleReference { get; set; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what this is but it seems more internal than external I would not surface. I feel the same about IIN + Issuer + Description. It's gated and while it's supported on the Source resource I'm not convinced we should surface it here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIN, Issuer and Description are already available on StripeCard though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed GoogleReference.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know they are visible but I wanted to avoid leaking them further. Can you at least do what we do on the other resource: and group them with a comment, or add a comment to each one about being gated?

@ob-stripe
Copy link
Contributor Author

I added XML comments for all attributes. ptal @remi-stripe

/// Whether the card was automatically updated at some point by Stripe or not.
/// </summary>
[JsonProperty("card_automatically_updated")]
public bool CardAutomaticallyUpdated { get; set; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is purely internal and should not be here (sorry only seeing it now)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll remove it, but it is returned by the public API though (cf. the sample JSON in #1274)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know, but I also know it should not and that we flagged it and that it should have been fixed already.

@ob-stripe
Copy link
Contributor Author

ptal @remi-stripe

@ob-stripe ob-stripe assigned remi-stripe and unassigned ob-stripe Sep 11, 2018
@ob-stripe ob-stripe merged commit c04bae7 into master Sep 11, 2018
@ob-stripe ob-stripe deleted the ob-update-source-3ds branch September 11, 2018 15:20
@ob-stripe
Copy link
Contributor Author

Released in 19.6.0.

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

Successfully merging this pull request may close these issues.

2 participants