From 21ea2ae08da17f4335634f50e012d52af8e7167d Mon Sep 17 00:00:00 2001 From: Tanmay Deep Sharma Date: Wed, 15 Jul 2026 15:30:46 +0530 Subject: [PATCH] revert(search): drop unnecessary reindex_for_search guard; trim automation_rule comments The guard was dead code: should_index? is only true when searchkick is loaded (both gated on advanced_search_allowed?), so #reindex is always defined when the callback fires. It only masked a flaky develop-owned message_spec and contradicts the fail-loudly-on-impossible-state guideline. Also trim verbose comments in automation_rule.rb. --- app/models/automation_rule.rb | 13 ++++--------- app/models/message.rb | 4 ---- 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/app/models/automation_rule.rb b/app/models/automation_rule.rb index f3400a7cc..74c5295c0 100644 --- a/app/models/automation_rule.rb +++ b/app/models/automation_rule.rb @@ -38,9 +38,7 @@ class AutomationRule < ApplicationRecord validate :execution_delay_supported_event after_update_commit :reauthorized!, if: -> { saved_change_to_conditions? } - # Rows already armed under the old definition must not fire on a config the rule no longer - # has (different delay, trigger, conditions, or actions). Delete rather than skip so the - # freed episode slot lets the new definition re-arm on the next matching event. + # Discard rows armed under the old definition; they re-arm on the next matching event. after_update :discard_stale_pending_executions, if: :execution_config_changed? scope :active, -> { where(active: true) } @@ -115,11 +113,8 @@ class AutomationRule < ApplicationRecord errors.add(:execution_delay, 'cannot be used with attribute_changed conditions.') end - # Conversation-level events (anything but message_created) key their episode on - # status_changed_at alone. A delayed condition on any other attribute (assignee, team, - # priority, ...) would collapse distinct qualifying periods into one episode and could - # fire on a stale window, so only status conditions are supported until episodes track - # per-attribute change times. + # Conversation-level episodes key on status_changed_at alone, so only status conditions + # can be delayed; other attributes would collapse distinct periods into one episode. def execution_delay_supported_event return if execution_delay.blank? || conditions.blank? || event_name == 'message_created' return if conditions.all? { |obj| obj['attribute_key'] == 'status' } @@ -133,7 +128,7 @@ class AutomationRule < ApplicationRecord end def discard_stale_pending_executions - # Includes stale processing rows: the sweep would otherwise reclaim and fire them. + # armed = pending + stale processing, which the sweep would otherwise reclaim. pending_executions.armed.delete_all end diff --git a/app/models/message.rb b/app/models/message.rb index 1930ddcf0..220bdd549 100644 --- a/app/models/message.rb +++ b/app/models/message.rb @@ -452,10 +452,6 @@ class Message < ApplicationRecord end def reindex_for_search - # searchkick (which defines #reindex) is only mixed in when advanced_search_allowed? is true - # at boot; guard so a should_index? true without a loaded index can't raise NoMethodError. - return unless respond_to?(:reindex) - reindex(mode: :async) end end