feat(rules): support multiple tags in the set transaction tags action (#3397)

* feat(rules): support multiple tags in the set transaction tags action

Fixes #3353. Reuses the existing DS::TagSelect multi-select tag picker
(made generic via attribute:/show_label:) instead of a native
<select multiple>, so the UX matches the rest of the app. Multiple
tag ids are stored as a comma-separated string in the existing
value column, keeping single-tag rows backward compatible with no
migration. Also closes a read-modify-write race in
SetTransactionTags#execute via with_lock.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(rules): address automated review findings on multi-tag actions

- Fix data export/import: multi-tag actions were exported/imported as
  one opaque comma string, losing all but a bogus combined tag on
  restore. Each tag id is now resolved/reconstructed independently,
  with a backward-compatible scalar value_ref for single-tag actions.
- Fix N+1 in Rule::Action#value_display (options queried once per tag).
- Add aria-label to DS::TagSelect's trigger button when show_label is
  false, so the control keeps an accessible name.
- Localize the "to" label in rule action rows (rules.actions.to_label).
- Use a monotonic counter instead of Date.now() for nested form
  indices, closing a same-millisecond collision window.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(rules): batch tag lookups in multi-tag import resolution

Avoids one find_by query per tag name when reconstructing multi-tag
rule actions during import.

* fix(rules): resolve jjmata review findings on multi-tag action

- rules_controller.js: prefix the JS-side nested-form index counter
  with "new_" so it can never collide with the numeric indexes Rails
  assigns to already-persisted conditions/actions on an edit form.
- data_exporter.rb: key the value_ref scalar/array decision off the
  number of tag ids on the action, not the number that still resolve,
  so a partially-orphaned multi-tag action keeps round-tripping as an
  array.
- rule_import.rb: split comma-separated set_transaction_tags values
  into individual tag names during CSV rule import, matching the
  batched resolution already used by Family::DataImporter.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(rules): CSV-quote multi-tag names so commas don't split them

CodeRabbit flagged that a tag name containing a comma (e.g. "Food,
Dining") would be silently split into two tags when round-tripped
through the comma-separated multi-tag value/CSV formats used by
Family::DataExporter, Family::DataImporter, and RuleImport.

Add Rule::Action.encode_multi_value_names/.decode_multi_value_names,
backed by Ruby's CSV line quoting, and use them at all three call
sites instead of a plain join(",")/split(","). A single name without
a comma round-trips byte-identical to before, so existing exports and
CSV rule templates are unaffected.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: GFR <248542187+gfr-free@users.noreply.github.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

app/components/DS/tag_select.html.erb

@@ -11,7 +11,9 @@
      data-action="click@window->tag-select#handleOutsideClick keydown->tag-select#handleKeydown">
   <div class="form-field">
     <div class="form-field__body">
-      <%= form.label :tag_ids, helpers.t("transactions.form.tags_label"), class: "form-field__label" %>
+      <% if show_label %>
+        <%= form.label attribute, label || helpers.t("transactions.form.tags_label"), class: "form-field__label" %>
+      <% end %>
 
       <button type="button"
               class="form-field__input form-field__input--multiselect-trigger min-h-7 flex items-center gap-2 pr-8 <%= "cursor-not-allowed text-subdued" if disabled %>"
@@ -20,6 +22,7 @@
               aria-haspopup="listbox"
               aria-expanded="false"
               aria-controls="<%= menu_id %>"
+              <% unless show_label %> aria-label="<%= label || helpers.t("transactions.form.tags_label") %>" <% end %>
               <%= "disabled" if disabled %>>
         <span class="flex flex-wrap gap-1"
               data-tag-select-target="selectionContainer"

app/components/DS/tag_select.rb

@@ -1,14 +1,17 @@
 class DS::TagSelect < DesignSystemComponent
-  attr_reader :form, :tags, :selected_ids, :disabled, :auto_submit, :update_url,
-              :menu_placement, :offset
+  attr_reader :form, :tags, :selected_ids, :attribute, :label, :show_label, :disabled,
+              :auto_submit, :update_url, :menu_placement, :offset
 
   MENU_PLACEMENTS = %w[auto down up].freeze
 
-  def initialize(form:, tags:, selected_ids:, disabled: false, auto_submit: false,
-                 update_url: nil, menu_placement: :auto, offset: 6)
+  def initialize(form:, tags:, selected_ids:, attribute: :tag_ids, label: nil, show_label: true,
+                 disabled: false, auto_submit: false, update_url: nil, menu_placement: :auto, offset: 6)
     @form = form
     @tags = tags
     @selected_ids = selected_ids.map(&:to_s)
+    @attribute = attribute
+    @label = label
+    @show_label = show_label
     @disabled = disabled
     @auto_submit = auto_submit
     @update_url = update_url
@@ -17,7 +20,7 @@ class DS::TagSelect < DesignSystemComponent
   end
 
   def field_name
-    "#{form.object_name}[tag_ids][]"
+    "#{form.object_name}[#{attribute}][]"
   end
 
   def menu_id

app/controllers/rules_controller.rb

@@ -191,7 +191,7 @@ class RulesController < ApplicationController
           sub_conditions_attributes: [ :id, :condition_type, :operator, :value, :_destroy ]
         ],
         actions_attributes: [
-          :id, :action_type, :value, :_destroy
+          :id, :action_type, :value, :_destroy, { value: [] }
         ]
       )
     end

app/javascript/controllers/rule/actions_controller.js

@@ -7,6 +7,7 @@ export default class extends Controller {
     "destroyField",
     "actionValue",
     "selectTemplate",
+    "multiSelectTemplate",
     "textTemplate"
   ];
 
@@ -32,6 +33,8 @@ export default class extends Controller {
 
     if (actionExecutor.type === "select") {
       this.#buildSelectFor(actionExecutor);
+    } else if (actionExecutor.type === "multi_select") {
+      this.#buildMultiSelectFor();
     } else if (actionExecutor.type === "text") {
       this.#buildTextInputFor();
     } else {
@@ -78,6 +81,17 @@ export default class extends Controller {
     this.actionValueTarget.classList.remove("hidden");
   }
 
+  #buildMultiSelectFor() {
+    // The tag list is baked into the server-rendered template (it doesn't
+    // depend on the selected executor like other select options do), so we
+    // just clone it as-is. The embedded tag-select Stimulus controller
+    // connects automatically once the clone is inserted into the DOM.
+    const template = this.multiSelectTemplateTarget.content.cloneNode(true);
+
+    this.actionValueTarget.appendChild(template);
+    this.actionValueTarget.classList.remove("hidden");
+  }
+
   #buildTextInputFor() {
     // Clone the text template
     const template = this.textTemplateTarget.content.cloneNode(true);

app/javascript/controllers/rules_controller.js

@@ -50,7 +50,12 @@ export default class extends Controller {
   }
 
   #uniqueKey() {
-    return Date.now();
+    // Prefixed so it can never collide with the numeric indexes Rails
+    // assigns to already-persisted conditions/actions when rendering an
+    // edit form (0, 1, 2, ...). A plain monotonic counter starting at 1
+    // would otherwise reuse index 1 and clobber an existing nested record.
+    this.keySequence = (this.keySequence ?? 0) + 1;
+    return `new_${this.keySequence}`;
   }
 
   // Updates the prefix visibility of all conditions and condition groups

app/models/family/data_exporter.rb

@@ -790,14 +790,34 @@ class Family::DataExporter
         return rule_operand(action.value, type: "Merchant", relation: @family.merchants, fallback_to_name: true)
       end
 
-      # Map tag UUIDs to names for portability
+      # Map tag UUIDs to names for portability. Stored as a comma-separated
+      # list of tag ids (see Rule::Action#value=), so each id is resolved
+      # independently rather than treating the whole string as one operand.
       if action.action_type == "set_transaction_tags"
-        return rule_operand(action.value, type: "Tag", relation: @family.tags, fallback_to_name: true)
+        return resolve_multi_tag_operand(action.value)
       end
 
       rule_operand(action.value)
     end
 
+    def resolve_multi_tag_operand(value)
+      ids = value.to_s.split(",")
+      records = ids.map { |id| resolve_rule_operand_record(@family.tags, id, fallback_to_name: true) }
+      names = records.each_with_index.map { |record, i| record&.name || ids[i] }
+      refs = records.compact.map { |record| rule_value_ref("Tag", record) }
+
+      {
+        value: Rule::Action.encode_multi_value_names(names),
+        # A single tag keeps the pre-existing scalar value_ref shape for
+        # backward compatibility with older exports; only genuinely
+        # multi-tag actions use an array. Keyed off `ids.size` (not
+        # `refs.size`) so a partially-orphaned multi-tag action (one tag
+        # since deleted) still round-trips as an array instead of silently
+        # dropping the surviving tag's name on import.
+        value_ref: ids.size <= 1 ? refs.first : refs
+      }
+    end
+
     def rule_operand(value, type: nil, relation: nil, fallback_to_name: false)
       record = relation && resolve_rule_operand_record(relation, value, fallback_to_name: fallback_to_name)
 

app/models/family/data_importer.rb

@@ -1417,6 +1417,13 @@ class Family::DataImporter
 
     def resolve_rule_action_value(action_data)
       action_type = action_data["action_type"]
+
+      # Tags store a comma-separated list of ids (see Rule::Action#value=) and
+      # value_ref may be an array for multi-tag actions, so this needs its own
+      # resolution path rather than the generic scalar one below (which
+      # assumes value_ref is nil or a single hash).
+      return resolve_multi_tag_action_value(action_data) if action_type == "set_transaction_tags"
+
       value = rule_operand_value(action_data)
 
       return value unless value.present?
@@ -1440,14 +1447,28 @@ class Family::DataImporter
         return merchant.id
       end
 
-      # Map tag names to IDs
-      if action_type == "set_transaction_tags"
-        tag = @family.tags.find_by(name: value)
-        tag ||= @family.tags.create!(name: value)
-        return tag.id
+      value
+    end
+
+    def resolve_multi_tag_action_value(action_data)
+      value_ref = action_data["value_ref"]
+      refs = case value_ref
+      when Array then value_ref
+      when Hash then [ value_ref ]
+      else []
       end
 
-      value
+      names = refs.any? ? refs.map { |ref| ref["name"] } : Rule::Action.decode_multi_value_names(action_data["value"])
+      tags_by_name = @family.tags.where(name: names).index_by(&:name)
+
+      tag_ids = names.filter_map do |name|
+        next if name.blank?
+
+        tag = tags_by_name[name] ||= @family.tags.create!(name: name)
+        tag.id
+      end
+
+      tag_ids.join(",")
     end
 
     def rule_operand_value(data)

app/models/rule/action.rb

@@ -20,8 +20,32 @@ class Rule::Action < ApplicationRecord
   after_create_commit :seed_notification_baseline
   after_update_commit :seed_notification_baseline, if: :saved_change_to_action_type?
 
+  # Accepts an Array (e.g. from a multi-select tag input) and stores it as a
+  # comma-separated string in the existing `value` column, so multi-value
+  # actions don't require a schema change. A single scalar value round-trips
+  # unchanged, which keeps existing single-value rows backward compatible.
+  def value=(val)
+    val = val.reject(&:blank?).join(",") if val.is_a?(Array)
+    super(val)
+  end
+
+  # Encodes a list of tag (or other) names as a single comma-separated string
+  # for the portable `value`/CSV-import representation, CSV-quoting any name
+  # that itself contains a comma (e.g. "Food, Dining") so it round-trips as
+  # one name instead of being split into two on import.
+  def self.encode_multi_value_names(names)
+    CSV.generate_line(names, row_sep: "")
+  end
+
+  # Inverse of .encode_multi_value_names. Also accepts a plain unquoted
+  # comma-separated string (the format used before quoting was introduced),
+  # which CSV parses the same way as long as no name contains a comma.
+  def self.decode_multi_value_names(str)
+    CSV.parse_line(str.to_s) || []
+  end
+
   def apply(resource_scope, ignore_attribute_locks: false, rule_run: nil)
-    executor.execute(resource_scope, value: value, ignore_attribute_locks: ignore_attribute_locks, rule_run: rule_run) || 0
+    executor.execute(resource_scope, value: execution_value, ignore_attribute_locks: ignore_attribute_locks, rule_run: rule_run) || 0
   end
 
   def options
@@ -29,15 +53,13 @@ class Rule::Action < ApplicationRecord
   end
 
   def value_display
-    if value.present?
-      if options
-        options.find { |option| option.last == value }&.first
-      else
-        ""
-      end
-    else
-      ""
-    end
+    return "" if value.blank?
+
+    cached_options = options
+    return "" if cached_options.blank?
+
+    labels_by_id = cached_options.to_h { |label, id| [ id.to_s, label ] }
+    Array(execution_value).filter_map { |v| labels_by_id[v] }.join(", ")
   end
 
   def executor
@@ -45,6 +67,10 @@ class Rule::Action < ApplicationRecord
   end
 
   private
+    def execution_value
+      executor.type == "multi_select" ? value.to_s.split(",") : value
+    end
+
     def seed_notification_baseline
       return unless action_type == "send_email_notification"
 

app/models/rule/action_executor.rb

@@ -1,5 +1,5 @@
 class Rule::ActionExecutor
-  TYPES = [ "select", "function", "text" ]
+  TYPES = [ "select", "multi_select", "function", "text" ]
 
   def initialize(rule)
     @rule = rule

app/models/rule/action_executor/set_transaction_tags.rb

@@ -1,6 +1,6 @@
 class Rule::ActionExecutor::SetTransactionTags < Rule::ActionExecutor
   def type
-    "select"
+    "multi_select"
   end
 
   def options
@@ -8,8 +8,9 @@ class Rule::ActionExecutor::SetTransactionTags < Rule::ActionExecutor
   end
 
   def execute(transaction_scope, value: nil, ignore_attribute_locks: false, rule_run: nil)
-    tag = family.tags.find_by_id(value)
-    return 0 unless tag
+    tag_ids = Array(value).compact_blank
+    selected_tag_ids = family.tags.where(id: tag_ids).pluck(:id)
+    return 0 if selected_tag_ids.empty?
 
     scope = transaction_scope
 
@@ -18,17 +19,22 @@ 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
+      # `with_lock` closes the read-modify-write race window: without it, two
+      # concurrent rule applications on the same transaction could each read
+      # the same "before" tag_ids and one write could clobber the other.
+      txn.with_lock do
+        # Merge the selected tags 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 + selected_tag_ids).uniq
 
-      txn.enrich_attribute(
-        :tag_ids,
-        merged_tag_ids,
-        source: "rule",
-        ignore_locks: ignore_attribute_locks
-      )
+        txn.enrich_attribute(
+          :tag_ids,
+          merged_tag_ids,
+          source: "rule",
+          ignore_locks: ignore_attribute_locks
+        )
+      end
     end
   end
 end

app/models/rule_import.rb

@@ -268,19 +268,31 @@ class RuleImport < Import
         return merchant.id
       end
 
-      # Map tag names to UUIDs
+      # Map tag names to UUIDs. `value` may be a comma-separated list of tag
+      # names for multi-tag actions (see Rule::Action#value=), so each name
+      # is resolved independently rather than treating the whole string as
+      # a single tag name.
       if action_type == "set_transaction_tags"
-        tag = family.tags.find_by(name: value)
-        # Create tag if it doesn't exist
-        unless tag
-          tag = family.tags.create!(name: value)
-        end
-        return tag.id
+        return resolve_import_multi_tag_value(value)
       end
 
       value
     end
 
+    def resolve_import_multi_tag_value(value)
+      names = Rule::Action.decode_multi_value_names(value).map(&:strip).reject(&:blank?)
+      return value if names.empty?
+
+      tags_by_name = family.tags.where(name: names).index_by(&:name)
+
+      tag_ids = names.map do |name|
+        tag = tags_by_name[name] ||= family.tags.create!(name: name)
+        tag.id
+      end
+
+      tag_ids.join(",")
+    end
+
     def parse_boolean(value)
       return true if value.to_s.downcase.in?(%w[true 1 yes y])
       return false if value.to_s.downcase.in?(%w[false 0 no n])

app/views/rule/actions/_action.html.erb

@@ -15,9 +15,17 @@
                 data: { rule__actions_target: "actionValue" } do %>
       <%# Initial rendering based on the action executor type. %>
       <%# Subsequent renders are injected by the Stimulus controller, which uses the templates from below. %>
-      <span class="font-medium text-primary uppercase text-xs">to</span>
+      <span class="font-medium text-primary uppercase text-xs"><%= t("rules.actions.to_label") %></span>
       <% if action.executor.type == "select" %>
         <%= form.select :value, action.options || [], {} %>
+      <% elsif action.executor.type == "multi_select" %>
+        <%= render DS::TagSelect.new(
+              form: form,
+              tags: rule.family.tags.alphabetically,
+              selected_ids: action.value.to_s.split(","),
+              attribute: :value,
+              show_label: false
+            ) %>
       <% elsif action.executor.type == "text" %>
         <%= form.text_field :value, placeholder: t("rules.actions.value_placeholder") %>
       <% end %>
@@ -32,12 +40,23 @@
 
   <%# Templates for different input types - these will be cloned and used by the Stimulus controller %>
   <template data-rule--actions-target="selectTemplate">
-    <span class="font-medium text-primary uppercase text-xs">to</span>
+    <span class="font-medium text-primary uppercase text-xs"><%= t("rules.actions.to_label") %></span>
     <%= form.select :value, [], {} %>
   </template>
 
+  <template data-rule--actions-target="multiSelectTemplate">
+    <span class="font-medium text-primary uppercase text-xs"><%= t("rules.actions.to_label") %></span>
+    <%= render DS::TagSelect.new(
+          form: form,
+          tags: rule.family.tags.alphabetically,
+          selected_ids: [],
+          attribute: :value,
+          show_label: false
+        ) %>
+  </template>
+
   <template data-rule--actions-target="textTemplate">
-    <span class="font-medium text-primary uppercase text-xs">to</span>
+    <span class="font-medium text-primary uppercase text-xs"><%= t("rules.actions.to_label") %></span>
     <%= form.text_field :value, placeholder: t("rules.actions.value_placeholder") %>
   </template>
 

config/locales/views/rules/en.yml

@@ -51,6 +51,7 @@ en:
       view_usage_history: View usage history
       confirm_changes: Confirm changes
     actions:
+      to_label: to
       value_placeholder: Enter a value
     update:
       success: Rule updated

test/models/family/data_exporter_test.rb

@@ -582,6 +582,146 @@ class Family::DataExporterTest < ActiveSupport::TestCase
     end
   end
 
+  test "exports rule actions with multiple tags and maps each tag UUID independently" do
+    second_tag = @family.tags.create!(name: "Second Tag", color: "#0000FF")
+
+    tag_rule = @family.rules.build(
+      name: "Multi Tag Rule",
+      resource_type: "transaction",
+      active: true
+    )
+    tag_rule.conditions.build(
+      condition_type: "transaction_name",
+      operator: "like",
+      value: "test"
+    )
+    tag_rule.actions.build(
+      action_type: "set_transaction_tags",
+      value: [ @tag.id, second_tag.id ]
+    )
+    tag_rule.save!
+
+    zip_data = @exporter.generate_export
+
+    Zip::File.open_buffer(zip_data) do |zip|
+      ndjson_content = zip.read("all.ndjson")
+      lines = ndjson_content.split("\n")
+
+      rule_lines = lines.select do |line|
+        parsed = JSON.parse(line)
+        parsed["type"] == "Rule" && parsed["data"]["name"] == "Multi Tag Rule"
+      end
+
+      assert rule_lines.any?
+
+      rule_data = JSON.parse(rule_lines.first)
+      actions = rule_data["data"]["actions"]
+
+      assert_equal 1, actions.length
+      # Should export both tag names, comma-separated, not a single opaque id string
+      assert_equal "Test Tag,Second Tag", actions[0]["value"]
+      assert_equal(
+        [
+          { "type" => "Tag", "id" => @tag.id, "name" => "Test Tag" },
+          { "type" => "Tag", "id" => second_tag.id, "name" => "Second Tag" }
+        ],
+        actions[0]["value_ref"]
+      )
+    end
+  end
+
+  test "exports a multi-tag action's value CSV-quoted when a tag name contains a comma" do
+    comma_tag = @family.tags.create!(name: "Food, Dining", color: "#0000FF")
+
+    tag_rule = @family.rules.build(
+      name: "Comma Tag Name Rule",
+      resource_type: "transaction",
+      active: true
+    )
+    tag_rule.conditions.build(
+      condition_type: "transaction_name",
+      operator: "like",
+      value: "test"
+    )
+    tag_rule.actions.build(
+      action_type: "set_transaction_tags",
+      value: [ @tag.id, comma_tag.id ]
+    )
+    tag_rule.save!
+
+    zip_data = @exporter.generate_export
+
+    Zip::File.open_buffer(zip_data) do |zip|
+      ndjson_content = zip.read("all.ndjson")
+      lines = ndjson_content.split("\n")
+
+      rule_lines = lines.select do |line|
+        parsed = JSON.parse(line)
+        parsed["type"] == "Rule" && parsed["data"]["name"] == "Comma Tag Name Rule"
+      end
+
+      assert rule_lines.any?
+
+      rule_data = JSON.parse(rule_lines.first)
+      actions = rule_data["data"]["actions"]
+
+      # The comma-containing name must be quoted so it round-trips as one
+      # name rather than splitting into "Food" and " Dining" on import.
+      assert_equal "Test Tag,\"Food, Dining\"", actions[0]["value"]
+      assert_equal [ "Test Tag", "Food, Dining" ], CSV.parse_line(actions[0]["value"])
+    end
+  end
+
+  test "exports a partially-orphaned multi-tag action's value_ref as an array" do
+    second_tag = @family.tags.create!(name: "Second Tag", color: "#0000FF")
+
+    tag_rule = @family.rules.build(
+      name: "Orphaned Multi Tag Rule",
+      resource_type: "transaction",
+      active: true
+    )
+    tag_rule.conditions.build(
+      condition_type: "transaction_name",
+      operator: "like",
+      value: "test"
+    )
+    tag_rule.actions.build(
+      action_type: "set_transaction_tags",
+      value: [ @tag.id, second_tag.id ]
+    )
+    tag_rule.save!
+
+    second_tag.destroy!
+
+    zip_data = @exporter.generate_export
+
+    Zip::File.open_buffer(zip_data) do |zip|
+      ndjson_content = zip.read("all.ndjson")
+      lines = ndjson_content.split("\n")
+
+      rule_lines = lines.select do |line|
+        parsed = JSON.parse(line)
+        parsed["type"] == "Rule" && parsed["data"]["name"] == "Orphaned Multi Tag Rule"
+      end
+
+      assert rule_lines.any?
+
+      rule_data = JSON.parse(rule_lines.first)
+      actions = rule_data["data"]["actions"]
+
+      assert_equal 1, actions.length
+      # value_ref should stay an array (not collapse to a scalar Hash) even
+      # though only one of the two original tag ids still resolves, so the
+      # importer's array-handling branch keeps running instead of the
+      # legacy single-tag scalar branch.
+      assert_kind_of Array, actions[0]["value_ref"]
+      assert_equal(
+        [ { "type" => "Tag", "id" => @tag.id, "name" => "Test Tag" } ],
+        actions[0]["value_ref"]
+      )
+    end
+  end
+
   test "exports compound conditions with sub-conditions" do
     # Create a rule with compound conditions
     compound_rule = @family.rules.build(

test/models/family/data_importer_test.rb

@@ -2075,6 +2075,112 @@ class Family::DataImporterTest < ActiveSupport::TestCase
     assert_not @family.categories.exists?(name: stale_category_id)
   end
 
+  test "imports a multi-tag rule action by resolving each tag id ref independently" do
+    ndjson = build_ndjson([
+      {
+        type: "Rule",
+        version: 1,
+        data: {
+          name: "Tag As Weekly And Recurring",
+          resource_type: "transaction",
+          active: true,
+          conditions: [
+            { condition_type: "transaction_name", operator: "like", value: "subscription" }
+          ],
+          actions: [
+            {
+              action_type: "set_transaction_tags",
+              value: "Weekly,Recurring",
+              value_ref: [
+                { "type" => "Tag", "id" => "source-tag-1", "name" => "Weekly" },
+                { "type" => "Tag", "id" => "source-tag-2", "name" => "Recurring" }
+              ]
+            }
+          ]
+        }
+      }
+    ])
+
+    importer = Family::DataImporter.new(@family, ndjson)
+    importer.import!
+
+    rule = @family.rules.find_by!(name: "Tag As Weekly And Recurring")
+    weekly_tag = @family.tags.find_by!(name: "Weekly")
+    recurring_tag = @family.tags.find_by!(name: "Recurring")
+
+    imported_tag_ids = rule.actions.first.value.split(",")
+    assert_equal [ weekly_tag.id, recurring_tag.id ].sort, imported_tag_ids.sort
+    # Regression guard: must not create one bogus tag literally named "Weekly,Recurring"
+    assert_not @family.tags.exists?(name: "Weekly,Recurring")
+    assert_equal 2, @family.tags.count
+  end
+
+  test "imports a multi-tag rule action with a comma-containing tag name from CSV-quoted value" do
+    ndjson = build_ndjson([
+      {
+        type: "Rule",
+        version: 1,
+        data: {
+          name: "Comma Tag Name Rule",
+          resource_type: "transaction",
+          active: true,
+          conditions: [
+            { condition_type: "transaction_name", operator: "like", value: "subscription" }
+          ],
+          actions: [
+            {
+              action_type: "set_transaction_tags",
+              value: "Weekly,\"Food, Dining\""
+            }
+          ]
+        }
+      }
+    ])
+
+    importer = Family::DataImporter.new(@family, ndjson)
+    importer.import!
+
+    rule = @family.rules.find_by!(name: "Comma Tag Name Rule")
+    weekly_tag = @family.tags.find_by!(name: "Weekly")
+    comma_tag = @family.tags.find_by!(name: "Food, Dining")
+
+    imported_tag_ids = rule.actions.first.value.split(",")
+    assert_equal [ weekly_tag.id, comma_tag.id ].sort, imported_tag_ids.sort
+    assert_equal 2, @family.tags.count
+  end
+
+  test "imports a multi-tag rule action from a legacy single-tag value_ref hash" do
+    ndjson = build_ndjson([
+      {
+        type: "Rule",
+        version: 1,
+        data: {
+          name: "Legacy Single Tag Action",
+          resource_type: "transaction",
+          active: true,
+          conditions: [
+            { condition_type: "transaction_name", operator: "like", value: "subscription" }
+          ],
+          actions: [
+            {
+              action_type: "set_transaction_tags",
+              value: "Weekly",
+              value_ref: { "type" => "Tag", "id" => "source-tag-1", "name" => "Weekly" }
+            }
+          ]
+        }
+      }
+    ])
+
+    importer = Family::DataImporter.new(@family, ndjson)
+    importer.import!
+
+    rule = @family.rules.find_by!(name: "Legacy Single Tag Action")
+    weekly_tag = @family.tags.find_by!(name: "Weekly")
+
+    assert_equal weekly_tag.id, rule.actions.first.value
+  end
+
   test "preserves explicit false rule operand values" do
     importer = Family::DataImporter.new(@family, "")
 

test/models/rule/action_test.rb

@@ -119,6 +119,66 @@ class Rule::ActionTest < ActiveSupport::TestCase
     assert_equal [ tag ], @txn2.tags
   end
 
+  test "set_transaction_tags applies multiple tags from one action" do
+    tag1 = @family.tags.create!(name: "Rule tag 1")
+    tag2 = @family.tags.create!(name: "Rule tag 2")
+
+    action = Rule::Action.new(
+      rule: @transaction_rule,
+      action_type: "set_transaction_tags",
+      value: [ tag1.id, tag2.id ]
+    )
+
+    action.apply(@rule_scope)
+
+    [ @txn1, @txn2, @txn3 ].each do |transaction|
+      transaction.reload
+      assert_includes transaction.tags, tag1
+      assert_includes transaction.tags, tag2
+      assert_equal 2, transaction.tags.count
+    end
+  end
+
+  test "set_transaction_tags value_display shows all selected tag names" do
+    tag1 = @family.tags.create!(name: "Rule tag 1")
+    tag2 = @family.tags.create!(name: "Rule tag 2")
+
+    action = Rule::Action.new(
+      rule: @transaction_rule,
+      action_type: "set_transaction_tags",
+      value: [ tag1.id, tag2.id ]
+    )
+
+    assert_equal "Rule tag 1, Rule tag 2", action.value_display
+  end
+
+  test "set_transaction_tags ignores unknown or malformed tag ids without raising" do
+    real_tag = @family.tags.create!(name: "Rule tag real")
+
+    action = Rule::Action.new(
+      rule: @transaction_rule,
+      action_type: "set_transaction_tags",
+      value: [ real_tag.id, "not-a-real-id", "", nil ]
+    )
+
+    action.apply(@rule_scope)
+
+    [ @txn1, @txn2, @txn3 ].each do |transaction|
+      transaction.reload
+      assert_equal [ real_tag ], transaction.tags
+    end
+  end
+
+  test "set_transaction_tags with only unknown tag ids is a no-op" do
+    action = Rule::Action.new(
+      rule: @transaction_rule,
+      action_type: "set_transaction_tags",
+      value: [ "not-a-real-id" ]
+    )
+
+    assert_equal 0, action.apply(@rule_scope)
+  end
+
   test "set_transaction_merchant" do
     merchant = @family.merchants.create!(name: "Rule test merchant")
 

test/models/rule_import_test.rb

@@ -157,6 +157,62 @@ class RuleImportTest < ActiveSupport::TestCase
     assert_equal existing_tag.id, action.value
   end
 
+  test "imports multi-tag actions, reusing existing tags and creating missing ones" do
+    existing_tag = @family.tags.create!(name: "Existing Tag")
+
+    csv = <<~CSV
+      name,resource_type,active,effective_date,conditions,actions
+      "Multi tag rule","transaction",true,,"[{\"condition_type\":\"transaction_name\",\"operator\":\"like\",\"value\":\"test\"}]","[{\"action_type\":\"set_transaction_tags\",\"value\":\"Existing Tag,New Tag\"}]"
+    CSV
+
+    import = @family.imports.create!(type: "RuleImport", raw_file_str: csv, col_sep: ",")
+    import.generate_rows_from_csv
+
+    assert_difference -> { Tag.where(family: @family).count }, 1 do
+      import.send(:import!)
+    end
+
+    new_tag = Tag.find_by!(family: @family, name: "New Tag")
+
+    rule = Rule.find_by!(family: @family, name: "Multi tag rule")
+    action = rule.actions.first
+    assert_equal "set_transaction_tags", action.action_type
+    assert_equal [ existing_tag.id, new_tag.id ], action.value.split(",")
+  end
+
+  test "imports a multi-tag action preserving a tag name that contains a comma" do
+    existing_tag = @family.tags.create!(name: "Existing Tag")
+
+    actions_json = [
+      {
+        action_type: "set_transaction_tags",
+        value: CSV.generate_line([ "Existing Tag", "Food, Dining" ], row_sep: "")
+      }
+    ].to_json
+    conditions_json = [
+      { condition_type: "transaction_name", operator: "like", value: "test" }
+    ].to_json
+
+    csv = CSV.generate do |csv_out|
+      csv_out << %w[name resource_type active effective_date conditions actions]
+      csv_out << [ "Comma tag rule", "transaction", true, nil, conditions_json, actions_json ]
+    end
+
+    import = @family.imports.create!(type: "RuleImport", raw_file_str: csv, col_sep: ",")
+    import.generate_rows_from_csv
+
+    assert_difference -> { Tag.where(family: @family).count }, 1 do
+      import.send(:import!)
+    end
+
+    comma_tag = Tag.find_by!(family: @family, name: "Food, Dining")
+
+    rule = Rule.find_by!(family: @family, name: "Comma tag rule")
+    action = rule.actions.first
+    assert_equal "set_transaction_tags", action.action_type
+    assert_equal [ existing_tag.id, comma_tag.id ], action.value.split(",")
+  end
+
   test "updates existing rule when re-importing with same name" do
     # First import
     import1 = @family.imports.create!(type: "RuleImport", raw_file_str: @csv, col_sep: ",")