Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Edit order totals calculation #26

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Saggre
Copy link

@Saggre Saggre commented Feb 3, 2022

// Order totals
$order_total = $this->order->get_total();
$order_total_tax = $this->order->get_total_tax();
$order_total_excl = $order_total - $order_total_tax;
// Shipping totals
$order_shipping = $this->order->get_shipping_total();
$order_shipping_tax = $this->order->get_shipping_tax();
$order_shipping_excl = $order_shipping - $order_shipping_tax;
// Order totals without shipping
$order_subtotal = $order_total - $order_shipping;
$order_subtotal_tax = $order_total_tax - $order_shipping_tax;
$order_subtotal_excl = $order_total_excl - $order_subtotal_tax;

I found that the plugin tries to send a negative subtotal into Smart Send API when there's a discount that is larger than subtotal, but smaller than total price. This is caused by the current subtotal calculation not taking discounts into account resulting in:

$order_shipping > $order_total
$order_subtotal = $order_total - $order_shipping;
$order_subtotal < 0

And this results in the following API error:

{
	"links": {
		"about": "https://app.smartsend.io/docs/errors/ValidationException",
		"status": "https://app.smartsend.io/status"
	},
	"id": null,
	"code": "ValidationException",
	"message": "The given data was invalid.",
	"errors": {
		"subtotal_price_excluding_tax": [
			"The subtotal_price_excluding_tax must be between 0 and 10000000."
		],
		"parcels.0.total_price_excluding_tax": [
			"The parcels.0.total_price_excluding_tax must be between 0 and 10000000."
		]
	}
}

This is the new totals calculation. The fix is just to get subtotal directly from WC_Order and throw the calculations around a bit. I invite you to check it thoroughly because it's a sensitive and mistake-prone topic.

// Order totals
$order_total = $this->order->get_total();
$order_total_tax = $this->order->get_total_tax();
$order_total_excl = $order_total - $order_total_tax;

// Shipping totals
$order_shipping_excl = (float)$this->order->get_shipping_total();
$order_shipping_tax = (float)$this->order->get_shipping_tax();
$order_shipping = $order_shipping_excl + $order_shipping_tax;

// Order totals without shipping
$order_subtotal_tax = $order_total_tax - $order_shipping_tax;
$order_subtotal_excl = $this->order->get_subtotal();
$order_subtotal = $order_subtotal_excl + $order_subtotal_tax;

get_shipping_total() and get_subtotal() in my understanding return a taxless price, and get_total() returns price including tax.

@bilfeldt
Copy link
Member

bilfeldt commented Feb 3, 2022

@Saggre Thanks for the PR. Do you have a guide for recreating the issue you have seen? Perhaps just a screendump from an order would be enough to recreate it locally and verify the solution.

@Saggre
Copy link
Author

Saggre commented Feb 4, 2022

@bilfeldt Quickest way to recreate the issue is to manually add a negative fee to an order in wp admin panel.
Any negative WC_Order_Item that is not a 'line_item' will create this issue though.

Negative fee

This order results in the following request json fields with plugin Version: 8.0.25:

"subtotal_price_excluding_tax": 5.58,
"subtotal_price_including_tax": -2.3599999999999996,
"shipping_price_excluding_tax": 6.52,
"shipping_price_including_tax": 8.26,
"total_price_excluding_tax": 4.87,
"total_price_including_tax": 5.9,
"total_tax_amount": 1.03,

And this is how the json should look after the fix:

"subtotal_price_excluding_tax": 6.61,
"subtotal_price_including_tax": 5.9,
"shipping_price_excluding_tax": 8.26,
"shipping_price_including_tax": 10,
"total_price_excluding_tax": 4.87,
"total_price_including_tax": 5.9,
"total_tax_amount": 1.03,

Note that there's actually a mistake in my solution with subtotal_price_including_tax.

@bilfeldt
Copy link
Member

bilfeldt commented Feb 6, 2022

@Saggre Thanks for the PR. After having looked at the template file responsible for the admin page you listed plugins/woocommerce/includes/admin/meta-boxes/views/html-order-items.php these seems to be the valid methods:

$order->get_subtotal(); // including VAT - without any discounts
$order->get_total_discount(); // including VAT
$order->get_discount_tax();
$order->get_shipping_total(); // including VAT
$order->get_total_fees();

Meaning that the following calculation should work:

// Order totals
$order_total_incl_tax = $this->order->get_total();
$order_total_tax = $this->order->get_total_tax();
$order_total_excl_tax = $order_total_incl_tax - $order_total_tax;

// Shipping
$order_shipping_incl_tax = (float) $this->order->get_shipping_total(); // Note returns string for some reason
$order_shipping_tax = (float) $this->order->get_shipping_tax(); // Note returns string for some reason
$order_shipping_excl_tax = $order_shipping_incl_tax - $order_shipping_tax;

// Order subtotals (without shipping but with discount)
$order_subtotal_incl_tax = $this->order->get_subtotal() - $this->get_total_discount();
$order_subtotal_tax = $order_total_tax - $order_shipping_tax - $order->get_discount_tax();
$order_subtotal_excl_tax = $order_subtotal_incl_tax - $order_subtotal_tax;

Note that all available methods can be found here: https://woocommerce.github.io/code-reference/classes/WC-Abstract-Order.html

@bilfeldt
Copy link
Member

bilfeldt commented Feb 6, 2022

@Saggre I have now updated the calculation on your PR. Can you confirm if this is working on the test order that you have?

@Saggre
Copy link
Author

Saggre commented Feb 7, 2022

@bilfeldt Now with the same test order, and a test environment with only WooCommerce and Smart Send, the values look like this:

"subtotal_price_excluding_tax": 7.32,
"subtotal_price_including_tax": 6.61,
"shipping_price_excluding_tax": 6.52,
"shipping_price_including_tax": 8.26,
"total_price_excluding_tax": 4.87,
"total_price_including_tax": 5.9,
"total_tax_amount": 1.03,

shipping_price_including_tax should be 10€ and subtotal_price_including_tax should >= subtotal_price_excluding_tax.

Here get_subtotal() is used to test for price excluding VAT:
https://github.com/woocommerce/woocommerce/blob/edcffa99123091bd11f63aa593d65e2c985f952d/plugins/woocommerce/tests/php/includes/abstracts/class-wc-abstract-order-test.php#L125

And here get_shipping_total() is used to get shipping price excluding VAT:
https://github.com/woocommerce/woocommerce/blob/f3927c786a62b8bd5ca7ba7c4ef05f2ebdf0b787/plugins/woocommerce/includes/abstracts/abstract-wc-order.php#L2002-L2008

With my latest commit, the values look correct when comparing to the order screenshot (Shop VAT is 21%).

"subtotal_price_excluding_tax": 6.61,
"subtotal_price_including_tax": 8,
"shipping_price_excluding_tax": 8.26,
"shipping_price_including_tax": 10,
"total_price_excluding_tax": 4.87,
"total_price_including_tax": 5.9,
"total_tax_amount": 1.03,

What threw it off in my previous commit was that $order->get_total_tax() includes taxes from fees and other extra line items, but when calculating subtotal tax we want them removed. In the commit, $extra_line_item_tax is the total for this extra tax (which is -2.10€ in my screenshot's case).

I included filter smart_send_order_custom_line_items for a way to make the calculation compatible with plugins that include custom line items, such as some coupon or gift card plugins.

The filter could also be implemented in the following format:

$extra_line_item_types = apply_filters( 'smart_send_order_custom_line_item_types', array( 'fee' ), $this->order );
$extra_line_items      = $this->order->get_items( $extra_line_item_types );

@bilfeldt
Copy link
Member

bilfeldt commented Feb 8, 2022

@Saggre thanks for the details.

I think what is causing the confusion here is the WooCommerce config woocommerce_prices_include_tax which determine whether or not the value in DB is including or excluding tax.

It seems that $order->get_total() always includes tax:

I have not yet found test examples confirming the behaviour of get_shipping_total() and get_subtotal () but you found:

I had not originally spotted that you have added a "fee" and not a "discount" - of cause our plugin should support both. I have created these two test orders with different config option woocommerce_prices_include_tax and will do some testing on these shortly.

94
90

I guess it will be possible to use the method $oder->get_total_fees() but is there a good way to get the tax of these fees @Saggre ?

@Saggre
Copy link
Author

Saggre commented Feb 8, 2022

@bilfeldt Calling $line_item->get_total_tax() for each fee line item is one option

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants