Commit 2471885b7da for woocommerce
commit 2471885b7da34b747e00cd41b5a902b166f58536
Author: Oleksandr Aratovskyi <79862886+oaratovskyi@users.noreply.github.com>
Date: Wed Jun 24 15:17:53 2026 +0300
Fix WooPayments activation after test account disable (#65895)
* fix: avoid stale WooPayments onboarding lock success
* chore: add changelog for WooPayments activation fix
* chore: Clarify WooPayments test account disable setup
Context: disable_test_account now prepares the WooPayments request before Phase 2 runs it outside the Core onboarding lock.
Problem: stale comments, dead initializers, and unnecessary WPCOM test setup made the two-phase flow harder to read.
Solution: reword the Phase-1 comments, document the Phase-2 idempotency assumption, remove dead initializers, and remove misleading WPCOM connection setup from the focused tests.
Refs WOOPRD-3553
* fix: Avoid clearing concurrent onboarding locks
Context: disable_test_account releases the Core onboarding lock before calling the internal WooPayments endpoint.
Problem: the Phase-2 finalizer wrote the shared token-less lock option back to 0 unconditionally, which could clear a concurrent request's lock.
Solution: rely on the Phase-1 clear, webhook delete behavior, and lock TTL, and update the tests to expect no trailing Phase-2 normalization write.
Refs WOOPRD-3553
* test: Reuse WooPayments disable option-state helper
Context: the disable_test_account tests share a helper for NOX lock and profile option mocks.
Problem: one webhook-delete test still duplicated that option mocking, leaving two copies of the same test setup semantics.
Solution: route the test through mock_disable_test_account_option_state() while preserving its webhook lock-delete assertions.
Refs WOOPRD-3553
* refactor: Extract shared WooPayments onboarding exception helper
The two-phase split of disable_test_account() introduced a second,
structurally identical exception-to-WP_Error catch block within the
same method. Two copies of the same error code, message shape, and
data array risk drifting apart if either is edited in isolation.
Extract the conversion into get_onboarding_client_api_exception_error()
and call it from both the preparation and the internal-request phases,
correcting the copied catch comments that no longer described what the
blocks do.
* docs: Document WooPayments disable Phase 2 concurrency safety
The previous inline note flagged Phase 2's idempotency assumption as
"unverified," leaving an open unknown in shipped code. Verification
against the WooPayments client (disable_test_drive_account in
class-wc-payments-onboarding-service.php) shows the endpoint is not
idempotent but fails safe: it overwrites its account cache before the
delete, so a duplicate concurrent call short-circuits on
is_stripe_connected() === false and returns a benign "account does not
exist" error rather than re-deleting.
Replace the unverified note with that verified behavior and record that
request-scoped locking is deferred to the broader shared-lock contract.
* test: Clean up WooPayments test-account disable tests
Address review feedback on the disable_test_account test coverage:
- Remove mock_working_wpcom_connection(), a helper left without any
call sites after the two-phase tests stopped using it.
- Make the lock-change assertion faithful: the account.deleted webhook
calls delete_option, which Core's update_option tracker never sees,
so assert only Core's two writes ([set, clear]) and check the deleted
option value separately.
- Rename the unexpected-error test to reflect that the lock is cleared
in Phase 1 before the internal call, not as a result of the throw,
and note that the fatal Error propagates past Phase 2's Exception
catch by design.
* refactor: Deduplicate WooPayments test account disable error handling
Code review follow-ups on disable_test_account():
- Build the Phase 2 request params once instead of repeating the identical
array in both the test-drive and sandbox branches.
- Reuse a single error-message variable across the Phase 1 and Phase 2
exception handlers so the wording can only drift in one place.
- Correct the Phase 2 comment: a duplicate concurrent call is guarded
against re-deleting the account, but it still surfaces to the second
caller as a hard ApiException( FAILED_DEPENDENCY ) via the is_wp_error()
check below, not a benign "account does not exist" no-op.
* test: Strengthen WooPayments test account disable suite
Code review follow-ups on the disable_test_account test suite:
- Add coverage for the sandbox account path (routes to the onboarding
reset endpoint) and the no-account no-op (makes no internal request and
still succeeds), both asserting the Phase 1/Phase 2 lock ordering.
- Drop a dead $account_exists assignment that no assertion read.
- Replace a tautological getMessage() assertion with a check that the lock
is already cleared when a Phase 2 \Error propagates.
- Restore the mock_working_wpcom_connection() helper, now used by the new
success-path tests.
* docs: Mark WooPayments disable test helper params as by-reference
The mock_account_state() and mock_disable_test_account_option_state()
helpers take several parameters by reference, but their @param tags
documented them as by-value. Add the & prefix so the docblocks match the
signatures and the by-reference contract is clear to future callers.
* docs: Warn that WooPayments clear_onboarding_lock has no ownership check
clear_onboarding_lock() writes 0 unconditionally with no per-request
token, so it clears whatever lock is currently set — including one a
concurrent request just acquired. Document that it must only release a
lock the same request set (e.g. the finally paired with
set_onboarding_lock()) and never after work that runs unlocked, so future
callers don't rediscover the sharp edge. Safe concurrent release would
need a compare-and-swap token, deferred to the broader shared-lock work.
* test: Close WooPayments disable_test_account coverage gaps
Code review follow-ups filling two gaps in the disable_test_account suite:
- Add the test-drive success path (only the sandbox success path was
covered): a connected test-drive account routes to the test-drive disable
endpoint, not the onboarding reset endpoint, and marks NOX steps
completed on success.
- Guard the Phase 2 lock-write invariant: the internal call and the
post-call step bookkeeping must not write the shared onboarding lock
again. Re-introducing an unconditional Phase 2 clear_onboarding_lock()
(which would stomp a concurrent request's lock) appends a third lock
write and fails the test.
* refactor: Reuse WooPayments onboarding exception helper for handlers
Context: get_onboarding_client_api_exception_error() was extracted to dedupe the
exception-to-WP_Error conversion in disable_test_account(), but four sibling
onboarding handlers still built the same WP_Error inline.
Problem: onboarding_test_account_init(), get_onboarding_kyc_session(),
finish_onboarding_kyc_session(), and reset_onboarding() each duplicated the
identical error code and data shape, which can drift from the helper over time.
Solution: route all four catch blocks through the shared helper. The helper
still returns the same error code and data array, so behavior is unchanged.
Refs WOOPRD-3553
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix: Drop exception stack traces from WooPayments onboarding errors
Context: get_onboarding_client_api_exception_error() builds the WP_Error for
internal onboarding API exceptions. Its data array is forwarded to ApiException
additional data (documented as exposable in the error response) and is persisted
to the NOX profile option by the onboarding step-failed handlers
(onboarding_test_account_init, get_onboarding_kyc_session,
finish_onboarding_kyc_session).
Problem: the data included the full exception stack trace via getTrace().
Stack frame arguments can carry file paths, tokens, and other sensitive values,
so the trace is unsafe to persist or expose. It was never read by any consumer.
Solution: stop storing the trace; keep only the bounded error code and message.
No observability is lost because nothing consumed the trace. A comment documents
the deliberate omission so it is not re-introduced.
Refs WOOPRD-3553
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test: Improve WooPayments disable_test_account test fidelity
PR review surfaced three test-quality gaps in the disable_test_account
coverage that could let real regressions slip through:
- The lock-failure test mocked an HTTP 409 / onboarding_locked response
the WooPayments client cannot emit (its disable endpoint always
returns HTTP 400 / bad_request). The mock now reflects what the client
actually returns, so the test documents the real concurrent-call
outcome instead of an impossible one.
- mock_account_state took its connected flag by reference, implying a
mid-test mutation capability no caller uses. Switched to pass-by-value
to make the helper's contract honest.
- The two success-path tests only asserted the NOX profile was
non-empty, so dropping a step write would go unnoticed. They now
assert both the payment_methods and test_account steps are recorded
as completed.
Tests only; no production behavior change.
* docs: Note TTL self-heal as a safe clear_onboarding_lock caller
The clear_onboarding_lock() warning said the method must only release a lock
the same request set, but is_onboarding_locked() also calls it to self-heal a
lock that has exceeded its TTL — a caller the warning did not acknowledge,
leaving the docblock internally inconsistent.
Document the TTL self-heal as a second case where clearing is safe regardless
of ownership, since a stale timestamp means no live request is expected to
still hold the lock.
Refs WOOPRD-3553
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test: Assert Phase 2 disable payload params, not just endpoint
The test-drive and sandbox success-path tests only checked which endpoint Phase
2 hit, so a regression that routed correctly but dropped or swapped the request
payload would have passed unnoticed.
Capture the request params alongside the endpoint and assert the forwarded
"from" and the validated source. "test-source" is not whitelisted, so the
service normalizes it to the default via validate_onboarding_source(); assert
that normalized value rather than the raw input.
Refs WOOPRD-3553
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix: Skip onboarding lock re-check in post-disable bookkeeping
The two-phase test-account disable releases Core's shared NOX lock before the
internal WooPayments mutation, so Phase 2 runs unlocked. The post-disable
bookkeeping then called mark_onboarding_step_completed(), which re-runs the
full onboarding acceptance checks — including the shared lock check. A
concurrent request that acquired the lock during that unlocked window would
turn an already-committed account mutation into an onboarding-locked (409)
error during cleanup, re-creating the first-click activation failure this work
set out to fix.
Extract the post-check body of mark_onboarding_step_completed() into a private
record_onboarding_step_completed() that records the step without re-checking
whether a new onboarding action is allowed, and use it for the post-disable
bookkeeping. The mutation has already committed, so this is internal state
sync rather than a new user action — mirroring mark_onboarding_step_failed()
and mark_onboarding_step_blocked(), which already skip those checks.
Refs WOOPRD-3553
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test: Track NOX profile writes explicitly in disable mock helper
mock_disable_test_account_option_state() inferred whether the NOX profile
option had been written from "! empty( $updated_profile )", so an intentional
write of an empty profile was indistinguishable from never having been written
and reads would silently fall back to the stored profile. No current test hits
this, but it left the mock able to mask a regression in a future
profile-clearing path.
Track writes with an explicit flag instead, so a written-empty profile is
returned as the written value rather than falling back to the stored one.
Refs WOOPRD-3553
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
diff --git a/plugins/woocommerce/changelog/fix-woopayments-activate-payments-lock b/plugins/woocommerce/changelog/fix-woopayments-activate-payments-lock
new file mode 100644
index 00000000000..11934dba0ca
--- /dev/null
+++ b/plugins/woocommerce/changelog/fix-woopayments-activate-payments-lock
@@ -0,0 +1,4 @@
+Significance: patch
+Type: fix
+
+Fix the WooPayments test account activation flow requiring a second click when switching to live payments.
diff --git a/plugins/woocommerce/src/Internal/Admin/Settings/PaymentsProviders/WooPayments/WooPaymentsService.php b/plugins/woocommerce/src/Internal/Admin/Settings/PaymentsProviders/WooPayments/WooPaymentsService.php
index 77c0403359d..9bac46517d0 100644
--- a/plugins/woocommerce/src/Internal/Admin/Settings/PaymentsProviders/WooPayments/WooPaymentsService.php
+++ b/plugins/woocommerce/src/Internal/Admin/Settings/PaymentsProviders/WooPayments/WooPaymentsService.php
@@ -472,6 +472,31 @@ class WooPaymentsService {
public function mark_onboarding_step_completed( string $step_id, string $location, bool $overwrite = false, ?string $source = self::SESSION_ENTRY_DEFAULT ): bool {
$this->check_if_onboarding_step_action_is_acceptable( $step_id, $location );
+ return $this->record_onboarding_step_completed( $step_id, $location, $overwrite, $source );
+ }
+
+ /**
+ * Record an onboarding step as completed without re-checking whether a new onboarding action is allowed.
+ *
+ * This is for internal use only, by callers that have already committed an account mutation and
+ * just need to sync the resulting NOX onboarding state. Unlike mark_onboarding_step_completed(),
+ * it deliberately skips check_if_onboarding_step_action_is_acceptable() — including the shared
+ * onboarding lock check — because the state change is the consequence of work that already
+ * happened, not a new user action. Re-checking the lock here would let a concurrent request that
+ * acquired the lock during an unlocked phase turn an already-successful mutation into an
+ * onboarding-locked error. This mirrors mark_onboarding_step_failed() and
+ * mark_onboarding_step_blocked(), which skip the same checks for the same reason.
+ *
+ * @param string $step_id The ID of the onboarding step.
+ * @param string $location The location for which we are onboarding.
+ * This is an ISO 3166-1 alpha-2 country code.
+ * @param bool $overwrite Whether to overwrite the step status if it is already completed and update the timestamp.
+ * @param string|null $source Optional. The source for the current onboarding flow.
+ * If not provided, it will identify the source as the WC Admin Payments settings.
+ *
+ * @return bool Whether the onboarding step was marked as completed.
+ */
+ private function record_onboarding_step_completed( string $step_id, string $location, bool $overwrite = false, ?string $source = self::SESSION_ENTRY_DEFAULT ): bool {
// Clear possible failed status for the step.
$this->clear_onboarding_step_failed( $step_id, $location );
@@ -1001,14 +1026,9 @@ class WooPaymentsService {
);
} catch ( Exception $e ) {
// Catch any exceptions to allow for proper error handling and onboarding unlock.
- $response = new WP_Error(
- 'woocommerce_woopayments_onboarding_client_api_exception',
- esc_html__( 'An unexpected error happened while initializing the test account.', 'woocommerce' ),
- array(
- 'code' => $e->getCode(),
- 'message' => $e->getMessage(),
- 'trace' => $e->getTrace(),
- )
+ $response = $this->get_onboarding_client_api_exception_error(
+ $e,
+ esc_html__( 'An unexpected error happened while initializing the test account.', 'woocommerce' )
);
}
@@ -1138,14 +1158,9 @@ class WooPaymentsService {
);
} catch ( Exception $e ) {
// Catch any exceptions to allow for proper error handling and onboarding unlock.
- $response = new WP_Error(
- 'woocommerce_woopayments_onboarding_client_api_exception',
- esc_html__( 'An unexpected error happened while creating the KYC session.', 'woocommerce' ),
- array(
- 'code' => $e->getCode(),
- 'message' => $e->getMessage(),
- 'trace' => $e->getTrace(),
- )
+ $response = $this->get_onboarding_client_api_exception_error(
+ $e,
+ esc_html__( 'An unexpected error happened while creating the KYC session.', 'woocommerce' )
);
}
@@ -1251,14 +1266,9 @@ class WooPaymentsService {
);
} catch ( Exception $e ) {
// Catch any exceptions to allow for proper error handling and onboarding unlock.
- $response = new WP_Error(
- 'woocommerce_woopayments_onboarding_client_api_exception',
- esc_html__( 'An unexpected error happened while finalizing the KYC session.', 'woocommerce' ),
- array(
- 'code' => $e->getCode(),
- 'message' => $e->getMessage(),
- 'trace' => $e->getTrace(),
- )
+ $response = $this->get_onboarding_client_api_exception_error(
+ $e,
+ esc_html__( 'An unexpected error happened while finalizing the KYC session.', 'woocommerce' )
);
}
@@ -1426,14 +1436,9 @@ class WooPaymentsService {
}
} catch ( Exception $e ) {
// Catch any exceptions to allow for proper error handling and onboarding unlock.
- $response = new WP_Error(
- 'woocommerce_woopayments_onboarding_client_api_exception',
- esc_html__( 'An unexpected error happened while resetting onboarding.', 'woocommerce' ),
- array(
- 'code' => $e->getCode(),
- 'message' => $e->getMessage(),
- 'trace' => $e->getTrace(),
- )
+ $response = $this->get_onboarding_client_api_exception_error(
+ $e,
+ esc_html__( 'An unexpected error happened while resetting onboarding.', 'woocommerce' )
);
}
@@ -1501,58 +1506,68 @@ class WooPaymentsService {
$event_props = array();
$source = $this->validate_onboarding_source( $source );
- // Lock the onboarding to prevent concurrent actions.
+ $endpoint = '';
+
+ // Both internal calls take the same parameters; only the endpoint differs per account type.
+ $params = array(
+ 'from' => ! empty( $from ) ? esc_attr( $from ) : self::FROM_PAYMENT_SETTINGS,
+ 'source' => $source,
+ );
+
+ // The same message is reused wherever an unexpected exception is converted to a WP_Error.
+ $exception_error_message = esc_html__( 'An unexpected error happened while disabling the test account.', 'woocommerce' );
+
+ // Briefly lock the onboarding while Core determines the account transition to perform.
+ // The internal WooPayments endpoint must run after this lock is cleared because it may
+ // trigger account deletion webhooks that also touch the shared NOX lock option.
$this->set_onboarding_lock();
try {
- $has_test_account = $this->has_test_account();
- $has_sandbox_account = $this->has_sandbox_account();
+ $had_test_account = $this->has_test_account();
+ $had_sandbox_account = $this->has_sandbox_account();
$event_props = array(
- 'account_type' => $has_test_account ? 'test_drive' : ( $has_sandbox_account ? 'sandbox' : 'unknown' ),
+ 'account_type' => $had_test_account ? 'test_drive' : ( $had_sandbox_account ? 'sandbox' : 'unknown' ),
'source' => $source,
);
- // First, check if we have a test account to disable.
- if ( $has_test_account ) {
- // Call the WooPayments API to disable the test account and prepare for the switch to live.
- $response = $this->proxy->call_static(
- Utils::class,
- 'rest_endpoint_post_request',
- '/wc/v3/payments/onboarding/test_drive_account/disable',
- array(
- 'from' => ! empty( $from ) ? esc_attr( $from ) : self::FROM_PAYMENT_SETTINGS,
- 'source' => $source,
- )
- );
- } elseif ( $has_sandbox_account ) {
- // Call the WooPayments API to reset onboarding.
+ if ( $had_test_account ) {
+ // Prepare the WooPayments API disable call for Phase 2, after the lock is released.
+ $endpoint = '/wc/v3/payments/onboarding/test_drive_account/disable';
+ } elseif ( $had_sandbox_account ) {
+ // Prepare the WooPayments API onboarding reset call for Phase 2, after the lock is released.
+ $endpoint = '/wc/v3/payments/onboarding/reset';
+ }
+ } catch ( Exception $e ) {
+ // Convert the exception to a WP_Error; the onboarding lock is released in the finally below.
+ $response = $this->get_onboarding_client_api_exception_error( $e, $exception_error_message );
+ } finally {
+ // Unlock before making the internal WooPayments request to avoid self-conflicting
+ // with WooPayments account cleanup and account.deleted webhook side effects.
+ $this->clear_onboarding_lock();
+ }
+
+ // Phase 2 runs after the shared lock is released to avoid the account.deleted webhook
+ // self-conflict. The WooPayments endpoint is not idempotent: a duplicate concurrent call is
+ // guarded against re-deleting the account (WooPayments overwrites its account cache before
+ // the delete, so is_stripe_connected() short-circuits), but it still surfaces to the second
+ // caller as a hard ApiException( FAILED_DEPENDENCY ) via the is_wp_error() check below.
+ // Request-scoped locking for that residual window is deferred to the broader
+ // Core/WooPayments shared-lock contract.
+ if ( ! is_wp_error( $response ) && ! empty( $endpoint ) ) {
+ try {
$response = $this->proxy->call_static(
Utils::class,
'rest_endpoint_post_request',
- '/wc/v3/payments/onboarding/reset',
- array(
- 'from' => ! empty( $from ) ? esc_attr( $from ) : self::FROM_PAYMENT_SETTINGS,
- 'source' => $source,
- )
+ $endpoint,
+ $params
);
+ } catch ( Exception $e ) {
+ // Convert the exception to a WP_Error so the failure is surfaced to the caller.
+ $response = $this->get_onboarding_client_api_exception_error( $e, $exception_error_message );
}
- } catch ( Exception $e ) {
- // Catch any exceptions to allow for proper error handling and onboarding unlock.
- $response = new WP_Error(
- 'woocommerce_woopayments_onboarding_client_api_exception',
- esc_html__( 'An unexpected error happened while disabling the test account.', 'woocommerce' ),
- array(
- 'code' => $e->getCode(),
- 'message' => $e->getMessage(),
- 'trace' => $e->getTrace(),
- )
- );
}
- // Unlock the onboarding after the API call finished or errored.
- $this->clear_onboarding_lock();
-
// Make sure the onboarding mode is reset.
if ( class_exists( 'WC_Payments_Onboarding_Service' ) && defined( 'WC_Payments_Onboarding_Service::TEST_MODE_OPTION' ) ) {
$this->proxy->call_function( 'update_option', Constants::get_constant( 'WC_Payments_Onboarding_Service::TEST_MODE_OPTION' ), 'no' );
@@ -1586,12 +1601,19 @@ class WooPaymentsService {
);
}
+ // The account mutation above has already committed, so the following is internal NOX state
+ // sync, not a new user action. Use the internal record path that does not re-check the
+ // onboarding lock: Phase 2 ran unlocked, so a concurrent request may now hold the lock, and
+ // re-checking it here would turn an already-successful disable into an onboarding-locked
+ // error (the shared, token-less lock also can't be safely reacquired around this bookkeeping).
+ // See record_onboarding_step_completed().
+
// For sanity, make sure the payment methods step is marked as completed.
// This is to avoid the user being prompted to set up payment methods again.
- $this->mark_onboarding_step_completed( self::ONBOARDING_STEP_PAYMENT_METHODS, $location );
+ $this->record_onboarding_step_completed( self::ONBOARDING_STEP_PAYMENT_METHODS, $location );
// For sanity, make sure the test account step is marked as completed and not blocked or failed.
// After disabling a test account, the user should be prompted to set up a live account.
- $this->mark_onboarding_step_completed( self::ONBOARDING_STEP_TEST_ACCOUNT, $location );
+ $this->record_onboarding_step_completed( self::ONBOARDING_STEP_TEST_ACCOUNT, $location );
$this->clear_onboarding_step_blocked( self::ONBOARDING_STEP_TEST_ACCOUNT, $location );
$this->clear_onboarding_step_failed( self::ONBOARDING_STEP_TEST_ACCOUNT, $location );
// Clear the NOX profile data for the business verification step sub-step data.
@@ -1611,6 +1633,30 @@ class WooPaymentsService {
return $response;
}
+ /**
+ * Build a WP_Error for an exception thrown while talking to the internal WooPayments onboarding API.
+ *
+ * @param Exception $e The caught exception.
+ * @param string $message The human-readable, already-escaped error message.
+ *
+ * @return WP_Error
+ */
+ private function get_onboarding_client_api_exception_error( Exception $e, string $message ): WP_Error {
+ // Deliberately exclude the exception stack trace from this error data. The data is
+ // passed to ApiException as additional data (documented as exposable in the error
+ // response) and is persisted to the NOX profile option by the onboarding step-failed
+ // handlers, so it must stay free of stack frames, whose arguments can carry file
+ // paths, tokens, and other sensitive values. Keep only these bounded scalars.
+ return new WP_Error(
+ 'woocommerce_woopayments_onboarding_client_api_exception',
+ $message,
+ array(
+ 'code' => $e->getCode(),
+ 'message' => $e->getMessage(),
+ )
+ );
+ }
+
/**
* Send a Tracks event.
*
@@ -1778,6 +1824,17 @@ class WooPaymentsService {
/**
* Unlock the onboarding.
*
+ * WARNING: this writes 0 unconditionally and has no ownership check, so it clears whatever
+ * lock is currently set — including one a concurrent request acquired. Call it only when
+ * clearing the lock is safe regardless of ownership:
+ * - to release a lock this same request set (e.g. in the finally that pairs with
+ * set_onboarding_lock()); or
+ * - to self-heal a lock that has already exceeded its TTL (see is_onboarding_locked()),
+ * where the stale timestamp means no live request is expected to still hold it.
+ * Do NOT call it after work that runs unlocked, where another request may have taken the
+ * lock in the meantime. Safe concurrent release would require a per-request token
+ * (compare-and-swap), which is deferred to the broader shared-lock work.
+ *
* @return void
*/
private function clear_onboarding_lock(): void {
diff --git a/plugins/woocommerce/tests/php/src/Internal/Admin/Settings/PaymentsProviders/WooPayments/WooPaymentsServiceTest.php b/plugins/woocommerce/tests/php/src/Internal/Admin/Settings/PaymentsProviders/WooPayments/WooPaymentsServiceTest.php
index 1f29e1142fe..1f22d615559 100644
--- a/plugins/woocommerce/tests/php/src/Internal/Admin/Settings/PaymentsProviders/WooPayments/WooPaymentsServiceTest.php
+++ b/plugins/woocommerce/tests/php/src/Internal/Admin/Settings/PaymentsProviders/WooPayments/WooPaymentsServiceTest.php
@@ -8090,6 +8090,653 @@ class WooPaymentsServiceTest extends WC_Unit_Test_Case {
// There is no automatic completion of the step due to its async nature.
}
+ /**
+ * @testdox Should preserve an internal request failure even when the account.deleted webhook clears the shared lock.
+ */
+ public function test_disable_test_account_preserves_internal_lock_failure_after_account_delete(): void {
+ $location = 'US';
+
+ $account_is_connected = true;
+ $account_status = array(
+ 'status' => 'complete',
+ 'testDrive' => true,
+ 'isLive' => false,
+ 'paymentsEnabled' => true,
+ 'detailsSubmitted' => true,
+ );
+ $this->mock_provider
+ ->expects( $this->any() )
+ ->method( 'is_account_connected' )
+ ->willReturnCallback(
+ function () use ( &$account_is_connected ) {
+ return $account_is_connected;
+ }
+ );
+ $this->mock_account_service
+ ->expects( $this->any() )
+ ->method( 'get_account_status_data' )
+ ->willReturnCallback(
+ function () use ( &$account_status ) {
+ return $account_status;
+ }
+ );
+
+ $lock_value = 0;
+ $lock_changes = array();
+ $stored_profile = array();
+ $updated_profile = array();
+ $this->mock_disable_test_account_option_state( $lock_value, $lock_changes, $updated_profile, $stored_profile );
+
+ $requests_made = array();
+ $this->mockable_proxy->register_static_mocks(
+ array(
+ Utils::class => array(
+ 'rest_endpoint_post_request' => function ( string $endpoint, array $params = array() ) use ( &$account_status, &$lock_value, &$requests_made ) {
+ if ( '/wc/v3/payments/onboarding/test_drive_account/disable' === $endpoint ) {
+ $requests_made[] = $params;
+ $account_status = array(
+ 'error' => true,
+ );
+ // Simulate the WooPayments account.deleted webhook deleting the shared lock
+ // option (a delete_option call, which Core's update_option tracker does not see).
+ $lock_value = null;
+
+ // The internal test-drive disable endpoint surfaces every thrown exception as
+ // HTTP 400 / bad_request (disable_test_drive_account in the WooPayments client),
+ // so that is what rest_endpoint_post_request returns to Core here.
+ return new WP_Error(
+ 'woocommerce_settings_payments_rest_error',
+ 'REST request POST /wc/v3/payments/onboarding/test_drive_account/disable failed with: (bad_request) Failed to disable the test-drive account.',
+ array(
+ 'code' => 'bad_request',
+ 'message' => 'Failed to disable the test-drive account.',
+ 'data' => array(
+ 'status' => 400,
+ ),
+ )
+ );
+ }
+
+ throw new \Exception( esc_html( 'POST endpoint response is not mocked: ' . $endpoint ) );
+ },
+ ),
+ )
+ );
+
+ try {
+ $this->sut->disable_test_account( $location, 'test-from', 'test-source' );
+
+ $this->fail( 'Expected ApiException not thrown.' );
+ } catch ( ApiException $e ) {
+ $this->assertSame( 'woocommerce_woopayments_onboarding_client_api_error', $e->getErrorCode() );
+ $this->assertSame( \WP_Http::FAILED_DEPENDENCY, $e->getCode() );
+ }
+
+ $this->assertCount( 1, $requests_made, 'The internal test-drive disable endpoint should be called once.' );
+ // Core never re-writes the lock after Phase 1, so the webhook's delete (modeled as null)
+ // survives; the get_option mock resolves null via "$lock_value ?? $default_value", so a
+ // webhook-deleted lock and a Core-cleared 0 are indistinguishable to is_onboarding_locked().
+ $this->assertNull( $lock_value, 'The webhook-deleted onboarding lock should remain deleted after the internal request fails.' );
+ $this->assertSame(
+ array( $this->current_time, 0 ),
+ $lock_changes,
+ 'Core should set and then clear its own preflight lock (two update_option writes); the webhook-triggered delete is not an update_option event.'
+ );
+ $this->assertSame(
+ array(),
+ $updated_profile,
+ 'NOX steps should not be marked completed when the internal disable request fails.'
+ );
+ }
+
+ /**
+ * @testdox Should clear the onboarding lock before the internal disable call, even when that call throws an unexpected error.
+ */
+ public function test_disable_test_account_clears_lock_before_internal_disable_even_when_it_throws(): void {
+ $location = 'US';
+
+ $account_exists = true;
+ $this->mock_account_state( $account_exists );
+
+ $lock_value = 0;
+ $lock_changes = array();
+ $updated_profile = array();
+ $this->mock_disable_test_account_option_state( $lock_value, $lock_changes, $updated_profile );
+
+ $this->mockable_proxy->register_static_mocks(
+ array(
+ Utils::class => array(
+ 'rest_endpoint_post_request' => function ( string $endpoint ) {
+ if ( '/wc/v3/payments/onboarding/test_drive_account/disable' === $endpoint ) {
+ throw new \Error( 'Simulated engine failure.' );
+ }
+
+ throw new \Exception( esc_html( 'POST endpoint response is not mocked: ' . $endpoint ) );
+ },
+ ),
+ )
+ );
+
+ try {
+ $this->sut->disable_test_account( $location, 'test-from', 'test-source' );
+
+ $this->fail( 'Expected Error not thrown.' );
+ } catch ( \Error $e ) {
+ // The internal call runs in Phase 2, which only catches Exception, so a fatal \Error
+ // propagates by design. The lock must already be cleared by Phase 1's finally at this point.
+ $this->assertSame( 0, $lock_value, 'The lock is cleared in Phase 1 before the internal call, so it is already cleared when a Phase 2 \Error propagates.' );
+ }
+
+ $this->assertSame( array( $this->current_time, 0 ), $lock_changes, 'Core sets then clears its own preflight lock; a propagating \Error adds no further lock writes.' );
+ }
+
+ /**
+ * @testdox Should preserve internal non-200 failures as client API errors and unlock.
+ */
+ public function test_disable_test_account_preserves_internal_non_200_as_client_api_error_and_unlocks(): void {
+ $location = 'US';
+
+ $account_exists = true;
+ $this->mock_account_state( $account_exists );
+
+ $lock_value = 0;
+ $lock_changes = array();
+ $updated_profile = array();
+ $this->mock_disable_test_account_option_state( $lock_value, $lock_changes, $updated_profile );
+
+ $this->mockable_proxy->register_static_mocks(
+ array(
+ Utils::class => array(
+ 'rest_endpoint_post_request' => function ( string $endpoint ) {
+ if ( '/wc/v3/payments/onboarding/test_drive_account/disable' === $endpoint ) {
+ return new WP_Error(
+ 'woocommerce_settings_payments_rest_error',
+ 'Internal WooPayments bad request.',
+ array(
+ 'code' => 'wcpay_bad_request',
+ 'message' => 'Internal WooPayments bad request.',
+ 'data' => array(
+ 'status' => 400,
+ ),
+ )
+ );
+ }
+
+ throw new \Exception( esc_html( 'POST endpoint response is not mocked: ' . $endpoint ) );
+ },
+ ),
+ )
+ );
+
+ try {
+ $this->sut->disable_test_account( $location, 'test-from', 'test-source' );
+
+ $this->fail( 'Expected ApiException not thrown.' );
+ } catch ( ApiException $e ) {
+ $this->assertSame( 'woocommerce_woopayments_onboarding_client_api_error', $e->getErrorCode() );
+ $this->assertSame( \WP_Http::FAILED_DEPENDENCY, $e->getCode() );
+ }
+
+ $this->assertSame( 0, $lock_value, 'The onboarding lock should be cleared after non-lock internal failures.' );
+ $this->assertSame( array( $this->current_time, 0 ), $lock_changes );
+ $this->assertSame( array(), $updated_profile, 'NOX steps should not be marked completed when the disable failed.' );
+ }
+
+ /**
+ * @testdox Should reject an active onboarding lock before calling internal test account disable.
+ */
+ public function test_disable_test_account_rejects_active_onboarding_lock_before_internal_disable(): void {
+ $location = 'US';
+
+ $endpoint_calls = 0;
+ $this->mockable_proxy->register_static_mocks(
+ array(
+ Utils::class => array(
+ 'rest_endpoint_post_request' => function () use ( &$endpoint_calls ) {
+ $endpoint_calls++;
+
+ return array(
+ 'success' => true,
+ );
+ },
+ ),
+ )
+ );
+
+ $lock_value = $this->current_time;
+ $lock_changes = array();
+ $updated_profile = array();
+ $this->mock_disable_test_account_option_state( $lock_value, $lock_changes, $updated_profile );
+
+ try {
+ $this->sut->disable_test_account( $location, 'test-from', 'test-source' );
+
+ $this->fail( 'Expected ApiException not thrown.' );
+ } catch ( ApiException $e ) {
+ $this->assertSame( 'woocommerce_woopayments_onboarding_locked', $e->getErrorCode() );
+ $this->assertSame( \WP_Http::CONFLICT, $e->getCode() );
+ }
+
+ $this->assertSame( 0, $endpoint_calls, 'The internal WooPayments endpoint should not be called while the lock is active.' );
+ $this->assertSame( $this->current_time, $lock_value, 'The active lock should remain untouched.' );
+ $this->assertSame( array(), $lock_changes, 'Core should not set or clear its own lock after rejecting an active lock.' );
+ }
+
+ /**
+ * @testdox Should self-heal an expired onboarding lock before disabling the test account.
+ */
+ public function test_disable_test_account_self_heals_expired_onboarding_lock_before_internal_disable(): void {
+ $location = 'US';
+
+ $account_exists = true;
+ $this->mock_account_state( $account_exists );
+
+ $endpoint_calls = 0;
+ $disable_test_account_result = function ( string $endpoint ) use ( &$endpoint_calls ) {
+ if ( '/wc/v3/payments/onboarding/test_drive_account/disable' === $endpoint ) {
+ ++$endpoint_calls;
+
+ return new WP_Error(
+ 'woocommerce_settings_payments_rest_error',
+ 'Internal WooPayments bad request.',
+ array(
+ 'code' => 'wcpay_bad_request',
+ 'message' => 'Internal WooPayments bad request.',
+ 'data' => array(
+ 'status' => 400,
+ ),
+ )
+ );
+ }
+
+ throw new \Exception( esc_html( 'POST endpoint response is not mocked: ' . $endpoint ) );
+ };
+ $this->mockable_proxy->register_static_mocks(
+ array(
+ Utils::class => array(
+ 'rest_endpoint_post_request' => $disable_test_account_result,
+ ),
+ )
+ );
+
+ $lock_value = $this->current_time - WooPaymentsService::NOX_ONBOARDING_LOCKED_TTL_SECONDS - 1;
+ $lock_changes = array();
+ $updated_profile = array();
+ $this->mock_disable_test_account_option_state( $lock_value, $lock_changes, $updated_profile );
+
+ try {
+ $this->sut->disable_test_account( $location, 'test-from', 'test-source' );
+
+ $this->fail( 'Expected ApiException not thrown.' );
+ } catch ( ApiException $e ) {
+ $this->assertSame( 'woocommerce_woopayments_onboarding_client_api_error', $e->getErrorCode() );
+ $this->assertSame( \WP_Http::FAILED_DEPENDENCY, $e->getCode() );
+ }
+
+ $this->assertSame( 1, $endpoint_calls, 'The internal WooPayments endpoint should be called after the stale lock self-heals.' );
+ $this->assertSame( 0, $lock_value, 'The onboarding lock should finish normalized to 0.' );
+ $this->assertSame(
+ array( 0, $this->current_time, 0 ),
+ $lock_changes,
+ 'Core should clear the stale lock, acquire and clear its preflight lock.'
+ );
+ }
+
+ /**
+ * @testdox Should disable a sandbox account via the onboarding reset endpoint and clear the lock around it.
+ */
+ public function test_disable_test_account_uses_reset_endpoint_for_sandbox_account(): void {
+ $location = 'US';
+
+ // A connected account that is neither a test-drive nor a live account is a sandbox account.
+ $this->mock_provider
+ ->expects( $this->any() )
+ ->method( 'is_account_connected' )
+ ->willReturn( true );
+ $this->mock_account_service
+ ->expects( $this->any() )
+ ->method( 'get_account_status_data' )
+ ->willReturn(
+ array(
+ 'status' => 'complete',
+ 'testDrive' => false,
+ 'isLive' => false,
+ 'paymentsEnabled' => true,
+ 'detailsSubmitted' => true,
+ )
+ );
+ $this->mock_working_wpcom_connection();
+
+ $lock_value = 0;
+ $lock_changes = array();
+ $updated_profile = array();
+ $this->mock_disable_test_account_option_state( $lock_value, $lock_changes, $updated_profile );
+
+ $requested_endpoints = array();
+ $requested_params = array();
+ $this->mockable_proxy->register_static_mocks(
+ array(
+ Utils::class => array(
+ 'rest_endpoint_post_request' => function ( string $endpoint, array $params = array() ) use ( &$requested_endpoints, &$requested_params ) {
+ $requested_endpoints[] = $endpoint;
+ $requested_params[] = $params;
+
+ return array(
+ 'success' => true,
+ );
+ },
+ ),
+ )
+ );
+
+ $result = $this->sut->disable_test_account( $location, 'test-from', 'test-source' );
+
+ $this->assertSame( array( 'success' => true ), $result, 'Disabling a sandbox account should succeed.' );
+ $this->assertSame(
+ array( '/wc/v3/payments/onboarding/reset' ),
+ $requested_endpoints,
+ 'A sandbox account should be disabled via the onboarding reset endpoint, not the test-drive disable endpoint.'
+ );
+ // The Phase 2 payload must carry the caller's "from" and the validated source so a regression that
+ // routes correctly but drops or swaps these fails. "test-source" is not whitelisted, so the service
+ // normalizes it to the default via validate_onboarding_source().
+ $this->assertSame( 'test-from', $requested_params[0]['from'] ?? null, 'The reset request should forward the caller "from".' );
+ $this->assertSame( WooPaymentsService::SESSION_ENTRY_DEFAULT, $requested_params[0]['source'] ?? null, 'The reset request should carry the validated onboarding source.' );
+ $this->assertSame(
+ array( $this->current_time, 0 ),
+ $lock_changes,
+ 'Core should set its preflight lock and clear it before the Phase 2 reset call.'
+ );
+ $this->assertSame( 0, $lock_value, 'The onboarding lock should be cleared after the reset call.' );
+ $this->assert_onboarding_step_completed( $updated_profile, $location, WooPaymentsService::ONBOARDING_STEP_PAYMENT_METHODS );
+ $this->assert_onboarding_step_completed( $updated_profile, $location, WooPaymentsService::ONBOARDING_STEP_TEST_ACCOUNT );
+ }
+
+ /**
+ * @testdox Should make no internal request and still succeed when there is no test or sandbox account to disable.
+ */
+ public function test_disable_test_account_makes_no_request_when_no_test_or_sandbox_account(): void {
+ $location = 'US';
+
+ // No connected account means there is neither a test-drive nor a sandbox account to disable.
+ $this->mock_provider
+ ->expects( $this->any() )
+ ->method( 'is_account_connected' )
+ ->willReturn( false );
+ $this->mock_account_service
+ ->expects( $this->any() )
+ ->method( 'get_account_status_data' )
+ ->willReturn( array() );
+ $this->mock_working_wpcom_connection();
+
+ $lock_value = 0;
+ $lock_changes = array();
+ $updated_profile = array();
+ $this->mock_disable_test_account_option_state( $lock_value, $lock_changes, $updated_profile );
+
+ $endpoint_calls = 0;
+ $this->mockable_proxy->register_static_mocks(
+ array(
+ Utils::class => array(
+ 'rest_endpoint_post_request' => function () use ( &$endpoint_calls ) {
+ ++$endpoint_calls;
+
+ return array(
+ 'success' => true,
+ );
+ },
+ ),
+ )
+ );
+
+ $result = $this->sut->disable_test_account( $location, 'test-from', 'test-source' );
+
+ $this->assertSame( array( 'success' => true ), $result, 'The no-op disable should return a success response.' );
+ $this->assertSame( 0, $endpoint_calls, 'No internal WooPayments request should be made when there is no account to disable.' );
+ $this->assertSame(
+ array( $this->current_time, 0 ),
+ $lock_changes,
+ 'Core should still set and clear its preflight lock even when the Phase 2 request is skipped.'
+ );
+ $this->assertSame( 0, $lock_value, 'The onboarding lock should be cleared after the no-op.' );
+ }
+
+ /**
+ * @testdox Should disable a test-drive account via the test-drive endpoint and mark steps completed on success.
+ */
+ public function test_disable_test_account_marks_steps_completed_on_successful_test_drive_disable(): void {
+ $location = 'US';
+
+ $account_exists = true;
+ $this->mock_account_state( $account_exists );
+ $this->mock_working_wpcom_connection();
+
+ $lock_value = 0;
+ $lock_changes = array();
+ $updated_profile = array();
+ $this->mock_disable_test_account_option_state( $lock_value, $lock_changes, $updated_profile );
+
+ $requested_endpoints = array();
+ $requested_params = array();
+ $this->mockable_proxy->register_static_mocks(
+ array(
+ Utils::class => array(
+ 'rest_endpoint_post_request' => function ( string $endpoint, array $params = array() ) use ( &$requested_endpoints, &$requested_params ) {
+ $requested_endpoints[] = $endpoint;
+ $requested_params[] = $params;
+
+ return array(
+ 'success' => true,
+ );
+ },
+ ),
+ )
+ );
+
+ $result = $this->sut->disable_test_account( $location, 'test-from', 'test-source' );
+
+ $this->assertSame( array( 'success' => true ), $result, 'Disabling a test-drive account should succeed.' );
+ $this->assertSame(
+ array( '/wc/v3/payments/onboarding/test_drive_account/disable' ),
+ $requested_endpoints,
+ 'A test-drive account should be disabled via the test-drive disable endpoint, not the onboarding reset endpoint.'
+ );
+ // The Phase 2 payload must carry the caller's "from" and the validated source so a regression that
+ // routes correctly but drops or swaps these fails. "test-source" is not whitelisted, so the service
+ // normalizes it to the default via validate_onboarding_source().
+ $this->assertSame( 'test-from', $requested_params[0]['from'] ?? null, 'The disable request should forward the caller "from".' );
+ $this->assertSame( WooPaymentsService::SESSION_ENTRY_DEFAULT, $requested_params[0]['source'] ?? null, 'The disable request should carry the validated onboarding source.' );
+ $this->assertSame(
+ array( $this->current_time, 0 ),
+ $lock_changes,
+ 'Core should set its preflight lock and clear it before the Phase 2 disable call.'
+ );
+ $this->assertSame( 0, $lock_value, 'The onboarding lock should be cleared after the disable call.' );
+ $this->assert_onboarding_step_completed( $updated_profile, $location, WooPaymentsService::ONBOARDING_STEP_PAYMENT_METHODS );
+ $this->assert_onboarding_step_completed( $updated_profile, $location, WooPaymentsService::ONBOARDING_STEP_TEST_ACCOUNT );
+ }
+
+ /**
+ * @testdox Should not write the onboarding lock during or after the Phase 2 internal call.
+ */
+ public function test_disable_test_account_does_not_write_onboarding_lock_during_phase_2(): void {
+ // Guards the invariant that Phase 2 must not call clear_onboarding_lock(): because the lock
+ // is a shared, token-less option, an unconditional Phase 2 clear would stomp a concurrent
+ // request's freshly acquired lock. The proof is that no lock write happens once Phase 2 runs.
+ $location = 'US';
+
+ $account_exists = true;
+ $this->mock_account_state( $account_exists );
+ $this->mock_working_wpcom_connection();
+
+ $lock_value = 0;
+ $lock_changes = array();
+ $updated_profile = array();
+ $this->mock_disable_test_account_option_state( $lock_value, $lock_changes, $updated_profile );
+
+ $lock_writes_at_phase_2 = null;
+ $this->mockable_proxy->register_static_mocks(
+ array(
+ Utils::class => array(
+ 'rest_endpoint_post_request' => function ( string $endpoint ) use ( &$lock_changes, &$lock_writes_at_phase_2 ) {
+ if ( '/wc/v3/payments/onboarding/test_drive_account/disable' === $endpoint ) {
+ // Snapshot the lock writes that happened before the internal call ran.
+ $lock_writes_at_phase_2 = $lock_changes;
+
+ return array(
+ 'success' => true,
+ );
+ }
+
+ throw new \Exception( esc_html( 'POST endpoint response is not mocked: ' . $endpoint ) );
+ },
+ ),
+ )
+ );
+
+ $result = $this->sut->disable_test_account( $location, 'test-from', 'test-source' );
+
+ $this->assertSame( array( 'success' => true ), $result, 'Disabling the test-drive account should succeed.' );
+ $this->assertSame(
+ array( $this->current_time, 0 ),
+ $lock_writes_at_phase_2,
+ 'By the time the Phase 2 internal call runs, Core should have already set and cleared its preflight lock.'
+ );
+ $this->assertSame(
+ $lock_writes_at_phase_2,
+ $lock_changes,
+ 'Phase 2 and the post-call step bookkeeping must not write the onboarding lock again; an extra write means an unconditional Phase 2 clear was re-introduced.'
+ );
+ }
+
+ /**
+ * @testdox Should complete the post-disable bookkeeping for a test-drive account even if a concurrent request re-acquires the onboarding lock during the unlocked Phase 2 window.
+ */
+ public function test_disable_test_account_completes_bookkeeping_when_lock_reacquired_during_phase_2_for_test_drive(): void {
+ $location = 'US';
+
+ $account_exists = true;
+ $this->mock_account_state( $account_exists );
+ $this->mock_working_wpcom_connection();
+
+ $lock_value = 0;
+ $lock_changes = array();
+ $updated_profile = array();
+ $this->mock_disable_test_account_option_state( $lock_value, $lock_changes, $updated_profile );
+
+ // Simulate a concurrent onboarding request acquiring the shared lock during the unlocked
+ // Phase 2 window: the internal disable mutation commits, but by the time Core runs its
+ // post-disable bookkeeping the lock is held again. Because the mutation has already
+ // happened, the bookkeeping must not re-check the lock (which would throw onboarding_locked
+ // after a successful mutation) and must not stomp the concurrent request's lock.
+ $current_time = $this->current_time;
+ $this->mockable_proxy->register_static_mocks(
+ array(
+ Utils::class => array(
+ 'rest_endpoint_post_request' => function ( string $endpoint ) use ( &$lock_value, $current_time ) {
+ if ( '/wc/v3/payments/onboarding/test_drive_account/disable' === $endpoint ) {
+ // A concurrent request grabs the lock after Core released it for Phase 2.
+ $lock_value = $current_time;
+
+ return array(
+ 'success' => true,
+ );
+ }
+
+ throw new \Exception( esc_html( 'POST endpoint response is not mocked: ' . $endpoint ) );
+ },
+ ),
+ )
+ );
+
+ $result = $this->sut->disable_test_account( $location, 'test-from', 'test-source' );
+
+ $this->assertSame( array( 'success' => true ), $result, 'The disable should succeed even though the lock was re-acquired before the bookkeeping ran.' );
+ // The committed mutation's bookkeeping must run to completion instead of throwing an onboarding-locked error.
+ $this->assert_onboarding_step_completed( $updated_profile, $location, WooPaymentsService::ONBOARDING_STEP_PAYMENT_METHODS );
+ $this->assert_onboarding_step_completed( $updated_profile, $location, WooPaymentsService::ONBOARDING_STEP_TEST_ACCOUNT );
+ // Core must only set and release its own preflight lock; the post-disable bookkeeping must
+ // not write the lock at all, so the concurrent request's lock is left intact (not stomped to 0).
+ $this->assertSame(
+ array( $current_time, 0 ),
+ $lock_changes,
+ 'Only the preflight set and release should be written; the post-disable bookkeeping must not write the lock.'
+ );
+ $this->assertSame(
+ $current_time,
+ $lock_value,
+ "The concurrent request's onboarding lock must remain intact after the bookkeeping completes."
+ );
+ }
+
+ /**
+ * @testdox Should complete the post-reset bookkeeping for a sandbox account even if a concurrent request re-acquires the onboarding lock during the unlocked Phase 2 window.
+ */
+ public function test_disable_test_account_completes_bookkeeping_when_lock_reacquired_during_phase_2_for_sandbox(): void {
+ $location = 'US';
+
+ // A connected account that is neither a test-drive nor a live account is a sandbox account.
+ $this->mock_provider
+ ->expects( $this->any() )
+ ->method( 'is_account_connected' )
+ ->willReturn( true );
+ $this->mock_account_service
+ ->expects( $this->any() )
+ ->method( 'get_account_status_data' )
+ ->willReturn(
+ array(
+ 'status' => 'complete',
+ 'testDrive' => false,
+ 'isLive' => false,
+ 'paymentsEnabled' => true,
+ 'detailsSubmitted' => true,
+ )
+ );
+ $this->mock_working_wpcom_connection();
+
+ $lock_value = 0;
+ $lock_changes = array();
+ $updated_profile = array();
+ $this->mock_disable_test_account_option_state( $lock_value, $lock_changes, $updated_profile );
+
+ // Same race as the test-drive path, but for the sandbox reset endpoint.
+ $current_time = $this->current_time;
+ $this->mockable_proxy->register_static_mocks(
+ array(
+ Utils::class => array(
+ 'rest_endpoint_post_request' => function ( string $endpoint ) use ( &$lock_value, $current_time ) {
+ if ( '/wc/v3/payments/onboarding/reset' === $endpoint ) {
+ // A concurrent request grabs the lock after Core released it for Phase 2.
+ $lock_value = $current_time;
+
+ return array(
+ 'success' => true,
+ );
+ }
+
+ throw new \Exception( esc_html( 'POST endpoint response is not mocked: ' . $endpoint ) );
+ },
+ ),
+ )
+ );
+
+ $result = $this->sut->disable_test_account( $location, 'test-from', 'test-source' );
+
+ $this->assertSame( array( 'success' => true ), $result, 'The reset should succeed even though the lock was re-acquired before the bookkeeping ran.' );
+ $this->assert_onboarding_step_completed( $updated_profile, $location, WooPaymentsService::ONBOARDING_STEP_PAYMENT_METHODS );
+ $this->assert_onboarding_step_completed( $updated_profile, $location, WooPaymentsService::ONBOARDING_STEP_TEST_ACCOUNT );
+ $this->assertSame(
+ array( $current_time, 0 ),
+ $lock_changes,
+ 'Only the preflight set and release should be written; the post-reset bookkeeping must not write the lock.'
+ );
+ $this->assertSame(
+ $current_time,
+ $lock_value,
+ "The concurrent request's onboarding lock must remain intact after the bookkeeping completes."
+ );
+ }
+
/**
* Test get_onboarding_kyc_session throws exception when extension is not active.
*
@@ -9553,6 +10200,113 @@ class WooPaymentsServiceTest extends WC_Unit_Test_Case {
$this->assertFalse( $result['apple_google'], 'Explicit stored apple_google should take precedence over OR/recommended.' );
}
+ /**
+ * Mock a working WPCOM connection.
+ *
+ * Required for onboarding step completion that depends on the WPCOM connection step.
+ */
+ private function mock_working_wpcom_connection(): void {
+ $this->mock_wpcom_connection_manager
+ ->expects( $this->any() )
+ ->method( 'is_connected' )
+ ->willReturn( true );
+ $this->mock_wpcom_connection_manager
+ ->expects( $this->any() )
+ ->method( 'has_connected_owner' )
+ ->willReturn( true );
+ }
+
+ /**
+ * Mock a WooPayments account state for test-drive disable tests.
+ *
+ * @param bool $account_exists Whether the account is currently connected.
+ */
+ private function mock_account_state( bool $account_exists ): void {
+ $this->mock_provider
+ ->expects( $this->any() )
+ ->method( 'is_account_connected' )
+ ->willReturn( $account_exists );
+ $this->mock_account_service
+ ->expects( $this->any() )
+ ->method( 'get_account_status_data' )
+ ->willReturn(
+ array(
+ 'status' => 'complete',
+ 'testDrive' => true,
+ 'isLive' => false,
+ 'paymentsEnabled' => true,
+ 'detailsSubmitted' => true,
+ )
+ );
+ }
+
+ /**
+ * Mock NOX lock and profile options for test-drive disable tests.
+ *
+ * @param mixed &$lock_value The current lock value.
+ * @param array &$lock_changes Recorded lock writes.
+ * @param array &$updated_profile The current updated NOX profile.
+ * @param array $stored_profile The initial stored NOX profile.
+ */
+ private function mock_disable_test_account_option_state( &$lock_value, array &$lock_changes, array &$updated_profile, array $stored_profile = array() ): void {
+ // Track writes explicitly rather than inferring them from a non-empty profile, so that an
+ // intentional write of an empty profile is distinct from "never written" and reads do not
+ // silently fall back to the stored profile.
+ $profile_written = false;
+ $this->mockable_proxy->register_function_mocks(
+ array(
+ 'get_option' => function ( $option_name, $default_value = null ) use ( &$lock_value, &$updated_profile, &$profile_written, $stored_profile ) {
+ if ( WooPaymentsService::NOX_ONBOARDING_LOCKED_KEY === $option_name ) {
+ return $lock_value ?? $default_value;
+ }
+ if ( WooPaymentsService::NOX_PROFILE_OPTION_KEY === $option_name ) {
+ return $profile_written ? $updated_profile : $stored_profile;
+ }
+
+ return $default_value;
+ },
+ 'update_option' => function ( $option_name, $value ) use ( &$lock_value, &$lock_changes, &$updated_profile, &$profile_written ) {
+ if ( WooPaymentsService::NOX_ONBOARDING_LOCKED_KEY === $option_name ) {
+ $lock_value = $value;
+ $lock_changes[] = $value;
+
+ return true;
+ }
+ if ( WooPaymentsService::NOX_PROFILE_OPTION_KEY === $option_name ) {
+ $updated_profile = $value;
+ $profile_written = true;
+
+ return true;
+ }
+
+ return true;
+ },
+ )
+ );
+ }
+
+ /**
+ * Assert that an onboarding step is recorded as completed in the NOX profile.
+ *
+ * @param array $profile The full NOX profile option value.
+ * @param string $location The onboarding location (ISO 3166-1 alpha-2 country code).
+ * @param string $step_id The onboarding step ID that should be completed.
+ */
+ private function assert_onboarding_step_completed( array $profile, string $location, string $step_id ): void {
+ $steps = $profile['onboarding'][ $location ]['steps'] ?? array();
+
+ $this->assertArrayHasKey(
+ $step_id,
+ $steps,
+ sprintf( 'The "%s" onboarding step should be recorded on a successful disable.', $step_id )
+ );
+ $this->assertArrayHasKey(
+ WooPaymentsService::ONBOARDING_STEP_STATUS_COMPLETED,
+ $steps[ $step_id ]['statuses'] ?? array(),
+ sprintf( 'The "%s" onboarding step should be marked completed on a successful disable.', $step_id )
+ );
+ }
+
/**
* Helper method to invoke private methods for testing.
*