Account::Sync model and test fixture simplifications (#968)

* Add sync model

* Fresh fixtures for sync tests

* Sync tests overhaul

* Fix entry tests

* Complete remaining model test updates

* Update system tests

* Update demo data task

* Add system tests back to PR checks

* More simplifications, add empty family to fixtures for easier testing

test/controllers/account/entries_controller_test.rb

@@ -3,49 +3,48 @@ require "test_helper"
 class Account::EntriesControllerTest < ActionDispatch::IntegrationTest
   setup do
     sign_in @user = users(:family_admin)
-    @account = accounts(:savings)
-    @transaction_entry = @account.entries.account_transactions.first
-    @valuation_entry = @account.entries.account_valuations.first
+    @transaction = account_entries :transaction
+    @valuation = account_entries :valuation
   end
 
   test "should edit valuation entry" do
-    get edit_account_entry_url(@account, @valuation_entry)
+    get edit_account_entry_url(@valuation.account, @valuation)
     assert_response :success
   end
 
   test "should show transaction entry" do
-    get account_entry_url(@account, @transaction_entry)
+    get account_entry_url(@transaction.account, @transaction)
     assert_response :success
   end
 
   test "should show valuation entry" do
-    get account_entry_url(@account, @valuation_entry)
+    get account_entry_url(@valuation.account, @valuation)
     assert_response :success
   end
 
   test "should get list of transaction entries" do
-    get transaction_account_entries_url(@account)
+    get transaction_account_entries_url(@transaction.account)
     assert_response :success
   end
 
   test "should get list of valuation entries" do
-    get valuation_account_entries_url(@account)
+    get valuation_account_entries_url(@valuation.account)
     assert_response :success
   end
 
   test "gets new entry by type" do
-    get new_account_entry_url(@account, entryable_type: "Account::Valuation")
+    get new_account_entry_url(@valuation.account, entryable_type: "Account::Valuation")
     assert_response :success
   end
 
   test "should create valuation" do
     assert_difference [ "Account::Entry.count", "Account::Valuation.count" ], 1 do
-      post account_entries_url(@account), params: {
+      post account_entries_url(@valuation.account), params: {
         account_entry: {
           name: "Manual valuation",
           amount: 19800,
           date: Date.current,
-          currency: @account.currency,
+          currency: @valuation.account.currency,
           entryable_type: "Account::Valuation",
           entryable_attributes: {}
         }
@@ -54,16 +53,16 @@ class Account::EntriesControllerTest < ActionDispatch::IntegrationTest
 
     assert_equal "Valuation created", flash[:notice]
     assert_enqueued_with job: AccountSyncJob
-    assert_redirected_to account_path(@account)
+    assert_redirected_to account_path(@valuation.account)
   end
 
   test "error when valuation already exists for date" do
     assert_no_difference_in_entries do
-      post account_entries_url(@account), params: {
+      post account_entries_url(@valuation.account), params: {
         account_entry: {
           amount: 19800,
-          date: @valuation_entry.date,
-          currency: @valuation_entry.currency,
+          date: @valuation.date,
+          currency: @valuation.currency,
           entryable_type: "Account::Valuation",
           entryable_attributes: {}
         }
@@ -71,33 +70,33 @@ class Account::EntriesControllerTest < ActionDispatch::IntegrationTest
     end
 
     assert_equal "Date has already been taken", flash[:error]
-    assert_redirected_to account_path(@account)
+    assert_redirected_to account_path(@valuation.account)
   end
 
   test "can update entry without entryable attributes" do
     assert_no_difference_in_entries do
-      patch account_entry_url(@account, @valuation_entry), params: {
+      patch account_entry_url(@valuation.account, @valuation), params: {
         account_entry: {
           name: "Updated name"
         }
       }
     end
 
-    assert_redirected_to account_entry_url(@account, @valuation_entry)
+    assert_redirected_to account_entry_url(@valuation.account, @valuation)
     assert_enqueued_with(job: AccountSyncJob)
   end
 
   test "should update transaction entry with entryable attributes" do
     assert_no_difference_in_entries do
-      patch account_entry_url(@account, @transaction_entry), params: {
+      patch account_entry_url(@transaction.account, @transaction), params: {
         account_entry: {
           name: "Updated name",
           date: Date.current,
           currency: "USD",
           amount: 20,
-          entryable_type: @transaction_entry.entryable_type,
+          entryable_type: @transaction.entryable_type,
           entryable_attributes: {
-            id: @transaction_entry.entryable_id,
+            id: @transaction.entryable_id,
             tag_ids: [ Tag.first.id, Tag.second.id ],
             category_id: Category.first.id,
             merchant_id: Merchant.first.id,
@@ -108,17 +107,17 @@ class Account::EntriesControllerTest < ActionDispatch::IntegrationTest
       }
     end
 
-    assert_redirected_to account_entry_url(@account, @transaction_entry)
+    assert_redirected_to account_entry_url(@transaction.account, @transaction)
     assert_enqueued_with(job: AccountSyncJob)
   end
 
   test "should destroy transaction entry" do
-    [ @transaction_entry, @valuation_entry ].each do |entry|
+    [ @transaction, @valuation ].each do |entry|
       assert_difference -> { Account::Entry.count } => -1, -> { entry.entryable_class.count } => -1 do
-        delete account_entry_url(@account, entry)
+        delete account_entry_url(entry.account, entry)
       end
 
-      assert_redirected_to account_url(@account)
+      assert_redirected_to account_url(entry.account)
       assert_enqueued_with(job: AccountSyncJob)
     end
   end

test/controllers/account/transfers_controller_test.rb

@@ -14,8 +14,8 @@ class Account::TransfersControllerTest < ActionDispatch::IntegrationTest
     assert_difference "Account::Transfer.count", 1 do
       post account_transfers_url, params: {
         account_transfer: {
-          from_account_id: accounts(:checking).id,
-          to_account_id: accounts(:savings).id,
+          from_account_id: accounts(:depository).id,
+          to_account_id: accounts(:credit_card).id,
           date: Date.current,
           amount: 100,
           currency: "USD",
@@ -28,7 +28,7 @@ class Account::TransfersControllerTest < ActionDispatch::IntegrationTest
 
   test "can destroy transfer" do
     assert_difference -> { Account::Transfer.count } => -1, -> { Account::Transaction.count } => 0 do
-      delete account_transfer_url(account_transfers(:credit_card_payment))
+      delete account_transfer_url(account_transfers(:one))
     end
   end
 end

test/controllers/accounts_controller_test.rb

@@ -3,7 +3,7 @@ require "test_helper"
 class AccountsControllerTest < ActionDispatch::IntegrationTest
   setup do
     sign_in @user = users(:family_admin)
-    @account = accounts(:checking)
+    @account = accounts(:depository)
   end
 
   test "gets accounts list" do
@@ -33,7 +33,7 @@ class AccountsControllerTest < ActionDispatch::IntegrationTest
   test "can sync all accounts" do
     post sync_all_accounts_path
     assert_redirected_to accounts_url
-    assert_equal "Successfully queued #{ @user.family.accounts.size } accounts for syncing.", flash[:notice]
+    assert_equal "Successfully queued accounts for syncing.", flash[:notice]
   end
 
   test "should update account" do

test/controllers/categories_controller_test.rb

@@ -3,6 +3,7 @@ require "test_helper"
 class CategoriesControllerTest < ActionDispatch::IntegrationTest
   setup do
     sign_in users(:family_admin)
+    @transaction = account_transactions :one
   end
 
   test "index" do
@@ -37,7 +38,7 @@ class CategoriesControllerTest < ActionDispatch::IntegrationTest
 
     assert_difference "Category.count", +1 do
       post categories_url, params: {
-        transaction_id: account_transactions(:checking_one).id,
+        transaction_id: @transaction.id,
         category: {
           name: "New Category",
           color: color } }
@@ -48,7 +49,7 @@ class CategoriesControllerTest < ActionDispatch::IntegrationTest
     assert_redirected_to transactions_url
     assert_equal "New Category", new_category.name
     assert_equal color, new_category.color
-    assert_equal account_transactions(:checking_one).reload.category, new_category
+    assert_equal @transaction.reload.category, new_category
   end
 
   test "edit" do

test/controllers/tag/deletions_controller_test.rb

@@ -3,8 +3,7 @@ require "test_helper"
 class Tag::DeletionsControllerTest < ActionDispatch::IntegrationTest
   setup do
     sign_in @user = users(:family_admin)
-    @user_tags = @user.family.tags
-    @tag = tags(:hawaii_trip)
+    @tag = tags(:one)
   end
 
   test "should get new" do
@@ -13,7 +12,7 @@ class Tag::DeletionsControllerTest < ActionDispatch::IntegrationTest
   end
 
   test "create with replacement" do
-    replacement_tag = tags(:trips)
+    replacement_tag = tags(:two)
 
     affected_transaction_count = @tag.transactions.count
 

test/controllers/transactions_controller_test.rb

@@ -1,10 +1,11 @@
 require "test_helper"
 
 class TransactionsControllerTest < ActionDispatch::IntegrationTest
+  include Account::EntriesTestHelper
+
   setup do
     sign_in @user = users(:family_admin)
-    @transaction_entry = account_entries(:checking_one)
-    @recent_transaction_entries = @user.family.entries.account_transactions.reverse_chronological.limit(20).to_a
+    @transaction = account_entries(:transaction)
   end
 
   test "should get new" do
@@ -13,9 +14,9 @@ class TransactionsControllerTest < ActionDispatch::IntegrationTest
   end
 
   test "prefills account_id" do
-    get new_transaction_url(account_id: @transaction_entry.account.id)
+    get new_transaction_url(account_id: @transaction.account.id)
     assert_response :success
-    assert_select "option[selected][value='#{@transaction_entry.account.id}']"
+    assert_select "option[selected][value='#{@transaction.account.id}']"
   end
 
   test "should create transaction" do
@@ -45,11 +46,11 @@ class TransactionsControllerTest < ActionDispatch::IntegrationTest
       post transactions_url, params: {
         account_entry: {
           nature: "expense",
-          account_id: @transaction_entry.account_id,
-          amount: @transaction_entry.amount,
-          currency: @transaction_entry.currency,
-          date: @transaction_entry.date,
-          name: @transaction_entry.name,
+          account_id: @transaction.account_id,
+          amount: @transaction.amount,
+          currency: @transaction.currency,
+          date: @transaction.date,
+          name: @transaction.name,
           entryable_type: "Account::Transaction",
           entryable_attributes: {}
         }
@@ -58,7 +59,7 @@ class TransactionsControllerTest < ActionDispatch::IntegrationTest
 
     created_entry = Account::Entry.order(created_at: :desc).first
 
-    assert_redirected_to account_url(@transaction_entry.account)
+    assert_redirected_to account_url(@transaction.account)
     assert created_entry.amount.positive?, "Amount should be positive"
   end
 
@@ -67,11 +68,11 @@ class TransactionsControllerTest < ActionDispatch::IntegrationTest
       post transactions_url, params: {
         account_entry: {
           nature: "income",
-          account_id: @transaction_entry.account_id,
-          amount: @transaction_entry.amount,
-          currency: @transaction_entry.currency,
-          date: @transaction_entry.date,
-          name: @transaction_entry.name,
+          account_id: @transaction.account_id,
+          amount: @transaction.amount,
+          currency: @transaction.currency,
+          date: @transaction.date,
+          name: @transaction.name,
           entryable_type: "Account::Transaction",
           entryable_attributes: { category_id: categories(:food_and_drink).id }
         }
@@ -80,83 +81,79 @@ class TransactionsControllerTest < ActionDispatch::IntegrationTest
 
     created_entry = Account::Entry.order(created_at: :desc).first
 
-    assert_redirected_to account_url(@transaction_entry.account)
+    assert_redirected_to account_url(@transaction.account)
     assert created_entry.amount.negative?, "Amount should be negative"
   end
 
-  test "should get paginated index with most recent transactions first" do
-    get transactions_url(per_page: 10)
-    assert_response :success
-
-    @recent_transaction_entries.first(10).each do |transaction|
-      assert_dom "#" + dom_id(transaction), count: 1
-    end
-  end
-
   test "transaction count represents filtered total" do
+    family = families(:empty)
+    sign_in family.users.first
+    account = family.accounts.create! name: "Test", balance: 0, accountable: Depository.new
+
+    3.times do
+      create_transaction(account: account)
+    end
+
     get transactions_url(per_page: 10)
-    assert_dom "#total-transactions", count: 1, text: @user.family.entries.account_transactions.select { |t| t.currency == "USD" }.count.to_s
 
-    new_transaction = @user.family.accounts.first.entries.create! \
-      entryable: Account::Transaction.new,
-      name: "Transaction to search for",
-      date: Date.current,
-      amount: 0,
-      currency: "USD"
+    assert_dom "#total-transactions", count: 1, text: family.entries.account_transactions.size.to_s
 
-    get transactions_url(q: { search: new_transaction.name })
+    searchable_transaction = create_transaction(account: account, name: "Unique test name")
+
+    get transactions_url(q: { search: searchable_transaction.name })
 
     # Only finds 1 transaction that matches filter
-    assert_dom "#" + dom_id(new_transaction), count: 1
+    assert_dom "#" + dom_id(searchable_transaction), count: 1
     assert_dom "#total-transactions", count: 1, text: "1"
   end
 
-  test "can navigate to paginated result" do
-    get transactions_url(page: 2, per_page: 10)
+  test "can paginate" do
+    family = families(:empty)
+    sign_in family.users.first
+    account = family.accounts.create! name: "Test", balance: 0, accountable: Depository.new
+
+    11.times do
+      create_transaction(account: account)
+    end
+
+    sorted_transactions = family.entries.account_transactions.reverse_chronological.to_a
+
+    assert_equal 11, sorted_transactions.count
+
+    get transactions_url(page: 1, per_page: 10)
+
     assert_response :success
-
-    visible_transaction_entries = @recent_transaction_entries[10, 10].reject { |e| e.transfer.present? }
-
-    visible_transaction_entries.each do |transaction|
+    sorted_transactions.first(10).each do |transaction|
       assert_dom "#" + dom_id(transaction), count: 1
     end
-  end
 
-  test "loads last page when page is out of range" do
-    user_oldest_transaction_entry = @user.family.entries.account_transactions.chronological.first
-    get transactions_url(page: 9999999999)
+    get transactions_url(page: 2, per_page: 10)
 
-    assert_response :success
-    assert_dom "#" + dom_id(user_oldest_transaction_entry), count: 1
+    assert_dom "#" + dom_id(sorted_transactions.last), count: 1
+
+    get transactions_url(page: 9999999, per_page: 10) # out of range loads last page
+
+    assert_dom "#" + dom_id(sorted_transactions.last), count: 1
   end
 
   test "can destroy many transactions at once" do
-    delete_count = 10
+    transactions = @user.family.entries.account_transactions
+    delete_count = transactions.size
+
     assert_difference([ "Account::Transaction.count", "Account::Entry.count" ], -delete_count) do
       post bulk_delete_transactions_url, params: {
         bulk_delete: {
-          entry_ids: @recent_transaction_entries.first(delete_count).pluck(:id)
+          entry_ids: transactions.pluck(:id)
         }
       }
     end
 
     assert_redirected_to transactions_url
-    assert_equal "10 transactions deleted", flash[:notice]
+    assert_equal "#{delete_count} transactions deleted", flash[:notice]
   end
 
   test "can update many transactions at once" do
-    transactions = @user.family.entries.account_transactions.reverse_chronological.limit(20)
-
-    transactions.each do |transaction|
-      transaction.update! \
-        date: Date.current,
-        entryable_attributes: {
-          id: transaction.account_transaction.id,
-          category_id: Category.first.id,
-          merchant_id: Merchant.first.id,
-          notes: "Starting note"
-        }
-    end
+    transactions = @user.family.entries.account_transactions
 
     assert_difference [ "Account::Entry.count", "Account::Transaction.count" ], 0 do
       post bulk_update_transactions_url, params: {
@@ -173,9 +170,7 @@ class TransactionsControllerTest < ActionDispatch::IntegrationTest
     assert_redirected_to transactions_url
     assert_equal "#{transactions.count} transactions updated", flash[:notice]
 
-    transactions.reload
-
-    transactions.each do |transaction|
+    transactions.reload.each do |transaction|
       assert_equal 1.day.ago.to_date, transaction.date
       assert_equal Category.second, transaction.account_transaction.category
       assert_equal Merchant.second, transaction.account_transaction.merchant