From 3b8eb75c93d34e2e4863d6829e20773d6a46a0a8 Mon Sep 17 00:00:00 2001 From: Claudiu Lodromanean Date: Wed, 8 Mar 2017 11:51:38 -0800 Subject: [PATCH 1/3] Better handling of nested arrays in apply_changes --- includes/abstracts/abstract-wc-data.php | 9 +++- tests/framework/class-wc-mock-wc-data.php | 16 +++++++ tests/unit-tests/crud/data.php | 54 +++++++++++++++++++++++ tests/unit-tests/order/crud.php | 28 +++++++++++- 4 files changed, 104 insertions(+), 3 deletions(-) diff --git a/includes/abstracts/abstract-wc-data.php b/includes/abstracts/abstract-wc-data.php index 52b1139d05c..8de8d51c54e 100644 --- a/includes/abstracts/abstract-wc-data.php +++ b/includes/abstracts/abstract-wc-data.php @@ -550,7 +550,14 @@ abstract class WC_Data { * @since 2.7.0 */ public function apply_changes() { - $this->data = array_merge( $this->data, $this->changes ); + foreach ( $this->changes as $key => $change ) { + if ( is_array( $change ) ) { + $this->data[ $key ] = array_key_exists( $key, $this->data ) ? array_merge( $this->data[ $key ], $change ) : $change; + } else { + $this->data[ $key ] = $change; + } + } + $this->changes = array(); } diff --git a/tests/framework/class-wc-mock-wc-data.php b/tests/framework/class-wc-mock-wc-data.php index 0e55c615cc4..7dd26e05bb6 100644 --- a/tests/framework/class-wc-mock-wc-data.php +++ b/tests/framework/class-wc-mock-wc-data.php @@ -193,6 +193,22 @@ class WC_Mock_WC_Data extends WC_Data { ); } + /** + * Set the data to any arbitrary data. + * @param array $data + */ + public function set_data( $data ) { + $this->data = $data; + } + + /** + * Set the changes to any arbitrary changes. + * @param array $changes + */ + public function set_changes( $changes ) { + $this->changes = $changes; + } + /** * Simple save. */ diff --git a/tests/unit-tests/crud/data.php b/tests/unit-tests/crud/data.php index 4e4e52cf4fd..164b60f7417 100644 --- a/tests/unit-tests/crud/data.php +++ b/tests/unit-tests/crud/data.php @@ -273,4 +273,58 @@ class WC_Tests_CRUD_Data extends WC_Unit_Test_Case { $object->update_meta_data( 'test_field_0', 'another field 2' ); $this->assertEquals( 'val1', $object->get_meta( 'test_field_2' ) ); } + + function test_apply_changes() { + $data = array( + 'prop1' => 'value1', + 'prop2' => 'value2', + ); + + $changes = array( + 'prop1' => 'new_value1', + 'prop3' => 'value3' + ); + + $object = new WC_Mock_WC_Data; + $object->set_data( $data ); + $object->set_changes( $changes ); + $object->apply_changes(); + + $new_data = $object->get_data(); + $new_changes = $object->get_changes(); + + $this->assertEquals( 'new_value1', $new_data['prop1'] ); + $this->assertEquals( 'value2', $new_data['prop2'] ); + $this->assertEquals( 'value3', $new_data['prop3'] ); + $this->assertEmpty( $new_changes ); + } + + function test_apply_changes_nested() { + $data = array( + 'prop1' => 'value1', + 'prop2' => array( + 'subprop1' => 1, + 'subprop2' => 2, + ), + ); + + $changes = array( + 'prop2' => array( + 'subprop1' => 1000, + 'subprop3' => 3, + ), + ); + + $object = new WC_Mock_WC_Data; + $object->set_data( $data ); + $object->set_changes( $changes ); + $object->apply_changes(); + + $new_data = $object->get_data(); + + $this->assertEquals( 'value1', $new_data['prop1'] ); + $this->assertEquals( 1000, $new_data['prop2']['subprop1'] ); + $this->assertEquals( 2, $new_data['prop2']['subprop2'] ); + $this->assertEquals( 3, $new_data['prop2']['subprop3'] ); + } } diff --git a/tests/unit-tests/order/crud.php b/tests/unit-tests/order/crud.php index d82376aed71..2a46889f373 100644 --- a/tests/unit-tests/order/crud.php +++ b/tests/unit-tests/order/crud.php @@ -814,7 +814,7 @@ class WC_Tests_CRUD_Orders extends WC_Unit_Test_Case { */ function test_get_billing_state() { $object = new WC_Order(); - $set_to = 'Boulder'; + $set_to = 'Oregon'; $object->set_billing_state( $set_to ); $this->assertEquals( $set_to, $object->get_billing_state() ); } @@ -864,6 +864,18 @@ class WC_Tests_CRUD_Orders extends WC_Unit_Test_Case { $this->assertEquals( $set_to, $object->get_billing_phone() ); } + function test_set_billing_after_save() { + $object = new WC_Order(); + $phone = '123456678'; + $object->set_billing_phone( $phone ); + $object->save(); + $state = 'Oregon'; + $object->set_billing_state( $state ); + + $this->assertEquals( $phone, $object->get_billing_phone() ); + $this->assertEquals( $state, $object->get_billing_state() ); + } + /** * Test: get_shipping_first_name */ @@ -929,7 +941,7 @@ class WC_Tests_CRUD_Orders extends WC_Unit_Test_Case { */ function test_get_shipping_state() { $object = new WC_Order(); - $set_to = 'Boulder'; + $set_to = 'Oregon'; $object->set_shipping_state( $set_to ); $this->assertEquals( $set_to, $object->get_shipping_state() ); } @@ -954,6 +966,18 @@ class WC_Tests_CRUD_Orders extends WC_Unit_Test_Case { $this->assertEquals( $set_to, $object->get_shipping_country() ); } + function test_set_shipping_after_save() { + $object = new WC_Order(); + $country = 'US'; + $object->set_shipping_country( $country ); + $object->save(); + $state = 'Oregon'; + $object->set_shipping_state( $state ); + + $this->assertEquals( $country, $object->get_shipping_country() ); + $this->assertEquals( $state, $object->get_shipping_state() ); + } + /** * Test: get_payment_method */ From b645fc55130d47c6aea82ce0eedc9d5680ee63f5 Mon Sep 17 00:00:00 2001 From: Claudiu Lodromanean Date: Wed, 8 Mar 2017 12:02:14 -0800 Subject: [PATCH 2/3] Docblocks --- tests/unit-tests/crud/data.php | 6 ++++++ tests/unit-tests/order/crud.php | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/tests/unit-tests/crud/data.php b/tests/unit-tests/crud/data.php index 164b60f7417..ff978270980 100644 --- a/tests/unit-tests/crud/data.php +++ b/tests/unit-tests/crud/data.php @@ -274,6 +274,9 @@ class WC_Tests_CRUD_Data extends WC_Unit_Test_Case { $this->assertEquals( 'val1', $object->get_meta( 'test_field_2' ) ); } + /** + * Test applying changes + */ function test_apply_changes() { $data = array( 'prop1' => 'value1', @@ -299,6 +302,9 @@ class WC_Tests_CRUD_Data extends WC_Unit_Test_Case { $this->assertEmpty( $new_changes ); } + /** + * Test applying changes with a nested array + */ function test_apply_changes_nested() { $data = array( 'prop1' => 'value1', diff --git a/tests/unit-tests/order/crud.php b/tests/unit-tests/order/crud.php index 2a46889f373..0d413b23a1c 100644 --- a/tests/unit-tests/order/crud.php +++ b/tests/unit-tests/order/crud.php @@ -864,6 +864,9 @@ class WC_Tests_CRUD_Orders extends WC_Unit_Test_Case { $this->assertEquals( $set_to, $object->get_billing_phone() ); } + /** + * Test: Setting/getting billing settings after an order is saved + */ function test_set_billing_after_save() { $object = new WC_Order(); $phone = '123456678'; @@ -966,6 +969,9 @@ class WC_Tests_CRUD_Orders extends WC_Unit_Test_Case { $this->assertEquals( $set_to, $object->get_shipping_country() ); } + /** + * Test: Setting/getting shipping settings after an order is saved + */ function test_set_shipping_after_save() { $object = new WC_Order(); $country = 'US'; From 8a18702c278ae0e3e72491d5d92c7bbd5a90eac7 Mon Sep 17 00:00:00 2001 From: Claudiu Lodromanean Date: Thu, 9 Mar 2017 09:06:05 -0800 Subject: [PATCH 3/3] Just use array_replace_recursive in apply_changes --- includes/abstracts/abstract-wc-data.php | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/includes/abstracts/abstract-wc-data.php b/includes/abstracts/abstract-wc-data.php index 8de8d51c54e..63568afe08d 100644 --- a/includes/abstracts/abstract-wc-data.php +++ b/includes/abstracts/abstract-wc-data.php @@ -550,14 +550,7 @@ abstract class WC_Data { * @since 2.7.0 */ public function apply_changes() { - foreach ( $this->changes as $key => $change ) { - if ( is_array( $change ) ) { - $this->data[ $key ] = array_key_exists( $key, $this->data ) ? array_merge( $this->data[ $key ], $change ) : $change; - } else { - $this->data[ $key ] = $change; - } - } - + $this->data = array_replace_recursive( $this->data, $this->changes ); $this->changes = array(); }