Update/normalize donation events

[leogermani]

All Submissions:

• Have you followed the Newspack Contributing guideline?
• Does your code follow the WordPress' coding standards and VIP Go coding standards?
• Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

Normalizes the donation_new event so it works the same way in both platforms.

Before the change in this PR, donations processed through Woocommerce would trigger a donation_new event before they were confirmed, while in Stripe it would only be triggered after confirmation.

Now the event is only triggered after confirmation in both platforms.

We keep the current WooCommerce listener as a new event called donation_order_processed as suggested in p1676562193774819-slack-1676536201.225739 (even though I'm not sure we need it)

How to test the changes in this Pull Request:

1. Add the following snippet to your site:

Data_Events::register_handler( 'debug_donation_new_events', 'donation_new' );

function debug_donation_new_events( $timestamp, $data, $user_id ) {
	error_log( print_r( $data, true ) );
}

Data_Events::register_handler( 'debug_donation_subscription_new_events', 'donation_subscription_new' );

function debug_donation_subscription_new_events( $timestamp, $data, $user_id ) {
	error_log( print_r( $data, true ) );
}

2. Set up Stripe as the Reader Revenue platform
3. Make donations
4. Watch the logs and make sure you see the correct events being triggered at the correct times, with the correct data
5. Do the same for the Newspack platform and confirm they are consistent
6. Make sure to test with cards that will return failed payments to make sure events are not triggered in those situations

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?

[dkoo]

Approving because it works, but I noticed something that may be outside the scope of this PR—when using the Stripe RR platform, the user_id value always seems to be 0. Tested on both an existing and a new browser session.

[leogermani]

Approving because it works, but I noticed something that may be outside the scope of this PR—when using the Stripe RR platform, the user_id value always seems to be 0. Tested on both an existing and a new browser session.

I could not reproduce this. Did you have RAS enabled when you tested? This will happen if RAS is not enabled because a user won't be created.

I really want @miguelpeixe to have a look at this, but I'll merge it if I get to the next task that depends on it so I'm not blocked. We can always revisit/revert it if Miguel finds a problem.

[dkoo]

I could not reproduce this. Did you have RAS enabled when you tested?

RAS is enabled on my site, and the user ID is populated when Newspack is the Reader Revenue platform. It's only 0 for Stripe.

[miguelpeixe]

Working as described

when using the Stripe RR platform, the user_id value always seems to be 0. Tested on both an existing and a new browser session.

@dkoo Perhaps you are unauthenticated and donating with an already registered email? The donation will not be tied to the user ID if that's the case.

[leogermani]

The donation will not be tied to the user ID if that's the case.

Why not? Shouldn't it?

[dkoo]

Why not? Shouldn't it?

That was my assumption as well—I was assuming if using an email address that is known to be associated with a user ID, that user ID should be sent with the data.

[miguelpeixe]

We can double-check with @adekbadek, but that's not how it's designed:

newspack-plugin/includes/reader-revenue/stripe/class-stripe-connection.php

Lines 771 to 775 in 2624d53

	$user_id = Reader_Activation::register_reader( $email_address, $full_name, true, $reader_metadata );
	if ( ! \is_wp_error( $user_id ) && false !== $user_id ) {
	$client_metadata['userId'] = $user_id;
	}
	}

$user_id will be false if the user is already registered:

newspack-plugin/includes/reader-activation/class-reader-activation.php

Lines 1284 to 1298 in 2624d53

	/**
	* Register a reader given its email.
	*
	* Due to authentication or auth intention, this method should be used
	* preferably on POST or API requests to avoid issues with caching.
	*
	* @param string $email Email address.
	* @param string $display_name Reader display name to be used on account creation.
	* @param bool $authenticate Whether to authenticate after registering. Default to true.
	* @param array $metadata Any metadata to pass along to the action hook.
	*
	* @return int|false|\WP_Error The created user ID in case of registration, false if the user already exists, or a WP_Error object.
	*/
	public static function register_reader( $email, $display_name = '', $authenticate = true, $metadata = [] ) {
	if ( ! self::is_enabled() ) {

I'm not sure we can/should relate a donation to a user account without authorization.

[dkoo]

Thanks for the additional info—let's leave as-is for now

[matticbot]

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

The release is available on GitHub release

Your semantic-release bot 📦🚀

[matticbot]

🎉 This PR is included in version 1.106.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀