Partial refund of shipping costs not possible

[RabbitZzZ]

Hello,

I have an order which has already been refunded. The partial refund of the products worked but the shop manager forgot to put in the value for the shipping costs I suppose. If we now want to do partial refund again (1 product in the order already completely refunded) and only put in the value in the shipping field and try to save it, we always get the error "The partial refund data is incorrect.".

I already tried using different order states but without success. 

This should be possible, right?

[the.rampage.rado]

What's the payment method you're using?

[musicmaster]

12 hours ago, RabbitZzZ said:

I have to put this up again, beacause the refunds are still not functioning correctly.

The shipping costs hardly ever get on there correctly.

First scenerario:
- order via bankwire, payment received
- partial refund, all products with full price and shipping cost
-> shipping (tax incl.) on the order slip get rounded down to full euro

Second scenario
- order via bankwire, payment received
- partial refund, only shipping cost put in
-> order slip with all products refunded, shipping rounded down to full euro

Other people have a problem to reproduce your problem. So the obvious next step would be that you try to reproduce it on a test shop (just a fresh TB 1.1 installation on localhost). That would help to pinpoint the problem.

[RabbitZzZ]

I did and the problem persists.

Installed TB 1.1.0 with demo data and tried to refund the first demo order.

Also added a new order manually, set it to payment received and tried to refund all products and shipping via partial refund

-> refunded product and shipping prices get rounded down
-> prices and taxes on order slip are not congruent

I attached a screenshot, invoice and order slip for illustration.

FA000002.pdf order-slip-000002.pdf

[Traumflug]

Let's not forget this: https://github.com/thirtybees/thirtybees/issues/1077

[RabbitZzZ]

Bank transfer in this case

[the.rampage.rado]

And probably you rely heavily on the inbuild income stats?

Just refund the money and then if needed manually corect this in the report. I doubt it happens more than once in a month. There are far more important bugs or missing functionalities in the code.

[RabbitZzZ]

It's not a problem of the stats but merely of having correct accounting. The invoices and order slips have to conform to bank account activity. This is important because of correct tax calculation and everything.

I suppose the error is somewhere in the condition in line 813 of AdminOrdersController.php 

[the.rampage.rado]

If you use the inbuild invoicing that's another word...

[RabbitZzZ]

Yes we do.

[Occam]

3 hours ago, RabbitZzZ said:

It's not a problem of the stats but merely of having correct accounting. The invoices and order slips have to conform to bank account activity. This is important because of correct tax calculation and everything.

Which they do! But after the refund you find another document - the credit note for refund. This is the proper way to refund or partially refund an order. Neither the original order nor the invoice should be changed for legal reasons. In PrestaShop it's just the same.

6 hours ago, RabbitZzZ said:

I have an order which has already been refunded. The partial refund of the products worked but the shop manager forgot to put in the value for the shipping costs I suppose. If we now want to do partial refund again (1 product in the order already completely refunded) and only put in the value in the shipping field and try to save it, we always get the error "The partial refund data is incorrect.".

Must be for some other reason. I cannot confirm the described behavior. I made 2 different partial refunds for he same order (first a product, then the shipping costs) and both worked without any error message. And for each action a correct credit note was generated. This even works if a state Payment refund was added after the first refund.

Edited August 29, 2019 by Occam

[RabbitZzZ]

Was it also the one and only product in that same order? When I try and no products in the order are left for refunding I can't refund just the shipping costs on their own.

And: order slip = credit note (the template in TB is called order-slip), this isn't correct if the shipping costs aren't on there but are in fact refunded

Edited August 29, 2019 by RabbitZzZ

[Occam]

2 hours ago, RabbitZzZ said:

order slip = credit note

Yep

2 hours ago, RabbitZzZ said:

Was it also the one and only product in that same order?

Same result, no matter if one or more products. Testet only with tb 1.1.0.

[RabbitZzZ]

Ok, what would you have done differently then?

In my case it doesn't work in 2 shops. Both are recently updated to 1.1.0 without errors.

What I have done:

- create order with one product
- set order to payment received
- partial refund for the product
- partial refund for the shipping cost -> ERROR

[Occam]

i tested the same for orders with single or several products. The only difference is: I did it on a fresh (and not upgraded) 1.1.0. And with only native modules installed.

[colorful-ant]

I'm not sure if it might be due to input errors. I have no problems with versions 1.0.7, 1.0.8 and 1.1.0.

Were the shipping costs also the correct value?

 

 

EDIT:
my pdf´s are correct with refund and bank wire

Edited August 30, 2019 by colorful-ant

[RabbitZzZ]

That's very odd. Input is 100% right.

Also: Before the update there weren't any shipping costs on the pdf automatically. Now after the update, the shipping costs are refunded automatically, even if I don't type them in the field. The difference then ist, that they don't appear as an extra line on the PDF but only in the sum. If I put them in the field before refunding, everything ist correct.

[RabbitZzZ]

I have to put this up again, beacause the refunds are still not functioning correctly.

The shipping costs hardly ever get on there correctly.

First scenerario:
- order via bankwire, payment received
- partial refund, all products with full price and shipping cost
-> shipping (tax incl.) on the order slip get rounded down to full euro

Second scenario
- order via bankwire, payment received
- partial refund, only shipping cost put in
-> order slip with all products refunded, shipping rounded down to full euro

[Pedalman]

It is a hijack but I still dream about an order edit / credit-slip edit / order status delete/add/edit function for orderid in TB. Would be very handy in many situations.

[Occam]

Hi @RabbitZzZ, the current tax calculation works pretty good for invoices. But the refund is a mess in 1.1.0, no matter if master or dev release. You may circumvent the bug(s) by using an elder version of the TaxCalculator.php which I attach in the zip below. The refund of shipping costs will work correct then, but even if you just refund the shipping costs, the product taxes are always displayed in tax details with a minus sign. It doesn't effect the total refund amount but is confusing for customers, like in this credit slip:

Seems there is more than one bug to fix.

A working TaxCalculator.php: Fix TaxCalculator Bug.zip

[datakick]

14 hours ago, Occam said:

Hi @RabbitZzZ, the current tax calculation works pretty good for invoices. But the refund is a mess in 1.1.0, no matter if master or dev release. You may circumvent the bug(s) by using an elder version of the TaxCalculator.php which I attach in the zip below. The refund of shipping costs will work correct then, but even if you just refund the shipping costs, the product taxes are always displayed in tax details with a minus sign. It doesn't effect the total refund amount but is confusing for customers, like in this credit slip:

Seems there is more than one bug to fix.

A working TaxCalculator.php

I fail to understand what your code fixes.

I have downloaded your fix, and compared it to 1.1.x version. The only difference is in rounding -- the new version rounds values to _TB_PRICE_DATABASE_PRECISION_ (6 decimal points), whereas your version does not round at all.

diff --git a/classes/tax/TaxCalculator.php b/classes/tax/TaxCalculator.php
index 0b007c9330..be0a9b06c8 100644
--- a/classes/tax/TaxCalculator.php
+++ b/classes/tax/TaxCalculator.php
@@ -92,10 +92,7 @@ class TaxCalculatorCore
      */
     public function addTaxes($priceTaxExcluded)
     {
-        return round(
-            $priceTaxExcluded * (1 + ($this->getTotalRate() / 100)),
-            _TB_PRICE_DATABASE_PRECISION_
-        );
+        return $priceTaxExcluded * (1 + ($this->getTotalRate() / 100));
     }

     /**
@@ -110,10 +107,7 @@ class TaxCalculatorCore
      */
     public function removeTaxes($priceTaxIncluded)
     {
-        return round(
-            $priceTaxIncluded / (1 + $this->getTotalRate() / 100),
-            _TB_PRICE_DATABASE_PRECISION_
-        );
+        return $priceTaxIncluded / (1 + $this->getTotalRate() / 100);
     }

     /**
@@ -176,16 +170,10 @@ class TaxCalculatorCore

         foreach ($this->taxes as $tax) {
             if ($this->computation_method == static::ONE_AFTER_ANOTHER_METHOD) {
-                $taxesAmounts[$tax->id] = round(
-                    $priceTaxExcluded * (abs($tax->rate) / 100),
-                    _TB_PRICE_DATABASE_PRECISION_
-                );
+                $taxesAmounts[$tax->id] = $priceTaxExcluded * (abs($tax->rate) / 100);
                 $priceTaxExcluded = $priceTaxExcluded + $taxesAmounts[$tax->id];
             } else {
-                $taxesAmounts[$tax->id] = round(
-                    $priceTaxExcluded * (abs($tax->rate) / 100),
-                    _TB_PRICE_DATABASE_PRECISION_
-                );
+                $taxesAmounts[$tax->id] = ($priceTaxExcluded * (abs($tax->rate) / 100));
             }
         }

 

[Occam]

First of all: This is not "my version", I just took it from a former release of tb. 

However, using this version prevents the shipping costs from being rounded to the full dollar/euro, as tb 1.1.0 does with refund by default. I don't claim to have solved all problems caused by the changed core rounding methods. This is just a quick'n'dirty solution to help the user  - and it works for the credit slip.

Now the programmers of tb got to find out what is causing this bug. I just wanted to offer an interim solution to avoid it.

Edited October 20, 2019 by Occam

[datakick]

1 hour ago, Occam said:

First of all: This is not "my version", I just took it from a former release of tb. 

However, using this version prevents the shipping costs from being rounded to the full dollar/euro, as tb 1.1.0 does with refund by default. I don't claim to have solved all problems caused by the changed core rounding methods. This is just a quick'n'dirty solution to help the user  - and it works for the credit slip.

Now the programmers of tb got to find out what is causing this bug. I just wanted to offer an interim solution to avoid it.

You should verify that you have correctly defined constant _TB_PRICE_DATABASE_PRECISION_ in file defined.inc.php. It's possible that it's missing in your system --> php would probably consider missing value it as 0, resulting in the weird behavior you are describing.

If the constant is defined and has value 6, it's not really possible for the the result to be rounded to full dollar/euro.

This new rounding code ensures that result contains numbers like this: 39.999999 instead of 39.999999813893179847984789149816487163821357615

 

[Occam]

17 minutes ago, datakick said:

You should verify that you have correctly defined constant _TB_PRICE_DATABASE_PRECISION_ in file defined.inc.php. It's possible that it's missing in your system --> php would probably consider missing value it as 0, resulting in the weird behavior you are describing.

😁 This was the first I did check. How about you test first my proposal before you make any speculation as to what I might have done wrong.  That is not the best approach. 

[datakick]

14 minutes ago, Occam said:

How about you test first my proposal before you make any speculation as to what I might have done wrong.  

I did. In fact, I can't reproduce this problem on my 1.1.x at all. With or without your changes. That's why I speculate what's wrong. @RabbitZzZ said he installed TB 1.1.0 with demo data and tried to refund the first demo order. I did the same, and it worked fine. 

In order to investigate further, I need exact reprosteps.

[Occam]

I did not use demo data. And I am able to reproduce this behavior in Bleeding Edge, too.

Edited October 20, 2019 by Occam