From 64f4fb85e2e42fa60d9184f1bb9975264d1b76e8 Mon Sep 17 00:00:00 2001 From: Claudio Sanches Date: Thu, 23 Jul 2020 19:10:42 -0300 Subject: [PATCH 1/2] Move variation validation logic to add to cart --- includes/class-wc-cart.php | 91 +++++++++++++- includes/class-wc-form-handler.php | 112 ++---------------- tests/legacy/unit-tests/cart/cart.php | 15 ++- tests/legacy/unit-tests/checkout/checkout.php | 20 +++- tests/legacy/unit-tests/totals/totals.php | 10 +- tests/php/includes/class-wc-cart-test.php | 63 ++++++++++ 6 files changed, 202 insertions(+), 109 deletions(-) create mode 100644 tests/php/includes/class-wc-cart-test.php diff --git a/includes/class-wc-cart.php b/includes/class-wc-cart.php index 59d853710b7..b0ffff4cd79 100644 --- a/includes/class-wc-cart.php +++ b/includes/class-wc-cart.php @@ -1013,6 +1013,95 @@ class WC_Cart extends WC_Legacy_Cart { return false; } + if ( $product_data->is_type( 'variation' ) ) { + $missing_attributes = array(); + $parent_data = wc_get_product( $product_data->get_parent_id() ); + + $variation_attributes = $product_data->get_variation_attributes(); + // Filter out 'any' variations, which are empty, as they need to be explicitly specified while adding to cart. + $variation_attributes = array_filter( $variation_attributes ); + + // Gather posted attributes. + $posted_attributes = array(); + + foreach ( $parent_data->get_attributes() as $attribute ) { + if ( ! $attribute['is_variation'] ) { + continue; + } + $attribute_key = 'attribute_' . sanitize_title( $attribute['name'] ); + + if ( isset( $variation[ $attribute_key ] ) ) { // phpcs:ignore WordPress.Security.NonceVerification.Recommended + if ( $attribute['is_taxonomy'] ) { + // Don't use wc_clean as it destroys sanitized characters. + $value = sanitize_title( wp_unslash( $variation[ $attribute_key ] ) ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended + } else { + $value = html_entity_decode( wc_clean( wp_unslash( $variation[ $attribute_key ] ) ), ENT_QUOTES, get_bloginfo( 'charset' ) ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended + } + + // Don't include if it's empty. + if ( ! empty( $value ) ) { + $posted_attributes[ $attribute_key ] = $value; + } + } + } + + // Merge variation attributes and posted attributes. + $posted_and_variation_attributes = array_merge( $variation_attributes, $posted_attributes ); + + // If no variation ID is set, attempt to get a variation ID from posted attributes. + if ( empty( $variation_id ) ) { + $data_store = WC_Data_Store::load( 'product' ); + $variation_id = $data_store->find_matching_product_variation( $parent_data, $posted_attributes ); + } + + // Do we have a variation ID? + if ( empty( $variation_id ) ) { + throw new Exception( __( 'Please choose product options…', 'woocommerce' ) ); + } + + // Check the data we have is valid. + $variation_data = wc_get_product_variation_attributes( $variation_id ); + $attributes = array(); + + foreach ( $parent_data->get_attributes() as $attribute ) { + if ( ! $attribute['is_variation'] ) { + continue; + } + + // Get valid value from variation data. + $attribute_key = 'attribute_' . sanitize_title( $attribute['name'] ); + $valid_value = isset( $variation_data[ $attribute_key ] ) ? $variation_data[ $attribute_key ] : ''; + + /** + * If the attribute value was posted, check if it's valid. + * + * If no attribute was posted, only error if the variation has an 'any' attribute which requires a value. + */ + if ( isset( $posted_and_variation_attributes[ $attribute_key ] ) ) { + $value = $posted_and_variation_attributes[ $attribute_key ]; + + // Allow if valid or show error. + if ( $valid_value === $value ) { + $attributes[ $attribute_key ] = $value; + } elseif ( '' === $valid_value && in_array( $value, $attribute->get_slugs(), true ) ) { + // If valid values are empty, this is an 'any' variation so get all possible values. + $attributes[ $attribute_key ] = $value; + } else { + /* translators: %s: Attribute name. */ + throw new Exception( sprintf( __( 'Invalid value posted for %s', 'woocommerce' ), wc_attribute_label( $attribute['name'] ) ) ); + } + } elseif ( '' === $valid_value ) { + $missing_attributes[] = wc_attribute_label( $attribute['name'] ); + } + + $variation = $attributes; + } + if ( ! empty( $missing_attributes ) ) { + /* translators: %s: Attribute name. */ + throw new Exception( sprintf( _n( '%s is a required field', '%s are required fields', count( $missing_attributes ), 'woocommerce' ), wc_format_list_of_items( $missing_attributes ) ) ); + } + } + // Load cart item data - may be added by other plugins. $cart_item_data = (array) apply_filters( 'woocommerce_add_cart_item_data', $cart_item_data, $product_id, $variation_id, $quantity ); @@ -1471,7 +1560,7 @@ class WC_Cart extends WC_Legacy_Cart { if ( 0 < $coupon_usage_limit && 0 === get_current_user_id() ) { // For guest, usage per user has not been enforced yet. Enforce it now. $coupon_data_store = $coupon->get_data_store(); - $billing_email = strtolower( sanitize_email( $billing_email ) ); + $billing_email = strtolower( sanitize_email( $billing_email ) ); if ( $coupon_data_store && $coupon_data_store->get_usage_by_email( $coupon, $billing_email ) >= $coupon_usage_limit ) { $coupon->add_coupon_message( WC_Coupon::E_WC_COUPON_USAGE_LIMIT_REACHED ); } diff --git a/includes/class-wc-form-handler.php b/includes/class-wc-form-handler.php index 80a4336aba5..498870eb954 100644 --- a/includes/class-wc-form-handler.php +++ b/includes/class-wc-form-handler.php @@ -46,7 +46,7 @@ class WC_Form_Handler { $user = get_user_by( 'login', sanitize_user( wp_unslash( $_GET['login'] ) ) ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended $user_id = $user ? $user->ID : 0; } else { - $user_id = absint( $_GET['id'] ); + $user_id = absint( $_GET['id'] ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended } $value = sprintf( '%d:%s', $user_id, wp_unslash( $_GET['key'] ) ); // phpcs:ignore @@ -638,7 +638,7 @@ class WC_Form_Handler { if ( ( ! empty( $_POST['apply_coupon'] ) || ! empty( $_POST['update_cart'] ) || ! empty( $_POST['proceed'] ) ) && wp_verify_nonce( $nonce_value, 'woocommerce-cart' ) ) { $cart_updated = false; - $cart_totals = isset( $_POST['cart'] ) ? wp_unslash( $_POST['cart'] ) : ''; // PHPCS: input var ok, CSRF ok, sanitization ok. + $cart_totals = isset( $_POST['cart'] ) ? wp_unslash( $_POST['cart'] ) : ''; // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized if ( ! WC()->cart->is_empty() && is_array( $cart_totals ) ) { foreach ( WC()->cart->get_cart() as $cart_item_key => $values ) { @@ -868,108 +868,16 @@ class WC_Form_Handler { * @return bool success or not */ private static function add_to_cart_handler_variable( $product_id ) { - try { - $variation_id = empty( $_REQUEST['variation_id'] ) ? '' : absint( wp_unslash( $_REQUEST['variation_id'] ) ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended - $quantity = empty( $_REQUEST['quantity'] ) ? 1 : wc_stock_amount( wp_unslash( $_REQUEST['quantity'] ) ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended - $missing_attributes = array(); - $variations = array(); - $variation_attributes = array(); - $adding_to_cart = wc_get_product( $product_id ); + $variation_id = empty( $_REQUEST['variation_id'] ) ? '' : absint( wp_unslash( $_REQUEST['variation_id'] ) ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended + $quantity = empty( $_REQUEST['quantity'] ) ? 1 : wc_stock_amount( wp_unslash( $_REQUEST['quantity'] ) ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended + $variations = array(); - if ( ! $adding_to_cart ) { - return false; + foreach ( $_REQUEST as $key => $value ) { // phpcs:ignore WordPress.Security.NonceVerification.Recommended + if ( 'attribute_' !== substr( $key, 0, 10 ) ) { + continue; } - // If the $product_id was in fact a variation ID, update the variables. - if ( $adding_to_cart->is_type( 'variation' ) ) { - $variation_attributes = $adding_to_cart->get_variation_attributes(); - // Filter out 'any' variations, which are empty, as they need to be explicitly specified while adding to cart. - $variation_attributes = array_filter( $variation_attributes ); - $variation_id = $product_id; - $product_id = $adding_to_cart->get_parent_id(); - $adding_to_cart = wc_get_product( $product_id ); - - if ( ! $adding_to_cart ) { - return false; - } - } - - // Gather posted attributes. - $posted_attributes = array(); - - foreach ( $adding_to_cart->get_attributes() as $attribute ) { - if ( ! $attribute['is_variation'] ) { - continue; - } - $attribute_key = 'attribute_' . sanitize_title( $attribute['name'] ); - - if ( isset( $_REQUEST[ $attribute_key ] ) ) { // phpcs:ignore WordPress.Security.NonceVerification.Recommended - if ( $attribute['is_taxonomy'] ) { - // Don't use wc_clean as it destroys sanitized characters. - $value = sanitize_title( wp_unslash( $_REQUEST[ $attribute_key ] ) ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended - } else { - $value = html_entity_decode( wc_clean( wp_unslash( $_REQUEST[ $attribute_key ] ) ), ENT_QUOTES, get_bloginfo( 'charset' ) ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended - } - - $posted_attributes[ $attribute_key ] = $value; - } - } - - // Merge variation attributes and posted attributes. - $posted_and_variation_attributes = array_merge( $variation_attributes, $posted_attributes ); - - // If no variation ID is set, attempt to get a variation ID from posted attributes. - if ( empty( $variation_id ) ) { - $data_store = WC_Data_Store::load( 'product' ); - $variation_id = $data_store->find_matching_product_variation( $adding_to_cart, $posted_attributes ); - } - - // Do we have a variation ID? - if ( empty( $variation_id ) ) { - throw new Exception( __( 'Please choose product options…', 'woocommerce' ) ); - } - - // Check the data we have is valid. - $variation_data = wc_get_product_variation_attributes( $variation_id ); - - foreach ( $adding_to_cart->get_attributes() as $attribute ) { - if ( ! $attribute['is_variation'] ) { - continue; - } - - // Get valid value from variation data. - $attribute_key = 'attribute_' . sanitize_title( $attribute['name'] ); - $valid_value = isset( $variation_data[ $attribute_key ] ) ? $variation_data[ $attribute_key ] : ''; - - /** - * If the attribute value was posted, check if it's valid. - * - * If no attribute was posted, only error if the variation has an 'any' attribute which requires a value. - */ - if ( isset( $posted_and_variation_attributes[ $attribute_key ] ) ) { - $value = $posted_and_variation_attributes[ $attribute_key ]; - - // Allow if valid or show error. - if ( $valid_value === $value ) { - $variations[ $attribute_key ] = $value; - } elseif ( '' === $valid_value && in_array( $value, $attribute->get_slugs(), true ) ) { - // If valid values are empty, this is an 'any' variation so get all possible values. - $variations[ $attribute_key ] = $value; - } else { - /* translators: %s: Attribute name. */ - throw new Exception( sprintf( __( 'Invalid value posted for %s', 'woocommerce' ), wc_attribute_label( $attribute['name'] ) ) ); - } - } elseif ( '' === $valid_value ) { - $missing_attributes[] = wc_attribute_label( $attribute['name'] ); - } - } - if ( ! empty( $missing_attributes ) ) { - /* translators: %s: Attribute name. */ - throw new Exception( sprintf( _n( '%s is a required field', '%s are required fields', count( $missing_attributes ), 'woocommerce' ), wc_format_list_of_items( $missing_attributes ) ) ); - } - } catch ( Exception $e ) { - wc_add_notice( $e->getMessage(), 'error' ); - return false; + $variations[ sanitize_title( wp_unslash( $key ) ) ] = wp_unslash( $value ); } $passed_validation = apply_filters( 'woocommerce_add_to_cart_validation', true, $product_id, $quantity, $variation_id, $variations ); @@ -1083,7 +991,7 @@ class WC_Form_Handler { return; } - if ( in_array( $field, array( 'password_1', 'password_2' ) ) ) { + if ( in_array( $field, array( 'password_1', 'password_2' ), true ) ) { // Don't unslash password fields // @see https://github.com/woocommerce/woocommerce/issues/23922. $posted_fields[ $field ] = $_POST[ $field ]; // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized, WordPress.Security.ValidatedSanitizedInput.MissingUnslash diff --git a/tests/legacy/unit-tests/cart/cart.php b/tests/legacy/unit-tests/cart/cart.php index 8c54669d7df..9d0a74f689c 100644 --- a/tests/legacy/unit-tests/cart/cart.php +++ b/tests/legacy/unit-tests/cart/cart.php @@ -1311,7 +1311,16 @@ class WC_Tests_Cart extends WC_Unit_Test_Case { $variation = array_shift( $variations ); // Add the product to the cart. Methods returns boolean on failure, string on success. - $this->assertNotFalse( WC()->cart->add_to_cart( $product->get_id(), 1, $variation['variation_id'], array( 'Size' => ucfirst( $variation['attributes']['attribute_pa_size'] ) ) ) ); + $result = WC()->cart->add_to_cart( + $product->get_id(), + 1, + $variation['variation_id'], + array( + 'attribute_pa_colour' => 'red', // Set a value since this is an 'any' attribute. + 'attribute_pa_number' => '2', // Set a value since this is an 'any' attribute. + ) + ); + $this->assertNotFalse( $result ); // Check if the item is in the cart. $this->assertEquals( 1, WC()->cart->get_cart_contents_count() ); @@ -2200,10 +2209,10 @@ class WC_Tests_Cart extends WC_Unit_Test_Case { $this->assertCount( 0, WC()->cart->get_cart_contents() ); $this->assertEquals( 0, WC()->cart->get_cart_contents_count() ); - // Check that the notices contain an error message about an invalid colour. + // Check that the notices contain an error message about invalid colour and number. $this->assertArrayHasKey( 'error', $notices ); $this->assertCount( 1, $notices['error'] ); - $this->assertEquals( 'colour is a required field', $notices['error'][0]['notice'] ); + $this->assertEquals( 'colour and number are required fields', $notices['error'][0]['notice'] ); } /** diff --git a/tests/legacy/unit-tests/checkout/checkout.php b/tests/legacy/unit-tests/checkout/checkout.php index d6639ec88ab..da883e05897 100644 --- a/tests/legacy/unit-tests/checkout/checkout.php +++ b/tests/legacy/unit-tests/checkout/checkout.php @@ -279,7 +279,15 @@ class WC_Tests_Checkout extends WC_Unit_Test_Case { $variation->set_manage_stock( true ); $variation->set_stock_quantity( 10 ); $variation->save(); - WC()->cart->add_to_cart( $variation->get_id(), 9 ); + WC()->cart->add_to_cart( + $variation->get_id(), + 9, + 0, + array( + 'attribute_pa_colour' => 'red', // Set a value since this is an 'any' attribute. + 'attribute_pa_number' => '2', // Set a value since this is an 'any' attribute. + ) + ); $this->assertEquals( true, WC()->cart->check_cart_items() ); $checkout = WC_Checkout::instance(); @@ -299,7 +307,15 @@ class WC_Tests_Checkout extends WC_Unit_Test_Case { $this->assertEquals( 9, wc_get_held_stock_quantity( $variation ) ); WC()->cart->empty_cart(); - WC()->cart->add_to_cart( $variation->get_stock_managed_by_id(), 2 ); + WC()->cart->add_to_cart( + $variation->get_stock_managed_by_id(), + 2, + 0, + array( + 'attribute_pa_colour' => 'red', + 'attribute_pa_number' => '2', + ) + ); $this->assertEquals( false, WC()->cart->check_cart_items() ); } diff --git a/tests/legacy/unit-tests/totals/totals.php b/tests/legacy/unit-tests/totals/totals.php index 2edd6d00fd2..71cca3b3279 100644 --- a/tests/legacy/unit-tests/totals/totals.php +++ b/tests/legacy/unit-tests/totals/totals.php @@ -88,7 +88,15 @@ class WC_Tests_Totals extends WC_Unit_Test_Case { WC()->cart->add_to_cart( $product2->get_id(), 2 ); $variations = $product3->get_available_variations(); $variation = array_shift( $variations ); - WC()->cart->add_to_cart( $product3->get_id(), 1, $variation['variation_id'], array( 'Size' => ucfirst( $variation['attributes']['attribute_pa_size'] ) ) ); + WC()->cart->add_to_cart( + $product3->get_id(), + 1, + $variation['variation_id'], + array( + 'attribute_pa_colour' => 'red', // Set a value since this is an 'any' attribute. + 'attribute_pa_number' => '2', // Set a value since this is an 'any' attribute. + ) + ); WC()->cart->add_discount( $coupon->get_code() ); diff --git a/tests/php/includes/class-wc-cart-test.php b/tests/php/includes/class-wc-cart-test.php new file mode 100644 index 00000000000..6dd6e7fb7c6 --- /dev/null +++ b/tests/php/includes/class-wc-cart-test.php @@ -0,0 +1,63 @@ +cart->empty_cart(); + WC()->customer->set_is_vat_exempt( false ); + WC()->session->set( 'wc_notices', null ); + } + + /** + * @testdox should throw a notice to the cart if an "any" attribute is empty. + */ + public function test_add_variation_to_the_cart_with_empty_attributes() { + WC()->cart->empty_cart(); + WC()->session->set( 'wc_notices', null ); + + $product = WC_Helper_Product::create_variation_product(); + $variations = $product->get_available_variations(); + + // Get a variation with small pa_size and any pa_colour and pa_number. + $variation = $variations[0]; + + // Add variation using parent id. + WC()->cart->add_to_cart( + $variation['variation_id'], + 1, + 0, + array( + 'attribute_pa_colour' => '', + 'attribute_pa_number' => '', + ) + ); + $notices = WC()->session->get( 'wc_notices', array() ); + + // Check that the second add to cart call increases the quantity of the existing cart-item. + $this->assertCount( 0, WC()->cart->get_cart_contents() ); + $this->assertEquals( 0, WC()->cart->get_cart_contents_count() ); + + // Check that the notices contain an error message about invalid colour and number. + $this->assertArrayHasKey( 'error', $notices ); + $this->assertCount( 1, $notices['error'] ); + $this->assertEquals( 'colour and number are required fields', $notices['error'][0]['notice'] ); + + // Reset cart. + WC()->cart->empty_cart(); + WC()->customer->set_is_vat_exempt( false ); + $product->delete( true ); + } +} From 2043954cebf3ccddc62e4f1c08751fc7e051a7f5 Mon Sep 17 00:00:00 2001 From: Claudio Sanches Date: Mon, 27 Jul 2020 21:53:28 -0300 Subject: [PATCH 2/2] Fixed class name --- tests/php/includes/class-wc-cart-test.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/php/includes/class-wc-cart-test.php b/tests/php/includes/class-wc-cart-test.php index 6dd6e7fb7c6..d609f90322e 100644 --- a/tests/php/includes/class-wc-cart-test.php +++ b/tests/php/includes/class-wc-cart-test.php @@ -1,14 +1,14 @@