Refactor and improve invoices

- `runner 'Invoice.cancel_overdue_invoices'` in `schedule.rb` is
changed to `rake 'invoices:cancel_overdue'`.
- `invoices.payment_term` database column is removed and its value is
hardcoded in UI.
- `invoices.paid_at` is removed as unused
- `invoices.due_date` column's type is now `date`.
- `Invoice#invoice_items` renamed to `Invoice#items` and `Invoice`
interface to get a list of items is unified.
- Default date format in UI.
- Default translations are used.
- Tests improved.
- Specs converted to tests and removed along with factories.
- Database structure improved.

test/fixtures/account_activities.yml

@@ -1,7 +1,5 @@
-DEFAULTS: &DEFAULTS
-  account_id: 1
-
 one:
-  <<: *DEFAULTS
-  invoice: paid
+  account: cash
+  invoice: one
   bank_transaction: one
+  created_at: <%= Time.zone.parse('2010-07-05 10:00') %>

test/fixtures/accounts.yml

@@ -4,6 +4,12 @@ cash:
   currency: EUR
   registrar: bestnames
 
+two:
+  account_type: cash
+  balance: 100
+  currency: EUR
+  registrar: goodnames
+
 not_in_use_cash:
   account_type: cash
   balance: 0

test/fixtures/invoice_items.yml

@@ -1,13 +1,6 @@
 one:
   description: Acme services
   price: 5
-  amount: 1
+  quantity: 1
   unit: pc
-  invoice: valid
-
-two:
-  description: Acme services
-  price: 5
-  amount: 2
-  unit: pc
-  invoice: valid
+  invoice: one

test/fixtures/invoices.yml

@@ -1,5 +1,5 @@
-DEFAULTS: &DEFAULTS
-  created_at: <%= Date.parse '2010-07-05' %>
+one:
+  issue_date: <%= Date.parse '2010-07-05' %>
   due_date: <%= Date.parse '2010-07-06' %>
   currency: EUR
   seller_name: John Doe
@@ -10,30 +10,15 @@ DEFAULTS: &DEFAULTS
   total: 16.50
   reference_no: 13
 
-valid:
-  <<: *DEFAULTS
-
-exported:
-  <<: *DEFAULTS
-  in_directo: true
-
-cancelled:
-  <<: *DEFAULTS
-  cancelled_at: <%= Date.parse '2010-07-05' %>
-
-paid:
-  <<: *DEFAULTS
-  total: 1
-
-outstanding:
-  <<: *DEFAULTS
-  due_date: <%= Date.parse '2010-07-04' %>
-
-overdue:
-  <<: *DEFAULTS
-  due_date: <%= Date.parse '2010-07-03' %>
-
 for_payments_test:
-  <<: *DEFAULTS
-  total: 12.00
   number: 1
+  issue_date: <%= Date.parse '2010-07-05' %>
+  due_date: <%= Date.parse '2010-07-06' %>
+  currency: EUR
+  seller_name: John Doe
+  seller_iban: 1234
+  buyer: bestnames
+  buyer_name: Jane Doe
+  vat_rate: 0.1
+  reference_no: 13
+  total: 12.00

test/integration/tasks/data_migrations/regenerate_registrar_reference_numbers_test.rb

@@ -35,7 +35,7 @@ class RegenerateRegistrarReferenceNumbersTaskTest < ActiveSupport::TestCase
   def test_keeps_iso_reference_number_on_the_invoice_unchanged
     registrar = registrars(:bestnames)
     registrar.update_column(:reference_no, 'RF1111')
-    invoice = registrar.invoices.first
+    invoice = invoices(:one)
     invoice.update!(reference_no: 'RF2222')
 
     capture_io { run_task }

test/models/account_activity_test.rb

@@ -0,0 +1,11 @@
+require 'test_helper'
+
+class AccountActivityTest < ActiveSupport::TestCase
+  setup do
+    @account_activity = account_activities(:one)
+  end
+
+  def test_fixture_is_valid
+    assert @account_activity.valid?
+  end
+end

test/models/bank_transaction_test.rb

@@ -2,7 +2,7 @@ require 'test_helper'
 
 class BankTransactionTest < ActiveSupport::TestCase
   def test_matches_against_invoice_reference_number
-    invoices(:valid).update!(number: '2222', total: 10, reference_no: '1111')
+    invoices(:one).update!(account_activity: nil, number: '2222', total: 10, reference_no: '1111')
     transaction = BankTransaction.new(description: 'invoice #2222', sum: 10, reference_no: '1111')
 
     assert_difference 'AccountActivity.count' do
@@ -20,7 +20,7 @@ class BankTransactionTest < ActiveSupport::TestCase
   end
 
   def test_underpayment_is_not_matched_with_invoice
-    invoices(:valid).update!(number: '2222', total: 10)
+    invoices(:one).update!(account_activity: nil, number: '2222', total: 10)
     transaction = BankTransaction.new(sum: 9)
 
     assert_no_difference 'AccountActivity.count' do
@@ -30,7 +30,7 @@ class BankTransactionTest < ActiveSupport::TestCase
   end
 
   def test_overpayment_is_not_matched_with_invoice
-    invoices(:valid).update!(number: '2222', total: 10)
+    invoices(:one).update!(account_activity: nil, number: '2222', total: 10)
     transaction = BankTransaction.new(sum: 11)
 
     assert_no_difference 'AccountActivity.count' do
@@ -40,7 +40,7 @@ class BankTransactionTest < ActiveSupport::TestCase
   end
 
   def test_cancelled_invoice_is_not_matched
-    invoices(:valid).update!(number: '2222', total: 10, cancelled_at: '2010-07-05')
+    invoices(:one).update!(account_activity: nil, number: '2222', total: 10, cancelled_at: '2010-07-05')
     transaction = BankTransaction.new(sum: 10)
 
     assert_no_difference 'AccountActivity.count' do

test/models/invoice/cancellable_test.rb

@@ -0,0 +1,69 @@
+require 'test_helper'
+
+class CancellableInvoiceTest < ActiveSupport::TestCase
+  setup do
+    @invoice = invoices(:one)
+  end
+
+  def test_non_cancelled_scope_returns_non_cancelled_invoices
+    @invoice.update!(cancelled_at: nil)
+    assert Invoice.non_cancelled.include?(@invoice), 'Should return cancelled invoice'
+  end
+
+  def test_non_cancelled_scope_does_not_return_cancelled_invoices
+    @invoice.update!(cancelled_at: '2010-07-05')
+    assert_not Invoice.non_cancelled.include?(@invoice), 'Should not return cancelled invoice'
+  end
+
+  def test_cancellable_when_unpaid_and_not_yet_cancelled
+    @invoice.account_activity = nil
+    @invoice.cancelled_at = nil
+    assert @invoice.cancellable?
+  end
+
+  def test_not_cancellable_when_paid
+    assert @invoice.paid?
+    assert_not @invoice.cancellable?
+  end
+
+  def test_not_cancellable_when_already_cancelled
+    @invoice.cancelled_at = '2010-07-05'
+    assert_not @invoice.cancellable?
+  end
+
+  def test_cancels_an_invoice
+    travel_to Time.zone.parse('2010-07-05 08:00')
+    @invoice.account_activity = nil
+    assert @invoice.cancellable?
+    assert_nil @invoice.cancelled_at
+
+    @invoice.cancel
+    @invoice.reload
+
+    assert @invoice.cancelled?
+    assert_equal Time.zone.parse('2010-07-05 08:00'), @invoice.cancelled_at
+  end
+
+  def test_throws_an_exception_when_trying_to_cancel_already_cancelled_invoice
+    @invoice.cancelled_at = '2010-07-05'
+
+    e = assert_raise do
+      @invoice.cancel
+    end
+    assert_equal 'Invoice cannot be cancelled', e.message
+  end
+
+  def test_not_cancelled
+    @invoice.cancelled_at = nil
+
+    assert @invoice.not_cancelled?
+    assert_not @invoice.cancelled?
+  end
+
+  def test_cancelled
+    @invoice.cancelled_at = '2010-07-05'
+
+    assert @invoice.cancelled?
+    assert_not @invoice.not_cancelled?
+  end
+end

test/models/invoice/payable_test.rb

@@ -0,0 +1,53 @@
+require 'test_helper'
+
+class InvoiceTest < ActiveSupport::TestCase
+  setup do
+    @invoice = invoices(:one)
+  end
+
+  def test_unpaid_scope_returns_unpaid_invoices
+    @invoice.account_activity = nil
+    assert Invoice.unpaid.include?(@invoice), 'Should return unpaid invoice'
+  end
+
+  def test_unpaid_scope_does_not_return_paid_invoices
+    assert @invoice.paid?
+    assert_not Invoice.unpaid.include?(@invoice), 'Should not return paid invoice'
+  end
+
+  def test_paid_when_there_is_an_account_activity
+    assert @invoice.account_activity
+
+    assert @invoice.paid?
+    assert_not @invoice.unpaid?
+  end
+
+  def test_unpaid_when_there_is_no_account_activity
+    @invoice.account_activity = nil
+
+    assert @invoice.unpaid?
+    assert_not @invoice.paid?
+  end
+
+  def test_payable_when_unpaid_and_not_cancelled
+    @invoice.account_activity = nil
+    @invoice.cancelled_at = nil
+
+    assert @invoice.payable?
+  end
+
+  def test_not_payable_when_already_paid
+    assert @invoice.paid?
+    assert_not @invoice.payable?
+  end
+
+  def test_not_payable_when_cancelled
+    @invoice.cancelled_at = '2010-07-05'
+    assert_not @invoice.payable?
+  end
+
+  def test_returns_receipt_date
+    assert_equal Time.zone.parse('2010-07-05 10:00'), @invoice.account_activity.created_at
+    assert_equal Date.parse('2010-07-05'), @invoice.receipt_date
+  end
+end

test/models/invoice_item_test.rb

@@ -0,0 +1,8 @@
+require 'test_helper'
+
+class InvoiceItemTest < ActiveSupport::TestCase
+  def test_calculates_sum_without_vat
+    invoice_item = InvoiceItem.new(price: 5, quantity: 2)
+    assert_equal 10, invoice_item.item_sum_without_vat
+  end
+end

test/models/invoice_test.rb

@@ -2,11 +2,40 @@ require 'test_helper'
 
 class InvoiceTest < ActiveSupport::TestCase
   setup do
-    @invoice = invoices(:valid)
+    @invoice = invoices(:one)
   end
 
-  def test_valid
-    assert @invoice.valid?
+  def test_fixture_is_valid
+    assert @invoice.valid?, proc { @invoice.errors.full_messages }
+  end
+
+  def test_overdue_scope_returns_unpaid_uncancelled_invoices_with_past_due_date
+    travel_to Time.zone.parse('2010-07-05')
+    @invoice.update!(account_activity: nil, cancelled_at: nil, due_date: '2010-07-04')
+
+    assert Invoice.overdue.include?(@invoice), 'Should return overdue invoice'
+  end
+
+  def test_overdue_scope_does_not_return_paid_invoices
+    assert @invoice.paid?
+    assert_not Invoice.overdue.include?(@invoice), 'Should not return paid invoice'
+  end
+
+  def test_overdue_scope_does_not_return_cancelled_invoices
+    @invoice.update!(cancelled_at: '2010-07-05')
+    assert_not Invoice.overdue.include?(@invoice), 'Should not return cancelled invoice'
+  end
+
+  def test_overdue_scope_does_not_return_invoices_with_due_due_of_today_or_in_the_future
+    travel_to Time.zone.parse('2010-07-05')
+    @invoice.update!(due_date: '2010-07-05')
+
+    assert_not Invoice.overdue.include?(@invoice), 'Should not return non-overdue invoice'
+  end
+
+  def test_invalid_without_issue_date
+    @invoice.issue_date = nil
+    assert @invoice.invalid?
   end
 
   def test_optional_vat_rate
@@ -30,7 +59,7 @@ class InvoiceTest < ActiveSupport::TestCase
 
   def test_serializes_and_deserializes_vat_rate
     invoice = @invoice.dup
-    invoice.invoice_items = @invoice.invoice_items
+    invoice.items = @invoice.items
     invoice.vat_rate = BigDecimal('25.5')
     invoice.save!
     invoice.reload
@@ -42,7 +71,7 @@ class InvoiceTest < ActiveSupport::TestCase
     invoice = @invoice.dup
     invoice.vat_rate = nil
     invoice.buyer = registrar
-    invoice.invoice_items = @invoice.invoice_items
+    invoice.items = @invoice.items
 
     registrar.stub(:effective_vat_rate, BigDecimal(55)) do
       invoice.save!
@@ -59,7 +88,9 @@ class InvoiceTest < ActiveSupport::TestCase
   end
 
   def test_calculates_vat_amount
-    assert_equal BigDecimal('1.5'), @invoice.vat_amount
+    invoice_item = InvoiceItem.new(price: 25, quantity: 2)
+    invoice = Invoice.new(vat_rate: 10, items: [invoice_item, invoice_item.dup])
+    assert_equal 10, invoice.vat_amount
   end
 
   def test_vat_amount_is_zero_when_vat_rate_is_blank
@@ -69,7 +100,7 @@ class InvoiceTest < ActiveSupport::TestCase
 
   def test_calculates_subtotal
     line_item = InvoiceItem.new
-    invoice = Invoice.new(invoice_items: [line_item, line_item])
+    invoice = Invoice.new(items: [line_item, line_item])
 
     line_item.stub(:item_sum_without_vat, BigDecimal('2.5')) do
       assert_equal BigDecimal(5), invoice.subtotal
@@ -84,7 +115,7 @@ class InvoiceTest < ActiveSupport::TestCase
     line_item = InvoiceItem.new
     invoice = Invoice.new
     invoice.vat_rate = 10
-    invoice.invoice_items = [line_item, line_item]
+    invoice.items = [line_item, line_item]
 
     line_item.stub(:item_sum_without_vat, BigDecimal('2.5')) do
       assert_equal BigDecimal('5.50'), invoice.total
@@ -102,8 +133,25 @@ class InvoiceTest < ActiveSupport::TestCase
     invoice = @invoice.dup
     invoice.buyer_vat_no = nil
     invoice.buyer = registrar
-    invoice.invoice_items = @invoice.invoice_items
+    invoice.items = @invoice.items
     invoice.save!
     assert_equal 'US1234', invoice.buyer_vat_no
   end
-end
+
+  def test_invalid_without_invoice_items
+    @invoice.items.clear
+    assert @invoice.invalid?
+  end
+
+  def test_iterates_over_invoice_items
+    invoice = Invoice.new(items: [InvoiceItem.new(description: 'test')])
+
+    iteration_count = 0
+    invoice.each do |invoice_item|
+      assert_equal 'test', invoice_item.description
+      iteration_count += 1
+    end
+
+    assert_equal 1, iteration_count
+  end
+end

test/models/payment_orders/bank_link_test.rb

@@ -10,8 +10,8 @@ class BankLinkTest < ActiveSupport::TestCase
     @invoice = invoices(:for_payments_test)
     invoice_item = invoice_items(:one)
 
-    @invoice.invoice_items << invoice_item
-    @invoice.invoice_items << invoice_item
+    @invoice.items << invoice_item
+    @invoice.items << invoice_item
 
     travel_to '2018-04-01 00:30 +0300'
     create_new_bank_link

test/models/payment_orders/every_pay_test.rb

@@ -7,8 +7,8 @@ class EveryPayTest < ActiveSupport::TestCase
     @invoice = invoices(:for_payments_test)
     invoice_item = invoice_items(:one)
 
-    @invoice.invoice_items << invoice_item
-    @invoice.invoice_items << invoice_item
+    @invoice.items << invoice_item
+    @invoice.items << invoice_item
 
     params = {
       response:

test/models/registrar_test.rb

@@ -83,4 +83,17 @@ class RegistrarTest < ActiveSupport::TestCase
       registrars(:goodnames).update!(reference_no: '1234')
     end
   end
+
+  def test_issues_new_invoice
+    travel_to Time.zone.parse('2010-07-05')
+    @original_days_to_keep_invoices_active_setting = Setting.days_to_keep_invoices_active
+    Setting.days_to_keep_invoices_active = 10
+
+    invoice = @registrar.issue_prepayment_invoice(100)
+
+    assert_equal Date.parse('2010-07-05'), invoice.issue_date
+    assert_equal Date.parse('2010-07-15'), invoice.due_date
+
+    Setting.days_to_keep_invoices_active = @original_days_to_keep_invoices_active_setting
+  end
 end

test/services/overdue_invoice_canceller_test.rb

@@ -0,0 +1,62 @@
+require 'test_helper'
+
+class OverdueInvoiceCancellerTest < ActiveSupport::TestCase
+  setup do
+    @invoice = invoices(:one)
+  end
+
+  def test_default_delay
+    assert_equal 30.days, OverdueInvoiceCanceller.default_delay
+  end
+
+  def test_uses_default_delay_when_not_configured
+    Setting.days_to_keep_overdue_invoices_active = nil
+    canceller = OverdueInvoiceCanceller.new
+    assert_equal OverdueInvoiceCanceller.default_delay, canceller.delay
+  end
+
+  def test_uses_configured_delay
+    Setting.days_to_keep_overdue_invoices_active = 1
+    canceller = OverdueInvoiceCanceller.new
+    assert_equal 1.day, canceller.delay
+  end
+
+  def test_cancels_overdue_invoices
+    travel_to Time.zone.parse('2010-07-05')
+    @invoice.update!(account_activity: nil, cancelled_at: nil, due_date: '2010-07-03')
+    assert @invoice.cancellable?
+
+    canceller = OverdueInvoiceCanceller.new(delay: 1.day)
+    canceller.cancel
+    @invoice.reload
+
+    assert @invoice.cancelled?
+  end
+
+  def test_yields_cancelled_invoices
+    travel_to Time.zone.parse('2010-07-05')
+    @invoice.update!(account_activity: nil, cancelled_at: nil, due_date: '2010-07-03')
+    assert @invoice.cancellable?
+
+    canceller = OverdueInvoiceCanceller.new(delay: 1.day)
+
+    iteration_count = 0
+    canceller.cancel do |invoice|
+      assert_equal @invoice, invoice
+      iteration_count += 1
+    end
+    assert_equal 1, iteration_count
+  end
+
+  def test_keeps_not_overdue_invoices_intact
+    travel_to Time.zone.parse('2010-07-05')
+    @invoice.update!(account_activity: nil, cancelled_at: nil, due_date: '2010-07-04')
+    assert @invoice.cancellable?
+
+    canceller = OverdueInvoiceCanceller.new(delay: 1.day)
+    canceller.cancel
+    @invoice.reload
+
+    assert @invoice.not_cancelled?
+  end
+end

test/system/admin_area/invoices_test.rb

@@ -0,0 +1,20 @@
+require 'test_helper'
+
+class AdminAreaInvoicesTest < ApplicationSystemTestCase
+  setup do
+    sign_in users(:admin)
+    @invoice = invoices(:one)
+  end
+
+  def test_cancels_an_invoice
+    @invoice.account_activity = nil
+    assert @invoice.cancellable?
+
+    visit admin_invoice_url(@invoice)
+    click_on 'Cancel'
+    @invoice.reload
+
+    assert @invoice.cancelled?
+    assert_text 'Invoice has been cancelled'
+  end
+end

test/system/registrar_area/invoices/list_test.rb

@@ -1,12 +1,12 @@
 require 'test_helper'
 
 class ListInvoicesTest < ApplicationSystemTestCase
-  def setup
-    super
-
+  setup do
     @user = users(:api_bestnames)
-    @registrar_invoices = @user.registrar.invoices
     sign_in @user
+
+    @invoice = invoices(:one)
+    eliminate_effect_of_other_invoices
   end
 
   def test_show_balance
@@ -14,15 +14,31 @@ class ListInvoicesTest < ApplicationSystemTestCase
     assert_text "Your current account balance is 100,00 EUR"
   end
 
-  def test_show_multiple_invoices
-    @invoices = invoices
-    @registrar_invoices = []
-    @invoices.each do |invoice|
-      @registrar_invoices << invoice
-    end
+  def test_show_invoices_of_current_registrar
+    registrar = registrars(:bestnames)
+    @user.update!(registrar: registrar)
+    @invoice.update!(seller: registrar)
 
-    visit registrar_invoices_path
-    assert_text "Unpaid", count: 5
-    assert_text "Invoice no.", count: 7
+    visit registrar_invoices_url
+
+    assert_css '.invoice'
   end
-end
+
+  def test_do_not_show_invoices_of_other_registrars
+    registrar = registrars(:goodnames)
+    @user.update!(registrar: registrar)
+    @invoice.update!(seller: registrar)
+
+    visit registrar_invoices_url
+
+    assert_no_css '.invoice'
+  end
+
+  private
+
+  def eliminate_effect_of_other_invoices
+    Invoice.connection.disable_referential_integrity do
+      Invoice.delete_all("id != #{@invoice.id}")
+    end
+  end
+end

test/system/registrar_area/invoices/payment_callback_test.rb

@@ -12,8 +12,8 @@ class PaymentCallbackTest < ApplicationSystemTestCase
     @invoice = invoices(:for_payments_test)
     invoice_item = invoice_items(:one)
 
-    @invoice.invoice_items << invoice_item
-    @invoice.invoice_items << invoice_item
+    @invoice.items << invoice_item
+    @invoice.items << invoice_item
     @user.registrar.invoices << @invoice
   end
 

test/system/registrar_area/invoices/payment_return_test.rb

@@ -12,8 +12,8 @@ class PaymentReturnTest < ApplicationSystemTestCase
     @invoice = invoices(:for_payments_test)
     invoice_item = invoice_items(:one)
 
-    @invoice.invoice_items << invoice_item
-    @invoice.invoice_items << invoice_item
+    @invoice.items << invoice_item
+    @invoice.items << invoice_item
     @user.registrar.invoices << @invoice
   end
 

test/system/registrar_area/invoices_test.rb

@@ -0,0 +1,20 @@
+require 'test_helper'
+
+class RegistrarAreaInvoicesTest < ApplicationSystemTestCase
+  setup do
+    sign_in users(:api_bestnames)
+    @invoice = invoices(:one)
+  end
+
+  def test_cancels_an_invoice
+    @invoice.account_activity = nil
+    assert @invoice.cancellable?
+
+    visit registrar_invoice_url(@invoice)
+    click_on 'Cancel'
+    @invoice.reload
+
+    assert @invoice.cancelled?
+    assert_text 'Invoice has been cancelled'
+  end
+end

test/tasks/data_migrations/populate_invoice_issue_date_test.rb

@@ -0,0 +1,37 @@
+require 'test_helper'
+
+class PopulateInvoiceIssueDateTaskTest < ActiveSupport::TestCase
+  setup do
+    @invoice = invoices(:one)
+  end
+
+  def test_populates_invoice_issue_date
+    eliminate_effect_of_other_invoices
+    @invoice.update_columns(issue_date: nil, created_at: Time.zone.parse('2010-07-05'))
+    assert_nil @invoice.read_attribute(:issue_date)
+
+    capture_io { run_task }
+    @invoice.reload
+
+    assert_equal Date.parse('2010-07-05'), @invoice.issue_date
+  end
+
+  def test_outputs_results
+    eliminate_effect_of_other_invoices
+    @invoice.update_columns(issue_date: nil, created_at: Time.zone.parse('2010-07-05'))
+
+    assert_output("Invoices processed: 1\n") { run_task }
+  end
+
+  private
+
+  def eliminate_effect_of_other_invoices
+    Invoice.connection.disable_referential_integrity do
+      Invoice.delete_all("id != #{@invoice.id}")
+    end
+  end
+
+  def run_task
+    Rake::Task['data_migrations:populate_invoice_issue_date'].execute
+  end
+end

test/tasks/invoices/cancel_overdue_test.rb

@@ -0,0 +1,41 @@
+require 'test_helper'
+
+class CancelOverdueInvoicesTaskTest < ActiveSupport::TestCase
+  setup do
+    @invoice = invoices(:one)
+    eliminate_effect_of_other_invoices
+  end
+
+  def test_cancels_overdue_invoices
+    @invoice.update!(account_activity: nil, cancelled_at: nil, due_date: '2010-07-05')
+    assert @invoice.cancellable?
+
+    capture_io do
+      run_task
+    end
+    @invoice.reload
+
+    assert @invoice.cancelled?
+  end
+
+  def test_output
+    @invoice.update!(account_activity: nil, cancelled_at: nil, due_date: '2010-07-05')
+    assert @invoice.cancellable?
+
+    assert_output "Invoice ##{@invoice.id} is cancelled\nCancelled total: 1\n" do
+      run_task
+    end
+  end
+
+  private
+
+  def eliminate_effect_of_other_invoices
+    Invoice.connection.disable_referential_integrity do
+      Invoice.delete_all("id != #{@invoice.id}")
+    end
+  end
+
+  def run_task
+    Rake::Task['invoices:cancel_overdue'].execute
+  end
+end