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.
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user