fix(accounts): preserve closed-account financial history

app/controllers/api/v1/accounts_controller.rb

@@ -63,10 +63,10 @@ def accounts_scope
       scope = current_resource_owner.family.accounts
                                     .accessible_by(current_resource_owner)
                                     .includes(:accountable, account_providers: :provider)
-      include_disabled_accounts? ? scope : scope.visible
+      include_disabled_accounts? ? scope.historical : scope.visible
     end
 
     def include_disabled_accounts?
-      ActiveModel::Type::Boolean.new.cast(params[:include_disabled])
+      ActiveModel::Type::Boolean.new.cast(params[:include_disabled]) || false
     end
 end

app/controllers/api/v1/balance_sheet_controller.rb

@@ -9,10 +9,14 @@ class Api::V1::BalanceSheetController < Api::V1::BaseController
   # Returns net worth, total assets, and total liabilities as Money objects.
   def show
     family = current_resource_owner.family
-    balance_sheet = family.balance_sheet
+    balance_sheet = family.balance_sheet(
+      user: current_resource_owner,
+      include_disabled: include_disabled_accounts?
+    )
 
     render json: {
       currency: family.currency,
+      include_disabled: include_disabled_accounts?,
       net_worth: balance_sheet.net_worth_money.as_json,
       assets: balance_sheet.assets.total_money.as_json,
       liabilities: balance_sheet.liabilities.total_money.as_json
@@ -24,4 +28,10 @@ def show
     def ensure_read_scope
       authorize_scope!(:read)
     end
+
+    def include_disabled_accounts?
+      return @include_disabled_accounts if defined?(@include_disabled_accounts)
+
+      @include_disabled_accounts = ActiveModel::Type::Boolean.new.cast(params[:include_disabled]) || false
+    end
 end

app/controllers/api/v1/balances_controller.rb

@@ -50,7 +50,10 @@ def balances_scope
     end
 
     def accessible_account_ids
-      @accessible_account_ids ||= current_resource_owner.family.accounts.accessible_by(current_resource_owner).select(:id)
+      @accessible_account_ids ||= current_resource_owner.family.accounts
+                                        .accessible_by(current_resource_owner)
+                                        .historical
+                                        .select(:id)
     end
 
     def apply_filters(query)

app/controllers/api/v1/holdings_controller.rb

@@ -7,8 +7,7 @@ class Api::V1::HoldingsController < Api::V1::BaseController
   before_action :set_holding, only: [ :show ]
 
   def index
-    family = current_resource_owner.family
-    holdings_query = family.holdings.joins(:account).where(accounts: { status: [ "draft", "active" ] })
+    holdings_query = holding_history_scope
 
     holdings_query = apply_filters(holdings_query)
     holdings_query = holdings_query.includes(:account, :security).chronological
@@ -21,6 +20,8 @@ def index
     @per_page = safe_per_page_param
 
     render :index
+  rescue InvalidFilterError => e
+    render_validation_error(e.message, [ e.message ])
   rescue ArgumentError => e
     render_validation_error(e.message, [ e.message ])
   rescue => e
@@ -36,8 +37,9 @@ def show
   private
 
     def set_holding
-      family = current_resource_owner.family
-      @holding = family.holdings.joins(:account).where(accounts: { status: %w[draft active] }).find(params[:id])
+      raise ActiveRecord::RecordNotFound unless valid_uuid?(params[:id])
+
+      @holding = holding_history_scope.find(params[:id])
     rescue ActiveRecord::RecordNotFound
       render json: { error: "not_found", message: "Holding not found" }, status: :not_found
     end
@@ -46,12 +48,30 @@ def ensure_read_scope
       authorize_scope!(:read)
     end
 
+    def holding_history_scope
+      account_ids = current_resource_owner.family.accounts
+        .accessible_by(current_resource_owner)
+        .historical
+        .select(:id)
+
+      current_resource_owner.family.holdings
+        .joins(:account)
+        .where(accounts: { id: account_ids })
+    end
+
     def apply_filters(query)
       if params[:account_id].present?
+        raise InvalidFilterError, "account_id must be a valid UUID" unless valid_uuid?(params[:account_id])
+
         query = query.where(account_id: params[:account_id])
       end
       if params[:account_ids].present?
-        query = query.where(account_id: Array(params[:account_ids]))
+        account_ids = Array(params[:account_ids])
+        unless account_ids.all? { |account_id| valid_uuid?(account_id) }
+          raise InvalidFilterError, "account_ids must contain valid UUIDs"
+        end
+
+        query = query.where(account_id: account_ids)
       end
       if params[:date].present?
         query = query.where(date: parse_date!(params[:date], "date"))

app/controllers/api/v1/trades_controller.rb

@@ -5,11 +5,11 @@ class Api::V1::TradesController < Api::V1::BaseController
 
   before_action :ensure_read_scope, only: [ :index, :show ]
   before_action :ensure_write_scope, only: [ :create, :update, :destroy ]
-  before_action :set_trade, only: [ :show, :update, :destroy ]
+  before_action :set_readable_trade, only: :show
+  before_action :set_writable_trade, only: [ :update, :destroy ]
 
   def index
-    family = current_resource_owner.family
-    trades_query = family.trades.visible
+    trades_query = trade_history_scope
 
     trades_query = apply_filters(trades_query)
     trades_query = trades_query.includes({ entry: :account }, :security, :category).reverse_chronological
@@ -22,6 +22,8 @@ def index
     @per_page = safe_per_page_param
 
     render :index
+  rescue InvalidFilterError => e
+    render_validation_error(e.message, [ e.message ])
   rescue ArgumentError => e
     render_validation_error(e.message, [ e.message ])
   rescue => e
@@ -39,7 +41,10 @@ def create
       return render_validation_error("Account ID is required", [ "Account ID is required" ])
     end
 
-    account = current_resource_owner.family.accounts.visible.find(trade_params[:account_id])
+    account = current_resource_owner.family.accounts
+      .writable_by(current_resource_owner)
+      .visible
+      .find(trade_params[:account_id])
 
     unless account.supports_trades?
       return render_validation_error(
@@ -75,6 +80,8 @@ def create
   rescue ActiveRecord::RecordNotFound => e
     message = (e.model == "Account") ? "Account not found" : "Security not found"
     render json: { error: "not_found", message: message }, status: :not_found
+  rescue ArgumentError => e
+    render_validation_error(e.message, [ e.message ])
   rescue => e
     log_and_render_error("create", e)
   end
@@ -91,6 +98,8 @@ def update
     else
       render_validation_error("Trade could not be updated", @entry.errors.full_messages)
     end
+  rescue ArgumentError => e
+    render_validation_error(e.message, [ e.message ])
   rescue => e
     log_and_render_error("update", e)
   end
@@ -107,14 +116,34 @@ def destroy
 
   private
 
-    def set_trade
-      family = current_resource_owner.family
-      @trade = family.trades.visible.find(params[:id])
+    def set_readable_trade
+      load_trade_with_account_scope(readable_trade_account_scope)
+    end
+
+    def set_writable_trade
+      load_trade_with_account_scope(writable_trade_account_scope)
+    end
+
+    def load_trade_with_account_scope(account_scope)
+      raise ActiveRecord::RecordNotFound unless valid_uuid?(params[:id])
+
+      @trade = current_resource_owner.family.trades
+        .joins(entry: :account)
+        .merge(account_scope)
+        .find(params[:id])
       @entry = @trade.entry
     rescue ActiveRecord::RecordNotFound
       render json: { error: "not_found", message: "Trade not found" }, status: :not_found
     end
 
+    def readable_trade_account_scope
+      Account.accessible_by(current_resource_owner).merge(Account.historical)
+    end
+
+    def writable_trade_account_scope
+      Account.writable_by(current_resource_owner).merge(Account.visible)
+    end
+
     def ensure_read_scope
       authorize_scope!(:read)
     end
@@ -123,16 +152,34 @@ def ensure_write_scope
       authorize_scope!(:write)
     end
 
+    def trade_history_scope
+      account_ids = current_resource_owner.family.accounts
+        .accessible_by(current_resource_owner)
+        .historical
+        .select(:id)
+
+      current_resource_owner.family.trades
+        .joins(entry: :account)
+        .where(entries: { account_id: account_ids })
+    end
+
     def apply_filters(query)
       need_entry_join = params[:account_id].present? || params[:account_ids].present? ||
                         params[:start_date].present? || params[:end_date].present?
       query = query.joins(:entry) if need_entry_join
 
       if params[:account_id].present?
+        raise InvalidFilterError, "account_id must be a valid UUID" unless valid_uuid?(params[:account_id])
+
         query = query.where(entries: { account_id: params[:account_id] })
       end
       if params[:account_ids].present?
-        query = query.where(entries: { account_id: Array(params[:account_ids]) })
+        account_ids = Array(params[:account_ids])
+        unless account_ids.all? { |account_id| valid_uuid?(account_id) }
+          raise InvalidFilterError, "account_ids must contain valid UUIDs"
+        end
+
+        query = query.where(entries: { account_id: account_ids })
       end
       if params[:start_date].present?
         query = query.where("entries.date >= ?", parse_date!(params[:start_date], "start_date"))
@@ -161,7 +208,6 @@ def build_entry_params_for_update
       flat = trade_update_params.to_h
       entry_params = {
         name: flat[:name],
-        date: flat[:date],
         amount: flat[:amount],
         currency: flat[:currency],
         notes: flat[:notes],
@@ -172,6 +218,7 @@ def build_entry_params_for_update
           category_id: flat[:category_id]
         }.compact_blank
       }.compact
+      entry_params[:date] = parse_date!(flat[:date], "date") if flat[:date].present?
 
       original_qty = flat[:qty]
       original_price = flat[:price]
@@ -183,6 +230,7 @@ def build_entry_params_for_update
         is_sell = type_or_nature.present? ? trade_sell_from_type_or_nature?(type_or_nature) : @trade.qty.negative?
         signed_qty = is_sell ? -qty.to_d.abs : qty.to_d.abs
         entry_params[:entryable_attributes][:qty] = signed_qty
+        entry_params[:entryable_attributes][:price] = price.to_d if original_price.present?
         entry_params[:amount] = signed_qty * price.to_d
         ticker = @trade.security&.ticker
         entry_params[:name] = Trade.build_name(is_sell ? "sell" : "buy", signed_qty.abs, ticker) if ticker.present?
@@ -242,7 +290,7 @@ def build_create_form_params(account)
 
       {
         account: account,
-        date: trade_params[:date],
+        date: parse_date!(trade_params[:date], "date"),
         qty: qty,
         price: price,
         currency: trade_params[:currency].presence || account.currency,

app/controllers/api/v1/transactions_controller.rb

@@ -6,13 +6,14 @@ class Api::V1::TransactionsController < Api::V1::BaseController
   # Ensure proper scope authorization for read vs write access
   before_action :ensure_read_scope, only: [ :index, :show ]
   before_action :ensure_write_scope, only: [ :create, :update, :destroy ]
-  before_action :set_transaction, only: [ :show, :update, :destroy ]
+  before_action :set_readable_transaction, only: [ :show ]
+  before_action :set_writable_transaction, only: [ :update, :destroy ]
 
   def index
     family = current_resource_owner.family
     accessible_account_ids = family.accounts
       .accessible_by(current_resource_owner)
-      .where.not(status: "pending_deletion")
+      .historical
       .select(:id)
     transactions_query = family.transactions
       .joins(:entry).where(entries: { account_id: accessible_account_ids })
@@ -90,7 +91,7 @@ def create
       return
     end
 
-    account = family.accounts.writable_by(current_resource_owner).find(account_id_param)
+    account = family.accounts.writable_by(current_resource_owner).visible.find(account_id_param)
 
     if idempotency_key_requested? && (existing_entry = existing_idempotent_entry(account))
       return render_existing_idempotent_entry(existing_entry)
@@ -113,6 +114,11 @@ def create
       }, status: :unprocessable_entity
     end
 
+  rescue ActiveRecord::RecordNotFound
+    render json: {
+      error: "not_found",
+      message: "Account not found"
+    }, status: :not_found
   rescue ActiveRecord::RecordNotUnique
     if idempotency_key_requested? && account && (existing_entry = existing_idempotent_entry(account))
       render_existing_idempotent_entry(existing_entry)
@@ -127,7 +133,7 @@ def create
       error: "internal_server_error",
       message: "An unexpected error occurred"
     }, status: :internal_server_error
-end
+  end
 
   def update
     if @entry.split_child?
@@ -200,13 +206,20 @@ def destroy
 
   private
 
-    def set_transaction
+    def set_readable_transaction
+      set_transaction_with_account_scope(readable_transaction_account_scope)
+    end
+
+    def set_writable_transaction
+      set_transaction_with_account_scope(writable_transaction_account_scope)
+    end
+
+    def set_transaction_with_account_scope(account_scope)
       raise ActiveRecord::RecordNotFound unless valid_uuid?(params[:id])
 
-      family = current_resource_owner.family
-      @transaction = family.transactions
+      @transaction = current_resource_owner.family.transactions
         .joins(entry: :account)
-        .merge(Account.accessible_by(current_resource_owner))
+        .merge(account_scope)
         .find(params[:id])
       @entry = @transaction.entry
     rescue ActiveRecord::RecordNotFound
@@ -216,6 +229,14 @@ def set_transaction
       }, status: :not_found
     end
 
+    def readable_transaction_account_scope
+      Account.accessible_by(current_resource_owner).merge(Account.historical)
+    end
+
+    def writable_transaction_account_scope
+      Account.writable_by(current_resource_owner).merge(Account.visible)
+    end
+
     def ensure_read_scope
       authorize_scope!(:read)
     end

app/controllers/api/v1/valuations_controller.rb

@@ -12,7 +12,10 @@ class Api::V1::ValuationsController < Api::V1::BaseController
 
   def index
     family = current_resource_owner.family
-    accessible_account_ids = family.accounts.accessible_by(current_resource_owner).select(:id)
+    accessible_account_ids = family.accounts
+      .accessible_by(current_resource_owner)
+      .historical
+      .select(:id)
     valuations_query = family.entries
       .where(entryable_type: "Valuation", account_id: accessible_account_ids)
       .includes(:account, :entryable)

app/controllers/concerns/api/v1/security_resource_filtering.rb

@@ -29,7 +29,10 @@ def trade_security_ids
     end
 
     def accessible_account_ids
-      @accessible_account_ids ||= current_resource_owner.family.accounts.visible.accessible_by(current_resource_owner).select(:id)
+      @accessible_account_ids ||= current_resource_owner.family.accounts
+                                        .accessible_by(current_resource_owner)
+                                        .historical
+                                        .select(:id)
     end
 
     def parse_boolean_filter_param(key)

app/controllers/concerns/api/v1/transfer_decision_filtering.rb

@@ -37,7 +37,10 @@ def accessible_transactions
     end
 
     def accessible_account_ids
-      @accessible_account_ids ||= Current.family.accounts.accessible_by(Current.user).select(:id)
+      @accessible_account_ids ||= Current.family.accounts
+                                      .accessible_by(Current.user)
+                                      .historical
+                                      .select(:id)
     end
 
     def apply_transfer_status_filter(query, status_model)