diff --git a/app/controllers/concerns/entryable_resource.rb b/app/controllers/concerns/entryable_resource.rb index 84aac1d48ca..918b32bb138 100644 --- a/app/controllers/concerns/entryable_resource.rb +++ b/app/controllers/concerns/entryable_resource.rb @@ -119,7 +119,7 @@ def prepared_entry_params def entry_params params.require(:account_entry).permit( - :account_id, :name, :date, :amount, :currency, :excluded, :notes, :nature, + :account_id, :name, :enriched_name, :date, :amount, :currency, :excluded, :notes, :nature, entryable_attributes: self.class.permitted_entryable_attributes ) end diff --git a/app/models/account.rb b/app/models/account.rb index 400e8beaa73..19d883ca094 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -151,6 +151,7 @@ def update_balance!(balance) else entries.create! \ date: Date.current, + name: "Balance update", amount: balance, currency: currency, entryable: Account::Valuation.new diff --git a/app/models/account/data_enricher.rb b/app/models/account/data_enricher.rb index e0615cc134b..0be57dc1bc5 100644 --- a/app/models/account/data_enricher.rb +++ b/app/models/account/data_enricher.rb @@ -50,7 +50,7 @@ def enrich_transactions category.save! if category.present? entry.update!( enriched_at: Time.current, - name: entry.enriched_at.nil? && info.name ? info.name : entry.name, + enriched_name: info.name, entryable_attributes: entryable_attributes ) end diff --git a/app/models/account/entry.rb b/app/models/account/entry.rb index 1801fb9ede4..4d7334fbb75 100644 --- a/app/models/account/entry.rb +++ b/app/models/account/entry.rb @@ -10,7 +10,7 @@ class Account::Entry < ApplicationRecord delegated_type :entryable, types: Account::Entryable::TYPES, dependent: :destroy accepts_nested_attributes_for :entryable - validates :date, :amount, :currency, presence: true + validates :date, :name, :amount, :currency, presence: true validates :date, uniqueness: { scope: [ :account_id, :entryable_type ] }, if: -> { account_valuation? } validates :date, comparison: { greater_than: -> { min_supported_date } } @@ -47,14 +47,6 @@ def sync_account_later account.sync_later(start_date: sync_start_date) end - def inflow? - amount <= 0 && account_transaction? - end - - def outflow? - amount > 0 && account_transaction? - end - def entryable_name_short entryable_type.demodulize.underscore end @@ -63,6 +55,10 @@ def balance_trend(entries, balances) Account::BalanceTrendCalculator.new(self, entries, balances).trend end + def display_name + enriched_name.presence || name + end + class << self # arbitrary cutoff date to avoid expensive sync operations def min_supported_date diff --git a/app/models/account/syncer.rb b/app/models/account/syncer.rb index df5a7b03ec9..5b2e4aba0a2 100644 --- a/app/models/account/syncer.rb +++ b/app/models/account/syncer.rb @@ -11,7 +11,8 @@ def run update_account_info(balances, holdings) unless account.plaid_account_id.present? convert_records_to_family_currency(balances, holdings) unless account.currency == account.family.currency - if account.family.data_enrichment_enabled? + # Enrich if user opted in or if we're syncing transactions from a Plaid account + if account.family.data_enrichment_enabled? || account.plaid_account_id.present? account.enrich_data_later else Rails.logger.info("Data enrichment is disabled, skipping enrichment for account #{account.id}") diff --git a/app/models/account/trade.rb b/app/models/account/trade.rb index b8ebd7b898a..70b0c8f32c2 100644 --- a/app/models/account/trade.rb +++ b/app/models/account/trade.rb @@ -26,11 +26,6 @@ def buy? qty > 0 end - def name - prefix = sell? ? "Sell " : "Buy " - prefix + "#{qty.abs} shares of #{security.ticker}" - end - def unrealized_gain_loss return nil if sell? current_price = security.current_price diff --git a/app/models/account/trade_builder.rb b/app/models/account/trade_builder.rb index 191d810029e..e62947f7313 100644 --- a/app/models/account/trade_builder.rb +++ b/app/models/account/trade_builder.rb @@ -31,7 +31,11 @@ def buildable end def build_trade + prefix = type == "sell" ? "Sell " : "Buy " + trade_name = prefix + "#{qty.to_i.abs} shares of #{security.ticker}" + account.entries.new( + name: trade_name, date: date, amount: signed_amount, currency: currency, diff --git a/app/models/account/transaction.rb b/app/models/account/transaction.rb index fbf2aa9e54c..6b8f4995ee5 100644 --- a/app/models/account/transaction.rb +++ b/app/models/account/transaction.rb @@ -48,10 +48,6 @@ def searchable_keys end end - def name - entry.name || (entry.amount.positive? ? "Expense" : "Income") - end - def eod_balance entry.amount_money end diff --git a/app/models/account/transfer.rb b/app/models/account/transfer.rb index 174576e8b3f..ea9084131c4 100644 --- a/app/models/account/transfer.rb +++ b/app/models/account/transfer.rb @@ -33,11 +33,11 @@ def to_account end def inflow_transaction - entries.find { |e| e.inflow? } + entries.find { |e| e.amount.negative? } end def outflow_transaction - entries.find { |e| e.outflow? } + entries.find { |e| e.amount.positive? } end def update_entries!(params) diff --git a/app/models/account/valuation.rb b/app/models/account/valuation.rb index 5a4d1b8f66c..93ebf5ffb1f 100644 --- a/app/models/account/valuation.rb +++ b/app/models/account/valuation.rb @@ -10,8 +10,4 @@ def requires_search?(_params) false end end - - def name - "Balance update" - end end diff --git a/app/models/demo/generator.rb b/app/models/demo/generator.rb index 29985a365b6..36e86200a28 100644 --- a/app/models/demo/generator.rb +++ b/app/models/demo/generator.rb @@ -303,6 +303,7 @@ def create_valuation!(account, date, amount) date: date, amount: amount, currency: "USD", + name: "Balance update", entryable: Account::Valuation.new end diff --git a/app/views/account/trades/_trade.html.erb b/app/views/account/trades/_trade.html.erb index 2215cd94bcc..8c5c924ee0b 100644 --- a/app/views/account/trades/_trade.html.erb +++ b/app/views/account/trades/_trade.html.erb @@ -13,14 +13,14 @@
<%= tag.div class: ["flex items-center gap-2"] do %>
- <%= trade.name.first.upcase %> + <%= entry.display_name.first.upcase %>
<% if entry.new_record? %> - <%= content_tag :p, trade.name %> + <%= content_tag :p, entry.display_name %> <% else %> - <%= link_to trade.name, + <%= link_to entry.display_name, account_entry_path(entry), data: { turbo_frame: "drawer", turbo_prefetch: false }, class: "hover:underline hover:text-gray-800" %> diff --git a/app/views/account/transactions/_transaction.html.erb b/app/views/account/transactions/_transaction.html.erb index d63ddf8b6f4..dfe0955a156 100644 --- a/app/views/account/transactions/_transaction.html.erb +++ b/app/views/account/transactions/_transaction.html.erb @@ -11,17 +11,17 @@
<%= content_tag :div, class: ["flex items-center gap-2"] do %> - <% if entry.account_transaction.merchant&.icon_url %> - <%= image_tag entry.account_transaction.merchant.icon_url, class: "w-6 h-6 rounded-full" %> + <% if transaction.merchant&.icon_url %> + <%= image_tag transaction.merchant.icon_url, class: "w-6 h-6 rounded-full" %> <% else %> - <%= render "shared/circle_logo", name: transaction.name, size: "sm" %> + <%= render "shared/circle_logo", name: entry.display_name, size: "sm" %> <% end %>
<% if entry.new_record? %> - <%= content_tag :p, transaction.name %> + <%= content_tag :p, entry.display_name %> <% else %> - <%= link_to transaction.name, + <%= link_to entry.display_name, entry.transfer.present? ? account_transfer_path(entry.transfer) : account_entry_path(entry), data: { turbo_frame: "drawer", turbo_prefetch: false }, class: "hover:underline hover:text-gray-800" %> @@ -41,7 +41,7 @@ <% end %>
- <%= render "account/transfers/account_logos", transfer: entry.transfer, outflow: entry.outflow? %> + <%= render "account/transfers/account_logos", transfer: entry.transfer, outflow: entry.amount.positive? %>
<% else %>
@@ -65,7 +65,7 @@
<%= content_tag :p, format_money(-entry.amount_money), - class: ["text-green-600": entry.inflow?] %> + class: ["text-green-600": entry.amount.negative?] %>
<% if balance_trend %> diff --git a/app/views/account/transactions/show.html.erb b/app/views/account/transactions/show.html.erb index ecce8921cfb..b512b43ad50 100644 --- a/app/views/account/transactions/show.html.erb +++ b/app/views/account/transactions/show.html.erb @@ -9,7 +9,8 @@ url: account_transaction_path(@entry), class: "space-y-2", data: { controller: "auto-submit-form" } do |f| %> - <%= f.text_field :name, + + <%= f.text_field @entry.enriched_at.present? ? :enriched_name : :name, label: t(".name_label"), "data-auto-submit-form-target": "auto" %> diff --git a/app/views/account/valuations/_form.html.erb b/app/views/account/valuations/_form.html.erb index b56e847bbbb..f3f0aa5fac1 100644 --- a/app/views/account/valuations/_form.html.erb +++ b/app/views/account/valuations/_form.html.erb @@ -8,6 +8,7 @@ <% end %>
+ <%= form.hidden_field :name, value: "Balance update" %> <%= form.date_field :date, label: true, required: true, value: Date.current, min: Account::Entry.min_supported_date, max: Date.current %> <%= form.money_field :amount, label: t(".amount"), required: true %>
diff --git a/app/views/account/valuations/_valuation.html.erb b/app/views/account/valuations/_valuation.html.erb index 0761fd17b03..3e34cfbaa54 100644 --- a/app/views/account/valuations/_valuation.html.erb +++ b/app/views/account/valuations/_valuation.html.erb @@ -18,9 +18,9 @@
<% if entry.new_record? %> - <%= content_tag :p, entry.entryable.name %> + <%= content_tag :p, entry.display_name %> <% else %> - <%= link_to entry.entryable.name, + <%= link_to entry.display_name, account_entry_path(entry), data: { turbo_frame: "drawer", turbo_prefetch: false }, class: "hover:underline hover:text-gray-800" %> diff --git a/config/brakeman.ignore b/config/brakeman.ignore index 6ebccac2e9b..ce280c4d1ce 100644 --- a/config/brakeman.ignore +++ b/config/brakeman.ignore @@ -23,6 +23,29 @@ ], "note": "" }, + { + "warning_type": "Mass Assignment", + "warning_code": 105, + "fingerprint": "5bfdb129316655dc4e02f3a599156660414a6562212a5f61057d376f6888f078", + "check_name": "PermitAttributes", + "message": "Potentially dangerous key allowed for mass assignment", + "file": "app/controllers/concerns/entryable_resource.rb", + "line": 122, + "link": "https://brakemanscanner.org/docs/warning_types/mass_assignment/", + "code": "params.require(:account_entry).permit(:account_id, :name, :enriched_name, :date, :amount, :currency, :excluded, :notes, :nature, :entryable_attributes => self.class.permitted_entryable_attributes)", + "render_path": null, + "location": { + "type": "method", + "class": "EntryableResource", + "method": "entry_params" + }, + "user_input": ":account_id", + "confidence": "High", + "cwe_id": [ + 915 + ], + "note": "" + }, { "warning_type": "Mass Assignment", "warning_code": 105, @@ -80,29 +103,6 @@ ], "note": "" }, - { - "warning_type": "Mass Assignment", - "warning_code": 105, - "fingerprint": "f158202dcc66f2273ddea5e5296bad7146a50ca6667f49c77372b5b234542334", - "check_name": "PermitAttributes", - "message": "Potentially dangerous key allowed for mass assignment", - "file": "app/controllers/concerns/entryable_resource.rb", - "line": 122, - "link": "https://brakemanscanner.org/docs/warning_types/mass_assignment/", - "code": "params.require(:account_entry).permit(:account_id, :name, :date, :amount, :currency, :excluded, :notes, :nature, :entryable_attributes => self.class.permitted_entryable_attributes)", - "render_path": null, - "location": { - "type": "method", - "class": "EntryableResource", - "method": "entry_params" - }, - "user_input": ":account_id", - "confidence": "High", - "cwe_id": [ - 915 - ], - "note": "" - }, { "warning_type": "Dynamic Render Path", "warning_code": 15, @@ -138,6 +138,6 @@ "note": "" } ], - "updated": "2024-11-27 15:33:53 -0500", + "updated": "2024-12-18 17:46:13 -0500", "brakeman_version": "6.2.2" } diff --git a/db/migrate/20241218132503_add_enriched_name_field.rb b/db/migrate/20241218132503_add_enriched_name_field.rb new file mode 100644 index 00000000000..08d837c401b --- /dev/null +++ b/db/migrate/20241218132503_add_enriched_name_field.rb @@ -0,0 +1,37 @@ +class AddEnrichedNameField < ActiveRecord::Migration[7.2] + def change + add_column :account_entries, :enriched_name, :string + + reversible do |dir| + dir.up do + execute <<-SQL + UPDATE account_entries ae + SET name = CASE ae.entryable_type + WHEN 'Account::Trade' THEN + CASE + WHEN EXISTS ( + SELECT 1 FROM account_trades t + WHERE t.id = ae.entryable_id AND t.qty < 0 + ) THEN 'Sell trade' + ELSE 'Buy trade' + END + WHEN 'Account::Transaction' THEN + CASE + WHEN ae.amount > 0 THEN 'Expense' + ELSE 'Income' + END + WHEN 'Account::Valuation' THEN 'Balance update' + ELSE 'Unknown entry' + END + WHERE name IS NULL + SQL + + change_column_null :account_entries, :name, false + end + + dir.down do + change_column_null :account_entries, :name, true + end + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 5ae96e5274b..7beb5097e92 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: 2024_12_17_141716) do +ActiveRecord::Schema[7.2].define(version: 2024_12_18_132503) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -39,7 +39,7 @@ t.decimal "amount", precision: 19, scale: 4 t.string "currency" t.date "date" - t.string "name" + t.string "name", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false t.uuid "transfer_id" @@ -49,6 +49,7 @@ t.boolean "excluded", default: false t.string "plaid_id" t.datetime "enriched_at" + t.string "enriched_name" t.index ["account_id"], name: "index_account_entries_on_account_id" t.index ["import_id"], name: "index_account_entries_on_import_id" t.index ["transfer_id"], name: "index_account_entries_on_transfer_id" diff --git a/test/interfaces/accountable_resource_interface_test.rb b/test/interfaces/accountable_resource_interface_test.rb index c1fe92089b5..d44baf7e581 100644 --- a/test/interfaces/accountable_resource_interface_test.rb +++ b/test/interfaces/accountable_resource_interface_test.rb @@ -76,7 +76,7 @@ module AccountableResourceInterfaceTest end test "updates account balance by editing existing valuation for today" do - @account.entries.create! date: Date.current, amount: 6000, currency: "USD", entryable: Account::Valuation.new + @account.entries.create! date: Date.current, amount: 6000, currency: "USD", name: "Balance update", entryable: Account::Valuation.new assert_no_difference [ "Account::Entry.count", "Account::Valuation.count" ] do patch account_url(@account), params: { diff --git a/test/models/account/entry_test.rb b/test/models/account/entry_test.rb index 9c541b6bbe9..5a9d9ec9587 100644 --- a/test/models/account/entry_test.rb +++ b/test/models/account/entry_test.rb @@ -90,10 +90,4 @@ class Account::EntryTest < ActiveSupport::TestCase assert_equal Money.new(-200), family.entries.income_total("USD") end - - # See: https://github.com/maybe-finance/maybe/wiki/vision#signage-of-money - test "transactions with negative amounts are inflows, positive amounts are outflows to an account" do - assert create_transaction(amount: -10).inflow? - assert create_transaction(amount: 10).outflow? - end end diff --git a/test/system/trades_test.rb b/test/system/trades_test.rb index 9e26b7083a4..9ca85c08875 100644 --- a/test/system/trades_test.rb +++ b/test/system/trades_test.rb @@ -23,7 +23,7 @@ class TradesTest < ApplicationSystemTestCase end test "can create buy transaction" do - shares_qty = 25.0 + shares_qty = 25 open_new_trade_modal @@ -38,7 +38,7 @@ class TradesTest < ApplicationSystemTestCase visit_account_trades within_trades do - assert_text "Buy 10.0 shares of AAPL" + assert_text "Purchase 10 shares of AAPL" assert_text "Buy #{shares_qty} shares of AAPL" end end @@ -60,7 +60,7 @@ class TradesTest < ApplicationSystemTestCase visit_account_trades within_trades do - assert_text "Sell #{aapl.qty} shares of AAPL" + assert_text "Sell #{aapl.qty.round} shares of AAPL" end end