From 61ea299b0ef1cf52bf8c8cd0a15e8d4765529bfa Mon Sep 17 00:00:00 2001 From: Twooey Date: Fri, 28 Aug 2026 17:18:54 -0400 Subject: [PATCH] Fix Stripe authorization lifecycle timing --- README.md | 10 +- .../includes/Order/AuditTrail.php | 18 +-- .../includes/Order/SupplierOrderJob.php | 23 +++- .../tests/Unit/Order/SupplierOrderJobTest.php | 119 ++++++++++++++++++ .../bookstore-core/tests/bootstrap.php | 6 + 5 files changed, 157 insertions(+), 19 deletions(-) create mode 100644 wp-content/plugins/bookstore-core/tests/Unit/Order/SupplierOrderJobTest.php diff --git a/README.md b/README.md index 4765b11..0408027 100644 --- a/README.md +++ b/README.md @@ -219,10 +219,12 @@ card is actually charged for the first time here) before moving to no sellable offer, margin no longer holds, fraud check fails, or the supplier submission itself fails — lands the order in `bsc-exception` ("Needs Review"), which is the review queue: just a filterable -WooCommerce order status, not a separate table. Because nothing is -charged until capture, most failures are a **void** (`AuditTrail::record_void()`, -zero cost) rather than a refund — a refund only happens for the rarer -case of something going wrong *after* a real capture. +WooCommerce order status, not a separate table. Because nothing is charged +until capture, most failures request a zero-cost authorization release rather +than a refund. They remain in **Needs Review** until the Stripe order note or +dashboard confirms release; only then should `AuditTrail::record_void()` be +recorded. A refund is reserved for the rarer case of something going wrong +*after* a real capture. `wp bookstore force-offer-status ` (see `wp bookstore help`) forces a specific ISBN's synthetic offer into a diff --git a/wp-content/plugins/bookstore-core/includes/Order/AuditTrail.php b/wp-content/plugins/bookstore-core/includes/Order/AuditTrail.php index b78acc8..c6f95a4 100644 --- a/wp-content/plugins/bookstore-core/includes/Order/AuditTrail.php +++ b/wp-content/plugins/bookstore-core/includes/Order/AuditTrail.php @@ -7,10 +7,11 @@ defined('ABSPATH') || exit; /** * bsc_order_audit: one row per order line, written once at checkout * (CheckoutRecheck::run_checks()) and updated once — either on refund - * (record_refund(), a post-capture reversal) or on void (record_void(), - * a pre-capture supplier-order failure — the far more common case under - * the preauth-then-capture flow). Style mirrors Catalog\SupplierOffer - * exactly. + * (record_refund(), a post-capture reversal) or after a confirmed void + * (record_void()). Requesting cancellation alone is not confirmation: the + * Stripe status-change hook exposes no success result, so callers must + * reconcile the order note/dashboard before recording a void. Style mirrors + * Catalog\SupplierOffer exactly. */ class AuditTrail { @@ -68,7 +69,7 @@ class AuditTrail * against the expectation this row already captured at checkout time. * Only reachable now for a refund *after* a real capture (e.g. a * manual admin refund post-fulfillment) — a failed supplier order - * before capture goes through record_void() instead, below. + * before capture uses record_void() only after Stripe confirms release. */ public static function record_refund(int $order_id): void { @@ -83,10 +84,9 @@ class AuditTrail } /** - * Preauth-then-capture rework: a failed supplier order before capture - * reverses the hold (Order\SupplierOrderJob::fail_and_release()) - * rather than refunding a charge that was never made — genuinely - * zero cost, not the processing-fee loss record_refund() models. + * A confirmed pre-capture authorization release is genuinely zero cost, + * unlike the processing-fee loss record_refund() models. Do not call this + * merely because cancellation was requested. */ public static function record_void(int $order_id): void { diff --git a/wp-content/plugins/bookstore-core/includes/Order/SupplierOrderJob.php b/wp-content/plugins/bookstore-core/includes/Order/SupplierOrderJob.php index 5184489..d67a3fe 100644 --- a/wp-content/plugins/bookstore-core/includes/Order/SupplierOrderJob.php +++ b/wp-content/plugins/bookstore-core/includes/Order/SupplierOrderJob.php @@ -28,17 +28,20 @@ class SupplierOrderJob { public static function register(): void { - add_action('woocommerce_order_status_on-hold', [self::class, 'accept_stripe_authorization']); + // Stripe fires this only after it has persisted the transaction ID, + // captured flag, and On hold status. The generic On hold transition + // fires too early inside Stripe's process_response() and can race the + // save of `_stripe_charge_captured=no`. + add_action('wc_gateway_stripe_process_response', [self::class, 'accept_stripe_authorization'], 10, 2); add_action('woocommerce_order_status_' . OrderStatuses::AUTHORIZED, [self::class, 'schedule_for_order']); add_action('bsc_submit_supplier_order', [self::class, 'run']); } /** Promote only manually authorized orders from the official Stripe gateway. */ - public static function accept_stripe_authorization(int $order_id): void + public static function accept_stripe_authorization(object $response, \WC_Order $order): void { - $order = wc_get_order($order_id); if ( - !$order + !$order->has_status('on-hold') || !self::is_stripe_order($order) || $order->get_meta('_stripe_charge_captured') !== 'no' ) { @@ -160,9 +163,17 @@ class SupplierOrderJob */ private static function fail_and_release(\WC_Order $order, string $reason): void { + $order->update_meta_data('_bsc_stripe_release_requested', 'yes'); + $order->save(); $order->update_status('cancelled', $reason . ' Releasing the Stripe authorization.'); - AuditTrail::record_void($order->get_id()); - $order->update_status(OrderStatuses::EXCEPTION, 'Stripe authorization release requested; order requires review.'); + // Stripe's cancelled-status handler performs the API call but exposes + // no success result to third-party callbacks. Do not record a + // confirmed zero-cost void here; a failed API call must remain visible + // for reconciliation instead of being reported as successful. + $order->update_status( + OrderStatuses::EXCEPTION, + 'Stripe authorization release requested; verify the Stripe order note or dashboard before closing review.' + ); } private static function is_stripe_order(\WC_Order $order): bool diff --git a/wp-content/plugins/bookstore-core/tests/Unit/Order/SupplierOrderJobTest.php b/wp-content/plugins/bookstore-core/tests/Unit/Order/SupplierOrderJobTest.php new file mode 100644 index 0000000..fc7fbf6 --- /dev/null +++ b/wp-content/plugins/bookstore-core/tests/Unit/Order/SupplierOrderJobTest.php @@ -0,0 +1,119 @@ +with([SupplierOrderJob::class, 'accept_stripe_authorization'], 10, 2) + ->once(); + Actions\expectAdded('woocommerce_order_status_' . OrderStatuses::AUTHORIZED)->once(); + Actions\expectAdded('bsc_submit_supplier_order')->once(); + + SupplierOrderJob::register(); + } + + public function test_promotes_a_saved_uncaptured_stripe_authorization(): void + { + Functions\expect('apply_filters')->once()->andReturn([ + 'pass' => true, + 'reason' => 'test pass', + ]); + $order = Mockery::mock(\WC_Order::class); + $order->shouldReceive('has_status')->with('on-hold')->once()->andReturn(true); + $order->shouldReceive('get_payment_method')->once()->andReturn('stripe'); + $order->shouldReceive('get_meta')->with('_stripe_charge_captured')->once()->andReturn('no'); + $order->shouldReceive('add_order_note')->once(); + $order->shouldReceive('update_status') + ->with(OrderStatuses::AUTHORIZED, Mockery::type('string')) + ->once(); + + SupplierOrderJob::accept_stripe_authorization((object) ['captured' => false], $order); + } + + public function test_ignores_an_on_hold_stripe_order_without_persisted_uncaptured_state(): void + { + $order = Mockery::mock(\WC_Order::class); + $order->shouldReceive('has_status')->with('on-hold')->once()->andReturn(true); + $order->shouldReceive('get_payment_method')->once()->andReturn('stripe'); + $order->shouldReceive('get_meta')->with('_stripe_charge_captured')->once()->andReturn(''); + $order->shouldNotReceive('update_status'); + + SupplierOrderJob::accept_stripe_authorization((object) [], $order); + } + + public function test_ignores_non_authorization_stripe_responses(): void + { + $order = Mockery::mock(\WC_Order::class); + $order->shouldReceive('has_status')->with('on-hold')->once()->andReturn(false); + $order->shouldNotReceive('get_payment_method'); + $order->shouldNotReceive('get_meta'); + $order->shouldNotReceive('update_status'); + + SupplierOrderJob::accept_stripe_authorization((object) ['captured' => true], $order); + } + + public function test_capture_success_requires_stripes_persisted_confirmation(): void + { + $order = Mockery::mock(\WC_Order::class); + $order->shouldReceive('update_status')->with('processing', Mockery::type('string'))->once(); + $order->shouldReceive('get_id')->once()->andReturn(42); + + $reloaded = Mockery::mock(\WC_Order::class); + $reloaded->shouldReceive('get_meta')->with('_stripe_charge_captured')->once()->andReturn('yes'); + $reloaded->shouldReceive('update_status') + ->with(OrderStatuses::SUPPLIER_ORDERED, Mockery::type('string')) + ->once(); + Functions\expect('wc_get_order')->with(42)->once()->andReturn($reloaded); + + $this->invokePrivate('capture_and_complete', $order); + } + + public function test_capture_without_stripe_confirmation_routes_to_review(): void + { + $order = Mockery::mock(\WC_Order::class); + $order->shouldReceive('update_status')->with('processing', Mockery::type('string'))->once(); + $order->shouldReceive('get_id')->once()->andReturn(43); + + $reloaded = Mockery::mock(\WC_Order::class); + $reloaded->shouldReceive('get_meta')->with('_stripe_charge_captured')->once()->andReturn('no'); + $reloaded->shouldReceive('update_status') + ->with(OrderStatuses::EXCEPTION, Mockery::type('string')) + ->once(); + Functions\expect('wc_get_order')->with(43)->once()->andReturn($reloaded); + + $this->invokePrivate('capture_and_complete', $order); + } + + public function test_release_is_recorded_as_requested_not_as_a_confirmed_void(): void + { + $order = Mockery::mock(\WC_Order::class); + $order->shouldReceive('update_meta_data') + ->with('_bsc_stripe_release_requested', 'yes') + ->once(); + $order->shouldReceive('save')->once(); + $order->shouldReceive('update_status')->with('cancelled', Mockery::type('string'))->once(); + $order->shouldReceive('update_status') + ->with(OrderStatuses::EXCEPTION, Mockery::on( + static fn (string $note): bool => str_contains($note, 'verify') + )) + ->once(); + + $this->invokePrivate('fail_and_release', $order, 'Unavailable.'); + } + + private function invokePrivate(string $method, mixed ...$arguments): mixed + { + $reflection = new \ReflectionMethod(SupplierOrderJob::class, $method); + return $reflection->invoke(null, ...$arguments); + } +} diff --git a/wp-content/plugins/bookstore-core/tests/bootstrap.php b/wp-content/plugins/bookstore-core/tests/bootstrap.php index 1c96eba..3308846 100644 --- a/wp-content/plugins/bookstore-core/tests/bootstrap.php +++ b/wp-content/plugins/bookstore-core/tests/bootstrap.php @@ -27,6 +27,12 @@ if (!class_exists(\WC_Cart::class)) { } } +if (!class_exists(\WC_Order::class)) { + class WC_Order + { + } +} + if (!class_exists(\WP_Error::class)) { class WP_Error {