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

Update for mid-Feb API changes #1146

Merged
merged 6 commits into from
Mar 14, 2019
Merged

Update for mid-Feb API changes #1146

merged 6 commits into from
Mar 14, 2019

Conversation

yuki-stripe
Copy link
Collaborator

Summary

Updating to accommodate some API changes https://stripe.com/docs/upgrades#2019-02-11

  • Deprecates STPPaymentIntentsStatusRequiresSource, replaced by STPPaymentIntentsStatusRequiresPaymentMethod
  • Deprecates STPPaymentIntentsStatusRequiresSourceAction, replaced by STPPaymentIntentsStatusRequiresAction
  • Deprecates STPPaymentIntentSourceAction and STPPaymentSourceActionAuthorizeWithURL classes, replaced by STPPaymentIntentAction and STPPaymentActionRedirectToURL
  • Deprecates nextSourceAction on STPPaymentIntent, replaced by nextAction

See https://paper.dropbox.com/doc/API-Changes-for-mid-Feb-version--AZT10970k8GfUgnOyuAeN~hOAg-PC7zkuMmx0i42AeypiP04

Testing

Reworked STPPaymentIntentSourceActionTest -> STPPaymentIntentActionTest.
Updated STPPaymentIntentFunctionalTest, STPPaymentIntentTest

To see what it would look like for a user who was still using the deprecated classes, I did the following:

  1. Turn off warnings as errors
  2. Added back the old STPPaymentIntentSourceActionTest, but replaced the test JSON with the new version (ie "authorize_with_url" -> "redirect_to_url")
    Result: The test passes, and you get warnings like this everywhere:

Screen Shot 2019-03-13 at 2 24 09 PM

@yuki-stripe yuki-stripe changed the base branch from yuki-add-wallet-ideal-present-2 to master March 13, 2019 21:28
…uthorizeWithURL -> STPPaymentIntentAction, STPPaymentIntentActionRedirectToURL
…tStatusRequiresSource, STPPaymentIntentStatusRequiresSourceAction
Copy link
Contributor

@danj-stripe danj-stripe left a comment

Choose a reason for hiding this comment

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

Looks totally reasonable to me. I have a couple code improvements, some docs suggestions.

I'm assuming you'll want to make at least some of the changes, and happy to re-review when you have.

Stripe/PublicHeaders/STPPaymentIntent.h Outdated Show resolved Hide resolved
Stripe/PublicHeaders/STPPaymentIntent.h Outdated Show resolved Hide resolved
Stripe/PublicHeaders/STPPaymentIntentAction.h Outdated Show resolved Hide resolved
Stripe/PublicHeaders/STPPaymentIntentAction.h Outdated Show resolved Hide resolved
Stripe/PublicHeaders/STPPaymentIntentEnums.h Outdated Show resolved Hide resolved
Stripe/STPPaymentIntent.m Outdated Show resolved Hide resolved
Stripe/STPPaymentIntentAction.m Show resolved Hide resolved
Tests/Tests/STPPaymentIntentTest.m Outdated Show resolved Hide resolved
Tests/Tests/STPPaymentIntentTest.m Outdated Show resolved Hide resolved
Tests/Tests/STPRedirectContextTest.m Outdated Show resolved Hide resolved
Copy link
Contributor

@danj-stripe danj-stripe left a comment

Choose a reason for hiding this comment

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

Was it a mistake to remove the requires_source string? Otherwise this looks great

Stripe/STPPaymentIntent.m Show resolved Hide resolved
Tests/Tests/STPPaymentIntentTest.m Outdated Show resolved Hide resolved
…entStatusRequiresPaymentMethod instead of a STPPaymentIntentStatusRequiresSource
Copy link
Contributor

@danj-stripe danj-stripe left a comment

Choose a reason for hiding this comment

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

🎉

@stripe-ci stripe-ci assigned yuki-stripe and unassigned danj-stripe Mar 14, 2019
@yuki-stripe yuki-stripe merged commit 9ae5935 into master Mar 14, 2019
@yuki-stripe yuki-stripe deleted the yuki-api-updates branch March 14, 2019 19:59
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.

3 participants