From 51579d3731867bcada1d842e26cf4c8c5028e6dd Mon Sep 17 00:00:00 2001 From: Dream <42954461+eureka928@users.noreply.github.com> Date: Mon, 26 Jan 2026 10:53:05 -0500 Subject: [PATCH] FR: Add transaction type as rule condition option (#790) * Add transaction type condition filter for rules Add ability to filter rules by transaction type (income, expense, transfer). This allows users to create rules that differentiate between transactions with the same name but different types. - Add Rule::ConditionFilter::TransactionType with select dropdown - Register in TransactionResource condition_filters - Add tests for income, expense, and transfer filtering Closes #373 * Address PR review feedback for transaction type filter - Fix income filter to exclude transfers and investment_contribution - Fix expense filter to include investment_contribution regardless of sign - Add i18n for option and operator labels - Add tests for edge cases (transfer inflows, investment contributions) Logic now matches Transaction::Search#apply_type_filter for consistency. --- .../rule/condition_filter/transaction_type.rb | 42 +++++ .../rule/registry/transaction_resource.rb | 1 + config/locales/views/rules/en.yml | 6 + test/models/rule/condition_test.rb | 167 ++++++++++++++++++ 4 files changed, 216 insertions(+) create mode 100644 app/models/rule/condition_filter/transaction_type.rb diff --git a/app/models/rule/condition_filter/transaction_type.rb b/app/models/rule/condition_filter/transaction_type.rb new file mode 100644 index 000000000..4c852d236 --- /dev/null +++ b/app/models/rule/condition_filter/transaction_type.rb @@ -0,0 +1,42 @@ +class Rule::ConditionFilter::TransactionType < Rule::ConditionFilter + # Transfer kinds matching Transaction#transfer? method + TRANSFER_KINDS = %w[funds_movement cc_payment loan_payment].freeze + + def type + "select" + end + + def options + [ + [ I18n.t("rules.condition_filters.transaction_type.income"), "income" ], + [ I18n.t("rules.condition_filters.transaction_type.expense"), "expense" ], + [ I18n.t("rules.condition_filters.transaction_type.transfer"), "transfer" ] + ] + end + + def operators + [ [ I18n.t("rules.condition_filters.transaction_type.equal_to"), "=" ] ] + end + + def prepare(scope) + scope.with_entry + end + + def apply(scope, operator, value) + # Logic matches Transaction::Search#apply_type_filter for consistency + case value + when "income" + # Negative amounts, excluding transfers and investment_contribution + scope.where("entries.amount < 0") + .where.not(kind: TRANSFER_KINDS + %w[investment_contribution]) + when "expense" + # Positive amounts OR investment_contribution (regardless of sign), excluding transfers + scope.where("entries.amount >= 0 OR transactions.kind = 'investment_contribution'") + .where.not(kind: TRANSFER_KINDS) + when "transfer" + scope.where(kind: TRANSFER_KINDS) + else + scope + end + end +end diff --git a/app/models/rule/registry/transaction_resource.rb b/app/models/rule/registry/transaction_resource.rb index fac1d6667..3e0117fc7 100644 --- a/app/models/rule/registry/transaction_resource.rb +++ b/app/models/rule/registry/transaction_resource.rb @@ -7,6 +7,7 @@ class Rule::Registry::TransactionResource < Rule::Registry [ Rule::ConditionFilter::TransactionName.new(rule), Rule::ConditionFilter::TransactionAmount.new(rule), + Rule::ConditionFilter::TransactionType.new(rule), Rule::ConditionFilter::TransactionMerchant.new(rule), Rule::ConditionFilter::TransactionCategory.new(rule), Rule::ConditionFilter::TransactionDetails.new(rule), diff --git a/config/locales/views/rules/en.yml b/config/locales/views/rules/en.yml index 5cbd252d5..905dca236 100644 --- a/config/locales/views/rules/en.yml +++ b/config/locales/views/rules/en.yml @@ -37,3 +37,9 @@ en: pending: Pending success: Success failed: Failed + condition_filters: + transaction_type: + income: Income + expense: Expense + transfer: Transfer + equal_to: Equal to diff --git a/test/models/rule/condition_test.rb b/test/models/rule/condition_test.rb index 353963629..774e77812 100644 --- a/test/models/rule/condition_test.rb +++ b/test/models/rule/condition_test.rb @@ -331,4 +331,171 @@ class Rule::ConditionTest < ActiveSupport::TestCase assert_equal 4, filtered.count assert_not filtered.map(&:id).include?(transaction_entry.transaction.id) end + + test "applies transaction_type condition for income" do + scope = @rule_scope + + condition = Rule::Condition.new( + rule: @transaction_rule, + condition_type: "transaction_type", + operator: "=", + value: "income" + ) + + scope = condition.prepare(scope) + filtered = condition.apply(scope) + + # transaction2 has amount -200 (income) + assert_equal 1, filtered.count + assert filtered.all? { |t| t.entry.amount.negative? } + end + + test "applies transaction_type condition for expense" do + scope = @rule_scope + + condition = Rule::Condition.new( + rule: @transaction_rule, + condition_type: "transaction_type", + operator: "=", + value: "expense" + ) + + scope = condition.prepare(scope) + filtered = condition.apply(scope) + + # transaction1, 3, 4, 5 have positive amounts (expenses) + assert_equal 4, filtered.count + assert filtered.all? { |t| t.entry.amount.positive? && !t.transfer? } + end + + test "applies transaction_type condition for transfer" do + scope = @rule_scope + + # Create a transfer transaction + transfer_entry = create_transaction( + date: Date.current, + account: @account, + amount: 500, + name: "Transfer to savings" + ) + transfer_entry.transaction.update!(kind: "funds_movement") + + condition = Rule::Condition.new( + rule: @transaction_rule, + condition_type: "transaction_type", + operator: "=", + value: "transfer" + ) + + scope = condition.prepare(scope) + filtered = condition.apply(scope) + + assert_equal 1, filtered.count + assert_equal transfer_entry.transaction.id, filtered.first.id + assert filtered.first.transfer? + end + + test "transaction_type expense excludes transfers" do + scope = @rule_scope + + # Create a transfer with positive amount (would look like expense) + transfer_entry = create_transaction( + date: Date.current, + account: @account, + amount: 500, + name: "Transfer to savings" + ) + transfer_entry.transaction.update!(kind: "funds_movement") + + condition = Rule::Condition.new( + rule: @transaction_rule, + condition_type: "transaction_type", + operator: "=", + value: "expense" + ) + + scope = condition.prepare(scope) + filtered = condition.apply(scope) + + # Should NOT include the transfer even though it has positive amount + assert_not filtered.map(&:id).include?(transfer_entry.transaction.id) + end + + test "transaction_type income excludes transfers" do + scope = @rule_scope + + # Create a transfer inflow (negative amount) + transfer_entry = create_transaction( + date: Date.current, + account: @account, + amount: -500, + name: "Transfer from savings" + ) + transfer_entry.transaction.update!(kind: "funds_movement") + + condition = Rule::Condition.new( + rule: @transaction_rule, + condition_type: "transaction_type", + operator: "=", + value: "income" + ) + + scope = condition.prepare(scope) + filtered = condition.apply(scope) + + # Should NOT include the transfer even though it has negative amount + assert_not filtered.map(&:id).include?(transfer_entry.transaction.id) + end + + test "transaction_type expense includes investment_contribution regardless of amount sign" do + scope = @rule_scope + + # Create investment contribution with negative amount (inflow from provider) + contribution_entry = create_transaction( + date: Date.current, + account: @account, + amount: -1000, + name: "401k contribution" + ) + contribution_entry.transaction.update!(kind: "investment_contribution") + + condition = Rule::Condition.new( + rule: @transaction_rule, + condition_type: "transaction_type", + operator: "=", + value: "expense" + ) + + scope = condition.prepare(scope) + filtered = condition.apply(scope) + + # Should include investment_contribution even with negative amount + assert filtered.map(&:id).include?(contribution_entry.transaction.id) + end + + test "transaction_type income excludes investment_contribution" do + scope = @rule_scope + + # Create investment contribution with negative amount + contribution_entry = create_transaction( + date: Date.current, + account: @account, + amount: -1000, + name: "401k contribution" + ) + contribution_entry.transaction.update!(kind: "investment_contribution") + + condition = Rule::Condition.new( + rule: @transaction_rule, + condition_type: "transaction_type", + operator: "=", + value: "income" + ) + + scope = condition.prepare(scope) + filtered = condition.apply(scope) + + # Should NOT include investment_contribution even with negative amount + assert_not filtered.map(&:id).include?(contribution_entry.transaction.id) + end end