Line items with adjustments get double-applied discount amounts, causes under-reporting of tax liability

[jasonfb]

Please note I am using the 2-3-stable branch, I have not tested or compared to master branch.

Bug

Line item transactions are created in Avalara here:

https://github.com/railsdog/spree_avatax_certified/blob/2-3-stable/app/models/spree/avalara_transaction.rb#L295

Notice that the field used is 'pre_tax_amount', which is a field directly on the line_items table.

The table contains fields for quantity, price, and adjustment_total. The price column holds the pirce of the item before it is discounted, so the acutal price before the discount is quantity * price.

The discounted price is (quantity * price) + adjustment_total. (Note that adjustment_total is stored here in a negative values, as the total amount of adjustment that this line item has applied to it.)

Note that Spree stores pre_tax_amount as the result of the equation (quantity * price) + adjustment_total, so pre_tax_amount contains the price of the items after they have received their discount.

When the total transaction is passed to Avalara, one of the global (top-level) fields passed is "Discount", seen here:

https://github.com/railsdog/spree_avatax_certified/blob/2-3-stable/app/models/spree/avalara_transaction.rb#L419

Because all_adjustments scoops up both the order-level adjustments and the line item adjustments, this appears to create additional discounts in Avatax, thus the Avatax system is recording discounts on top of already-discounted prices, reducing the taxable amount to below the actual taxable amount.

Affected

This problem appears to only affect line items that have per-line item adjusments (feature introduced in Spree 2.2). As far as I can tell, other kinds of adjustments (whole order adjustments and shipping adjustments) are not affected by this problem.

Suggested Fix

I can see two possible remedies for this:

1) pass the before-discount line item price to avalara in the transactions. in avalara_transaction.rb#L295 change line_item.pre_tax_amount.to_f to (line_item.quantity * line_item.price). If the origional design intentaiton was to pass the full line item price, before discounts, this is probably the solution that will bring the implmentation correclty in line with the original design.

2) As an alternative fix continue to pass the discounted line items in the Line item transactions, but change the Discount that is passed to only include order-wide adjustments and not line item adjustments. For this fix, avalara_transaction.rb#L419 changes from order_details.all_adjustments to order.adjustments

(however, I'm not sure if this may remove the shipping adjustments which are needed? If so, maybe (order.adjustments + order.shipment_adjustments)

[jasonfb]

I think the root of the fix has to do with GAAP and how Avatax is designed to work -- should the line items be submitted before-discounted price, or is it OK to submit them after discounts have been applied.

I spoke to Avalara support (Cratha) just now and they informed me that either solution is acceptable in terms of the Avatax software.

For us at Mack Weldon, I'm actually more inclined to go with solution #2 (above), because I don't really want order level discounts applied proportionally in Avalara. I can definitely come up with use cases for which this does work correctly (for example, let's say 1 line item is discounted 10% but another one is discounted $5. Proportional application on Avalara's side won't work)

So my only final question has to do with whether or not the spree shipping adjustments should be passed as discounts at all. I'm inclined to say no, since these are actually charges and not discounts.

I would propose changing line 419 to :

        :Discount => order_details.adjustments.where(source_type: "Spree::PromotionAction").any? ? order_details.adjustments.where(source_type: "Spree::PromotionAction").pluck(:amount).reduce(&:+).to_f.abs : 0,

this way, only order-level adjustments will be passed to Avalara.

Adjusted line items will continue to have their reduced amounts passed to Avalara, as currently implemented.

So essentially, from Avalara's perspective, Spree line-item level adjustments will be as if they don't exist.

[acreilly]

Thank you for looking into this. I had put some time in it today, but needed to hear back from Avalara on a few things. I will put this through and test it. Should be completed by tomorrow.

[acreilly]

By changing https://github.com/railsdog/spree_avatax_certified/blob/2-3-stable/app/models/spree/avalara_transaction.rb#L295 to line[:Amount] = line_item.price.to_f Fixes the issue, just keep in mind when dealing with returns, it will suggest refunding the full amount and not the actual amount paid for.

[jasonfb]

@acreilly I see you seem to be suggesting that we go with option #1 (as I outlined above).

I think perhaps you meant that to be (line_item.price * line_item.quantity).to_f

In Spree's domain model, line_item.price is the price for 1 single item.

the Spree Avatax documentation, it clearly says that the "Amount" field on the line item should be the qty * unit price. I have highlighted the relevant section from the Avatax docs below:

So, essentially, you're saying that the items should be passed with the full prices to Avatax, and then keep the all_adjustments as the sum of the amounts passed on the Document level in the "Discount" field?

Why are you also passing shipping adjustments as well to Avatax?

[acreilly]

Can you highlight what you are talking about with the shipping adjustments?

On Sat, Jun 13, 2015 at 9:52 AM, Jason Fleetwood-Boldt < notifications@github.com> wrote:

@acreilly https://github.com/acreilly I see you seem to be suggesting that we go with option #1 https://github.com/railsdog/spree_avatax_certified/pull/1 (as I outlined above).

I think perhaps you meant that to be ``` (line_item.price * line_item.quantity).to_f ***,

In Spree's domain model, line_item.price is the price for 1 single item.

the Spree Avatax documentation, it clearly says that the "Amount" field on the line item should be the qty * unit price. I have highlighted the relevant section from the Avatax docs below:

[image: 2015-06-13_0947] https://cloud.githubusercontent.com/assets/59002/8144542/8d67ca96-11b1-11e5-80d0-947f4faa3149.png

So, essentially, you're saying that the items should be passed with the full prices to Avatax, and then keep the all_adjustments as the sum of the amounts passed on the Document level in the "Discount" field?

Why are you also passing shipping adjustments as well to Avatax?

— Reply to this email directly or view it on GitHub https://github.com/railsdog/spree_avatax_certified/issues/37#issuecomment-111711464 .

Allison Reilly

[jasonfb]

I was just referring to order_details.all_adjustments -- this include order, lineitem, and shipping adjustments by default. I guess that is by design then?

I think I understand why it is implemented this way, so if the intention is to include all those adjustments in the document-level "Discount" field then that makes sense to me.

[jasonfb]

regarding (line_item.price * line_item.quantity).to_f

it looks like you are already a step ahead of me...

https://github.com/railsdog/spree_avatax_certified/commit/f50c6ecdadb661f02a27025ff6ea7709049923c0

[dhonig]

@acreilly re-opening. We need to make sure we have a few eyes on this and have some good code review before closing.

[acreilly]

No problem, I have made the fix. Unfortunately, when I began working on this gem, Avalara's documentation was not nearly as good as it is now. @jasonfb Would you like to update the gem on your side and test if it is working for you?

[jasonfb]

Yes I'll have to go over this with Charley (our head of Finance) on Monday and get back to you.

[acreilly]

Was the issue fixed for you?