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

Inconsistent handling of repeated card_present payment failures #10023

Open
staskus opened this issue Dec 20, 2024 · 2 comments · May be fixed by #10168
Open

Inconsistent handling of repeated card_present payment failures #10023

staskus opened this issue Dec 20, 2024 · 2 comments · May be fixed by #10168
Assignees
Labels

Comments

@staskus
Copy link

staskus commented Dec 20, 2024

Describe the bug

We have identified an issue with the handling of repeated card_present payment failures in WooPayments. Specifically, when a payment is retried for the same Payment Intent:

  • The webhook does not mark the order as failed since an identical order note already exists.
  • No failed email sent to customers

More context and discussion: pcreKM-3aC-p2 and #10050 (comment)

To Reproduce

Initial Payment Attempt:

  1. Use a physical test card and WooCommerce app to attempt a card_present payment with a designated decline response.
    Observe the webhook behavior:
  • Order status is set to "failed".
  • An order note is added:
A payment of $2.01 failed using WooPayments (pi_3QQ3LsFxOPlITqSQ18tFkImm). With the following message: Your card was declined.

Retry the Payment:

  1. Retry the payment for the same Payment Intent (pi_3QQ3LsFxOPlITqSQ18tFkImm).
    Observe the webhook behavior:
  • Order status remains unchanged.
  • No new order note is added, and no order failure actions are triggered.

Feel free to contact @staskus to help reproduce and test the issue.

Expected behavior

Each payment attempt (initial or retried) should trigger the appropriate order status update, notes, and failure handling, regardless of whether the Payment Intent or error message is identical.

Acceptance Criteria

(Written by @htdat)

  • Separate the webhook handling for card_present payment failure to a separate method.
    • In process_webhook_payment_intent_failed, check if the payment method type is CARD_PRESENT, branch it to use a new Order_Service method, something like mark_terminal_payment_failed (as we already have had mark_terminal_payment_completed).
  • In the new mark_terminal_payment_failed method, add a note for each failure with additional unique info such as charge_id or payment_method_id or timestamp in the human-readable format. See pcreKM-3aC-p2#comment-4188
  • Also in this new method, trigger do_action( 'woocommerce_order_status_failed action' ), and ensure a failure email sent to customers. See Handling card_present payment failures for already failed order #10050 (comment)

Additional context

@staskus
Copy link
Author

staskus commented Dec 23, 2024

There's also a related issue with order status. mark_payment_failed (and subsequent actions) are skipped when the order is already in failed status. Therefore, the client is required to make a remote request to change the order to pending before making a payment again which is inefficient. Could there be a special case handling for card_present payments in this case as well?

@htdat
Copy link
Member

htdat commented Jan 7, 2025

There's also a related issue with order status. mark_payment_failed (and subsequent actions) are skipped when the order is already in failed status. Therefore, the client is required to make a remote request to change the order to pending before making a payment again which is inefficient. Could there be a special case handling for card_present payments in this case as well?

For posterity, #10050 was created for this case but then was closed as we can resolve it within this issue with the updated acceptance criteria.

@zmaglica zmaglica self-assigned this Jan 15, 2025
@zmaglica zmaglica linked a pull request Jan 21, 2025 that will close this issue
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants