Skip to content
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

Add dynamic shipping tax calculation #176

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 18 additions & 5 deletions Model/Config.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* Copyright © 2016 FireGento e.V.
* See LICENSE.md bundled with this module for license details.
*/

namespace FireGento\MageSetup\Model;

use FireGento\MageSetup\Model\Config\Reader;
Expand All @@ -13,6 +14,8 @@
*/
class Config implements ConfigInterface
{
public const CONFIG_PATH_DYNAMIC_SHIPPING_TAX_CLASS = 'tax/classes/dynamic_shipping_tax_class';

/**
* Configuration reader
*
Expand Down Expand Up @@ -40,17 +43,17 @@ class Config implements ConfigInterface
/**
* Config constructor.
*
* @param Reader $reader
* @param Reader $reader
* @param CacheInterface $cache
* @param mixed $country
* @param mixed $country
*/
public function __construct(
Reader $reader,
CacheInterface $cache,
$country
) {
$this->reader = $reader;
$this->cache = $cache;
$this->reader = $reader;
$this->cache = $cache;
$this->country = $country;

$this->initialize();
Expand Down Expand Up @@ -188,7 +191,17 @@ public function getCmsBlocks()
*/
private function initialize()
{
$data = $this->reader->read();
$data = $this->reader->read();
$this->loadedConfig = $data;
}

/**
* Method for retrieving dynamic config shipping path
*
* @return string
*/
public function getDynamicShippingConfigPath(): string
{
return self::CONFIG_PATH_DYNAMIC_SHIPPING_TAX_CLASS;
Copy link
Member

Choose a reason for hiding this comment

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

I do not think this adds much value. I think we should add a new method in \FireGento\MageSetup\Model\System\Config, which returns the actual configuration value of that option.

}
}
48 changes: 48 additions & 0 deletions Model/System/Config/Source/Tax/Dynamic.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
<?php
/**
* Copyright © 2020 FireGento e.V.
* See LICENSE.md bundled with this module for license details.
*/

namespace FireGento\MageSetup\Model\System\Config\Source\Tax;

use Magento\Framework\Data\OptionSourceInterface;

/**
* Source model for Dynamic taxes.
*/
class Dynamic implements OptionSourceInterface
{
public const DYNAMIC_TYPE_SHIPPING_TAX_DEFAULT = 0;
Copy link
Member

Choose a reason for hiding this comment

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

Maybe DYNAMIC_SHIPPING_TAX_NO?

public const DYNAMIC_TYPE_HIGHEST_PRODUCT_TAX = 1;
Copy link
Member

Choose a reason for hiding this comment

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

Maybe DYNAMIC_SHIPPING_TAX_HIGHEST_PRODUCT_TAX?


/**
* @var array
*/
private $options;

/**
* Method for returning options array
*
* @return array|array[]
*/
public function toOptionArray(): array
{
if (null === $this->options) {
$options = [
[
'value' => self::DYNAMIC_TYPE_SHIPPING_TAX_DEFAULT,
'label' => __('No dynamic shipping tax calculation')
],
[
'value' => self::DYNAMIC_TYPE_HIGHEST_PRODUCT_TAX,
'label' => __('Use the highest product tax')
]
];

$this->options = $options;
}

return $this->options;
}
}
192 changes: 192 additions & 0 deletions Plugin/Tax/Config/ShippingTaxPlugin.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,192 @@
<?php

declare(strict_types=1);

namespace FireGento\MageSetup\Plugin\Tax\Config;

use FireGento\MageSetup\Model\Config as FireGentoConfig;
use FireGento\MageSetup\Model\System\Config\Source\Tax\Dynamic as FireGentoSource;
use Magento\Checkout\Model\Cart;
use Magento\Customer\Model\ResourceModel\GroupRepository;
use Magento\Customer\Model\Session;
use Magento\Framework\App\Config\ScopeConfigInterface;
use Magento\Quote\Model\Quote\Item;
use Magento\Store\Model\ScopeInterface;
use Magento\Store\Model\Store;
use Magento\Tax\Model\Calculation\Proxy;
use Magento\Tax\Model\Config;

/**
* Class ShippingTaxPlugin
*
* FireGento\MageSetup\Plugin\Tax\Config
*/
class ShippingTaxPlugin
{

/**
* @var ScopeConfigInterface
*/
private $scopeConfig;

/**
* @var Cart
*/
private $cart;

/**
* @var Session
*/
private $customerSession;

/**
* @var GroupRepository
*/
private $groupRepository;

/**
* @var Proxy
*/
private $taxCalculation;

/**
* @var FireGentoConfig
*/
private $config;

/**
* Constructor class
*
* @param ScopeConfigInterface $scopeConfig
* @param Cart $cart
* @param Session $customerSession
* @param GroupRepository $groupRepository
* @param FireGentoConfig $config
*/
public function __construct(
ScopeConfigInterface $scopeConfig,
Cart $cart,
Session $customerSession,
GroupRepository $groupRepository,
FireGentoConfig $config
) {
$this->scopeConfig = $scopeConfig;
$this->cart = $cart;
$this->customerSession = $customerSession;
$this->groupRepository = $groupRepository;
$this->config = $config;
}

/**
* After plugin for \Magento\Tax\Model\Config::getShippingTaxClass
*
* @param Config $config
* @param int $shippingTaxClass
* @param null $store
*
* @return bool|int|mixed
*/
public function afterGetShippingTaxClass(Config $config, int $shippingTaxClass, $store = null)
{
$dynamicType = (int)$this->scopeConfig->getValue(
$this->config->getDynamicShippingConfigPath(),
ScopeInterface::SCOPE_STORE,
$store
);

$quoteItems = $this->cart->getItems();

// If the default behaviour was configured or there are no products in cart, use default tax class id
if ($dynamicType === FireGentoSource::DYNAMIC_TYPE_SHIPPING_TAX_DEFAULT || count($quoteItems) === 0) {
return $shippingTaxClass;
}

$taxClassId = false;

// Retrieve the highest product tax class
if ($dynamicType === FireGentoSource::DYNAMIC_TYPE_HIGHEST_PRODUCT_TAX) {
$taxClassId = $this->getHighestProductTaxClassId($quoteItems, $store);
}

// If no tax class id was found, use default one
if (!$taxClassId) {
$taxClassId = $shippingTaxClass;
}

return $taxClassId;
}

/**
* Method for getting highest product tax class id
*
* @param array $quoteItems
* @param null|string|bool|int|Store $store
*
* @return bool|mixed|null
*/

Copy link
Member

Choose a reason for hiding this comment

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

Please remove this line.

private function getHighestProductTaxClassId($quoteItems, $store)
{
$taxClassIds = [];
$highestTaxRate = null;

foreach ($quoteItems as $quoteItem) {

/** @var $quoteItem Item */
if ($quoteItem->getParentItem()) {
continue;
}

// Retrieve the tax percent
$taxPercent = $quoteItem->getTaxPercent();
if (!$taxPercent) {
$taxPercent = $this->getTaxPercent($quoteItem->getTaxClassId(), $store);
}

// Add the tax class
if (($taxPercent) && !in_array($taxPercent, $taxClassIds)) {
$taxClassIds[$taxPercent] = $quoteItem->getTaxClassId();
}
}

/**
* Fetch the highest tax rate
*/
ksort($taxClassIds);
if (count($taxClassIds) > 0) {
$highestTaxRate = array_pop($taxClassIds);
}
if (!$highestTaxRate || $highestTaxRate === null) {
return false;
}

return $highestTaxRate;
}

/**
* Method for getting tax prcentage
*
* @param int $productTaxClassId
* @param null|string|bool|int|Store $store
*
* @return int
* @throws \Magento\Framework\Exception\LocalizedException
* @throws \Magento\Framework\Exception\NoSuchEntityException
*/
private function getTaxPercent(int $productTaxClassId, $store): int

Choose a reason for hiding this comment

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

The returnType of the rate is float, not int

{
$groupId = $this->customerSession->getCustomerGroupId();
$group = $this->groupRepository->getById($groupId);
$customerTaxClassId = $group->getTaxClassId();

$request = $this->taxCalculation->getRateRequest(null, null, $customerTaxClassId, $store);

Choose a reason for hiding this comment

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

This is not actually set in the constructor.

$request->setData('product_class_id', $productTaxClassId);

$taxPercent = $this->taxCalculation->getRate($request);
if (!$taxPercent) {
$taxPercent = 0;
}

return $taxPercent;
}
}
14 changes: 10 additions & 4 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,16 @@
"name": "firegento/magesetup2",
"description": "MageSetup provides the necessary configuration (system config, tax, agreements, etc. for a national market.",
"require": {
"php": "~7.2.0||~7.3.0",
"magento/module-store": "*",
"magento/module-backend": "*",
"magento/framework": "*"
"php": "~7.2.0|~7.3.0",
Copy link
Member

Choose a reason for hiding this comment

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

Please fix the formatting.

"magento/framework": "*",
"magento/module-backend": "*",
"magento/module-catalog": "*",
"magento/module-checkout": "*",
"magento/module-configurable-product": "*",
"magento/module-customer": "*",
"magento/module-quote": "*",
"magento/module-store": "*",
"magento/module-tax": "*"
},
"type": "magento2-module",
"license": "GPL-3.0",
Expand Down
8 changes: 8 additions & 0 deletions etc/adminhtml/system.xml
Original file line number Diff line number Diff line change
Expand Up @@ -142,5 +142,13 @@
</field>
</group>
</section>
<section id="tax">
<group id="classes">
<field id="dynamic_shipping_tax_class" translate="label" type="select" sortOrder="11" showInDefault="1" showInWebsite="1" showInStore="0">
<label>Dynamic Shipping Tax Class</label>
<source_model>FireGento\MageSetup\Model\System\Config\Source\Tax\Dynamic</source_model>
</field>
</group>
</section>
</system>
</config>
4 changes: 4 additions & 0 deletions etc/di.xml
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@
<plugin name="mageSetupEmailSourceVariables" type="FireGento\MageSetup\Plugin\Email\Model\Source\Variables"/>
</type>

<type name="Magento\Tax\Model\Config">
<plugin name="mageSetupDynamicShippingTaxClass" type="FireGento\MageSetup\Plugin\Tax\Config\ShippingTaxPlugin" sortOrder="1"/>
</type>

<!-- Email variables for versions >= M2.3.0-->
<type name="Magento\Variable\Model\Source\Variables">
<arguments>
Expand Down
6 changes: 5 additions & 1 deletion etc/module.xml
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,14 @@
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:Module/etc/module.xsd">
<module name="FireGento_MageSetup" setup_version="2.0.1">
<sequence>
<module name="Magento_Store"/>
<module name="Magento_Backend"/>
<module name="Magento_Catalog"/>
<module name="Magento_Checkout"/>
<module name="Magento_Customer"/>
<module name="Magento_ConfigurableProduct"/>
<module name="Magento_Quote"/>
<module name="Magento_Store"/>
<module name="Magento_Tax"/>
</sequence>
</module>
</config>
2 changes: 2 additions & 0 deletions i18n/de_DE.csv
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,5 @@ SWIFT,SWIFT
"CMS Page for Shipping Info","CMS-Seite für Versandkosten"
"Show ""incl. Shipping Cost"" instead of ""excl. Shipping Cost""","Zeige ""inkl. Versandkosten"" anstatt ""exkl. Versandkosten"""
"Display Delivery Time on Product Listing","Lieferzeit auf Produktübersichtsseiten anzeigen"
"No dynamic shipping tax calculation","Keine dynamische Versandsteuerberechnung"
Copy link
Member

Choose a reason for hiding this comment

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

Please re-generate the English language file and then edit the German file. Hint:

n98-magerun2.phar i18n:collect-phrases -o app/code/FireGento/MageSetup/i18n/en_US.csv app/code/FireGento/MageSetup/

Copy link
Member

Choose a reason for hiding this comment

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

It seems that you re-generated the English file. Great! Now IMHO, we need to update the German file accordingly (add the strings to the position similar to the EN file, not at the end of the file).

"Use the highest product tax","Verwenden Sie die höchste Produktsteuer"
3 changes: 3 additions & 0 deletions i18n/en_US.csv
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
%1%,%1%
"SubProcessor %1 does not exist.","SubProcessor %1 does not exist."
"-- No Page --","-- No Page --"
"No dynamic shipping tax calculation","No dynamic shipping tax calculation"
"Use the highest product tax","Use the highest product tax"
"Visible in Checkout","Visible in Checkout"
Imprint,Imprint
"Shop Name","Shop Name"
Expand Down Expand Up @@ -55,3 +57,4 @@ SWIFT,SWIFT
"CMS Page for Shipping Info","CMS Page for Shipping Info"
"Show ""incl. Shipping Cost"" instead of ""excl. Shipping Cost""","Show ""incl. Shipping Cost"" instead of ""excl. Shipping Cost"""
"Display Delivery Time on Product Listing","Display Delivery Time on Product Listing"
"Dynamic Shipping Tax Class","Dynamic Shipping Tax Class"