Always require invoice VAT rate

Closes #1031

app/controllers/admin/registrars_controller.rb

@@ -2,6 +2,7 @@ module Admin
   class RegistrarsController < BaseController
     load_and_authorize_resource
     before_action :set_registrar, only: [:show, :edit, :update, :destroy]
+    helper_method :registry_vat_rate
 
     def index
       @q = Registrar.joins(:accounts).ordered.search(params[:q])
@@ -74,5 +75,9 @@ module Admin
                                         :billing_email,
                                         :language)
     end
+
+    def registry_vat_rate
+      Registry.current.vat_rate
+    end
   end
 end

app/models/invoice.rb

@@ -28,16 +28,12 @@ class Invoice < ActiveRecord::Base
 
   validates :due_date, :currency, :seller_name,
             :seller_iban, :buyer_name, :items, presence: true
-  validates :vat_rate, numericality: { greater_than_or_equal_to: 0, less_than: 100 },
-            allow_nil: true
 
   before_create :set_invoice_number
-  before_create :apply_default_vat_rate, unless: :vat_rate?
   before_create :calculate_total, unless: :total?
   before_create :apply_default_buyer_vat_no, unless: :buyer_vat_no?
 
   attribute :vat_rate, ::Type::VATRate.new
-  attr_readonly :vat_rate
 
   def set_invoice_number
     last_no = Invoice.order(number: :desc).where('number IS NOT NULL').limit(1).pluck(:number).first
@@ -85,7 +81,6 @@ class Invoice < ActiveRecord::Base
   end
 
   def vat_amount
-    return 0 unless vat_rate
     subtotal * vat_rate / 100
   end
 
@@ -105,10 +100,6 @@ class Invoice < ActiveRecord::Base
 
   private
 
-  def apply_default_vat_rate
-    self.vat_rate = buyer.effective_vat_rate
-  end
-
   def apply_default_buyer_vat_no
     self.buyer_vat_no = buyer.vat_no
   end

app/models/invoice/vat_rate_calculator.rb

@@ -0,0 +1,19 @@
+class Invoice
+  class VatRateCalculator
+    attr_reader :registry
+    attr_reader :registrar
+
+    def initialize(registry: Registry.current, registrar:)
+      @registry = registry
+      @registrar = registrar
+    end
+
+    def calculate
+      if registrar.vat_liable_locally?(registry)
+        registry.vat_rate
+      else
+        registrar.vat_rate || 0
+      end
+    end
+  end
+end

app/models/registrar.rb

@@ -22,9 +22,9 @@ class Registrar < ActiveRecord::Base
   validates :reference_no, format: Billing::ReferenceNo::REGEXP
   validate :forbid_special_code
 
-  validates :vat_rate, presence: true, if: 'foreign_vat_payer? && vat_no.blank?'
-  validates :vat_rate, absence: true, if: :home_vat_payer?
-  validates :vat_rate, absence: true, if: 'foreign_vat_payer? && vat_no?'
+  validates :vat_rate, presence: true, if: 'vat_liable_in_foreign_country? && vat_no.blank?'
+  validates :vat_rate, absence: true, if: :vat_liable_locally?
+  validates :vat_rate, absence: true, if: 'vat_liable_in_foreign_country? && vat_no?'
   validates :vat_rate, numericality: { greater_than_or_equal_to: 0, less_than: 100 },
             allow_nil: true
 
@@ -52,7 +52,9 @@ class Registrar < ActiveRecord::Base
   end
 
   def issue_prepayment_invoice(amount, description = nil)
-    invoices.create(
+    vat_rate = ::Invoice::VatRateCalculator.new(registrar: self).calculate
+
+    invoices.create!(
       issue_date: Time.zone.today,
       due_date: (Time.zone.now + Setting.days_to_keep_invoices_active.days).to_date,
       description: description,
@@ -84,6 +86,7 @@ class Registrar < ActiveRecord::Base
       buyer_url: website,
       buyer_email: email,
       reference_no: reference_no,
+      vat_rate: vat_rate,
       items_attributes: [
         {
           description: 'prepayment',
@@ -146,12 +149,16 @@ class Registrar < ActiveRecord::Base
     end
   end
 
-  def effective_vat_rate
-    if home_vat_payer?
-      Registry.instance.vat_rate
-    else
-      vat_rate
-    end
+  def vat_country=(country)
+    self.address_country_code = country.alpha2
+  end
+
+  def vat_country
+    country
+  end
+
+  def vat_liable_locally?(registry = Registry.current)
+    vat_country == registry.vat_country
   end
 
   def notify(action)
@@ -169,11 +176,7 @@ class Registrar < ActiveRecord::Base
     errors.add(:code, :forbidden) if code == 'CID'
   end
 
-  def home_vat_payer?
-    country == Registry.instance.legal_address_country
-  end
-
-  def foreign_vat_payer?
-    !home_vat_payer?
+  def vat_liable_in_foreign_country?
+    !vat_liable_locally?
   end
 end

app/models/registry.rb

@@ -1,11 +1,13 @@
 class Registry
-  include Singleton
+  include ActiveModel::Model
 
-  def vat_rate
-    Setting.registry_vat_prc.to_d * 100
-  end
+  attr_accessor :vat_rate
+  attr_accessor :vat_country
 
-  def legal_address_country
-    Country.new(Setting.registry_country_code)
+  def self.current
+    vat_rate = Setting.registry_vat_prc.to_d * 100
+    vat_country = Country.new(Setting.registry_country_code)
+
+    new(vat_rate: vat_rate, vat_country: vat_country)
   end
 end

app/views/admin/registrars/_form.html.erb

@@ -60,7 +60,7 @@
     </div>
 
     <%= render 'admin/registrars/form/address', f: f %>
-    <%= render 'admin/registrars/form/billing', f: f %>
+    <%= render 'admin/registrars/form/billing', f: f, registry_vat_rate: registry_vat_rate %>
 
     <div class="row">
         <div class="col-md-8">

app/views/admin/registrars/form/_address.html.erb

@@ -51,6 +51,7 @@
                         <%= f.select :address_country_code,
                                      SortedCountry.all_options(f.object.address_country_code), {},
                                      required: true, class: 'form-control' %>
+                        <span class="help-block"><%= t '.country_hint' %></span>
                     </div>
                 </div>
 

app/views/admin/registrars/form/_billing.html.erb

@@ -21,12 +21,14 @@
                     <div class="col-md-4 control-label">
                         <%= f.label :vat_rate %>
                     </div>
-                    <div class="col-md-3">
-                        <div class="input-group">
+                    <div class="col-md-8">
+                        <div class="col-md-4 input-group">
                             <%= f.number_field :vat_rate, min: 0, max: 99.9, step: 0.1,
                                                class: 'form-control' %>
                             <div class="input-group-addon">%</div>
                         </div>
+                        <span class="help-block"><%= t '.vat_rate_hint', registry_vat_rate:
+                            number_to_percentage(registry_vat_rate, precision: 1) %></span>
                     </div>
                 </div>
 

config/locales/admin/registrars.en.yml

@@ -52,10 +52,15 @@ en:
 
         address:
           header: Address
-          hint: Used as a billing address
+          country_hint: Affects VAT rate calculation
+          hint: This address is used as a billing address
 
         billing:
           header: Billing
+          vat_rate_hint: >-
+            Applies to new invoices.
+            Leave blank if a registrar is VAT-registered;
+            registry's rate of %{registry_vat_rate} will be applied in this case.
           no_reference_number_hint: Reference number will be generated automatically
           disabled_reference_number_hint: Reference number cannot be changed
 

config/locales/registrars.en.yml

@@ -6,3 +6,6 @@ en:
           attributes:
             code:
               forbidden: is forbidden
+            vat_rate:
+              present: >-
+                must be blank when a registrar is VAT-registered in the same country as registry

lib/tasks/data_migrations/populate_invoice_vat_rate.rake

@@ -0,0 +1,16 @@
+namespace :data_migrations do
+  task populate_invoice_vat_rate: :environment do
+    processed_invoice_count = 0
+
+    Invoice.transaction do
+      Invoice.where(vat_rate: nil).find_each do |invoice|
+        vat_rate = Invoice::VatRateCalculator.new(registrar: invoice.buyer).calculate
+        invoice.update_columns(vat_rate: vat_rate)
+
+        processed_invoice_count += 1
+      end
+    end
+
+    puts "Invoices processed: #{processed_invoice_count}"
+  end
+end

test/models/invoice/vat_rate_calculator_test.rb

@@ -0,0 +1,32 @@
+require 'test_helper'
+
+class Invoice::VatRateCalculatorTest < ActiveSupport::TestCase
+  def test_applies_registry_vat_rate_when_registrar_is_vat_liable_locally
+    registry_vat_rate = 20
+    registry = Registry.new(vat_rate: registry_vat_rate, vat_country: Country.new(:us))
+    registrar = Registrar.new(vat_rate: 10, vat_country: Country.new(:us))
+
+    vat_calculator = Invoice::VatRateCalculator.new(registry: registry, registrar: registrar)
+
+    assert_equal registry_vat_rate, vat_calculator.calculate
+  end
+
+  def test_applies_registrar_vat_rate_when_registrar_is_vat_liable_in_foreign_country
+    registrar_vat_rate = 20
+    registry = Registry.new(vat_rate: 10, vat_country: Country.new(:gb))
+    registrar = Registrar.new(vat_rate: registrar_vat_rate, vat_country: Country.new(:us))
+
+    vat_calculator = Invoice::VatRateCalculator.new(registry: registry, registrar: registrar)
+
+    assert_equal registrar_vat_rate, vat_calculator.calculate
+  end
+
+  def test_applies_zero_vat_rate_when_registrar_is_vat_liable_in_foreign_country_and_vat_rate_is_absent
+    registry = Registry.new(vat_country: Country.new(:gb))
+    registrar = Registrar.new(vat_rate: nil, vat_country: Country.new(:us))
+
+    vat_calculator = Invoice::VatRateCalculator.new(registry: registry, registrar: registrar)
+
+    assert vat_calculator.calculate.zero?
+  end
+end

test/models/invoice_test.rb

@@ -33,53 +33,11 @@ class InvoiceTest < ActiveSupport::TestCase
     assert_not Invoice.overdue.include?(@invoice), 'Should not return non-overdue invoice'
   end
 
-  def test_optional_vat_rate
-    @invoice.vat_rate = nil
-    assert @invoice.valid?
-  end
-
-  def test_vat_rate_validation
-    @invoice.vat_rate = -1
-    assert @invoice.invalid?
-
-    @invoice.vat_rate = 0
-    assert @invoice.valid?
-
-    @invoice.vat_rate = 99.9
-    assert @invoice.valid?
-
-    @invoice.vat_rate = 100
-    assert @invoice.invalid?
-  end
-
   def test_serializes_and_deserializes_vat_rate
-    invoice = @invoice.dup
-    invoice.items = @invoice.items
-    invoice.vat_rate = BigDecimal('25.5')
-    invoice.save!
-    invoice.reload
-    assert_equal BigDecimal('25.5'), invoice.vat_rate
-  end
-
-  def test_vat_rate_defaults_to_effective_vat_rate_of_a_registrar
-    registrar = registrars(:bestnames)
-    invoice = @invoice.dup
-    invoice.vat_rate = nil
-    invoice.buyer = registrar
-    invoice.items = @invoice.items
-
-    registrar.stub(:effective_vat_rate, BigDecimal(55)) do
-      invoice.save!
-    end
-
-    assert_equal BigDecimal(55), invoice.vat_rate
-  end
-
-  def test_vat_rate_cannot_be_updated
-    @invoice.vat_rate = BigDecimal(21)
+    @invoice.vat_rate = BigDecimal('25.5')
     @invoice.save!
     @invoice.reload
-    refute_equal BigDecimal(21), @invoice.vat_rate
+    assert_equal BigDecimal('25.5'), @invoice.vat_rate
   end
 
   def test_calculates_vat_amount
@@ -88,11 +46,6 @@ class InvoiceTest < ActiveSupport::TestCase
     assert_equal 10, invoice.vat_amount
   end
 
-  def test_vat_amount_is_zero_when_vat_rate_is_blank
-    @invoice.vat_rate = nil
-    assert_equal 0, @invoice.vat_amount
-  end
-
   def test_calculates_subtotal
     line_item = InvoiceItem.new
     invoice = Invoice.new(items: [line_item, line_item])

test/models/registrar/vat_test.rb

@@ -1,97 +0,0 @@
-require 'test_helper'
-
-class RegistrarVATTest < ActiveSupport::TestCase
-  setup do
-    @registrar = registrars(:bestnames)
-  end
-
-  def test_optional_vat_no
-    @registrar.vat_no = ''
-    assert @registrar.valid?
-
-    @registrar.vat_no = 'any'
-    assert @registrar.valid?
-  end
-
-  def test_apply_vat_rate_from_registry_when_registrar_is_local_vat_payer
-    Setting.registry_country_code = 'US'
-    @registrar.address_country_code = 'US'
-
-    Registry.instance.stub(:vat_rate, BigDecimal('5.5')) do
-      assert_equal BigDecimal('5.5'), @registrar.effective_vat_rate
-    end
-  end
-
-  def test_require_no_vat_rate_when_registrar_is_local_vat_payer
-    @registrar.vat_rate = 1
-    assert @registrar.invalid?
-
-    @registrar.vat_rate = nil
-    assert @registrar.valid?
-  end
-
-  def test_apply_vat_rate_from_registrar_when_registrar_is_foreign_vat_payer
-    Setting.registry_country_code = 'US'
-    @registrar.address_country_code = 'DE'
-    @registrar.vat_rate = BigDecimal('5.6')
-    assert_equal BigDecimal('5.6'), @registrar.effective_vat_rate
-  end
-
-  def test_require_vat_rate_when_registrar_is_foreign_vat_payer_and_vat_no_is_absent
-    @registrar.address_country_code = 'DE'
-    @registrar.vat_no = ''
-
-    @registrar.vat_rate = ''
-    assert @registrar.invalid?
-    assert @registrar.errors.added?(:vat_rate, :blank)
-
-    @registrar.vat_rate = 5
-    assert @registrar.valid?
-  end
-
-  def test_require_no_vat_rate_when_registrar_is_foreign_vat_payer_and_vat_no_is_present
-    @registrar.address_country_code = 'DE'
-    @registrar.vat_no = 'valid'
-
-    @registrar.vat_rate = 1
-    assert @registrar.invalid?
-
-    @registrar.vat_rate = nil
-    assert @registrar.valid?
-  end
-
-  def test_vat_rate_validation
-    @registrar.address_country_code = 'DE'
-    @registrar.vat_no = ''
-
-    @registrar.vat_rate = -1
-    assert @registrar.invalid?
-
-    @registrar.vat_rate = 0
-    assert @registrar.valid?
-
-    @registrar.vat_rate = 99.9
-    assert @registrar.valid?
-
-    @registrar.vat_rate = 100
-    assert @registrar.invalid?
-  end
-
-  def test_serializes_and_deserializes_vat_rate
-    @registrar.address_country_code = 'DE'
-    @registrar.vat_rate = BigDecimal('25.5')
-    @registrar.save!
-    @registrar.reload
-    assert_equal BigDecimal('25.5'), @registrar.vat_rate
-  end
-
-  def test_parses_vat_rate_as_a_string
-    @registrar.vat_rate = '25.5'
-    assert_equal BigDecimal('25.5'), @registrar.vat_rate
-  end
-
-  def test_treats_empty_vat_rate_as_nil
-    @registrar.vat_rate = ''
-    assert_nil @registrar.vat_rate
-  end
-end

test/models/registrar_test.rb

@@ -111,9 +111,82 @@ class RegistrarTest < ActiveSupport::TestCase
     assert_equal 'Main Street 1, NY, NY State, 1234', registrar.address
   end
 
+  def test_invalid_with_vat_rate_when_registrar_is_vat_liable_locally
+    registrar = registrar_with_local_vat_liability
+
+    registrar.vat_rate = 1
+
+    assert registrar.invalid?
+    assert_includes registrar.errors.full_messages_for(:vat_rate),
+                    'VAT rate must be blank when a registrar is VAT-registered in the same' \
+                    ' country as registry'
+  end
+
+  def test_invalid_with_vat_rate_when_registrar_is_vat_liable_in_foreign_country_and_vat_no_is_present
+    registrar = registrar_with_foreign_vat_liability
+
+    registrar.vat_no = 'valid'
+    registrar.vat_rate = 1
+
+    assert registrar.invalid?
+  end
+
+  def test_invalid_without_vat_rate_when_registrar_is_vat_liable_in_foreign_country_and_vat_no_is_absent
+    registrar = registrar_with_foreign_vat_liability
+
+    registrar.vat_no = ''
+    registrar.vat_rate = ''
+
+    assert registrar.invalid?
+  end
+
+  def test_vat_rate_validation
+    registrar = registrar_with_foreign_vat_liability
+
+    registrar.vat_rate = -1
+    assert registrar.invalid?
+
+    registrar.vat_rate = 0
+    assert registrar.valid?
+
+    registrar.vat_rate = 99.9
+    assert registrar.valid?
+
+    registrar.vat_rate = 100
+    assert registrar.invalid?
+  end
+
+  def test_serializes_and_deserializes_vat_rate
+    @registrar.address_country_code = 'DE'
+    @registrar.vat_rate = BigDecimal('25.5')
+    @registrar.save!
+    @registrar.reload
+    assert_equal BigDecimal('25.5'), @registrar.vat_rate
+  end
+
+  def test_aliases_vat_country_to_country
+    vat_country = Country.new(:us)
+    registrar = Registrar.new(vat_country: vat_country)
+    assert_equal vat_country, registrar.vat_country
+  end
+
   private
 
   def valid_registrar
     registrars(:bestnames)
   end
+
+  def registrar_with_local_vat_liability
+    registrar = valid_registrar
+    registrar.vat_country = Country.new(:us)
+    Registry.current.vat_country = Country.new(:us)
+    registrar
+  end
+
+  def registrar_with_foreign_vat_liability
+    registrar = valid_registrar
+    registrar.vat_country = Country.new(:gb)
+    Registry.current.vat_country = Country.new(:us)
+    registrar
+  end
 end

test/models/registry_test.rb

@@ -1,20 +1,12 @@
 require 'test_helper'
 
 class RegistryTest < ActiveSupport::TestCase
-  setup do
-    @registry = Registry.send(:new)
-  end
+  def test_returns_current_registry
+    Setting.registry_vat_prc = 0.2
+    Setting.registry_country_code = 'US'
 
-  def test_implements_singleton
-    assert_equal Registry.instance.object_id, Registry.instance.object_id
-  end
-
-  def test_vat_rate
-    original_vat_prc = Setting.registry_vat_prc
-    Setting.registry_vat_prc = 0.25
-
-    assert_equal BigDecimal(25), @registry.vat_rate
-
-    Setting.registry_vat_prc = original_vat_prc
+    registry = Registry.current
+    assert_equal 20, registry.vat_rate
+    assert_equal Country.new(:us), registry.vat_country
   end
 end

test/tasks/data_migrations/populate_invoice_vat_rate_test.rb

@@ -0,0 +1,40 @@
+require 'test_helper'
+
+class PopulateInvoiceVatRateTaskTest < ActiveSupport::TestCase
+  def test_populates_invoice_issue_date
+    invoice = invoice_without_vat_rate
+
+    capture_io do
+      run_task
+    end
+    invoice.reload
+
+    assert_not_nil invoice.vat_rate
+  end
+
+  def test_output
+    eliminate_effect_of_all_invoices_except(invoice_without_vat_rate)
+
+    assert_output "Invoices processed: 1\n" do
+      run_task
+    end
+  end
+
+  private
+
+  def invoice_without_vat_rate
+    invoice = invoices(:one)
+    invoice.update_columns(vat_rate: nil)
+    invoice
+  end
+
+  def eliminate_effect_of_all_invoices_except(invoice)
+    Invoice.connection.disable_referential_integrity do
+      Invoice.delete_all("id != #{invoice.id}")
+    end
+  end
+
+  def run_task
+    Rake::Task['data_migrations:populate_invoice_vat_rate'].execute
+  end
+end