From b6a13d4ded46d8b22c3c7a9b0f575d63a8357273 Mon Sep 17 00:00:00 2001 From: Zaman Date: Wed, 8 Jul 2026 00:08:16 +0600 Subject: [PATCH 1/2] GOOWOO-773 500 Error Switching Shipping Rate --- src/MerchantCenter/MarketService.php | 110 ++++++-- tests/Unit/API/Google/SettingsTest.php | 46 +++- .../Unit/MerchantCenter/MarketServiceTest.php | 252 ++++++++++++++++-- 3 files changed, 372 insertions(+), 36 deletions(-) diff --git a/src/MerchantCenter/MarketService.php b/src/MerchantCenter/MarketService.php index 80fbccd633..f1e46fca41 100644 --- a/src/MerchantCenter/MarketService.php +++ b/src/MerchantCenter/MarketService.php @@ -115,6 +115,15 @@ public function register(): void {} /** * Returns all markets, keyed by ID with the synthesised primary always first. * + * The shipping method (`shipping_rate`/`shipping_time`) is a single, store-wide + * setting held in the MERCHANT_CENTER option. Secondary markets keep their own + * snapshot of it from when they were created, but those copies can drift out of + * date (e.g. the merchant later switches the global rate to `manual`). Every + * returned market therefore reflects the current global values rather than its + * stored snapshot, so every consumer — the sync check, the currency map builder + * and the REST responses — reads a single source of truth. The same is done for + * `free_shipping` (see GOOWOO-698). + * * @return array[] Keyed by market ID ('primary', then secondary IDs). */ public function get_markets(): array { @@ -122,13 +131,19 @@ public function get_markets(): array { $secondary = is_array( $stored ) ? $stored : []; unset( $secondary['primary'] ); - $all_rates = $this->get_cached_shipping_rates(); - $all_countries = $this->wc->get_countries(); - $is_flat_mode = $this->is_flat_shipping_rate(); + $all_rates = $this->get_cached_shipping_rates(); + $all_countries = $this->wc->get_countries(); + $is_flat_mode = $this->is_flat_shipping_rate(); + $global_shipping = $this->global_shipping_method(); foreach ( $secondary as &$market ) { $country = $market['country'] ?? null; + // Overwrite the stored snapshot with the live global shipping method so + // no decision is ever made against a stale per-market copy. + $market['shipping_rate'] = $global_shipping['shipping_rate']; + $market['shipping_time'] = $global_shipping['shipping_time']; + // DB rate rows are retained when the merchant switches modes so they // can be restored later, so the read boundary has to gate them. $market['free_shipping'] = ( $is_flat_mode && $country && isset( $all_rates[ $country ]['free_shipping_threshold'] ) ) @@ -259,7 +274,9 @@ public function add_market( string $id, array $config ): void { $this->remove_country_from_target_audience( $config['country'] ); } - if ( 'manual' !== ( $config['shipping_rate'] ?? null ) ) { + // The shipping method is global, so whether Merchant Center needs a sync is + // decided by the global setting, not by this market's stored snapshot. + if ( $this->global_shipping_is_syncable() ) { $this->schedule_shipping_sync(); } @@ -269,9 +286,10 @@ public function add_market( string $id, array $config ): void { * Fires after a secondary market is successfully added. * * @param string $id The market ID. - * @param array $config The market configuration as persisted, including shipping_rate and shipping_time defaults. + * @param array $config The market configuration as persisted. The shipping_rate/shipping_time + * reflect the current global shipping method (see get_markets()). */ - do_action( 'woocommerce_gla_market_added', $id, $config ); + do_action( 'woocommerce_gla_market_added', $id, array_merge( $config, $this->global_shipping_method() ) ); } /** @@ -331,6 +349,12 @@ public function update_market( string $id, array $config ): array { return $this->fire_market_updated_action( $id ); } + // Secondary markets don't own a shipping method — it is driven by the global + // setting (see get_markets()). The Edit Market screen still submits these + // fields on every save, so drop them rather than letting them mutate the + // stored snapshot or trip the shipping-sync change detection below. + unset( $config['shipping_rate'], $config['shipping_time'] ); + $markets = $this->get_stored_secondary_markets(); $existing = $markets[ $id ] ?? []; $merged = array_merge( $existing, $config ); @@ -355,7 +379,9 @@ public function update_market( string $id, array $config ): array { ); } - $shipping_keys = [ 'country', 'currency', 'shipping_rate', 'shipping_time' ]; + // shipping_rate/shipping_time are global and were dropped above, so only a + // country or currency change can affect what this market syncs to Google. + $shipping_keys = [ 'country', 'currency' ]; foreach ( $shipping_keys as $key ) { if ( ( $existing[ $key ] ?? null ) !== ( $merged[ $key ] ?? null ) ) { $this->schedule_shipping_sync(); @@ -502,7 +528,6 @@ public function delete_market( string $id ): void { $deleted_config = $markets[ $id ]; $country = $deleted_config['country'] ?? null; $feed_label = $deleted_config['feed_label'] ?? null; - $shipping_rate = $deleted_config['shipping_rate'] ?? null; unset( $markets[ $id ] ); $this->options->update( OptionsInterface::MARKETS, $markets ); @@ -518,7 +543,10 @@ public function delete_market( string $id ): void { ->schedule( [ 'feed_label' => $feed_label ] ); } - if ( 'manual' !== $shipping_rate ) { + // The shipping method is global. Deleting a market whose stored snapshot said + // `manual` while the global rate is flat/automatic must still notify Google, + // so the decision reads the global setting rather than the deleted snapshot. + if ( $this->global_shipping_is_syncable() ) { $this->schedule_shipping_sync(); } @@ -529,8 +557,10 @@ public function delete_market( string $id ): void { * * @param string $id The market ID. * @param array $deleted_config The market configuration as it existed at the time of deletion. + * The shipping_rate/shipping_time reflect the current global + * shipping method (see get_markets()). */ - do_action( 'woocommerce_gla_market_deleted', $id, $deleted_config ); + do_action( 'woocommerce_gla_market_deleted', $id, array_merge( $deleted_config, $this->global_shipping_method() ) ); } /** @@ -580,17 +610,19 @@ public function get_all_countries(): array { /** * Whether any configured market needs its shipping settings synced to Merchant Center. * - * Returns true when at least one market has a non-`manual` `shipping_rate` - * combined with `shipping_time === 'flat'`. A non-`manual` secondary market - * is enough to require a sync even when the primary itself is `manual`. + * A market is syncable when its `shipping_rate` is `flat` or `automatic` and its + * `shipping_time` is `flat`. Anything else — `manual`, or a missing/unrecognised + * value — is not syncable and must not schedule a sync, because the DB shipping + * adapter would then be asked to push rates the merchant never entered. + * + * Every market reflects the global shipping method (see get_markets()), so in + * practice this answers "is the global shipping method syncable?". * * @return bool */ public function has_syncable_markets(): bool { foreach ( $this->get_markets() as $market ) { - $rate = $market['shipping_rate'] ?? null; - $time = $market['shipping_time'] ?? null; - if ( 'manual' !== $rate && 'flat' === $time ) { + if ( $this->is_syncable_shipping_method( $market['shipping_rate'] ?? null, $market['shipping_time'] ?? null ) ) { return true; } } @@ -726,6 +758,52 @@ private function is_flat_shipping_rate(): bool { return is_array( $mc_settings ) && 'flat' === ( $mc_settings['shipping_rate'] ?? null ); } + /** + * Returns the store-wide shipping method from the MERCHANT_CENTER option. + * + * This is the single source of truth for every market's shipping_rate and + * shipping_time; both values are null when the setting is unset. + * + * @return array{shipping_rate: string|null, shipping_time: string|null} + */ + private function global_shipping_method(): array { + $mc_settings = $this->options->get( OptionsInterface::MERCHANT_CENTER, [] ); + if ( ! is_array( $mc_settings ) ) { + $mc_settings = []; + } + + return [ + 'shipping_rate' => $mc_settings['shipping_rate'] ?? null, + 'shipping_time' => $mc_settings['shipping_time'] ?? null, + ]; + } + + /** + * Whether the global shipping method can be synced to Merchant Center. + * + * Used by add_market()/delete_market() to decide whether the change needs to be + * pushed to Google. Mirrors the per-market predicate in has_syncable_markets(). + * + * @return bool + */ + private function global_shipping_is_syncable(): bool { + $global = $this->global_shipping_method(); + + return $this->is_syncable_shipping_method( $global['shipping_rate'], $global['shipping_time'] ); + } + + /** + * Whether a shipping method (rate + time) is syncable to Merchant Center. + * + * @param string|null $rate The shipping_rate value. + * @param string|null $time The shipping_time value. + * + * @return bool True when the rate is `flat` or `automatic` and the time is `flat`. + */ + private function is_syncable_shipping_method( $rate, $time ): bool { + return in_array( $rate, [ 'flat', 'automatic' ], true ) && 'flat' === $time; + } + /** * Returns shipping rates, fetched lazily and cached on the service instance. * diff --git a/tests/Unit/API/Google/SettingsTest.php b/tests/Unit/API/Google/SettingsTest.php index 2293668edb..64147e89b1 100644 --- a/tests/Unit/API/Google/SettingsTest.php +++ b/tests/Unit/API/Google/SettingsTest.php @@ -161,13 +161,20 @@ function ( $key, $fallback = null ) { $this->assertSame( 'EUR', $by_country['FR']->getCurrency() ); } + /** + * The shipping method is global, so MarketService::get_markets() returns the same + * method for every market (GOOWOO-773) — a mix of `manual` and non-`manual` markets + * is no longer representable. When the global method is `manual`, every market is + * skipped and the currency map is empty. build_country_currency_map keeps the guard + * defensively so a stray manual market can never leak into the synced services. + */ public function test_generate_shipping_settings_skips_manual_markets_from_currency_map(): void { $this->market_service->method( 'get_markets' )->willReturn( [ 'primary' => [ 'country' => 'US', 'currency' => [ 'USD' ], - 'shipping_rate' => 'flat', + 'shipping_rate' => 'manual', 'shipping_time' => 'flat', ], 'fr' => [ @@ -183,7 +190,42 @@ public function test_generate_shipping_settings_skips_manual_markets_from_curren $map = $this->invoke( 'build_country_currency_map' ); - $this->assertSame( [ 'US' => 'USD' ], $map ); + $this->assertSame( [], $map ); + } + + /** + * With a non-manual global method every market contributes to the currency map, + * each with its own currency. + */ + public function test_generate_shipping_settings_includes_every_market_in_currency_map(): void { + $this->market_service->method( 'get_markets' )->willReturn( + [ + 'primary' => [ + 'country' => 'US', + 'currency' => [ 'USD' ], + 'shipping_rate' => 'flat', + 'shipping_time' => 'flat', + ], + 'fr' => [ + 'country' => 'FR', + 'currency' => [ 'EUR' ], + 'shipping_rate' => 'flat', + 'shipping_time' => 'flat', + ], + ] + ); + + $this->wc_proxy->method( 'get_woocommerce_currency' )->willReturn( 'USD' ); + + $map = $this->invoke( 'build_country_currency_map' ); + + $this->assertSame( + [ + 'US' => 'USD', + 'FR' => 'EUR', + ], + $map + ); } public function test_generate_shipping_settings_prefers_per_row_currency_over_country_map(): void { diff --git a/tests/Unit/MerchantCenter/MarketServiceTest.php b/tests/Unit/MerchantCenter/MarketServiceTest.php index 03a2605980..e28ae42144 100644 --- a/tests/Unit/MerchantCenter/MarketServiceTest.php +++ b/tests/Unit/MerchantCenter/MarketServiceTest.php @@ -129,6 +129,43 @@ public function test_get_markets_returns_primary_and_stored_secondary(): void { $this->assertSame( 'primary', $result['primary']['id'] ); } + /** + * Regression (GOOWOO-773): a secondary market's stored shipping snapshot must be + * replaced by the current global method on read, so no consumer ever sees a stale + * value. Here the stored snapshot is automatic/flat but the global rate is manual. + */ + public function test_get_markets_overwrites_secondary_shipping_with_global_method(): void { + $secondary = [ + 'fr' => [ + 'country' => 'FR', + 'language' => [ 'fr' ], + 'currency' => [ 'EUR' ], + 'feed_label' => 'FR', + 'shipping_rate' => 'automatic', + 'shipping_time' => 'flat', + ], + ]; + + $this->set_up_options_get( + [ + OptionsInterface::MARKETS => $secondary, + OptionsInterface::MERCHANT_CENTER => [ + 'shipping_rate' => 'manual', + 'shipping_time' => 'manual', + ], + ] + ); + $this->set_up_primary_market_dependencies( 'US', [ 'US' ] ); + + $result = $this->market_service->get_markets(); + + $this->assertSame( 'manual', $result['fr']['shipping_rate'] ); + $this->assertSame( 'manual', $result['fr']['shipping_time'] ); + // The primary is composed from the same global option. + $this->assertSame( 'manual', $result['primary']['shipping_rate'] ); + $this->assertSame( 'manual', $result['primary']['shipping_time'] ); + } + public function test_get_markets_falls_back_to_default_when_empty(): void { $this->set_up_options_get( [ OptionsInterface::MARKETS => null ] ); $this->set_up_primary_market_dependencies( 'US', [ 'US' ] ); @@ -854,7 +891,13 @@ public function test_update_market_secondary_validates_merged_config(): void { $this->market_service->update_market( 'gb', [ 'country' => '' ] ); } - public function test_update_market_secondary_partial_update_succeeds(): void { + /** + * The Edit Market screen submits shipping_rate/shipping_time on every save, but + * a secondary market does not own a shipping method — it is global. Those params + * must be dropped (not persisted, not error), while the market's other fields + * still save, and the returned market reflects the global shipping method. + */ + public function test_update_market_secondary_drops_shipping_params_but_saves_other_fields(): void { $existing = [ 'gb' => [ 'country' => 'GB', @@ -865,12 +908,31 @@ public function test_update_market_secondary_partial_update_succeeds(): void { ], ]; - $this->set_up_options_get_with_tracking( [ OptionsInterface::MARKETS => $existing ] ); + $this->set_up_options_get_with_tracking( + [ + OptionsInterface::MARKETS => $existing, + OptionsInterface::MERCHANT_CENTER => [ + 'shipping_rate' => 'flat', + 'shipping_time' => 'flat', + ], + ] + ); $this->set_up_primary_market_dependencies( 'US', [ 'US' ] ); - $result = $this->market_service->update_market( 'gb', [ 'shipping_rate' => 'flat' ] ); + $result = $this->market_service->update_market( + 'gb', + [ + 'shipping_rate' => 'manual', + 'shipping_time' => 'manual', + 'currency' => [ 'EUR' ], + ] + ); + // The submitted shipping params are dropped; the result reflects the global method. $this->assertSame( 'flat', $result['shipping_rate'] ); + $this->assertSame( 'flat', $result['shipping_time'] ); + // The non-shipping field still saved. + $this->assertSame( [ 'EUR' ], $result['currency'] ); $this->assertSame( 'GB', $result['country'] ); } @@ -951,23 +1013,27 @@ public function test_update_market_does_not_schedule_cleanup_when_non_feed_label ); } - public function test_add_market_with_manual_shipping_rate_does_not_schedule_shipping_sync(): void { + public function test_add_market_does_not_schedule_shipping_sync_when_global_is_manual(): void { $config = [ - 'country' => 'DE', - 'language' => [ 'de' ], - 'currency' => [ 'EUR' ], - 'feed_label' => 'DE', - 'shipping_rate' => 'manual', + 'country' => 'DE', + 'language' => [ 'de' ], + 'currency' => [ 'EUR' ], + 'feed_label' => 'DE', ]; $this->set_up_options_get( [ OptionsInterface::MARKETS => [], + OptionsInterface::MERCHANT_CENTER => [ + 'shipping_rate' => 'manual', + 'shipping_time' => 'flat', + ], OptionsInterface::TARGET_AUDIENCE => [ 'countries' => [ 'US' ] ], ] ); $this->options->method( 'update' )->willReturn( true ); + // Global rate is manual → nothing to sync, regardless of the stored snapshot. $this->shipping_settings_job->expects( $this->never() ) ->method( 'schedule' ); @@ -995,30 +1061,76 @@ public function test_update_market_does_not_schedule_shipping_sync_when_only_lan $this->market_service->update_market( 'gb', [ 'language' => [ 'en', 'cy' ] ] ); } - public function test_delete_market_with_manual_shipping_rate_does_not_schedule_shipping_sync(): void { + public function test_delete_market_does_not_schedule_shipping_sync_when_global_is_manual(): void { $existing = [ 'gb' => [ 'country' => 'GB', 'language' => [ 'en' ], 'currency' => [ 'GBP' ], 'feed_label' => 'GB', - 'shipping_rate' => 'manual', + // Stale snapshot says non-manual, but the global rate below is manual. + 'shipping_rate' => 'automatic', 'shipping_time' => 'flat', ], ]; - $this->set_up_options_get( [ OptionsInterface::MARKETS => $existing ] ); + $this->set_up_options_get( + [ + OptionsInterface::MARKETS => $existing, + OptionsInterface::MERCHANT_CENTER => [ + 'shipping_rate' => 'manual', + 'shipping_time' => 'flat', + ], + ] + ); $this->options->method( 'update' )->willReturn( true ); $this->shipping_rate_query->method( 'get_results' )->willReturn( [] ); $this->shipping_time_query->method( 'get_results' )->willReturn( [] ); + // Global rate is manual → no sync, even though the deleted snapshot was automatic. $this->shipping_settings_job->expects( $this->never() ) ->method( 'schedule' ); $this->market_service->delete_market( 'gb' ); } + /** + * Inverse direction: the deleted market's stale snapshot says `manual`, but the + * global rate is `flat`, so Google must still be notified after deletion. + */ + public function test_delete_market_schedules_shipping_sync_when_global_is_flat_despite_stale_manual(): void { + $existing = [ + 'gb' => [ + 'country' => 'GB', + 'language' => [ 'en' ], + 'currency' => [ 'GBP' ], + 'feed_label' => 'GB', + 'shipping_rate' => 'manual', + 'shipping_time' => 'flat', + ], + ]; + + $this->set_up_options_get( + [ + OptionsInterface::MARKETS => $existing, + OptionsInterface::MERCHANT_CENTER => [ + 'shipping_rate' => 'flat', + 'shipping_time' => 'flat', + ], + ] + ); + $this->options->method( 'update' )->willReturn( true ); + + $this->shipping_rate_query->method( 'get_results' )->willReturn( [] ); + $this->shipping_time_query->method( 'get_results' )->willReturn( [] ); + + $this->shipping_settings_job->expects( $this->once() ) + ->method( 'schedule' ); + + $this->market_service->delete_market( 'gb' ); + } + public function test_add_market_secondary_schedules_update_all_products(): void { $config = [ 'country' => 'GB', @@ -1339,6 +1451,10 @@ public function test_add_market_does_not_schedule_cleanup(): void { $this->set_up_options_get( [ OptionsInterface::MARKETS => [], + OptionsInterface::MERCHANT_CENTER => [ + 'shipping_rate' => 'flat', + 'shipping_time' => 'flat', + ], OptionsInterface::TARGET_AUDIENCE => [ 'countries' => [ 'US' ] ], ] ); @@ -1347,7 +1463,7 @@ public function test_add_market_does_not_schedule_cleanup(): void { $this->cleanup_job->expects( $this->never() ) ->method( 'schedule' ); - // shipping_rate defaults to 'flat' (non-manual) → shipping sync IS scheduled. + // Global shipping method is flat/flat (syncable) → shipping sync IS scheduled. $this->shipping_settings_job->expects( $this->once() ) ->method( 'schedule' ); @@ -1423,7 +1539,15 @@ public function test_delete_market_schedules_cleanup_with_feed_label(): void { ], ]; - $this->set_up_options_get( [ OptionsInterface::MARKETS => $existing ] ); + $this->set_up_options_get( + [ + OptionsInterface::MARKETS => $existing, + OptionsInterface::MERCHANT_CENTER => [ + 'shipping_rate' => 'flat', + 'shipping_time' => 'flat', + ], + ] + ); $this->options->method( 'update' )->willReturn( true ); $this->shipping_rate_query->method( 'get_results' )->willReturn( [] ); @@ -1433,7 +1557,7 @@ public function test_delete_market_schedules_cleanup_with_feed_label(): void { ->method( 'schedule' ) ->with( [ 'feed_label' => 'GB' ] ); - // Non-manual shipping_rate → shipping sync also scheduled. + // Global shipping method is flat/flat (syncable) → shipping sync also scheduled. $this->shipping_settings_job->expects( $this->once() ) ->method( 'schedule' ); @@ -1848,7 +1972,11 @@ public function test_delete_market_fires_woocommerce_gla_market_deleted_hook_onc $this->set_up_options_get( [ - OptionsInterface::MARKETS => [ 'fr' => $stored_config ], + OptionsInterface::MARKETS => [ 'fr' => $stored_config ], + OptionsInterface::MERCHANT_CENTER => [ + 'shipping_rate' => 'flat', + 'shipping_time' => 'flat', + ], ] ); @@ -1868,8 +1996,16 @@ public function test_delete_market_fires_woocommerce_gla_market_deleted_hook_onc remove_action( 'woocommerce_gla_market_deleted', $listener, 10 ); } + // The payload carries the deleted config with the current global shipping method. + $expected_config = array_merge( + $stored_config, + [ + 'shipping_rate' => 'flat', + 'shipping_time' => 'flat', + ] + ); $this->assertCount( 1, $captured ); - $this->assertSame( [ 'fr', $stored_config ], $captured[0] ); + $this->assertSame( [ 'fr', $expected_config ], $captured[0] ); } public function test_delete_market_coerces_null_options_to_empty_array(): void { @@ -2243,6 +2379,12 @@ function ( $key, $value ) use ( &$update_calls ) { $this->assertSame( 'manual', $stored_jp['shipping_time'] ); } + /** + * An explicit shipping_rate/shipping_time in the add request is still persisted + * as the stored snapshot. This is storage-only: the snapshot no longer drives any + * decision (get_markets() overwrites it with the global method on read, and the + * sync gate reads the global setting), but the write itself is preserved. + */ public function test_add_market_explicit_shipping_mode_takes_precedence_over_mc_settings(): void { $mc_settings = [ 'shipping_rate' => 'automatic', @@ -2726,7 +2868,14 @@ public function test_shipping_rates_fetched_once_across_multiple_get_markets_cal $this->market_service->get_markets(); } - public function test_has_syncable_markets_true_when_only_secondary_is_non_manual(): void { + /** + * Regression (GOOWOO-773): a secondary market keeps a stale snapshot of the + * shipping method (here `flat`) from when it was created. After the merchant + * switches the global rate to `manual`, the sync check must follow the global + * setting, not the stale snapshot — otherwise a sync is attempted with no DB + * rates and the DB adapter throws (500 error on saving the setting). + */ + public function test_has_syncable_markets_false_when_global_is_manual_despite_stale_secondary(): void { $this->set_up_options_get( [ OptionsInterface::MERCHANT_CENTER => [ @@ -2747,9 +2896,70 @@ public function test_has_syncable_markets_true_when_only_secondary_is_non_manual ); $this->set_up_primary_market_dependencies( 'US', [ 'US' ] ); + $this->assertFalse( $this->market_service->has_syncable_markets() ); + } + + /** + * The inverse of the regression case: a secondary market's stale snapshot says + * `manual`, but the global rate is `flat`, so the store is syncable. + */ + public function test_has_syncable_markets_true_when_global_is_flat_despite_stale_manual_secondary(): void { + $this->set_up_options_get( + [ + OptionsInterface::MERCHANT_CENTER => [ + 'shipping_rate' => 'flat', + 'shipping_time' => 'flat', + ], + OptionsInterface::MARKETS => [ + 'fr' => [ + 'country' => 'FR', + 'feed_label' => 'FR', + 'language' => [ 'fr' ], + 'currency' => [ 'EUR' ], + 'shipping_rate' => 'manual', + 'shipping_time' => 'flat', + ], + ], + ] + ); + $this->set_up_primary_market_dependencies( 'US', [ 'US' ] ); + + $this->assertTrue( $this->market_service->has_syncable_markets() ); + } + + public function test_has_syncable_markets_true_when_global_is_automatic(): void { + $this->set_up_options_get( + [ + OptionsInterface::MERCHANT_CENTER => [ + 'shipping_rate' => 'automatic', + 'shipping_time' => 'flat', + ], + OptionsInterface::MARKETS => [], + ] + ); + $this->set_up_primary_market_dependencies( 'US', [ 'US' ] ); + $this->assertTrue( $this->market_service->has_syncable_markets() ); } + /** + * A missing/unset shipping_rate (with a flat time) is not syncable — the old + * `'manual' !== $rate` check would have wrongly treated null as syncable. + */ + public function test_has_syncable_markets_false_when_rate_is_null(): void { + $this->set_up_options_get( + [ + OptionsInterface::MERCHANT_CENTER => [ + 'shipping_time' => 'flat', + ], + OptionsInterface::MARKETS => [], + ] + ); + $this->set_up_primary_market_dependencies( 'US', [ 'US' ] ); + + $this->assertFalse( $this->market_service->has_syncable_markets() ); + } + public function test_has_syncable_markets_false_when_every_market_is_manual(): void { $this->set_up_options_get( [ @@ -3010,6 +3220,10 @@ public function test_delete_market_fires_market_deleted_hook_on_success(): void $this->set_up_options_get( [ OptionsInterface::MARKETS => [ 'gb' => $existing_entry ], + OptionsInterface::MERCHANT_CENTER => [ + 'shipping_rate' => 'flat', + 'shipping_time' => 'flat', + ], OptionsInterface::TARGET_AUDIENCE => [ 'countries' => [ 'US' ] ], ] ); @@ -3033,6 +3247,8 @@ function ( $id, $deleted_config ) use ( &$fired_count, &$captured_id, &$captured $this->market_service->delete_market( 'gb' ); + // The stored snapshot already matches the global method (flat/flat), so the + // payload equals the deleted config. $this->assertSame( 1, $fired_count ); $this->assertSame( 'gb', $captured_id ); $this->assertSame( $existing_entry, $captured_config ); From 67c2994a7ecd55b653391e61b2755f2cc44b6c0d Mon Sep 17 00:00:00 2001 From: Zaman Date: Thu, 9 Jul 2026 23:58:41 +0600 Subject: [PATCH 2/2] Added missing test --- .../Unit/MerchantCenter/MarketServiceTest.php | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/tests/Unit/MerchantCenter/MarketServiceTest.php b/tests/Unit/MerchantCenter/MarketServiceTest.php index e28ae42144..54b0f70d71 100644 --- a/tests/Unit/MerchantCenter/MarketServiceTest.php +++ b/tests/Unit/MerchantCenter/MarketServiceTest.php @@ -936,6 +936,62 @@ public function test_update_market_secondary_drops_shipping_params_but_saves_oth $this->assertSame( 'GB', $result['country'] ); } + /** + * A shipping-only edit (the Edit Market screen echoing the global method back on + * save, while the stored snapshot is stale) must be a no-op: no shipping sync is + * scheduled and the stored snapshot is left untouched. This is the exact scenario + * the ticket names, and locks in both halves of the drop behaviour. + */ + public function test_update_market_secondary_shipping_only_edit_does_not_schedule_shipping_sync(): void { + $existing = [ + 'gb' => [ + 'country' => 'GB', + 'language' => [ 'en' ], + 'currency' => [ 'GBP' ], + 'feed_label' => 'GB', + // Stale snapshot from before the merchant switched the global method. + 'shipping_rate' => 'automatic', + 'shipping_time' => 'flat', + ], + ]; + + $this->set_up_options_get( + [ + OptionsInterface::MARKETS => $existing, + OptionsInterface::MERCHANT_CENTER => [ + 'shipping_rate' => 'manual', + 'shipping_time' => 'manual', + ], + ] + ); + $this->set_up_primary_market_dependencies( 'US', [ 'US' ] ); + + $update_calls = []; + $this->options->method( 'update' ) + ->willReturnCallback( + function ( $key, $value ) use ( &$update_calls ) { + $update_calls[ $key ] = $value; + return true; + } + ); + + $this->shipping_settings_job->expects( $this->never() ) + ->method( 'schedule' ); + + // The Edit Market screen echoes the global method back on save. + $this->market_service->update_market( + 'gb', + [ + 'shipping_rate' => 'manual', + 'shipping_time' => 'manual', + ] + ); + + // The submitted params were dropped, not persisted: the stored snapshot is untouched. + $this->assertSame( 'automatic', $update_calls[ OptionsInterface::MARKETS ]['gb']['shipping_rate'] ); + $this->assertSame( 'flat', $update_calls[ OptionsInterface::MARKETS ]['gb']['shipping_time'] ); + } + public function test_update_market_secondary_returns_updated_market(): void { $existing = [ 'gb' => [