Skip to content

GOOWOO-773 Fix 500 Error Switching Shipping Rate to Manual #3569

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
zamanq wants to merge 5 commits into
base: feature/multi-lingual-support
Choose a base branch
from
+435 −37
Open

GOOWOO-773 Fix 500 Error Switching Shipping Rate to Manual #3569

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b6a13d4
GOOWOO-773 500 Error Switching Shipping Rate
zamanq Jul 7, 2026
0d90221
Merge branch 'feature/multi-lingual-support' of https://github.com/wo…
zamanq Jul 9, 2026
67c2994
Added missing test
zamanq Jul 9, 2026
28142f9
Resolved conflicts.
jamesmorrison Jul 16, 2026
68db2b8
Resolved conflicts.
jamesmorrison Jul 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 96 additions & 17 deletions src/MerchantCenter/MarketService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,19 +177,34 @@ private function has_markets_requiring_conversion(): bool {
/**
* 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 {
$secondary = $this->get_stored_secondary_markets();

$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 ) {
$market = $this->apply_site_locale_when_not_multilingual( $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'] ) )
Expand Down Expand Up @@ -427,7 +442,9 @@ public function add_market( string $id, array $config ): void {
$this->extend_shipping_to_country( $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();
}

Expand All @@ -437,9 +454,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() ) );
}

/**
Expand Down Expand Up @@ -499,6 +517,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 );
Expand Down Expand Up @@ -539,7 +563,9 @@ public function update_market( string $id, array $config ): array {
->schedule( [ 'feed_labels' => $orphaned ] );
}

$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();
Expand Down Expand Up @@ -693,7 +719,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 );
Expand Down Expand Up @@ -721,7 +746,10 @@ public function delete_market( string $id ): void {
);
}

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();
}

Expand All @@ -732,8 +760,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() ) );
}

/**
Expand Down Expand Up @@ -1041,18 +1071,21 @@ public function get_shipping_sync_countries(): array {
/**
* Whether any configured market needs its shipping settings synced to Merchant Center.
*
* Returns true when at least one participating 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 or 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.
*
* Only participating markets count: a syncable secondary market is enough to
* require a sync even when the primary itself is `manual`, while markets
* excluded from syncing (see get_participating_markets()) never require one.
* Every market reflects the global shipping method (see get_markets()).
*
* @return bool
*/
public function has_syncable_markets(): bool {
foreach ( $this->get_participating_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;
}
}
Expand Down Expand Up @@ -1220,6 +1253,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.
*
Expand Down
46 changes: 44 additions & 2 deletions tests/Unit/API/Google/SettingsTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,13 +226,20 @@ function ( $key, $fallback = null ) {
$this->assertSame( 'EUR', $by_country['FR']['currencyCode'] );
}

/**
* 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_participating_markets' )->willReturn(
[
'primary' => [
'country' => 'US',
'currency' => [ 'USD' ],
'shipping_rate' => 'flat',
'shipping_rate' => 'manual',
'shipping_time' => 'flat',
],
'fr' => [
Expand All @@ -248,7 +255,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_participating_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 {
Expand Down
Loading
Loading