From fb01bbbae3a0d12d7483a68ca60d3e55ac537e0c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Juan=20Jos=C3=A9=20Mata?= <juanjo.mata@gmail.com>
Date: Mon, 7 Sep 2026 20:10:03 -0700
Subject: [PATCH] fix(rules): keep dynamically added nested-attribute keys
 numeric (#3444)

* fix(rules): keep dynamically added nested-attribute keys numeric

Rails strong params only treat integer-keyed hashes as nested attributes
(ActionController::Parameters.nested_attribute? matches /\A-?\d+\z/), so
rules_controller.js keys of the form "new_1"/"new_2" (introduced in
457698f, PR #3397) were silently dropped on submit: creating a rule from
the modal failed with "must have at least one action" even though an
action was present, and the re-rendered form lost the entered rows.

Date.now() alone can repeat within the same millisecond and a small
counter alone can collide with the numeric indexes Rails assigns to
persisted rows on edit forms, so combine them (Date.now() * 1000 + seq)
in both rules_controller.js and rule/conditions_controller.js.

Adds a system test that creates a rule through the modal with
dynamically added rows; no system test previously covered this path,
which is why the regression shipped.

* Fix rule creation regression

---------

Co-authored-by: Instinct Agent <agent@instinct.com>
---
 .../controllers/rule/conditions_controller.js |  3 ++-
 .../controllers/rules_controller.js           |  6 +----
 test/system/rules_test.rb                     | 22 +++++++++++++++++++
 3 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/app/javascript/controllers/rule/conditions_controller.js b/app/javascript/controllers/rule/conditions_controller.js
index ad23c862c..766c40e05 100644
--- a/app/javascript/controllers/rule/conditions_controller.js
+++ b/app/javascript/controllers/rule/conditions_controller.js
@@ -123,7 +123,8 @@ export default class extends Controller {
   }
 
   #uniqueKey() {
-    return Date.now();
+    this.keySequence = (this.keySequence ?? 0) + 1;
+    return Date.now() * 1000 + this.keySequence;
   }
 
   #toggleValueFieldVisibility() {
diff --git a/app/javascript/controllers/rules_controller.js b/app/javascript/controllers/rules_controller.js
index 5466a0a62..77c40d4b9 100644
--- a/app/javascript/controllers/rules_controller.js
+++ b/app/javascript/controllers/rules_controller.js
@@ -50,12 +50,8 @@ export default class extends Controller {
   }
 
   #uniqueKey() {
-    // 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}`;
+    return Date.now() * 1000 + this.keySequence;
   }
 
   // Updates the prefix visibility of all conditions and condition groups
diff --git a/test/system/rules_test.rb b/test/system/rules_test.rb
index 6de961284..2e54c814b 100644
--- a/test/system/rules_test.rb
+++ b/test/system/rules_test.rb
@@ -56,4 +56,26 @@ class RulesTest < ApplicationSystemTestCase
     assert_selector "h3", text: "Legacy bad rule"
     assert_text "Nicht unterstützt (name)"
   end
+
+  test "creates a transaction rule through the modal with dynamically added condition and action" do
+    visit new_rule_path(resource_type: "transaction")
+
+    within "dialog" do
+      click_on "Add condition"
+      find("[data-rules-target='conditionsList'] input[name$='[value]']").fill_in(with: "Coffee")
+      click_on "Add action"
+      click_on "Create Rule"
+    end
+
+    # A successful create lands on the confirmation dialog; before the
+    # nested-attribute keys were numeric, the submit failed validation with
+    # "must have at least one action" because strong params dropped the rows.
+    assert_text "Confirm changes"
+
+    rule = Rule.order(:created_at).last
+    assert_equal "transaction_name", rule.conditions.first.condition_type
+    assert_equal "Coffee", rule.conditions.first.value
+    assert_equal "set_transaction_category", rule.actions.first.action_type
+    assert rule.actions.first.value.present?
+  end
 end