Add pending refund notes #10600
Merged
Add pending refund notes #10600
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Test the buildOption 1. Jetpack Beta
Option 2. Jurassic Ninja - available for logged-in A12s🚀 Launch a JN site with this branch 🚀 ℹ️ Install this Tampermonkey script to get more options. Build info:
Note: the build is updated when a new commit is pushed to this PR. |
|
Size Change: 0 B Total Size: 1.27 MB ℹ️ View Unchanged
|
…ook_refund_updated`
- Remove the un-used code in a prev commit
|
FYI ready for the initial review. The unit tests are still in progress, but I would love to have initial feedback meanwhile. |
|
@htdat I noticed the following error while testing. It saw it when I tried to visit the order details page, after a checkout: I'm a bit confused, since the class should be available, given the call to |
mgascam
requested changes
Mar 24, 2025
There was a problem hiding this comment.
I think we are missing to include the new constant file for the refund status:
diff --git a/includes/class-wc-payments.php b/includes/class-wc-payments.php
index 41759213d..321b1f1b4 100644
--- a/includes/class-wc-payments.php
+++ b/includes/class-wc-payments.php
@@ -473,6 +473,7 @@ class WC_Payments {
include_once __DIR__ . '/exceptions/class-invalid-address-exception.php';
include_once __DIR__ . '/constants/class-base-constant.php';
include_once __DIR__ . '/constants/class-country-code.php';
+ include_once __DIR__ . '/constants/class-refund-status.php';
include_once __DIR__ . '/constants/class-country-test-cards.php';
include_once __DIR__ . '/constants/class-currency-code.php';
include_once __DIR__ . '/constants/class-fraud-meta-box-type.php';
mgascam
added
pr: needs author reply
and removed
pr: needs author reply
pr: needs review
labels
|
@mgascam - yeah, I missed adding the file into the include list. The PR is ready to review. I've added tests covering for the webhook processing change; but I will still need to add tests for changes in Order Service. In the meantime, you can review the code too :D Edit: with the most recent commit, all tests have been created and updated for changes. |
|
Hi @htdat I wanted to share one scenario I found while testing the PR. I did the checkout with the Asynchronous success card. The thing is my test account did not have balance to process the refund, so it was pending and I canceled it in the Stripe dashboard. This is the payment for your reference: https://dashboard.stripe.com/test/connect/accounts/acct_1PxRhsR4vt8oOW8F/payments/pi_3R6dCwR4vt8oOW8F1rjgCeW9. If you have a look, you can see that the last triggered event was Sorry It's a bit late for me to debug it and confirm my theory. I'll do it tomorrow but just wanted to share with you in case you can also have a look 🙏 |
htdat
force-pushed
the
fix/1195-pending-refund
branch
from
ef9653c to
0028acf
Compare
htdat
force-pushed
the
fix/1195-pending-refund
branch
from
aec38a9 to
d27e300
Compare
|
Thanks @mgascam. I handled that Basically, the |
mgascam
approved these changes
Mar 26, 2025
There was a problem hiding this comment.
Thanks for addressing my comments @htdat. The code changes look good to me. I appreciate the streamlined process as well of the addition of the refund status constant. I left a comment regarding the public docs url to double-check with you. I also saw an opportunity to enhance the refund failure reason in the order notes, but we can tackle it in a follow up issue.
Manual tests worked as intended for me. I tested both the success and failure flows from the Stripe dash, the Order admin and the timeline and did not find any issues. Great work!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1195
Changes proposed in this Pull Request
Changes based on this spike #1195 (comment):
add_note_and_metadata_for_refund: change its name and add one param$is_pendingtoadd_note_and_metadata_for_created_refundso that it can handle bothpendingandsucceedednote.process_refundwithwc_create_refund.pendingandsucceeded. The most notable change is onprocess_webhook_refund_updatedwhere I moved the handled the failed refund intoOrder_Service.Testing instructions
npm run changelogto add a changelog file, choosepatchto leave it empty if the change is not significant. You can add multiple changelog files in one PR by running this command a few times.Post merge
Important
Add documents for this new link https://woocommerce.com/document/woopayments/managing-money/#pending-refunds
Update: asked here p1743136317673289-slack-C03EE4CPPK5