Skip to content

Commit

Permalink
Fix negative fee
Browse files Browse the repository at this point in the history
  • Loading branch information
kilbot committed Dec 5, 2024
1 parent cf7eebd commit 09b8009
Show file tree
Hide file tree
Showing 6 changed files with 119 additions and 36 deletions.
41 changes: 39 additions & 2 deletions includes/API/Orders_Controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
*
Expand Down Expand Up @@ -391,15 +428,15 @@ 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,
);

// 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,
);
Expand Down
3 changes: 3 additions & 0 deletions readme.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
53 changes: 26 additions & 27 deletions tests/bootstrap.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 :(
Expand All @@ -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' ) );
Expand Down Expand Up @@ -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();
4 changes: 2 additions & 2 deletions tests/framework/class-wc-unit-test-case.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

/**
Expand Down
44 changes: 44 additions & 0 deletions tests/includes/API/Test_Order_Taxes.php
Original file line number Diff line number Diff line change
Expand Up @@ -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'] );
}
}
10 changes: 5 additions & 5 deletions tests/includes/API/Test_Orders_Controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
}

/**
Expand All @@ -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 ) {
Expand Down Expand Up @@ -834,7 +834,7 @@ public function test_create_order_with_decimal_quantity() {
}

/**
*
*
*/
public function test_filter_order_by_cashier() {
$order1 = OrderHelper::create_order();
Expand All @@ -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();
Expand Down

0 comments on commit 09b8009

Please sign in to comment.