Fix AND prefix on rule form (#2234)

* Fix AND prefix on rule form

This new condition prefix data target is used to ensure the AND prefix is added/removed to additional conditions/groups when there aren't any saved conditions yet.

* Ensure the condition group "Add condition" button is type button to avoid form submission

* Add prefix update when removing a subcondition

* Use data condition to update the prefix on conditions, condition groups, and subconditions

* Use condition remove instead of element remove for condition groups so prefix logic runs

* Add back explicit show_prefixes to ensure subconditions never have a prefix

* Run the prefix update runs on a load of a form, which handles prefixes on an edit since no conditions change

* Ensure saved items that are marked for removal don't impact the index

* Simplify logic since we don't process subconditions

* Clean up comments

* Add primary_condition_title field to rule model

app/javascript/controllers/rule/conditions_controller.js

@@ -21,12 +21,23 @@ export default class extends Controller {
   }
 
   remove(e) {
+    // Find the parent rules controller before removing the condition
+    const rulesEl = this.element.closest('[data-controller~="rules"]');
+
     if (e.params.destroy) {
       this.destroyFieldTarget.value = true;
       this.element.classList.add("hidden");
     } else {
       this.element.remove();
     }
+
+    // Update the prefixes of all conditions from the parent rules controller
+    if (rulesEl) {
+      const rulesController = this.application.getControllerForElementAndIdentifier(rulesEl, "rules");
+      if (rulesController && typeof rulesController.updateConditionPrefixes === "function") {
+        rulesController.updateConditionPrefixes();
+      }
+    }
   }
 
   handleConditionTypeChange(e) {

app/javascript/controllers/rules_controller.js

@@ -11,11 +11,17 @@ export default class extends Controller {
     "effectiveDateInput",
   ];
 
+  connect() {
+    // Update condition prefixes on first connection (form render on edit)
+    this.updateConditionPrefixes();
+  }
+
   addConditionGroup() {
     this.#appendTemplate(
       this.conditionGroupTemplateTarget,
       this.conditionsListTarget,
     );
+    this.updateConditionPrefixes();
   }
 
   addCondition() {
@@ -23,6 +29,7 @@ export default class extends Controller {
       this.conditionTemplateTarget,
       this.conditionsListTarget,
     );
+    this.updateConditionPrefixes();
   }
 
   addAction() {
@@ -45,4 +52,27 @@ export default class extends Controller {
   #uniqueKey() {
     return Date.now();
   }
+
+  // Updates the prefix visibility of all conditions and condition groups
+  // This is also called by the rule/conditions_controller when a subcondition is removed
+  updateConditionPrefixes() {
+    const conditions = Array.from(this.conditionsListTarget.children);
+    let conditionIndex = 0;
+
+    conditions.forEach((condition) => {
+      // Only process visible conditions, this prevents conditions that are marked for removal and hidden
+      // from being added to the index. This is important when editing a rule.
+      if (!condition.classList.contains('hidden')) {
+        const prefixEl = condition.querySelector('[data-condition-prefix]');
+        if (prefixEl) {
+          if (conditionIndex === 0) {
+            prefixEl.classList.add('hidden');
+          } else {
+            prefixEl.classList.remove('hidden');
+          }
+          conditionIndex++;
+        }
+      }
+    });
+  }
 }

app/models/rule.rb

@@ -46,6 +46,18 @@ class Rule < ApplicationRecord
     RuleJob.perform_later(self, ignore_attribute_locks: ignore_attribute_locks)
   end
 
+  def primary_condition_title
+    return "No conditions" if conditions.none?
+
+    first_condition = conditions.first
+    if first_condition.compound? && first_condition.sub_conditions.any?
+      first_sub_condition = first_condition.sub_conditions.first
+      "If #{first_sub_condition.filter.label.downcase} #{first_sub_condition.operator} #{first_sub_condition.value_display}"
+    else
+      "If #{first_condition.filter.label.downcase} #{first_condition.operator} #{first_condition.value_display}"
+    end
+  end
+
   private
     def matching_resources_scope
       scope = registry.resource_scope

app/views/rule/conditions/_condition.html.erb

@@ -4,8 +4,11 @@
 <% rule = condition.rule %>
 
 <li data-controller="rule--conditions" data-rule--conditions-condition-filters-value="<%= rule.condition_filters.to_json %>" class="flex items-center gap-3">
-  <% if form.index.to_i > 0 && show_prefix %>
+
-    <div class="pl-4">
+  <%# Conditionally render the prefix %>
+  <%# Condition groups pass in show_prefix: false for subconditions since the ANY/ALL selector makes that clear %>
+  <% if show_prefix %>
+    <div class="pl-2" data-condition-prefix>
       <span class="font-medium uppercase text-xs">and</span>
     </div>
   <% end %>

app/views/rule/conditions/_condition_group.html.erb

@@ -3,17 +3,16 @@
 <% condition = form.object %>
 <% rule = condition.rule %>
 
-<li data-controller="rule--conditions element-removal" class="border border-secondary rounded-md p-4 space-y-3">
+<li data-controller="rule--conditions" class="border border-secondary rounded-md p-4 space-y-3">
 
   <%= form.hidden_field :condition_type, value: "compound" %>
 
   <div class="flex items-center justify-between gap-2">
     <div class="flex items-center gap-2">
-      <% unless form.index == 0 %>
+      <%# Show prefix on condition groups, except the first one %>
-        <div class="pl-2">
+      <div class="pl-2" data-condition-prefix>
-          <span class="font-medium uppercase text-xs">and</span>
+        <span class="font-medium uppercase text-xs">and</span>
-        </div>
+      </div>
-      <% end %>
       <p class="text-sm text-secondary">match</p>
       <%= form.select :operator, [["all", "and"], ["any", "or"]], { container_class: "w-fit" }, data: { rules_target: "operatorField" } %>
       <p class="text-sm text-secondary">of the following conditions</p>
@@ -21,16 +20,16 @@
 
     <%= icon(
       "trash-2",
-      size: "sm",
       as_button: true,
-      data: { action: "element-removal#remove" }
+      size: "sm",
+      data: { action: "rule--conditions#remove" }
     ) %>
   </div>
 
   <%# Sub-condition template, used by Stimulus controller to add new sub-conditions dynamically %>
   <template data-rule--conditions-target="subConditionTemplate">
     <%= form.fields_for :sub_conditions, Rule::Condition.new(parent: condition, condition_type: rule.condition_filters.first.key), child_index: "IDX_PLACEHOLDER" do |scf| %>
-      <%= render "rule/conditions/condition", form: scf %>
+      <%= render "rule/conditions/condition", form: scf, show_prefix: false %>
     <% end %>
   </template>
 
@@ -44,6 +43,7 @@
     text: "Add condition",
     leading_icon: "plus",
     variant: "ghost",
+    type: "button",
     data: { action: "rule--conditions#addSubCondition" }
   ) %>
 </li>

app/views/rules/_form.html.erb

@@ -12,7 +12,7 @@
   <section class="space-y-4">
     <h3 class="text-sm font-medium text-primary">If <%= rule.resource_type %></h3>
 
-    <%# Condition template, used by Stimulus controller to add new conditions dynamically %>
+    <%# Condition Group template, used by Stimulus controller to add new conditions dynamically %>
     <template data-rules-target="conditionGroupTemplate">
       <%= f.fields_for :conditions, Rule::Condition.new(rule: rule, condition_type: "compound", operator: "and"), child_index: "IDX_PLACEHOLDER" do |cf| %>
         <%= render "rule/conditions/condition_group", form: cf %>

app/views/rules/_rule.html.erb

@@ -4,12 +4,8 @@
   <div class="text-sm space-y-1.5">
     <% if rule.conditions.any? %>
       <p class="flex items-center flex-wrap gap-1.5">
-        <span class="px-2 py-1 border border-alpha-black-200 rounded-full">
+        <span class="px-2 py-1 border border-secondary rounded-full">
-          <% if rule.conditions.first.compound? %>
+          <%= rule.primary_condition_title %>
-            If <%= rule.conditions.first.sub_conditions.first.filter.label %> <%= rule.conditions.first.sub_conditions.first.operator %> <%= rule.conditions.first.sub_conditions.first.value_display %>
-          <% else %>
-            If <%= rule.conditions.first.filter.label %> <%= rule.conditions.first.operator %> <%= rule.conditions.first.value_display %>
-          <% end %>
         </span>
 
         <% if rule.conditions.count > 1 %>