Merge pull request #13086 from rioug/fix-tax-rate-refund

Remove tax rate refund code path

app/models/spree/tax_rate.rb

@@ -105,16 +105,7 @@ module Spree
         if default_zone_or_zone_match?(item.order)
           calculator.compute(item)
         else
-          # Tax refund should not be possible with the way our production server are configured
-          Alert.raise(
-            "Notice: Tax refund should not be possible, please check the default zone and " \
-            "the tax rate zone configuration"
-          ) do |payload|
-            payload.add_metadata :order_tax_zone, item.order.tax_zone
-            payload.add_metadata :tax_rate_zone, zone
-            payload.add_metadata :default_zone, Zone.default_tax
-          end
-          # In this case, it's a refund.
+          # In this case, it's a refund (for instance offering a manual discount via an adjustment)
           calculator.compute(item) * - 1
         end
       else

app/models/spree/zone.rb

@@ -7,6 +7,8 @@ module Spree
     has_and_belongs_to_many :shipping_methods, join_table: 'spree_shipping_methods_zones'
 
     validates :name, presence: true, uniqueness: true
+    validates :zone_members, presence: true
+
     after_save :remove_defunct_members
     after_save :remove_previous_default
 

app/views/spree/admin/zones/index.html.haml

@@ -22,7 +22,7 @@
       %tr
         %th= sort_link [:spree, @search], :name, t("spree.name"), title: 'zones_order_by_name_title'
         %th
-          = sort_link [:spree, @search], :description, t("spree.description"), {}, {title: 'zones_order_by_description_title'}
+          = sort_link [:spree, @search], :description, t("spree.description"), {title: 'zones_order_by_description_title'}
         %th= t("spree.default_tax")
         %th.actions
     %tbody

app/views/spree/admin/zones/new.html.haml

@@ -11,5 +11,7 @@
 
 = form_for [:admin, @zone] do |zone_form|
   = render partial: 'form', locals: { zone_form: zone_form }
+  = render partial: 'member_type', locals: { type: 'country', zone_form: zone_form }
+  = render partial: 'member_type', locals: { type: 'state', zone_form: zone_form }
   .clear
   = render partial: 'spree/admin/shared/new_resource_links'

db/default/zones.rb

@@ -1,22 +1,25 @@
 # frozen_string_literal: true
 
 unless Spree::Zone.find_by(name: "EU_VAT")
-  eu_vat = Spree::Zone.create!(name: "EU_VAT",
-                               description: "Countries that make up the EU VAT zone.")
+  eu_vat = Spree::Zone.new(
+    name: "EU_VAT", description: "Countries that make up the EU VAT zone."
+  )
 
   ["Poland", "Finland", "Portugal", "Romania", "Germany", "France",
    "Slovakia", "Hungary", "Slovenia", "Ireland", "Austria", "Spain",
    "Italy", "Belgium", "Sweden", "Latvia", "Bulgaria", "United Kingdom",
    "Lithuania", "Cyprus", "Luxembourg", "Malta", "Denmark", "Netherlands",
    "Estonia"].each do |name|
-    eu_vat.zone_members.create!(zoneable: Spree::Country.find_by!(name:))
+    eu_vat.zone_members.new(zoneable: Spree::Country.find_by!(name:))
   end
+  eu_vat.save!
 end
 
 unless Spree::Zone.find_by(name: "North America")
-  north_america = Spree::Zone.create!(name: "North America", description: "USA + Canada")
+  north_america = Spree::Zone.new(name: "North America", description: "USA + Canada")
 
   ["United States", "Canada"].each do |name|
-    north_america.zone_members.create!(zoneable: Spree::Country.find_by!(name:))
+    north_america.zone_members.new(zoneable: Spree::Country.find_by!(name:))
   end
+  north_america.save!
 end

lib/tasks/sample_data/addressing.rb

@@ -16,8 +16,10 @@ module Addressing
   end
 
   def zone
-    zone = Spree::Zone.find_or_create_by!(name: ENV.fetch('CHECKOUT_ZONE'))
-    zone.members.create!(zoneable: country) unless zone.zoneables.include?(country)
+    zone = Spree::Zone.find_or_create_by(name: ENV.fetch('CHECKOUT_ZONE'))
+    zone.members << Spree::ZoneMember.create(zoneable: country) unless
+      zone.zoneables.include?(country)
+    zone.save!
     zone
   end
 

spec/factories/zone_factory.rb

@@ -1,21 +1,18 @@
 # frozen_string_literal: true
 
 FactoryBot.define do
-  factory :zone, class: Spree::Zone do
+  factory :zone, aliases: [:zone_with_member], class: Spree::Zone do
     sequence(:name) { |n| "#{generate(:random_string)}#{n}" }
     description { generate(:random_string) }
-  end
+    default_tax { true }
+    zone_members { [Spree::ZoneMember.new(zoneable: member)] }
 
-  factory :zone_with_member, parent: :zone do
     transient do
       member { Spree::Country.find_by(name: "Australia") }
     end
-
-    default_tax { true }
-    zone_members { [Spree::ZoneMember.new(zoneable: member)] }
   end
 
-  factory :zone_with_state_member, parent: :zone_with_member do
+  factory :zone_with_state_member, parent: :zone do
     member { Spree::State.find_by(name: "Victoria") }
   end
 end

spec/helpers/spree/base_helper_spec.rb

@@ -26,9 +26,8 @@ RSpec.describe Spree::BaseHelper do
     context "with a checkout zone defined" do
       context "checkout zone is of type country" do
         before do
-          @country_zone = create(:zone, name: "CountryZone")
-          @country_zone.members.create(zoneable: country)
-          allow(ENV).to receive(:fetch).and_return(@country_zone.name)
+          country_zone = create(:zone, name: "CountryZone", member: country)
+          allow(ENV).to receive(:fetch).and_return(country_zone.name)          
         end
 
         it "return only the countries defined by the checkout zone" do

spec/models/invoice/data_presenter_spec.rb

@@ -61,9 +61,10 @@ RSpec.describe Invoice::DataPresenter do
         expect(presenter.display_line_item_tax_rate(taxable_line_item)).to eq "15.0%, 20.0%"
       end
 
-      context "one of the tax rate is appliable to a different tax zone" do
+      context "one of the tax rate is applicable to a different tax zone" do
         before do
-          order.line_items.last.tax_category.tax_rates.last.update!(zone: create(:zone))
+          new_zone = create(:zone, default_tax: false, member: Spree::Country.last)
+          order.line_items.last.tax_category.tax_rates.last.update!(zone: new_zone)
           order.create_tax_charge!
           Orders::GenerateInvoiceService.new(order).generate_or_update_latest_invoice
         end

spec/models/spree/tax_rate_spec.rb

@@ -4,7 +4,7 @@ require 'spec_helper'
 
 module Spree
   RSpec.describe TaxRate do
-    describe "#match" do
+    describe ".match" do
       let!(:zone) { create(:zone_with_member) }
       let!(:order) { create(:order, distributor: hub, bill_address: create(:address)) }
       let!(:tax_rate) {
@@ -44,7 +44,7 @@ module Spree
         let(:tax_category) { create(:tax_category) }
         let(:calculator) { ::Calculator::FlatRate.new }
 
-        it "should return an empty array when tax_zone is nil" do
+        it "returns an empty array when tax_zone is nil" do
           allow(order).to receive(:tax_zone) { nil }
           expect(Spree::TaxRate.match(order)).to eq []
         end
@@ -55,79 +55,70 @@ module Spree
           end
 
           context "when there is no default tax zone" do
-            before do
-              @zone = create(:zone, name: "Country Zone", default_tax: false, zone_members: [])
-              @zone.zone_members.create(zoneable: country)
-            end
+            let(:zone) { create( :zone, name: "Country Zone", default_tax: false, member: country) }
 
-            it "should return an empty array" do
-              allow(order).to receive(:tax_zone) { @zone }
+            it "returns an empty array" do
+              allow(order).to receive(:tax_zone).and_return(zone)
               expect(Spree::TaxRate.match(order)).to eq []
             end
 
-            it "should return the rate that matches the rate zone" do
+            it "returns the rate that matches the rate zone" do
               rate = Spree::TaxRate.create(
                 amount: 1,
-                zone: @zone,
+                zone:,
                 tax_category:,
                 calculator:
               )
 
-              allow(order).to receive(:tax_zone) { @zone }
+              allow(order).to receive(:tax_zone).and_return(zone)
 
               expect(Spree::TaxRate.match(order)).to eq [rate]
             end
 
-            it "should return all rates that match the rate zone" do
+            it "returns all rates that match the rate zone" do
               rate1 = Spree::TaxRate.create(
                 amount: 1,
-                zone: @zone,
+                zone:,
                 tax_category:,
                 calculator:
               )
 
               rate2 = Spree::TaxRate.create(
                 amount: 2,
-                zone: @zone,
+                zone:,
                 tax_category:,
                 calculator: ::Calculator::FlatRate.new
               )
 
-              allow(order).to receive(:tax_zone) { @zone }
+              allow(order).to receive(:tax_zone).and_return(zone)
 
               expect(Spree::TaxRate.match(order)).to eq [rate1, rate2]
             end
 
             context "when the tax_zone is contained within a rate zone" do
-              before do
-                sub_zone = create(:zone, name: "State Zone", zone_members: [])
-                sub_zone.zone_members.create(zoneable: create(:state, country:))
-                allow(order).to receive(:tax_zone) { sub_zone }
+              let(:sub_zone) { create(:zone, name: "State Zone", member: create(:state, country:)) }
 
-                @rate = Spree::TaxRate.create(
+              it "returns the rate zone" do
+                allow(order).to receive(:tax_zone).and_return(sub_zone)
+
+                rate = Spree::TaxRate.create(
                   amount: 1,
-                  zone: @zone,
+                  zone:,
                   tax_category:,
                   calculator:
                 )
-              end
 
-              it "should return the rate zone" do
-                expect(Spree::TaxRate.match(order)).to eq [@rate]
+                expect(Spree::TaxRate.match(order)).to eq [rate]
               end
             end
           end
 
           context "when there is a default tax zone" do
-            before do
-              @zone = create(:zone, name: "Country Zone", default_tax: true, zone_members: [])
-              @zone.zone_members.create(zoneable: country)
-            end
-
+            let(:zone) { create(:zone, name: "Country Zone", default_tax: true, member: country) }
             let(:included_in_price) { false }
             let!(:rate) do
               Spree::TaxRate.create(amount: 1,
-                                    zone: @zone,
+                                    zone:,
                                     tax_category:,
                                     calculator:,
                                     included_in_price:)
@@ -137,7 +128,7 @@ module Spree
 
             context "when the order has the same tax zone" do
               before do
-                allow(order).to receive(:tax_zone) { @zone }
+                allow(order).to receive(:tax_zone) { zone }
                 allow(order).to receive(:billing_address) { tax_address }
               end
 
@@ -149,12 +140,13 @@ module Spree
 
               context "when the tax is a VAT" do
                 let(:included_in_price) { true }
+
                 it { is_expected.to eq [rate] }
               end
             end
 
             context "when the order has a different tax zone" do
-              let(:other_zone) { create(:zone, name: "Other Zone") }
+              let(:other_zone) { create(:zone, name: "Other Zone", default_tax: false) }
 
               before do
                 allow(order).to receive(:tax_zone) { other_zone }
@@ -200,47 +192,7 @@ module Spree
         end
       end
 
-      context "adjust" do
-        let(:order) { create(:order) }
-        let(:tax_category_1) { build_stubbed(:tax_category) }
-        let(:tax_category_2) { build_stubbed(:tax_category) }
-        let(:rate_1) { build_stubbed(:tax_rate, tax_category: tax_category_1) }
-        let(:rate_2) { build_stubbed(:tax_rate, tax_category: tax_category_2) }
-        let(:line_items) { [build_stubbed(:line_item)] }
-
-        context "with line items" do
-          let(:line_item) { build_stubbed(:line_item, tax_category: tax_category_1) }
-          let(:line_items) { [line_item] }
-
-          before do
-            allow(Spree::TaxRate).to receive(:match) { [rate_1, rate_2] }
-          end
-
-          it "should apply adjustments for two tax rates to the order" do
-            expect(rate_1).to receive(:adjust)
-            expect(rate_2).not_to receive(:adjust)
-            Spree::TaxRate.adjust(order, line_items)
-          end
-        end
-
-        context "with shipments" do
-          let(:shipment) { build_stubbed(:shipment, order:) }
-          let(:shipments) { [shipment] }
-
-          before do
-            allow(shipment).to receive(:tax_category) { tax_category_1 }
-            allow(Spree::TaxRate).to receive(:match) { [rate_1, rate_2] }
-          end
-
-          it "should apply adjustments for two tax rates to the order" do
-            expect(rate_1).to receive(:adjust)
-            expect(rate_2).not_to receive(:adjust)
-            Spree::TaxRate.adjust(order, shipments)
-          end
-        end
-      end
-
-      context "default" do
+      describe ".default" do
         let(:tax_category) { create(:tax_category) }
         let(:country) { create(:country) }
         let(:calculator) { ::Calculator::FlatRate.new }
@@ -248,7 +200,7 @@ module Spree
         context "when there is no default tax_category" do
           before { tax_category.is_default = false }
 
-          it "should return 0" do
+          it "returns 0" do
             expect(Spree::TaxRate.default).to eq 0
           end
         end
@@ -259,30 +211,29 @@ module Spree
           context "when the default category has tax rates in the default tax zone" do
             before(:each) do
               allow(DefaultCountry).to receive(:id) { country.id }
-              @zone = create(:zone, name: "Country Zone", default_tax: true)
-              @zone.zone_members.create(zoneable: country)
+              zone = create(:zone, name: "Country Zone", default_tax: true, member: country)
               rate = Spree::TaxRate.create(
                 amount: 1,
-                zone: @zone,
+                zone:,
                 tax_category:,
                 calculator:
               )
             end
 
-            it "should return the correct tax_rate" do
+            it "returns the correct tax_rate" do
               expect(Spree::TaxRate.default.to_f).to eq 1.0
             end
           end
 
           context "when the default category has no tax rates in the default tax zone" do
-            it "should return 0" do
+            it "returns 0" do
               expect(Spree::TaxRate.default).to eq 0
             end
           end
         end
       end
 
-      describe "#adjust" do
+      describe ".adjust" do
         let!(:country) { create(:country, name: "Default Country") }
         let!(:state) { create(:state, name: "Default State", country:) }
         let!(:zone) { create(:zone_with_member, default_tax: true, member: country ) }
@@ -312,12 +263,12 @@ module Spree
         context "not taxable line item " do
           let!(:line_item) { order.contents.add(nontaxable, 1) }
 
-          it "should not create a tax adjustment" do
+          it "does not create a tax adjustment" do
             Spree::TaxRate.adjust(order, order.line_items)
             expect(line_item.adjustments.tax.charge.count).to eq 0
           end
 
-          it "should not create a refund" do
+          it "does not create a refund" do
             Spree::TaxRate.adjust(order, order.line_items)
             expect(line_item.adjustments.credit.count).to eq 0
           end
@@ -331,14 +282,26 @@ module Spree
             rate2.update_column(:included_in_price, true)
           end
 
+          it "applies adjustments for the matching tax rates to the order" do
+            line_item = build_stubbed(:line_item, tax_category: category)
+            rate3 = create(:tax_rate, amount: 0.05, zone:)
+
+            allow(Spree::TaxRate).to receive(:match) { [rate1, rate3] }
+
+            expect(rate1).to receive(:adjust)
+            expect(rate3).not_to receive(:adjust)
+
+            Spree::TaxRate.adjust(order, [line_item])
+          end
+
           context "when price includes tax" do
             context "when zone is contained by default tax zone" do
-              it "should create two adjustments, one for each tax rate" do
+              it "creates two adjustments, one for each tax rate" do
                 Spree::TaxRate.adjust(order, order.line_items)
                 expect(line_item.adjustments.count).to eq 2
               end
 
-              it "should not create a tax refund" do
+              it "does not create a tax refund" do
                 Spree::TaxRate.adjust(order, order.line_items)
                 expect(line_item.adjustments.credit.count).to eq 0
               end
@@ -347,16 +310,17 @@ module Spree
             context "when order's zone is neither the default zone, or included " \
                     "in the default zone, but matches the rate's zone" do
               before do
-                # With no zone members, this zone will not contain anything
-                zone.zone_members.delete_all
+                # Create a new default zone, so the order's zone won't match this new one
+                create(:zone_with_member, default_tax: true)
               end
 
-              it "should create an adjustment" do
+              it "creates an adjustment" do
                 Spree::TaxRate.adjust(order, order.line_items)
+
                 expect(line_item.adjustments.charge.count).to eq 2
               end
 
-              it "should not create a tax refund for each tax rate" do
+              it "does not create a tax refund for each tax rate" do
                 Spree::TaxRate.adjust(order, order.line_items)
                 expect(line_item.adjustments.credit.count).to eq 0
               end
@@ -377,25 +341,15 @@ module Spree
                 order.all_adjustments.delete_all
               end
 
-              it "should not create positive adjustments" do
+              it "does not create positive adjustments" do
                 Spree::TaxRate.adjust(order, order.line_items)
                 expect(line_item.adjustments.charge.count).to eq 0
               end
 
-              it "should create a tax refund for each tax rate" do
+              it "creates a tax refund for each tax rate" do
                 Spree::TaxRate.adjust(order, order.line_items)
                 expect(line_item.adjustments.credit.count).to eq 2
               end
-
-              it "notifies bugsnag" do
-                # there are two tax rate
-                expect(Bugsnag).to receive(:notify).with(
-                  "Notice: Tax refund should not be possible, please check the default zone and " \
-                  "the tax rate zone configuration"
-                ).twice.and_call_original
-
-                Spree::TaxRate.adjust(order, order.line_items)
-              end
             end
           end
 
@@ -409,21 +363,21 @@ module Spree
               Spree::TaxRate.adjust(order, order.line_items)
             end
 
-            it "should not delete adjustments for complete order when taxrate is deleted" do
+            it "does not delete adjustments for complete order when taxrate is deleted" do
               rate1.destroy!
               rate2.destroy!
               expect(line_item.adjustments.count).to eq 2
             end
 
-            it "should create adjustments" do
+            it "creates adjustments" do
               expect(line_item.adjustments.count).to eq 2
             end
 
-            it "should not create a tax refund" do
+            it "does not create a tax refund" do
               expect(line_item.adjustments.credit.count).to eq 0
             end
 
-            it "should remove adjustments when tax_zone is removed" do
+            it "removes adjustments when tax_zone is removed" do
               expect(line_item.adjustments.count).to eq 2
               allow(order).to receive(:tax_zone) { nil }
               Spree::TaxRate.adjust(order, order.line_items)
@@ -431,6 +385,22 @@ module Spree
             end
           end
         end
+
+        context "with shipments" do
+          let(:shipment) { build_stubbed(:shipment, order:) }
+
+          it "applies adjustments for two tax rates to the order" do
+            rate3 = create(:tax_rate, amount: 0.05, zone:)
+
+            allow(shipment).to receive(:tax_category) { category }
+            allow(Spree::TaxRate).to receive(:match) { [rate1, rate3] }
+
+            expect(rate1).to receive(:adjust)
+            expect(rate3).not_to receive(:adjust)
+
+            Spree::TaxRate.adjust(order, [shipment])
+          end
+        end
       end
     end
   end

spec/models/spree/zone_spec.rb

@@ -3,7 +3,13 @@
 require 'spec_helper'
 
 RSpec.describe Spree::Zone do
-  context "#match" do
+  describe "validations" do
+    it { is_expected.to validate_presence_of(:name) }
+    it { is_expected.to validate_uniqueness_of(:name) }
+    it { is_expected.to validate_presence_of(:zone_members) }
+  end
+
+  describe "#match" do
     let(:country_zone) { create(:zone, name: 'CountryZone') }
     let(:country) do
       country = create(:country)
@@ -63,14 +69,12 @@ RSpec.describe Spree::Zone do
     end
   end
 
-  context "#countries" do
+  describe "#countries" do
     let(:state) { create(:state) }
     let(:country) { state.country }
 
     context "when zone consists of countries" do
-      let(:country_zone) { create(:zone, name: 'CountryZone') }
-
-      before { country_zone.members.create(zoneable: country) }
+      let(:country_zone) { create(:zone, name: 'CountryZone', member: country) }
 
       it 'should return a list of countries' do
         expect(country_zone.countries).to eq [country]
@@ -78,17 +82,15 @@ RSpec.describe Spree::Zone do
     end
 
     context "when zone consists of states" do
-      let(:state_zone) { create(:zone, name: 'StateZone') }
-
-      before { state_zone.members.create(zoneable: state) }
+      let(:state_zone) { create(:zone, name: 'StateZone', member: state) }
 
       it 'should return a list of countries' do
-        expect(state_zone.countries).to eq [state.country]
+        expect(state_zone.countries).to eq [country]
       end
     end
   end
 
-  context "#contains_address?" do
+  describe "#contains_address?" do
     let(:state) { create(:state) }
     let(:country) { state.country }
     let(:address) { create(:address, country:, state:) }
@@ -112,13 +114,13 @@ RSpec.describe Spree::Zone do
     end
   end
 
-  context ".default_tax" do
+  describe ".default_tax" do
     context "when there is a default tax zone specified" do
-      before { @foo_zone = create(:zone, name: 'whatever', default_tax: true) }
+      let!(:default_zone) { create(:zone, name: 'whatever', default_tax: true) }
 
       it "should be the correct zone" do
-        foo_zone = create(:zone, name: 'foo')
-        expect(Spree::Zone.default_tax).to eq @foo_zone
+        create(:zone, name: 'foo', default_tax: false)
+        expect(Spree::Zone.default_tax).to eq default_zone
       end
     end
 
@@ -129,135 +131,122 @@ RSpec.describe Spree::Zone do
     end
   end
 
-  context "#contains?" do
-    let(:country1) { create(:country) }
-    let(:country2) { create(:country) }
-    let(:country3) { create(:country) }
-
-    before do
-      @source = create(:zone, name: 'source', zone_members: [])
-      @target = create(:zone, name: 'target', zone_members: [])
-    end
-
-    context "when the target has no members" do
-      before { @source.members.create(zoneable: country1) }
-
-      it "should be false" do
-        expect(@source.contains?(@target)).to be_falsy
-      end
-    end
-
-    context "when the source has no members" do
-      before { @target.members.create(zoneable: country1) }
-
-      it "should be false" do
-        expect(@source.contains?(@target)).to be_falsy
-      end
-    end
+  describe "#contains?" do
+    let(:country_member1) { Spree::ZoneMember.create(zoneable: create(:country)) }
+    let(:country_member2) { Spree::ZoneMember.create(zoneable: create(:country)) }
+    let(:country_member3) { Spree::ZoneMember.create(zoneable: create(:country)) }
+    let(:source) { create(:zone, name: 'source') }
+    let(:target) { create(:zone, name: 'target') }
 
     context "when both zones are the same zone" do
       before do
-        @source.members.create(zoneable: country1)
-        @target = @source
+        source.zone_members = [country_member1]
       end
 
       it "should be true" do
-        expect(@source.contains?(@target)).to be_truthy
+        target = source
+
+        expect(source.contains?(target)).to be_truthy
       end
     end
 
     context "when both zones are of the same type" do
       before do
-        @source.members.create(zoneable: country1)
-        @source.members.create(zoneable: country2)
+        source.zone_members = [country_member1, country_member2]
       end
 
       context "when all members are included in the zone we check against" do
         before do
-          @target.members.create(zoneable: country1)
-          @target.members.create(zoneable: country2)
+          target.zone_members = [country_member1, country_member2]
         end
 
         it "should be true" do
-          expect(@source.contains?(@target)).to be_truthy
+          expect(source.contains?(target)).to be_truthy
         end
       end
 
       context "when some members are included in the zone we check against" do
         before do
-          @target.members.create(zoneable: country1)
-          @target.members.create(zoneable: country2)
-          @target.members.create(zoneable: create(:country))
+          target.zone_members = [
+            country_member1, country_member2, Spree::ZoneMember.create(zoneable: create(:country))
+          ]
         end
 
         it "should be false" do
-          expect(@source.contains?(@target)).to be_falsy
+          expect(source.contains?(target)).to be_falsy
         end
       end
 
       context "when none of the members are included in the zone we check against" do
         before do
-          @target.members.create(zoneable: create(:country))
-          @target.members.create(zoneable: create(:country))
+          target.zone_members = [
+            Spree::ZoneMember.create(zoneable: create(:country)),
+            Spree::ZoneMember.create(zoneable: create(:country))
+          ]
         end
 
         it "should be false" do
-          expect(@source.contains?(@target)).to be_falsy
+          expect(source.contains?(target)).to be_falsy
         end
       end
     end
 
     context "when checking country against state" do
       before do
-        @source.members.create(zoneable: create(:state))
-        @target.members.create(zoneable: country1)
+        source.zone_members = [Spree::ZoneMember.create(zoneable: create(:state))]
+        target.zone_members = [country_member1]
       end
 
       it "should be false" do
-        expect(@source.contains?(@target)).to be_falsy
+        expect(source.contains?(target)).to be_falsy
       end
     end
 
     context "when checking state against country" do
-      before { @source.members.create(zoneable: country1) }
+      before { source.zone_members = [country_member1] }
 
       context "when all states contained in one of the countries we check against" do
         before do
-          state1 = create(:state, country: country1)
-          @target.members.create(zoneable: state1)
+          state1 = create(:state, country: country_member1.zoneable)
+          target.zone_members = [Spree::ZoneMember.create(zoneable: state1)]
         end
 
         it "should be true" do
-          expect(@source.contains?(@target)).to be_truthy
+          expect(source.contains?(target)).to be_truthy
         end
       end
 
       context "when some states contained in one of the countries we check against" do
         before do
-          state1 = create(:state, country: country1)
-          @target.members.create(zoneable: state1)
-          @target.members.create(zoneable: create(:state, country: country2))
+          state1 = create(:state, country: country_member1.zoneable)
+          state2 = create(:state, country: country_member2.zoneable)
+          target.zone_members = [
+            Spree::ZoneMember.create(zoneable: state1),
+            Spree::ZoneMember.create(zoneable: state2)
+          ]
         end
 
         it "should be false" do
-          expect(@source.contains?(@target)).to be_falsy
+          expect(source.contains?(target)).to be_falsy
         end
       end
 
       context "when none of the states contained in any of the countries we check against" do
         before do
-          @target.members.create(zoneable: create(:state, country: country2))
-          @target.members.create(zoneable: create(:state, country: country2))
+          target.zone_members = [
+            Spree::ZoneMember.create(zoneable: create(:state, country: country_member2.zoneable)),
+            Spree::ZoneMember.create(zoneable: create(:state, country: country_member2.zoneable))
+          ]
         end
 
         it "should be false" do
-          expect(@source.contains?(@target)).to be_falsy
+          expect(source.contains?(target)).to be_falsy
         end
       end
     end
   end
 
-  context "#save" do
+  describe "#save" do
     context "when default_tax is true" do
       it "should clear previous default tax zone" do
         zone1 = create(:zone, name: 'foo', default_tax: true)
@@ -268,37 +257,32 @@ RSpec.describe Spree::Zone do
 
     context "when a zone member country is added to an existing zone consisting of state members" do
       it "should remove existing state members" do
-        zone = create(:zone, name: 'foo', zone_members: [])
         state = create(:state)
+        zone = create(:zone, name: 'foo', member: state)
+
         country = create(:country)
-        zone.members.create(zoneable: state)
         country_member = zone.members.create(zoneable: country)
         zone.save
+
         expect(zone.reload.members).to eq [country_member]
       end
     end
   end
 
-  context "#kind" do
+  describe "#kind" do
     context "when the zone consists of country zone members" do
-      before do
-        @zone = create(:zone, name: 'country', zone_members: [])
-        @zone.members.create(zoneable: create(:country))
-      end
+      let!(:zone) { create(:zone, name: 'country', member: create(:country)) }
 
       it "should return the kind of zone member" do
-        expect(@zone.kind).to eq "country"
+        expect(zone.kind).to eq "country"
       end
     end
 
     context "when the zone consists of state zone members" do
-      before do
-        @zone = create(:zone, name: 'state', zone_members: [])
-        @zone.members.create(zoneable: create(:state))
-      end
+      let!(:zone) { create(:zone, name: 'country', member: create(:state)) }
 
       it "should return the kind of zone member" do
-        expect(@zone.kind).to eq "state"
+        expect(zone.kind).to eq "state"
       end
     end
   end

spec/system/admin/adjustments_spec.rb

@@ -25,7 +25,7 @@ RSpec.describe '
   }
 
   let!(:tax_category_included) { create(:tax_category, name: 'TVA 20%', is_default: true) }
-  let!(:default_tax_zone) { create(:zone, default_tax: true) }
+  let!(:default_tax_zone) { create(:zone, default_tax: true, member: Spree::Country.last) }
   let!(:tax_rate2) {
     create(:tax_rate, name: "TVA 20%", amount: 0.2, zone: default_tax_zone, included_in_price: true,
                       tax_category: tax_category_included, calculator: Calculator::DefaultTax.new )

spec/system/admin/configuration/tax_rates_spec.rb

@@ -6,8 +6,10 @@ RSpec.describe "Tax Rates" do
   include AuthenticationHelper
 
   let!(:calculator) { create(:calculator_per_item, calculable: create(:order)) }
-  let!(:tax_rate) { create(:tax_rate, name: "IVA", calculator:) }
-  let!(:zone) { create(:zone, name: "Ilhas") }
+  let!(:tax_rate) {
+    create(:tax_rate, name: "IVA", calculator:, zone: create(:zone, default_tax: false))
+  }
+  let!(:zone) { create(:zone, name: "Ilhas", default_tax: false) }
   let!(:tax_category) { create(:tax_category, name: "Full") }
 
   before do

spec/system/admin/configuration/zones_spec.rb

@@ -6,6 +6,10 @@ RSpec.describe "Zones" do
   include AuthenticationHelper
   include WebHelper
 
+  before do
+    Spree::Zone.delete_all
+  end
+
   it "list existing zones" do
     login_as_admin
     visit spree.edit_admin_general_settings_path
@@ -34,6 +38,11 @@ RSpec.describe "Zones" do
 
     fill_in "zone_name", with: "japan"
     fill_in "zone_description", with: "japanese time zone"
+    choose "Country Based"
+
+    click_link "Add country"
+    find('.select2').find(:xpath, 'option[2]').select_option
+
     click_button "Create"
 
     expect(page).to have_content("successfully created!")