search

smartsendio / woocommerce

Smart Send module for WooCommerce
GNU General Public License v3.0
0 stars 2 forks source link

Edit order totals calculation #26

Open Saggre opened 2 years ago

Saggre commented 2 years ago

https://github.com/smartsendio/woocommerce/blob/4d754e409e883d602833d103881eb62d8da9dd0f/smart-send-logistics/includes/class-ss-shipping-shipment.php#L370-L383

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 commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

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