Add condition support for commerce_tax_type entity type

I've realized that using Drupal conditions on the tax rate config entities wouldn't cover the use case where no rate should be used (tax holiday, tax exemptions), so this will need to go through code or Rules instead. Updated the issue summary.

Still applicable in the new commerce_tax, we want TaxRule content entities that will embed conditions ala promotions.
They can then be evaluated by the DefaultTaxRateResolver.
We need to finalize the conditions API first for that to be useful.

I'm not sure how to implement it, as there is rules conditions but for promotions we use commerce conditions. I don't think we can put commerce conditions to rules, can we? Why commerce conditions do not inherit from Core conditions like rules conditions?

I think this must get into the RC, it's something to basic, any store needs to add some conditions for taxes.

I'm not sure how to implement it, as there is rules conditions but for promotions we use commerce conditions. I don't think we can put commerce conditions to rules, can we? Why commerce conditions do not inherit from Core conditions like rules conditions?

We use (or will use) commerce conditions for everything, yes. Promotions, payment gateways, shipping methods, tax rules.
There was no gain in extending core conditions because we use them in different ways and have different expectations. I'd argue that even Rules is wrong to reuse core conditions, since they rely on everything being context, instead of using config forms.

I think this must get into the RC, it's something to basic, any store needs to add some conditions for taxes.

This use case can currently be satisfied by writing a TaxRateResolver. That's why this UI is not a blocker for RC1 (but will be added prior to 2.0)

@bojanz

This use case can currently be satisfied by writing a TaxRateResolver.

Is an example out there for writing an custom TaxRateResolver? Thanks in advance.

Use DefaultTaxRateResolver or ChainTaxRateResolver as example
You just have to implement TaxRateResolverInterface and in your services add your Resolver with more priority:

services:
  your_module.custom_tax_rate_resolver:
    class: Drupal\your_module\Resolver\CustomTaxRateResolver
    tags:
      - { name: commerce_tax.tax_rate_resolver, priority: 100 }

@edurenye thanks, I will try that!

@mathiasgmeiner
Here is an example from my implementation of a tax rate resolver. When the ordered item is a 'book', the reduced rate should be used. Else the default rate.

Those files are not related to the issue itself.

@MegaChriz that's so nice of you! Thanks a lot!

@bojanz, you said tax rules UI "will be added prior to 2.0", but it doesn't seem that way. Are we waiting for the first stable D8 release of Rules then or do I misunderstand the issue ?

We are not depending on Rules, so that doesn't matter.

This feature got postponed to a future release because we didn't have time to write it before 2.0. See #2913801: Commerce 2.x roadmap.

This needs the issue summary to be updated to outline the work that's needed here.

It looks like lots of users of Commerce are writing custom code to deal with this -- with some spec on what's needed for Commerce, perhaps some of that code could be put towards a patch instead?

I have written a resolver, (thanks MegaChriz for the example) but am having a hard time applying two tax rates to a product type.

The store is in Canada, but I cannot use the built in Canadian taxes because I have some products that get only GST and some products that are entirely tax free.

So if I have just one tax type with three tax rates (PST, GST, none), how do I go about applying PST & GST to a product type?

We have now published a module which satisfies the majority use case: https://www.drupal.org/project/commerce_product_tax

It allows you to specify which variations are tax free, or use one of the non-standard rates (reduced, zero, etc).
It is also possible to select a rate for multiple zones, so in the EU you can select one rate for FR one for DE, etc.

I am going to remove this issue from the roadmap, and wait a few months to see how users use it. We can then decide whether we want to come back to this issue, promote the contrib, add the contrib as-is to Commerce, etc.

Want to chime in with one more scenario where TaxType conditions will prove themselves useful.

I have a site that serves both B2B and B2C. Both the merchant and its clients are based in Norway hence norwegian_vat-based TaxType was created to apply Norwegian VAT on all items. Unfortunately, TaxType settings for B2B and B2C must be slightly different - the business needs to have "Display taxes of this type inclusive in product prices." unchecked while for the latter tax (VAT) needs to be displayed inclusive in product prices. Since TaxTypeInterface::isDisplayInclusive doesn't take $order context I cannot make it dynamically return the value thus I had to create 2 TaxType entities - one for B2C and second for B2B - both based on the same plugin. Unfortunately, this leads to either of two TaxType entities applying at the same time causing VAT to be calculated (added) twice. Therefore I've been forced to extend the tax plugin and provide additional checks in the TaxTypeInterface::applies() method that allowed me to dynamically determine which plugin should apply.
This could quite easy solvable if TaxTypes (commerce_tax_type entity type) would have support for conditions.

Moreover, https://www.drupal.org/project/commerce_product_tax works on commerce_tax_type entity basis, and even if more tax types are using the same plugin it only resolves tax rate belonging to configured TaxType. To solve it I was required to create a custom resolver that matches the rate selected in the commerce_product_tax field with the rate coming from the same plugin but a different TaxType than set on the field configuration.

+1 on adding conditions support to tax types. It's a common request to limit the tax type application based on the order type, customer role... OR whatever else condition.

This could address issues like #3284780: Disable tax application by order type.

The attached patch adds conditions support to tax types.

1. I'm wondering if this should be a 3.x feature only.
2. I'm wondering if TaxType::applies() should invoke the applies() method of the tax type plugin.

We even have tests coverage for this newly added logic in the tax order processor.

Thoughts?

Test failures are unrelated.