Account namespace updates: part 6 (transactions) (#904)

* Move Transaction to Account namespace

* Fix improper routes, improve separation of concerns

* Replace account transactions list partial with view

* Remove logs

* Consolidate transaction views

* Remove unused code

* Transfer style tweaks

* Remove more unused code

* Add back totals by currency helper

test/controllers/account/transactions_controller_test.rb

@@ -0,0 +1,40 @@
+require "test_helper"
+
+class Account::TransactionsControllerTest < ActionDispatch::IntegrationTest
+  setup do
+    sign_in @user = users(:family_admin)
+    @transaction = account_transactions(:checking_one)
+    @account = @transaction.account
+    @recent_transactions = @user.family.transactions.ordered.limit(20).to_a
+  end
+
+  test "should show transaction" do
+    get account_transaction_url(@transaction.account, @transaction)
+    assert_response :success
+  end
+
+  test "should update transaction" do
+    patch account_transaction_url(@transaction.account, @transaction), params: {
+      transaction: {
+        account_id: @transaction.account_id,
+        amount: @transaction.amount,
+        currency: @transaction.currency,
+        date: @transaction.date,
+        name: @transaction.name,
+        tag_ids: [ Tag.first.id, Tag.second.id ]
+      }
+    }
+
+    assert_redirected_to account_transaction_url(@transaction.account, @transaction)
+    assert_enqueued_with(job: AccountSyncJob)
+  end
+
+  test "should destroy transaction" do
+    assert_difference("Account::Transaction.count", -1) do
+      delete account_transaction_url(@transaction.account, @transaction)
+    end
+
+    assert_redirected_to account_url(@transaction.account)
+    assert_enqueued_with(job: AccountSyncJob)
+  end
+end

test/controllers/account/transfers_controller_test.rb

@@ -26,7 +26,7 @@ class Account::TransfersControllerTest < ActionDispatch::IntegrationTest
   end
 
   test "can destroy transfer" do
-    assert_difference -> { Account::Transfer.count } => -1, -> { Transaction.count } => 0 do
+    assert_difference -> { Account::Transfer.count } => -1, -> { Account::Transaction.count } => 0 do
       delete account_transfer_url(account_transfers(:credit_card_payment))
     end
   end

test/controllers/categories_controller_test.rb

@@ -37,7 +37,7 @@ class CategoriesControllerTest < ActionDispatch::IntegrationTest
 
     assert_difference "Category.count", +1 do
       post categories_url, params: {
-        transaction_id: transactions(:checking_one).id,
+        transaction_id: account_transactions(:checking_one).id,
         category: {
           name: "New Category",
           color: color } }
@@ -48,7 +48,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 transactions(:checking_one).reload.category, new_category
+    assert_equal account_transactions(:checking_one).reload.category, new_category
   end
 
   test "edit" do

test/controllers/category/deletions_controller_test.rb

@@ -30,7 +30,7 @@ class Category::DeletionsControllerTest < ActionDispatch::IntegrationTest
     assert_not_empty @category.transactions
 
     assert_difference "Category.count", -1 do
-      assert_difference "Transaction.where(category: nil).count", @category.transactions.count do
+      assert_difference "Account::Transaction.where(category: nil).count", @category.transactions.count do
         post category_deletions_url(@category)
       end
     end

test/controllers/transactions_controller_test.rb

@@ -3,10 +3,81 @@ require "test_helper"
 class TransactionsControllerTest < ActionDispatch::IntegrationTest
   setup do
     sign_in @user = users(:family_admin)
-    @transaction = transactions(:checking_one)
+    @transaction = account_transactions(:checking_one)
     @recent_transactions = @user.family.transactions.ordered.limit(20).to_a
   end
 
+  test "should get new" do
+    get new_transaction_url
+    assert_response :success
+  end
+
+  test "prefills account_id" do
+    get new_transaction_url(account_id: @transaction.account.id)
+    assert_response :success
+    assert_select "option[selected][value='#{@transaction.account.id}']"
+  end
+
+  test "should create transaction" do
+    account = @user.family.accounts.first
+    transaction_params = {
+      account_id: account.id,
+      amount: 100.45,
+      currency: "USD",
+      date: Date.current,
+      name: "Test transaction"
+    }
+
+    assert_difference("Account::Transaction.count") do
+      post transactions_url, params: { transaction: transaction_params }
+    end
+
+    assert_equal transaction_params[:amount].to_d, Account::Transaction.order(created_at: :desc).first.amount
+    assert_equal "New transaction created successfully", flash[:notice]
+    assert_enqueued_with(job: AccountSyncJob)
+    assert_redirected_to account_url(account)
+  end
+
+  test "expenses are positive" do
+    assert_difference("Account::Transaction.count") do
+      post transactions_url, params: {
+        transaction: {
+          nature: "expense",
+          account_id: @transaction.account_id,
+          amount: @transaction.amount,
+          currency: @transaction.currency,
+          date: @transaction.date,
+          name: @transaction.name
+        }
+      }
+    end
+
+    created_transaction = Account::Transaction.order(created_at: :desc).first
+
+    assert_redirected_to account_url(@transaction.account)
+    assert created_transaction.amount.positive?, "Amount should be positive"
+  end
+
+  test "incomes are negative" do
+    assert_difference("Account::Transaction.count") do
+      post transactions_url, params: {
+        transaction: {
+          nature: "income",
+          account_id: @transaction.account_id,
+          amount: @transaction.amount,
+          currency: @transaction.currency,
+          date: @transaction.date,
+          name: @transaction.name
+        }
+      }
+    end
+
+    created_transaction = Account::Transaction.order(created_at: :desc).first
+
+    assert_redirected_to account_url(@transaction.account)
+    assert created_transaction.amount.negative?, "Amount should be negative"
+  end
+
   test "should get paginated index with most recent transactions first" do
     get transactions_url
     assert_response :success
@@ -49,104 +120,14 @@ class TransactionsControllerTest < ActionDispatch::IntegrationTest
     assert_dom "#" + dom_id(user_oldest_transaction), count: 1
   end
 
-  test "should get new" do
-    get new_transaction_url
-    assert_response :success
-  end
-
-  test "prefills account_id if provided" do
-    get new_transaction_url(account_id: @transaction.account_id)
-    assert_response :success
-    assert_select "option[selected][value='#{@transaction.account_id}']"
-  end
-
-  test "should create transaction" do
-    account = @user.family.accounts.first
-    transaction_params = {
-      account_id: account.id,
-      amount: 100.45,
-      currency: "USD",
-      date: Date.current,
-      name: "Test transaction"
-    }
-
-    assert_difference("Transaction.count") do
-      post transactions_url, params: { transaction: transaction_params }
-    end
-
-    assert_equal transaction_params[:amount].to_d, Transaction.order(created_at: :desc).first.amount
-    assert_equal "New transaction created successfully", flash[:notice]
-    assert_enqueued_with(job: AccountSyncJob)
-    assert_redirected_to transactions_url
-  end
-
-  test "expenses are positive" do
-    assert_difference("Transaction.count") do
-      post transactions_url, params: { transaction: {
-        nature: "expense",
-        account_id: @transaction.account_id,
-        amount: @transaction.amount,
-        currency: @transaction.currency,
-        date: @transaction.date,
-        name: @transaction.name } }
-    end
-
-    assert_redirected_to transactions_url
-    assert Transaction.order(created_at: :desc).first.amount.positive?, "Amount should be positive"
-  end
-
-  test "incomes are negative" do
-    assert_difference("Transaction.count") do
-      post transactions_url, params: {
-        transaction: {
-          nature: "income",
-          account_id: @transaction.account_id,
-          amount: @transaction.amount,
-          currency: @transaction.currency,
-          date: @transaction.date,
-          name: @transaction.name
-        }
-      }
-    end
-
-    assert_redirected_to transactions_url
-    assert Transaction.order(created_at: :desc).first.amount.negative?, "Amount should be negative"
-  end
-
-  test "should show transaction" do
-    get transaction_url(@transaction)
-    assert_response :success
-  end
-
-  test "should update transaction" do
-    patch transaction_url(@transaction), params: {
-      transaction: {
-        account_id: @transaction.account_id,
-        amount: @transaction.amount,
-        currency: @transaction.currency,
-        date: @transaction.date,
-        name: @transaction.name,
-        tag_ids: [ Tag.first.id, Tag.second.id ]
-      }
-    }
-
-    assert_redirected_to transaction_url(@transaction)
-    assert_enqueued_with(job: AccountSyncJob)
-  end
-
-  test "should destroy transaction" do
-    assert_difference("Transaction.count", -1) do
-      delete transaction_url(@transaction)
-    end
-
-    assert_redirected_to transactions_url
-    assert_enqueued_with(job: AccountSyncJob)
-  end
-
   test "can destroy many transactions at once" do
     delete_count = 10
-    assert_difference("Transaction.count", -delete_count) do
-      post bulk_delete_transactions_url, params: { bulk_delete: { transaction_ids: @recent_transactions.first(delete_count).pluck(:id) } }
+    assert_difference("Account::Transaction.count", -delete_count) do
+      post bulk_delete_transactions_url, params: {
+        bulk_delete: {
+          transaction_ids: @recent_transactions.first(delete_count).pluck(:id)
+        }
+      }
     end
 
     assert_redirected_to transactions_url

test/fixtures/taggings.yml

@@ -1,10 +1,10 @@
 one:
   tag: hawaii_trip
   taggable: checking_one
-  taggable_type: Transaction
+  taggable_type: Account::Transaction
 
 two:
   tag: emergency_fund
   taggable: checking_two
-  taggable_type: Transaction
+  taggable_type: Account::Transaction
 

test/models/account/transaction_test.rb

@@ -1,15 +1,15 @@
 require "test_helper"
 
-class TransactionTest < ActiveSupport::TestCase
+class Account::TransactionTest < ActiveSupport::TestCase
   setup do
-    @transaction = transactions(:checking_one)
+    @transaction = account_transactions(:checking_one)
     @family = families(:dylan_family)
   end
 
   # See: https://github.com/maybe-finance/maybe/wiki/vision#signage-of-money
   test "negative amounts are inflows, positive amounts are outflows to an account" do
-    inflow_transaction = transactions(:checking_four)
-    outflow_transaction = transactions(:checking_five)
+    inflow_transaction = account_transactions(:checking_four)
+    outflow_transaction = account_transactions(:checking_five)
 
     assert inflow_transaction.amount < 0
     assert inflow_transaction.inflow?
@@ -35,8 +35,8 @@ class TransactionTest < ActiveSupport::TestCase
   end
 
   test "triggers sync with correct start date when transaction deleted" do
-    prior_transaction = transactions(:checking_two) # 12 days ago
-    current_transaction = transactions(:checking_one) # 5 days ago
+    prior_transaction = account_transactions(:checking_two) # 12 days ago
+    current_transaction = account_transactions(:checking_one) # 5 days ago
     current_transaction.destroy!
 
     current_transaction.account.expects(:sync_later).with(prior_transaction.date)

test/models/account/transfer_test.rb

@@ -15,7 +15,7 @@ class Account::TransferTest < ActiveSupport::TestCase
 
   test "transfer must have 2 transactions" do
     invalid_transfer_1 = Account::Transfer.new transactions: [ @outflow ]
-    invalid_transfer_2 = Account::Transfer.new transactions: [ @inflow, @outflow, transactions(:savings_four) ]
+    invalid_transfer_2 = Account::Transfer.new transactions: [ @inflow, @outflow, account_transactions(:savings_four) ]
 
     assert invalid_transfer_1.invalid?
     assert invalid_transfer_2.invalid?

test/models/account_test.rb

@@ -99,7 +99,7 @@ class AccountTest < ActiveSupport::TestCase
   end
 
   test "should destroy dependent transactions" do
-    assert_difference("Transaction.count", -@account.transactions.count) do
+    assert_difference("Account::Transaction.count", -@account.transactions.count) do
       @account.destroy
     end
   end

test/models/category_test.rb

@@ -34,7 +34,7 @@ class CategoryTest < ActiveSupport::TestCase
   end
 
   test "replacing and destroying" do
-    transctions = categories(:food_and_drink).transactions.to_a
+    transactions = categories(:food_and_drink).transactions.to_a
 
     categories(:food_and_drink).replace_and_destroy!(categories(:income))
 

test/models/import_test.rb

@@ -45,7 +45,7 @@ class ImportTest < ActiveSupport::TestCase
     # Fixtures already define "Food & Drink" and "Income", so these should not be created
     # "Shopping" is a new category, but should only be created 1x during import
     assert_difference \
-      -> { Transaction.count } => 4,
+      -> { Account::Transaction.count } => 4,
       -> { Category.count } => 1,
       -> { Tagging.count } => 4,
       -> { Tag.count } => 2 do
@@ -59,11 +59,11 @@ class ImportTest < ActiveSupport::TestCase
 
   test "publishes a valid import with missing data" do
     @empty_import.update! raw_csv_str: valid_csv_with_missing_data
-    assert_difference -> { Category.count } => 1, -> { Transaction.count } => 2 do
+    assert_difference -> { Category.count } => 1, -> { Account::Transaction.count } => 2 do
       @empty_import.publish
     end
 
-    assert_not_nil Transaction.find_sole_by(name: Import::FALLBACK_TRANSACTION_NAME)
+    assert_not_nil Account::Transaction.find_sole_by(name: Import::FALLBACK_TRANSACTION_NAME)
 
     @empty_import.reload
 
@@ -73,7 +73,7 @@ class ImportTest < ActiveSupport::TestCase
   test "failed publish results in error status" do
     @empty_import.update! raw_csv_str: valid_csv_with_invalid_values
 
-    assert_difference "Transaction.count", 0 do
+    assert_difference "Account::Transaction.count", 0 do
       @empty_import.publish
     end
 

test/system/settings_test.rb

@@ -14,7 +14,7 @@ class SettingsTest < ApplicationSystemTestCase
       [ "Tags", "Tags", tags_path ],
       [ "Categories", "Categories", categories_path ],
       [ "Merchants", "Merchants", merchants_path ],
-      [ "Rules", "Rules", transaction_rules_path ],
+      [ "Rules", "Rules", account_transaction_rules_path ],
       [ "Imports", "Imports", imports_path ],
       [ "What's New", "What's New", changelog_path ],
       [ "Feedback", "Feedback", feedback_path ],

test/system/transactions_test.rb

@@ -132,7 +132,7 @@ class TransactionsTest < ApplicationSystemTestCase
   private
 
     def number_of_transactions_on_page
-      page_size = 50
+      page_size = 10
 
       [ @user.family.transactions.where(transfer_id: nil).count, page_size ].min
     end

test/system/transfers_test.rb

@@ -33,8 +33,8 @@ class TransfersTest < ApplicationSystemTestCase
 
   test "can match 2 transactions and create a transfer" do
     transfer_date = Date.current
-    outflow = Transaction.create! name: "Outflow from savings account", date: transfer_date, account: accounts(:savings), amount: 100
-    inflow = Transaction.create! name: "Inflow to checking account", date: transfer_date, account: accounts(:checking), amount: -100
+    outflow = Account::Transaction.create! name: "Outflow from savings account", date: transfer_date, account: accounts(:savings), amount: 100
+    inflow = Account::Transaction.create! name: "Inflow to checking account", date: transfer_date, account: accounts(:checking), amount: -100
 
     visit transactions_url