Skip to content

Commit 09b8009

Browse files
committed
Fix negative fee
1 parent cf7eebd commit 09b8009

File tree

6 files changed

+119
-36
lines changed

6 files changed

+119
-36
lines changed

includes/API/Orders_Controller.php

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
use Automattic\WooCommerce\Utilities\OrderUtil;
2222
use WCPOS\WooCommercePOS\Services\Cache;
2323
use WP_Error;
24+
use WC_Tax;
2425

2526
use const WCPOS\WooCommercePOS\PLUGIN_NAME;
2627

@@ -96,6 +97,7 @@ public function wcpos_dispatch_request( $dispatch_result, WP_REST_Request $reque
9697
add_filter( 'woocommerce_order_get_items', array( $this, 'wcpos_order_get_items' ), 10, 3 );
9798
add_action( 'woocommerce_before_order_object_save', array( $this, 'wcpos_before_order_object_save' ), 10, 2 );
9899
add_filter( 'woocommerce_rest_shop_order_object_query', array( $this, 'wcpos_shop_order_query' ), 10, 2 );
100+
add_action( 'woocommerce_order_item_fee_after_calculate_taxes', array( $this, 'wcpos_order_item_fee_after_calculate_taxes' ), 10, 2 );
99101

100102
/**
101103
* 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 )
325327
return $item;
326328
}
327329

330+
/**
331+
* The way WooCommerce handles negative fees is ... weird.
332+
* They by-pass the normal tax calculation, disregard the tax_status and tax_class, and apply the taxes to the fee line.
333+
* 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
334+
* the wrong result.
335+
*
336+
* @param WC_Order_Item_Fee $fee_item The fee item.
337+
* @param array $calculate_tax_for The tax calculation data.
338+
*/
339+
public function wcpos_order_item_fee_after_calculate_taxes( $fee_item, $calculate_tax_for ) {
340+
if ( $fee_item->get_total() < 0 ) {
341+
// Respect the fee line's tax_class and tax_status.
342+
$tax_class = $fee_item->get_tax_class();
343+
$tax_status = $fee_item->get_tax_status();
344+
345+
if ( $tax_status === 'taxable' ) {
346+
// Use the tax_class if set, otherwise default.
347+
$calculate_tax_for['tax_class'] = $tax_class ? : '';
348+
349+
// Find rates and calculate taxes for the fee.
350+
$tax_rates = WC_Tax::find_rates( $calculate_tax_for );
351+
$discount_taxes = WC_Tax::calc_tax( $fee_item->get_total(), $tax_rates );
352+
353+
// Apply calculated taxes to the fee item.
354+
$fee_item->set_taxes( array( 'total' => $discount_taxes ) );
355+
} else {
356+
// Set taxes to none if tax_status is 'none'.
357+
$fee_item->set_taxes( false );
358+
}
359+
360+
// Save the updated fee item.
361+
$fee_item->save();
362+
}
363+
}
364+
328365
/**
329366
* Gets the product ID from posted ID.
330367
*
@@ -391,15 +428,15 @@ public function get_collection_params() {
391428

392429
// Add 'pos_cashier' parameter
393430
$params['pos_cashier'] = array(
394-
'description' => __('Filter orders by POS cashier.', 'woocommerce-pos'),
431+
'description' => __( 'Filter orders by POS cashier.', 'woocommerce-pos' ),
395432
'type' => 'integer',
396433
'required' => false,
397434
);
398435

399436
// Add 'pos_store' parameter
400437
// @NOTE - this is different to 'store_id' which is the store the request was made from.
401438
$params['pos_store'] = array(
402-
'description' => __('Filter orders by POS store.', 'woocommerce-pos'),
439+
'description' => __( 'Filter orders by POS store.', 'woocommerce-pos' ),
403440
'type' => 'integer',
404441
'required' => false,
405442
);

readme.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,9 @@ There is more information on our website at [https://wcpos.com](https://wcpos.co
8888

8989
== Changelog ==
9090

91+
= 1.7.2 - 2024/12/XX =
92+
* Fix: Negative fees with tax_status='none' and/or tax_class are now applied correctly to the order
93+
9194
= 1.7.1 - 2024/11/14 =
9295
* Fix: Error updating quantity for Product Variations when decimal quantities enabled
9396
* Plugin Conflict: The wePOS plugin alters the standard WC REST API response, which in turn breaks WooCommerce POS

tests/bootstrap.php

Lines changed: 26 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public function __construct() {
2727

2828
// Require composer dependencies.
2929
require_once $this->plugin_dir . '/vendor/autoload.php';
30-
$this->initialize_code_hacker();
30+
// $this->initialize_code_hacker();
3131

3232
// Bootstrap WP_Mock to initialize built-in features
3333
// NOTE: CodeHacker and WP_Mock are not compatible :(
@@ -49,7 +49,7 @@ public function __construct() {
4949
$this->includes();
5050

5151
// re-initialize dependency injection, this needs to be the last operation after everything else is in place.
52-
$this->initialize_dependency_injection();
52+
// $this->initialize_dependency_injection();
5353

5454
// Use existing behavior for wp_die during actual test execution.
5555
remove_filter( 'wp_die_handler', array( $this, 'fail_if_died' ) );
@@ -190,32 +190,31 @@ private function initialize_code_hacker(): void {
190190
/**
191191
* Re-initialize the dependency injection engine.
192192
*
193-
* The dependency injection engine has been already initialized as part of the Woo initialization, but we need
194-
* to replace the registered read-only container with a fully configurable one for testing.
195-
* To this end we hack a bit and use reflection to grab the underlying container that the read-only one stores
196-
* in a private property.
197-
*
198-
* Additionally, we replace the legacy/function proxies with mockable versions to easily replace anything
199-
* in tests as appropriate.
200-
*
201-
* @throws \Exception The Container class doesn't have a 'container' property.
193+
* This adjusts the container for testing, enabling the use of mockable proxies and other test-specific overrides.
202194
*/
203-
private function initialize_dependency_injection(): void {
204-
require_once $this->plugin_dir . '/tests/Tools/DependencyManagement/MockableLegacyProxy.php';
205-
206-
try {
207-
$inner_container_property = new ReflectionProperty( \Automattic\WooCommerce\Container::class, 'container' );
208-
} catch ( ReflectionException $ex ) {
209-
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?' );
210-
}
211-
212-
$inner_container_property->setAccessible( true );
213-
$inner_container = $inner_container_property->getValue( wc_get_container() );
214-
215-
$inner_container->replace( LegacyProxy::class, MockableLegacyProxy::class );
216-
217-
$GLOBALS['wc_container'] = $inner_container;
218-
}
195+
// private function initialize_dependency_injection(): void {
196+
// Check if WooCommerce provides a testing container.
197+
// if ( ! class_exists( \Automattic\WooCommerce\Internal\DependencyManagement\TestingContainer::class ) ) {
198+
// throw new \Exception( 'TestingContainer class is not available in the current WooCommerce version.' );
199+
// }
200+
201+
// Create a new TestingContainer instance.
202+
// $testing_container = new \Automattic\WooCommerce\Internal\DependencyManagement\TestingContainer();
203+
204+
// Replace the legacy proxy with a mockable version for testing.
205+
// $testing_container->register(
206+
// LegacyProxy::class,
207+
// function () {
208+
// return new MockableLegacyProxy();
209+
// }
210+
// );
211+
212+
// Replace the global WooCommerce container with the testing container.
213+
// \Automattic\WooCommerce\Container::set( $testing_container );
214+
215+
// Store the container globally for test access if necessary.
216+
// $GLOBALS['wc_container'] = $testing_container;
217+
// }
219218
}
220219

221220
Bootstrap::instance();

tests/framework/class-wc-unit-test-case.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,11 +66,11 @@ public function setUp(): void {
6666
WC_Post_types::register_post_types();
6767
WC_Post_types::register_taxonomies();
6868

69-
CodeHacker::reset_hacks();
69+
// CodeHacker::reset_hacks();
7070

7171
// Reset the instance of MockableLegacyProxy that was registered during bootstrap,
7272
// in order to start the test in a clean state (without anything mocked).
73-
wc_get_container()->get( LegacyProxy::class )->reset();
73+
// wc_get_container()->get( LegacyProxy::class )->reset();
7474
}
7575

7676
/**

tests/includes/API/Test_Order_Taxes.php

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,4 +285,48 @@ public function test_create_order_with_customer_shipping_address_as_tax_location
285285
$this->assertEquals( '1.22', $data['cart_tax'] );
286286
$this->assertEquals( '1.22', $data['total_tax'] );
287287
}
288+
289+
/**
290+
*
291+
*/
292+
public function test_fee_lines_should_respect_tax_status_when_negative() {
293+
$this->assertEquals( 'base', WC_Admin_Settings::get_option( 'woocommerce_tax_based_on' ) );
294+
$this->assertEquals( 'US:CA', WC_Admin_Settings::get_option( 'woocommerce_default_country' ) );
295+
296+
$request = $this->wp_rest_post_request( '/wcpos/v1/orders' );
297+
$request->set_body_params(
298+
array(
299+
'line_items' => array(
300+
array(
301+
'product_id' => 1,
302+
'quantity' => 1,
303+
'price' => 20,
304+
'total' => '20.00',
305+
),
306+
),
307+
'fee_lines' => array(
308+
array(
309+
'name' => 'Fee',
310+
'total' => '-10',
311+
'tax_status' => 'none',
312+
),
313+
),
314+
)
315+
);
316+
317+
$response = $this->server->dispatch( $request );
318+
$data = $response->get_data();
319+
$this->assertEquals( 201, $response->get_status() );
320+
321+
// fee line taxes
322+
$this->assertEquals( 1, \count( $data['fee_lines'] ) );
323+
$this->assertEquals( '-10.000000', $data['fee_lines'][0]['total'] );
324+
$this->assertEquals( '0.000000', $data['fee_lines'][0]['total_tax'] );
325+
$this->assertEquals( 0, \count( $data['fee_lines'][0]['taxes'] ) );
326+
327+
// order taxes
328+
$this->assertEquals( 1, \count( $data['tax_lines'] ) );
329+
$this->assertEquals( '2.000000', $data['total_tax'] );
330+
$this->assertEquals( '12.000000', $data['total'] );
331+
}
288332
}

tests/includes/API/Test_Orders_Controller.php

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ public function test_order_api_get_all_ids(): void {
139139
$data = $response->get_data();
140140
$ids = wp_list_pluck( $data, 'id' );
141141

142-
$this->assertEquals( array( $order1->get_id(), $order2->get_id() ), $ids );
142+
$this->assertEqualsCanonicalizing( array( $order1->get_id(), $order2->get_id() ), $ids );
143143
}
144144

145145
/**
@@ -158,7 +158,7 @@ public function test_order_api_get_all_ids_with_date_modified_gmt(): void {
158158
$data = $response->get_data();
159159
$ids = wp_list_pluck( $data, 'id' );
160160

161-
$this->assertEquals( array( $order1->get_id(), $order2->get_id() ), $ids );
161+
$this->assertEqualsCanonicalizing( array( $order1->get_id(), $order2->get_id() ), $ids );
162162

163163
// Verify that date_modified_gmt is present for all products and correctly formatted.
164164
foreach ( $data as $d ) {
@@ -834,7 +834,7 @@ public function test_create_order_with_decimal_quantity() {
834834
}
835835

836836
/**
837-
*
837+
*
838838
*/
839839
public function test_filter_order_by_cashier() {
840840
$order1 = OrderHelper::create_order();
@@ -854,8 +854,8 @@ public function test_filter_order_by_cashier() {
854854
$this->assertEquals( array( $order2->get_id() ), $ids );
855855
}
856856

857-
/**
858-
*
857+
/**
858+
*
859859
*/
860860
public function test_filter_order_by_store() {
861861
$order1 = OrderHelper::create_order();

0 commit comments

Comments
 (0)