VAT

[MatteoPiovanelli-Laser]

Based on Territories, I'll move on to building a VAT feature.

The idea is that different product categories have different VAT rates depending on where the goods are and where they need to be shipped.

This feature will allow the BO user to define a "table" like:

...	Region 1	Region 2	...	Region N
Category 1	value 1,1	value 1,2	...	value 1,N
Category 2	value 2,1	value 2,2	...	value 2,N
...	...	...	...	...
Category M	value M,1	value M,2	...	value M,N

Then, in each product there would be a Part or Field (I am thinking Part) to select the category.

The regions up there are the first level territories of a specific hierarchy (selected in a feature setting). At the time of computing taxes, those regions are compared with the territory selected for shipping to find the correct rate to be applied for the product. The setting would also need a "default" region, that allows showing prices with applied VAT on frontend (this is a requirement in the EU).

The simplest case for VAT in the EU is an ecommerce selling a single product category, with a small sales volume (there is a specific limit to this, depending on the country of origin of the goods). The table there would be:

...	EU	Not EU
Category	Rate	0.0

Every other case I am aware of can be managed by having the required hierarchy and the rows for the categories.

[bleroy]

This is great. I'm just hoping that this can be built on top of the tax extensibility I put in place. If not, I'd like us to think about how to fix the foundations rather than build a new thing next to it.

[MatteoPiovanelli-Laser]

I think it can work with what's in place. I did a bit of analysis a while back and it looked compatible, but of course I should look at it properly again before writing code. A possible issue being that i am not a tax expert, and the cases I know to consider may not be enough

[MatteoPiovanelli-Laser]

I have been going through my notes regarding VAT. Note that I'll talk about VAT, because that what is used in most of the world (inclouding EU): in the USA they have sales tax. There is no difference for what we are doing here, because the only actual differences are in accounting (as long as this system is not being used for specific import-export cases), so for the sake ofdiscussion here let's assume VAT and sales tax are the same and save us some time for now.

This way of handling VAT would supersede the current implementations: both StateOrCountryTaxPartand ZipCodeTaxPart could easily be "implemented" in the way I described above, under the assumption that there is a single product category. Looking at the implementation of ShoppingCartQuantityProduct, I think it would be necessary to define (probably as a setting) a "default category": ShoppingCartQuantityProduct as a reference to the ProductPart, but there is nothing mandating the fact that the product should have the ContentPart we will use to mark its product category. This would play well with "replacing" the two tax parts above.

ProductPart objects have no tax-related properties in them, and this is good. I would possibly add some wording somewhere in the editor to highlight the fact that the ProductPart.Price is before VAT.

In ShoppingCartBase.Taxes(decimal) only the first TaxAmountis returned, being the one from the ITax with the highest priority (and with value greater than 0). The ITax objects are obtained by calling the GetTaxes() method of all ITaxProvider implementations. This makes me think that we only need one ITax object, that has that category/territory/rate table. The ITaxProvider would return that object. Extending things a bit, a very large business may actually need more than one of those objects, assuming they can have products shipped from different locations in the world. Hence, the ITax implementation for this feature would be a ContentPart, so that we may eventually have a bunch of ContentItems that have it. (I am not planning on figuring out right now how to dynamically ahndle priority there, so I'll just have an input with a number).

The other place in the code where GetTaxes() is called is in the TaxAdminController, in order to list the existing ContentItems that have been created for the taxes.

What's missing from the current taxes implementation? We need the ability to compute taxes on single products/items based on category and region. For the "cart total" this is not an issue, but on the frontend, in the EU, you have to show for each product the price after application of VAT. You can do it for a "default" country of your choosing, and then have the customer pay the correct amount at the end (notifying them of the price change). On top of that, it would be useful to show on the editor for a product what is the price you are setting for the final customer. On principle, this feature could be "attached" to its own VATPart on the product ContentItem.

Let me know if this all makes sense so far.

[bleroy]

Where you got me confused is when you talk about an ITax implementation that is also a content part. I could see a tax service using information from a special VATPart, but having a part taking the responsibilities of a service is entirely counter standard Orchard practices.

[MatteoPiovanelli-Laser]

ITax extends IContent. Its current implementations are both parts.

[bleroy]

Oops, I guess I was thinking of ITaxProvider. Not sure what good ITax is then, but I must be misremembering things.

[MatteoPiovanelli-Laser]

As far as I can tell, the point of ITax was to be the interface getting called by the ITaxProviders to do their computations. ITaxProviders are what is called in other places in the code to fetch the ITax objects to be used. I guess since ITax is IContent it's not getting injected, so we use corresponidng ITaxProviders to fetch the information.

[MatteoPiovanelli-Laser]

All in all, my main complaint with the Nwazet.Taxes feature for implementing this territory-based VAT is that StateOrCountryTaxPart and ZipCodeTaxPart and the corresponding providers are all implemented in that very feature, along with the TaxAdminController and TaxAdminMenu, that I would use for this new feature as well.

On the other hand, that controller does not need anyITaxProvider to be implemented, i.e. it will not fail, even though it won't give the option to configure taxes in that case. I would then move those classes to a "Nwazet.BaseTaxImplementations" feature. I would need to figure out a migration path such that this does not mess things up. The new feature containing the implementations I discussed above would then be able to work side by side with everything else, and it would be possible ot not have those base implementations along with it.

[bleroy]

That's a fair complaint, the abstractions should not be in the same feature as the implementation.

[MatteoPiovanelli-Laser]

I am having issues in writing retrocompatible migrations. I post those in the following gist:

https://gist.github.com/MatteoPiovanelli-Laser/2bf66dcf313a8b912492e63956dfb1d2

The idea is:

• Moving the base implementations to their own feature means that the new Nwazet.BaseTaxImplementations needs to have a migration that creates the same table
• If Nwazet.Taxes was initially not enabled, enabling it now moves its migration version number up to 4 directly, so it does not create the tables. Nwazet.BaseTaxImplementations can then be activated without issues.
• If you already have Nwazet.Taxes enabled, its migration has run and created the table for StateOrCountryTaxPartRecord.
• At this point, for retrocompatibility, since you have Nwazet.Taxes active, Nwazet.BaseTaxImplementations needs to be activated, so that you have the same exact functionality. However, it does not have to create any table.
• My idea was to wrap SchemaBuilder.CreateTable in a try...catch block. However, it looks like something else is also failing, because with that code (see the gist above) i just get "Transaction not connected, or was disconnected"

[MatteoPiovanelli-Laser]

hold on, thanks to @Hazzamanic on gitter I was able to fix this. There is still an issue, but it's probably in Orchard, as even though the migrations have run without issues, the Nwazet.BaseTaxImplementations feature appears to not be enabled (I see it disabled in the modules/features page, and I don't see the buttons for its providers in the TaxAdmin page) unless I also restart IIS. It feels as if the list of enabled feature in the SHellDescriptor is not being updated correctly

[MatteoPiovanelli-Laser]

note: work on this feature is happening here: https://github.com/LaserSrl/Nwazet.Commerce/tree/AdvancedVAT

[MatteoPiovanelli-Laser]

The one place where taxes are actually computed, I think, is ShoppingCartBase.Taxes(decimal). I may change that so that rather than calling ITax.ComputeTax(), it calls a service to compute taxes. This way I can have stuff being injected there. The signature would look something like

decimal TaxService<T>.ComputeTax(T ITaxImplementation, TaxContext context)

The TaxContext would contain the parameters that are now passed to ITax.ComputeTax. For the implementations that are already there, this service's method would call ITax.ComputeTax.

Basically, the method from ShoppingCartBasecould become:

public virtual TaxAmount Taxes(decimal subTotal = 0) {
            if (Country == null && ZipCode == null) return null;
            var taxes = _taxProviders
                .SelectMany(p => p.GetTaxes())
                .OrderByDescending(t => t.Priority);
            var shippingPrice = ShippingOption == null ? 0 : ShippingOption.Price;
            if (subTotal.Equals(0)) {
                subTotal = Subtotal();
            }
            var context = new TaxContext(GetProducts(), subTotal, shippingPrice, Country, ZipCode);
            return (
                from tax in taxes
                let name = tax.Name
                let amount = _taxProviderServices.Sum(ComputeTax(tax, context)) // Add all taxes?
                where amount > 0
                select new TaxAmount { Name = name, Amount = amount }
                ).FirstOrDefault() ?? new TaxAmount { Amount = 0, Name = null };
        }

[bleroy]

Yes, that sounds like a good change, consistent with how weird ITax looks, being implemented by parts: the idea of a service that takes context is much more orchardy.

[MatteoPiovanelli-Laser]

disorganized notes I have on the design of this:

Each VATConfigurationParthandles a single product category (with controls to ensure categories are unique):

• This is the part implementing ITax.
• This would mean that from TaxAdminwe will create one tax ContentItem for each product category.
• If in a VATConfigurationPart we set the category as the default one, this overrides the previous default (kind of like what happens when setting the HomePage in an AutoroutePart).
• Handle somehow attempts to delete the default category.
• We need a default category to handle the case where no category has been assigned to a product (maybe we only have one category in the whole store)
• When in the VATConfigurationPart we select the hierarchy to be used for taxes there, the category is "added" to the HierarchyVATPartfor the hierarchy and the TerritoryVATPartin all its territories (this latter part is where we will set the rate).
• This means that HierarchyVATPartand TerritoryVATPartcan each refer to one or several categories, because I feel there should not be a need to redefine hierarchies for each product category we handle.
• There may be a way to add HierarchyVATPartand TerritoryVATPart"at runtime" to those hierarchies and territories that actually need it. May be by abusing the Activatedevent handler for those items.
• VATConfigurationParthas also a default value for the rate: this value is used when computing prices to show at frontend whenever the selected destination territory for the products is not found in the hierarchy: for example, an EU seller would only need to define EU countries in the hierarchy, because that's where he has to apply VAT: sales outside the EU are VAT-free (on principle, excluding specific agreements with foreign countries).
• Given that we have a destination territory, we can apply its rate for a product of a given category. If the territory does not set a rate for that category in its TerritoryVATPart, we look for it in its parent territory, and so on recursively until we either find one, or run out of hierarchy to climb up: in the latter case we use the default rate.

One thing to note, I think, is that this feature is only really for guidance of the people handling the finance of the ecommerce that uses it: it will do things so that when reviewing orders they can see for each product the tax due, but it should not be a replacement for an actual accountant veryfing those values.

Orders are a thing I would like to review at a later date, because they look a bit too rigid to me in the way they work right now in Nwazet.

[bleroy]

So just to be clear, categories in this context really are taxonomies? If so, why isn't the VAT config part added to the relevant taxonomy type then?

For falling back when a product has nothing specified, maybe that's a job for a generic tax provider like the current ones?

[MatteoPiovanelli-Laser]

Categories here would be product fiscal categories. I am not confident that taxonomies as they are should be used for that. I wad thinking to use something specific for this.

[bleroy]

Let's call them something more specific than categories then.

[MatteoPiovanelli-Laser]

The Italian for what I mean is "Categoria merceologica". That is the actual wording from fiscal documents. Google translates that to "product category". I think I'll call the properies representing that to either ProductCategory or FiscalProductCategory or ProductFiscalCategory. Or something related anyway.

[bleroy]

We've used "Tax" in many names already. TaxCategory, or TaxationCategory?

[MatteoPiovanelli-Laser]

Hello,

I am finally able to work on this some more.

[MatteoPiovanelli-Laser]

I've done a bunch of this feature here: https://github.com/LaserSrl/Nwazet.Commerce/tree/AdvancedVAT

Basically, VAT configuration based on territories/hierarchies is in place. It works roughly like this:

• Admin creates a VAT Configuration from the Tax Admin. That is basically the ContentItem (ITax) that represents what we were discussing as TAXCategory. Each fo these items sets a default rate that will be applied for products of this category whose destination is any territory that is not configured differently below.
• In each Hierarchy in the site, Admin can select the TAX Categories, and configure a rate for each one. These rates will be applied to all products matching the category that are sold with destination to one of the territories in the category.
• In each Territory in a Hierarchy, where a tax category has been selected at the hierarchy level, for each of the categories selected in the hierarchy the admin may define a different rate to be applied for that territory and all of its children.

Example: a store selling Wine and Books from Italy. Exports to the UK are large enough that the store is not allowed to use the Italian VAT when exporting there, while it should use Italian VAT for the rest of the EU. No VAT should be applied to exports outside the EU. Vat categories defined:

• Wine, with default rate 0%
• Book, with default rate 0% Hierarchy defined:
• EU (with 28 territories, one for each country in the EU).
  • For Wine the rate is set to 22% for the whole hierarchy
  • For Books the rate is set to 4% for the whole hierarchy
  • UK (Territory): has its rate for books set to 0%. Does not set a custom rate for Wine because it is the same as the one for the rest of the EU (I am not sure this is actually the case for the real world).

From that store:

• Selling a book has an applied VAT of 4% anywhere in the EU except the UK, where it is 0%. Outside the EU the applied VAT is 0%.
• Selling Wine has an applied VAT of 22% everywhere in the EU, including the UK. Otside the EU the applied VAT is 0%.

For each product, there is a new part where we can select the Tax Category. The value "Default" matches the VAT category configured as default.

I'll add a site setting to select the "default" destination country: by this I mean the country that will be used to estimate VAT on products when no destination country is selected yet. This is because in the EU we are required by default to show the final product price at frontend (provided that said price is allowed to change right before finalizing the purchase, in case VAT for the destination country is different that the one used for displaying products on front end). One option available for that site setting will be to show the price before tax, for US ecommerce.

[MatteoPiovanelli-Laser]

Here's something I propose and that I am currently setting myself up for doing:

A, IProductPriceService whose methods are there just to return the price properties of a ProductPart (Price, DiscountPrice, ShippingCost, PriceTiers) The base implementation would simply return the properties from the part.

Advanced implementations would allow to easily get the values "modified" by taxes.

This service would mostly be used when presenting stuff to the frontend, as a shorthand to those properties and to avoid managing the active features in some weird way.

[bleroy]

That all sounds great to me. 👍

[MatteoPiovanelli-Laser]

I just now found an issue: taxes and shipping options are computed for the last time before inserting the shipping address. That may lead to charging wrong amounts

[MatteoPiovanelli-Laser]

I will be adding an interface for providers to add/inject things in the Orders. Right now, rather than changing OrderPart, these will work by either attaching a different part (e.g. AdditionalOrderInfoPart) to Orders at runtime, or simply by handling their own IRepository<AdditionalOrderInfoRecord>. There will be a need to show this info in the views for the OrderPart, and that can work in either case through drivers.

This stuff will work in OrderService, injecting there an IEnumerable, and invoking the creation methods from those in the call to CreateOrder.

The specific case I have in mind would be storing all VAT information from an order (rate for each product, at the very least), and then displaying that information in the table for the products in the editor view, in an additional column. I can achieve this effect in the view by adding the column in js, but I'd rather have a more structured solution: one method of the new provider interface would basically return columns to add in the tbody of the products' table. I would call this method in the existing OrderPartDriver and have the information about the additional columns added to the OrderEditorViewModel.

[bleroy]

Yeah, adding a column in JS won't do it, I agree with your more structured solution. The view should not have to worry about such details, and should get what it needs from the view model.

[MatteoPiovanelli-Laser]

I think what I have works. I will try to play with it some more locally to find more issues, but hopefully I'll be able to file a PR early next week.