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

Store balance transaction ID in order metadata #7945

Merged
merged 21 commits into from
Jan 18, 2024
Merged
Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
f33af70
Streamline the params in attach_intent_info_to_order
anu-rock Dec 27, 2023
fbc8664
Save balance transaction id to order meta while processing a payment
anu-rock Dec 27, 2023
7f9fc28
Add changelog
anu-rock Dec 27, 2023
1bf3da7
Merge branch 'develop' into update/7856-add-charge-txn-id-to-order-meta
anu-rock Dec 27, 2023
83d5af8
Replace the value of balance transaction in create charge helper with…
anu-rock Dec 27, 2023
d73ab09
Fix psalm error UndefinedMethod
anu-rock Jan 2, 2024
5f1a297
Merge branch 'develop' into update/7856-add-charge-txn-id-to-order-meta
anu-rock Jan 2, 2024
8c2f80f
Update method call in the main gateway class
anu-rock Jan 2, 2024
1e58fcc
Merge branch 'develop' into update/7856-add-charge-txn-id-to-order-meta
mordeth Jan 8, 2024
a7bef87
Remove code duplication between the two attach intent methods
anu-rock Jan 11, 2024
adc9a67
Prepend wcpay to the meta key
anu-rock Jan 11, 2024
1f4681a
Remove the unnecessary variable in test
anu-rock Jan 11, 2024
69c82bd
Use null coalescing instead of ternary operator
anu-rock Jan 11, 2024
bb87366
Revert "Remove the unnecessary variable in test"
anu-rock Jan 11, 2024
4d9d729
Merge branch 'develop' into update/7856-add-charge-txn-id-to-order-meta
dpaun1985 Jan 15, 2024
e745e71
Prevent overwriting order meta after successful payment
anu-rock Jan 16, 2024
2af5d52
Add test for the new setter method
anu-rock Jan 16, 2024
8022a4a
Save payment transaction id in the new order service
anu-rock Jan 16, 2024
edd6872
Fix the undefined variable error
anu-rock Jan 16, 2024
16f83e8
Merge branch 'develop' into update/7856-add-charge-txn-id-to-order-meta
anu-rock Jan 18, 2024
d3d2655
Merge branch 'develop' into update/7856-add-charge-txn-id-to-order-meta
anu-rock Jan 18, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions includes/class-wc-payments-order-service.php
Original file line number Diff line number Diff line change
Expand Up @@ -773,12 +773,12 @@ 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_Abstract_Intention $intent The payment or setup intention object.
* @param WC_Order $order The order.
* @param WC_Payments_API_Payment_Intention|WC_Payments_API_Setup_Intention $intent The payment or setup intention object.
Copy link
Contributor Author

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.

*
* @throws Order_Not_Found_Exception
*/
public function attach_intent_info_to_order( WC_Order $order, WC_Payments_API_Abstract_Intention $intent ) {
public function attach_intent_info_to_order( WC_Order $order, $intent ) {
Copy link
Contributor

Choose a reason for hiding this comment

The 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 attach_intent_info_to_order__legacy .
This method extracts the data that needs to be added to the order, and saves it on the order. attach_intent_info_to_order__legacy only saves the information to the order.
We can keep this method to extract the data and call the attach_intent_info_to_order__legacy in this method.
So, we could:

  1. change this method name to add_intent_info_to_order
  2. keep the old name for attach_intent_info_to_order__legacy ( attach_intent_info_to_order )
  3. call attach_intent_info_to_order in this method

Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.

change this method name to add_intent_info_to_order

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();
Expand Down