Problem/Motivation

In #3310714: Properly process complex tax responses, when possible, each tax line returned by AvaTax gets it's own line on the order. However, some merchants prefer the simple "Sales tax" line.

Proposed resolution

Let's make this a configuration option. It can be added to the Advanced section of the AvaTax configuration form.

Remaining tasks

  • Add a checkbox to the configuration form.
  • Add a boolean to the commerce_avatax.schema.yml
  • Add a condition to ">Drupal\commerce_avatax\Plugin\Commerce\TaxType::apply()

User interface changes

An optionally simplified tax line on the order.

Data model changes

commerce_avatax.schema.yml

commerce_avatax.settings:
  mapping: 
    combine_lines: 
      type: boolean
      label: 'Combine tax lines'

Issue fork commerce_avatax-3472117

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

scottsawyer created an issue. See original summary.

djween’s picture

djween commented

This is definitely needed as state and county tax are split on separate line items (which is not the ecommerce norm) for US tax and it confuses the end user customer.

scottsawyer opened merge request !23

scottsawyer’s picture

scottsawyer
Primary language English
Location Atlanta
commented
Status: Active » Needs review

Ok, what the MR does is add a toggle in the Avatax setting to consolidate taxes. It also provides a "label" field, so you can call your taxes whatever you want.

Then, on the order, in the apply() method, it simply renames all of the taxes to whatever you named it. Commerce will automatically combine exact match adjustments (ignoring the amount). This has the effect of consolidating the tax lines on the order.

jsacksick made their first commit to this issue’s fork.

jsacksick’s picture

jsacksick commented
Status: Needs review » Fixed

@scottsawyer: Trusting you on this one, though I suspect the following line might be problematic:

$label = $config->get('combine_tax_lines_label') ?? $this->t('Sales tax');
As "combine_tax_lines_label" is an empty string by default (and the "??" operator works for NULL values).

Perhaps it should have been:

$label = $config->get('combine_tax_lines_label') ?: $this->t('Sales tax');

instead, but since you have a :required state (which in my experience was buggy in the past), it shouldn't be a problem hopefully.

  • jsacksick committed 1dae11e6 on 8.x-1.x 
    Revert "Issue #3472117 by scottsawyer, djween: Allow consolidating sales...

  • jsacksick committed 09c8a849 on 2.x 
    Revert "Issue #3472117 by scottsawyer, djween: Allow consolidating sales...
jsacksick’s picture

jsacksick commented
Status: Fixed » Needs work

This was discussed internally and it was finally decided to revert the MR for the following reasons:

1. There are quite a few jurisdictions that require the taxes to be itemized. Allowing them to be arbitrarily combined by a checkbox can cause a merchant out of compliance. (While some jurisdictions might not have a requirement, you are safer to itemize than not.)
2. If the merchant changes the label, the source_id will be now be inconsistent between orders.
3. If the merchant unchecks the combine checkbox, the source_id will be inconsistent. Additionally, any order that took place while the checkbox was checked are not “switched back”, requiring a data update to fix. (And there isn’t an easy way to get the original tax label.)
4. The fallback label passing through $this->t() means that the source_id will be different, depending on language. (To be fair, this isn’t specific to this MR...our current fallback does the same...I just noticed this while reviewing this MR.)
5. As referenced in the other issue, the percentage/rate will be incorrect when adjustments are combined, as only the amounts are summed. For example, on a $100 transaction with a state rate of 5% and city rate of 1%, the state tax is $5 and the city tax is $1. When the adjustments are combined, they creates an amount of $6. But the percentage/rate will be 5% (or 1% depending on sort order of the adjustments.)
Some options to consider:
1. Check if there is a setting in Avatax to combine tax lines. (Doubtful, but worth a look.)
2. Check if there is an indicator in the lines payload that would signal to us whether tax lines can be combined, or must be displayed individually.
3. Consider handling this at the adjustmenttransformer or display level, rather than here. (Or, adding an additional property to adjustments, rather than overwriting the source_id...this would require an enhancement in commerce core.)

scottsawyer’s picture

scottsawyer
Primary language English
Location Atlanta
commented

Thanks for your consideration and feedback, we will certainly look into the options you presented. Though, I have to say, I find some of the reasoning behind rejecting the current implementation a bit flimsy.

  • Previous to the issue referenced, taxes were already combined into a single tax adjustment, so the inconsistency of the source_id was already thrust upon us with no way out.
  • This issue doesn't change the default behavior, it is a choice given to the merchant. In fact, it just optionally restores a behavior that some merchants expect.

Anyway, this is a source of contention for our customer for a while now, so we'll keep looking into alternative approaches.

rhovland’s picture

rhovland
he/him
Primary language English
Location Oregon
commented

I agree that this should be a display option and not actually touch the underlying tax information. The problem (which we've had customers debate with us about because they're "tax experts") is the split lines display of taxes on the order/checkout not what gets recorded with avatax.

This probably means a feature in commerce_tax, not here, that combines the display of tax adjustments when configured to do so.