Transactions cleanup (#817)

An overhaul and cleanup of the transactions feature including:

- Simplification of transactions search and filtering
- Consolidation of account sync logic after transaction change
- Split sidebar modal and modal into "drawer" and "modal" concepts
- Refactor of transaction partials and folder organization
- Cleanup turbo frames and streams for transaction updates, including new Transactions::RowsController for inline updates
- Refactored and added several integration and systems tests

test/application_system_test_case.rb

@@ -1,6 +1,10 @@
 require "test_helper"
 
 class ApplicationSystemTestCase < ActionDispatch::SystemTestCase
+  setup do
+    Capybara.default_max_wait_time = 5
+  end
+
   driven_by :selenium, using: ENV["CI"].present? ? :headless_chrome : :chrome, screen_size: [ 1400, 1400 ]
 
   private

test/controllers/transactions_controller_test.rb

@@ -4,12 +4,49 @@ class TransactionsControllerTest < ActionDispatch::IntegrationTest
   setup do
     sign_in @user = users(:family_admin)
     @transaction = transactions(:checking_one)
-    @account = @transaction.account
+    @recent_transactions = @user.family.transactions.ordered.limit(20).to_a
   end
 
-  test "should get index" do
+  test "should get paginated index with most recent transactions first" do
     get transactions_url
     assert_response :success
+
+    @recent_transactions.first(10).each do |transaction|
+      assert_dom "#" + dom_id(transaction), count: 1
+    end
+  end
+
+  test "transaction count represents filtered total" do
+    get transactions_url
+    assert_dom "#total-transactions", count: 1, text: @user.family.transactions.count.to_s
+
+    new_transaction = @user.family.accounts.first.transactions.create! \
+      name: "Transaction to search for",
+      date: Date.current,
+      amount: 0
+
+    get transactions_url(q: { search: new_transaction.name })
+
+    # Only finds 1 transaction that matches filter
+    assert_dom "#" + dom_id(new_transaction), count: 1
+    assert_dom "#total-transactions", count: 1, text: "1"
+  end
+
+  test "can navigate to paginated result" do
+    get transactions_url(page: 2)
+    assert_response :success
+
+    @recent_transactions[10, 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 = @user.family.transactions.ordered.last
+    get transactions_url(page: 9999999999)
+
+    assert_response :success
+    assert_dom "#" + dom_id(user_oldest_transaction), count: 1
   end
 
   test "should get new" do
@@ -24,35 +61,25 @@ class TransactionsControllerTest < ActionDispatch::IntegrationTest
   end
 
   test "should create transaction" do
-    name = "transaction_name"
+    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: { account_id: @transaction.account_id, amount: @transaction.amount, currency: @transaction.currency, date: @transaction.date, name: } }
+      post transactions_url, params: { transaction: transaction_params }
     end
 
+    assert_equal transaction_params[:amount].to_d, Transaction.order(created_at: :desc).first.amount
+    assert_equal flash[:notice], "New transaction created successfully"
+    assert_enqueued_with(job: AccountSyncJob)
     assert_redirected_to transactions_url
   end
 
-  test "create should sync account with correct start date" do
-    assert_enqueued_with(job: AccountSyncJob, args: [ @account, @transaction.date ]) do
-      post transactions_url, params: { transaction: { account_id: @transaction.account_id, amount: @transaction.amount, currency: @transaction.currency, date: @transaction.date, name: @transaction.name } }
-    end
-  end
-
-  test "creation preserves decimals" do
-    assert_difference("Transaction.count") do
-      post transactions_url, params: { transaction: {
-        nature: "expense",
-        account_id: @transaction.account_id,
-        amount: 123.45,
-        currency: @transaction.currency,
-        date: @transaction.date,
-        name: @transaction.name } }
-    end
-
-    assert_redirected_to transactions_url
-    assert_equal 123.45.to_d, Transaction.order(created_at: :desc).first.amount
-  end
-
   test "expenses are positive" do
     assert_difference("Transaction.count") do
       post transactions_url, params: { transaction: {
@@ -88,26 +115,19 @@ class TransactionsControllerTest < ActionDispatch::IntegrationTest
     assert_response :success
   end
 
-  test "should get edit" do
-    get edit_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 } }
+    patch transaction_url(@transaction), params: {
+      transaction: {
+        account_id: @transaction.account_id,
+        amount: @transaction.amount,
+        currency: @transaction.currency,
+        date: @transaction.date,
+        name: @transaction.name
+      }
+    }
+
     assert_redirected_to transaction_url(@transaction)
-  end
-
-  test "update should sync account with correct start date" do
-    new_date = @transaction.date - 1.day
-    assert_enqueued_with(job: AccountSyncJob, args: [ @account, new_date ]) do
-      patch transaction_url(@transaction), params: { transaction: { account_id: @transaction.account_id, amount: @transaction.amount, currency: @transaction.currency, date: new_date, name: @transaction.name } }
-    end
-
-    new_date = @transaction.reload.date + 1.day
-    assert_enqueued_with(job: AccountSyncJob, args: [ @account, @transaction.date ]) do
-      patch transaction_url(@transaction), params: { transaction: { account_id: @transaction.account_id, amount: @transaction.amount, currency: @transaction.currency, date: new_date, name: @transaction.name } }
-    end
+    assert_enqueued_with(job: AccountSyncJob)
   end
 
   test "should destroy transaction" do
@@ -116,17 +136,6 @@ class TransactionsControllerTest < ActionDispatch::IntegrationTest
     end
 
     assert_redirected_to transactions_url
-  end
-
-  test "destroy should sync account with correct start date" do
-    first, second = @transaction.account.transactions.order(:date).all
-
-    assert_enqueued_with(job: AccountSyncJob, args: [ @account, first.date ]) do
-      delete transaction_url(second)
-    end
-
-    assert_enqueued_with(job: AccountSyncJob, args: [ @account, nil ]) do
-      delete transaction_url(first)
-    end
+    assert_enqueued_with(job: AccountSyncJob)
   end
 end

test/models/account/syncable_test.rb

@@ -1,6 +1,18 @@
 require "test_helper"
 
 class Account::SyncableTest < ActiveSupport::TestCase
+  include ActiveJob::TestHelper
+
+  setup do
+    @account = accounts(:savings_with_valuation_overrides)
+  end
+
+  test "triggers sync job" do
+    assert_enqueued_with(job: AccountSyncJob, args: [ @account, Date.current ]) do
+      @account.sync_later(Date.current)
+    end
+  end
+
   test "account has no balances until synced" do
     account = accounts(:savings_with_valuation_overrides)
 

test/models/transaction_test.rb

@@ -1,14 +1,44 @@
 require "test_helper"
 
 class TransactionTest < ActiveSupport::TestCase
+  setup do
+    @transaction = transactions(:checking_one)
+  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)
 
     assert inflow_transaction.amount < 0
+    assert inflow_transaction.inflow?
+
     assert outflow_transaction.amount >= 0
-    assert Transaction.inflows.include? inflow_transaction
-    assert Transaction.outflows.include? outflow_transaction
+    assert outflow_transaction.outflow?
+  end
+
+  test "triggers sync with correct start date when transaction is set to prior date" do
+    prior_date = @transaction.date - 1
+    @transaction.update! date: prior_date
+
+    @transaction.account.expects(:sync_later).with(prior_date)
+    @transaction.sync_account_later
+  end
+
+  test "triggers sync with correct start date when transaction is set to future date" do
+    prior_date = @transaction.date
+    @transaction.update! date: @transaction.date + 1
+
+    @transaction.account.expects(:sync_later).with(prior_date)
+    @transaction.sync_account_later
+  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
+    current_transaction.destroy!
+
+    current_transaction.account.expects(:sync_later).with(prior_transaction.date)
+    current_transaction.sync_account_later
   end
 end

test/system/transactions_test.rb

@@ -0,0 +1,94 @@
+require "application_system_test_case"
+
+class TransactionsTest < ApplicationSystemTestCase
+  setup do
+    sign_in @user = users(:family_admin)
+
+    @test_category = @user.family.transaction_categories.create! name: "System Test Category"
+    @test_merchant = @user.family.transaction_merchants.create! name: "System Test Merchant"
+    @target_txn = @user.family.accounts.first.transactions.create! \
+      name: "Oldest transaction",
+      date: 10.years.ago.to_date,
+      category: @test_category,
+      merchant: @test_merchant,
+      amount: 100
+
+    visit transactions_url
+  end
+
+  test "can search for a transaction" do
+    assert_selector "h1", text: "Transactions"
+
+    within "form#transactions-search" do
+      fill_in "Search transactions by name", with: @target_txn.name
+    end
+
+    assert_selector "#" + dom_id(@target_txn), count: 1
+
+    within "#transaction-search-filters" do
+      assert_text @target_txn.name
+    end
+  end
+
+  test "can open filters and apply one or more" do
+    find("#transaction-filters-button").click
+
+    within "#transaction-filters-menu" do
+      check(@target_txn.account.name)
+      click_button "Category"
+      check(@test_category.name)
+      click_button "Apply"
+    end
+
+    assert_selector "#" + dom_id(@target_txn), count: 1
+
+    within "#transaction-search-filters" do
+      assert_text @target_txn.account.name
+      assert_text @target_txn.category.name
+    end
+  end
+
+  test "all filters work and empty state shows if no match" do
+    find("#transaction-filters-button").click
+
+    within "#transaction-filters-menu" do
+      click_button "Account"
+      check(@target_txn.account.name)
+
+      click_button "Date"
+      fill_in "q_start_date", with: 10.days.ago.to_date
+      fill_in "q_end_date", with: Date.current
+
+      click_button "Type"
+      assert_text "Filter by type coming soon..."
+
+      click_button "Amount"
+      assert_text "Filter by amount coming soon..."
+
+      click_button "Category"
+      check(@test_category.name)
+
+      click_button "Merchant"
+      check(@test_merchant.name)
+
+      click_button "Apply"
+    end
+
+    assert_text "No transactions found"
+
+    # Page reload doesn't affect results
+    visit current_url
+
+    assert_text "No transactions found"
+
+    within "ul#transaction-search-filters" do
+      find("li", text: @target_txn.account.name).first("a").click
+      find("li", text: "on or after #{10.days.ago.to_date}").first("a").click
+      find("li", text: "on or before #{Date.current}").first("a").click
+      find("li", text: @target_txn.category.name).first("a").click
+      find("li", text: @target_txn.merchant.name).first("a").click
+    end
+
+    assert_selector "#" + dom_id(@user.family.transactions.ordered.first), count: 1
+  end
+end

test/test_helper.rb

@@ -33,7 +33,7 @@ end
 module ActiveSupport
   class TestCase
     # Run tests in parallel with specified workers
-    parallelize(workers: :number_of_processors)
+    parallelize(workers: :number_of_processors) unless ENV["DISABLE_PARALLELIZATION"]
 
     # https://github.com/simplecov-ruby/simplecov/issues/718#issuecomment-538201587
     if ENV["COVERAGE"]