Account namespace updates: part 4 (transfers, singular namespacing) (#896)

* Move Transfer to Account namespace

* Fix partial resolution due to namespacing plurality

* Make category and tag controllers consistent with namespacing convention

* Update stale partial reference

app/controllers/account/logos_controller.rb

@@ -1,4 +1,4 @@
-class Accounts::LogosController < ApplicationController
+class Account::LogosController < ApplicationController
   def show
     @account = Current.family.accounts.find(params[:account_id])
     render_placeholder

app/controllers/account/transfers_controller.rb

@@ -1,17 +1,17 @@
-class TransfersController < ApplicationController
+class Account::TransfersController < ApplicationController
   layout "with_sidebar"
 
   before_action :set_transfer, only: :destroy
 
   def new
-    @transfer = Transfer.new
+    @transfer = Account::Transfer.new
   end
 
   def create
     from_account = Current.family.accounts.find(transfer_params[:from_account_id])
     to_account = Current.family.accounts.find(transfer_params[:to_account_id])
 
-    @transfer = Transfer.build_from_accounts from_account, to_account, \
+    @transfer = Account::Transfer.build_from_accounts from_account, to_account, \
                                              date: transfer_params[:date],
                                              amount: transfer_params[:amount].to_d,
                                              currency: transfer_params[:currency],
@@ -20,7 +20,10 @@ class TransfersController < ApplicationController
     if @transfer.save
       redirect_to transactions_path, notice: t(".success")
     else
-      render :new, status: :unprocessable_entity
+      # TODO: this is not an ideal way to handle errors and should eventually be improved.
+      # See: https://github.com/hotwired/turbo-rails/pull/367
+      flash[:error] = @transfer.errors.full_messages.to_sentence
+      redirect_to transactions_path
     end
   end
 
@@ -32,10 +35,10 @@ class TransfersController < ApplicationController
   private
 
     def set_transfer
-      @transfer = Transfer.find(params[:id])
+      @transfer = Account::Transfer.find(params[:id])
     end
 
     def transfer_params
-      params.require(:transfer).permit(:from_account_id, :to_account_id, :amount, :currency, :date, :name)
+      params.require(:account_transfer).permit(:from_account_id, :to_account_id, :amount, :currency, :date, :name)
     end
 end

app/controllers/category/deletions_controller.rb

@@ -1,4 +1,4 @@
-class Categories::DeletionsController < ApplicationController
+class Category::DeletionsController < ApplicationController
   layout "with_sidebar"
 
   before_action :set_category

app/controllers/category/dropdowns_controller.rb

@@ -1,4 +1,4 @@
-class Categories::DropdownsController < ApplicationController
+class Category::DropdownsController < ApplicationController
   before_action :set_from_params
 
   def show

app/controllers/tag/deletions_controller.rb

@@ -1,4 +1,4 @@
-class Tags::DeletionsController < ApplicationController
+class Tag::DeletionsController < ApplicationController
   layout "with_sidebar"
 
   before_action :set_tag

app/controllers/transaction/rows_controller.rb

@@ -1,4 +1,4 @@
-class Transactions::RowsController < ApplicationController
+class Transaction::RowsController < ApplicationController
   before_action :set_transaction, only: %i[ show update ]
 
   def show

app/controllers/transaction/rules_controller.rb

@@ -0,0 +1,6 @@
+class Transaction::RulesController < ApplicationController
+  layout "with_sidebar"
+
+  def index
+  end
+end

app/controllers/transactions/rules_controller.rb

@@ -1,6 +0,0 @@
-class Transactions::RulesController < ApplicationController
-  layout "with_sidebar"
-
-  def index
-  end
-end

app/models/account/transfer.rb

@@ -1,4 +1,4 @@
-class Transfer < ApplicationRecord
+class Account::Transfer < ApplicationRecord
   has_many :transactions, dependent: :nullify
 
   validate :transaction_count, :from_different_accounts, :net_zero_flows, :all_transactions_marked

app/models/transaction.rb

@@ -4,7 +4,7 @@ class Transaction < ApplicationRecord
   monetize :amount
 
   belongs_to :account
-  belongs_to :transfer, optional: true
+  belongs_to :transfer, optional: true, class_name: "Account::Transfer"
   belongs_to :category, optional: true
   belongs_to :merchant, optional: true
   has_many :taggings, as: :taggable, dependent: :destroy
@@ -70,7 +70,7 @@ class Transaction < ApplicationRecord
       update_all marked_as_transfer: true
 
       # Attempt to "auto match" and save a transfer if 2 transactions selected
-      Transfer.new(transactions: all).save if all.count == 2
+      Account::Transfer.new(transactions: all).save if all.count == 2
     end
 
     def daily_totals(transactions, period: Period.last_30_days, currency: Current.family.currency)

app/views/account/transfers/_form.html.erb

@@ -1,4 +1,4 @@
-<%= form_with model: transfer do |f| %>
+<%= form_with model: transfer, data: { turbo_frame: "_top" } do |f| %>
   <section>
     <fieldset class="bg-gray-50 rounded-lg p-1 grid grid-flow-col justify-stretch gap-x-2">
       <%= link_to new_transaction_path(nature: "expense"), data: { turbo_frame: :modal }, class: "flex px-4 py-1 rounded-lg items-center space-x-2 justify-center text-gray-400" do %>

app/views/account/transfers/_transfer.html.erb

@@ -2,7 +2,7 @@
   <details class="group flex items-center text-gray-900 p-4 text-sm font-medium">
     <summary class="flex items-center justify-between">
       <div class="flex items-center gap-4">
-        <%= button_to transfer_path(transfer),
+        <%= button_to account_transfer_path(transfer),
                       method: :delete,
                       class: "flex items-center group/transfer",
                       data: {

app/views/accounts/new.html.erb

@@ -78,7 +78,7 @@
           <%= f.hidden_field :accountable_type %>
           <%= f.text_field :name, placeholder: t(".name.placeholder"), required: "required", label: t(".name.label"), autofocus: true %>
           <%= f.collection_select :institution_id, Current.family.institutions.alphabetically, :id, :name, { include_blank: t(".ungrouped"), label: t(".institution") } %>
-          <%= render "accounts/account/#{permitted_accountable_partial(@account.accountable_type)}", f: f %>
+          <%= render "accounts/accountables/#{permitted_accountable_partial(@account.accountable_type)}", f: f %>
           <%= f.money_field :balance_money, label: t(".balance"), required: "required" %>
 
           <div>

app/views/category/dropdowns/show.html.erb

@@ -11,7 +11,7 @@
         <%= t(".no_categories") %>
       </div>
       <% @categories.each do |category| %>
-        <%= render partial: "categories/dropdowns/row", locals: { category: } %>
+        <%= render partial: "category/dropdowns/row", locals: { category: } %>
       <% end %>
     </div>
     <hr>

app/views/transactions/_date_group.html.erb

@@ -1,5 +1,5 @@
 <%# locals: (date:, group:) %>
-<div class="bg-gray-25 rounded-xl p-1 w-full" data-bulk-select-target="group">
+<div id="date-group-<%= date %>" class="bg-gray-25 rounded-xl p-1 w-full" data-bulk-select-target="group">
   <div class="py-2 px-4 flex items-center justify-between font-medium text-xs text-gray-500">
     <div class="flex pl-0.5 items-center gap-4">
       <%= check_box_tag "#{date}_transactions_selection",

app/views/transactions/_form.html.erb

@@ -3,7 +3,7 @@
     <fieldset class="bg-gray-50 rounded-lg p-1 grid grid-flow-col justify-stretch gap-x-2">
       <%= radio_tab_tag form: f, name: :nature, value: :expense, label: t(".expense"), icon: "minus-circle", checked: params[:nature] == "expense" || params[:nature].nil? %>
       <%= radio_tab_tag form: f, name: :nature, value: :income, label: t(".income"), icon: "plus-circle", checked: params[:nature] == "income" %>
-      <%= link_to new_transfer_path, data: { turbo_frame: :modal }, class: "flex px-4 py-1 rounded-lg items-center space-x-2 justify-center text-gray-400 group-has-[:checked]:bg-white group-has-[:checked]:text-gray-800 group-has-[:checked]:shadow-sm" do %>
+      <%= link_to new_account_transfer_path, data: { turbo_frame: :modal }, class: "flex px-4 py-1 rounded-lg items-center space-x-2 justify-center text-gray-400 group-has-[:checked]:bg-white group-has-[:checked]:text-gray-800 group-has-[:checked]:shadow-sm" do %>
         <%= lucide_icon "arrow-right-left", class: "w-5 h-5" %>
         <%= tag.span t(".transfer") %>
       <% end %>

app/views/transactions/_selection_bar.html.erb

@@ -18,7 +18,7 @@
                       accept: t(".mark_transfers_confirm"),
                     }
                   } do |f| %>
-      <button type="button" data-bulk-select-scope-param="bulk_update" data-action="bulk-select#submitBulkRequest" class="p-1.5 group hover:bg-gray-700 flex items-center justify-center rounded-md" title="Mark as transfer">
+      <button id="bulk-transfer-btn" type="button" data-bulk-select-scope-param="bulk_update" data-action="bulk-select#submitBulkRequest" class="p-1.5 group hover:bg-gray-700 flex items-center justify-center rounded-md" title="Mark as transfer">
         <%= lucide_icon "arrow-right-left", class: "w-5 group-hover:text-white" %>
       </button>
     <% end %>

config/locales/views/account/transfers/en.yml

@@ -0,0 +1,28 @@
+---
+en:
+  account:
+    transfers:
+      create:
+        success: Transfer created
+      destroy:
+        success: Transfer removed
+      form:
+        amount: Amount
+        date: Date
+        description: Description
+        description_placeholder: Transfer from Checking to Savings
+        expense: Expense
+        from: From
+        income: Income
+        select_account: Select account
+        submit: Create transfer
+        to: To
+        transfer: Transfer
+      new:
+        title: New transfer
+      transfer:
+        remove_body: This will NOT delete the underlying transactions.  It will just
+          remove the transfer.
+        remove_confirm: Confirm
+        remove_title: Remove transfer?
+        transfer_name: Transfer from %{from_account} to %{to_account}

config/locales/views/categories/en.yml

@@ -3,27 +3,6 @@ en:
   categories:
     create:
       success: New transaction category created successfully
-    deletions:
-      create:
-        success: Transaction category deleted successfully
-      new:
-        category: Category
-        delete_and_leave_uncategorized: Delete "%{category_name}" and leave uncategorized
-        delete_and_recategorize: Delete "%{category_name}" and assign new category
-        delete_category: Delete category?
-        explanation: By deleting this category, every transaction that has the "%{category_name}"
-          category will be uncategorized. Instead of leaving them uncategorized, you
-          can also assign a new category below.
-        replacement_category_prompt: Select category
-    dropdowns:
-      row:
-        delete: Delete category
-        edit: Edit category
-      show:
-        add_new: Add new
-        clear: Clear
-        no_categories: No categories found
-        search_placeholder: Search
     edit:
       edit: Edit category
     form:

config/locales/views/category/deletions/en.yml

@@ -0,0 +1,15 @@
+---
+en:
+  category:
+    deletions:
+      create:
+        success: Transaction category deleted successfully
+      new:
+        category: Category
+        delete_and_leave_uncategorized: Delete "%{category_name}" and leave uncategorized
+        delete_and_recategorize: Delete "%{category_name}" and assign new category
+        delete_category: Delete category?
+        explanation: By deleting this category, every transaction that has the "%{category_name}"
+          category will be uncategorized. Instead of leaving them uncategorized, you
+          can also assign a new category below.
+        replacement_category_prompt: Select category

config/locales/views/category/dropdowns/en.yml

@@ -0,0 +1,12 @@
+---
+en:
+  category:
+    dropdowns:
+      row:
+        delete: Delete category
+        edit: Edit category
+      show:
+        add_new: Add new
+        clear: Clear
+        no_categories: No categories found
+        search_placeholder: Search

config/locales/views/tag/deletions/en.yml

@@ -0,0 +1,15 @@
+---
+en:
+  tag:
+    deletions:
+      create:
+        deleted: Tag deleted
+      new:
+        delete_and_leave_uncategorized: Delete "%{tag_name}"
+        delete_and_recategorize: Delete "%{tag_name}" and assign new tag
+        delete_tag: Delete tag?
+        explanation: "%{tag_name} will be removed from transactions and other taggable
+          entities.  Instead of leaving them untagged, you can also assign a new tag
+          below."
+        replacement_tag_prompt: Select tag
+        tag: Tag

config/locales/views/tags/en.yml

@@ -3,18 +3,6 @@ en:
   tags:
     create:
       created: Tag created
-    deletions:
-      create:
-        deleted: Tag deleted
-      new:
-        delete_and_leave_uncategorized: Delete "%{tag_name}"
-        delete_and_recategorize: Delete "%{tag_name}" and assign new tag
-        delete_tag: Delete tag?
-        explanation: "%{tag_name} will be removed from transactions and other taggable
-          entities.  Instead of leaving them untagged, you can also assign a new tag
-          below."
-        replacement_tag_prompt: Select tag
-        tag: Tag
     edit:
       edit: Edit tag
     form:

config/locales/views/transfers/en.yml

@@ -1,27 +0,0 @@
----
-en:
-  transfers:
-    create:
-      success: Transfer created
-    destroy:
-      success: Transfer removed
-    form:
-      amount: Amount
-      date: Date
-      description: Description
-      description_placeholder: Transfer from Checking to Savings
-      expense: Expense
-      from: From
-      income: Income
-      select_account: Select account
-      submit: Create transfer
-      to: To
-      transfer: Transfer
-    new:
-      title: New transfer
-    transfer:
-      remove_body: This will NOT delete the underlying transactions.  It will just
-        remove the transfer.
-      remove_confirm: Confirm
-      remove_title: Remove transfer?
-      transfer_name: Transfer from %{from_account} to %{to_account}

config/routes.rb

@@ -38,18 +38,24 @@ Rails.application.routes.draw do
   end
 
   resources :tags, except: %i[ show destroy ] do
-    resources :deletions, only: %i[ new create ], module: :tags
+    resources :deletions, only: %i[ new create ], module: :tag
+  end
+
+  namespace :category do
+    resource :dropdown, only: :show
   end
 
   resources :categories do
-    resources :deletions, only: %i[ new create ], module: :categories
+    resources :deletions, only: %i[ new create ], module: :category
-    collection do
-      resource :dropdown, only: :show, module: :categories, as: :category_dropdown
-    end
   end
 
   resources :merchants, only: %i[ index new create edit update destroy ]
 
+  namespace :transaction do
+    resources :rows, only: %i[ show update ]
+    resources :rules, only: %i[ index ]
+  end
+
   resources :transactions do
     collection do
       post "bulk_delete"
@@ -57,17 +63,14 @@ Rails.application.routes.draw do
       post "bulk_update"
       post "mark_transfers"
       post "unmark_transfers"
-
-      scope module: :transactions, as: :transaction do
-        resources :rows, only: %i[ show update ]
-        resources :rules, only: %i[ index ]
-      end
     end
   end
 
-  resources :transfers, only: %i[ new create destroy ]
+  namespace :account do
+    resources :transfers, only: %i[ new create destroy ]
+  end
 
-  resources :accounts, shallow: true do
+  resources :accounts do
     collection do
       get :summary
       get :list
@@ -77,11 +80,8 @@ Rails.application.routes.draw do
       post :sync
     end
 
-    scope module: :accounts do
+    resource :logo, only: :show, module: :account
-      resource :logo, only: :show
+    resources :valuations, shallow: true
-    end
-
-    resources :valuations
   end
 
   resources :institutions, except: %i[ index show ]

db/migrate/20240620125026_rename_transfer_table.rb

@@ -0,0 +1,5 @@
+class RenameTransferTable < ActiveRecord::Migration[7.2]
+  def change
+    rename_table :transfers, :account_transfers
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.2].define(version: 2024_06_20_122201) do
+ActiveRecord::Schema[7.2].define(version: 2024_06_20_125026) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "pgcrypto"
   enable_extension "plpgsql"
@@ -32,6 +32,11 @@ ActiveRecord::Schema[7.2].define(version: 2024_06_20_122201) do
     t.index ["account_id"], name: "index_account_balances_on_account_id"
   end
 
+  create_table "account_transfers", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
+  end
+
   create_table "accounts", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
     t.string "subtype"
     t.uuid "family_id", null: false
@@ -313,11 +318,6 @@ ActiveRecord::Schema[7.2].define(version: 2024_06_20_122201) do
     t.index ["transfer_id"], name: "index_transactions_on_transfer_id"
   end
 
-  create_table "transfers", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
-    t.datetime "created_at", null: false
-    t.datetime "updated_at", null: false
-  end
-
   create_table "users", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
     t.uuid "family_id", null: false
     t.string "first_name"
@@ -362,10 +362,10 @@ ActiveRecord::Schema[7.2].define(version: 2024_06_20_122201) do
   add_foreign_key "merchants", "families"
   add_foreign_key "taggings", "tags"
   add_foreign_key "tags", "families"
+  add_foreign_key "transactions", "account_transfers", column: "transfer_id"
   add_foreign_key "transactions", "accounts", on_delete: :cascade
   add_foreign_key "transactions", "categories", on_delete: :nullify
   add_foreign_key "transactions", "merchants"
-  add_foreign_key "transactions", "transfers"
   add_foreign_key "users", "families"
   add_foreign_key "valuations", "accounts", on_delete: :cascade
 end

test/controllers/account/transfers_controller_test.rb

@@ -1,19 +1,19 @@
 require "test_helper"
 
-class TransfersControllerTest < ActionDispatch::IntegrationTest
+class Account::TransfersControllerTest < ActionDispatch::IntegrationTest
   setup do
     sign_in users(:family_admin)
   end
 
   test "should get new" do
-    get new_transfer_url
+    get new_account_transfer_url
     assert_response :success
   end
 
   test "can create transfers" do
-    assert_difference "Transfer.count", 1 do
+    assert_difference "Account::Transfer.count", 1 do
-      post transfers_url, params: {
+      post account_transfers_url, params: {
-        transfer: {
+        account_transfer: {
           from_account_id: accounts(:checking).id,
           to_account_id: accounts(:savings).id,
           date: Date.current,
@@ -26,8 +26,8 @@ class TransfersControllerTest < ActionDispatch::IntegrationTest
   end
 
   test "can destroy transfer" do
-    assert_difference -> { Transfer.count } => -1, -> { Transaction.count } => 0 do
+    assert_difference -> { Account::Transfer.count } => -1, -> { Transaction.count } => 0 do
-      delete transfer_url(transfers(:credit_card_payment))
+      delete account_transfer_url(account_transfers(:credit_card_payment))
     end
   end
 end

test/controllers/category/deletions_controller_test.rb

@@ -1,6 +1,6 @@
 require "test_helper"
 
-class Categories::DeletionsControllerTest < ActionDispatch::IntegrationTest
+class Category::DeletionsControllerTest < ActionDispatch::IntegrationTest
   setup do
     sign_in users(:family_admin)
     @category = categories(:food_and_drink)

test/controllers/tag/deletions_controller_test.rb

@@ -1,6 +1,6 @@
 require "test_helper"
 
-class Tags::DeletionsControllerTest < ActionDispatch::IntegrationTest
+class Tag::DeletionsControllerTest < ActionDispatch::IntegrationTest
   setup do
     sign_in @user = users(:family_admin)
     @user_tags = @user.family.tags

test/fixtures/accounts.yml

@@ -22,7 +22,7 @@ checking:
 
 savings:
   family: dylan_family
-  name: Savings account with valuation overrides
+  name: Savings account
   balance: 19700
   accountable_type: Depository
   accountable: depository_savings

test/models/transfer_test.rb

@@ -8,14 +8,14 @@ class TransferTest < ActiveSupport::TestCase
   end
 
   test "transfer valid if it has inflow and outflow from different accounts for the same amount" do
-    transfer = Transfer.create! transactions: [ @inflow, @outflow ]
+    transfer = Account::Transfer.create! transactions: [ @inflow, @outflow ]
 
     assert transfer.valid?
   end
 
   test "transfer must have 2 transactions" do
-    invalid_transfer_1 = Transfer.new transactions: [ @outflow ]
+    invalid_transfer_1 = Account::Transfer.new transactions: [ @outflow ]
-    invalid_transfer_2 = Transfer.new transactions: [ @inflow, @outflow, transactions(:savings_four) ]
+    invalid_transfer_2 = Account::Transfer.new transactions: [ @inflow, @outflow, transactions(:savings_four) ]
 
     assert invalid_transfer_1.invalid?
     assert invalid_transfer_2.invalid?
@@ -27,7 +27,7 @@ class TransferTest < ActiveSupport::TestCase
     outflow = account.transactions.create! date: Date.current, name: "Outflow", amount: 100
 
     assert_raise ActiveRecord::RecordInvalid do
-      Transfer.create! transactions: [ inflow, outflow ]
+      Account::Transfer.create! transactions: [ inflow, outflow ]
     end
   end
 
@@ -35,7 +35,7 @@ class TransferTest < ActiveSupport::TestCase
     @inflow.update! marked_as_transfer: false
 
     assert_raise ActiveRecord::RecordInvalid do
-      Transfer.create! transactions: [ @inflow, @outflow ]
+      Account::Transfer.create! transactions: [ @inflow, @outflow ]
     end
   end
 
@@ -43,7 +43,7 @@ class TransferTest < ActiveSupport::TestCase
     @outflow.update! amount: 105
 
     assert_raises ActiveRecord::RecordInvalid do
-      Transfer.create! transactions: [ @inflow, @outflow ]
+      Account::Transfer.create! transactions: [ @inflow, @outflow ]
     end
   end
 end

test/system/transfers_test.rb

@@ -0,0 +1,82 @@
+require "application_system_test_case"
+
+class TransfersTest < ApplicationSystemTestCase
+  setup do
+    sign_in @user = users(:family_admin)
+    visit transactions_url
+  end
+
+  test "can create a transfer" do
+    checking_name = accounts(:checking).name
+    savings_name = accounts(:savings).name
+    transfer_date = Date.current
+
+    click_on "New transaction"
+
+    # Will navigate to different route in same modal
+    click_on "Transfer"
+    assert_text "New transfer"
+
+    fill_in "Description", with: "Transfer txn name"
+    select checking_name, from: "From"
+    select savings_name, from: "To"
+    fill_in "account_transfer[amount]", with: 500
+    fill_in "Date", with: transfer_date
+    click_button "Create transfer"
+
+    within "#date-group-" + transfer_date.to_s do
+      transfer_name = "Transfer from #{checking_name} to #{savings_name}"
+      find("details", text: transfer_name).click
+      assert_text "Transfer txn name", count: 2
+    end
+  end
+
+  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
+
+    visit transactions_url
+
+    transaction_checkbox(inflow).check
+    transaction_checkbox(outflow).check
+
+    bulk_transfer_action_button.click
+
+    click_on "Mark as transfers"
+
+    within "#date-group-" + transfer_date.to_s do
+      transfer_name = "Transfer from #{outflow.account.name} to #{inflow.account.name}"
+      find("details", text: transfer_name).click
+      assert_text inflow.name
+      assert_text outflow.name
+    end
+  end
+
+  test "can mark a single transaction as a transfer" do
+    txn = @user.family.transactions.ordered.first
+
+    within "#" + dom_id(txn) do
+      assert_text "Uncategorized"
+    end
+
+    transaction_checkbox(txn).check
+
+    bulk_transfer_action_button.click
+    click_on "Mark as transfers"
+
+    within "#" + dom_id(txn) do
+      assert_no_text "Uncategorized"
+    end
+  end
+
+  private
+
+    def transaction_checkbox(transaction)
+      find("#" + dom_id(transaction, "selection"))
+    end
+
+    def bulk_transfer_action_button
+      find("#bulk-transfer-btn")
+    end
+end