From 09b8009787c831549ec6635a3a4ecee36fea8530 Mon Sep 17 00:00:00 2001 From: Paul Kilmurray Date: Thu, 5 Dec 2024 17:09:13 +0100 Subject: [PATCH] Fix negative fee --- includes/API/Orders_Controller.php | 41 +++++++++++++- readme.txt | 3 ++ tests/bootstrap.php | 53 +++++++++---------- tests/framework/class-wc-unit-test-case.php | 4 +- tests/includes/API/Test_Order_Taxes.php | 44 +++++++++++++++ tests/includes/API/Test_Orders_Controller.php | 10 ++-- 6 files changed, 119 insertions(+), 36 deletions(-) diff --git a/includes/API/Orders_Controller.php b/includes/API/Orders_Controller.php index cb44fcae..b13a44b4 100644 --- a/includes/API/Orders_Controller.php +++ b/includes/API/Orders_Controller.php @@ -21,6 +21,7 @@ use Automattic\WooCommerce\Utilities\OrderUtil; use WCPOS\WooCommercePOS\Services\Cache; use WP_Error; +use WC_Tax; use const WCPOS\WooCommercePOS\PLUGIN_NAME; @@ -96,6 +97,7 @@ public function wcpos_dispatch_request( $dispatch_result, WP_REST_Request $reque add_filter( 'woocommerce_order_get_items', array( $this, 'wcpos_order_get_items' ), 10, 3 ); add_action( 'woocommerce_before_order_object_save', array( $this, 'wcpos_before_order_object_save' ), 10, 2 ); add_filter( 'woocommerce_rest_shop_order_object_query', array( $this, 'wcpos_shop_order_query' ), 10, 2 ); + add_action( 'woocommerce_order_item_fee_after_calculate_taxes', array( $this, 'wcpos_order_item_fee_after_calculate_taxes' ), 10, 2 ); /** * Check if the request is for all orders and if the 'posts_per_page' is set to -1. @@ -325,6 +327,41 @@ public function prepare_line_items( $posted, $action = 'create', $item = null ) return $item; } + /** + * The way WooCommerce handles negative fees is ... weird. + * They by-pass the normal tax calculation, disregard the tax_status and tax_class, and apply the taxes to the fee line. + * This is a problem because if people want to apply a negative fee to an order, and set tax_status to 'none', it will give + * the wrong result. + * + * @param WC_Order_Item_Fee $fee_item The fee item. + * @param array $calculate_tax_for The tax calculation data. + */ + public function wcpos_order_item_fee_after_calculate_taxes( $fee_item, $calculate_tax_for ) { + if ( $fee_item->get_total() < 0 ) { + // Respect the fee line's tax_class and tax_status. + $tax_class = $fee_item->get_tax_class(); + $tax_status = $fee_item->get_tax_status(); + + if ( $tax_status === 'taxable' ) { + // Use the tax_class if set, otherwise default. + $calculate_tax_for['tax_class'] = $tax_class ? : ''; + + // Find rates and calculate taxes for the fee. + $tax_rates = WC_Tax::find_rates( $calculate_tax_for ); + $discount_taxes = WC_Tax::calc_tax( $fee_item->get_total(), $tax_rates ); + + // Apply calculated taxes to the fee item. + $fee_item->set_taxes( array( 'total' => $discount_taxes ) ); + } else { + // Set taxes to none if tax_status is 'none'. + $fee_item->set_taxes( false ); + } + + // Save the updated fee item. + $fee_item->save(); + } + } + /** * Gets the product ID from posted ID. * @@ -391,7 +428,7 @@ public function get_collection_params() { // Add 'pos_cashier' parameter $params['pos_cashier'] = array( - 'description' => __('Filter orders by POS cashier.', 'woocommerce-pos'), + 'description' => __( 'Filter orders by POS cashier.', 'woocommerce-pos' ), 'type' => 'integer', 'required' => false, ); @@ -399,7 +436,7 @@ public function get_collection_params() { // Add 'pos_store' parameter // @NOTE - this is different to 'store_id' which is the store the request was made from. $params['pos_store'] = array( - 'description' => __('Filter orders by POS store.', 'woocommerce-pos'), + 'description' => __( 'Filter orders by POS store.', 'woocommerce-pos' ), 'type' => 'integer', 'required' => false, ); diff --git a/readme.txt b/readme.txt index e28858c5..b95ffb86 100644 --- a/readme.txt +++ b/readme.txt @@ -88,6 +88,9 @@ There is more information on our website at [https://wcpos.com](https://wcpos.co == Changelog == += 1.7.2 - 2024/12/XX = +* Fix: Negative fees with tax_status='none' and/or tax_class are now applied correctly to the order + = 1.7.1 - 2024/11/14 = * Fix: Error updating quantity for Product Variations when decimal quantities enabled * Plugin Conflict: The wePOS plugin alters the standard WC REST API response, which in turn breaks WooCommerce POS diff --git a/tests/bootstrap.php b/tests/bootstrap.php index 58cc0ce2..c55f8bf0 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -27,7 +27,7 @@ public function __construct() { // Require composer dependencies. require_once $this->plugin_dir . '/vendor/autoload.php'; - $this->initialize_code_hacker(); + // $this->initialize_code_hacker(); // Bootstrap WP_Mock to initialize built-in features // NOTE: CodeHacker and WP_Mock are not compatible :( @@ -49,7 +49,7 @@ public function __construct() { $this->includes(); // re-initialize dependency injection, this needs to be the last operation after everything else is in place. - $this->initialize_dependency_injection(); + // $this->initialize_dependency_injection(); // Use existing behavior for wp_die during actual test execution. remove_filter( 'wp_die_handler', array( $this, 'fail_if_died' ) ); @@ -190,32 +190,31 @@ private function initialize_code_hacker(): void { /** * Re-initialize the dependency injection engine. * - * The dependency injection engine has been already initialized as part of the Woo initialization, but we need - * to replace the registered read-only container with a fully configurable one for testing. - * To this end we hack a bit and use reflection to grab the underlying container that the read-only one stores - * in a private property. - * - * Additionally, we replace the legacy/function proxies with mockable versions to easily replace anything - * in tests as appropriate. - * - * @throws \Exception The Container class doesn't have a 'container' property. + * This adjusts the container for testing, enabling the use of mockable proxies and other test-specific overrides. */ - private function initialize_dependency_injection(): void { - require_once $this->plugin_dir . '/tests/Tools/DependencyManagement/MockableLegacyProxy.php'; - - try { - $inner_container_property = new ReflectionProperty( \Automattic\WooCommerce\Container::class, 'container' ); - } catch ( ReflectionException $ex ) { - throw new \Exception( "Error when trying to get the private 'container' property from the " . \Automattic\WooCommerce\Container::class . ' class using reflection during unit testing bootstrap, has the property been removed or renamed?' ); - } - - $inner_container_property->setAccessible( true ); - $inner_container = $inner_container_property->getValue( wc_get_container() ); - - $inner_container->replace( LegacyProxy::class, MockableLegacyProxy::class ); - - $GLOBALS['wc_container'] = $inner_container; - } + // private function initialize_dependency_injection(): void { + // Check if WooCommerce provides a testing container. + // if ( ! class_exists( \Automattic\WooCommerce\Internal\DependencyManagement\TestingContainer::class ) ) { + // throw new \Exception( 'TestingContainer class is not available in the current WooCommerce version.' ); + // } + + // Create a new TestingContainer instance. + // $testing_container = new \Automattic\WooCommerce\Internal\DependencyManagement\TestingContainer(); + + // Replace the legacy proxy with a mockable version for testing. + // $testing_container->register( + // LegacyProxy::class, + // function () { + // return new MockableLegacyProxy(); + // } + // ); + + // Replace the global WooCommerce container with the testing container. + // \Automattic\WooCommerce\Container::set( $testing_container ); + + // Store the container globally for test access if necessary. + // $GLOBALS['wc_container'] = $testing_container; + // } } Bootstrap::instance(); diff --git a/tests/framework/class-wc-unit-test-case.php b/tests/framework/class-wc-unit-test-case.php index 6da78aa5..4fb1ea08 100644 --- a/tests/framework/class-wc-unit-test-case.php +++ b/tests/framework/class-wc-unit-test-case.php @@ -66,11 +66,11 @@ public function setUp(): void { WC_Post_types::register_post_types(); WC_Post_types::register_taxonomies(); - CodeHacker::reset_hacks(); + // CodeHacker::reset_hacks(); // Reset the instance of MockableLegacyProxy that was registered during bootstrap, // in order to start the test in a clean state (without anything mocked). - wc_get_container()->get( LegacyProxy::class )->reset(); + // wc_get_container()->get( LegacyProxy::class )->reset(); } /** diff --git a/tests/includes/API/Test_Order_Taxes.php b/tests/includes/API/Test_Order_Taxes.php index ef07d65b..a615600d 100644 --- a/tests/includes/API/Test_Order_Taxes.php +++ b/tests/includes/API/Test_Order_Taxes.php @@ -285,4 +285,48 @@ public function test_create_order_with_customer_shipping_address_as_tax_location $this->assertEquals( '1.22', $data['cart_tax'] ); $this->assertEquals( '1.22', $data['total_tax'] ); } + + /** + * + */ + public function test_fee_lines_should_respect_tax_status_when_negative() { + $this->assertEquals( 'base', WC_Admin_Settings::get_option( 'woocommerce_tax_based_on' ) ); + $this->assertEquals( 'US:CA', WC_Admin_Settings::get_option( 'woocommerce_default_country' ) ); + + $request = $this->wp_rest_post_request( '/wcpos/v1/orders' ); + $request->set_body_params( + array( + 'line_items' => array( + array( + 'product_id' => 1, + 'quantity' => 1, + 'price' => 20, + 'total' => '20.00', + ), + ), + 'fee_lines' => array( + array( + 'name' => 'Fee', + 'total' => '-10', + 'tax_status' => 'none', + ), + ), + ) + ); + + $response = $this->server->dispatch( $request ); + $data = $response->get_data(); + $this->assertEquals( 201, $response->get_status() ); + + // fee line taxes + $this->assertEquals( 1, \count( $data['fee_lines'] ) ); + $this->assertEquals( '-10.000000', $data['fee_lines'][0]['total'] ); + $this->assertEquals( '0.000000', $data['fee_lines'][0]['total_tax'] ); + $this->assertEquals( 0, \count( $data['fee_lines'][0]['taxes'] ) ); + + // order taxes + $this->assertEquals( 1, \count( $data['tax_lines'] ) ); + $this->assertEquals( '2.000000', $data['total_tax'] ); + $this->assertEquals( '12.000000', $data['total'] ); + } } diff --git a/tests/includes/API/Test_Orders_Controller.php b/tests/includes/API/Test_Orders_Controller.php index 66855c62..b3fec6ee 100644 --- a/tests/includes/API/Test_Orders_Controller.php +++ b/tests/includes/API/Test_Orders_Controller.php @@ -139,7 +139,7 @@ public function test_order_api_get_all_ids(): void { $data = $response->get_data(); $ids = wp_list_pluck( $data, 'id' ); - $this->assertEquals( array( $order1->get_id(), $order2->get_id() ), $ids ); + $this->assertEqualsCanonicalizing( array( $order1->get_id(), $order2->get_id() ), $ids ); } /** @@ -158,7 +158,7 @@ public function test_order_api_get_all_ids_with_date_modified_gmt(): void { $data = $response->get_data(); $ids = wp_list_pluck( $data, 'id' ); - $this->assertEquals( array( $order1->get_id(), $order2->get_id() ), $ids ); + $this->assertEqualsCanonicalizing( array( $order1->get_id(), $order2->get_id() ), $ids ); // Verify that date_modified_gmt is present for all products and correctly formatted. foreach ( $data as $d ) { @@ -834,7 +834,7 @@ public function test_create_order_with_decimal_quantity() { } /** - * + * */ public function test_filter_order_by_cashier() { $order1 = OrderHelper::create_order(); @@ -854,8 +854,8 @@ public function test_filter_order_by_cashier() { $this->assertEquals( array( $order2->get_id() ), $ids ); } - /** - * + /** + * */ public function test_filter_order_by_store() { $order1 = OrderHelper::create_order();