From 0896d054a84ad01ebd267a437cbef3562ea5c8f7 Mon Sep 17 00:00:00 2001 From: Thomas Roberts <5656702+opr@users.noreply.github.com> Date: Tue, 28 Feb 2023 06:50:32 +0000 Subject: [PATCH] Fix more instances of hardcoded `pickup_location` string (https://github.com/woocommerce/woocommerce-blocks/pull/8542) * Move get_local_pickup_method_ids to LocalPickupUtils file This is because we need it to be accessible by other classes. * Remove pickup_location strings & get all collectable methods dynamically --------- Co-authored-by: Tung Du --- .../src/BlockTypes/Checkout.php | 3 +- .../src/Shipping/ShippingController.php | 37 +++---------------- .../StoreApi/Utilities/LocalPickupUtils.php | 34 +++++++++++++++++ 3 files changed, 42 insertions(+), 32 deletions(-) create mode 100644 plugins/woocommerce-blocks/src/StoreApi/Utilities/LocalPickupUtils.php diff --git a/plugins/woocommerce-blocks/src/BlockTypes/Checkout.php b/plugins/woocommerce-blocks/src/BlockTypes/Checkout.php index 98044ad7a7b..5e7da29c449 100644 --- a/plugins/woocommerce-blocks/src/BlockTypes/Checkout.php +++ b/plugins/woocommerce-blocks/src/BlockTypes/Checkout.php @@ -2,6 +2,7 @@ namespace Automattic\WooCommerce\Blocks\BlockTypes; use Automattic\WooCommerce\Blocks\Package; +use Automattic\WooCommerce\StoreApi\Utilities\LocalPickupUtils; /** * Checkout class. @@ -277,7 +278,7 @@ class Checkout extends AbstractBlock { $formatted_shipping_methods = array_reduce( $shipping_methods, function( $acc, $method ) { - if ( 'pickup_location' === $method->id ) { + if ( in_array( $method->id, LocalPickupUtils::get_local_pickup_method_ids(), true ) ) { return $acc; } if ( $method->supports( 'settings' ) ) { diff --git a/plugins/woocommerce-blocks/src/Shipping/ShippingController.php b/plugins/woocommerce-blocks/src/Shipping/ShippingController.php index 9735f04cc16..ce88e97fa98 100644 --- a/plugins/woocommerce-blocks/src/Shipping/ShippingController.php +++ b/plugins/woocommerce-blocks/src/Shipping/ShippingController.php @@ -3,6 +3,7 @@ namespace Automattic\WooCommerce\Blocks\Shipping; use Automattic\WooCommerce\Blocks\Assets\Api as AssetApi; use Automattic\WooCommerce\Blocks\Assets\AssetDataRegistry; +use Automattic\WooCommerce\StoreApi\Utilities\LocalPickupUtils; use Automattic\WooCommerce\Utilities\ArrayUtil; /** @@ -49,7 +50,7 @@ class ShippingController { true ); } - $this->asset_data_registry->add( 'collectableMethodIds', array( $this, 'get_local_pickup_method_ids' ), true ); + $this->asset_data_registry->add( 'collectableMethodIds', array( 'Automattic\WooCommerce\StoreApi\Utilities\LocalPickupUtils', 'get_local_pickup_method_ids' ), true ); add_action( 'rest_api_init', [ $this, 'register_settings' ] ); add_action( 'admin_enqueue_scripts', [ $this, 'admin_scripts' ] ); add_action( 'admin_enqueue_scripts', [ $this, 'hydrate_client_settings' ] ); @@ -61,32 +62,6 @@ class ShippingController { add_filter( 'pre_update_option_pickup_location_pickup_locations', array( $this, 'flush_cache' ) ); } - /** - * Gets a list of payment method ids that support the 'local-pickup' feature. - * - * @return string[] List of payment method ids that support the 'local-pickup' feature. - */ - public function get_local_pickup_method_ids() { - $all_methods_supporting_local_pickup = array_reduce( - WC()->shipping()->get_shipping_methods(), - function( $methods, $method ) { - if ( $method->supports( 'local-pickup' ) ) { - $methods[] = $method->id; - } - return $methods; - }, - array() - ); - - // We use array_values because this will be used in JS, so we don't need the (numerical) keys. - return array_values( - // This array_unique is necessary because WC()->shipping()->get_shipping_methods() can return duplicates. - array_unique( - $all_methods_supporting_local_pickup - ) - ); - } - /** * Register Local Pickup settings for rest api. */ @@ -293,7 +268,7 @@ class ShippingController { $chosen_method_instance = explode( ':', $chosen_method )[1] ?? 0; // phpcs:ignore WooCommerce.Commenting.CommentHooks.MissingHookComment - if ( $chosen_method_id && true === apply_filters( 'woocommerce_apply_base_tax_for_local_pickup', true ) && 'pickup_location' === $chosen_method_id ) { + if ( $chosen_method_id && true === apply_filters( 'woocommerce_apply_base_tax_for_local_pickup', true ) && in_array( $chosen_method_id, LocalPickupUtils::get_local_pickup_method_ids(), true ) ) { $pickup_locations = get_option( 'pickup_location_pickup_locations', [] ); $pickup_location = $pickup_locations[ $chosen_method_instance ] ?? []; @@ -321,12 +296,12 @@ class ShippingController { * @return array */ public function filter_shipping_packages( $packages ) { - // Check all packages for an instance of the pickup_location shipping method. + // Check all packages for an instance of a collectable shipping method. $valid_packages = array_filter( $packages, function( $package ) { $shipping_method_ids = ArrayUtil::select( $package['rates'] ?? [], 'get_method_id', ArrayUtil::SELECT_BY_OBJECT_METHOD ); - return in_array( 'pickup_location', $shipping_method_ids, true ); + return ! empty( array_intersect( LocalPickupUtils::get_local_pickup_method_ids(), $shipping_method_ids ) ); } ); @@ -337,7 +312,7 @@ class ShippingController { $package['rates'] = array_filter( $package['rates'], function( $rate ) { - return 'pickup_location' !== $rate->get_method_id(); + return ! in_array( $rate->get_method_id(), LocalPickupUtils::get_local_pickup_method_ids(), true ); } ); return $package; diff --git a/plugins/woocommerce-blocks/src/StoreApi/Utilities/LocalPickupUtils.php b/plugins/woocommerce-blocks/src/StoreApi/Utilities/LocalPickupUtils.php new file mode 100644 index 00000000000..836e3599668 --- /dev/null +++ b/plugins/woocommerce-blocks/src/StoreApi/Utilities/LocalPickupUtils.php @@ -0,0 +1,34 @@ +shipping()->get_shipping_methods(), + function( $methods, $method ) { + if ( $method->supports( 'local-pickup' ) ) { + $methods[] = $method->id; + } + return $methods; + }, + array() + ); + + // We use array_values because this will be used in JS, so we don't need the (numerical) keys. + return array_values( + // This array_unique is necessary because WC()->shipping()->get_shipping_methods() can return duplicates. + array_unique( + $all_methods_supporting_local_pickup + ) + ); + } +}