From 32f8ba28ca34504bad185f3457cb806ebc209e48 Mon Sep 17 00:00:00 2001 From: Geoff Taylor Date: Wed, 6 Nov 2024 15:00:11 -0500 Subject: [PATCH] =?UTF-8?q?fix:=20ID=20resolution=20made=20consistent=20ac?= =?UTF-8?q?ross=20all=20edit=20and=20delete=20node=20mu=E2=80=A6=20(#902)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: ID resolution made consistent across all edit and delete node mutations * chore: linter compliances met --- .../data/mutation/class-order-mutation.php | 77 ++++++++++++------- includes/mutation/class-coupon-create.php | 18 ++--- includes/mutation/class-coupon-delete.php | 16 ++-- includes/mutation/class-order-create.php | 2 +- .../mutation/class-order-delete-items.php | 23 +++--- includes/mutation/class-order-delete.php | 23 +++--- includes/mutation/class-order-update.php | 29 +++---- .../mutation/class-review-delete-restore.php | 7 +- includes/mutation/class-review-update.php | 12 ++- includes/mutation/class-tax-rate-create.php | 8 +- includes/mutation/class-tax-rate-delete.php | 8 +- tests/wpunit/OrderMutationsTest.php | 6 +- 12 files changed, 127 insertions(+), 102 deletions(-) diff --git a/includes/data/mutation/class-order-mutation.php b/includes/data/mutation/class-order-mutation.php index 525e62ac..41008d61 100644 --- a/includes/data/mutation/class-order-mutation.php +++ b/includes/data/mutation/class-order-mutation.php @@ -9,6 +9,7 @@ namespace WPGraphQL\WooCommerce\Data\Mutation; use GraphQL\Error\UserError; +use WPGraphQL\Utils\Utils; /** @@ -22,8 +23,9 @@ class Order_Mutation { * @param \WPGraphQL\AppContext $context AppContext instance. * @param \GraphQL\Type\Definition\ResolveInfo $info ResolveInfo instance. * @param string $mutation Mutation being executed. - * @param integer|null $order_id Order ID. - * + * @param integer|null|false $order_id Order ID. + * @throws \GraphQL\Error\UserError Error locating order. + * * @return boolean */ public static function authorized( $input, $context, $info, $mutation = 'create', $order_id = null ) { @@ -34,18 +36,38 @@ public static function authorized( $input, $context, $info, $mutation = 'create' */ $post_type_object = get_post_type_object( 'shop_order' ); - return apply_filters( - "graphql_woocommerce_authorized_to_{$mutation}_orders", - current_user_can( - 'delete' === $mutation - ? $post_type_object->cap->delete_posts - : $post_type_object->cap->edit_posts - ), - $order_id, - $input, - $context, - $info - ); + if ( ! $order_id ) { + return apply_filters( + "graphql_woocommerce_authorized_to_{$mutation}_orders", + current_user_can( $post_type_object->cap->edit_posts ), + $order_id, + $input, + $context, + $info + ); + } + + /** @var false|\WC_Order $order */ + $order = \wc_get_order( $order_id ); + if ( false === $order ) { + throw new UserError( + sprintf( + /* translators: %d: Order ID */ + __( 'Failed to find order with ID of %d.', 'wp-graphql-woocommerce' ), + $order_id + ) + ); + } + + $post_type = get_post_type( $order_id ); + if ( false === $post_type ) { + throw new UserError( __( 'Failed to identify the post type of the order.', 'wp-graphql-woocommerce' ) ); + } + + // Return true if user is owner or admin. + $is_owner = 0 !== get_current_user_id() && $order->get_customer_id() === get_current_user_id(); + $is_admin = \wc_rest_check_post_permissions( $post_type, 'edit', $order_id ); + return $is_owner || $is_admin; } /** @@ -565,25 +587,26 @@ public static function apply_coupons( $order_id, $coupons ) { /** * Validates order customer * - * @param array $input Input data describing order. + * @param string $customer_id ID of customer for order. * * @return bool */ - public static function validate_customer( $input ) { - if ( ! empty( $input['customerId'] ) ) { - // Make sure customer exists. - if ( false === get_user_by( 'id', $input['customerId'] ) ) { - return false; - } - // Make sure customer is part of blog. - if ( is_multisite() && ! is_user_member_of_blog( $input['customerId'] ) ) { - add_user_to_blog( get_current_blog_id(), $input['customerId'], 'customer' ); - } + public static function validate_customer( $customer_id ) { + $id = Utils::get_database_id_from_id( $customer_id ); + if ( ! $id ) { + return false; + } - return true; + if ( false === get_user_by( 'id', $id ) ) { + return false; } - return false; + // Make sure customer is part of blog. + if ( is_multisite() && ! is_user_member_of_blog( $id ) ) { + add_user_to_blog( get_current_blog_id(), $id, 'customer' ); + } + + return true; } /** diff --git a/includes/mutation/class-coupon-create.php b/includes/mutation/class-coupon-create.php index f351f2bd..fce43772 100644 --- a/includes/mutation/class-coupon-create.php +++ b/includes/mutation/class-coupon-create.php @@ -12,8 +12,8 @@ use GraphQL\Error\UserError; use GraphQL\Type\Definition\ResolveInfo; -use GraphQLRelay\Relay; use WPGraphQL\AppContext; +use WPGraphQL\Utils\Utils; use WPGraphQL\WooCommerce\Data\Mutation\Coupon_Mutation; use WPGraphQL\WooCommerce\Model\Coupon; @@ -163,16 +163,14 @@ public static function get_output_fields() { */ public static function mutate_and_get_payload( $input, AppContext $context, ResolveInfo $info ) { // Retrieve order ID. - $coupon_id = 0; - if ( ! empty( $input['id'] ) && is_numeric( $input['id'] ) ) { - $coupon_id = absint( $input['id'] ); - } elseif ( ! empty( $input['id'] ) ) { - $id_components = Relay::fromGlobalId( $input['id'] ); - if ( empty( $id_components['id'] ) || empty( $id_components['type'] ) ) { - throw new UserError( __( 'The "id" provided is invalid', 'wp-graphql-woocommerce' ) ); - } + if ( ! empty( $input['id'] ) ) { + $coupon_id = Utils::get_database_id_from_id( $input['id'] ); + } else { + $coupon_id = 0; + } - $coupon_id = absint( $id_components['id'] ); + if ( false === $coupon_id ) { + throw new UserError( __( 'Coupon ID provided is invalid. Please check input and try again.', 'wp-graphql-woocommerce' ) ); } $coupon = new \WC_Coupon( $coupon_id ); diff --git a/includes/mutation/class-coupon-delete.php b/includes/mutation/class-coupon-delete.php index 988bf842..9e74fe8f 100644 --- a/includes/mutation/class-coupon-delete.php +++ b/includes/mutation/class-coupon-delete.php @@ -12,8 +12,8 @@ use GraphQL\Error\UserError; use GraphQL\Type\Definition\ResolveInfo; -use GraphQLRelay\Relay; use WPGraphQL\AppContext; +use WPGraphQL\Utils\Utils; use WPGraphQL\WooCommerce\Model\Coupon; /** @@ -87,17 +87,11 @@ public static function get_output_fields() { */ public static function mutate_and_get_payload( $input, AppContext $context, ResolveInfo $info ) { // Retrieve order ID. - $coupon_id = 0; - if ( ! empty( $input['id'] ) && is_numeric( $input['id'] ) ) { - $coupon_id = absint( $input['id'] ); - } elseif ( ! empty( $input['id'] ) ) { - $id_components = Relay::fromGlobalId( $input['id'] ); - if ( empty( $id_components['id'] ) || empty( $id_components['type'] ) ) { - throw new UserError( __( 'The "id" provided is invalid', 'wp-graphql-woocommerce' ) ); - } - - $coupon_id = absint( $id_components['id'] ); + $coupon_id = Utils::get_database_id_from_id( $input['id'] ); + if ( empty( $coupon_id ) ) { + throw new UserError( __( 'Coupon ID provided is missing or invalid. Please check input and try again.', 'wp-graphql-woocommerce' ) ); } + $coupon = new Coupon( $coupon_id ); if ( ! $coupon->ID ) { diff --git a/includes/mutation/class-order-create.php b/includes/mutation/class-order-create.php index dff79bb3..72f06548 100644 --- a/includes/mutation/class-order-create.php +++ b/includes/mutation/class-order-create.php @@ -167,7 +167,7 @@ public static function mutate_and_get_payload() { WC()->payment_gateways(); // Validate customer ID, if set. - if ( ! empty( $input['customerId'] ) && ! Order_Mutation::validate_customer( $input ) ) { + if ( ! empty( $input['customerId'] ) && ! Order_Mutation::validate_customer( $input['customerId'] ) ) { throw new UserError( __( 'Customer ID is invalid.', 'wp-graphql-woocommerce' ) ); } diff --git a/includes/mutation/class-order-delete-items.php b/includes/mutation/class-order-delete-items.php index 582a8aff..9b7b1786 100644 --- a/includes/mutation/class-order-delete-items.php +++ b/includes/mutation/class-order-delete-items.php @@ -12,8 +12,8 @@ use GraphQL\Error\UserError; use GraphQL\Type\Definition\ResolveInfo; -use GraphQLRelay\Relay; use WPGraphQL\AppContext; +use WPGraphQL\Utils\Utils; use WPGraphQL\WooCommerce\Data\Mutation\Order_Mutation; use WPGraphQL\WooCommerce\Model\Order; @@ -47,11 +47,12 @@ public static function get_input_fields() { [ 'id' => [ 'type' => 'ID', - 'description' => __( 'Order global ID', 'wp-graphql-woocommerce' ), + 'description' => __( 'Database ID or global ID of the order', 'wp-graphql-woocommerce' ), ], 'orderId' => [ - 'type' => 'Int', - 'description' => __( 'Order WP ID', 'wp-graphql-woocommerce' ), + 'type' => 'Int', + 'description' => __( 'Order WP ID', 'wp-graphql-woocommerce' ), + 'deprecationReason' => __( 'Use "id" field instead.', 'wp-graphql-woocommerce' ), ], 'itemIds' => [ 'type' => [ 'list_of' => 'Int' ], @@ -87,20 +88,20 @@ public static function mutate_and_get_payload() { // Retrieve order ID. $order_id = null; if ( ! empty( $input['id'] ) ) { - $id_components = Relay::fromGlobalId( $input['id'] ); - if ( empty( $id_components['id'] ) || empty( $id_components['type'] ) ) { - throw new UserError( __( 'The "id" provided is invalid', 'wp-graphql-woocommerce' ) ); - } - $order_id = absint( $id_components['id'] ); + $order_id = Utils::get_database_id_from_id( $input['id'] ); } elseif ( ! empty( $input['orderId'] ) ) { $order_id = absint( $input['orderId'] ); } else { - throw new UserError( __( 'No order ID provided.', 'wp-graphql-woocommerce' ) ); + throw new UserError( __( 'Order ID provided is missing or invalid. Please check input and try again.', 'wp-graphql-woocommerce' ) ); + } + + if ( ! $order_id ) { + throw new UserError( __( 'Order ID provided is invalid. Please check input and try again.', 'wp-graphql-woocommerce' ) ); } // Check if authorized to delete items on this order. if ( ! Order_Mutation::authorized( $input, $context, $info, 'delete-items', $order_id ) ) { - throw new UserError( __( 'User does not have the capabilities necessary to delete an order.', 'wp-graphql-woocommerce' ) ); + throw new UserError( __( 'User does not have the capabilities necessary to delete order items.', 'wp-graphql-woocommerce' ) ); } // Confirm item IDs. diff --git a/includes/mutation/class-order-delete.php b/includes/mutation/class-order-delete.php index 24aa6b5b..74b43293 100644 --- a/includes/mutation/class-order-delete.php +++ b/includes/mutation/class-order-delete.php @@ -12,9 +12,9 @@ use GraphQL\Error\UserError; use GraphQL\Type\Definition\ResolveInfo; -use GraphQLRelay\Relay; use WC_Order_Factory; use WPGraphQL\AppContext; +use WPGraphQL\Utils\Utils; use WPGraphQL\WooCommerce\Data\Mutation\Order_Mutation; use WPGraphQL\WooCommerce\Model\Order; @@ -48,11 +48,12 @@ public static function get_input_fields() { [ 'id' => [ 'type' => 'ID', - 'description' => __( 'Order global ID', 'wp-graphql-woocommerce' ), + 'description' => __( 'Database ID or global ID of the order', 'wp-graphql-woocommerce' ), ], 'orderId' => [ - 'type' => 'Int', - 'description' => __( 'Order WP ID', 'wp-graphql-woocommerce' ), + 'type' => 'Int', + 'description' => __( 'Order WP ID', 'wp-graphql-woocommerce' ), + 'deprecationReason' => __( 'Use "id" field instead.', 'wp-graphql-woocommerce' ), ], 'forceDelete' => [ 'type' => 'Boolean', @@ -86,17 +87,17 @@ public static function get_output_fields() { public static function mutate_and_get_payload() { return static function ( $input, AppContext $context, ResolveInfo $info ) { // Retrieve order ID. - $order_id = null; + $order_id = false; if ( ! empty( $input['id'] ) ) { - $id_components = Relay::fromGlobalId( $input['id'] ); - if ( empty( $id_components['id'] ) || empty( $id_components['type'] ) ) { - throw new UserError( __( 'The "id" provided is invalid', 'wp-graphql-woocommerce' ) ); - } - $order_id = absint( $id_components['id'] ); + $order_id = Utils::get_database_id_from_id( $input['id'] ); } elseif ( ! empty( $input['orderId'] ) ) { $order_id = absint( $input['orderId'] ); } else { - throw new UserError( __( 'No order ID provided.', 'wp-graphql-woocommerce' ) ); + throw new UserError( __( 'Order ID provided is missing or invalid. Please check input and try again.', 'wp-graphql-woocommerce' ) ); + } + + if ( ! $order_id ) { + throw new UserError( __( 'Order ID provided is invalid. Please check input and try again.', 'wp-graphql-woocommerce' ) ); } // Check if authorized to delete this order. diff --git a/includes/mutation/class-order-update.php b/includes/mutation/class-order-update.php index 8fa16abd..2fc8cf71 100644 --- a/includes/mutation/class-order-update.php +++ b/includes/mutation/class-order-update.php @@ -12,9 +12,9 @@ use GraphQL\Error\UserError; use GraphQL\Type\Definition\ResolveInfo; -use GraphQLRelay\Relay; use WC_Order_Factory; use WPGraphQL\AppContext; +use WPGraphQL\Utils\Utils; use WPGraphQL\WooCommerce\Data\Mutation\Order_Mutation; use WPGraphQL\WooCommerce\Model\Order; @@ -49,15 +49,16 @@ public static function get_input_fields() { [ 'id' => [ 'type' => 'ID', - 'description' => __( 'Order global ID', 'wp-graphql-woocommerce' ), + 'description' => __( 'Database ID or global ID of the order', 'wp-graphql-woocommerce' ), ], 'orderId' => [ - 'type' => 'Int', - 'description' => __( 'Order WP ID', 'wp-graphql-woocommerce' ), + 'type' => 'Int', + 'description' => __( 'Order WP ID', 'wp-graphql-woocommerce' ), + 'deprecationReason' => __( 'Use "id" field instead.', 'wp-graphql-woocommerce' ), ], 'customerId' => [ - 'type' => 'Int', - 'description' => __( 'Order customer ID', 'wp-graphql-woocommerce' ), + 'type' => 'ID', + 'description' => __( 'Database ID or global ID of the customer for the order', 'wp-graphql-woocommerce' ), ], ] ); @@ -89,15 +90,15 @@ public static function mutate_and_get_payload() { // Retrieve order ID. $order_id = null; if ( ! empty( $input['id'] ) ) { - $id_components = Relay::fromGlobalId( $input['id'] ); - if ( empty( $id_components['id'] ) || empty( $id_components['type'] ) ) { - throw new UserError( __( 'The "id" provided is invalid', 'wp-graphql-woocommerce' ) ); - } - $order_id = absint( $id_components['id'] ); + $order_id = Utils::get_database_id_from_id( $input['id'] ); } elseif ( ! empty( $input['orderId'] ) ) { $order_id = absint( $input['orderId'] ); } else { - throw new UserError( __( 'No order ID provided.', 'wp-graphql-woocommerce' ) ); + throw new UserError( __( 'Order ID provided is missing or invalid. Please check input and try again.', 'wp-graphql-woocommerce' ) ); + } + + if ( ! $order_id ) { + throw new UserError( __( 'Order ID provided is invalid. Please check input and try again.', 'wp-graphql-woocommerce' ) ); } // Check if authorized to update this order. @@ -133,7 +134,7 @@ public static function mutate_and_get_payload() { \WC()->payment_gateways(); // Validate customer ID. - if ( ! empty( $input['customerId'] ) && ! Order_Mutation::validate_customer( $input ) ) { + if ( ! empty( $input['customerId'] ) && ! Order_Mutation::validate_customer( $input['customerId'] ) ) { throw new UserError( __( 'New customer ID is invalid.', 'wp-graphql-woocommerce' ) ); } @@ -147,7 +148,7 @@ public static function mutate_and_get_payload() { } // Actions for after the order is saved. - if ( true === $input['isPaid'] ) { + if ( isset( $input['isPaid'] ) && true === $input['isPaid'] ) { $order->payment_complete( ! empty( $input['transactionId'] ) ? $input['transactionId'] diff --git a/includes/mutation/class-review-delete-restore.php b/includes/mutation/class-review-delete-restore.php index 0aa3f1fb..0fa1312f 100644 --- a/includes/mutation/class-review-delete-restore.php +++ b/includes/mutation/class-review-delete-restore.php @@ -14,6 +14,7 @@ use GraphQL\Type\Definition\ResolveInfo; use GraphQLRelay\Relay; use WPGraphQL\AppContext; +use WPGraphQL\Utils\Utils; /** * Class Review_Delete_Restore @@ -130,12 +131,12 @@ public static function get_output_fields( $restore = false ) { public static function mutate_and_get_payload() { return static function ( $input, AppContext $context, ResolveInfo $info ) { // Retrieve the product review rating for the payload. - $id_parts = Relay::fromGlobalId( $input['id'] ); - if ( empty( $id_parts['id'] ) ) { + $id = Utils::get_database_id_from_id( $input['id'] ); + if ( ! $id ) { throw new UserError( __( 'Invalid Product Review ID provided', 'wp-graphql-woocommerce' ) ); } - $rating = get_comment_meta( absint( $id_parts['id'] ), 'rating' ); + $rating = get_comment_meta( absint( $id ), 'rating' ); // @codingStandardsIgnoreLine switch ( $info->fieldName ) { diff --git a/includes/mutation/class-review-update.php b/includes/mutation/class-review-update.php index e66088eb..1c290ab7 100644 --- a/includes/mutation/class-review-update.php +++ b/includes/mutation/class-review-update.php @@ -12,9 +12,9 @@ use GraphQL\Error\UserError; use GraphQL\Type\Definition\ResolveInfo; -use GraphQLRelay\Relay; use WPGraphQL\AppContext; use WPGraphQL\Mutation\CommentUpdate; +use WPGraphQL\Utils\Utils; /** * Class Review_Update @@ -79,12 +79,10 @@ public static function mutate_and_get_payload() { 'clientMutationId' => 1, ]; - $payload = []; - $id_parts = ! empty( $input['id'] ) ? Relay::fromGlobalId( $input['id'] ) : null; - $payload['id'] = isset( $id_parts['id'] ) && absint( $id_parts['id'] ) ? absint( $id_parts['id'] ) : null; - - if ( empty( $payload['id'] ) ) { - throw new UserError( __( 'The Review could not be updated', 'wp-graphql-woocommerce' ) ); + $payload = []; + $id = Utils::get_database_id_from_id( $input['id'] ); + if ( ! $id ) { + throw new UserError( __( 'Provided review ID missing or invalid ', 'wp-graphql-woocommerce' ) ); } if ( array_intersect_key( $input, $skip ) !== $input ) { diff --git a/includes/mutation/class-tax-rate-create.php b/includes/mutation/class-tax-rate-create.php index 8fdfcf06..35567777 100644 --- a/includes/mutation/class-tax-rate-create.php +++ b/includes/mutation/class-tax-rate-create.php @@ -13,6 +13,7 @@ use GraphQL\Error\UserError; use GraphQL\Type\Definition\ResolveInfo; use WPGraphQL\AppContext; +use WPGraphQL\Utils\Utils; /** * Class - Tax_Rate_Create @@ -117,7 +118,10 @@ public static function get_output_fields() { * @return array */ public static function mutate_and_get_payload( $input, AppContext $context, ResolveInfo $info ) { - $id = ! empty( $input['id'] ) ? $input['id'] : null; + $id = ! empty( $input['id'] ) ? Utils::get_database_id_from_id( $input['id'] ) : null; + if ( false === $id ) { + throw new UserError( __( 'Invalid ID provided.', 'wp-graphql-woocommerce' ) ); + } $action = ! $id ? 'create' : 'update'; $permission = ! $id ? 'create' : 'edit'; if ( ! \wc_rest_check_manager_permissions( 'settings', $permission ) ) { @@ -217,7 +221,7 @@ public static function mutate_and_get_payload( $input, AppContext $context, Reso /** * Filter tax rate object before responding. * - * @param object $tax_rate_id The shipping method object. + * @param int $tax_rate_id The shipping method object. * @param array $input Request input. */ do_action( "graphql_woocommerce_tax_rate_{$action}", $id, $input ); diff --git a/includes/mutation/class-tax-rate-delete.php b/includes/mutation/class-tax-rate-delete.php index 8b5867bf..5b6f66f2 100644 --- a/includes/mutation/class-tax-rate-delete.php +++ b/includes/mutation/class-tax-rate-delete.php @@ -13,6 +13,7 @@ use GraphQL\Error\UserError; use GraphQL\Type\Definition\ResolveInfo; use WPGraphQL\AppContext; +use WPGraphQL\Utils\Utils; /** * Class - Tax_Rate_Delete @@ -75,11 +76,14 @@ public static function mutate_and_get_payload() { throw new UserError( __( 'Sorry, you are not allowed to delete tax rates.', 'wp-graphql-woocommerce' ), \rest_authorization_required_code() ); } global $wpdb; - $id = $input['id']; + $id = Utils::get_database_id_from_id( $input['id'] ); + if ( ! $id ) { + throw new UserError( __( 'Invalid tax rate ID.', 'wp-graphql-woocommerce' ) ); + } $tax = $context->get_loader( 'tax_rate' )->load( $id ); if ( ! $tax ) { - throw new UserError( __( 'Invalid tax rate ID.', 'wp-graphql-woocommerce' ) ); + throw new UserError( __( 'Failed to locate tax rate', 'wp-graphql-woocommerce' ) ); } /** diff --git a/tests/wpunit/OrderMutationsTest.php b/tests/wpunit/OrderMutationsTest.php index db18c2c6..b3977f82 100644 --- a/tests/wpunit/OrderMutationsTest.php +++ b/tests/wpunit/OrderMutationsTest.php @@ -625,7 +625,7 @@ public function testUpdateOrderMutation() { * * User without necessary capabilities cannot update order an order. */ - wp_set_current_user( $this->customer ); + wp_set_current_user( $this->factory->user->create( [ 'role' => 'customer' ] ) ); $actual = $this->orderMutation( $updated_input, 'updateOrder', @@ -902,7 +902,7 @@ public function testDeleteOrderMutation() { * * User without necessary capabilities cannot delete order an order. */ - wp_set_current_user( $this->customer ); + wp_set_current_user( $this->factory->user->create( [ 'role' => 'customer' ] ) ); $actual = $this->orderMutation( $deleted_input, 'deleteOrder', @@ -1056,7 +1056,7 @@ public function testDeleteOrderItemsMutation() { * * User without necessary capabilities cannot delete order an order. */ - wp_set_current_user( $this->customer ); + wp_set_current_user( $this->factory->user->create( [ 'role' => 'customer' ] ) ); $actual = $this->orderMutation( $deleted_items_input, 'deleteOrderItems',