[FR] Tax improvements

[Cruiser13]

Q	A
Bug report?	no
Feature request?	yes
BC Break report?	no
RFC?	yes

I'd like to request some improvements on the tax system. At the moment, product has one tax rule by default. We should have the possibility to add a default value set for this, otherwise you'll always have to add the same tax rule every time you're adding a product which is annoying if you have the same tax rule for ~99% of your products.

As of coreshop 2.0 final or 2.1 (not sure) the ability to set a Store to a tax rule has been removed. It'd be great to eighter being able to set one tax rule per store on the product itself or to add a store condition on the tax rules as the product is allowed to have exactly one tax rule at the moment but the tax rules might differ from store to store. I believe that we'd need to add the same product twice to the database, one to each store, with each one of the tax rules. I'd prefer to do it like with the store prices instead.

What do you think? Did I miss some configuration somewhere or how do you guys solve store specific taxes and setting tax rule on your products?

[dpfaffenbauer]

1. Defaulting to a tax-rule is not really possible, since you could potentially have a lot of rules. You can solve that in your project with pre-add event-listeners.
2. It didn't make any sense to configure a store on a tax rule when a product can be in multiple stores, that's why we removed it.
3. Yes, I guess it would make sense to configure tax rules on a store level, but not before https://github.com/coreshop/CoreShop/issues/903. If you have like 10 stores, and the same price and tax-rule, it is quite cumbersome dealing with that. So, yes, we could do multiple tax-rules per product

[solverat]

I'm with you @Cruiser13, it is very annoying to add store, active and taxdata every time you or your customer is adding a new product. And you also right - this is always the same process in 99.9999% percent. Also, it's quite dangerous too since it often leads to misconfiguration and weird frontend behaviors.

But! We're still unable define a default out of the box. CoreShop is still a framework which should allow you to define your custom workflows rather than defining every little conceptual detail. So we came up with a simple event listener, which is enabled in almost every CS project by default:

<?php

namespace AppBundle\EventListener;

use AppBundle\CoreShop\Model\ProductInterface;
use CoreShop\Component\Core\Model\CategoryInterface;
use CoreShop\Component\Resource\Repository\RepositoryInterface;
use CoreShop\Component\Store\Repository\StoreRepositoryInterface;
use CoreShop\Component\Taxation\Model\TaxRuleGroupInterface;
use CoreShop\Component\Taxation\Repository\TaxRuleRepositoryInterface;
use Pimcore\Event\DataObjectEvents;
use Pimcore\Event\Model\DataObjectEvent;
use Pimcore\Model\DataObject;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;

class ProductDefaultEnrichmentListener implements EventSubscriberInterface
{
    /**
     * @var StoreRepositoryInterface
     */
    protected $storeRepository;

    /**
     * @var TaxRuleRepositoryInterface
     */
    protected $taxRuleRepository;

    /**
     * @var RepositoryInterface
     */
    protected $taxRuleGroupRepository;

    /**
     * @param StoreRepositoryInterface $storeRepository
     * @param RepositoryInterface      $taxRuleGroupRepository
     */
    public function __construct(
        StoreRepositoryInterface $storeRepository,
        RepositoryInterface $taxRuleGroupRepository
    ) {
        $this->storeRepository = $storeRepository;
        $this->taxRuleGroupRepository = $taxRuleGroupRepository;
    }

    /**
     * @return array
     */
    public static function getSubscribedEvents()
    {
        return [
            DataObjectEvents::PRE_ADD => [
                ['setDefaultsToProduct', 0],
                ['setDefaultsToCategory', 1]
            ]
        ];
    }

    /**
     * @param DataObjectEvent $event
     */
    public function setDefaultsToProduct(DataObjectEvent $event)
    {
        $object = $event->getObject();
        if (!$object instanceof ProductInterface) {
            return;
        }

        if ($object->getType() === DataObject\AbstractObject::OBJECT_TYPE_VARIANT) {
            return;
        }

        if ($object instanceof ProductInterface) {
            if (!$object->getTaxRule() instanceof TaxRuleGroupInterface) {
                $taxRule = $this->taxRuleGroupRepository->findOneBy(['name' => 'YOUR_TAX_NAME']);
                $object->setTaxRule($taxRule);
            }

            if ($object->getActive() !== false) {
                $object->setActive(true);
            }

            if ($object->getIsTracked() !== true) {
                $object->setIsTracked(false);
            }
        }

        if (!is_array($object->getStores()) || count($object->getStores()) === 0) {
            $object->setStores([$this->storeRepository->findStandard()->getId()]);
        }
    }

    /**
     * @param DataObjectEvent $event
     */
    public function setDefaultsToCategory(DataObjectEvent $event)
    {
        $object = $event->getObject();
        if (!$object instanceof CategoryInterface) {
            return;
        }

        if ($object->getType() === DataObject\AbstractObject::OBJECT_TYPE_VARIANT) {
            return;
        }

        if (!is_array($object->getStores()) || count($object->getStores()) === 0) {
            $object->setStores([$this->storeRepository->findStandard()->getId()]);
        }
    }
}

[solverat]

BTW: we should add this to the documentation (A "Best Practice" section maybe?)

[dpfaffenbauer]

agreed

[Cruiser13]

@solverat thank you for your input. I thought we could use the same default setting pimcore does use for select fields in class for example. Event listeners are possible of course and a docs entry would be great too.

[dpfaffenbauer]

The default-values as Pimcore uses them, work only for within Pimcore Backend UI, not API wise. If we make a solution we make it good and not half thought thru. API wise (within Core Extensions) it is not possible to configure default values and set them on creation except for EventListener. I don't really like the idea of using an EventListener in Combination with CoreExtensions to set default values though....

So, for now, only thing is to use Custom Event Listeners and assign it yourself.