Supply a default tax rate

[Noah-Silvera]

What is the goal of this PR?

All tax adjustments need a TaxRate as a source. Previously, we were relying on a user to have a tax rate with the name "Sales Tax" configured. This was an easy to miss configuration step. We should instead create the placeholder Spree::TaxRate desired with a descriptive name.

How do you manually test these changes? (if applicable)

1. Calculate some taxes
2. Visit the tax rates page of the admin
   • Ensure the default rate is shown

Merge Checklist

• Run the manual tests
• Update the changelog

[AlistairNorman]

Are you going to get a bunch of failures because you made the new tax amount 0 and the old one was 0.5?

[benjaminwil]

I think this is great, but I'm curious how you decided on the name "TaxJar Placeholder Rate".

I think this name, too, like the previous name, could be confusing. Someone might decide, "Oh, a placeholder? I should delete this."

Maybe a more explicit name, like "Solidus TaxJar Connection Rate", could be advantageous.

[forkata]

Thanks @Noah-Silvera and @adammathys! I am in favour of this change in general, I am just not sure if we need to change the name of the tax rate.

[benjaminwil]

Chris's comment leads me to think that the tax rate (or tax rate name?) needs to be configurable.

[Noah-Silvera]

Maybe a more explicit name, like "Solidus TaxJar Connection Rate", could be advantageous.

I like this name!

but stores that already use the extension will end up with two active tax rates, the previous Sales Tax rate which a user would have created manually and the new TaxJar Placeholder Rate.

The motivation for changing the name is that currently users will end up with a rate in their system with a default amount of 0.05 and no indication that it is connected to our extension other than a little piece of documentation in the readme. This feels like a pretty brittle dependency. If a user does not understand why that tax rate is there, and changes the name or deletes it, our extension will fail.

This change a.) Ensures a Tax Rate is always available and b.) changes the name of the Tax Rate to relate it to the extension and gives it a default amount of 0 to discourage users from messing with it.

Having two active tax rates doesn't seem like a huge problem to me! Esp. since the sales tax rate doesn't apply to the app at all since this extension completely takes over tax calculation. We could add a step in the upgrading guide for the next release to delete the outdated "Sales Tax" rate after upgrade!

Chris's comment leads me to think that the tax rate (or tax rate name?) needs to be configurable.

I'm not sure this would make sense, if the purpose of this change is to stop people from messing with the tax rate by giving it a clearer name to describe it's purpose!
Someone can configure the label used on the Spree::Tax::ItemTax, which I think is usually something directly configurable on the tax rate!

solidus_taxjar/lib/super_good/solidus_taxjar.rb

Line 68 in dae7838

self.line_item_tax_label_maker = ->(taxjar_line_item, spree_line_item) { "Sales Tax" }

[forkata]

The motivation for changing the name is that currently users will end up with a rate in their system with a default amount of 0.05 and no indication that it is connected to our extension other than a little piece of documentation in the readme.

I am not sure that is the case outside of specs. We currently rely on users setting up the tax rate themselves, so the amount on the rate would be whatever the user creates, right?

I think I am onboard with the change proposed here. I do think we should use a slightly better name, something that non-technical users can recognize and possibly also leave the note in the documentation that the rate must exists, or it will be created by the extension. I like @benjaminwil's proposed name but would drop the "Connection" part of it as that doesn't really add any clarity. Something like "Solidus TaxJar Rate" or just "TaxJar Rate" makes the most sense to me.

[Noah-Silvera]

I am not sure that is the case outside of specs. We currently rely on users setting up the tax rate themselves, so the amount on the rate would be whatever the user creates, right?

We discussed this on slack, but good to make a note of it here for visibility

This extension completely replaces the calculator that calculates tax based on Spree::TaxRates. No matter how many Spree::TaxRates with whatever percentage exist, the taxjar API rates will still be used. Going to make a wiki page describing how this happens in more detail and I'll link it here!

TL;DR - Welcome to TaxJar, the extension where tax rates are made up and the amounts don't matter!

[Noah-Silvera]

I added a wiki page better describing how this extension hooks into tax calculation in solidus!

https://github.com/SuperGoodSoft/solidus_taxjar/wiki/How-Solidus-TaxJar-calculates-tax

[Noah-Silvera]

Updated the name of the default tax rate to Solidus Taxjar Rate

[Noah-Silvera]

Added a link to the wiki page in the readme and updated the changelog

[Noah-Silvera]

Just a rebase on master!

[forkata]

Looks like tests are failing due to this octokit/octokit.rb#1420

[Noah-Silvera]

Just pushed to fix a changelog conflict.