Add customer payment methods and PPCP chargeback reporting #25
Conversation
chrismccluskey
changed the title
src/inc/sift-events/sift-events.php
Outdated
| @@ -565,12 +569,17 @@ public static function update_or_create_order( string $order_id, \WC_Order $orde | |||
| } | |||
|
|
|||
| // Determine user and session context. | |||
| $user_id = wp_get_current_user()->ID ?? null; // Check first for logged-in user. | |||
| $user_id = get_current_user_id() ?? wp_get_current_user()->ID ?? null; // Check first for logged-in user. | |||
There was a problem hiding this comment.
get_current_user_id() definition:
function get_current_user_id() {
if ( ! function_exists( 'wp_get_current_user' ) ) {
return 0;
}
$user = wp_get_current_user();
return ( isset( $user->ID ) ? (int) $user->ID : 0 );
}
As you can see it is using wp_get_current_user internally. So I am not sure what this change adds...
There was a problem hiding this comment.
I'm not sure if it's needed anymore. At one point this made a difference, but I think I addressed that issue in another way.
src/inc/sift-events/sift-events.php
Outdated
| } | ||
|
|
||
| if ( ! $user_id || $is_admin ) { | ||
| $user_id = $order->get_user_id() ?? null; // Use order user ID if it isn't available otherwise | ||
| } | ||
|
|
||
| $user = $user_id ? get_userdata( $user_id ) : null; |
There was a problem hiding this comment.
$user here is only used later to get the $user_id again. Maybe we should not need that...
And simply do
$properties = array(
'$user_id' => $user_id ? self::format_user_id( $user_id ) : ''
[ ... ]
What do you think?
There was a problem hiding this comment.
I'll check and simplify if possible.
src/inc/sift-events/sift-events.php
Outdated
| if ( ! $user_id && $is_system ) { | ||
| $user_id = $order->get_user_id() ?? null; // Use order user ID if is a system update | ||
| } | ||
|
|
||
| if ( ! $user_id || $is_admin ) { | ||
| $user_id = $order->get_user_id() ?? null; // Use order user ID if it isn't available otherwise |
There was a problem hiding this comment.
I have the feeling that this whole block can be made more readible... I don't understand much of the logic behind it...
If $user_id is null when getting there, getting the id from the $order should be tried, even if it is not system or admin, no?
There was a problem hiding this comment.
It probably can. I didn't know the reasoning behind getting the user_id from the order only when it's a system update. I think the more simple logic is just to try to get the user_id from the order if the user_id isn't available or if it's admin (this would remove the $is_system check).
|
|
||
| add_filter( 'sift_for_woocommerce_ppcp-gateway_payment_gateway_string', fn() => '$paypal' ); | ||
|
|
||
| add_action( | ||
| 'woocommerce_payments_before_webhook_delivery', |
There was a problem hiding this comment.
ppcp is using WcPay webhooks structure?
There was a problem hiding this comment.
Great catch! This isn't the correct filter. I went looking for one for PPCP and could not find one. We may not be able to report chargebacks from PPCP as a result.
There was a problem hiding this comment.
Adding to this a bit, because I'm a bit surprised to see this myself. I've been trying to find some clue about where dispute webhooks would be handled with ppcp, and I'm not finding anything. Some things I thought would be related but appear not to be (unless I'm misunderstanding) are:
- Payment capture reversed - it sounds like chargeback, but it's something separate (
PAYMENT.CAPTURE.REVERSED). - Capture status reason - chargeback / buyer_complaint. This would sound like it's what we want, except that it happens during capture and not via a webhook. I think this is important because I'm thinking this happens when the customer is trying to purchase something and they already have a dispute (though I may be wrong). I would assume that the typical lifecycle of chargebacks means that the customer needs some time to determine that the charge was fraudulent, that it wouldn't happen in the same request as making the purchase, meaning it would need to come in through the webhook.
According to PayPal docs, the webhook event we want to listen to is CUSTOMER.DISPUTE.CREATED, and in the admin for this payment gateway, we can see that this event is added when the webhook is created (this gateway allows setting up webhooks from inside wp-admin with the click of a button).
What I'm hoping to find at this point is a generic way to hook into all webhook events for ppcp. I'm not finding anything yet with the search terms I would expect to see (webhook, before, dispute, etc..) It's possible that this plugin may not have implemented these webhooks yet.
There was a problem hiding this comment.
It's possible that this plugin may not have implemented these webhooks yet.
Yes, I am not surprised.
This is why I have been talking about reusing @chrisfromthelc code here where he added an endpoint we can send the webhook to.
| * | ||
| * @return void | ||
| */ | ||
| function send_chargeback_to_sift( $event ): void { |
There was a problem hiding this comment.
I kept these functions in case we ever get a filter to allow us to use them in the future.
|
Changes proposed in this Pull Request
Testing instructions
$update_accountis correct.$create_accountis correct.Mentions # 554-gh-automattic/gold