diff --git a/app/models/rule/action_executor/set_transaction_tags.rb b/app/models/rule/action_executor/set_transaction_tags.rb index c1dbc30a2..80166950e 100644 --- a/app/models/rule/action_executor/set_transaction_tags.rb +++ b/app/models/rule/action_executor/set_transaction_tags.rb @@ -9,6 +9,7 @@ class Rule::ActionExecutor::SetTransactionTags < Rule::ActionExecutor def execute(transaction_scope, value: nil, ignore_attribute_locks: false, rule_run: nil) tag = family.tags.find_by_id(value) + return 0 unless tag scope = transaction_scope @@ -17,9 +18,14 @@ class Rule::ActionExecutor::SetTransactionTags < Rule::ActionExecutor end count_modified_resources(scope) do |txn| + # Merge the new tag with existing tags instead of replacing them + # This preserves tags set by users or other rules + existing_tag_ids = txn.tag_ids || [] + merged_tag_ids = (existing_tag_ids + [ tag.id ]).uniq + txn.enrich_attribute( :tag_ids, - [ tag.id ], + merged_tag_ids, source: "rule" ) end diff --git a/test/models/account/provider_import_adapter_test.rb b/test/models/account/provider_import_adapter_test.rb index 9fe7df7d8..2f004632f 100644 --- a/test/models/account/provider_import_adapter_test.rb +++ b/test/models/account/provider_import_adapter_test.rb @@ -1219,6 +1219,41 @@ class Account::ProviderImportAdapterTest < ActiveSupport::TestCase end end + test "preserves transaction tags when re-importing existing transaction" do + tag = Tag.create!(name: "Salary", family: @family) + + # Create initial transaction + entry = @adapter.import_transaction( + external_id: "plaid_tag_test", + amount: 1000.00, + currency: "USD", + date: Date.today, + name: "Paycheck", + source: "plaid" + ) + + # Add tag to the transaction (simulating user or rule action) + entry.transaction.tags << tag + entry.transaction.save! + assert_equal [ tag ], entry.transaction.reload.tags + + # Re-import the same transaction with updated data + assert_no_difference "@account.entries.count" do + updated_entry = @adapter.import_transaction( + external_id: "plaid_tag_test", + amount: 1000.00, + currency: "USD", + date: Date.today, + name: "Updated Paycheck Name", + source: "plaid" + ) + + assert_equal entry.id, updated_entry.id + # Tags should be preserved + assert_equal [ tag ], updated_entry.transaction.reload.tags + end + end + test "Plaid pending_transaction_id takes priority over amount matching" do # Create TWO pending transactions with same amount pending1 = @adapter.import_transaction( diff --git a/test/models/rule/action_test.rb b/test/models/rule/action_test.rb index bcb8846af..c437e558b 100644 --- a/test/models/rule/action_test.rb +++ b/test/models/rule/action_test.rb @@ -59,6 +59,51 @@ class Rule::ActionTest < ActiveSupport::TestCase end end + test "set_transaction_tags preserves existing tags" do + existing_tag = @family.tags.create!(name: "Existing tag") + new_tag = @family.tags.create!(name: "New tag from rule") + + # Add existing tag to transaction + @txn2.tags << existing_tag + @txn2.save! + assert_equal [ existing_tag ], @txn2.reload.tags + + action = Rule::Action.new( + rule: @transaction_rule, + action_type: "set_transaction_tags", + value: new_tag.id + ) + + action.apply(@rule_scope) + + # Transaction should have BOTH the existing tag and the new tag + @txn2.reload + assert_includes @txn2.tags, existing_tag + assert_includes @txn2.tags, new_tag + assert_equal 2, @txn2.tags.count + end + + test "set_transaction_tags does not duplicate existing tags" do + tag = @family.tags.create!(name: "Single tag") + + # Add tag to transaction + @txn2.tags << tag + @txn2.save! + assert_equal [ tag ], @txn2.reload.tags + + action = Rule::Action.new( + rule: @transaction_rule, + action_type: "set_transaction_tags", + value: tag.id + ) + + action.apply(@rule_scope) + + # Transaction should still have just one tag (not duplicated) + @txn2.reload + assert_equal [ tag ], @txn2.tags + end + test "set_transaction_merchant" do merchant = @family.merchants.create!(name: "Rule test merchant")