From 4e7d3333cfbf1949034913a6cb5fae8227be4f0c Mon Sep 17 00:00:00 2001 From: claudiulodro Date: Tue, 27 Jun 2017 13:53:56 -0700 Subject: [PATCH 1/2] Use type check in coupon constructor --- includes/class-wc-cart.php | 7 +++++++ includes/class-wc-coupon.php | 13 ++++++++++--- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/includes/class-wc-cart.php b/includes/class-wc-cart.php index 45614f6ea21..c8e66307fa3 100644 --- a/includes/class-wc-cart.php +++ b/includes/class-wc-cart.php @@ -1736,6 +1736,13 @@ class WC_Cart { // Get the coupon. $the_coupon = new WC_Coupon( $coupon_code ); + // Prevent adding coupons by post ID. + if ( $the_coupon->get_code() !== $coupon_code ) { + $the_coupon->set_code( $coupon_code ); + $the_coupon->add_coupon_message( WC_Coupon::E_WC_COUPON_NOT_EXIST ); + return false; + } + // Check it can be used with cart. if ( ! $the_coupon->is_valid() ) { wc_add_notice( $the_coupon->get_error_message(), 'error' ); diff --git a/includes/class-wc-coupon.php b/includes/class-wc-coupon.php index 12db1fc16ae..d61b0eb5016 100644 --- a/includes/class-wc-coupon.php +++ b/includes/class-wc-coupon.php @@ -85,11 +85,18 @@ class WC_Coupon extends WC_Legacy_Coupon { } elseif ( $coupon = apply_filters( 'woocommerce_get_shop_coupon_data', false, $data ) ) { $this->read_manual_coupon( $data, $coupon ); return; - } elseif ( is_numeric( $data ) && 'shop_coupon' === get_post_type( $data ) ) { + } elseif ( is_int( $data ) && 'shop_coupon' === get_post_type( $data ) ) { $this->set_id( $data ); } elseif ( ! empty( $data ) ) { - $this->set_id( wc_get_coupon_id_by_code( $data ) ); - $this->set_code( $data ); + $id = wc_get_coupon_id_by_code( $data ); + + // Need to support numeric strings for backwards compatibility. + if ( ! $id && 'shop_coupon' === get_post_type( $data ) ) { + $this->set_id( $data ); + } else { + $this->set_id( $id ); + $this->set_code( $data ); + } } else { $this->set_object_read( true ); } From f5d5890a501c4fa5255941cc626ff1ea1e1f7d41 Mon Sep 17 00:00:00 2001 From: claudiulodro Date: Thu, 29 Jun 2017 14:05:38 -0700 Subject: [PATCH 2/2] Add tests --- tests/unit-tests/cart/cart.php | 19 ++++++++++++++ tests/unit-tests/coupon/coupon.php | 40 ++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/tests/unit-tests/cart/cart.php b/tests/unit-tests/cart/cart.php index e0a1983f6cc..a00c180c622 100644 --- a/tests/unit-tests/cart/cart.php +++ b/tests/unit-tests/cart/cart.php @@ -587,6 +587,25 @@ class WC_Tests_Cart extends WC_Unit_Test_Case { } + /** + * Test add_discount allows coupons by code but not by ID. + * + * @since 3.2 + */ + public function test_add_discount_code_id() { + + $coupon = new WC_Coupon; + $coupon->set_code( 'test' ); + $coupon->set_amount( 100 ); + $coupon->save(); + + $success = WC()->cart->add_discount( $coupon->get_code() ); + $this->assertTrue( $success ); + + $success = WC()->cart->add_discount( (string) $coupon->get_id() ); + $this->assertFalse( $success ); + } + public function test_add_invidual_use_coupon() { $iu_coupon = WC_Helper_Coupon::create_coupon( 'code1' ); $iu_coupon->set_individual_use( true ); diff --git a/tests/unit-tests/coupon/coupon.php b/tests/unit-tests/coupon/coupon.php index 7476c0fa054..d20c1df9097 100644 --- a/tests/unit-tests/coupon/coupon.php +++ b/tests/unit-tests/coupon/coupon.php @@ -6,6 +6,46 @@ */ class WC_Tests_Coupon extends WC_Unit_Test_Case { + /** + * Test the code/id differentiation of the coupon constructor. + * + * @since 3.2 + */ + public function test_constructor_code_id() { + $string_code_1 = 'test'; + + // Coupon with a standard string code. + $coupon_1 = new WC_Coupon; + $coupon_1->set_code( $string_code_1 ); + $coupon_1->save(); + + // Coupon with a string code that is the same as coupon 1's ID. + $coupon_2 = new WC_Coupon; + $coupon_2->set_code( (string) $coupon_1->get_id() ); + $coupon_2->save(); + + $int_id_1 = $coupon_1->get_id(); + $int_id_2 = $coupon_2->get_id(); + $string_code_2 = $coupon_2->get_code(); + + // Test getting a coupon by integer ID. + $test_coupon = new WC_Coupon( $int_id_1 ); + $this->assertEquals( $int_id_1, $test_coupon->get_id() ); + $test_coupon = new WC_Coupon( $int_id_2 ); + $this->assertEquals( $int_id_2, $test_coupon->get_id() ); + + // Test getting a coupon by string code. + $test_coupon = new WC_Coupon( $string_code_1 ); + $this->assertEquals( $string_code_1, $test_coupon->get_code() ); + $test_coupon = new WC_Coupon( $string_code_2 ); + $this->assertEquals( $string_code_2, $test_coupon->get_code() ); + + // Test getting a coupon by string id. + // Required for backwards compatibility, but will try and initialize coupon by code if possible first. + $test_coupon = new WC_Coupon( (string) $coupon_2->get_id() ); + $this->assertEquals( $coupon_2->get_id(), $test_coupon->get_id() ); + } + /** * Test add_discount method. *