firegento / firegento-pdf

Generates nicer and configurable PDF for invoices, creditmemos and shippings in Magento
100 stars 62 forks source link

Shipping tax displayed seperatly in grandtotal -> how to change? #316

Open yuberlin opened 8 years ago

yuberlin commented 8 years ago

In the totals block taxes included in shipping costs are displayed seperatly. In order views or cart the taxes for shipping are combined with the product taxes.

So this is correct (taxes for shipping added to the 19%):

Subtotal €26.50: Shipping & Handling: €3.50 Grand Total (Excl.Tax): €26.90 MwSt. 7% (7%): €1.18 MwSt. 19% (19%): €1.92 Total Tax: €3.10 Grand Total (Incl.Tax) €30.00

And this is the grandtotal in invoice PDF:

Subtotal: 26,50 € Shipping & Handling: 3,50 € Grand Total (Excl.Tax): 26,90 € Shipping Taxes: 0,56 € MwSt. 7% (7,00%) 1,18 € MwSt. 19% (19,00%) 1,36 € Total Tax: 3,10 € Grand Total (Incl.Tax): 30,00 €

How i add the shipping tax to the other 19% taxes? Do i have to change Abstract.php (function insertTotals)?

sprankhub commented 8 years ago

I understand your issue and I think it would be much better if the shipping taxes would be included in the 7% / 19% taxes, but this is how Magento handles it. You may be able to change this with a hack in the insertTotals method, yeah. Mind that the shipping tax does not have to be 19% though.

nige-one commented 8 years ago

Had the same issue. Just add a model to your config.xml <grand_total> node like this:

<config>
    <global>
        <pdf>
            <totals>
                <grand_total>
                    <model>Namespace_Module_Model_Tax_Sales_Pdf_Grandtotal</model>
                </grand_total>
            </totals>
        ...

Your Grandtotal.php should have:

class Namespace_Module_Model_Tax_Sales_Pdf_Grandtotal extends FireGento_Pdf_Model_Tax_Sales_Pdf_Grandtotal
{

    public function _getShippingTax()
    {
        return 0;
    }

}

Luckily the origin _getShippingTax alters the grand_total taxes, so replacing it will leave the shipping tax in it.

sprankhub commented 8 years ago

@nige-one serious? If you return null, the shipping tax will be included into the 7% / 19% tax amounts!?

Schrank commented 8 years ago

We should add a section to the FAQ for this. Me personally wanted this feature at least once :-)

sprankhub commented 8 years ago

If this works reliably in all versions, we should implement a configuration option for it and should default it to "yes". In theory, it is a requirement in Germany to show the exact tax amount. So this is the last bit of making our PDFs valid for Germany.

nige-one commented 8 years ago

I guess returning a 0 will be more accurate, since I'm quite sure this gets subtracted from the corresponding tax amount somewhere. Will have a look into this and implement it as option, since this was my first thought as well.

sasoriza commented 7 years ago

Would be great to add this option and set it to yes by default. Why would Magento allow me to chose a tax class for shipping if it ignores this setting and instead always adds an extra tax field to my totals even if I select the same tax class for shipping as for products? This just doesn't make any sense...

sprankhub commented 7 years ago

@nige-one will you implement this or should I try to do it?

nige-one commented 7 years ago

Not sure if this makes sense because this behaviour results from Magento core. Look at getFullTaxInfo where the two arrays $taxClassAmount, $shippingTax are merged. If the resulting array is not empty each array element will be rendered for the totals block. This includes Magento's Shipping & Handling Tax line in the totals block which comes from $shippingTax. If the array is empty Magento uses another way of tax calculation for total block rendering. In this case all tax amounts are summed up by tax rates, which leads to the wanted behaviour.

Firegento_PDF relies on Magento's total block rendering, since getFullTaxInfo comes from core. So the place to go should be Magento's total block rendering process.

My first solution is still my favorite one. But of instead of returning 0 you have to return an empty array of course.

class Namespace_Module_Model_Tax_Sales_Pdf_Grandtotal extends FireGento_Pdf_Model_Tax_Sales_Pdf_Grandtotal
{
    public function _getShippingTax()
    {
        return array();
    }

    public function _getCalculatedTaxes()
    {
        return array();
    }
}

To make it an option in Firegento_PDF I think using _getFullRateInfo is the path to follow.

@sprankhub maybe we can talk this through before implementing it?!?

sprankhub commented 7 years ago

If your idea/fix results in the situation that taxes are splitted by tax rate, I think it is great! Since we already rewrite Mage_Tax_Model_Sales_Pdf_Grandtotal, we can simply add it there. So if you implement that and send a PR, I am happy to test! I am just not sure whether we still need the options NO_SUM_ON_DETAILS and HIDE_GRANDTOTAL_EXCL_TAX then...