TaxRate included_in_price has two conflicting meanings

[pedantic-git]

Solidus Version: 2.10.0.beta1

To Reproduce

1. Create a tax rate with included_in_price set to true and non-zero rate.
2. Generate a sale or use the admin to create VAT prices.

Current behavior

• When included_in_price is true, the VAT is added to the gross price so the net price is always the same, but the amount the customer pays varies according to the tax rate in their locale.
• When included_in_price is false, the Spree::Price object is never created by the VatPriceGenerator so the user is charged the same price for every purchase. However, the user is no longer shown the VAT at checkout.

Expected behavior

According to the documentation at https://guides.solidus.io/developers/taxation/value-added-tax.html, these two should be the other way round.

The expected behaviour as far as I can tell from the documentation:

• When included_in_price is true, the VatPriceGenerator does not generate Spree::Price objects for different locales, so that users pay the same gross price (and different net prices per locale). Or, it generates Spree::Price objects but they all have the same gross price.
• When included_in_price is false, the VatPriceGenerator does generate Spree::Price objects for different locales, so that users pay different gross prices (and the same net price) in different locales.
• VAT should be shown to users subjected to a TaxRate at checkout whether included_in_price is true or not.

Desktop (please complete the following information):

• OS: Linux
• Browser: Chrome
• Version: 79

Additional context

I think the main offending line is here - it should be .where(included_in_price: false).

But I appreciate many stores will have got things back-to-front now so changing this is tricky!

Thanks in advance for your help!!

[pedantic-git]

Thinking about this further, I wonder if the simplest solution is to add a second flag to TaxRate called something like keep_gross_price_the_same. If that flag is set to true, the VatPriceGenerator knows to create new locale Spree::Price objects with the same value (but different net amounts). That way it's backwards compatible and gives people the option of whether they want the gross or net price to stay the same between locales. The documentation will need updating too.

Let me know if you think this is a good idea and I can have a go at implementing it myself.

[aldesantis]

I agree that the behavior here is a bit weird. VAT adjustments should be generated only for tax rates which are not included_in_price, which is the opposite of the current behavior.

I'm not sure adding a new field is a good idea here. We'd be maintaining backwards compatibility at the cost of (even) greater complexity.

@kennyadsl any thoughts on this?

[pedantic-git]

@aldesantis When I read the documentation further, I think I understand that some people might want to do VAT this way - where the "price" (the amount the customer sees they have to pay) includes tax (so that's different from US sales tax, i.e. included_in_price == false, where the price is shown without tax) but the "price" does vary from locale to locale.

But I think it's equally likely that people will be like my client and want the price to stay the same, regardless of locale, and for the net to be the value that changes based on the local VAT rate.

So I think perhaps the only viable solution is to move to three options:

1. Price is always shown without tax and tax is added at checkout (current included_in_price == false)
2. Price is always shown with tax. Net price is always the same and gross price changes with locale (current included_in_price == true).
3. Price is always shown with tax and is always the same value. Net price varies with locale to ensure this (proposed keep_gross_price_the_same flag).

What do you think @aldesantis @kennyadsl ?

[kennyadsl]

@pedantic-git Thanks for the detailed issue. I think your solution may work technically even if I'm not sure it is compliant with taxing laws, at least in EU, but I can't call myself an expert to be honest.

This topic reminds me of this @mamhoff presentation at a past SolidusConf: https://www.youtube.com/watch?v=BRnY-BidYRM. Maybe he's still around and could provide some valuable insights here?

[mamhoff]

Solidus' core mechanics calculate VAT correctly, for a given price - that's what the Spree::TaxRate object is for. If I get it right, the use case here is that your client wants the same gross price across different countries, and they're prepared to eat the difference in net income (after they pay their VAT). I think this is very much a business decision that is most easily implemented by overriding the VAT price generator to generate VAT prices that are the same as the one in the admin. E.g.:

# app/services/export_price_generator.rb
class ExportPriceGenerator < Spree::Variant::VatPriceGenerator
  def run
    # Early return if there is no VAT rates in the current store.
    return if !variant.tax_category || variant_vat_rates.empty?
    country_isos_requiring_price.each do |country_iso|
      # Don't re-create the default price
      next if variant.default_price && variant.default_price.country_iso == country_iso
      foreign_price = find_or_initialize_price_by(country_iso, variant.default_price.currency)
      foreign_price.amount = variant.default_price.amount
    end
  end
end

along with

# app/models/spree/variant_decorator
module SpreeVariantDecorator do
  private
  def build_vat_prices
    ExportPriceGenerator.new(self).run
  end
end
Spree::Variant.prepend SpreeVariantDecorator

It might be an interesting PR to make VAT price generation a configurable class so that we don't need to override a private method on Spree::Variant to accomplish this feature.

[pedantic-git]

Hi @mamhoff - thanks so much for this monkeypatch solution!

I agree it's a business decision - I just think it's an incredibly common one. I'm sure my client is not the only customer who would appreciate something being added to core, either via a flag or - as you suggest - by overriding the VatPriceGenerator.

[kennyadsl]

I'm preparing a PR to make the class configurable and I think that for this scenario we could expand the current VAT guides page and explain what to change to support this in the host application. WDYT?

[mamhoff]

I'm preparing a PR to make the class configurable and I think that for this scenario we could expand the current VAT guides page and explain what to change to support this in the host application. WDYT?

Mention me, I'll be happy to proofread.

[pedantic-git]

Awesome. That works for me! Thanks for jumping on this!

[kennyadsl]

Here it is: #3451, let me know if it makes sense.

@pedantic-git can you please confirm that you tried the code above and it works? Thank you!

[pedantic-git]

I can confirm it works apart from a typo in the decorator (extraneous do), but since the decorator is already deprecated by your PR, that's irrelevant!

[kennyadsl]

I merged the PR and opened a new issue to better support this thing in core directly. Thanks for reporting!