From e21430703c449551317a3da0170e5b96da9099ef Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Fri, 26 Apr 2019 11:49:30 -0300 Subject: [PATCH 1/2] Remove redundant calls to var_export() in test_wc_query_string_form_fields() It is not necessary to use the $message parameter of assertEquals() to output the value of $actual_html, as PHPUnit already output this variable value in case of error. --- tests/unit-tests/templates/functions.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/unit-tests/templates/functions.php b/tests/unit-tests/templates/functions.php index 677347b2391..035c4809c41 100644 --- a/tests/unit-tests/templates/functions.php +++ b/tests/unit-tests/templates/functions.php @@ -124,22 +124,22 @@ class WC_Tests_Template_Functions extends WC_Unit_Test_Case { public function test_wc_query_string_form_fields() { $actual_html = wc_query_string_form_fields( '?test=1', array(), '', true ); $expected_html = ''; - $this->assertEquals( $expected_html, $actual_html, var_export( $actual_html, true ) ); // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_var_export + $this->assertEquals( $expected_html, $actual_html ); $actual_html = wc_query_string_form_fields( '?test=1&test2=something', array(), '', true ); $expected_html = ''; - $this->assertEquals( $expected_html, $actual_html, var_export( $actual_html, true ) ); // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_var_export + $this->assertEquals( $expected_html, $actual_html ); $actual_html = wc_query_string_form_fields( '?test.something=something', array(), '', true ); $expected_html = ''; - $this->assertEquals( $expected_html, $actual_html, var_export( $actual_html, true ) ); // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_var_export + $this->assertEquals( $expected_html, $actual_html ); $actual_html = wc_query_string_form_fields( '?test+something=something', array(), '', true ); $expected_html = ''; - $this->assertEquals( $expected_html, $actual_html, var_export( $actual_html, true ) ); // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_var_export + $this->assertEquals( $expected_html, $actual_html ); $actual_html = wc_query_string_form_fields( '?test%20something=something', array(), '', true ); $expected_html = ''; - $this->assertEquals( $expected_html, $actual_html, var_export( $actual_html, true ) ); // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_var_export + $this->assertEquals( $expected_html, $actual_html ); } } From 45c2c78ea28b55ca2f821c65858fc27cc32efe6c Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Fri, 26 Apr 2019 13:58:27 -0300 Subject: [PATCH 2/2] Fix: parameter values should be converted back as well when building form fields PR #23196 added a workaround to `parse_str()` limitation when dealing with full-stops, pluses, and spaces in the parameter key. This workaround involved temporarily replacing those three characters with placeholders before calling `parse_str()` and then replacing back to the original form. This commit fixes a bug in this logic that was replacing back only parameters keys and not parameters values. For example, if the query string is `?query.parameter=foo.bar`, the resulting field contained `foo{dot}bar`, instead of the expected `foo.bar`. --- includes/wc-template-functions.php | 8 +++++--- tests/unit-tests/templates/functions.php | 12 ++++++------ 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/includes/wc-template-functions.php b/includes/wc-template-functions.php index 1425babb547..d22f7ddb8a7 100644 --- a/includes/wc-template-functions.php +++ b/includes/wc-template-functions.php @@ -705,7 +705,7 @@ function wc_query_string_form_fields( $values = null, $exclude = array(), $curre $values = array(); if ( ! empty( $url_parts['query'] ) ) { - // This is to preserve full-stops and spaces in the query string when ran through parse_str. + // This is to preserve full-stops, pluses and spaces in the query string when ran through parse_str. $replace_chars = array( '.' => '{dot}', '+' => '{plus}', @@ -717,9 +717,11 @@ function wc_query_string_form_fields( $values = null, $exclude = array(), $curre // Parse the string. parse_str( $query_string, $parsed_query_string ); - // Convert the full-stops back and add to values array. + // Convert the full-stops, pluses and spaces back and add to values array. foreach ( $parsed_query_string as $key => $value ) { - $values[ str_replace( array_values( $replace_chars ), array_keys( $replace_chars ), $key ) ] = $value; + $new_key = str_replace( array_values( $replace_chars ), array_keys( $replace_chars ), $key ); + $new_value = str_replace( array_values( $replace_chars ), array_keys( $replace_chars ), $value ); + $values[ $new_key ] = $new_value; } } } diff --git a/tests/unit-tests/templates/functions.php b/tests/unit-tests/templates/functions.php index 035c4809c41..eb422a96712 100644 --- a/tests/unit-tests/templates/functions.php +++ b/tests/unit-tests/templates/functions.php @@ -130,16 +130,16 @@ class WC_Tests_Template_Functions extends WC_Unit_Test_Case { $expected_html = ''; $this->assertEquals( $expected_html, $actual_html ); - $actual_html = wc_query_string_form_fields( '?test.something=something', array(), '', true ); - $expected_html = ''; + $actual_html = wc_query_string_form_fields( '?test.something=something.else', array(), '', true ); + $expected_html = ''; $this->assertEquals( $expected_html, $actual_html ); - $actual_html = wc_query_string_form_fields( '?test+something=something', array(), '', true ); - $expected_html = ''; + $actual_html = wc_query_string_form_fields( '?test+something=something+else', array(), '', true ); + $expected_html = ''; $this->assertEquals( $expected_html, $actual_html ); - $actual_html = wc_query_string_form_fields( '?test%20something=something', array(), '', true ); - $expected_html = ''; + $actual_html = wc_query_string_form_fields( '?test%20something=something%20else', array(), '', true ); + $expected_html = ''; $this->assertEquals( $expected_html, $actual_html ); } }