Commit 6a1b974c098 for woocommerce
commit 6a1b974c0987075613569c7c9b970f00ea044176
Author: Oleksandr Aratovskyi <79862886+oaratovskyi@users.noreply.github.com>
Date: Thu Mar 12 12:04:42 2026 +0200
fix: place new gateways above offline PMs in payment settings (#63604)
* fix: place new gateways above offline PMs instead of at end of list (WOOPLUG-6189)
When a new payment gateway is installed, place it above offline payment
methods instead of at the absolute end — unless the merchant has
customized the ordering. Adds is_offline_group_last() helper to detect
default vs custom ordering. Fixes both Payments.php gateway loop and
PaymentsProviders.php enhance_order_map() fallback.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* changelog: add entry for WOOPLUG-6189
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: correct test expectations for new gateway placement behavior
Update PaymentsProvidersTest and PaymentsTest to match the new behavior
where gateways are placed above offline PMs in default ordering.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: add assertNotEmpty for unused $data in PaymentsTest
Address CodeRabbit review nitpick: assert on $data return value
to silence static analysis warnings and add basic verification.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: align fallback placement logic between display and persistence paths
Both Payments.php and PaymentsProviders.php now use
Utils::order_map_add_at_order() with max()+1 for the "append at end"
fallback, ensuring consistent collision handling and correct end-placement
when order values have gaps.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* test: strengthen assertions and clean up test data providers
- Add assertGreaterThan for stripe > gateway1 to properly verify
end-placement in custom ordering test
- Remove unused $new_gateway_id parameter from test method and data provider
- Replace bare strings with WC_Gateway_BACS::ID, WC_Gateway_Cheque::ID,
WC_Gateway_COD::ID class constants for consistency
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* test: assert on final provider output instead of intermediate order map
Switch both placement tests to verify $data (the sorted providers list
returned to the UI) instead of the captured_order_map intermediate.
This tests actual behavior rather than implementation details.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: skip suggestions in is_offline_group_last and extract shared placement method
- Add is_suggestion_order_map_id() skip in is_offline_group_last() so
persisted suggestion entries don't cause false negatives
- Extract order_map_add_gateway() on PaymentsProviders as the single
source of truth for new gateway placement logic
- Both display (Payments.php) and persistence (enhance_order_map) paths
now delegate to the same method
- Add test cases for suggestion after offline group and missing offline
group scenarios
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---------
Co-authored-by: oaratovskyi <oleksandr.aratovskyi@automattic.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
diff --git a/plugins/woocommerce/changelog/fix-wooplug-6189-gateway-ordering b/plugins/woocommerce/changelog/fix-wooplug-6189-gateway-ordering
new file mode 100644
index 00000000000..83a13d2e48c
--- /dev/null
+++ b/plugins/woocommerce/changelog/fix-wooplug-6189-gateway-ordering
@@ -0,0 +1,4 @@
+Significance: patch
+Type: fix
+
+Place newly installed payment gateways above offline payment methods instead of at the bottom of the list.
diff --git a/plugins/woocommerce/src/Internal/Admin/Settings/Payments.php b/plugins/woocommerce/src/Internal/Admin/Settings/Payments.php
index e37b0fae2e8..2087a3afb1b 100644
--- a/plugins/woocommerce/src/Internal/Admin/Settings/Payments.php
+++ b/plugins/woocommerce/src/Internal/Admin/Settings/Payments.php
@@ -136,9 +136,10 @@ class Payments {
foreach ( $payment_gateways as $payment_gateway ) {
// Determine the gateway's order value.
- // If we don't have an order for it, add it to the end.
+ // If we don't have an order for it, place it above offline PMs if the offline group
+ // is still at the bottom (default ordering). Otherwise, add to the end.
if ( ! isset( $providers_order_map[ $payment_gateway->id ] ) ) {
- $providers_order_map = Utils::order_map_add_at_order( $providers_order_map, $payment_gateway->id, count( $payment_providers ) );
+ $providers_order_map = $this->providers->order_map_add_gateway( $providers_order_map, $payment_gateway->id );
}
$payment_providers[] = $this->providers->get_payment_gateway_details(
diff --git a/plugins/woocommerce/src/Internal/Admin/Settings/PaymentsProviders.php b/plugins/woocommerce/src/Internal/Admin/Settings/PaymentsProviders.php
index 47ad39c9b1f..54916f7a6d9 100644
--- a/plugins/woocommerce/src/Internal/Admin/Settings/PaymentsProviders.php
+++ b/plugins/woocommerce/src/Internal/Admin/Settings/PaymentsProviders.php
@@ -560,6 +560,70 @@ class PaymentsProviders {
return in_array( $id, self::OFFLINE_METHODS, true );
}
+ /**
+ * Check if the offline payment methods group is the last non-offline entry in an order map.
+ *
+ * This is used to detect whether the merchant has customized the provider ordering.
+ * If the offline group is still at the bottom (its default position), new gateways
+ * should be inserted above it. If the merchant has moved it, we respect their layout
+ * and append new gateways at the end.
+ *
+ * @param array $order_map The payment providers order map.
+ *
+ * @return bool True if the offline group is the last non-offline entry, false otherwise.
+ */
+ public function is_offline_group_last( array $order_map ): bool {
+ if ( ! isset( $order_map[ self::OFFLINE_METHODS_ORDERING_GROUP ] ) ) {
+ return false;
+ }
+
+ $offline_group_order = $order_map[ self::OFFLINE_METHODS_ORDERING_GROUP ];
+
+ // Check if any non-offline, non-suggestion entry has an order higher than the offline group.
+ foreach ( $order_map as $id => $order ) {
+ if ( self::OFFLINE_METHODS_ORDERING_GROUP === $id ) {
+ continue;
+ }
+ if ( $this->is_offline_payment_method( $id ) ) {
+ continue;
+ }
+ if ( $this->is_suggestion_order_map_id( $id ) ) {
+ continue;
+ }
+ if ( $order > $offline_group_order ) {
+ return false;
+ }
+ }
+
+ return true;
+ }
+
+ /**
+ * Add a new gateway to an order map with offline-awareness.
+ *
+ * If the offline payment methods group is the last non-offline, non-suggestion entry,
+ * the gateway is placed above it. Otherwise, it is appended at the end.
+ *
+ * This is the single source of truth for new gateway placement logic,
+ * used by both the display path (Payments) and the persistence path (enhance_order_map).
+ *
+ * @param array $order_map The payment providers order map.
+ * @param string $id The gateway ID to add.
+ *
+ * @return array The updated order map.
+ */
+ public function order_map_add_gateway( array $order_map, string $id ): array {
+ if ( $this->is_offline_group_last( $order_map ) ) {
+ return Utils::order_map_add_at_order(
+ $order_map,
+ $id,
+ $order_map[ self::OFFLINE_METHODS_ORDERING_GROUP ]
+ );
+ }
+
+ return Utils::order_map_add_at_order( $order_map, $id, empty( $order_map ) ? 0 : max( $order_map ) + 1 );
+ }
+
/**
* Check if a payment gateway is a shell payment gateway.
*
@@ -1057,8 +1121,9 @@ class PaymentsProviders {
}
}
- // Add the missing payment gateway at the end.
- $order_map[ $id ] = empty( $order_map ) ? 0 : max( $order_map ) + 1;
+ // If the offline PMs group is the last non-offline entry, place above it.
+ // Otherwise (custom ordering or no offline group), place at the end.
+ $order_map = $this->order_map_add_gateway( $order_map, $id );
}
$handled_suggestion_ids = array_unique( $handled_suggestion_ids );
diff --git a/plugins/woocommerce/tests/php/src/Internal/Admin/Settings/PaymentsProvidersTest.php b/plugins/woocommerce/tests/php/src/Internal/Admin/Settings/PaymentsProvidersTest.php
index 0c8d0d52e9b..baef1775b56 100644
--- a/plugins/woocommerce/tests/php/src/Internal/Admin/Settings/PaymentsProvidersTest.php
+++ b/plugins/woocommerce/tests/php/src/Internal/Admin/Settings/PaymentsProvidersTest.php
@@ -3062,15 +3062,16 @@ class PaymentsProvidersTest extends WC_Unit_Test_Case {
array(
PaymentsProviders::OFFLINE_METHODS_ORDERING_GROUP => 0,
),
+ // New gateways are placed above the offline group (default ordering).
array(
- PaymentsProviders::OFFLINE_METHODS_ORDERING_GROUP,
- WC_Gateway_BACS::ID,
- WC_Gateway_Cheque::ID,
- WC_Gateway_COD::ID,
'gateway1',
'gateway2',
'gateway3_0',
'gateway3_1',
+ PaymentsProviders::OFFLINE_METHODS_ORDERING_GROUP,
+ WC_Gateway_BACS::ID,
+ WC_Gateway_Cheque::ID,
+ WC_Gateway_COD::ID,
),
$gateways + $offline_payment_methods_gateways,
array(),
@@ -3080,17 +3081,18 @@ class PaymentsProvidersTest extends WC_Unit_Test_Case {
array(
PaymentsProviders::OFFLINE_METHODS_ORDERING_GROUP => 0,
),
+ // New gateways (and their suggestions) are placed above the offline group (default ordering).
array(
- PaymentsProviders::OFFLINE_METHODS_ORDERING_GROUP,
- WC_Gateway_BACS::ID,
- WC_Gateway_Cheque::ID,
- WC_Gateway_COD::ID,
'_wc_pes_suggestion1',
'gateway1',
'gateway2',
'_wc_pes_suggestion3',
'gateway3_0',
'gateway3_1',
+ PaymentsProviders::OFFLINE_METHODS_ORDERING_GROUP,
+ WC_Gateway_BACS::ID,
+ WC_Gateway_Cheque::ID,
+ WC_Gateway_COD::ID,
),
$gateways + $offline_payment_methods_gateways,
$suggestions,
@@ -5943,4 +5945,196 @@ class PaymentsProvidersTest extends WC_Unit_Test_Case {
$this->sut->reset_memo();
}
+
+ /**
+ * @dataProvider data_provider_is_offline_group_last
+ *
+ * @param array $order_map The order map to test.
+ * @param bool $expected Whether the offline group should be considered last.
+ */
+ public function test_is_offline_group_last( array $order_map, bool $expected ) {
+ $sut = $this->sut;
+
+ $this->assertSame( $expected, $sut->is_offline_group_last( $order_map ) );
+ }
+
+ /**
+ * Data provider for test_is_offline_group_last.
+ */
+ public function data_provider_is_offline_group_last(): array {
+ return array(
+ 'empty order map' => array(
+ array(),
+ false,
+ ),
+ 'no offline group in map' => array(
+ array(
+ 'gateway1' => 0,
+ 'gateway2' => 1,
+ ),
+ false,
+ ),
+ 'offline group is last' => array(
+ array(
+ 'gateway1' => 0,
+ 'gateway2' => 1,
+ PaymentsProviders::OFFLINE_METHODS_ORDERING_GROUP => 2,
+ WC_Gateway_BACS::ID => 3,
+ WC_Gateway_Cheque::ID => 4,
+ WC_Gateway_COD::ID => 5,
+ ),
+ true,
+ ),
+ 'offline group is last, no offline PMs' => array(
+ array(
+ 'gateway1' => 0,
+ 'gateway2' => 1,
+ PaymentsProviders::OFFLINE_METHODS_ORDERING_GROUP => 2,
+ ),
+ true,
+ ),
+ 'gateway after offline group' => array(
+ array(
+ 'gateway1' => 0,
+ PaymentsProviders::OFFLINE_METHODS_ORDERING_GROUP => 1,
+ WC_Gateway_BACS::ID => 2,
+ WC_Gateway_Cheque::ID => 3,
+ WC_Gateway_COD::ID => 4,
+ 'gateway2' => 5,
+ ),
+ false,
+ ),
+ 'offline group at start' => array(
+ array(
+ PaymentsProviders::OFFLINE_METHODS_ORDERING_GROUP => 0,
+ WC_Gateway_BACS::ID => 1,
+ WC_Gateway_Cheque::ID => 2,
+ WC_Gateway_COD::ID => 3,
+ 'gateway1' => 4,
+ ),
+ false,
+ ),
+ 'only offline group and offline PMs' => array(
+ array(
+ PaymentsProviders::OFFLINE_METHODS_ORDERING_GROUP => 0,
+ WC_Gateway_BACS::ID => 1,
+ WC_Gateway_Cheque::ID => 2,
+ ),
+ true,
+ ),
+ 'only offline group' => array(
+ array(
+ PaymentsProviders::OFFLINE_METHODS_ORDERING_GROUP => 0,
+ ),
+ true,
+ ),
+ 'suggestion after offline group' => array(
+ array(
+ 'gateway1' => 0,
+ PaymentsProviders::OFFLINE_METHODS_ORDERING_GROUP => 1,
+ WC_Gateway_BACS::ID => 2,
+ WC_Gateway_Cheque::ID => 3,
+ WC_Gateway_COD::ID => 4,
+ PaymentsProviders::SUGGESTION_ORDERING_PREFIX . 'suggestion1' => 5,
+ ),
+ true,
+ ),
+ );
+ }
+
+ /**
+ * @dataProvider data_provider_enhance_order_map_new_gateway_placement
+ *
+ * @param array $gateway_ids The gateway IDs to register.
+ * @param array $start_order_map The starting order map.
+ * @param string[] $expected_order The expected order of IDs after enhancement.
+ */
+ public function test_enhance_order_map_new_gateway_placement(
+ array $gateway_ids,
+ array $start_order_map,
+ array $expected_order
+ ) {
+ // Mock payment gateways — all gateways including the new one are registered.
+ $this->mock_payment_gateways(
+ array_combine(
+ $gateway_ids,
+ array_map(
+ function () {
+ return array( 'enabled' => true );
+ },
+ $gateway_ids
+ )
+ )
+ );
+ // No suggestions for any gateway.
+ $this->mock_extension_suggestions
+ ->expects( $this->any() )
+ ->method( 'get_by_plugin_slug' )
+ ->willReturn( null );
+
+ $sut = $this->sut;
+
+ $result = $sut->enhance_order_map( $start_order_map );
+
+ // Extract the order — keys sorted by value.
+ $actual_order = array_keys( $result );
+ // Filter to only the IDs we care about for assertion clarity.
+ $actual_order = array_values( array_intersect( $actual_order, $expected_order ) );
+
+ $this->assertSame( $expected_order, $actual_order );
+ }
+
+ /**
+ * Data provider for test_enhance_order_map_new_gateway_placement.
+ */
+ public function data_provider_enhance_order_map_new_gateway_placement(): array {
+ return array(
+ 'new gateway placed above offline group (default ordering)' => array(
+ // gateway_ids: all registered gateways.
+ array( 'gateway1', 'stripe', 'bacs', 'cheque', 'cod' ),
+ // start_order_map: existing map WITHOUT the new gateway.
+ array(
+ 'gateway1' => 0,
+ PaymentsProviders::OFFLINE_METHODS_ORDERING_GROUP => 1,
+ WC_Gateway_BACS::ID => 2,
+ WC_Gateway_Cheque::ID => 3,
+ WC_Gateway_COD::ID => 4,
+ ),
+ // expected_order: stripe should be above offline group.
+ array( 'gateway1', 'stripe', PaymentsProviders::OFFLINE_METHODS_ORDERING_GROUP, WC_Gateway_BACS::ID, WC_Gateway_Cheque::ID, WC_Gateway_COD::ID ),
+ ),
+ 'new gateway placed at end (custom ordering — offline group not last)' => array(
+ array( 'gateway1', 'stripe', 'bacs', 'cheque', 'cod' ),
+ array(
+ PaymentsProviders::OFFLINE_METHODS_ORDERING_GROUP => 0,
+ WC_Gateway_BACS::ID => 1,
+ WC_Gateway_Cheque::ID => 2,
+ WC_Gateway_COD::ID => 3,
+ 'gateway1' => 4,
+ ),
+ // expected_order: stripe at the end since offline group is not last.
+ array( PaymentsProviders::OFFLINE_METHODS_ORDERING_GROUP, WC_Gateway_BACS::ID, WC_Gateway_Cheque::ID, WC_Gateway_COD::ID, 'gateway1', 'stripe' ),
+ ),
+ 'multiple new gateways placed above offline group' => array(
+ array( 'gateway1', 'stripe', 'paypal', 'bacs', 'cheque', 'cod' ),
+ array(
+ 'gateway1' => 0,
+ PaymentsProviders::OFFLINE_METHODS_ORDERING_GROUP => 1,
+ WC_Gateway_BACS::ID => 2,
+ WC_Gateway_Cheque::ID => 3,
+ WC_Gateway_COD::ID => 4,
+ ),
+ // expected_order: both stripe and paypal should be above offline group.
+ array( 'gateway1', 'stripe', 'paypal', PaymentsProviders::OFFLINE_METHODS_ORDERING_GROUP, WC_Gateway_BACS::ID, WC_Gateway_Cheque::ID, WC_Gateway_COD::ID ),
+ ),
+ 'new gateway placed at end (no offline group in map)' => array(
+ array( 'gateway1', 'stripe' ),
+ array(
+ 'gateway1' => 0,
+ ),
+ // expected_order: stripe at the end since there is no offline group.
+ array( 'gateway1', 'stripe' ),
+ ),
+ );
+ }
}
diff --git a/plugins/woocommerce/tests/php/src/Internal/Admin/Settings/PaymentsTest.php b/plugins/woocommerce/tests/php/src/Internal/Admin/Settings/PaymentsTest.php
index fa83d7f94f4..4fdfe1bcc9c 100644
--- a/plugins/woocommerce/tests/php/src/Internal/Admin/Settings/PaymentsTest.php
+++ b/plugins/woocommerce/tests/php/src/Internal/Admin/Settings/PaymentsTest.php
@@ -595,4 +595,125 @@ class PaymentsTest extends WC_Unit_Test_Case {
// Assert.
$this->assertTrue( $result );
}
+
+ /**
+ * Test that new gateways are placed above offline PMs when offline group is last.
+ */
+ public function test_get_payment_providers_new_gateway_above_offline_pms() {
+ // Arrange.
+ $location = 'US';
+
+ $gateways = array(
+ new FakePaymentGateway( 'gateway1', array( 'plugin_slug' => 'plugin1' ) ),
+ new FakePaymentGateway( 'stripe', array( 'plugin_slug' => 'woocommerce-gateway-stripe' ) ),
+ // The offline PMs.
+ new FakePaymentGateway( WC_Gateway_BACS::ID, array( 'plugin_slug' => 'woocommerce' ) ),
+ new FakePaymentGateway( WC_Gateway_Cheque::ID, array( 'plugin_slug' => 'woocommerce' ) ),
+ new FakePaymentGateway( WC_Gateway_COD::ID, array( 'plugin_slug' => 'woocommerce' ) ),
+ );
+ $this->mock_providers
+ ->expects( $this->atLeastOnce() )
+ ->method( 'get_payment_gateways' )
+ ->willReturn( $gateways );
+
+ // Order map has gateway1 and offline group, but NOT 'stripe'.
+ $this->mock_providers
+ ->expects( $this->any() )
+ ->method( 'get_order_map' )
+ ->willReturn(
+ array(
+ 'gateway1' => 0,
+ PaymentsProviders::OFFLINE_METHODS_ORDERING_GROUP => 1,
+ WC_Gateway_BACS::ID => 2,
+ WC_Gateway_Cheque::ID => 3,
+ WC_Gateway_COD::ID => 4,
+ )
+ );
+
+ // Pass through the order map unchanged.
+ $this->mock_providers
+ ->expects( $this->any() )
+ ->method( 'enhance_order_map' )
+ ->willReturnArgument( 0 );
+
+ $this->mock_providers
+ ->expects( $this->any() )
+ ->method( 'get_extension_suggestions' )
+ ->with( $location )
+ ->willReturn( array() );
+
+ // Act.
+ $data = $this->sut->get_payment_providers( $location );
+
+ // Assert: stripe should appear between gateway1 and the offline group in the final sorted output.
+ $provider_ids = array_column( $data, 'id' );
+ $stripe_index = array_search( 'stripe', $provider_ids, true );
+ $gateway1_index = array_search( 'gateway1', $provider_ids, true );
+ $offline_group_index = array_search( PaymentsProviders::OFFLINE_METHODS_ORDERING_GROUP, $provider_ids, true );
+
+ $this->assertNotFalse( $stripe_index, 'stripe should be in the providers list' );
+ $this->assertNotFalse( $offline_group_index, 'offline group should be in the providers list' );
+ $this->assertGreaterThan( $gateway1_index, $stripe_index, 'stripe should be after gateway1' );
+ $this->assertLessThan( $offline_group_index, $stripe_index, 'stripe should be before the offline group' );
+ }
+
+ /**
+ * Test that new gateways are placed at end when offline group is NOT last (custom ordering).
+ */
+ public function test_get_payment_providers_new_gateway_at_end_custom_ordering() {
+ // Arrange.
+ $location = 'US';
+
+ $gateways = array(
+ new FakePaymentGateway( 'gateway1', array( 'plugin_slug' => 'plugin1' ) ),
+ new FakePaymentGateway( 'stripe', array( 'plugin_slug' => 'woocommerce-gateway-stripe' ) ),
+ // The offline PMs.
+ new FakePaymentGateway( WC_Gateway_BACS::ID, array( 'plugin_slug' => 'woocommerce' ) ),
+ new FakePaymentGateway( WC_Gateway_Cheque::ID, array( 'plugin_slug' => 'woocommerce' ) ),
+ new FakePaymentGateway( WC_Gateway_COD::ID, array( 'plugin_slug' => 'woocommerce' ) ),
+ );
+ $this->mock_providers
+ ->expects( $this->atLeastOnce() )
+ ->method( 'get_payment_gateways' )
+ ->willReturn( $gateways );
+
+ // Custom ordering: offline group is NOT last (gateway1 is after it).
+ $this->mock_providers
+ ->expects( $this->any() )
+ ->method( 'get_order_map' )
+ ->willReturn(
+ array(
+ PaymentsProviders::OFFLINE_METHODS_ORDERING_GROUP => 0,
+ WC_Gateway_BACS::ID => 1,
+ WC_Gateway_Cheque::ID => 2,
+ WC_Gateway_COD::ID => 3,
+ 'gateway1' => 4,
+ )
+ );
+
+ // Pass through the order map unchanged.
+ $this->mock_providers
+ ->expects( $this->any() )
+ ->method( 'enhance_order_map' )
+ ->willReturnArgument( 0 );
+
+ $this->mock_providers
+ ->expects( $this->any() )
+ ->method( 'get_extension_suggestions' )
+ ->with( $location )
+ ->willReturn( array() );
+
+ // Act.
+ $data = $this->sut->get_payment_providers( $location );
+
+ // Assert: stripe should be at the end — after all existing gateways (custom ordering fallback).
+ $provider_ids = array_column( $data, 'id' );
+ $stripe_index = array_search( 'stripe', $provider_ids, true );
+ $gateway1_index = array_search( 'gateway1', $provider_ids, true );
+
+ $this->assertNotFalse( $stripe_index, 'stripe should be in the providers list' );
+ $this->assertGreaterThan( $gateway1_index, $stripe_index, 'stripe should be after gateway1' );
+ // Stripe should be the last non-offline-PM provider.
+ $this->assertSame( count( $provider_ids ) - 1, $stripe_index, 'stripe should be the last provider' );
+ }
}