Skip to content

MAGE-1205: Fix Revenue analytics' discount calculation with configured VAT #1758

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

Open
wants to merge 2 commits into
base: release/3.16.0-dev
Choose a base branch
from

Conversation

damcou
Copy link
Contributor

@damcou damcou commented May 23, 2025

This PR contains:

  • A fix for revenue calculation on place order events where discounts weren't properly calculated in case website has a setup with configured VAT.
  • Units tests for this precise feature.

@damcou damcou requested a review from cammonro May 23, 2025 09:41
@damcou damcou self-assigned this May 23, 2025
Copy link
Contributor

@cammonro cammonro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love the unit test - but I do have concerns about the tax calculations across different Magento configurations. Please see my comments in Jira for details.

@@ -268,7 +268,7 @@ protected function getQuoteItemDiscount(Item $item): float
*/
protected function getOrderItemSalePrice(OrderItem $item): float
{
return floatval($item->getPrice()) - $this->getOrderItemCartDiscount($item);
return floatval($item->getPriceInclTax()) - $this->getOrderItemCartDiscount($item);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This approach solves the issue with VAT when tax/calculation/price_includes_tax is set to 1 but breaks when it is not. Full details in Jira.

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