mirror of
https://github.com/we-promise/sure
synced 2026-04-25 17:15:07 +02:00
Reccuring transactions pattern fix (#308)
* Fix pattern identification without merchants - We already support the schema and data, but pattern identification now groups either per merchant or per transaciton name. * Fix missed this view * Fix update schema * Wrong schema pushed
This commit is contained in:
@@ -2,7 +2,7 @@ class RecurringTransaction < ApplicationRecord
|
||||
include Monetizable
|
||||
|
||||
belongs_to :family
|
||||
belongs_to :merchant
|
||||
belongs_to :merchant, optional: true
|
||||
|
||||
monetize :amount
|
||||
|
||||
@@ -11,6 +11,13 @@ class RecurringTransaction < ApplicationRecord
|
||||
validates :amount, presence: true
|
||||
validates :currency, presence: true
|
||||
validates :expected_day_of_month, presence: true, numericality: { greater_than: 0, less_than_or_equal_to: 31 }
|
||||
validate :merchant_or_name_present
|
||||
|
||||
def merchant_or_name_present
|
||||
if merchant_id.blank? && name.blank?
|
||||
errors.add(:base, "Either merchant or name must be present")
|
||||
end
|
||||
end
|
||||
|
||||
scope :for_family, ->(family) { where(family: family) }
|
||||
scope :expected_soon, -> { active.where("next_expected_date <= ?", 1.month.from_now) }
|
||||
@@ -35,9 +42,15 @@ class RecurringTransaction < ApplicationRecord
|
||||
[ expected_day_of_month + 2, 31 ].min)
|
||||
.order(date: :desc)
|
||||
|
||||
# Filter by merchant through the entryable (Transaction)
|
||||
entries.select do |entry|
|
||||
entry.entryable.is_a?(Transaction) && entry.entryable.merchant_id == merchant_id
|
||||
# Filter by merchant or name
|
||||
if merchant_id.present?
|
||||
# Match by merchant through the entryable (Transaction)
|
||||
entries.select do |entry|
|
||||
entry.entryable.is_a?(Transaction) && entry.entryable.merchant_id == merchant_id
|
||||
end
|
||||
else
|
||||
# Match by entry name
|
||||
entries.where(name: name)
|
||||
end
|
||||
end
|
||||
|
||||
@@ -90,6 +103,7 @@ class RecurringTransaction < ApplicationRecord
|
||||
amount: amount,
|
||||
currency: currency,
|
||||
merchant: merchant,
|
||||
name: merchant.present? ? merchant.name : name,
|
||||
recurring: true,
|
||||
projected: true
|
||||
)
|
||||
|
||||
@@ -17,14 +17,19 @@ class RecurringTransaction
|
||||
.includes(:entryable)
|
||||
.to_a
|
||||
|
||||
# Filter to only those with merchants and group by merchant and amount (preserve sign)
|
||||
# Group by merchant (if present) or name, along with amount (preserve sign) and currency
|
||||
grouped_transactions = entries_with_transactions
|
||||
.select { |entry| entry.entryable.is_a?(Transaction) && entry.entryable.merchant_id.present? }
|
||||
.group_by { |entry| [ entry.entryable.merchant_id, entry.amount.round(2), entry.currency ] }
|
||||
.select { |entry| entry.entryable.is_a?(Transaction) }
|
||||
.group_by do |entry|
|
||||
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 ]
|
||||
end
|
||||
|
||||
recurring_patterns = []
|
||||
|
||||
grouped_transactions.each do |(merchant_id, amount, currency), entries|
|
||||
grouped_transactions.each do |(identifier, amount, currency), entries|
|
||||
next if entries.size < 3 # Must have at least 3 occurrences
|
||||
|
||||
# Check if the last occurrence was within the last 45 days
|
||||
@@ -38,8 +43,10 @@ class RecurringTransaction
|
||||
if days_cluster_together?(days_of_month)
|
||||
expected_day = calculate_expected_day(days_of_month)
|
||||
|
||||
recurring_patterns << {
|
||||
merchant_id: merchant_id,
|
||||
# Unpack identifier - either [:merchant, id] or [:name, name_string]
|
||||
identifier_type, identifier_value = identifier
|
||||
|
||||
pattern = {
|
||||
amount: amount,
|
||||
currency: currency,
|
||||
expected_day_of_month: expected_day,
|
||||
@@ -47,16 +54,44 @@ class RecurringTransaction
|
||||
occurrence_count: entries.size,
|
||||
entries: entries
|
||||
}
|
||||
|
||||
# Set either merchant_id or name based on identifier type
|
||||
if identifier_type == :merchant
|
||||
pattern[:merchant_id] = identifier_value
|
||||
else
|
||||
pattern[:name] = identifier_value
|
||||
end
|
||||
|
||||
recurring_patterns << pattern
|
||||
end
|
||||
end
|
||||
|
||||
# Create or update RecurringTransaction records
|
||||
recurring_patterns.each do |pattern|
|
||||
recurring_transaction = family.recurring_transactions.find_or_initialize_by(
|
||||
merchant_id: pattern[:merchant_id],
|
||||
# Build find conditions based on whether it's merchant-based or name-based
|
||||
find_conditions = {
|
||||
amount: pattern[:amount],
|
||||
currency: pattern[:currency]
|
||||
)
|
||||
}
|
||||
|
||||
if pattern[:merchant_id].present?
|
||||
find_conditions[:merchant_id] = pattern[:merchant_id]
|
||||
find_conditions[:name] = nil
|
||||
else
|
||||
find_conditions[:name] = pattern[:name]
|
||||
find_conditions[:merchant_id] = nil
|
||||
end
|
||||
|
||||
recurring_transaction = family.recurring_transactions.find_or_initialize_by(find_conditions)
|
||||
|
||||
# Set the name or merchant_id on new records
|
||||
if recurring_transaction.new_record?
|
||||
if pattern[:merchant_id].present?
|
||||
recurring_transaction.merchant_id = pattern[:merchant_id]
|
||||
else
|
||||
recurring_transaction.name = pattern[:name]
|
||||
end
|
||||
end
|
||||
|
||||
recurring_transaction.assign_attributes(
|
||||
expected_day_of_month: pattern[:expected_day_of_month],
|
||||
|
||||
@@ -4,14 +4,23 @@
|
||||
<div class="pr-4 lg:pr-10 flex items-center gap-3 lg:gap-4 col-span-8">
|
||||
<div class="max-w-full">
|
||||
<%= content_tag :div, class: ["flex items-center gap-2"] do %>
|
||||
<% if recurring_transaction.merchant&.logo_url.present? %>
|
||||
<%= image_tag recurring_transaction.merchant.logo_url,
|
||||
class: "w-6 h-6 rounded-full",
|
||||
loading: "lazy" %>
|
||||
<% if recurring_transaction.merchant.present? %>
|
||||
<% if recurring_transaction.merchant.logo_url.present? %>
|
||||
<%= image_tag recurring_transaction.merchant.logo_url,
|
||||
class: "w-6 h-6 rounded-full",
|
||||
loading: "lazy" %>
|
||||
<% else %>
|
||||
<%= render DS::FilledIcon.new(
|
||||
variant: :text,
|
||||
text: recurring_transaction.merchant.name,
|
||||
size: "sm",
|
||||
rounded: true
|
||||
) %>
|
||||
<% end %>
|
||||
<% else %>
|
||||
<%= render DS::FilledIcon.new(
|
||||
variant: :text,
|
||||
text: recurring_transaction.merchant.name,
|
||||
text: recurring_transaction.name,
|
||||
size: "sm",
|
||||
rounded: true
|
||||
) %>
|
||||
@@ -21,7 +30,7 @@
|
||||
<div class="space-y-0.5">
|
||||
<div class="flex items-center gap-2 min-w-0">
|
||||
<div class="truncate flex-shrink">
|
||||
<%= recurring_transaction.merchant.name %>
|
||||
<%= recurring_transaction.merchant.present? ? recurring_transaction.merchant.name : recurring_transaction.name %>
|
||||
</div>
|
||||
|
||||
<div class="flex items-center gap-1 flex-shrink-0">
|
||||
|
||||
@@ -77,19 +77,29 @@
|
||||
<tr class="border-b border-subdued hover:bg-surface-hover <%= "opacity-60" unless recurring_transaction.active? %>">
|
||||
<td class="py-3 px-2 text-sm">
|
||||
<div class="flex items-center gap-2">
|
||||
<% if recurring_transaction.merchant&.logo_url.present? %>
|
||||
<%= image_tag recurring_transaction.merchant.logo_url,
|
||||
class: "w-6 h-6 rounded-full",
|
||||
loading: "lazy" %>
|
||||
<% if recurring_transaction.merchant.present? %>
|
||||
<% if recurring_transaction.merchant.logo_url.present? %>
|
||||
<%= image_tag recurring_transaction.merchant.logo_url,
|
||||
class: "w-6 h-6 rounded-full",
|
||||
loading: "lazy" %>
|
||||
<% else %>
|
||||
<%= render DS::FilledIcon.new(
|
||||
variant: :text,
|
||||
text: recurring_transaction.merchant.name,
|
||||
size: "sm",
|
||||
rounded: true
|
||||
) %>
|
||||
<% end %>
|
||||
<span class="text-primary font-medium"><%= recurring_transaction.merchant.name %></span>
|
||||
<% else %>
|
||||
<%= render DS::FilledIcon.new(
|
||||
variant: :text,
|
||||
text: recurring_transaction.merchant.name,
|
||||
text: recurring_transaction.name,
|
||||
size: "sm",
|
||||
rounded: true
|
||||
) %>
|
||||
<span class="text-primary font-medium"><%= recurring_transaction.name %></span>
|
||||
<% end %>
|
||||
<span class="text-primary font-medium"><%= recurring_transaction.merchant.name %></span>
|
||||
</div>
|
||||
</td>
|
||||
<td class="py-3 px-2 text-sm font-medium <%= recurring_transaction.amount.negative? ? "text-success" : "text-primary" %>">
|
||||
|
||||
@@ -26,7 +26,7 @@ en:
|
||||
title: No recurring transactions found
|
||||
description: Click "Identify Patterns" to automatically detect recurring transactions from your transaction history.
|
||||
table:
|
||||
merchant: Merchant
|
||||
merchant: Name
|
||||
amount: Amount
|
||||
expected_day: Expected Day
|
||||
next_date: Next Date
|
||||
|
||||
@@ -0,0 +1,5 @@
|
||||
class AddNameToRecurringTransactions < ActiveRecord::Migration[7.2]
|
||||
def change
|
||||
add_column :recurring_transactions, :name, :string, if_not_exists: true
|
||||
end
|
||||
end
|
||||
14
db/schema.rb
generated
14
db/schema.rb
generated
@@ -10,7 +10,7 @@
|
||||
#
|
||||
# It's strongly recommended that you check this file into your version control system.
|
||||
|
||||
ActiveRecord::Schema[7.2].define(version: 2025_10_31_132654) do
|
||||
ActiveRecord::Schema[7.2].define(version: 2025_11_10_104411) do
|
||||
# These are extensions that must be enabled in order to support this database
|
||||
enable_extension "pgcrypto"
|
||||
enable_extension "plpgsql"
|
||||
@@ -676,7 +676,7 @@ ActiveRecord::Schema[7.2].define(version: 2025_10_31_132654) do
|
||||
|
||||
create_table "recurring_transactions", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
|
||||
t.uuid "family_id", null: false
|
||||
t.uuid "merchant_id", null: false
|
||||
t.uuid "merchant_id"
|
||||
t.decimal "amount", precision: 19, scale: 4, null: false
|
||||
t.string "currency", null: false
|
||||
t.integer "expected_day_of_month", null: false
|
||||
@@ -686,7 +686,9 @@ ActiveRecord::Schema[7.2].define(version: 2025_10_31_132654) do
|
||||
t.integer "occurrence_count", default: 0, null: false
|
||||
t.datetime "created_at", null: false
|
||||
t.datetime "updated_at", null: false
|
||||
t.index ["family_id", "merchant_id", "amount", "currency"], name: "idx_recurring_txns_on_family_merchant_amount_currency", unique: true
|
||||
t.string "name"
|
||||
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.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"
|
||||
@@ -989,7 +991,7 @@ ActiveRecord::Schema[7.2].define(version: 2025_10_31_132654) do
|
||||
t.string "subtype"
|
||||
end
|
||||
|
||||
add_foreign_key "account_providers", "accounts"
|
||||
add_foreign_key "account_providers", "accounts", on_delete: :cascade
|
||||
add_foreign_key "accounts", "families"
|
||||
add_foreign_key "accounts", "imports"
|
||||
add_foreign_key "accounts", "plaid_accounts"
|
||||
@@ -1003,11 +1005,11 @@ ActiveRecord::Schema[7.2].define(version: 2025_10_31_132654) do
|
||||
add_foreign_key "budgets", "families"
|
||||
add_foreign_key "categories", "families"
|
||||
add_foreign_key "chats", "users"
|
||||
add_foreign_key "entries", "accounts"
|
||||
add_foreign_key "entries", "accounts", on_delete: :cascade
|
||||
add_foreign_key "entries", "imports"
|
||||
add_foreign_key "family_exports", "families"
|
||||
add_foreign_key "holdings", "account_providers"
|
||||
add_foreign_key "holdings", "accounts"
|
||||
add_foreign_key "holdings", "accounts", on_delete: :cascade
|
||||
add_foreign_key "holdings", "securities"
|
||||
add_foreign_key "impersonation_session_logs", "impersonation_sessions"
|
||||
add_foreign_key "impersonation_sessions", "users", column: "impersonated_id"
|
||||
|
||||
@@ -169,4 +169,159 @@ class RecurringTransactionTest < ActiveSupport::TestCase
|
||||
assert_equal "USD", recurring.currency
|
||||
assert_equal "active", recurring.status
|
||||
end
|
||||
|
||||
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!(
|
||||
date: months_ago.months.ago.beginning_of_month + 10.days,
|
||||
amount: 25.00,
|
||||
currency: "USD",
|
||||
name: "Local Coffee Shop",
|
||||
entryable: transaction
|
||||
)
|
||||
end
|
||||
|
||||
assert_difference "@family.recurring_transactions.count", 1 do
|
||||
RecurringTransaction.identify_patterns_for(@family)
|
||||
end
|
||||
|
||||
recurring = @family.recurring_transactions.last
|
||||
assert_nil recurring.merchant
|
||||
assert_equal "Local Coffee Shop", recurring.name
|
||||
assert_equal 25.00, recurring.amount
|
||||
assert_equal "USD", recurring.currency
|
||||
assert_equal "active", recurring.status
|
||||
assert_equal 3, recurring.occurrence_count
|
||||
end
|
||||
|
||||
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|
|
||||
transaction = Transaction.create!(
|
||||
merchant: @merchant,
|
||||
category: categories(:food_and_drink)
|
||||
)
|
||||
account.entries.create!(
|
||||
date: months_ago.months.ago.beginning_of_month + 5.days,
|
||||
amount: 15.99,
|
||||
currency: "USD",
|
||||
name: "Netflix Standard",
|
||||
entryable: transaction
|
||||
)
|
||||
end
|
||||
|
||||
# Second pattern: Netflix Premium
|
||||
[ 0, 1, 2 ].each do |months_ago|
|
||||
transaction = Transaction.create!(
|
||||
merchant: @merchant,
|
||||
category: categories(:food_and_drink)
|
||||
)
|
||||
account.entries.create!(
|
||||
date: months_ago.months.ago.beginning_of_month + 10.days,
|
||||
amount: 19.99,
|
||||
currency: "USD",
|
||||
name: "Netflix Premium",
|
||||
entryable: transaction
|
||||
)
|
||||
end
|
||||
|
||||
# Should create 2 patterns - one for each amount
|
||||
assert_difference "@family.recurring_transactions.count", 2 do
|
||||
RecurringTransaction.identify_patterns_for(@family)
|
||||
end
|
||||
end
|
||||
|
||||
test "matching_transactions works with name-based recurring transactions" do
|
||||
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!(
|
||||
date: months_ago.months.ago.beginning_of_month + 15.days,
|
||||
amount: 50.00,
|
||||
currency: "USD",
|
||||
name: "Gym Membership",
|
||||
entryable: transaction
|
||||
)
|
||||
end
|
||||
|
||||
RecurringTransaction.identify_patterns_for(@family)
|
||||
recurring = @family.recurring_transactions.last
|
||||
|
||||
# Verify matching transactions finds the correct entries
|
||||
matches = recurring.matching_transactions
|
||||
assert_equal 3, matches.size
|
||||
assert matches.all? { |entry| entry.name == "Gym Membership" }
|
||||
end
|
||||
|
||||
test "validation requires either merchant or name" do
|
||||
recurring = @family.recurring_transactions.build(
|
||||
amount: 25.00,
|
||||
currency: "USD",
|
||||
expected_day_of_month: 5,
|
||||
last_occurrence_date: Date.current,
|
||||
next_expected_date: 1.month.from_now.to_date
|
||||
)
|
||||
|
||||
assert_not recurring.valid?
|
||||
assert_includes recurring.errors[:base], "Either merchant or name must be present"
|
||||
end
|
||||
|
||||
test "both merchant-based and name-based patterns can coexist" do
|
||||
account = @family.accounts.first
|
||||
|
||||
# Create merchant-based pattern
|
||||
[ 0, 1, 2 ].each do |months_ago|
|
||||
transaction = Transaction.create!(
|
||||
merchant: @merchant,
|
||||
category: categories(:food_and_drink)
|
||||
)
|
||||
account.entries.create!(
|
||||
date: months_ago.months.ago.beginning_of_month + 5.days,
|
||||
amount: 15.99,
|
||||
currency: "USD",
|
||||
name: "Netflix Subscription",
|
||||
entryable: transaction
|
||||
)
|
||||
end
|
||||
|
||||
# Create name-based pattern (no merchant)
|
||||
[ 0, 1, 2 ].each do |months_ago|
|
||||
transaction = Transaction.create!(
|
||||
category: categories(:one)
|
||||
)
|
||||
account.entries.create!(
|
||||
date: months_ago.months.ago.beginning_of_month + 1.days,
|
||||
amount: 1200.00,
|
||||
currency: "USD",
|
||||
name: "Monthly Rent",
|
||||
entryable: transaction
|
||||
)
|
||||
end
|
||||
|
||||
assert_difference "@family.recurring_transactions.count", 2 do
|
||||
RecurringTransaction.identify_patterns_for(@family)
|
||||
end
|
||||
|
||||
# Verify both types exist
|
||||
merchant_based = @family.recurring_transactions.where.not(merchant_id: nil).first
|
||||
name_based = @family.recurring_transactions.where(merchant_id: nil).first
|
||||
|
||||
assert merchant_based.present?
|
||||
assert_equal @merchant, merchant_based.merchant
|
||||
|
||||
assert name_based.present?
|
||||
assert_equal "Monthly Rent", name_based.name
|
||||
end
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user