Skip to content

Commit

Permalink
Merge pull request #2527 from woocommerce/fix/flat-method-shipping-co…
Browse files Browse the repository at this point in the history
…st-with-comma-separator

Fix issue with comma separators for Shipping Rates
  • Loading branch information
jorgemd24 authored Aug 27, 2024
2 parents 4c96cd7 + fcf936d commit 59ae632
Show file tree
Hide file tree
Showing 4 changed files with 154 additions and 10 deletions.
26 changes: 26 additions & 0 deletions src/PluginHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -198,4 +198,30 @@ protected function get_site_url(): string {
protected function strip_url_protocol( string $url ): string {
return preg_replace( '#^https?://#', '', untrailingslashit( $url ) );
}

/**
* It tries to convert a string to a decimal number using the dot as the decimal separator.
* This is useful with functions like is_numeric as it doesn't recognize commas as decimal separators or any thousands separator.
* Note: Using wc_format_decimal with a dot as the decimal separator (WC -> Settings -> General) will strip out commas but won’t replace them with dots.
* For example, wc_format_decimal('2,4') will return 24 instead of 2.4.
*
* @param string $numeric_string The number to convert.
*
* @return string The number as a standard decimal. 1.245,63 -> 1245.65
*/
protected function convert_to_standard_decimal( string $numeric_string ): string {
$locale = localeconv();

$separators = [ wc_get_price_decimal_separator(), wc_get_price_thousand_separator(), $locale['thousands_sep'], $locale['mon_thousands_sep'], $locale['decimal_point'], $locale['mon_decimal_point'], ',' ];

if ( wc_get_price_decimals() > 0 ) {
// Replace all posible separators with dots.
$numeric_string = str_replace( $separators, '.', $numeric_string );
// Leave only the last dot that is the decimal separator.
return (string) preg_replace( '/\.(?=.*\.)/', '', $numeric_string );
} else {
// If no decimals remove all separators.
return str_replace( $separators, '', $numeric_string );
}
}
}
11 changes: 7 additions & 4 deletions src/Shipping/ZoneMethodsParser.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

use Automattic\WooCommerce\GoogleListingsAndAds\Infrastructure\Service;
use Automattic\WooCommerce\GoogleListingsAndAds\Proxies\WC;
use Automattic\WooCommerce\GoogleListingsAndAds\PluginHelper;
use WC_Shipping_Method;
use WC_Shipping_Zone;

Expand All @@ -19,6 +20,8 @@
*/
class ZoneMethodsParser implements Service {

use PluginHelper;

public const METHOD_FLAT_RATE = 'flat_rate';
public const METHOD_FREE = 'free_shipping';

Expand Down Expand Up @@ -123,15 +126,15 @@ protected function get_flat_rate_method_rate( object $method ): ?float {
$rate = null;

$flat_cost = 0;
$cost = $method->get_option( 'cost' );
$cost = $this->convert_to_standard_decimal( (string) $method->get_option( 'cost' ) );
// Check if the cost is a numeric value (and not null or a math expression).
if ( is_numeric( $cost ) ) {
$flat_cost = (float) $cost;
$rate = $flat_cost;
}

// Add the no class cost.
$no_class_cost = $method->get_option( 'no_class_cost' );
$no_class_cost = $this->convert_to_standard_decimal( (string) $method->get_option( 'no_class_cost' ) );
if ( is_numeric( $no_class_cost ) ) {
$rate = $flat_cost + (float) $no_class_cost;
}
Expand All @@ -155,7 +158,7 @@ protected function get_flat_rate_method_class_rates( object $method ): array {
$class_rates = [];

$flat_cost = 0;
$cost = $method->get_option( 'cost' );
$cost = $this->convert_to_standard_decimal( (string) $method->get_option( 'cost' ) );
// Check if the cost is a numeric value (and not null or a math expression).
if ( is_numeric( $cost ) ) {
$flat_cost = (float) $cost;
Expand All @@ -164,7 +167,7 @@ protected function get_flat_rate_method_class_rates( object $method ): array {
// Add shipping class costs.
$shipping_classes = $this->wc->get_shipping_classes();
foreach ( $shipping_classes as $shipping_class ) {
$shipping_class_cost = $method->get_option( 'class_cost_' . $shipping_class->term_id );
$shipping_class_cost = $this->convert_to_standard_decimal( (string) $method->get_option( 'class_cost_' . $shipping_class->term_id ) );
if ( is_numeric( $shipping_class_cost ) ) {
// Add the flat rate cost to the shipping class cost.
$class_rates[ $shipping_class->slug ] = [
Expand Down
73 changes: 73 additions & 0 deletions tests/Unit/Plugin/PluginHelperTraitTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
<?php
declare( strict_types=1 );

namespace Automattic\WooCommerce\GoogleListingsAndAds\Tests\Unit\API\Site\Controllers;

use PHPUnit\Framework\TestCase;
use Automattic\WooCommerce\GoogleListingsAndAds\PluginHelper;


/**
* Class PluginHelperTraitTest
*
* @package Automattic\WooCommerce\GoogleListingsAndAds\Tests\Unit\API\Site\Controllers
*/
class PluginHelperTraitTest extends TestCase {


/** @var PluginHelper $trait */
protected $trait;

/**
* Runs before each test is executed.
*/
public function setUp(): void {
parent::setUp();

// phpcs:ignore Universal.Classes.RequireAnonClassParentheses.Missing
$this->trait = new class {
use PluginHelper {
convert_to_standard_decimal as public;
}
};
}


public function test_comma_decimals_gets_converted_to_dot_decimals() {
$this->assertEquals( '10.5', $this->trait->convert_to_standard_decimal( '10,5' ) );
}

public function test_dot_decimals_remain_unchanged() {
$this->assertEquals( '10.5', $this->trait->convert_to_standard_decimal( '10.5' ) );
}

public function test_invalid_numbers() {
$this->assertEquals( 'no valid. number', $this->trait->convert_to_standard_decimal( 'no valid, number' ) );
}

public function test_with_thousands_separator() {
$this->assertEquals( '1234.5', $this->trait->convert_to_standard_decimal( '1' . wc_get_price_thousand_separator() . '234,5' ) );
}

public function test_with_no_decimals() {
$this->assertEquals( '12345', $this->trait->convert_to_standard_decimal( '12345' ) );
}

public function test_with_different_wc_decimal_separator() {
add_filter(
'wc_get_price_decimal_separator',
[ $this, 'callback_filter_decimal_separator' ]
);

$this->assertEquals( '10.5', $this->trait->convert_to_standard_decimal( '10-5' ) );
}

public function callback_filter_decimal_separator() {
return '-';
}

public function tearDown(): void {
parent::tearDown();
remove_filter( 'wc_get_price_decimal_separator', [ $this, 'callback_filter_decimal_separator' ] );
}
}
54 changes: 48 additions & 6 deletions tests/Unit/Shipping/ZoneMethodsParserTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,48 @@ public function test_returns_flat_rate_methods() {
$this->assertEquals( 10, $shipping_rates[0]->get_rate() );
}

public function test_returns_flat_rate_methods_with_decimals_and_comma() {
$flat_rate = $this->createMock( WC_Shipping_Flat_Rate::class );
$flat_rate->id = ZoneMethodsParser::METHOD_FLAT_RATE;
$flat_rate->expects( $this->any() )
->method( 'get_option' )
->willReturnMap(
[
[ 'cost', null, '10,6' ],
]
);

$zone = $this->createMock( WC_Shipping_Zone::class );
$zone->expects( $this->any() )
->method( 'get_shipping_methods' )
->willReturn( [ $flat_rate ] );

$shipping_rates = $this->methods_parser->parse( $zone );
$this->assertCount( 1, $shipping_rates );
$this->assertEquals( 10.6, $shipping_rates[0]->get_rate() );
}

public function test_returns_flat_rate_methods_with_decimals_and_dot() {
$flat_rate = $this->createMock( WC_Shipping_Flat_Rate::class );
$flat_rate->id = ZoneMethodsParser::METHOD_FLAT_RATE;
$flat_rate->expects( $this->any() )
->method( 'get_option' )
->willReturnMap(
[
[ 'cost', null, '10.6' ],
]
);

$zone = $this->createMock( WC_Shipping_Zone::class );
$zone->expects( $this->any() )
->method( 'get_shipping_methods' )
->willReturn( [ $flat_rate ] );

$shipping_rates = $this->methods_parser->parse( $zone );
$this->assertCount( 1, $shipping_rates );
$this->assertEquals( 10.6, $shipping_rates[0]->get_rate() );
}

public function test_returns_flat_rate_methods_including_shipping_classes() {
// Return three sample shipping classes.
$light_class = new \stdClass();
Expand All @@ -69,10 +111,10 @@ public function test_returns_flat_rate_methods_including_shipping_classes() {
->willReturnMap(
[
[ 'cost', null, 10 ],
[ 'class_cost_0', null, 5 ],
[ 'class_cost_0', null, '5,20' ], // Comma decimal separator will be converted to dot.
[ 'class_cost_1', null, 15 ],
[ 'class_cost_2', null, '[qty] / 10' ],
[ 'no_class_cost', null, 2 ],
[ 'no_class_cost', null, '6,7' ],
]
);

Expand Down Expand Up @@ -103,11 +145,11 @@ function ( ShippingRate $shipping_rate ) {

foreach ( $shipping_rates as $shipping_rate ) {
if ( [] === $shipping_rate->get_applicable_classes() ) {
// The `no_class_cost` should be added to the flat rate method cost (10+2=12).
$this->assertEquals( 12, $shipping_rate->get_rate() );
// The `no_class_cost` should be added to the flat rate method cost (10+6.7=16.7).
$this->assertEquals( 16.7, $shipping_rate->get_rate() );
} elseif ( [ 'light' ] === $shipping_rate->get_applicable_classes() ) {
// The shipping class costs should be added to the flat rate method cost (10+5=15).
$this->assertEquals( 15, $shipping_rate->get_rate() );
// The shipping class costs should be added to the flat rate method cost (10+5.20=15.20).
$this->assertEquals( 15.20, $shipping_rate->get_rate() );
} elseif ( [ 'heavy' ] === $shipping_rate->get_applicable_classes() ) {
// The shipping class costs should be added to the flat rate method cost (10+15=25).
$this->assertEquals( 25, $shipping_rate->get_rate() );
Expand Down

0 comments on commit 59ae632

Please sign in to comment.