Skip to content

Add hook to allow for custom webhook post-processing #4466

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

Open
wants to merge 25 commits into
base: develop
Choose a base branch
from

Conversation

daledupreez
Copy link
Contributor

@daledupreez daledupreez commented Jul 7, 2025

Towards STRIPE-564

Changes proposed in this Pull Request:

This PR proposes the addition of a new wc_stripe_webhook_received action that is triggered just after we process an inbound webhook from Stripe. This will make it easier for merchants and developers to include more details in WooCommerce based on the data from Stripe. The initial use case for this is to include more data for charges to allow for easier fraud detection while processing orders.

Testing instructions

  • Checkout to this branch
  • Connect your Stripe account
  • Make sure your store is publicly accessible and has webhooks configured
  • Add the following code snippet:
add_action( 'wc_stripe_webhook_received', function( $notification_type, $notification_data ) {
	WC_Stripe_Logger::info( 'TEST: wc_stripe_webhook_received action ' . $notification_type . ' with data: ' . print_r( $notification_data, true ) );
}, 10, 2 );
  • As a shopper, purchase any product
  • As a merchant, check your latest Stripe logs
  • Confirm you can see the test log:
Screenshot 2025-07-18 at 11 53 50
  • Covered with tests (or have a good reason not to test in description ☝️)
  • Tested on mobile (or does not apply)

Changelog entry

  • This Pull Request does not require a changelog entry. (Comment required below)
Changelog Entry Comment

Comment

Post merge

@daledupreez daledupreez self-assigned this Jul 7, 2025
@wjrosa wjrosa marked this pull request as ready for review July 18, 2025 13:52
Copy link

📈 PHP Unit Code Coverage Report

Package Line Rate Health
includes/class-wc-stripe-webhook-handler.php 37%
Summary 45% (7623 / 17004)

@wjrosa wjrosa requested review from diegocurbelo and malithsen July 18, 2025 15:01
@wjrosa wjrosa requested a review from diegocurbelo July 21, 2025 19:39
@wjrosa wjrosa self-assigned this Jul 21, 2025
@wjrosa wjrosa requested a review from diegocurbelo July 29, 2025 19:37
Copy link
Member

@diegocurbelo diegocurbelo left a comment

Choose a reason for hiding this comment

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

We decided to implement a unified get_webook_order(...) or get_order_from_webhook(...) in a new PR, so meanwhile I'm approving this one.

@wjrosa
Copy link
Contributor

wjrosa commented Aug 1, 2025

@daledupreez should I go ahead and merge this? Since I see you are working on the other PR, and Diego approved this one (and I tested it myself when developing it).

@daledupreez
Copy link
Contributor Author

@wjrosa, if you don't mind waiting, can I review it one more time on Monday and make sure I am up to speed on the current state? That should also make it easier for me to update my follow-up PR once this change is merged.

@wjrosa
Copy link
Contributor

wjrosa commented Aug 1, 2025

@daledupreez sure! No worries

Copy link
Contributor Author

@daledupreez daledupreez left a comment

Choose a reason for hiding this comment

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

@wjrosa, I ended up noticing some subtle but important things that I addressed directly, so I think I'd like your eyes on this before we ship:

  • I centralised the logic to trigger the action in a helper function, as we were only wrapping it in a try/catch in one location, and we were catching Exception instead of Throwable, and we had slightly different logging as a result. Everything is now consistent.
  • I added explicit initialisation of $this->resolved_order to null, and documented that the argument may be null when the webhook is not related to an order. That is really important for developers to be aware of.

I have some other comments that I think are less critical, but are worth a look.

@wjrosa
Copy link
Contributor

wjrosa commented Aug 7, 2025

Thanks for the review, @malithsen!

@daledupreez should we merge this now?

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.

4 participants