PERF: fixes N+1 for automation plugin (#32810)

- Add a safeguard until we implement pagination for the index, it
ensures we won't try to load more than 500 automations ever.
- Includes the fields of the automation so we don't have an N+1 and also
filter them in ruby land instead of doing two additional queries
- Ensures a field object in the field serializer is capable to find the
loaded automation associated to it instead of trying to reload it
- Includes pending_automations to avoid an N+1
This commit is contained in:
Joffrey JAFFEUX
2025-05-20 12:24:07 +02:00
committed by GitHub
parent f955d04d9c
commit 4c9384e50d
5 changed files with 38 additions and 8 deletions

View File

@ -5,7 +5,13 @@ module DiscourseAutomation
requires_plugin DiscourseAutomation::PLUGIN_NAME requires_plugin DiscourseAutomation::PLUGIN_NAME
def index def index
automations = DiscourseAutomation::Automation.order(:name).all automations =
DiscourseAutomation::Automation
.strict_loading
.includes(:fields, :pending_automations)
.order(:name)
.limit(500)
.all
serializer = serializer =
ActiveModel::ArraySerializer.new( ActiveModel::ArraySerializer.new(
automations, automations,
@ -19,7 +25,8 @@ module DiscourseAutomation
end end
def show def show
automation = DiscourseAutomation::Automation.find(params[:id]) automation =
DiscourseAutomation::Automation.includes(:fields, :pending_automations).find(params[:id])
render_serialized_automation(automation) render_serialized_automation(automation)
end end
@ -43,7 +50,8 @@ module DiscourseAutomation
def update def update
params.require(:automation) params.require(:automation)
automation = DiscourseAutomation::Automation.find(params[:id]) automation =
DiscourseAutomation::Automation.includes(:fields, :pending_automations).find(params[:id])
if automation.scriptable.forced_triggerable if automation.scriptable.forced_triggerable
params[:trigger] = automation.scriptable.forced_triggerable[:triggerable].to_s params[:trigger] = automation.scriptable.forced_triggerable[:triggerable].to_s
end end

View File

@ -7,7 +7,8 @@ module DiscourseAutomation
has_many :fields, has_many :fields,
class_name: "DiscourseAutomation::Field", class_name: "DiscourseAutomation::Field",
dependent: :delete_all, dependent: :delete_all,
foreign_key: "automation_id" foreign_key: "automation_id",
inverse_of: :automation
has_many :pending_automations, has_many :pending_automations,
class_name: "DiscourseAutomation::PendingAutomation", class_name: "DiscourseAutomation::PendingAutomation",
dependent: :delete_all, dependent: :delete_all,
@ -28,6 +29,16 @@ module DiscourseAutomation
attr_accessor :running_in_background attr_accessor :running_in_background
def trigger=(new_trigger)
@triggerable = nil
super
end
def script=(new_script)
@scriptable = nil
super
end
def running_in_background! def running_in_background!
@running_in_background = true @running_in_background = true
end end

View File

@ -4,7 +4,10 @@ module DiscourseAutomation
class Field < ActiveRecord::Base class Field < ActiveRecord::Base
self.table_name = "discourse_automation_fields" self.table_name = "discourse_automation_fields"
belongs_to :automation, class_name: "DiscourseAutomation::Automation" belongs_to :automation,
class_name: "DiscourseAutomation::Automation",
foreign_key: :automation_id,
inverse_of: :fields
around_save :on_update_callback around_save :on_update_callback

View File

@ -61,7 +61,7 @@ module DiscourseAutomation
not_found: scriptable.not_found, not_found: scriptable.not_found,
templates: templates:
process_templates(filter_fields_with_priority(scriptable.fields, object.trigger&.to_sym)), process_templates(filter_fields_with_priority(scriptable.fields, object.trigger&.to_sym)),
fields: process_fields(object.fields.where(target: "script")), fields: process_fields(script_fields),
} }
end end
@ -80,7 +80,7 @@ module DiscourseAutomation
doc: I18n.exists?(doc_key, :en) ? I18n.t(doc_key) : nil, doc: I18n.exists?(doc_key, :en) ? I18n.t(doc_key) : nil,
not_found: triggerable&.not_found, not_found: triggerable&.not_found,
templates: process_templates(triggerable&.fields || []), templates: process_templates(triggerable&.fields || []),
fields: process_fields(object.fields.where(target: "trigger")), fields: process_fields(trigger_fields),
settings: triggerable&.settings, settings: triggerable&.settings,
} }
end end
@ -141,6 +141,14 @@ module DiscourseAutomation
).as_json || [] ).as_json || []
end end
def script_fields
object.fields.select { |f| f.target == "script" }
end
def trigger_fields
object.fields.select { |f| f.target == "trigger" }
end
def scriptable def scriptable
object.scriptable object.scriptable
end end

View File

@ -5,7 +5,7 @@ en:
models: models:
fields: fields:
required_field: Field `%{name}` must be filled on `%{target}:%{target_name}`. required_field: Field `%{name}` must be filled on `%{target}:%{target_name}`.
invalid_field: Field’s component `%{component}` is not usable on `%{target}:%{target_name}.` invalid_field: Field’s component `%{component}` is not usable on `%{target}:%{target_name}`.
invalid_metadata: Data for `%{field}` is invalid or component `%{component}` is unknown. invalid_metadata: Data for `%{field}` is invalid or component `%{component}` is unknown.
triggerables: triggerables:
errors: errors: