From e99e38a91c56e35f1b85fef5028192fa15b5bda5 Mon Sep 17 00:00:00 2001 From: Pluto <128720033+it-education-md@users.noreply.github.com> Date: Fri, 13 Feb 2026 13:53:24 -0500 Subject: [PATCH] fix: Handle empty compound conditions on rules index (#965) * fix: Handle empty compound conditions on rules index * fix: avoid contradictory rule condition summary on /rules * refactor: move rules condition display logic from view to model * fix: localize rule title fallback and preload conditions in rules index --- app/controllers/rules_controller.rb | 2 +- app/models/rule.rb | 23 ++++++++++----- app/views/rules/_rule.html.erb | 12 ++++---- config/locales/views/rules/ca.yml | 1 + config/locales/views/rules/de.yml | 2 +- config/locales/views/rules/en.yml | 1 + config/locales/views/rules/es.yml | 2 +- config/locales/views/rules/fr.yml | 1 + config/locales/views/rules/nb.yml | 2 +- config/locales/views/rules/nl.yml | 1 + config/locales/views/rules/ro.yml | 2 +- config/locales/views/rules/tr.yml | 2 +- config/locales/views/rules/zh-CN.yml | 1 + config/locales/views/rules/zh-TW.yml | 1 + test/controllers/rules_controller_test.rb | 30 ++++++++++++++++++++ test/models/rule_test.rb | 34 +++++++++++++++++++++++ 16 files changed, 99 insertions(+), 18 deletions(-) diff --git a/app/controllers/rules_controller.rb b/app/controllers/rules_controller.rb index 07c355a56..5e7496cb8 100644 --- a/app/controllers/rules_controller.rb +++ b/app/controllers/rules_controller.rb @@ -11,7 +11,7 @@ class RulesController < ApplicationController @sort_by = "name" unless allowed_columns.include?(@sort_by) @direction = "asc" unless [ "asc", "desc" ].include?(@direction) - @rules = Current.family.rules.order(@sort_by => @direction) + @rules = Current.family.rules.includes(conditions: :sub_conditions).order(@sort_by => @direction) # Fetch recent rule runs with pagination recent_runs_scope = RuleRun diff --git a/app/models/rule.rb b/app/models/rule.rb index cd18deadc..74dbaece8 100644 --- a/app/models/rule.rb +++ b/app/models/rule.rb @@ -86,14 +86,23 @@ class Rule < ApplicationRecord end def primary_condition_title - return "No conditions" if conditions.none? + condition = displayed_condition + return I18n.t("rules.no_condition") if condition.blank? - 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}" + "If #{condition.filter.label.downcase} #{condition.operator} #{condition.value_display}" + end + + def displayed_condition + displayable_conditions.first + end + + def additional_displayable_conditions_count + [ displayable_conditions.size - 1, 0 ].max + end + + def displayable_conditions + conditions.filter_map do |condition| + condition.compound? ? condition.sub_conditions.first : condition end end diff --git a/app/views/rules/_rule.html.erb b/app/views/rules/_rule.html.erb index c0d59bea2..2e882e4ba 100644 --- a/app/views/rules/_rule.html.erb +++ b/app/views/rules/_rule.html.erb @@ -6,20 +6,22 @@

<%= rule.name %>

<% end %> <% if rule.conditions.any? %> + <% displayed_condition = rule.displayed_condition %> + <% additional_condition_count = rule.additional_displayable_conditions_count %>
IF

- <% if rule.conditions.first.compound? %> - <%= rule.conditions.first.sub_conditions.first.filter.label %> <%= rule.conditions.first.sub_conditions.first.operator %> <%= rule.conditions.first.sub_conditions.first.value_display %> + <% if displayed_condition.present? %> + <%= displayed_condition.filter.label %> <%= displayed_condition.operator %> <%= displayed_condition.value_display %> <% else %> - <%= rule.conditions.first.filter.label %> <%= rule.conditions.first.operator %> <%= rule.conditions.first.value_display %> + <%= t("rules.no_condition") %> <% end %> - <% if rule.conditions.count > 1 %> - and <%= rule.conditions.count - 1 %> more <%= rule.conditions.count - 1 == 1 ? "condition" : "conditions" %> + <% if additional_condition_count.positive? %> + and <%= additional_condition_count %> more <%= additional_condition_count == 1 ? "condition" : "conditions" %> <% end %>

diff --git a/config/locales/views/rules/ca.yml b/config/locales/views/rules/ca.yml index c4b40ca90..b1cacb21f 100644 --- a/config/locales/views/rules/ca.yml +++ b/config/locales/views/rules/ca.yml @@ -19,6 +19,7 @@ ca: success: Totes les regles s'han posat a cua per a execució view_usage: Veure l'historial d'ús no_action: Sense acció + no_condition: Sense condició recent_runs: columns: date_time: Data/Hora diff --git a/config/locales/views/rules/de.yml b/config/locales/views/rules/de.yml index a8a80bdff..cc57171b0 100644 --- a/config/locales/views/rules/de.yml +++ b/config/locales/views/rules/de.yml @@ -2,6 +2,7 @@ de: rules: no_action: Keine Aktion + no_condition: Keine Bedingung recent_runs: title: Letzte Ausführungen description: Zeige die Ausführungsgeschichte deiner Regeln einschließlich Erfolgs-/Fehlerstatus und Transaktionsanzahlen. @@ -22,4 +23,3 @@ de: pending: Ausstehend success: Erfolgreich failed: Fehlgeschlagen - diff --git a/config/locales/views/rules/en.yml b/config/locales/views/rules/en.yml index e390e0ecc..81eccc7e8 100644 --- a/config/locales/views/rules/en.yml +++ b/config/locales/views/rules/en.yml @@ -2,6 +2,7 @@ en: rules: no_action: No Action + no_condition: No condition actions: value_placeholder: Enter a value apply_all: diff --git a/config/locales/views/rules/es.yml b/config/locales/views/rules/es.yml index 472ed0240..3c494e8fb 100644 --- a/config/locales/views/rules/es.yml +++ b/config/locales/views/rules/es.yml @@ -2,6 +2,7 @@ es: rules: no_action: Sin acción + no_condition: Sin condición recent_runs: title: Ejecuciones Recientes description: Ver el historial de ejecución de tus reglas incluyendo el estado de éxito/fallo y los conteos de transacciones. @@ -22,4 +23,3 @@ es: pending: Pendiente success: Éxito failed: Fallido - diff --git a/config/locales/views/rules/fr.yml b/config/locales/views/rules/fr.yml index daa5c42cc..dfa7b9755 100644 --- a/config/locales/views/rules/fr.yml +++ b/config/locales/views/rules/fr.yml @@ -2,6 +2,7 @@ fr: rules: no_action: Aucune action + no_condition: Aucune condition actions: value_placeholder: Entrez une valeur apply_all: diff --git a/config/locales/views/rules/nb.yml b/config/locales/views/rules/nb.yml index 1787cf18e..c4a333b40 100644 --- a/config/locales/views/rules/nb.yml +++ b/config/locales/views/rules/nb.yml @@ -2,6 +2,7 @@ nb: rules: no_action: Ingen handling + no_condition: Ingen betingelse recent_runs: title: Siste Kjøringer description: Se kjøringsloggen for reglene dine inkludert suksess/feil-status og transaksjonsantall. @@ -22,4 +23,3 @@ nb: pending: Ventende success: Vellykket failed: Mislyktes - diff --git a/config/locales/views/rules/nl.yml b/config/locales/views/rules/nl.yml index 9ef32b833..a789d20c9 100644 --- a/config/locales/views/rules/nl.yml +++ b/config/locales/views/rules/nl.yml @@ -2,6 +2,7 @@ nl: rules: no_action: Geen actie + no_condition: Geen voorwaarde actions: value_placeholder: Voer een waarde in apply_all: diff --git a/config/locales/views/rules/ro.yml b/config/locales/views/rules/ro.yml index 3f3ccb8c5..c5e877aab 100644 --- a/config/locales/views/rules/ro.yml +++ b/config/locales/views/rules/ro.yml @@ -2,6 +2,7 @@ ro: rules: no_action: Nicio acțiune + no_condition: Nicio condiție recent_runs: title: Rulări Recente description: Vezi istoricul de execuție al regulilor tale incluzând statusul de succes/eșec și numărul de tranzacții. @@ -22,4 +23,3 @@ ro: pending: În Așteptare success: Succes failed: Eșuat - diff --git a/config/locales/views/rules/tr.yml b/config/locales/views/rules/tr.yml index c2d50c11d..b1418415e 100644 --- a/config/locales/views/rules/tr.yml +++ b/config/locales/views/rules/tr.yml @@ -2,6 +2,7 @@ tr: rules: no_action: İşlem yok + no_condition: Koşul yok recent_runs: title: Son Çalıştırmalar description: Başarı/başarısızlık durumu ve işlem sayıları dahil olmak üzere kurallarınızın yürütme geçmişini görüntüleyin. @@ -22,4 +23,3 @@ tr: pending: Beklemede success: Başarılı failed: Başarısız - diff --git a/config/locales/views/rules/zh-CN.yml b/config/locales/views/rules/zh-CN.yml index 5044d8b00..6f44adc5f 100644 --- a/config/locales/views/rules/zh-CN.yml +++ b/config/locales/views/rules/zh-CN.yml @@ -2,6 +2,7 @@ zh-CN: rules: no_action: 无操作 + no_condition: 无条件 recent_runs: columns: date_time: 日期/时间 diff --git a/config/locales/views/rules/zh-TW.yml b/config/locales/views/rules/zh-TW.yml index 5a8bdf3bd..84846ce24 100644 --- a/config/locales/views/rules/zh-TW.yml +++ b/config/locales/views/rules/zh-TW.yml @@ -2,6 +2,7 @@ zh-TW: rules: no_action: 無動作 + no_condition: 無條件 recent_runs: title: 最近執行紀錄 description: 查看規則的執行歷史，包括成功/失敗狀態以及交易處理筆數。 diff --git a/test/controllers/rules_controller_test.rb b/test/controllers/rules_controller_test.rb index 681bac37b..baa30a1f3 100644 --- a/test/controllers/rules_controller_test.rb +++ b/test/controllers/rules_controller_test.rb @@ -180,6 +180,36 @@ class RulesControllerTest < ActionDispatch::IntegrationTest assert_redirected_to rules_url end + test "index renders when rule has empty compound condition" do + malformed_rule = @user.family.rules.build(resource_type: "transaction") + malformed_rule.conditions.build(condition_type: "compound", operator: "and") + malformed_rule.actions.build(action_type: "exclude_transaction") + malformed_rule.save! + + get rules_url + + assert_response :success + assert_includes response.body, I18n.t("rules.no_condition") + end + + test "index uses next valid condition when first compound condition is empty" do + rule = @user.family.rules.build(resource_type: "transaction") + rule.conditions.build(condition_type: "compound", operator: "and") + rule.conditions.build(condition_type: "transaction_name", operator: "like", value: "edge-case-name") + rule.actions.build(action_type: "exclude_transaction") + rule.save! + + get rules_url + + assert_response :success + + assert_select "##{ActionView::RecordIdentifier.dom_id(rule)}" do + assert_select "span", text: /edge-case-name/ + assert_select "span", text: /#{Regexp.escape(I18n.t("rules.no_condition"))}/, count: 0 + assert_select "p", text: /and 1 more condition/, count: 0 + end + end + test "should get confirm_all" do get confirm_all_rules_url assert_response :success diff --git a/test/models/rule_test.rb b/test/models/rule_test.rb index 408cb8cca..6f50221be 100644 --- a/test/models/rule_test.rb +++ b/test/models/rule_test.rb @@ -138,6 +138,40 @@ class RuleTest < ActiveSupport::TestCase assert_equal [ "Compound conditions cannot be nested" ], rule.errors.full_messages end + test "displayed_condition falls back to next valid condition when first compound condition is empty" do + rule = Rule.new( + family: @family, + resource_type: "transaction", + actions: [ Rule::Action.new(action_type: "exclude_transaction") ], + conditions: [ + Rule::Condition.new(condition_type: "compound", operator: "and"), + Rule::Condition.new(condition_type: "transaction_name", operator: "like", value: "starbucks") + ] + ) + + displayed_condition = rule.displayed_condition + + assert_not_nil displayed_condition + assert_equal "transaction_name", displayed_condition.condition_type + assert_equal "like", displayed_condition.operator + assert_equal "starbucks", displayed_condition.value + end + + test "additional_displayable_conditions_count ignores empty compound conditions" do + rule = Rule.new( + family: @family, + resource_type: "transaction", + actions: [ Rule::Action.new(action_type: "exclude_transaction") ], + conditions: [ + Rule::Condition.new(condition_type: "compound", operator: "and"), + Rule::Condition.new(condition_type: "transaction_name", operator: "like", value: "first"), + Rule::Condition.new(condition_type: "transaction_amount", operator: ">", value: 100) + ] + ) + + assert_equal 1, rule.additional_displayable_conditions_count + end + test "rule matching on transaction details" do # Create PayPal transaction with underlying merchant in details paypal_entry = create_transaction(