Skip to content

Fixes #316 - #361

Open
Schrank wants to merge 1 commit into
developmentfrom
feature/316
Open

Fixes #316#361
Schrank wants to merge 1 commit into
developmentfrom
feature/316

Conversation

@Schrank

@Schrank Schrank commented Oct 15, 2017

Copy link
Copy Markdown
Member

Thanks @nige-one

@Schrank
Schrank requested a review from sprankhub October 15, 2017 12:27
@sprankhub

Copy link
Copy Markdown
Member

Did you test this @Schrank? I just tested it and it does not change anything for me. The "Shipping & Handling Tax" is still shown separately and does not show any tax rate. Are you sure this works @nige-one?

@Schrank

Schrank commented Oct 15, 2017

Copy link
Copy Markdown
Member Author

No, I just added it and then left the room :) (no joke)

@nige-one

Copy link
Copy Markdown

It works like a charm for me. Just tested it again, dropped my XML totals extension from #316 (comment) and copied the new file from the PR.

Maybe a configuration issue. Here is mine:
image

Note the overridden fields, which do not seem to take affect, since everything is still working as expected in the B2C configuration.
This installation is version 1.9.3.6.

By the way: I changed

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

to

    public function _getCalculatedTaxes()
    {
        return $this->_getTaxHelper()->getCalculatedTaxes($this->getSource());
    }

I really can't remember why at the moment, but I think it has something to do with rendering the order itself as PDF to use it as order confirmation. Probably this addresses #297 (comment) as well.

@sprankhub

Copy link
Copy Markdown
Member

@nige-one could you provide a screenshot of the totals section of the invoice PDF? Mine looks like this:

image

As you can see, it still dows not show the tax rate of the shipping and handling tax.

@nige-one

Copy link
Copy Markdown

Ok, I have to extend my answer.
Shipping tax and value of goods taxes get only summed up if you return an empty array for both functions _getShippingTax() and _getCalculatedTaxes().
So @Schrank's PR should lead to the wanted behaviour.

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.

3 participants