From f1991eaefe58e9cb37dbe4ab78b2ad85eee7c7a7 Mon Sep 17 00:00:00 2001 From: soky srm Date: Thu, 26 Mar 2026 19:01:35 +0100 Subject: [PATCH] Recurring scoping implementation (#1300) * Recurring scoping implementation * FIX tests and reviews --- .../recurring_transactions_controller.rb | 5 +- app/controllers/transactions_controller.rb | 2 + app/models/account.rb | 1 + app/models/recurring_transaction.rb | 23 ++- .../recurring_transaction/identifier.rb | 14 +- ...dd_account_id_to_recurring_transactions.rb | 64 +++++++ db/schema.rb | 9 +- .../transactions_controller_test.rb | 1 + test/fixtures/recurring_transactions.yml | 2 + .../recurring_transaction/identifier_test.rb | 1 + test/models/recurring_transaction_test.rb | 161 ++++++++++++++---- 11 files changed, 232 insertions(+), 51 deletions(-) create mode 100644 db/migrate/20260326112218_add_account_id_to_recurring_transactions.rb diff --git a/app/controllers/recurring_transactions_controller.rb b/app/controllers/recurring_transactions_controller.rb index 0f06b52fb..723e166d8 100644 --- a/app/controllers/recurring_transactions_controller.rb +++ b/app/controllers/recurring_transactions_controller.rb @@ -3,6 +3,7 @@ class RecurringTransactionsController < ApplicationController def index @recurring_transactions = Current.family.recurring_transactions + .accessible_by(Current.user) .includes(:merchant) .order(status: :asc, next_expected_date: :asc) @family = Current.family @@ -42,7 +43,7 @@ class RecurringTransactionsController < ApplicationController end def toggle_status - @recurring_transaction = Current.family.recurring_transactions.find(params[:id]) + @recurring_transaction = Current.family.recurring_transactions.accessible_by(Current.user).find(params[:id]) if @recurring_transaction.active? @recurring_transaction.mark_inactive! @@ -61,7 +62,7 @@ class RecurringTransactionsController < ApplicationController end def destroy - @recurring_transaction = Current.family.recurring_transactions.find(params[:id]) + @recurring_transaction = Current.family.recurring_transactions.accessible_by(Current.user).find(params[:id]) @recurring_transaction.destroy! flash[:notice] = t("recurring_transactions.deleted") diff --git a/app/controllers/transactions_controller.rb b/app/controllers/transactions_controller.rb index e987d9aa1..dc79aaeb6 100644 --- a/app/controllers/transactions_controller.rb +++ b/app/controllers/transactions_controller.rb @@ -54,6 +54,7 @@ class TransactionsController < ApplicationController # Load projected recurring transactions for next 10 days @projected_recurring = Current.family.recurring_transactions + .accessible_by(Current.user) .active .where("next_expected_date <= ? AND next_expected_date >= ?", 10.days.from_now.to_date, @@ -304,6 +305,7 @@ class TransactionsController < ApplicationController # Check if a recurring transaction already exists for this pattern existing = Current.family.recurring_transactions.find_by( + account_id: transaction.entry.account_id, merchant_id: transaction.merchant_id, name: transaction.merchant_id.present? ? nil : transaction.entry.name, currency: transaction.entry.currency, diff --git a/app/models/account.rb b/app/models/account.rb index 0224f967d..5de43f3c0 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -19,6 +19,7 @@ class Account < ApplicationRecord has_many :trades, through: :entries, source: :entryable, source_type: "Trade" has_many :holdings, dependent: :destroy has_many :balances, dependent: :destroy + has_many :recurring_transactions, dependent: :destroy monetize :balance, :cash_balance diff --git a/app/models/recurring_transaction.rb b/app/models/recurring_transaction.rb index bc8b2e8b3..eb2784ef5 100644 --- a/app/models/recurring_transaction.rb +++ b/app/models/recurring_transaction.rb @@ -2,6 +2,7 @@ class RecurringTransaction < ApplicationRecord include Monetizable belongs_to :family + belongs_to :account, optional: true belongs_to :merchant, optional: true monetize :amount @@ -35,6 +36,9 @@ class RecurringTransaction < ApplicationRecord scope :for_family, ->(family) { where(family: family) } scope :expected_soon, -> { active.where("next_expected_date <= ?", 1.month.from_now) } + scope :accessible_by, ->(user) { + where(account_id: Account.accessible_by(user).select(:id)).or(where(account_id: nil)) + } # Class methods for identification and cleanup # Schedules pattern identification with debounce to run after all syncs complete @@ -66,7 +70,8 @@ class RecurringTransaction < ApplicationRecord name: transaction.merchant_id.present? ? nil : entry.name, currency: entry.currency, expected_day: expected_day, - lookback_months: 6 + lookback_months: 6, + account: entry.account ) # Calculate amount variance from historical data @@ -89,6 +94,7 @@ class RecurringTransaction < ApplicationRecord create!( family: family, + account: entry.account, merchant_id: transaction.merchant_id, name: transaction.merchant_id.present? ? nil : entry.name, amount: entry.amount, @@ -106,10 +112,10 @@ class RecurringTransaction < ApplicationRecord end # Find matching transaction entries for variance calculation - def self.find_matching_transaction_entries(family:, merchant_id:, name:, currency:, expected_day:, lookback_months: 6) + def self.find_matching_transaction_entries(family:, merchant_id:, name:, currency:, expected_day:, lookback_months: 6, account: nil) lookback_date = lookback_months.months.ago.to_date - entries = family.entries + entries = (account.present? ? account.entries : family.entries) .where(entryable_type: "Transaction") .where(currency: currency) .where("entries.date >= ?", lookback_date) @@ -131,14 +137,15 @@ class RecurringTransaction < ApplicationRecord end # Find matching transaction amounts for variance calculation - def self.find_matching_transaction_amounts(family:, merchant_id:, name:, currency:, expected_day:, lookback_months: 6) + def self.find_matching_transaction_amounts(family:, merchant_id:, name:, currency:, expected_day:, lookback_months: 6, account: nil) matching_entries = find_matching_transaction_entries( family: family, merchant_id: merchant_id, name: name, currency: currency, expected_day: expected_day, - lookback_months: lookback_months + lookback_months: lookback_months, + account: account ) matching_entries.map(&:amount) @@ -173,8 +180,10 @@ class RecurringTransaction < ApplicationRecord def matching_transactions # For manual recurring with amount variance, match within range # For automatic recurring, match exact amount + base = account.present? ? account.entries : family.entries + entries = if manual? && has_amount_variance? - family.entries + base .where(entryable_type: "Transaction") .where(currency: currency) .where("entries.amount BETWEEN ? AND ?", expected_amount_min, expected_amount_max) @@ -183,7 +192,7 @@ class RecurringTransaction < ApplicationRecord [ expected_day_of_month + 2, 31 ].min) .order(date: :desc) else - family.entries + base .where(entryable_type: "Transaction") .where(currency: currency) .where("entries.amount = ?", amount) diff --git a/app/models/recurring_transaction/identifier.rb b/app/models/recurring_transaction/identifier.rb index 916b77483..82834c26a 100644 --- a/app/models/recurring_transaction/identifier.rb +++ b/app/models/recurring_transaction/identifier.rb @@ -24,12 +24,12 @@ class RecurringTransaction transaction = entry.entryable # Use merchant_id if present, otherwise use entry name identifier = transaction.merchant_id.present? ? [ :merchant, transaction.merchant_id ] : [ :name, entry.name ] - [ identifier, entry.amount.round(2), entry.currency ] + [ identifier, entry.amount.round(2), entry.currency, entry.account_id ] end recurring_patterns = [] - grouped_transactions.each do |(identifier, amount, currency), entries| + grouped_transactions.each do |(identifier, amount, currency, account_id), entries| next if entries.size < 3 # Must have at least 3 occurrences # Check if the last occurrence was within the last 45 days @@ -49,6 +49,7 @@ class RecurringTransaction pattern = { amount: amount, currency: currency, + account_id: account_id, expected_day_of_month: expected_day, last_occurrence_date: last_occurrence.date, occurrence_count: entries.size, @@ -70,7 +71,8 @@ class RecurringTransaction # Build find conditions based on whether it's merchant-based or name-based find_conditions = { amount: pattern[:amount], - currency: pattern[:currency] + currency: pattern[:currency], + account_id: pattern[:account_id] } if pattern[:merchant_id].present? @@ -148,7 +150,8 @@ class RecurringTransaction name: recurring.name, currency: recurring.currency, expected_day: recurring.expected_day_of_month, - lookback_months: 6 + lookback_months: 6, + account: recurring.account ) next if matching_entries.empty? @@ -180,7 +183,8 @@ class RecurringTransaction name: recurring_transaction.name, currency: recurring_transaction.currency, expected_day: recurring_transaction.expected_day_of_month, - lookback_months: 6 + lookback_months: 6, + account: recurring_transaction.account ) # Update if we have any matching transactions diff --git a/db/migrate/20260326112218_add_account_id_to_recurring_transactions.rb b/db/migrate/20260326112218_add_account_id_to_recurring_transactions.rb new file mode 100644 index 000000000..d8289f59d --- /dev/null +++ b/db/migrate/20260326112218_add_account_id_to_recurring_transactions.rb @@ -0,0 +1,64 @@ +class AddAccountIdToRecurringTransactions < ActiveRecord::Migration[7.2] + def up + add_reference :recurring_transactions, :account, type: :uuid, null: true, foreign_key: true + + # Backfill account_id from the most recent matching entry + execute <<~SQL + UPDATE recurring_transactions rt + SET account_id = subquery.account_id + FROM ( + SELECT DISTINCT ON (rt2.id) rt2.id AS recurring_transaction_id, e.account_id + FROM recurring_transactions rt2 + JOIN entries e ON e.entryable_type = 'Transaction' + AND e.currency = rt2.currency + AND e.amount = rt2.amount + AND EXTRACT(DAY FROM e.date) BETWEEN GREATEST(rt2.expected_day_of_month - 2, 1) AND LEAST(rt2.expected_day_of_month + 2, 31) + JOIN accounts a ON a.id = e.account_id AND a.family_id = rt2.family_id + LEFT JOIN transactions t ON t.id = e.entryable_id + WHERE rt2.account_id IS NULL + AND ( + (rt2.merchant_id IS NOT NULL AND t.merchant_id = rt2.merchant_id) + OR (rt2.merchant_id IS NULL AND e.name = rt2.name) + ) + ORDER BY rt2.id, e.date DESC + ) subquery + WHERE rt.id = subquery.recurring_transaction_id + SQL + + # Remove old unique indexes + remove_index :recurring_transactions, name: "idx_recurring_txns_merchant", if_exists: true + remove_index :recurring_transactions, name: "idx_recurring_txns_name", if_exists: true + + # Add new unique indexes that include account_id + add_index :recurring_transactions, + [ :family_id, :account_id, :merchant_id, :amount, :currency ], + unique: true, + where: "merchant_id IS NOT NULL", + name: "idx_recurring_txns_acct_merchant" + + add_index :recurring_transactions, + [ :family_id, :account_id, :name, :amount, :currency ], + unique: true, + where: "name IS NOT NULL AND merchant_id IS NULL", + name: "idx_recurring_txns_acct_name" + end + + def down + remove_index :recurring_transactions, name: "idx_recurring_txns_acct_merchant", if_exists: true + remove_index :recurring_transactions, name: "idx_recurring_txns_acct_name", if_exists: true + + add_index :recurring_transactions, + [ :family_id, :merchant_id, :amount, :currency ], + unique: true, + where: "merchant_id IS NOT NULL", + name: "idx_recurring_txns_merchant" + + add_index :recurring_transactions, + [ :family_id, :name, :amount, :currency ], + unique: true, + where: "name IS NOT NULL AND merchant_id IS NULL", + name: "idx_recurring_txns_name" + + remove_reference :recurring_transactions, :account + end +end diff --git a/db/schema.rb b/db/schema.rb index b0d3fd553..388dab435 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_03_24_100003) do +ActiveRecord::Schema[7.2].define(version: 2026_03_26_112218) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -1090,8 +1090,10 @@ ActiveRecord::Schema[7.2].define(version: 2026_03_24_100003) do t.decimal "expected_amount_min", precision: 19, scale: 4 t.decimal "expected_amount_max", precision: 19, scale: 4 t.decimal "expected_amount_avg", precision: 19, scale: 4 - t.index ["family_id", "merchant_id", "amount", "currency"], name: "idx_recurring_txns_merchant", unique: true, where: "(merchant_id IS NOT NULL)" - t.index ["family_id", "name", "amount", "currency"], name: "idx_recurring_txns_name", unique: true, where: "((name IS NOT NULL) AND (merchant_id IS NULL))" + t.uuid "account_id" + t.index ["account_id"], name: "index_recurring_transactions_on_account_id" + t.index ["family_id", "account_id", "merchant_id", "amount", "currency"], name: "idx_recurring_txns_acct_merchant", unique: true, where: "(merchant_id IS NOT NULL)" + t.index ["family_id", "account_id", "name", "amount", "currency"], name: "idx_recurring_txns_acct_name", unique: true, where: "((name IS NOT NULL) AND (merchant_id IS NULL))" t.index ["family_id", "status"], name: "index_recurring_transactions_on_family_id_and_status" t.index ["family_id"], name: "index_recurring_transactions_on_family_id" t.index ["merchant_id"], name: "index_recurring_transactions_on_merchant_id" @@ -1580,6 +1582,7 @@ ActiveRecord::Schema[7.2].define(version: 2026_03_24_100003) do add_foreign_key "oidc_identities", "users" add_foreign_key "plaid_accounts", "plaid_items" add_foreign_key "plaid_items", "families" + add_foreign_key "recurring_transactions", "accounts" add_foreign_key "recurring_transactions", "families" add_foreign_key "recurring_transactions", "merchants" add_foreign_key "rejected_transfers", "transactions", column: "inflow_transaction_id" diff --git a/test/controllers/transactions_controller_test.rb b/test/controllers/transactions_controller_test.rb index fdc9ce851..d7604f47b 100644 --- a/test/controllers/transactions_controller_test.rb +++ b/test/controllers/transactions_controller_test.rb @@ -223,6 +223,7 @@ end # Create existing recurring transaction family.recurring_transactions.create!( + account: account, merchant: merchant, amount: entry.amount, currency: entry.currency, diff --git a/test/fixtures/recurring_transactions.yml b/test/fixtures/recurring_transactions.yml index 768012871..f832a71d5 100644 --- a/test/fixtures/recurring_transactions.yml +++ b/test/fixtures/recurring_transactions.yml @@ -1,5 +1,6 @@ netflix_subscription: family: dylan_family + account: depository merchant: netflix amount: 15.99 currency: USD @@ -11,6 +12,7 @@ netflix_subscription: inactive_subscription: family: dylan_family + account: credit_card merchant: amazon amount: 9.99 currency: USD diff --git a/test/models/recurring_transaction/identifier_test.rb b/test/models/recurring_transaction/identifier_test.rb index d7a7f93be..2b0149886 100644 --- a/test/models/recurring_transaction/identifier_test.rb +++ b/test/models/recurring_transaction/identifier_test.rb @@ -172,6 +172,7 @@ class RecurringTransaction::IdentifierTest < ActiveSupport::TestCase # Create initial recurring transaction existing = @family.recurring_transactions.create!( + account: account, merchant: merchant, amount: 29.99, currency: "USD", diff --git a/test/models/recurring_transaction_test.rb b/test/models/recurring_transaction_test.rb index a7f638c3c..7b1b506f6 100644 --- a/test/models/recurring_transaction_test.rb +++ b/test/models/recurring_transaction_test.rb @@ -4,6 +4,7 @@ class RecurringTransactionTest < ActiveSupport::TestCase def setup @family = families(:dylan_family) @merchant = merchants(:netflix) + @account = accounts(:depository) # Clear any existing recurring transactions @family.recurring_transactions.destroy_all end @@ -11,13 +12,12 @@ class RecurringTransactionTest < ActiveSupport::TestCase test "identify_patterns_for creates recurring transactions for patterns with 3+ occurrences" do # Create a series of transactions with same merchant and amount on similar days # Use dates within the last 3 months: today, 1 month ago, 2 months ago - account = @family.accounts.first [ 0, 1, 2 ].each do |months_ago| transaction = Transaction.create!( merchant: @merchant, category: categories(:food_and_drink) ) - account.entries.create!( + @account.entries.create!( date: months_ago.months.ago.beginning_of_month + 5.days, amount: 15.99, currency: "USD", @@ -32,6 +32,7 @@ class RecurringTransactionTest < ActiveSupport::TestCase recurring = @family.recurring_transactions.last assert_equal @merchant, recurring.merchant + assert_equal @account, recurring.account assert_equal 15.99, recurring.amount assert_equal "USD", recurring.currency assert_equal "active", recurring.status @@ -40,13 +41,12 @@ class RecurringTransactionTest < ActiveSupport::TestCase test "identify_patterns_for does not create recurring transaction for less than 3 occurrences" do # Create only 2 transactions - account = @family.accounts.first 2.times do |i| transaction = Transaction.create!( merchant: @merchant, category: categories(:food_and_drink) ) - account.entries.create!( + @account.entries.create!( date: (i + 1).months.ago.beginning_of_month + 5.days, amount: 15.99, currency: "USD", @@ -62,6 +62,7 @@ class RecurringTransactionTest < ActiveSupport::TestCase test "calculate_next_expected_date handles end of month correctly" do recurring = @family.recurring_transactions.create!( + account: @account, merchant: @merchant, amount: 29.99, currency: "USD", @@ -78,6 +79,7 @@ class RecurringTransactionTest < ActiveSupport::TestCase test "should_be_inactive? returns true when last occurrence is over 2 months ago" do recurring = @family.recurring_transactions.create!( + account: @account, merchant: merchants(:amazon), amount: 19.99, currency: "USD", @@ -92,6 +94,7 @@ class RecurringTransactionTest < ActiveSupport::TestCase test "should_be_inactive? returns false when last occurrence is within 2 months" do recurring = @family.recurring_transactions.create!( + account: @account, merchant: merchants(:amazon), amount: 25.99, currency: "USD", @@ -106,6 +109,7 @@ class RecurringTransactionTest < ActiveSupport::TestCase test "cleanup_stale_for marks inactive when no recent occurrences" do recurring = @family.recurring_transactions.create!( + account: @account, merchant: merchants(:amazon), amount: 35.99, currency: "USD", @@ -122,6 +126,7 @@ class RecurringTransactionTest < ActiveSupport::TestCase test "record_occurrence! updates recurring transaction with new occurrence" do recurring = @family.recurring_transactions.create!( + account: @account, merchant: merchants(:amazon), amount: 45.99, currency: "USD", @@ -143,13 +148,12 @@ class RecurringTransactionTest < ActiveSupport::TestCase test "identify_patterns_for preserves sign for income transactions" do # Create recurring income transactions (negative amounts) - account = @family.accounts.first [ 0, 1, 2 ].each do |months_ago| transaction = Transaction.create!( merchant: @merchant, category: categories(:income) ) - account.entries.create!( + @account.entries.create!( date: months_ago.months.ago.beginning_of_month + 15.days, amount: -1000.00, currency: "USD", @@ -164,6 +168,7 @@ class RecurringTransactionTest < ActiveSupport::TestCase recurring = @family.recurring_transactions.last assert_equal @merchant, recurring.merchant + assert_equal @account, recurring.account assert_equal(-1000.00, recurring.amount) assert recurring.amount.negative?, "Income should have negative amount" assert_equal "USD", recurring.currency @@ -172,12 +177,11 @@ class RecurringTransactionTest < ActiveSupport::TestCase test "identify_patterns_for creates name-based recurring transactions for transactions without merchants" do # Create transactions without merchants (e.g., from CSV imports or standard accounts) - account = @family.accounts.first [ 0, 1, 2 ].each do |months_ago| transaction = Transaction.create!( category: categories(:food_and_drink) ) - account.entries.create!( + @account.entries.create!( date: months_ago.months.ago.beginning_of_month + 10.days, amount: 25.00, currency: "USD", @@ -192,6 +196,7 @@ class RecurringTransactionTest < ActiveSupport::TestCase recurring = @family.recurring_transactions.last assert_nil recurring.merchant + assert_equal @account, recurring.account assert_equal "Local Coffee Shop", recurring.name assert_equal 25.00, recurring.amount assert_equal "USD", recurring.currency @@ -201,7 +206,6 @@ class RecurringTransactionTest < ActiveSupport::TestCase test "identify_patterns_for creates separate patterns for same merchant but different names" do # Create two different recurring transactions from the same merchant - account = @family.accounts.first # First pattern: Netflix Standard [ 0, 1, 2 ].each do |months_ago| @@ -209,7 +213,7 @@ class RecurringTransactionTest < ActiveSupport::TestCase merchant: @merchant, category: categories(:food_and_drink) ) - account.entries.create!( + @account.entries.create!( date: months_ago.months.ago.beginning_of_month + 5.days, amount: 15.99, currency: "USD", @@ -224,7 +228,7 @@ class RecurringTransactionTest < ActiveSupport::TestCase merchant: @merchant, category: categories(:food_and_drink) ) - account.entries.create!( + @account.entries.create!( date: months_ago.months.ago.beginning_of_month + 10.days, amount: 19.99, currency: "USD", @@ -245,14 +249,12 @@ class RecurringTransactionTest < ActiveSupport::TestCase skip "merchant_id is NOT NULL in this schema; name-based patterns disabled" end - account = @family.accounts.first - # Create transactions for pattern [ 0, 1, 2 ].each do |months_ago| transaction = Transaction.create!( category: categories(:food_and_drink) ) - account.entries.create!( + @account.entries.create!( date: months_ago.months.ago.beginning_of_month + 15.days, amount: 50.00, currency: "USD", @@ -272,6 +274,7 @@ class RecurringTransactionTest < ActiveSupport::TestCase test "validation requires either merchant or name" do recurring = @family.recurring_transactions.build( + account: @account, amount: 25.00, currency: "USD", expected_day_of_month: 5, @@ -288,7 +291,6 @@ class RecurringTransactionTest < ActiveSupport::TestCase unless RecurringTransaction.columns_hash["merchant_id"].null skip "merchant_id is NOT NULL in this schema; name-based patterns disabled" end - account = @family.accounts.first # Create merchant-based pattern [ 0, 1, 2 ].each do |months_ago| @@ -296,7 +298,7 @@ class RecurringTransactionTest < ActiveSupport::TestCase merchant: @merchant, category: categories(:food_and_drink) ) - account.entries.create!( + @account.entries.create!( date: months_ago.months.ago.beginning_of_month + 5.days, amount: 15.99, currency: "USD", @@ -310,7 +312,7 @@ class RecurringTransactionTest < ActiveSupport::TestCase transaction = Transaction.create!( category: categories(:one) ) - account.entries.create!( + @account.entries.create!( date: months_ago.months.ago.beginning_of_month + 1.days, amount: 1200.00, currency: "USD", @@ -336,12 +338,11 @@ class RecurringTransactionTest < ActiveSupport::TestCase # Manual recurring transaction tests test "create_from_transaction creates a manual recurring transaction" do - account = @family.accounts.first transaction = Transaction.create!( merchant: @merchant, category: categories(:food_and_drink) ) - entry = account.entries.create!( + entry = @account.entries.create!( date: 2.months.ago, amount: 50.00, currency: "USD", @@ -357,6 +358,7 @@ class RecurringTransactionTest < ActiveSupport::TestCase assert recurring.present? assert recurring.manual? assert_equal @merchant, recurring.merchant + assert_equal @account, recurring.account assert_equal 50.00, recurring.amount assert_equal "USD", recurring.currency assert_equal 2.months.ago.day, recurring.expected_day_of_month @@ -367,8 +369,6 @@ class RecurringTransactionTest < ActiveSupport::TestCase end test "create_from_transaction automatically calculates amount variance from history" do - account = @family.accounts.first - # Create multiple historical transactions with varying amounts on the same day of month amounts = [ 90.00, 100.00, 110.00, 120.00 ] amounts.each_with_index do |amount, i| @@ -376,7 +376,7 @@ class RecurringTransactionTest < ActiveSupport::TestCase merchant: @merchant, category: categories(:food_and_drink) ) - account.entries.create!( + @account.entries.create!( date: (amounts.size - i).months.ago.beginning_of_month + 14.days, # Day 15 amount: amount, currency: "USD", @@ -385,11 +385,12 @@ class RecurringTransactionTest < ActiveSupport::TestCase ) end - # Mark the most recent one as recurring - most_recent_entry = account.entries.order(date: :desc).first + # Mark the most recent one as recurring (find the 120.00 entry we created last) + most_recent_entry = @account.entries.where(amount: 120.00, currency: "USD").order(date: :desc).first recurring = RecurringTransaction.create_from_transaction(most_recent_entry.transaction) assert recurring.manual? + assert_equal @account, recurring.account assert_equal 90.00, recurring.expected_amount_min assert_equal 120.00, recurring.expected_amount_max assert_equal 105.00, recurring.expected_amount_avg # (90 + 100 + 110 + 120) / 4 @@ -399,12 +400,11 @@ class RecurringTransactionTest < ActiveSupport::TestCase end test "create_from_transaction with single transaction sets fixed amount" do - account = @family.accounts.first transaction = Transaction.create!( merchant: @merchant, category: categories(:food_and_drink) ) - entry = account.entries.create!( + entry = @account.entries.create!( date: 1.month.ago, amount: 50.00, currency: "USD", @@ -415,6 +415,7 @@ class RecurringTransactionTest < ActiveSupport::TestCase recurring = RecurringTransaction.create_from_transaction(transaction) assert recurring.manual? + assert_equal @account, recurring.account assert_equal 50.00, recurring.expected_amount_min assert_equal 50.00, recurring.expected_amount_max assert_equal 50.00, recurring.expected_amount_avg @@ -424,10 +425,9 @@ class RecurringTransactionTest < ActiveSupport::TestCase end test "matching_transactions with amount variance matches within range" do - account = @family.accounts.first - # Create manual recurring with variance for day 15 of the month recurring = @family.recurring_transactions.create!( + account: @account, merchant: @merchant, amount: 100.00, currency: "USD", @@ -441,9 +441,9 @@ class RecurringTransactionTest < ActiveSupport::TestCase expected_amount_avg: 100.00 ) - # Create transactions with varying amounts on day 14 (within ±2 days of day 15) + # Create transactions with varying amounts on day 14 (within +/-2 days of day 15) transaction_within_range = Transaction.create!(merchant: @merchant, category: categories(:food_and_drink)) - entry_within = account.entries.create!( + entry_within = @account.entries.create!( date: Date.current.next_month.beginning_of_month + 13.days, # Day 14 amount: 90.00, currency: "USD", @@ -452,7 +452,7 @@ class RecurringTransactionTest < ActiveSupport::TestCase ) transaction_outside_range = Transaction.create!(merchant: @merchant, category: categories(:food_and_drink)) - entry_outside = account.entries.create!( + entry_outside = @account.entries.create!( date: Date.current.next_month.beginning_of_month + 14.days, # Day 15 amount: 150.00, currency: "USD", @@ -468,6 +468,7 @@ class RecurringTransactionTest < ActiveSupport::TestCase test "should_be_inactive? has longer threshold for manual recurring" do # Manual recurring - 6 months threshold manual_recurring = @family.recurring_transactions.create!( + account: @account, merchant: @merchant, amount: 50.00, currency: "USD", @@ -480,6 +481,7 @@ class RecurringTransactionTest < ActiveSupport::TestCase # Auto recurring - 2 months threshold with different amount to avoid unique constraint auto_recurring = @family.recurring_transactions.create!( + account: @account, merchant: @merchant, amount: 60.00, currency: "USD", @@ -496,6 +498,7 @@ class RecurringTransactionTest < ActiveSupport::TestCase test "update_amount_variance updates min/max/avg correctly" do recurring = @family.recurring_transactions.create!( + account: @account, merchant: @merchant, amount: 100.00, currency: "USD", @@ -527,10 +530,9 @@ class RecurringTransactionTest < ActiveSupport::TestCase end test "identify_patterns_for updates variance for manual recurring transactions" do - account = @family.accounts.first - # Create a manual recurring transaction with initial variance manual_recurring = @family.recurring_transactions.create!( + account: @account, merchant: @merchant, amount: 50.00, currency: "USD", @@ -552,7 +554,7 @@ class RecurringTransactionTest < ActiveSupport::TestCase merchant: @merchant, category: categories(:food_and_drink) ) - account.entries.create!( + @account.entries.create!( date: (amounts.size - i).months.ago.beginning_of_month + 14.days, amount: amount, currency: "USD", @@ -578,6 +580,7 @@ class RecurringTransactionTest < ActiveSupport::TestCase test "cleaner does not delete manual recurring transactions" do # Create inactive manual recurring manual_recurring = @family.recurring_transactions.create!( + account: @account, merchant: @merchant, amount: 50.00, currency: "USD", @@ -593,6 +596,7 @@ class RecurringTransactionTest < ActiveSupport::TestCase # Create inactive auto recurring with different merchant auto_recurring = @family.recurring_transactions.create!( + account: @account, merchant: merchants(:amazon), amount: 30.00, currency: "USD", @@ -612,4 +616,93 @@ class RecurringTransactionTest < ActiveSupport::TestCase assert RecurringTransaction.exists?(manual_recurring.id) assert_not RecurringTransaction.exists?(auto_recurring.id) end + + # Account access scoping tests + test "accessible_by scope returns only recurring transactions from accessible accounts" do + admin = users(:family_admin) + member = users(:family_member) + + # depository is shared with family_member (full_control) + # investment is NOT shared with family_member + shared_account = accounts(:depository) + unshared_account = accounts(:investment) + + shared_recurring = @family.recurring_transactions.create!( + account: shared_account, + merchant: @merchant, + amount: 15.99, + currency: "USD", + expected_day_of_month: 5, + last_occurrence_date: 1.month.ago.to_date, + next_expected_date: 5.days.from_now.to_date, + status: "active" + ) + + unshared_recurring = @family.recurring_transactions.create!( + account: unshared_account, + merchant: merchants(:amazon), + amount: 9.99, + currency: "USD", + expected_day_of_month: 15, + last_occurrence_date: 1.month.ago.to_date, + next_expected_date: 5.days.from_now.to_date, + status: "active" + ) + + # Admin (owner of all accounts) sees both + admin_results = @family.recurring_transactions.accessible_by(admin) + assert_includes admin_results, shared_recurring + assert_includes admin_results, unshared_recurring + + # Family member only sees the one from the shared account + member_results = @family.recurring_transactions.accessible_by(member) + assert_includes member_results, shared_recurring + assert_not_includes member_results, unshared_recurring + end + + test "identifier creates per-account patterns for same merchant on different accounts" do + account_a = accounts(:depository) + account_b = accounts(:credit_card) + + # Create pattern on account A + [ 0, 1, 2 ].each do |months_ago| + transaction = Transaction.create!( + merchant: @merchant, + category: categories(:food_and_drink) + ) + account_a.entries.create!( + date: months_ago.months.ago.beginning_of_month + 5.days, + amount: 15.99, + currency: "USD", + name: "Netflix Subscription", + entryable: transaction + ) + end + + # Create same pattern on account B + [ 0, 1, 2 ].each do |months_ago| + transaction = Transaction.create!( + merchant: @merchant, + category: categories(:food_and_drink) + ) + account_b.entries.create!( + date: months_ago.months.ago.beginning_of_month + 5.days, + amount: 15.99, + currency: "USD", + name: "Netflix Subscription", + entryable: transaction + ) + end + + assert_difference "@family.recurring_transactions.count", 2 do + RecurringTransaction.identify_patterns_for!(@family) + end + + recurring_a = @family.recurring_transactions.find_by(account: account_a, merchant: @merchant, amount: 15.99) + recurring_b = @family.recurring_transactions.find_by(account: account_b, merchant: @merchant, amount: 15.99) + + assert recurring_a.present? + assert recurring_b.present? + assert_not_equal recurring_a, recurring_b + end end