[request for comments] tax exempt users

[AdnanTheExcellent]

I am planning on adding tax exempt users to this gem and I want to get people's opinions on implementation before I continue. The goal would be to create a TaxExemption model which contains the tax exempt information about a user. Taxjar api calls would be added to create, update, and delete a customer in taxjar based off of the user and TaxExemption object.

model example:

module Spree
  class TaxExemption
    belongs_to :user, class_name: Spree.user_class
    belongs_to :address, class_name: 'Spree::Address'
    belongs_to :tax_exemption_type, class_name: 'Spree::TaxExemptionType'
    has_many :tax_exemption_states, class_name: 'Spree::TaxExemptionState'
    has_many :states, though: :tax_exemption_states, class_name: 'Spree::State'

    accepts_nested_attributes_for :tax_exemption_states
  end
end

module Spree
  class TaxExemptionState
    belongs_to :state, class_name: 'Spree::State'
    belongs_to :tax_exemption, class_name: 'Spree::TaxExemption'
    has_one_attached :tax_exemption_document, optional: true
  end
end

module Spree
  class TaxExemptionType
    has_many :tax_exemptions, class_name: 'Spree::TaxExemption' #%w[wholesale government marketplace other]
  end
end

taxjar API call examples in SuperGood::SolidusTaxJar::API

      def create_customer_for(user)
        taxjar_client.create_customer APIParams.customer_params(user)
      end

      def update_customer_for(user)
        taxjar_client.update_customer APIParams.customer_params(user)
      end

      def delete_customer_for(user)
        taxjar_client.delete_customer user.id
      end

Optionally, if we want to make a solidus backend UI for CRUDing tax exemptions, we could add a tab to the admin users section called "tax Exemptions" which has the tax exemption form. the states dropdown can be populated by calling taxjar_client.nexus_regions to know the eligible states for tax exemption.

let me know what you think

[jarednorman]

In no particular order, some questions and comments:

1. What's the goal of storing this info in the database? I'm always wary of storing data that duplicates data elsewhere in the database.
2. Doesn't the TaxExemption model map to a TaxJar customer? It's pretty easy to imagine another extension using Spree::TaxExemption and causing a collision and I prefer naming schemes that reflect better what they are representing, unless you're trying to hide the underlying details. Here it just seems like we're caching things, more or less.
3. Ideally I'd prefer if any models this extension provides are all nested under the same namespace as the rest of the code. I'm generally against adding new constants to namespaces you don't "own". I feel like SuperGood::SolidusTaxJar::Customer might be a long name, but at least it's consistent.
4. Couldn't the TaxExemptionType model be an enum on the TaxExemption, since we know the possible values?
5. Ah, I guess the document is why you're storing it in the database?

[AdnanTheExcellent]

In no particular order, some questions and comments:

1. What's the goal of storing this info in the database? I'm always wary of storing data that duplicates data elsewhere in the database.

2. Doesn't the `TaxExemption` model map to a TaxJar customer? It's pretty easy to imagine another extension using `Spree::TaxExemption` and causing a collision and I prefer naming schemes that reflect better what they are representing, unless you're trying to hide the underlying details. Here it just seems like we're caching things, more or less.

3. Ideally I'd prefer if any models this extension provides are all nested under the same namespace as the rest of the code. I'm generally against adding new constants to namespaces you don't "own". I feel like `SuperGood::SolidusTaxJar::Customer` might be a long name, but at least it's consistent.

4. Couldn't the `TaxExemptionType` model be an enum on the `TaxExemption`, since we know the possible values?

5. Ah, I guess the document is why you're storing it in the database?

1. A couple reasons storing it in the DB could be useful. First, this allows for flows like: In customer account settings, a customer can add tax exemption info and upload their tax exempt document(s) to create a TaxExemption. Someone at Company can then approve / reject TaxExemption. On approval, the taxjar api can send a create customer api call. This saves a lot of time when you're a B2B company and customer service has to deal with a lot of calls / emails manually handling tax exempt users (like Printivity does now). Second, if Taxjar goes down, goes out of business, or whatever, you have all the tax exempt info you need to make your own decisions. Printivity plans on implementing a failsafe to use the default tax calculator with our current solidus tax rate setup if taxjar goes down (like it did a couple days ago)
2. It pretty much maps to a TaxJar Customer. If you want to rename these to have a taxjar namespace, i am ok with that.
3. As mentioned in 2, i'm ok renaming these models.
4. Yes. i originally had it as an enum but made it its own model to follow suite with stuff like ReturnReasons in Solidus. Either way is fine with me.
5. Yes. Sometimes customer service needs to access these documents too.

[jarednorman]

Cool, thanks for the quick response.

1. That makes sense, our previous work around exemptions didn't require this, so I wanted to make sure there was some value there and there obviously is. ✅
2. Cool, let's do that then.
3. ⬆️
4. I'd prefer the enum as it's pretty easy to expand it into a model if that ends up being useful. Having it as a model would lead me to believe you can have arbitrary ones, like with return reasons, so I'm wary to do that.
5. Cool, I like it. This maps pretty well to how we've seen other people work with exemptions.

I'm happy with this approach overall.

[Noah-Silvera]

This was tackled at one point in #32 , but hasn't been revisited. It's optional work for the TaxJar certification, so going to table expanding this RFC out into tickets for now.