-
Notifications
You must be signed in to change notification settings - Fork 69
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
Store balance transaction ID in order metadata #7945
Changes from 15 commits
f33af70
fbc8664
7f9fc28
1bf3da7
83d5af8
d73ab09
5f1a297
8c2f80f
1e58fcc
a7bef87
adc9a67
1f4681a
69c82bd
bb87366
4d9d729
e745e71
2af5d52
8022a4a
edd6872
16f83e8
d3d2655
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
Significance: minor | ||
Type: update | ||
|
||
Store balance transaction ID in order metadata. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -113,6 +113,13 @@ class WC_Payments_Order_Service { | |
*/ | ||
const WCPAY_MODE_META_KEY = '_wcpay_mode'; | ||
|
||
/** | ||
* Meta key used to store payment transaction Id. | ||
* | ||
* @const string | ||
*/ | ||
const WCPAY_PAYMENT_TRANSACTION_ID_META_KEY = '_wcpay_payment_transaction_id'; | ||
|
||
/** | ||
* Client for making requests to the WooCommerce Payments API | ||
* | ||
|
@@ -531,6 +538,23 @@ public function set_charge_id_for_order( $order, $charge_id ) { | |
$order->save_meta_data(); | ||
} | ||
|
||
/** | ||
* Set the payment metadata for payment transaction id. | ||
* | ||
* @param mixed $order The order. | ||
* @param string $payment_transaction_id The value to be set. | ||
* | ||
* @throws Order_Not_Found_Exception | ||
*/ | ||
public function set_payment_transaction_id_for_order( $order, $payment_transaction_id ) { | ||
if ( ! isset( $payment_transaction_id ) || null === $payment_transaction_id ) { | ||
return; | ||
} | ||
$order = $this->get_order( $order ); | ||
$order->update_meta_data( self::WCPAY_PAYMENT_TRANSACTION_ID_META_KEY, $payment_transaction_id ); | ||
$order->save_meta_data(); | ||
} | ||
|
||
/** | ||
* Get the payment metadata for charge id. | ||
* | ||
|
@@ -773,17 +797,43 @@ public function get_fraud_meta_box_type_for_order( $order ) : string { | |
/** | ||
* Given the payment intent data, adds it to the given order as metadata and parses any notes that need to be added | ||
* | ||
* @param WC_Order $order The order. | ||
* @param WC_Payments_API_Payment_Intention|WC_Payments_API_Setup_Intention $intent The payment or setup intention object. | ||
* | ||
* @throws Order_Not_Found_Exception | ||
*/ | ||
public function attach_intent_info_to_order( WC_Order $order, $intent ) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. part of this method ( calling the setters and calling the save method) is identical to
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with this, common code can be extracted to a separate function as part of this refactor. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @dpaun1985 @naman03malhotra Thanks for the feedback. I fixed the code duplication in a7bef87. In the long term, I think it's desirable to remove the legacy method and keep only the refactored one. This will allow us to care about only one method when adding new intent properties to order meta.
Unfortunately, I cannot change the name as I'd like my changes to apply to all uses of this method. |
||
// first, let's prepare all the metadata needed for refunds, required for status change etc. | ||
$intent_id = $intent->get_id(); | ||
$intent_status = $intent->get_status(); | ||
$payment_method = $intent->get_payment_method_id(); | ||
$customer_id = $intent->get_customer_id(); | ||
$currency = $intent instanceof WC_Payments_API_Payment_Intention ? $intent->get_currency() : $order->get_currency(); | ||
$charge = $intent instanceof WC_Payments_API_Payment_Intention ? $intent->get_charge() : null; | ||
$charge_id = $charge ? $charge->get_id() : null; | ||
$payment_transaction = $charge ? $charge->get_balance_transaction() : null; | ||
$payment_transaction_id = $payment_transaction['id'] ?? ''; | ||
// next, save it in order meta. | ||
$this->attach_intent_info_to_order__legacy( $order, $intent_id, $intent_status, $payment_method, $customer_id, $charge_id, $currency, $payment_transaction_id ); | ||
} | ||
|
||
/** | ||
* Legacy version of the attach_intent_info_to_order method. | ||
* | ||
* TODO: This method should ultimately be merged with `attach_intent_info_to_order` and then removed. | ||
* | ||
* @param WC_Order $order The order. | ||
* @param string $intent_id The intent ID. | ||
* @param string $intent_status Intent status. | ||
* @param string $payment_method Payment method ID. | ||
* @param string $customer_id Customer ID. | ||
* @param string $charge_id Charge ID. | ||
* @param string $currency Currency code. | ||
* @param string $payment_transaction_id The transaction ID of the linked charge. | ||
* | ||
* @throws Order_Not_Found_Exception | ||
*/ | ||
public function attach_intent_info_to_order( $order, $intent_id, $intent_status, $payment_method, $customer_id, $charge_id, $currency ) { | ||
public function attach_intent_info_to_order__legacy( $order, $intent_id, $intent_status, $payment_method, $customer_id, $charge_id, $currency, $payment_transaction_id = null ) { | ||
// first, let's save all the metadata that needed for refunds, required for status change etc. | ||
$order->set_transaction_id( $intent_id ); | ||
$this->set_intent_id_for_order( $order, $intent_id ); | ||
|
@@ -792,6 +842,7 @@ public function attach_intent_info_to_order( $order, $intent_id, $intent_status, | |
$this->set_intention_status_for_order( $order, $intent_status ); | ||
$this->set_customer_id_for_order( $order, $customer_id ); | ||
$this->set_wcpay_intent_currency_for_order( $order, $currency ); | ||
$this->set_payment_transaction_id_for_order( $order, $payment_transaction_id ); | ||
$order->save(); | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was getting Psalm UndefinedMethod linting error, so replaced the abstract class type hint with a list of possible concrete types. I wonder if there's a proper way to use abstract types as type hints without Psalm complaining.