mirror of
https://github.com/we-promise/sure.git
synced 2026-04-19 03:54:08 +00:00
Fix tags getting removed after / during bank sync (#634)
* fix: Preserve transaction tags during rule application When rules set tags, they now ADD to existing tags instead of replacing them. This fixes issue #518 where tags were being removed during bank sync. The root cause was that SetTransactionTags called enrich_attribute with just the single tag from the rule, which replaced all existing tags. Now it merges the new tag with existing tags using .uniq to prevent duplicates. This preserves: - User-applied tags that shouldn't be overwritten by rules - Tags from other rules when multiple rules match the same transaction - Tags set during previous syncs * fix: Add nil guard for tag in SetTransactionTags Return early with 0 if the tag is not found, preventing NoMethodError when find_by_id returns nil. This matches the pattern used in SetTransactionMerchant. --------- Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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")
|
||||
|
||||
|
||||
Reference in New Issue
Block a user