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

Use Stripe-side info when displaying webhook status #3531

Merged
merged 7 commits into from
Nov 15, 2024

Conversation

annemirasol
Copy link
Contributor

@annemirasol annemirasol commented Oct 22, 2024

Instead of determining webhook status ('Enabled' vs 'Disabled') by using the presence or absence of secret keys, we check the actual status using Stripe API.

Fixes #3512

Changes proposed in this Pull Request:

In Stripe's Settings page, Account Status section, we display the webhook status as "Enabled" or "Disabled", based on the presence or absence of a webhook secret in the settings store. If the webhook has been disabled or deleted outside of WooCommerce, this approach leads to an inaccurate webhook status.

In this PR, we will display a more accurate webhook status by retrieving it using a Stripe API call.

Testing instructions

  1. Set up a publicly accessible WooCommerce store, e.g. using tunneling via ngrok or JT, or upload this branch's build to a JN site.
  2. Connect your store to your Stripe account.
  3. On successfully connecting, you should see the webhook status as "Enabled".
  4. Go to https://dashboard.stripe.com/test/webhooks, making sure you're on the correct Stripe account and mode (test/live). You should see the webhook for your store.
  5. Click on the webhook to load its details page.
  6. On the upper right corner, you should see a three-dot menu. Disable the webhook.
  7. Go back to your WooCommerce store, and reload the Stripe settings page.
  8. In develop, the webhook status still shows as Enabled.
  9. In this branch, the webhook status now shows Disabled.

  • Covered with tests (or have a good reason not to test in description ☝️)
  • Added changelog entry in both changelog.txt and readme.txt (or does not apply)
  • Tested on mobile (or does not apply)

Post merge

@annemirasol annemirasol self-assigned this Nov 11, 2024
@annemirasol annemirasol force-pushed the fix/3512-webhook-status branch 2 times, most recently from ab7ec61 to 092bbc2 Compare November 11, 2024 21:17
Instead of determining webhook status ('Enabled' vs 'Disabled')
by using the presence or absence of secret keys, we check
the actual status using Stripe API.
@annemirasol annemirasol marked this pull request as ready for review November 11, 2024 22:56
@annemirasol annemirasol requested review from a team and Mayisha and removed request for a team November 11, 2024 22:57
Copy link
Contributor

@Mayisha Mayisha left a comment

Choose a reason for hiding this comment

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

The code looks good and it is working perfectly.
I have tested by enabling/disabling the webhook in my Stripe dashboard. 🎉

I noticed one thing. On every page refresh we are fetching the webhook status from Stripe. Also on every save on the Stripe settings page (i.e enable/disable any method and save), we are fetching this from Stripe again.
Should we opt for a short term caching for this webhook status response? Similar to how we cache the account data for about 2 hours.

@annemirasol
Copy link
Contributor Author

Should we opt for a short term caching for this webhook status response? Similar to how we cache the account data for about 2 hours.

Good idea. I'll try to work that in for this PR.

@annemirasol
Copy link
Contributor Author

@Mayisha Caching added in 2bbe8df, thanks for suggesting that.

  • Cache is invalidated and API calls are made on page refresh, same behavior as account data.
  • API calls are no longer made when saving.

Copy link
Contributor

@Mayisha Mayisha left a comment

Choose a reason for hiding this comment

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

Thanks for the changes @annemirasol. It is working nicely. 🎉

✅ Fetching the webhook status via API call on page refresh.
✅ Returning the data from the cache on any changes in the settings.

@annemirasol annemirasol merged commit 15f958c into develop Nov 15, 2024
33 of 35 checks passed
@annemirasol annemirasol deleted the fix/3512-webhook-status branch November 15, 2024 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Check the Webhook endpoint status at Stripe
2 participants