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

Stripe: create native WC Subscriptions (if a flag is set) #2251

Merged
merged 15 commits into from
Feb 13, 2023

Conversation

adekbadek
Copy link
Member

@adekbadek adekbadek commented Jan 26, 2023

All Submissions:

Changes proposed in this Pull Request:

Changes the way Stripe donations are handled, so that it's possible that instead of a Stripe Subscription, a WC Subscriptions' Subscription is created for recurring payments.

There is an underlying change in how Stripe API is interacted with. The way to charge a customer is either through Sources, Tokens, or Payment Methods – the latter replaces the former two. This integration has used Payment Methods, but since WC Stripe Gateway uses a Source to process WC Subscriptions renewals, the integration was updated to use Sources instead of Payment Methods.

How to test the changes in this Pull Request:

There are two ways to pay, and two modes of payment. When testing, ideally test all four scenarios (let's name them with letters for easier communication):

Once Recurring (e.g. monthly)
Card form A B
PaymentRequest (e.g. Apple Pay) C D
  1. Set up Stripe, ensure the site is reachable by Stripe webhooks
  2. Ensure the WC suite is active, and "Stripe – Credit Card (Stripe)" set as the sole payment method in WC
  3. Test the payment scenarios for regressions – the recurring payments should result in Stripe Subscriptions shadowed by sync'd WC Subscriptions (which can't be edited, etc.)
  4. Set the NEWSPACK_USE_WC_SUBSCRIPTIONS_WITH_STRIPE_PLATFORM environment flag to true
  5. Test the payment scenarios with this new configuration – recurring payments should result in WC Subscriptions and one-time charges in Stripe (not Stripe Subscriptions)
  6. Open the WC Subscriptions (scenarios B & D) in WP admin renew them manually (via the "Subscription actions" meta box)
  7. Observe that the two subscriptions were renewed:
    1. Each got a new Stripe charge (successful)
    2. Each got a renewal order in WC
  8. Disable the Stripe gateway in WC, observe that a recurring donation results in a Stripe Subscription

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

Copy link
Contributor

@dkoo dkoo left a comment

Choose a reason for hiding this comment

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

I wasn't able to get through all of the testing steps. Perhaps some of these are due to user error or incorrect config on my test site, and maybe others are due to changes from other branches. Here's what I saw:

  1. Manually renewing a Woo subscription:

Open the WC Subscriptions (scenarios B & D) in WP admin renew them manually (via the "Subscription actions" meta box)

When I did this, it resulted in a pending order being added to the subscription. The order never seems to complete. Because the order hangs as pending, the subscription status is changed to "On Hold", and a transaction never occurs on Stripe.

  1. Custom metadata fields don't seem to be fully syncing. On master, I see all the NP_ fields being synced, but on this branch only a few are:

Screen Shot 2023-01-30 at 4 11 47 PM

I wonder if this is due to these changes (1, 2) to the custom fields? I tried rebasing against master but that didn't change anything for me.

  1. I wasn't able to test the C & D scenarios (PaymentIntent). Do we have a way to test these payment methods without requiring a real credit card transaction?

includes/reader-revenue/stripe/class-stripe-connection.php Outdated Show resolved Hide resolved
@adekbadek
Copy link
Member Author

When I did [manually renew a Woo subscription] it resulted in a pending order being added to the subscription. The order never seems to complete.

The order is created synchronously (as pending), but it's completed on webhook callback. Can you make sure the site is reachable for Stripe's webhook requests? Alternatively, maybe there's a PHP error logged during the webhook request?

Custom metadata fields don't seem to be fully syncing.

Good catch! This led me to fix an unidiomatic use of WC's order items (2621c96 – an order item has to be unique). But for the most part, the ESP data syncing is fixed in be22c5f.

I wasn't able to test the C & D scenarios (PaymentIntent). Do we have a way to test these payment methods without requiring a real credit card transaction?

As long as Stripe is configured to use test mode, you can test with real payment details and you won't be charged. See https://support.stripe.com/questions/testing-apple-pay-with-stripe

@adekbadek adekbadek requested a review from dkoo February 1, 2023 13:21
@dkoo
Copy link
Contributor

dkoo commented Feb 1, 2023

The order is created synchronously (as pending), but it's completed on webhook callback. Can you make sure the site is reachable for Stripe's webhook requests? Alternatively, maybe there's a PHP error logged during the webhook request?

I'm not seeing any PHP error resulting from the renewal, and in fact I don't see any webhook request in the Stripe dashboard when I process a renewal from the WooCommerce Subscription page. Is there another way I should try manually renewing the subscription? This is what I'm using:

Screen Shot 2023-02-01 at 4 19 49 PM

@dkoo
Copy link
Contributor

dkoo commented Feb 1, 2023

Good catch! This led me to fix an unidiomatic use of WC's order items (2621c96 – an order item has to be unique). But for the most part, the ESP data syncing is fixed in be22c5f.

Can confirm that the data syncing is fixed, now!

Without RAS enabled, the WP user won't be created on donation.
@adekbadek
Copy link
Member Author

Fixed a dependency on RAS in b56ad32. If RAS is not enabled, a user account will not be created on donation, so the code here needs to create one (WP User is required to set up a WC Subscription).

Copy link
Contributor

@dkoo dkoo left a comment

Choose a reason for hiding this comment

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

I'm going to approve this since the only outstanding issues seem to be related to my testing environment. Thanks for the fixes!

@github-actions github-actions bot added [Status] Approved The pull request has been reviewed and is ready to merge and removed [Status] Needs Review The issue or pull request needs to be reviewed labels Feb 3, 2023
@adekbadek adekbadek merged commit 009a8a5 into master Feb 13, 2023
@adekbadek adekbadek deleted the feat/sdb-native-wc-subs branch February 13, 2023 10:43
adekbadek added a commit to Automattic/newspack-blocks that referenced this pull request Feb 13, 2023
matticbot pushed a commit that referenced this pull request Feb 17, 2023
# [1.103.0-alpha.2](v1.103.0-alpha.1...v1.103.0-alpha.2) (2023-02-17)

### Bug Fixes

* **ads-wizard:** cancelled state of unit displayed on the list ([#2272](#2272)) ([e10b592](e10b592))
* **ads:** remove fixed height setting ([#2255](#2255)) ([0031b89](0031b89))
* change default auth strategy to "link" ([#2261](#2261)) ([c4b2e2c](c4b2e2c))
* update package.json to resolve a merge conflict with master ([f2408a3](f2408a3))
* **webhooks:** check if endpoint exists ([#2276](#2276)) ([5a95e2c](5a95e2c))

### Features

* bootstrap GA4 connector ([#2241](#2241)) ([f7582a4](f7582a4))
* **data-events:** mailchimp connector ([#2233](#2233)) ([08060c6](08060c6))
* **data-events:** standardize the use of "user_id" and "email" ([#2263](#2263)) ([d6b6903](d6b6903))
* **donations:** add payment metadata and update renewal description ([#2254](#2254)) ([b039304](b039304))
* **my-account:** display stripe billing link regardless of RR platform ([df540fe](df540fe))
* **perfmatters:** defaults adjustments, remove feature flag ([#2271](#2271)) ([cf5ad51](cf5ad51))
* **performance:** minify all inline CSS ([#2239](#2239)) ([fbb415f](fbb415f))
* **ras:** enable custom contact metadata prefixes ([#2249](#2249)) ([e9843e4](e9843e4))
* require all woo plugins for RAS ([#2260](#2260)) ([5f6a818](5f6a818))
* **stripe:** create native WC Subscriptions (if feature flag is set) ([#2251](#2251)) ([009a8a5](009a8a5))
* **webhooks:** support label for endpoints ([#2262](#2262)) ([39da7be](39da7be))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 1.103.0-alpha.2 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

matticbot pushed a commit that referenced this pull request Feb 17, 2023
# [1.104.0-alpha.1](v1.103.0...v1.104.0-alpha.1) (2023-02-17)

### Bug Fixes

* **ads-wizard:** cancelled state of unit displayed on the list ([#2272](#2272)) ([e10b592](e10b592))
* **ads:** remove fixed height setting ([#2255](#2255)) ([0031b89](0031b89))
* change default auth strategy to "link" ([#2261](#2261)) ([c4b2e2c](c4b2e2c))
* **webhooks:** check if endpoint exists ([#2276](#2276)) ([5a95e2c](5a95e2c))

### Features

* bootstrap GA4 connector ([#2241](#2241)) ([f7582a4](f7582a4))
* **data-events:** mailchimp connector ([#2233](#2233)) ([08060c6](08060c6))
* **data-events:** standardize the use of "user_id" and "email" ([#2263](#2263)) ([d6b6903](d6b6903))
* **donations:** add payment metadata and update renewal description ([#2254](#2254)) ([b039304](b039304))
* **my-account:** display stripe billing link regardless of RR platform ([df540fe](df540fe))
* **perfmatters:** defaults adjustments, remove feature flag ([#2271](#2271)) ([cf5ad51](cf5ad51))
* **performance:** minify all inline CSS ([#2239](#2239)) ([fbb415f](fbb415f))
* **ras:** enable custom contact metadata prefixes ([#2249](#2249)) ([e9843e4](e9843e4))
* require all woo plugins for RAS ([#2260](#2260)) ([5f6a818](5f6a818))
* **stripe:** create native WC Subscriptions (if feature flag is set) ([#2251](#2251)) ([009a8a5](009a8a5))
* **webhooks:** support label for endpoints ([#2262](#2262)) ([39da7be](39da7be))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 1.104.0-alpha.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

matticbot pushed a commit that referenced this pull request Feb 20, 2023
# [1.105.0-alpha.1](v1.104.0...v1.105.0-alpha.1) (2023-02-20)

### Bug Fixes

* **ads-wizard:** cancelled state of unit displayed on the list ([#2272](#2272)) ([e10b592](e10b592))
* **ads:** remove fixed height setting ([#2255](#2255)) ([0031b89](0031b89))
* change default auth strategy to "link" ([#2261](#2261)) ([c4b2e2c](c4b2e2c))
* **webhooks:** check if endpoint exists ([#2276](#2276)) ([5a95e2c](5a95e2c))

### Features

* bootstrap GA4 connector ([#2241](#2241)) ([f7582a4](f7582a4))
* **data-events:** mailchimp connector ([#2233](#2233)) ([08060c6](08060c6))
* **data-events:** standardize the use of "user_id" and "email" ([#2263](#2263)) ([d6b6903](d6b6903))
* **donations:** add payment metadata and update renewal description ([#2254](#2254)) ([b039304](b039304))
* **my-account:** display stripe billing link regardless of RR platform ([df540fe](df540fe))
* **perfmatters:** defaults adjustments, remove feature flag ([#2271](#2271)) ([cf5ad51](cf5ad51))
* **performance:** minify all inline CSS ([#2239](#2239)) ([fbb415f](fbb415f))
* **ras:** enable custom contact metadata prefixes ([#2249](#2249)) ([e9843e4](e9843e4))
* require all woo plugins for RAS ([#2260](#2260)) ([5f6a818](5f6a818))
* **stripe:** create native WC Subscriptions (if feature flag is set) ([#2251](#2251)) ([009a8a5](009a8a5))
* **webhooks:** support label for endpoints ([#2262](#2262)) ([39da7be](39da7be))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 1.105.0-alpha.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

adekbadek added a commit that referenced this pull request Feb 21, 2023
Stripe Subscriptions created before the changes in
#2251 will not have a Source, but the customer will
have PaymentMethods. These changes ensure that these
customers' subscriptions can be migrated.

This also means that not all changes from #2251 are
necessary – Stripe Payment Gateway creates sources,
but will handle a PaymentMethod ID if set as the source
ID in the subscription metadata.
@matticbot
Copy link
Contributor

🎉 This PR is included in version 1.105.0-hotfix-hpb-amp-optimisations.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

matticbot pushed a commit that referenced this pull request Feb 28, 2023
# [1.105.0](v1.104.1...v1.105.0) (2023-02-28)

### Bug Fixes

* **ads-wizard:** cancelled state of unit displayed on the list ([#2272](#2272)) ([e10b592](e10b592))
* **ads:** remove fixed height setting ([#2255](#2255)) ([0031b89](0031b89))
* change default auth strategy to "link" ([#2261](#2261)) ([c4b2e2c](c4b2e2c))
* **webhooks:** check if endpoint exists ([#2276](#2276)) ([5a95e2c](5a95e2c))

### Features

* bootstrap GA4 connector ([#2241](#2241)) ([f7582a4](f7582a4))
* **data-events:** mailchimp connector ([#2233](#2233)) ([08060c6](08060c6))
* **data-events:** standardize the use of "user_id" and "email" ([#2263](#2263)) ([d6b6903](d6b6903))
* **donations:** add payment metadata and update renewal description ([#2254](#2254)) ([b039304](b039304))
* **my-account:** display stripe billing link regardless of RR platform ([df540fe](df540fe))
* **perfmatters:** defaults adjustments, remove feature flag ([#2271](#2271)) ([cf5ad51](cf5ad51))
* **performance:** minify all inline CSS ([#2239](#2239)) ([fbb415f](fbb415f))
* **ras:** enable custom contact metadata prefixes ([#2249](#2249)) ([e9843e4](e9843e4))
* require all woo plugins for RAS ([#2260](#2260)) ([5f6a818](5f6a818))
* **stripe:** create native WC Subscriptions (if feature flag is set) ([#2251](#2251)) ([009a8a5](009a8a5))
* **webhooks:** support label for endpoints ([#2262](#2262)) ([39da7be](39da7be))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 1.105.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

adekbadek added a commit that referenced this pull request Mar 1, 2023
As it turns out, not all changes from #2251 are
necessary – Stripe Payment Gateway creates sources,
but will handle a PaymentMethod ID if set as the source
ID in the subscription metadata.
matticbot pushed a commit that referenced this pull request Mar 3, 2023
# [1.106.0-alpha.1](v1.105.0...v1.106.0-alpha.1) (2023-03-03)

### Bug Fixes

* **ads:** gam api availability according to error type ([#2289](#2289)) ([024fe08](024fe08))

### Features

* add a Add new button to subscription lists ([#2314](#2314)) ([9543ad2](9543ad2))
* add ga4 user registered handler ([#2281](#2281)) ([5eb2336](5eb2336))
* add pid to Logger ([#2290](#2290)) ([fd3011c](fd3011c))
* Add popup info to donations ([#2300](#2300)) ([7ea800b](7ea800b))
* allow external links in dashboard via a filter ([#2279](#2279)) ([3943b1a](3943b1a))
* campaigns listeners for the data events api ([#2291](#2291)) ([ab407d4](ab407d4))
* disable save button for unchanged settings ([#2259](#2259)) ([e06d72f](e06d72f)), closes [#1531](#1531)
* **donate-block:** support modal checkout ([#2256](#2256)) ([34226dd](34226dd))
* Normalize donation events ([#2299](#2299)) ([2624d53](2624d53))
* **perfmatters:** improve config ([267306e](267306e))
* prevent homepage from being unpublished ([#2307](#2307)) ([a151d53](a151d53))
* Remove the campaign rendered event ([#2301](#2301)) ([23caa1d](23caa1d))
* Stripe Subscriptions to WC subscriptions migrator ([#2298](#2298)) ([6904356](6904356)), closes [#2251](#2251)
* **wc:** force allowing subscription switching ([#2305](#2305)) ([c13e741](c13e741))
matticbot pushed a commit that referenced this pull request Mar 14, 2023
# [1.106.0](v1.105.1...v1.106.0) (2023-03-14)

### Bug Fixes

* **ads:** gam api availability according to error type ([#2289](#2289)) ([024fe08](024fe08))
* show handoff to finish Newspack setup only if setup is incomplete ([#2343](#2343)) ([1173b5b](1173b5b))

### Features

* add a Add new button to subscription lists ([#2314](#2314)) ([9543ad2](9543ad2))
* add ga4 user registered handler ([#2281](#2281)) ([5eb2336](5eb2336))
* add pid to Logger ([#2290](#2290)) ([fd3011c](fd3011c))
* Add popup info to donations ([#2300](#2300)) ([7ea800b](7ea800b))
* allow external links in dashboard via a filter ([#2279](#2279)) ([3943b1a](3943b1a))
* campaigns listeners for the data events api ([#2291](#2291)) ([ab407d4](ab407d4))
* disable save button for unchanged settings ([#2259](#2259)) ([e06d72f](e06d72f)), closes [#1531](#1531)
* **donate-block:** support modal checkout ([#2256](#2256)) ([34226dd](34226dd))
* Normalize donation events ([#2299](#2299)) ([2624d53](2624d53))
* **perfmatters:** improve config ([267306e](267306e))
* prevent homepage from being unpublished ([#2307](#2307)) ([a151d53](a151d53))
* Remove the campaign rendered event ([#2301](#2301)) ([23caa1d](23caa1d))
* Stripe Subscriptions to WC subscriptions migrator ([#2298](#2298)) ([6904356](6904356)), closes [#2251](#2251)
* **wc:** force allowing subscription switching ([#2305](#2305)) ([c13e741](c13e741))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants