diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb index 8b03a5f1f..e7e827645 100644 --- a/app/controllers/api/v1/accounts_controller.rb +++ b/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 diff --git a/app/controllers/api/v1/balance_sheet_controller.rb b/app/controllers/api/v1/balance_sheet_controller.rb index 24ac01165..16833df96 100644 --- a/app/controllers/api/v1/balance_sheet_controller.rb +++ b/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 diff --git a/app/controllers/api/v1/balances_controller.rb b/app/controllers/api/v1/balances_controller.rb index 0e08d030a..71ac93297 100644 --- a/app/controllers/api/v1/balances_controller.rb +++ b/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) diff --git a/app/controllers/api/v1/holdings_controller.rb b/app/controllers/api/v1/holdings_controller.rb index 7f08dfe3b..8b1ec7b1d 100644 --- a/app/controllers/api/v1/holdings_controller.rb +++ b/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")) diff --git a/app/controllers/api/v1/trades_controller.rb b/app/controllers/api/v1/trades_controller.rb index 0538fe321..4b2d5d842 100644 --- a/app/controllers/api/v1/trades_controller.rb +++ b/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, diff --git a/app/controllers/api/v1/transactions_controller.rb b/app/controllers/api/v1/transactions_controller.rb index f208d83b2..f40c7ca0c 100644 --- a/app/controllers/api/v1/transactions_controller.rb +++ b/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 diff --git a/app/controllers/api/v1/valuations_controller.rb b/app/controllers/api/v1/valuations_controller.rb index 24c9cfd21..0cd494db6 100644 --- a/app/controllers/api/v1/valuations_controller.rb +++ b/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) diff --git a/app/controllers/concerns/api/v1/security_resource_filtering.rb b/app/controllers/concerns/api/v1/security_resource_filtering.rb index 8f58391f1..95c313678 100644 --- a/app/controllers/concerns/api/v1/security_resource_filtering.rb +++ b/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) diff --git a/app/controllers/concerns/api/v1/transfer_decision_filtering.rb b/app/controllers/concerns/api/v1/transfer_decision_filtering.rb index fa2d0ac5c..9f60635cc 100644 --- a/app/controllers/concerns/api/v1/transfer_decision_filtering.rb +++ b/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) diff --git a/app/models/account.rb b/app/models/account.rb index b0595d308..e2f52a542 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -36,6 +36,7 @@ class Account < ApplicationRecord enum :classification, { asset: "asset", liability: "liability" }, validate: { allow_nil: true } scope :visible, -> { where(status: [ "draft", "active" ]) } + scope :historical, -> { where(status: [ "draft", "active", "disabled" ]) } scope :assets, -> { where(classification: "asset") } scope :liabilities, -> { where(classification: "liability") } scope :alphabetically, -> { order(:name) } diff --git a/app/models/balance_sheet.rb b/app/models/balance_sheet.rb index 6dd2db3ac..d4b662163 100644 --- a/app/models/balance_sheet.rb +++ b/app/models/balance_sheet.rb @@ -5,9 +5,10 @@ class BalanceSheet attr_reader :family, :user - def initialize(family, user: nil) + def initialize(family, user: nil, include_disabled: false) @family = family @user = user || Current.user + @include_disabled = include_disabled end def assets @@ -56,13 +57,22 @@ def sync_status_monitor end def account_totals - @account_totals ||= AccountTotals.new(family, user: user, sync_status_monitor: sync_status_monitor) + @account_totals ||= AccountTotals.new( + family, + user: user, + sync_status_monitor: sync_status_monitor, + include_disabled: include_disabled? + ) end def net_worth_series_builder @net_worth_series_builder ||= NetWorthSeriesBuilder.new(family, user: user) end + def include_disabled? + @include_disabled + end + def sorted(accounts) account_order = user&.account_order order_key = account_order&.key || "name_asc" diff --git a/app/models/balance_sheet/account_totals.rb b/app/models/balance_sheet/account_totals.rb index 5c767687f..b7c24cf29 100644 --- a/app/models/balance_sheet/account_totals.rb +++ b/app/models/balance_sheet/account_totals.rb @@ -1,8 +1,9 @@ class BalanceSheet::AccountTotals - def initialize(family, user: nil, sync_status_monitor:) + def initialize(family, user: nil, sync_status_monitor:, include_disabled: false) @family = family @user = user @sync_status_monitor = sync_status_monitor + @include_disabled = include_disabled end def asset_accounts @@ -27,7 +28,7 @@ def to_param = account.to_param def visible_accounts @visible_accounts ||= begin - scope = family.accounts.visible.with_attached_logo + scope = account_scope.with_attached_logo .includes( :account_shares, :accountable, @@ -64,11 +65,19 @@ def account_rows def cache_key shares_version = user ? AccountShare.where(user: user).maximum(:updated_at)&.to_i : nil family.build_cache_key( - [ "balance_sheet_account_ids", user&.id, shares_version ].compact.join("_"), + [ "balance_sheet_account_ids", include_disabled? ? "historical" : "visible", user&.id, shares_version ].compact.join("_"), invalidate_on_data_updates: true ) end + def account_scope + include_disabled? ? family.accounts.historical : family.accounts.visible + end + + def include_disabled? + @include_disabled + end + # Loads visible accounts, caching their IDs to speed up subsequent requests. # On cache miss, loads records once and writes IDs; on hit, filters by cached IDs. def accounts diff --git a/app/models/balance_sheet/net_worth_series_builder.rb b/app/models/balance_sheet/net_worth_series_builder.rb index 7c29a6ece..40c9de0e4 100644 --- a/app/models/balance_sheet/net_worth_series_builder.rb +++ b/app/models/balance_sheet/net_worth_series_builder.rb @@ -7,7 +7,7 @@ def initialize(family, user: nil) def net_worth_series(period: Period.last_30_days) Rails.cache.fetch(cache_key(period)) do builder = Balance::ChartSeriesBuilder.new( - account_ids: visible_account_ids, + account_ids: historical_account_ids, currency: family.currency, period: period, favorable_direction: "up" @@ -20,9 +20,9 @@ def net_worth_series(period: Period.last_30_days) private attr_reader :family, :user - def visible_account_ids - @visible_account_ids ||= begin - scope = family.accounts.visible + def historical_account_ids + @historical_account_ids ||= begin + scope = family.accounts.historical scope = scope.included_in_finances_for(user) if user scope.pluck(:id) end @@ -31,7 +31,7 @@ def visible_account_ids def cache_key(period) shares_version = user ? AccountShare.where(user: user).maximum(:updated_at)&.to_i : nil key = [ - "balance_sheet_net_worth_series", + "balance_sheet_net_worth_series_historical", user&.id, shares_version, period.start_date, diff --git a/app/models/family.rb b/app/models/family.rb index 168773701..aa36a08f6 100644 --- a/app/models/family.rb +++ b/app/models/family.rb @@ -173,8 +173,8 @@ def auto_detect_transaction_merchants(transaction_ids) AutoMerchantDetector.new(self, transaction_ids: transaction_ids).auto_detect end - def balance_sheet(user: Current.user) - BalanceSheet.new(self, user: user) + def balance_sheet(user: Current.user, include_disabled: false) + BalanceSheet.new(self, user: user, include_disabled: include_disabled) end def income_statement(user: Current.user) diff --git a/db/migrate/20260514090000_restore_trade_category_reference.rb b/db/migrate/20260514090000_restore_trade_category_reference.rb new file mode 100644 index 000000000..a5d94e9a7 --- /dev/null +++ b/db/migrate/20260514090000_restore_trade_category_reference.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class RestoreTradeCategoryReference < ActiveRecord::Migration[7.2] + def up + return if column_exists?(:trades, :category_id) + + add_reference :trades, :category, null: true, foreign_key: { on_delete: :nullify }, type: :uuid + end + + def down + return unless column_exists?(:trades, :category_id) + + remove_reference :trades, :category, foreign_key: { on_delete: :nullify }, type: :uuid + end +end diff --git a/db/schema.rb b/db/schema.rb index 943e4dfe3..c51b35b0d 100644 --- a/db/schema.rb +++ b/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: 2026_05_12_211200) do +ActiveRecord::Schema[7.2].define(version: 2026_05_14_090000) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -1734,6 +1734,8 @@ t.string "investment_activity_label" t.decimal "fee", precision: 19, scale: 4, default: "0.0", null: false t.jsonb "extra", default: {}, null: false + t.uuid "category_id" + t.index ["category_id"], name: "index_trades_on_category_id" t.index ["extra"], name: "index_trades_on_extra", using: :gin t.index ["investment_activity_label"], name: "index_trades_on_investment_activity_label" t.index ["security_id"], name: "index_trades_on_security_id" @@ -1943,6 +1945,7 @@ add_foreign_key "taggings", "tags" add_foreign_key "tags", "families" add_foreign_key "tool_calls", "messages" + add_foreign_key "trades", "categories", on_delete: :nullify add_foreign_key "trades", "securities" add_foreign_key "transactions", "categories", on_delete: :nullify add_foreign_key "transactions", "merchants" diff --git a/docs/api/closed-account-portability.md b/docs/api/closed-account-portability.md new file mode 100644 index 000000000..59db22d64 --- /dev/null +++ b/docs/api/closed-account-portability.md @@ -0,0 +1,46 @@ +# Closed account portability behavior + +Closed or disabled accounts are lifecycle state, not deletion. The account should stop normal sync/activity and can be hidden from default account lists, but the account and its historical ledger rows remain part of the family record. + +## Account visibility + +Default account navigation can focus on visible accounts. In the model layer, `Account.visible` means `draft` or `active`, so disabled accounts are intentionally omitted from default list-style views. + +API and export surfaces that serve portability or audit workflows should expose lifecycle state instead of silently dropping the account: + +| Surface | Behavior | +| --- | --- | +| Default account list | May hide disabled accounts by default. | +| `GET /api/v1/accounts` | Supports `include_disabled=true` to include disabled accounts while excluding accounts pending deletion. | +| `GET /api/v1/accounts/{id}` | Returns a disabled account only when `include_disabled=true` is provided; otherwise disabled and pending-deletion accounts return 404. | +| `GET /api/v1/balance_sheet` | Excludes disabled account balances by default and supports `include_disabled=true` for portability clients. | +| Family archive/export | Includes family accounts with their status metadata. | + +`pending_deletion` is different from disabled. Pending deletion is an in-progress destructive state and should not be treated as a normal closed-account archive state. + +## Ledger inclusion + +Transactions, trades, balances, holdings, valuations, transfers, rejected transfers, and referenced securities attached to a disabled account are still historical facts. Portability and audit surfaces should not apply the default account-list visibility rule to ledger history. + +Use these rules when changing transaction/search/export/report behavior: + +- Account-list visibility decides whether an account appears in default navigation. +- Ledger inclusion decides whether historical rows remain queryable, searchable, reportable, and exportable. +- Closing an account should not erase historical rows from global transaction history, search, reports, or migration/export surfaces. +- If an endpoint intentionally hides disabled-account data, the endpoint should expose an explicit inclusion parameter and document the default. +- Net worth history should include disabled accounts so closing an account does not create a false historical gain or loss when balances were moved elsewhere. + +## Migration and archive expectations + +Portable exports should preserve: + +- account ID, name, type, subtype, status, currency, and balance metadata; +- all historical transactions and other ledger rows scoped to the family; +- transfer and rejected-transfer relationships only when both transaction sides are exported; +- account state separately from whether rows are included in the archive. + +Importers should restore the account lifecycle state without treating disabled accounts as deleted. Restored historical rows should stay attached to the restored account even when the account remains disabled. + +## Sync behavior + +Closed or disabled accounts should not start new provider sync activity by default. Provider-specific syncers may still keep diagnostic/status records, but they should not turn disabled-account visibility into ledger deletion. diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index ebc4c4425..0c57e7eb9 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -2748,6 +2748,7 @@ components: type: object required: - currency + - include_disabled - net_worth - assets - liabilities @@ -2755,6 +2756,9 @@ components: currency: type: string description: Family primary currency + include_disabled: + type: boolean + description: Whether disabled account totals are included in this response. net_worth: "$ref": "#/components/schemas/Money" assets: @@ -2871,7 +2875,8 @@ paths: - name: include_disabled in: query required: false - description: Include disabled accounts in the response. Defaults to false. + description: Include disabled accounts in the response while still excluding + accounts pending deletion. Defaults to false. schema: type: boolean responses: @@ -2900,7 +2905,8 @@ paths: - name: include_disabled in: query required: false - description: Allow retrieving a disabled account. Defaults to false. + description: Allow retrieving a disabled account while still excluding accounts + pending deletion. Defaults to false. schema: type: boolean responses: @@ -3477,6 +3483,14 @@ paths: and total liabilities with amounts converted to the family's primary currency. security: - apiKeyAuth: [] + parameters: + - name: include_disabled + in: query + required: false + description: Include disabled account balances in current totals. Defaults + to false. + schema: + type: boolean responses: '200': description: balance sheet returned @@ -3497,6 +3511,8 @@ paths: - Balances security: - apiKeyAuth: [] + description: Returns balance history for accessible accounts, including disabled + accounts but excluding accounts pending deletion. parameters: - name: page in: query @@ -4331,6 +4347,8 @@ paths: - Holdings security: - apiKeyAuth: [] + description: Returns holding history for accessible accounts, including disabled + accounts but excluding accounts pending deletion. parameters: - name: page in: query @@ -4350,6 +4368,7 @@ paths: description: Filter by account ID schema: type: string + format: uuid - name: account_ids in: query required: false @@ -4358,6 +4377,7 @@ paths: type: array items: type: string + format: uuid - name: date in: query required: false @@ -4392,14 +4412,14 @@ paths: application/json: schema: "$ref": "#/components/schemas/HoldingCollection" - '401': - description: unauthorized + '422': + description: invalid filter content: application/json: schema: "$ref": "#/components/schemas/ErrorResponse" - '422': - description: invalid date filter + '401': + description: unauthorized content: application/json: schema: @@ -5446,6 +5466,8 @@ paths: - Rejected Transfers security: - apiKeyAuth: [] + description: Returns rejected transfer decisions for accessible accounts, including + disabled accounts but excluding accounts pending deletion. parameters: - name: page in: query @@ -5776,6 +5798,8 @@ paths: - Securities security: - apiKeyAuth: [] + description: Returns securities referenced by accessible investment data, including + disabled accounts but excluding accounts pending deletion. parameters: - name: page in: query @@ -6243,6 +6267,8 @@ paths: - Trades security: - apiKeyAuth: [] + description: Returns trade history for accessible accounts, including disabled + accounts but excluding accounts pending deletion. parameters: - name: page in: query @@ -6262,6 +6288,7 @@ paths: description: Filter by account ID schema: type: string + format: uuid - name: account_ids in: query required: false @@ -6270,6 +6297,7 @@ paths: type: array items: type: string + format: uuid - name: start_date in: query required: false @@ -6291,14 +6319,14 @@ paths: application/json: schema: "$ref": "#/components/schemas/TradeCollection" - '401': - description: unauthorized + '422': + description: invalid filter content: application/json: schema: "$ref": "#/components/schemas/ErrorResponse" - '422': - description: invalid date filter + '401': + description: unauthorized content: application/json: schema: @@ -6833,6 +6861,8 @@ paths: - Transfers security: - apiKeyAuth: [] + description: Returns transfer decisions for accessible accounts, including disabled + accounts but excluding accounts pending deletion. parameters: - name: page in: query @@ -7039,6 +7069,8 @@ paths: - Valuations security: - apiKeyAuth: [] + description: Returns valuation history for accessible accounts, including disabled + accounts but excluding accounts pending deletion. parameters: - name: page in: query diff --git a/spec/requests/api/v1/accounts_spec.rb b/spec/requests/api/v1/accounts_spec.rb index 91686840b..e0415c603 100644 --- a/spec/requests/api/v1/accounts_spec.rb +++ b/spec/requests/api/v1/accounts_spec.rb @@ -86,7 +86,7 @@ parameter name: :per_page, in: :query, type: :integer, required: false, description: 'Items per page (default: 25, max: 100)' parameter name: :include_disabled, in: :query, type: :boolean, required: false, - description: 'Include disabled accounts in the response. Defaults to false.' + description: 'Include disabled accounts in the response while still excluding accounts pending deletion. Defaults to false.' response '200', 'accounts listed' do schema '$ref' => '#/components/schemas/AccountCollection' @@ -114,7 +114,7 @@ security [ { apiKeyAuth: [] } ] produces 'application/json' parameter name: :include_disabled, in: :query, type: :boolean, required: false, - description: 'Allow retrieving a disabled account. Defaults to false.' + description: 'Allow retrieving a disabled account while still excluding accounts pending deletion. Defaults to false.' let(:id) { checking_account.id } diff --git a/spec/requests/api/v1/balance_sheet_spec.rb b/spec/requests/api/v1/balance_sheet_spec.rb index f42e079f9..b7134013f 100644 --- a/spec/requests/api/v1/balance_sheet_spec.rb +++ b/spec/requests/api/v1/balance_sheet_spec.rb @@ -40,6 +40,9 @@ 'with amounts converted to the family\'s primary currency.' security [ { apiKeyAuth: [] } ] produces 'application/json' + parameter name: :include_disabled, in: :query, required: false, + description: 'Include disabled account balances in current totals. Defaults to false.', + schema: { type: :boolean } response '200', 'balance sheet returned' do schema '$ref' => '#/components/schemas/BalanceSheet' diff --git a/spec/requests/api/v1/balances_spec.rb b/spec/requests/api/v1/balances_spec.rb index 641b76a5b..f3174e1a4 100644 --- a/spec/requests/api/v1/balances_spec.rb +++ b/spec/requests/api/v1/balances_spec.rb @@ -71,6 +71,7 @@ get 'List balance history records' do tags 'Balances' security [ { apiKeyAuth: [] } ] + description 'Returns balance history for accessible accounts, including disabled accounts but excluding accounts pending deletion.' produces 'application/json' parameter name: :page, in: :query, type: :integer, required: false, description: 'Page number (default: 1)' diff --git a/spec/requests/api/v1/holdings_spec.rb b/spec/requests/api/v1/holdings_spec.rb index 27d7ac4f3..a489f47e5 100644 --- a/spec/requests/api/v1/holdings_spec.rb +++ b/spec/requests/api/v1/holdings_spec.rb @@ -67,16 +67,18 @@ get 'List holdings' do tags 'Holdings' security [ { apiKeyAuth: [] } ] + description 'Returns holding history for accessible accounts, including disabled accounts but excluding accounts pending deletion.' produces 'application/json' parameter name: :page, in: :query, type: :integer, required: false, description: 'Page number (default: 1)' parameter name: :per_page, in: :query, type: :integer, required: false, description: 'Items per page (default: 25, max: 100)' - parameter name: :account_id, in: :query, type: :string, required: false, - description: 'Filter by account ID' + parameter name: :account_id, in: :query, required: false, + description: 'Filter by account ID', + schema: { type: :string, format: :uuid } parameter name: :account_ids, in: :query, required: false, description: 'Filter by multiple account IDs', - schema: { type: :array, items: { type: :string } } + schema: { type: :array, items: { type: :string, format: :uuid } } parameter name: :date, in: :query, required: false, description: 'Filter by exact date', schema: { type: :string, format: :date } @@ -103,6 +105,14 @@ run_test! end + response '422', 'invalid account filter' do + schema '$ref' => '#/components/schemas/ErrorResponse' + + let(:account_id) { 'not-a-uuid' } + + run_test! + end + response '200', 'holdings filtered by date range' do schema '$ref' => '#/components/schemas/HoldingCollection' @@ -137,7 +147,7 @@ run_test! end - response '422', 'invalid date filter' do + response '422', 'invalid filter' do schema '$ref' => '#/components/schemas/ErrorResponse' let(:start_date) { 'not-a-date' } diff --git a/spec/requests/api/v1/rejected_transfers_spec.rb b/spec/requests/api/v1/rejected_transfers_spec.rb index 91580a42b..e5a9e6528 100644 --- a/spec/requests/api/v1/rejected_transfers_spec.rb +++ b/spec/requests/api/v1/rejected_transfers_spec.rb @@ -74,6 +74,7 @@ get 'List rejected transfers' do tags 'Rejected Transfers' security [ { apiKeyAuth: [] } ] + description 'Returns rejected transfer decisions for accessible accounts, including disabled accounts but excluding accounts pending deletion.' produces 'application/json' parameter name: :page, in: :query, type: :integer, required: false, description: 'Page number (default: 1)' diff --git a/spec/requests/api/v1/securities_spec.rb b/spec/requests/api/v1/securities_spec.rb index b250ece3c..23f4397ff 100644 --- a/spec/requests/api/v1/securities_spec.rb +++ b/spec/requests/api/v1/securities_spec.rb @@ -82,6 +82,7 @@ get 'List securities referenced by family investment data' do tags 'Securities' security [ { apiKeyAuth: [] } ] + description 'Returns securities referenced by accessible investment data, including disabled accounts but excluding accounts pending deletion.' produces 'application/json' parameter name: :page, in: :query, type: :integer, required: false, description: 'Page number (default: 1)' diff --git a/spec/requests/api/v1/trades_spec.rb b/spec/requests/api/v1/trades_spec.rb index 97a40a03a..6d6664577 100644 --- a/spec/requests/api/v1/trades_spec.rb +++ b/spec/requests/api/v1/trades_spec.rb @@ -81,16 +81,18 @@ get 'List trades' do tags 'Trades' security [ { apiKeyAuth: [] } ] + description 'Returns trade history for accessible accounts, including disabled accounts but excluding accounts pending deletion.' produces 'application/json' parameter name: :page, in: :query, type: :integer, required: false, description: 'Page number (default: 1)' parameter name: :per_page, in: :query, type: :integer, required: false, description: 'Items per page (default: 25, max: 100)' - parameter name: :account_id, in: :query, type: :string, required: false, - description: 'Filter by account ID' + parameter name: :account_id, in: :query, required: false, + description: 'Filter by account ID', + schema: { type: :string, format: :uuid } parameter name: :account_ids, in: :query, required: false, description: 'Filter by multiple account IDs', - schema: { type: :array, items: { type: :string } } + schema: { type: :array, items: { type: :string, format: :uuid } } parameter name: :start_date, in: :query, required: false, description: 'Filter trades from this date (inclusive)', schema: { type: :string, format: :date } @@ -112,6 +114,14 @@ run_test! end + response '422', 'invalid account filter' do + schema '$ref' => '#/components/schemas/ErrorResponse' + + let(:account_id) { 'not-a-uuid' } + + run_test! + end + response '200', 'trades filtered by date range' do schema '$ref' => '#/components/schemas/TradeCollection' @@ -138,7 +148,7 @@ run_test! end - response '422', 'invalid date filter' do + response '422', 'invalid filter' do schema '$ref' => '#/components/schemas/ErrorResponse' let(:start_date) { 'not-a-date' } diff --git a/spec/requests/api/v1/transfers_spec.rb b/spec/requests/api/v1/transfers_spec.rb index 47fcdd823..67eb1a3c6 100644 --- a/spec/requests/api/v1/transfers_spec.rb +++ b/spec/requests/api/v1/transfers_spec.rb @@ -76,6 +76,7 @@ get 'List transfers' do tags 'Transfers' security [ { apiKeyAuth: [] } ] + description 'Returns transfer decisions for accessible accounts, including disabled accounts but excluding accounts pending deletion.' produces 'application/json' parameter name: :page, in: :query, type: :integer, required: false, description: 'Page number (default: 1)' diff --git a/spec/requests/api/v1/valuations_spec.rb b/spec/requests/api/v1/valuations_spec.rb index b164812cf..75a649d76 100644 --- a/spec/requests/api/v1/valuations_spec.rb +++ b/spec/requests/api/v1/valuations_spec.rb @@ -62,6 +62,7 @@ get 'List valuations' do tags 'Valuations' security [ { apiKeyAuth: [] } ] + description 'Returns valuation history for accessible accounts, including disabled accounts but excluding accounts pending deletion.' produces 'application/json' parameter name: :page, in: :query, type: :integer, required: false, description: 'Page number (default: 1)' diff --git a/spec/swagger_helper.rb b/spec/swagger_helper.rb index d27ad818f..43d948e9e 100644 --- a/spec/swagger_helper.rb +++ b/spec/swagger_helper.rb @@ -1509,9 +1509,10 @@ }, BalanceSheet: { type: :object, - required: %w[currency net_worth assets liabilities], + required: %w[currency include_disabled net_worth assets liabilities], properties: { currency: { type: :string, description: 'Family primary currency' }, + include_disabled: { type: :boolean, description: 'Whether disabled account totals are included in this response.' }, net_worth: { '$ref' => '#/components/schemas/Money' }, assets: { '$ref' => '#/components/schemas/Money' }, liabilities: { '$ref' => '#/components/schemas/Money' } diff --git a/test/controllers/api/v1/accounts_controller_test.rb b/test/controllers/api/v1/accounts_controller_test.rb index 65e1715cd..e08642d7d 100644 --- a/test/controllers/api/v1/accounts_controller_test.rb +++ b/test/controllers/api/v1/accounts_controller_test.rb @@ -81,6 +81,17 @@ class Api::V1::AccountsControllerTest < ActionDispatch::IntegrationTest end end + test "should return project-standard internal index errors" do + Api::V1::AccountsController.any_instance.stubs(:accounts_scope).raises(StandardError, "boom") + + get "/api/v1/accounts", headers: api_headers(@api_key) + + assert_response :internal_server_error + response_body = JSON.parse(response.body) + assert_equal "internal_server_error", response_body["error"] + assert_equal "An unexpected error occurred", response_body["message"] + end + test "should only return active accounts" do # Make one account inactive inactive_account = accounts(:depository) @@ -110,6 +121,23 @@ class Api::V1::AccountsControllerTest < ActionDispatch::IntegrationTest assert_equal "disabled", account["status"] end + test "should exclude pending deletion accounts when disabled accounts are requested" do + pending_deletion_account = @user.family.accounts.create!( + name: "Pending Delete Checking #{SecureRandom.hex(4)}", + accountable: Depository.new, + balance: 0, + currency: "USD", + status: "pending_deletion" + ) + + get "/api/v1/accounts", params: { include_disabled: true }, headers: api_headers(@api_key) + + assert_response :success + response_body = JSON.parse(response.body) + account_ids = response_body["accounts"].map { |account| account["id"] } + assert_not_includes account_ids, pending_deletion_account.id + end + test "should show active account" do account = accounts(:depository) @@ -149,6 +177,18 @@ class Api::V1::AccountsControllerTest < ActionDispatch::IntegrationTest assert_equal "Account not found", response_body["message"] end + test "should return project-standard internal show errors" do + account = accounts(:depository) + Api::V1::AccountsController.any_instance.stubs(:accounts_scope).raises(StandardError, "boom") + + get "/api/v1/accounts/#{account.id}", headers: api_headers(@api_key) + + assert_response :internal_server_error + response_body = JSON.parse(response.body) + assert_equal "internal_server_error", response_body["error"] + assert_equal "An unexpected error occurred", response_body["message"] + end + test "should require authentication on show" do account = accounts(:depository) @@ -204,6 +244,22 @@ class Api::V1::AccountsControllerTest < ActionDispatch::IntegrationTest assert_equal "disabled", response_body["status"] end + test "should hide pending deletion account even when disabled accounts are requested" do + pending_deletion_account = @user.family.accounts.create!( + name: "Pending Delete Show #{SecureRandom.hex(4)}", + accountable: Depository.new, + balance: 0, + currency: "USD", + status: "pending_deletion" + ) + + get "/api/v1/accounts/#{pending_deletion_account.id}", + params: { include_disabled: true }, + headers: api_headers(@api_key) + + assert_response :not_found + end + test "should expose subtype across account types" do expected_subtypes = { accounts(:depository) => "checking", diff --git a/test/controllers/api/v1/balance_sheet_controller_test.rb b/test/controllers/api/v1/balance_sheet_controller_test.rb index 4597a387f..77574c4af 100644 --- a/test/controllers/api/v1/balance_sheet_controller_test.rb +++ b/test/controllers/api/v1/balance_sheet_controller_test.rb @@ -32,6 +32,8 @@ class Api::V1::BalanceSheetControllerTest < ActionDispatch::IntegrationTest response_body = JSON.parse(response.body) assert response_body.key?("currency") + assert response_body.key?("include_disabled") + assert_equal false, response_body["include_disabled"] assert response_body.key?("net_worth") assert response_body.key?("assets") assert response_body.key?("liabilities") @@ -43,6 +45,27 @@ class Api::V1::BalanceSheetControllerTest < ActionDispatch::IntegrationTest end end + test "should include disabled account totals when requested" do + disabled_account = @family.accounts.create!( + name: "Disabled Savings", + accountable: Depository.new, + balance: 500, + currency: "USD" + ) + disabled_account.disable! + + get "/api/v1/balance_sheet", headers: api_headers(@auth) + assert_response :success + default_assets = JSON.parse(response.body).dig("assets", "amount").to_d + + get "/api/v1/balance_sheet", params: { include_disabled: true }, headers: api_headers(@auth) + + assert_response :success + response_body = JSON.parse(response.body) + assert_equal true, response_body["include_disabled"] + assert_equal BigDecimal("500"), response_body.dig("assets", "amount").to_d - default_assets + end + private def api_headers(auth) diff --git a/test/controllers/api/v1/balances_controller_test.rb b/test/controllers/api/v1/balances_controller_test.rb index afb2cb201..2c4a5e250 100644 --- a/test/controllers/api/v1/balances_controller_test.rb +++ b/test/controllers/api/v1/balances_controller_test.rb @@ -59,6 +59,30 @@ class Api::V1::BalancesControllerTest < ActionDispatch::IntegrationTest assert_not_includes response_data["balances"].map { |balance| balance["id"] }, @other_balance.id end + test "lists balances for disabled accounts and excludes pending deletion accounts" do + @account.disable! + pending_deletion_account = @family.accounts.create!( + name: "Pending Delete Balance", + accountable: Depository.new, + balance: 500, + currency: "USD", + status: "pending_deletion" + ) + pending_deletion_balance = pending_deletion_account.balances.create!( + date: Date.parse("2024-01-15"), + balance: 500, + cash_balance: 500, + currency: "USD" + ) + + get api_v1_balances_url, headers: api_headers(@api_key) + + assert_response :success + balance_ids = JSON.parse(response.body)["balances"].map { |balance| balance["id"] } + assert_includes balance_ids, @balance.id + assert_not_includes balance_ids, pending_deletion_balance.id + end + test "shows a balance" do get api_v1_balance_url(@balance), headers: api_headers(@api_key) @@ -71,6 +95,17 @@ class Api::V1::BalancesControllerTest < ActionDispatch::IntegrationTest assert_kind_of Integer, response_data["end_balance_cents"] end + test "shows a disabled account balance" do + @account.disable! + + get api_v1_balance_url(@balance), headers: api_headers(@api_key) + + assert_response :success + response_data = JSON.parse(response.body) + assert_equal @balance.id, response_data["id"] + assert_equal @account.id, response_data.dig("account", "id") + end + test "renders nullable cash balance fields" do balance_without_cash = @account.balances.create!( date: Date.parse("2024-01-16"), diff --git a/test/controllers/api/v1/holdings_controller_test.rb b/test/controllers/api/v1/holdings_controller_test.rb new file mode 100644 index 000000000..130b55da6 --- /dev/null +++ b/test/controllers/api/v1/holdings_controller_test.rb @@ -0,0 +1,170 @@ +# frozen_string_literal: true + +require "test_helper" + +class Api::V1::HoldingsControllerTest < ActionDispatch::IntegrationTest + setup do + @user = users(:family_admin) + @family = @user.family + @user.api_keys.active.destroy_all + + @api_key = ApiKey.create!( + user: @user, + name: "Test Read Key", + scopes: [ "read" ], + source: "web", + display_key: "test_read_#{SecureRandom.hex(8)}" + ) + + @account = create_investment_account(status: "active", name: "Holding Investment") + @holding = create_holding(@account, ticker: "HLD#{SecureRandom.hex(4).upcase}") + + other_family = families(:empty) + other_account = other_family.accounts.create!( + name: "Other Holding Investment", + accountable: Investment.new, + balance: 0, + currency: "USD" + ) + @other_holding = create_holding(other_account, ticker: "OHD#{SecureRandom.hex(4).upcase}") + end + + test "lists holdings scoped to accessible historical accounts" do + @account.disable! + active_account = create_investment_account(status: "active", name: "Active Holding") + active_holding = create_holding(active_account, ticker: "AH#{SecureRandom.hex(4).upcase}") + pending_deletion_account = create_investment_account(status: "pending_deletion", name: "Pending Delete Holding") + pending_deletion_holding = create_holding(pending_deletion_account, ticker: "PDH#{SecureRandom.hex(4).upcase}") + + get api_v1_holdings_url, headers: api_headers(@api_key) + + assert_response :success + response_data = JSON.parse(response.body) + holding_ids = response_data["holdings"].map { |holding| holding["id"] } + assert_includes holding_ids, @holding.id + assert_includes holding_ids, active_holding.id + assert_not_includes holding_ids, pending_deletion_holding.id + assert_not_includes holding_ids, @other_holding.id + end + + test "shows a disabled account holding" do + @account.disable! + + get api_v1_holding_url(@holding), headers: api_headers(@api_key) + + assert_response :success + response_data = JSON.parse(response.body) + assert_equal @holding.id, response_data["id"] + assert_equal @account.id, response_data.dig("account", "id") + end + + test "does not show a pending deletion account holding" do + pending_deletion_account = create_investment_account(status: "pending_deletion", name: "Pending Delete Show Holding") + pending_deletion_holding = create_holding(pending_deletion_account, ticker: "PHS#{SecureRandom.hex(4).upcase}") + + get api_v1_holding_url(pending_deletion_holding), headers: api_headers(@api_key) + + assert_response :not_found + response_data = JSON.parse(response.body) + assert_equal "not_found", response_data["error"] + end + + test "returns not found for malformed holding id" do + get api_v1_holding_url("not-a-uuid"), headers: api_headers(@api_key) + + assert_response :not_found + response_data = JSON.parse(response.body) + assert_equal "not_found", response_data["error"] + end + + test "requires authentication for index" do + get api_v1_holdings_url + + assert_response :unauthorized + response_data = JSON.parse(response.body) + assert_equal "unauthorized", response_data["error"] + end + + test "requires authentication for show" do + get api_v1_holding_url(@holding) + + assert_response :unauthorized + response_data = JSON.parse(response.body) + assert_equal "unauthorized", response_data["error"] + end + + test "requires read scope for index" do + get api_v1_holdings_url, headers: api_headers(no_scope_api_key) + + assert_response :forbidden + response_data = JSON.parse(response.body) + assert_equal "insufficient_scope", response_data["error"] + end + + test "requires read scope for show" do + get api_v1_holding_url(@holding), headers: api_headers(no_scope_api_key) + + assert_response :forbidden + response_data = JSON.parse(response.body) + assert_equal "insufficient_scope", response_data["error"] + end + + test "returns project-standard internal index errors" do + Api::V1::HoldingsController.any_instance.stubs(:holding_history_scope).raises(StandardError, "boom") + + get api_v1_holdings_url, headers: api_headers(@api_key) + + assert_response :internal_server_error + response_data = JSON.parse(response.body) + assert_equal "internal_server_error", response_data["error"] + assert_equal "An unexpected error occurred", response_data["message"] + end + + test "rejects malformed account_id filter" do + get api_v1_holdings_url, params: { account_id: "not-a-uuid" }, headers: api_headers(@api_key) + + assert_response :unprocessable_entity + response_data = JSON.parse(response.body) + assert_equal "validation_failed", response_data["error"] + assert_match "account_id must be a valid UUID", response_data["message"] + end + + private + + def create_investment_account(status:, name:) + @family.accounts.create!( + name: "#{name} #{SecureRandom.hex(4)}", + accountable: Investment.new, + balance: 0, + currency: "USD", + status: status + ) + end + + def create_holding(account, ticker:) + security = Security.create!( + ticker: ticker, + name: "#{ticker} Security", + country_code: "US", + exchange_operating_mic: "XNAS" + ) + + account.holdings.create!( + security: security, + date: Date.parse("2024-01-15"), + qty: 1, + price: 100, + amount: 100, + currency: account.currency + ) + end + + def api_headers(api_key) + { "X-Api-Key" => api_key.display_key } + end + + def no_scope_api_key + @api_key.update_column(:scopes, []) + @api_key + end +end diff --git a/test/controllers/api/v1/rejected_transfers_controller_test.rb b/test/controllers/api/v1/rejected_transfers_controller_test.rb index ad64de5ea..bd84d811f 100644 --- a/test/controllers/api/v1/rejected_transfers_controller_test.rb +++ b/test/controllers/api/v1/rejected_transfers_controller_test.rb @@ -55,6 +55,41 @@ class Api::V1::RejectedTransfersControllerTest < ActionDispatch::IntegrationTest assert_not_includes response_data["rejected_transfers"].map { |transfer| transfer["id"] }, @other_rejected_transfer.id end + test "lists rejected transfers for disabled accounts and excludes pending deletion accounts" do + @account.disable! + + pending_deletion_account = @family.accounts.create!( + name: "Pending Delete Rejected Transfer", + accountable: Depository.new, + balance: 0, + currency: "USD", + status: "pending_deletion" + ) + pending_outflow = create_transaction( + pending_deletion_account, + amount: 15, + date: Date.parse("2024-01-15"), + name: "Pending delete rejected outflow" + ) + pending_inflow = create_transaction( + @destination_account, + amount: -15, + date: Date.parse("2024-01-15"), + name: "Pending delete rejected inflow" + ) + pending_rejected_transfer = RejectedTransfer.create!( + outflow_transaction: pending_outflow, + inflow_transaction: pending_inflow + ) + + get api_v1_rejected_transfers_url, headers: api_headers(@api_key) + + assert_response :success + transfer_ids = JSON.parse(response.body)["rejected_transfers"].map { |transfer| transfer["id"] } + assert_includes transfer_ids, @rejected_transfer.id + assert_not_includes transfer_ids, pending_rejected_transfer.id + end + test "permits read write scope" do read_write_key = ApiKey.create!( user: @user, diff --git a/test/controllers/api/v1/securities_controller_test.rb b/test/controllers/api/v1/securities_controller_test.rb index 04b235b79..313bcf68d 100644 --- a/test/controllers/api/v1/securities_controller_test.rb +++ b/test/controllers/api/v1/securities_controller_test.rb @@ -73,6 +73,40 @@ class Api::V1::SecuritiesControllerTest < ActionDispatch::IntegrationTest assert response_data.key?("pagination") end + test "lists securities referenced by disabled accounts and excludes pending deletion accounts" do + @account.disable! + + pending_deletion_account = @family.accounts.create!( + name: "Pending Delete Investment #{SecureRandom.hex(4)}", + accountable: Investment.new, + balance: 0, + currency: "USD", + status: "pending_deletion" + ) + pending_deletion_security = Security.create!( + ticker: "PD#{SecureRandom.hex(4).upcase}", + name: "Pending Delete Security", + country_code: "US", + exchange_operating_mic: "XNAS" + ) + pending_deletion_account.holdings.create!( + security: pending_deletion_security, + date: Date.parse("2024-01-15"), + qty: 1, + price: 100, + amount: 100, + currency: "USD" + ) + + get api_v1_securities_url, headers: api_headers(@api_key) + + assert_response :success + security_ids = JSON.parse(response.body)["securities"].map { |security| security["id"] } + assert_includes security_ids, @holding_security.id + assert_includes security_ids, @trade_security.id + assert_not_includes security_ids, pending_deletion_security.id + end + test "shows a scoped security" do get api_v1_security_url(@holding_security), headers: api_headers(@api_key) diff --git a/test/controllers/api/v1/trades_controller_test.rb b/test/controllers/api/v1/trades_controller_test.rb new file mode 100644 index 000000000..5ee12798d --- /dev/null +++ b/test/controllers/api/v1/trades_controller_test.rb @@ -0,0 +1,377 @@ +# frozen_string_literal: true + +require "test_helper" + +class Api::V1::TradesControllerTest < ActionDispatch::IntegrationTest + setup do + @user = users(:family_admin) + @family = @user.family + @user.api_keys.active.destroy_all + + @api_key = ApiKey.create!( + user: @user, + name: "Test Read Key", + scopes: [ "read" ], + source: "web", + display_key: "test_read_#{SecureRandom.hex(8)}" + ) + + @account = create_investment_account(status: "active", name: "Trade Investment") + @trade = create_trade(@account, ticker: "TRD#{SecureRandom.hex(4).upcase}") + + other_family = families(:empty) + other_account = other_family.accounts.create!( + name: "Other Trade Investment", + accountable: Investment.new, + balance: 0, + currency: "USD" + ) + @other_trade = create_trade(other_account, ticker: "OTH#{SecureRandom.hex(4).upcase}") + end + + test "lists trades scoped to accessible historical accounts" do + @account.disable! + pending_deletion_account = create_investment_account(status: "pending_deletion", name: "Pending Delete Trade") + pending_deletion_trade = create_trade(pending_deletion_account, ticker: "PDT#{SecureRandom.hex(4).upcase}") + + get api_v1_trades_url, headers: api_headers(@api_key) + + assert_response :success + response_data = JSON.parse(response.body) + trade_ids = response_data["trades"].map { |trade| trade["id"] } + assert_includes trade_ids, @trade.id + assert_not_includes trade_ids, pending_deletion_trade.id + assert_not_includes trade_ids, @other_trade.id + end + + test "shows a disabled account trade" do + @account.disable! + + get api_v1_trade_url(@trade), headers: api_headers(@api_key) + + assert_response :success + response_data = JSON.parse(response.body) + assert_equal @trade.id, response_data["id"] + assert_equal @account.id, response_data.dig("account", "id") + end + + test "does not show a pending deletion account trade" do + pending_deletion_account = create_investment_account(status: "pending_deletion", name: "Pending Delete Show Trade") + pending_deletion_trade = create_trade(pending_deletion_account, ticker: "PDS#{SecureRandom.hex(4).upcase}") + + get api_v1_trade_url(pending_deletion_trade), headers: api_headers(@api_key) + + assert_response :not_found + response_data = JSON.parse(response.body) + assert_equal "not_found", response_data["error"] + end + + test "returns not found for malformed trade id" do + get api_v1_trade_url("not-a-uuid"), headers: api_headers(@api_key) + + assert_response :not_found + response_data = JSON.parse(response.body) + assert_equal "not_found", response_data["error"] + end + + test "requires authentication for index" do + get api_v1_trades_url + + assert_response :unauthorized + response_data = JSON.parse(response.body) + assert_equal "unauthorized", response_data["error"] + end + + test "requires authentication for show" do + get api_v1_trade_url(@trade) + + assert_response :unauthorized + response_data = JSON.parse(response.body) + assert_equal "unauthorized", response_data["error"] + end + + test "creates trade with write scope" do + assert_difference("Trade.count", 1) do + post api_v1_trades_url, + params: { trade: valid_trade_create_params }, + headers: api_headers(read_write_api_key), + as: :json + end + + assert_response :created + response_data = JSON.parse(response.body) + assert_equal @account.id, response_data.dig("account", "id") + end + + test "does not create trade for read-only shared account" do + member_key = read_write_api_key_for(users(:family_member), source: "web") + share_account_with_member(permission: "read_only") + + assert_no_difference("Trade.count") do + post api_v1_trades_url, + params: { trade: valid_trade_create_params }, + headers: api_headers(member_key), + as: :json + end + + assert_response :not_found + response_data = JSON.parse(response.body) + assert_equal "not_found", response_data["error"] + assert_equal "Account not found", response_data["message"] + end + + test "rejects invalid trade create params" do + assert_no_difference("Trade.count") do + post api_v1_trades_url, + params: { trade: valid_trade_create_params.except(:type) }, + headers: api_headers(read_write_api_key), + as: :json + end + + assert_response :unprocessable_entity + response_data = JSON.parse(response.body) + assert_equal "validation_failed", response_data["error"] + end + + test "rejects invalid date on create" do + assert_no_difference("Trade.count") do + post api_v1_trades_url, + params: { trade: valid_trade_create_params(date: "not-a-date") }, + headers: api_headers(read_write_api_key), + as: :json + end + + assert_response :unprocessable_entity + response_data = JSON.parse(response.body) + assert_equal "validation_failed", response_data["error"] + assert_match "Invalid date format", response_data["message"] + end + + test "updates trade with patch and write scope" do + patch api_v1_trade_url(@trade), + params: { trade: { date: "2024-02-01", qty: 2, price: 125, type: "sell" } }, + headers: api_headers(read_write_api_key), + as: :json + + assert_response :success + @trade.reload + assert_equal Date.parse("2024-02-01"), @trade.entry.date + assert_equal BigDecimal("-2"), @trade.qty + assert_equal BigDecimal("125"), @trade.price + end + + test "updates trade with put and write scope" do + put api_v1_trade_url(@trade), + params: { trade: { qty: 3, price: 75, type: "buy" } }, + headers: api_headers(read_write_api_key), + as: :json + + assert_response :success + @trade.reload + assert_equal BigDecimal("3"), @trade.qty + assert_equal BigDecimal("75"), @trade.price + end + + test "does not update trade for read-only shared account" do + member_key = read_write_api_key_for(users(:family_member), source: "web") + share_account_with_member(permission: "read_only") + + patch api_v1_trade_url(@trade), + params: { trade: { qty: 2, price: 100, type: "buy" } }, + headers: api_headers(member_key), + as: :json + + assert_response :not_found + response_data = JSON.parse(response.body) + assert_equal "not_found", response_data["error"] + assert_equal "Trade not found", response_data["message"] + assert_equal BigDecimal("1"), @trade.reload.qty + end + + test "rejects invalid date on update" do + patch api_v1_trade_url(@trade), + params: { trade: { date: "not-a-date" } }, + headers: api_headers(read_write_api_key), + as: :json + + assert_response :unprocessable_entity + response_data = JSON.parse(response.body) + assert_equal "validation_failed", response_data["error"] + assert_match "Invalid date format", response_data["message"] + end + + test "returns not found for malformed trade id on write" do + patch api_v1_trade_url("not-a-uuid"), + params: { trade: { qty: 2, price: 100, type: "buy" } }, + headers: api_headers(read_write_api_key), + as: :json + + assert_response :not_found + assert_equal "not_found", JSON.parse(response.body)["error"] + end + + test "destroys trade with write scope" do + trade = create_trade(@account, ticker: "DEL#{SecureRandom.hex(4).upcase}") + + assert_difference("Trade.count", -1) do + delete api_v1_trade_url(trade), headers: api_headers(read_write_api_key) + end + + assert_response :success + response_data = JSON.parse(response.body) + assert_equal "Trade deleted successfully", response_data["message"] + end + + test "does not destroy trade for read-only shared account" do + member_key = read_write_api_key_for(users(:family_member), source: "web") + share_account_with_member(permission: "read_only") + + assert_no_difference("Trade.count") do + delete api_v1_trade_url(@trade), headers: api_headers(member_key) + end + + assert_response :not_found + response_data = JSON.parse(response.body) + assert_equal "not_found", response_data["error"] + assert_equal "Trade not found", response_data["message"] + end + + test "requires authentication for create" do + assert_no_difference("Trade.count") do + post api_v1_trades_url, params: { trade: valid_trade_create_params }, as: :json + end + + assert_response :unauthorized + response_data = JSON.parse(response.body) + assert_equal "unauthorized", response_data["error"] + end + + test "blocks read-only key from write actions" do + assert_no_difference("Trade.count") do + post api_v1_trades_url, + params: { trade: valid_trade_create_params }, + headers: api_headers(@api_key), + as: :json + end + assert_response :forbidden + assert_equal "insufficient_scope", JSON.parse(response.body)["error"] + + patch api_v1_trade_url(@trade), + params: { trade: { qty: 2, price: 100, type: "buy" } }, + headers: api_headers(@api_key), + as: :json + assert_response :forbidden + assert_equal "insufficient_scope", JSON.parse(response.body)["error"] + + delete api_v1_trade_url(@trade), headers: api_headers(@api_key) + assert_response :forbidden + assert_equal "insufficient_scope", JSON.parse(response.body)["error"] + end + + test "returns project-standard internal index errors" do + Api::V1::TradesController.any_instance.stubs(:trade_history_scope).raises(StandardError, "boom") + + get api_v1_trades_url, headers: api_headers(@api_key) + + assert_response :internal_server_error + response_data = JSON.parse(response.body) + assert_equal "internal_server_error", response_data["error"] + assert_equal "An unexpected error occurred", response_data["message"] + end + + test "rejects malformed account_id filter" do + get api_v1_trades_url, params: { account_id: "not-a-uuid" }, headers: api_headers(@api_key) + + assert_response :unprocessable_entity + response_data = JSON.parse(response.body) + assert_equal "validation_failed", response_data["error"] + assert_match "account_id must be a valid UUID", response_data["message"] + end + + private + + def create_investment_account(status:, name:) + @family.accounts.create!( + name: "#{name} #{SecureRandom.hex(4)}", + accountable: Investment.new, + balance: 0, + currency: "USD", + status: status + ) + end + + def create_trade(account, ticker:) + security = Security.create!( + ticker: ticker, + name: "#{ticker} Security", + country_code: "US", + exchange_operating_mic: "XNAS" + ) + + account.entries.create!( + name: "Buy #{ticker}", + date: Date.parse("2024-01-15"), + amount: 100, + currency: account.currency, + entryable: Trade.new( + security: security, + qty: 1, + price: 100, + currency: account.currency + ) + ).entryable + end + + def api_headers(api_key) + { "X-Api-Key" => api_key.display_key } + end + + def read_write_api_key + @read_write_api_key ||= ApiKey.create!( + user: @user, + name: "Test Write Key", + scopes: [ "read_write" ], + source: "mobile", + display_key: "test_write_#{SecureRandom.hex(8)}" + ) + end + + def read_write_api_key_for(user, source:) + user.api_keys.active.where(source: source).destroy_all + ApiKey.create!( + user: user, + name: "Shared Account Write Key", + scopes: [ "read_write" ], + source: source, + display_key: "test_shared_write_#{SecureRandom.hex(8)}" + ) + end + + def share_account_with_member(permission:) + @account.account_shares.where(user: users(:family_member)).destroy_all + @account.account_shares.create!( + user: users(:family_member), + permission: permission, + include_in_finances: true + ) + end + + def valid_trade_create_params(overrides = {}) + security = Security.create!( + ticker: "NEW#{SecureRandom.hex(4).upcase}", + name: "New Trade Security", + country_code: "US", + exchange_operating_mic: "XNAS" + ) + + { + account_id: @account.id, + date: "2024-02-01", + qty: 2, + price: 50, + type: "buy", + security_id: security.id, + currency: @account.currency + }.merge(overrides) + end +end diff --git a/test/controllers/api/v1/transactions_controller_test.rb b/test/controllers/api/v1/transactions_controller_test.rb index 1f5123d9e..58578df34 100644 --- a/test/controllers/api/v1/transactions_controller_test.rb +++ b/test/controllers/api/v1/transactions_controller_test.rb @@ -192,6 +192,17 @@ class Api::V1::TransactionsControllerTest < ActionDispatch::IntegrationTest assert_response :unauthorized end + test "should return project-standard internal index errors" do + Api::V1::TransactionsController.any_instance.stubs(:safe_page_param).raises(StandardError, "boom") + + get api_v1_transactions_url, headers: api_headers(@api_key) + + assert_response :internal_server_error + response_data = JSON.parse(response.body) + assert_equal "internal_server_error", response_data["error"] + assert_equal "An unexpected error occurred", response_data["message"] + end + # SHOW action tests test "should show transaction with valid API key" do get api_v1_transaction_url(@transaction), headers: api_headers(@api_key) @@ -222,6 +233,19 @@ class Api::V1::TransactionsControllerTest < ActionDispatch::IntegrationTest assert_equal disabled_transaction.entry.account_id, response_data["account"]["id"] end + test "should not show pending deletion account transaction" do + pending_deletion_transaction = create_account_transaction( + status: "pending_deletion", + name: "Pending Delete Account Show" + ) + + get api_v1_transaction_url(pending_deletion_transaction), headers: api_headers(@api_key) + + assert_response :not_found + response_data = JSON.parse(response.body) + assert_equal "not_found", response_data["error"] + end + test "should return 404 for valid missing transaction id" do get api_v1_transaction_url(SecureRandom.uuid), headers: api_headers(@api_key) assert_response :not_found @@ -514,6 +538,30 @@ class Api::V1::TransactionsControllerTest < ActionDispatch::IntegrationTest assert_response :unprocessable_entity end + test "should reject create for disabled account" do + disabled_account = create_account(status: "disabled") + + assert_no_difference("Entry.count") do + post api_v1_transactions_url, + params: { + transaction: { + account_id: disabled_account.id, + name: "Closed Account Write", + amount: 25.00, + date: Date.current, + currency: "USD", + nature: "expense" + } + }, + headers: api_headers(@api_key) + end + + assert_response :not_found + response_data = JSON.parse(response.body) + assert_equal "not_found", response_data["error"] + assert_equal "Account not found", response_data["message"] + end + test "should reject invalid date on create" do transaction_params = { transaction: { @@ -544,6 +592,28 @@ class Api::V1::TransactionsControllerTest < ActionDispatch::IntegrationTest assert_response :unauthorized end + test "should return project-standard internal create errors" do + Entry.any_instance.stubs(:save).raises(StandardError, "boom") + + post api_v1_transactions_url, + params: { + transaction: { + account_id: @account.id, + name: "Boom Transaction", + amount: 25.00, + date: Date.current, + currency: "USD", + nature: "expense" + } + }, + headers: api_headers(@api_key) + + assert_response :internal_server_error + response_data = JSON.parse(response.body) + assert_equal "internal_server_error", response_data["error"] + assert_equal "An unexpected error occurred", response_data["message"] + end + # UPDATE action tests test "should update transaction with valid parameters" do update_params = { @@ -582,6 +652,20 @@ class Api::V1::TransactionsControllerTest < ActionDispatch::IntegrationTest assert_response :not_found end + test "should reject update for disabled account transaction" do + disabled_transaction = create_disabled_account_transaction(name: "Closed Account Update") + + put api_v1_transaction_url(disabled_transaction), + params: { transaction: { name: "Should Not Update" } }, + headers: api_headers(@api_key) + + assert_response :not_found + response_data = JSON.parse(response.body) + assert_equal "not_found", response_data["error"] + assert_equal "Transaction not found", response_data["message"] + assert_equal "Closed Account Update", disabled_transaction.entry.reload.name + end + test "should reject update without API key" do put api_v1_transaction_url(@transaction), params: { transaction: { name: "Test" } } assert_response :unauthorized @@ -656,6 +740,19 @@ class Api::V1::TransactionsControllerTest < ActionDispatch::IntegrationTest assert_equal new_tags.map(&:id), @transaction.tag_ids end + test "should return project-standard internal update errors" do + Entry.any_instance.stubs(:update).raises(StandardError, "boom") + + put api_v1_transaction_url(@transaction), + params: { transaction: { name: "Boom Update" } }, + headers: api_headers(@api_key) + + assert_response :internal_server_error + response_data = JSON.parse(response.body) + assert_equal "internal_server_error", response_data["error"] + assert_equal "An unexpected error occurred", response_data["message"] + end + # DESTROY action tests test "should destroy transaction" do entry_to_delete = @account.entries.create!( @@ -686,11 +783,36 @@ class Api::V1::TransactionsControllerTest < ActionDispatch::IntegrationTest assert_response :not_found end + test "should reject destroy for disabled account transaction" do + disabled_transaction = create_disabled_account_transaction(name: "Closed Account Delete") + + assert_no_difference("Entry.count") do + delete api_v1_transaction_url(disabled_transaction), headers: api_headers(@api_key) + end + + assert_response :not_found + response_data = JSON.parse(response.body) + assert_equal "not_found", response_data["error"] + assert_equal "Transaction not found", response_data["message"] + assert Entry.exists?(disabled_transaction.entry.id) + end + test "should reject destroy without API key" do delete api_v1_transaction_url(@transaction) assert_response :unauthorized end + test "should return project-standard internal destroy errors" do + Entry.any_instance.stubs(:destroy!).raises(StandardError, "boom") + + delete api_v1_transaction_url(@transaction), headers: api_headers(@api_key) + + assert_response :internal_server_error + response_data = JSON.parse(response.body) + assert_equal "internal_server_error", response_data["error"] + assert_equal "An unexpected error occurred", response_data["message"] + end + # JSON structure tests test "transaction JSON should have expected structure" do get api_v1_transaction_url(@transaction), headers: api_headers(@api_key) @@ -786,13 +908,7 @@ def create_disabled_account_transaction(name:, date: Date.current) end def create_account_transaction(status:, name:, date: Date.current) - account = @family.accounts.create!( - name: "#{status.titleize} Checking #{SecureRandom.hex(4)}", - balance: 0, - currency: "USD", - status: status, - accountable: Depository.new - ) + account = create_account(status: status) entry = account.entries.create!( name: name, @@ -804,4 +920,14 @@ def create_account_transaction(status:, name:, date: Date.current) entry.transaction end + + def create_account(status:) + @family.accounts.create!( + name: "#{status.titleize} Checking #{SecureRandom.hex(4)}", + balance: 0, + currency: "USD", + status: status, + accountable: Depository.new + ) + end end diff --git a/test/controllers/api/v1/transfers_controller_test.rb b/test/controllers/api/v1/transfers_controller_test.rb index e74dcfbda..4fc4199de 100644 --- a/test/controllers/api/v1/transfers_controller_test.rb +++ b/test/controllers/api/v1/transfers_controller_test.rb @@ -57,6 +57,38 @@ class Api::V1::TransfersControllerTest < ActionDispatch::IntegrationTest assert_not_includes response_data["transfers"].map { |transfer| transfer["id"] }, @other_transfer.id end + test "lists transfers for disabled accounts and excludes pending deletion accounts" do + @account.disable! + + pending_deletion_account = @family.accounts.create!( + name: "Pending Delete Transfer", + accountable: Depository.new, + balance: 0, + currency: "USD", + status: "pending_deletion" + ) + pending_outflow = create_transaction( + pending_deletion_account, + amount: 15, + date: Date.parse("2024-01-15"), + name: "Pending delete outflow" + ) + pending_inflow = create_transaction( + @destination_account, + amount: -15, + date: Date.parse("2024-01-15"), + name: "Pending delete inflow" + ) + pending_transfer = Transfer.create!(outflow_transaction: pending_outflow, inflow_transaction: pending_inflow) + + get api_v1_transfers_url, headers: api_headers(@api_key) + + assert_response :success + transfer_ids = JSON.parse(response.body)["transfers"].map { |transfer| transfer["id"] } + assert_includes transfer_ids, @transfer.id + assert_not_includes transfer_ids, pending_transfer.id + end + test "permits read write scope" do read_write_key = ApiKey.create!( user: @user, diff --git a/test/controllers/api/v1/valuations_controller_test.rb b/test/controllers/api/v1/valuations_controller_test.rb index b1a24bc97..c3de401b9 100644 --- a/test/controllers/api/v1/valuations_controller_test.rb +++ b/test/controllers/api/v1/valuations_controller_test.rb @@ -54,6 +54,21 @@ class Api::V1::ValuationsControllerTest < ActionDispatch::IntegrationTest assert_response :success end + test "should list disabled account valuations and exclude pending deletion account valuations" do + disabled_valuation = create_valuation_for_account(status: "disabled", name: "Disabled Valuation") + pending_deletion_valuation = create_valuation_for_account( + status: "pending_deletion", + name: "Pending Delete Valuation" + ) + + get api_v1_valuations_url, headers: api_headers(@api_key) + + assert_response :success + valuation_ids = JSON.parse(response.body)["valuations"].map { |valuation| valuation["id"] } + assert_includes valuation_ids, disabled_valuation.entry.id + assert_not_includes valuation_ids, pending_deletion_valuation.entry.id + end + test "should filter index by account_id" do get api_v1_valuations_url, params: { account_id: @account.id }, @@ -103,8 +118,8 @@ class Api::V1::ValuationsControllerTest < ActionDispatch::IntegrationTest assert_equal "account_id must be a valid UUID", response_data["message"] end - test "should not expose internal index errors" do - Api::V1::ValuationsController.any_instance.stubs(:safe_page_param).raises(StandardError, "database password leaked") + test "should return project-standard internal index errors" do + Api::V1::ValuationsController.any_instance.stubs(:safe_page_param).raises(StandardError, "boom") get api_v1_valuations_url, headers: api_headers(@api_key) assert_response :internal_server_error @@ -112,7 +127,6 @@ class Api::V1::ValuationsControllerTest < ActionDispatch::IntegrationTest response_data = JSON.parse(response.body) assert_equal "internal_server_error", response_data["error"] assert_equal "An unexpected error occurred", response_data["message"] - assert_not_includes response.body, "database password leaked" end test "should reject index without API key" do @@ -267,6 +281,25 @@ class Api::V1::ValuationsControllerTest < ActionDispatch::IntegrationTest assert_response :unauthorized end + test "should return project-standard internal create errors" do + Account.any_instance.stubs(:create_reconciliation).raises(StandardError, "boom") + + post api_v1_valuations_url, + params: { + valuation: { + account_id: @account.id, + amount: 10000.00, + date: Date.current + } + }, + headers: api_headers(@api_key) + + assert_response :internal_server_error + response_data = JSON.parse(response.body) + assert_equal "internal_server_error", response_data["error"] + assert_equal "An unexpected error occurred", response_data["message"] + end + # UPDATE action tests test "should update valuation with valid parameters" do entry = @valuation.entry @@ -330,6 +363,20 @@ class Api::V1::ValuationsControllerTest < ActionDispatch::IntegrationTest assert_response :unauthorized end + test "should return project-standard internal update errors" do + entry = @valuation.entry + Account.any_instance.stubs(:update_reconciliation).raises(StandardError, "boom") + + put api_v1_valuation_url(entry), + params: { valuation: { amount: 15000.00, date: Date.current } }, + headers: api_headers(@api_key) + + assert_response :internal_server_error + response_data = JSON.parse(response.body) + assert_equal "internal_server_error", response_data["error"] + assert_equal "An unexpected error occurred", response_data["message"] + end + # JSON structure tests test "valuation JSON should have expected structure" do # Create a new valuation to test the structure @@ -368,6 +415,24 @@ class Api::V1::ValuationsControllerTest < ActionDispatch::IntegrationTest private + def create_valuation_for_account(status:, name:) + account = @family.accounts.create!( + name: "#{status.titleize} Investment #{SecureRandom.hex(4)}", + balance: 0, + currency: "USD", + status: status, + accountable: Investment.new + ) + + account.entries.create!( + name: name, + amount: 1234.56, + currency: "USD", + date: Date.current, + entryable: Valuation.new + ).entryable + end + def api_headers(api_key) { "X-Api-Key" => api_key.plain_key } end diff --git a/test/models/balance_sheet_test.rb b/test/models/balance_sheet_test.rb index 9021baf27..7bd852ddb 100644 --- a/test/models/balance_sheet_test.rb +++ b/test/models/balance_sheet_test.rb @@ -1,6 +1,8 @@ require "test_helper" class BalanceSheetTest < ActiveSupport::TestCase + include BalanceTestHelper + setup do @family = families(:empty) end @@ -46,6 +48,39 @@ class BalanceSheetTest < ActiveSupport::TestCase assert_equal 1000, BalanceSheet.new(@family).liabilities.total end + test "disabled accounts can be included in totals for API portability" do + create_account(balance: 1000, accountable: CreditCard.new) + create_account(balance: 10000, accountable: Depository.new) + + other_liability = create_account(balance: 5000, accountable: OtherLiability.new) + other_liability.disable! + + balance_sheet = BalanceSheet.new(@family, include_disabled: true) + + assert_equal 10000 - 1000 - 5000, balance_sheet.net_worth + assert_equal 10000, balance_sheet.assets.total + assert_equal 1000 + 5000, balance_sheet.liabilities.total + end + + test "net worth series includes disabled accounts to avoid false archive jumps" do + active_account = create_account(balance: 100, accountable: Depository.new) + disabled_account = create_account(balance: 0, accountable: Depository.new) + disabled_account.disable! + + start_date = 1.day.ago.to_date + end_date = Date.current + create_balance(account: active_account, date: start_date, balance: 0) + create_balance(account: active_account, date: end_date, balance: 100) + create_balance(account: disabled_account, date: start_date, balance: 100) + create_balance(account: disabled_account, date: end_date, balance: 0) + + series = BalanceSheet.new(@family).net_worth_series( + period: Period.custom(start_date: start_date, end_date: end_date) + ) + + assert_equal [ 100, 100 ], series.map { |value| value.value.amount } + end + test "calculates asset group totals" do create_account(balance: 1000, accountable: Depository.new) create_account(balance: 2000, accountable: Depository.new) diff --git a/test/models/trade_test.rb b/test/models/trade_test.rb index 6223a5fc7..344c88301 100644 --- a/test/models/trade_test.rb +++ b/test/models/trade_test.rb @@ -52,6 +52,28 @@ class TradeTest < ActiveSupport::TestCase assert_equal 0, trade.fee end + test "category deletion nullifies trade category reference" do + category = Category.create!( + family: families(:dylan_family), + name: "Trade Category #{SecureRandom.hex(4)}", + color: "#2196F3", + lucide_icon: "trending-up" + ) + security = Security.create!(ticker: "CATNULL", exchange_operating_mic: "XNAS") + trade = Trade.create!( + security: security, + category: category, + price: 100, + qty: 1, + currency: "USD", + investment_activity_label: "Buy" + ) + + category.destroy! + + assert_nil trade.reload.category_id + end + test "exchange_rate setter stores normalized numeric value in extra" do trade = Trade.new trade.exchange_rate = "0.91"