Add remediation tool to fix incorrect analytics data from canceled authorizations #11140
Conversation
mgascam
force-pushed
the
cancel-auth-fee-remediation
branch
from
baba383 to
e41ef6a
Compare
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: 876 kB ℹ️ View Unchanged
|
There was a problem hiding this comment.
Pull Request Overview
This PR implements a data remediation migration to fix incorrect transaction fees and refund records that were created for canceled payment authorizations. The bug affected orders between April 2023 and November 2025 where canceled authorizations incorrectly had transaction fees and refund objects created despite no actual payment occurring.
Key Changes
- New migration class that processes affected orders in adaptive batches using Action Scheduler
- Integration with plugin upgrade hooks to automatically schedule remediation when updating from affected versions
- Comprehensive test coverage for the remediation logic and batch processing
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
includes/migrations/class-wc-payments-remediate-canceled-auth-fees.php |
Core migration class implementing batch processing, order remediation, and adaptive batch sizing |
includes/class-wc-payments.php |
Integration of remediation into plugin initialization and update hooks |
tests/unit/migrations/test-class-wc-payments-remediate-canceled-auth-fees.php |
Comprehensive unit tests covering all migration functionality |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
includes/migrations/class-wc-payments-remediate-canceled-auth-fees.php
Outdated
Show resolved
Hide resolved
includes/migrations/class-wc-payments-remediate-canceled-auth-fees.php
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
includes/migrations/class-wc-payments-remediate-canceled-auth-fees.php
Outdated
Show resolved
Hide resolved
includes/migrations/class-wc-payments-remediate-canceled-auth-fees.php
Outdated
Show resolved
Hide resolved
includes/migrations/class-wc-payments-remediate-canceled-auth-fees.php
Outdated
Show resolved
Hide resolved
includes/migrations/class-wc-payments-remediate-canceled-auth-fees.php
Outdated
Show resolved
Hide resolved
includes/migrations/class-wc-payments-remediate-canceled-auth-fees.php
Outdated
Show resolved
Hide resolved
…mprove unit tests
…ynamic button text and status description
… to 'cancelled' and add corresponding unit tests
… remediation process
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
includes/admin/class-wc-payments-admin-notice-canceled-auth-remediation.php
Outdated
Show resolved
Hide resolved
…emediation and add new inbox note
…s and correct post type placeholder
mgascam
changed the title
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Hi @elazzabi I requested your review because you worked on the fix and have some context already. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared | ||
| $order_ids = $wpdb->get_col( $wpdb->prepare( $sql, $params ) ); | ||
|
|
There was a problem hiding this comment.
[nitpick] The phpcs:ignore directive is too broad. While the SQL is properly prepared with $wpdb->prepare(), the ignore comment covers the entire line. Consider using a more specific phpcs directive or restructuring to avoid the need for ignoring.
| // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared | |
| $order_ids = $wpdb->get_col( $wpdb->prepare( $sql, $params ) ); | |
| $order_ids = $wpdb->get_col( $wpdb->prepare( $sql, $params ) ); // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- $sql is properly prepared above |
| $order->save(); | ||
|
|
||
| // Trigger analytics sync to update wc_order_stats table. This is necessary because | ||
| // WooCommerce doesn't automatically sync when refunds are deleted (see issue #1073). |
There was a problem hiding this comment.
[nitpick] The comment references "issue #1073" but doesn't provide context about which repository this issue is from. The @see tag on line 602 points to a woocommerce-admin GitHub issue, but the inline comment should clarify this is a WooCommerce core issue for better maintainability.
| // WooCommerce doesn't automatically sync when refunds are deleted (see issue #1073). | |
| // WooCommerce doesn't automatically sync when refunds are deleted (see WooCommerce core issue #1073: https://github.com/woocommerce/woocommerce/issues/1073). |
| $placeholders = implode( ', ', array_fill( 0, count( $refund_ids ), '%d' ) ); | ||
|
|
||
| // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared, WordPress.DB.PreparedSQLPlaceholders.UnfinishedPrepare | ||
| $wpdb->query( $wpdb->prepare( "DELETE FROM {$wpdb->prefix}wc_order_stats WHERE order_id IN ({$placeholders})", $refund_ids ) ); |
There was a problem hiding this comment.
[nitpick] The SQL query uses dynamic placeholder interpolation which triggers WordPress.DB.PreparedSQLPlaceholders.UnfinishedPrepare. While the code is safe (placeholders are generated correctly with array_fill and values are passed to prepare), consider extracting placeholder generation to a separate variable before the query to improve code clarity:
$placeholders = implode( ', ', array_fill( 0, count( $refund_ids ), '%d' ) );
$sql = "DELETE FROM {$wpdb->prefix}wc_order_stats WHERE order_id IN ({$placeholders})";
$wpdb->query( $wpdb->prepare( $sql, $refund_ids ) );| include_once WCPAY_ABSPATH . 'includes/migrations/class-wc-payments-remediate-canceled-auth-fees.php'; | ||
| $remediation = new WC_Payments_Remediate_Canceled_Auth_Fees(); | ||
| return $remediation->has_affected_orders(); |
There was a problem hiding this comment.
The has_affected_orders() method instantiates the remediation class every time it's called, which may trigger database queries to check for affected orders. If can_be_added() is called frequently (e.g., on every admin page load), this could cause performance issues. Consider caching the result or implementing a lighter-weight check.
| include_once WCPAY_ABSPATH . 'includes/migrations/class-wc-payments-remediate-canceled-auth-fees.php'; | |
| $remediation = new WC_Payments_Remediate_Canceled_Auth_Fees(); | |
| return $remediation->has_affected_orders(); | |
| static $result = null; | |
| if ( null !== $result ) { | |
| return $result; | |
| } | |
| include_once WCPAY_ABSPATH . 'includes/migrations/class-wc-payments-remediate-canceled-auth-fees.php'; | |
| $remediation = new WC_Payments_Remediate_Canceled_Auth_Fees(); | |
| $result = $remediation->has_affected_orders(); | |
| return $result; |
| ); | ||
|
|
||
| // Verify entries exist. | ||
| $count_before = $wpdb->get_var( "SELECT COUNT(*) FROM {$wpdb->prefix}wc_order_stats WHERE order_id IN (99991, 99992)" ); |
There was a problem hiding this comment.
[nitpick] The SQL query is not using prepared statements with placeholders. The hardcoded order IDs (99991, 99992) are safe in this test context, but this represents a SQL injection vulnerability pattern if copied elsewhere. Consider using $wpdb->prepare() even in tests to demonstrate best practices:
$count_before = $wpdb->get_var( $wpdb->prepare( "SELECT COUNT(*) FROM {$wpdb->prefix}wc_order_stats WHERE order_id IN (%d, %d)", 99991, 99992 ) );| // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared | ||
| $order_ids = $wpdb->get_col( $wpdb->prepare( $sql, $params ) ); |
There was a problem hiding this comment.
[nitpick] The phpcs:ignore directive is too broad here as well. The SQL is properly prepared with $wpdb->prepare(), but the ignore comment should be more specific about which rule is being ignored and why.
| // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared | |
| $order_ids = $wpdb->get_col( $wpdb->prepare( $sql, $params ) ); | |
| $order_ids = $wpdb->get_col( $wpdb->prepare( $sql, $params ) ); // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- SQL is properly prepared with $wpdb->prepare() |
| 'name' => __( 'Fix canceled authorization analytics', 'woocommerce-payments' ), | ||
| 'button' => $this->get_remediation_button_text(), | ||
| 'desc' => $this->get_remediation_description(), | ||
| 'confirm' => __( 'This will update order metadata and delete incorrect refund records for affected orders. Make sure you have a recent backup before proceeding. Continue?', 'woocommerce-payments' ), |
There was a problem hiding this comment.
[nitpick] The confirmation message should include a warning about the potential impact on analytics reporting. While the message mentions updating metadata and deleting refund records, it doesn't explicitly warn that this will affect historical WooCommerce Analytics data, which merchants should be aware of before proceeding.
| 'confirm' => __( 'This will update order metadata and delete incorrect refund records for affected orders. Make sure you have a recent backup before proceeding. Continue?', 'woocommerce-payments' ), | |
| 'confirm' => __( 'This will update order metadata and delete incorrect refund records for affected orders. This will affect historical WooCommerce Analytics data. Make sure you have a recent backup before proceeding. Continue?', 'woocommerce-payments' ), |
| $reflection->invoke( $this->remediation, [ 99991, 99992 ] ); | ||
|
|
||
| // Verify entries are deleted. | ||
| $count_after = $wpdb->get_var( "SELECT COUNT(*) FROM {$wpdb->prefix}wc_order_stats WHERE order_id IN (99991, 99992)" ); |
There was a problem hiding this comment.
[nitpick] Same issue here - the SQL query should use prepared statements even in tests to demonstrate best practices and avoid establishing patterns that could lead to SQL injection vulnerabilities if copied to production code.
Summary
Fixes WOOPMNT-5495
Adds a remediation tool to fix incorrect analytics data caused by a bug that existed between April 2023 and November 2025. When payment authorizations were canceled (not captured), the system incorrectly:
_wcpay_transaction_feeand_wcpay_netmetadata)This caused negative values in WooCommerce Analytics for affected stores using manual capture.
Changes
includes/migrations/class-wc-payments-remediate-canceled-auth-fees.phpincludes/notes/class-wc-payments-notes-canceled-auth-remediation.phpincludes/class-wc-payments.phpincludes/class-wc-payments-status.phptests/unit/migrations/test-class-wc-payments-remediate-canceled-auth-fees.phptests/unit/notes/test-class-wc-payments-notes-canceled-auth-remediation.phpHow It Works
_intention_status = canceledcreated after April 2023 that have incorrect fee metadata, WCPay refunds, or "refunded" statuswp_wc_order_statstable and triggers WooCommerce Analytics syncSafety Measures
_intention_status = canceled_wcpay_refund_idmetadata (preserves manual refunds)wc_get_logger()Testing Instructions
Setup - Create test data
wp eval-file create-test-data-canceled-auth.phpVerify the problem exists
Run the remediation
Verify the fix
Test scenarios
Cleanup
npm run changelogto add a changelog filePost merge