From 98e4e001645acdba311d8d3a8b3d20784048779b Mon Sep 17 00:00:00 2001 From: merefield Date: Tue, 24 Feb 2026 11:35:38 +0000 Subject: [PATCH 01/14] PERF: fix workflow admin N+1 paths --- .../admin/workflow_step_options_controller.rb | 59 ++++++---- .../admin/workflow_steps_controller.rb | 73 ++++++------ .../admin/workflows_controller.rb | 24 ++-- app/models/discourse_workflow/workflow.rb | 104 +++++++++++------- .../workflow_category_serializer.rb | 7 ++ .../discourse_workflow/workflow_serializer.rb | 32 +++--- .../workflow_step_serializer.rb | 23 ++-- .../post_notification_handler.rb | 18 +-- spec/lib/post_notification_handler_spec.rb | 48 ++++++-- .../workflow_step_options_controller_spec.rb | 43 +++++--- .../admin/workflow_steps_controller_spec.rb | 64 +++++++++++ .../admin/workflows_controller_spec.rb | 72 +++++++++--- 12 files changed, 395 insertions(+), 172 deletions(-) create mode 100644 app/serializers/discourse_workflow/workflow_category_serializer.rb create mode 100644 spec/requests/admin/workflow_steps_controller_spec.rb diff --git a/app/controllers/discourse_workflow/admin/workflow_step_options_controller.rb b/app/controllers/discourse_workflow/admin/workflow_step_options_controller.rb index e67d6cd..784ef1b 100644 --- a/app/controllers/discourse_workflow/admin/workflow_step_options_controller.rb +++ b/app/controllers/discourse_workflow/admin/workflow_step_options_controller.rb @@ -5,20 +5,29 @@ module Admin class WorkflowStepOptionsController < ::Admin::AdminController requires_plugin ::DiscourseWorkflow::PLUGIN_NAME - before_action :set_workflow_step, only: [:index, :new, :create] - before_action :set_workflow_step_option, only: [:show, :edit, :update, :destroy] + before_action :set_workflow_step, only: %i[index new create] + before_action :set_workflow_step_option, only: %i[show edit update destroy] def index - if @workflow_step.present? - @workflow_step_options = WorkflowStepOption.where(workflow_step_id: @workflow_step.id).order(:position) - else - @workflow_step_options = WorkflowStepOption.all.order(:position) - end + @workflow_step_options = + if @workflow_step.present? + WorkflowStepOption.where(workflow_step_id: @workflow_step.id).order(:position).to_a + else + WorkflowStepOption.all.order(:position).to_a + end + ActiveRecord::Associations::Preloader.new( + records: @workflow_step_options, + associations: %i[workflow_option workflow_step], + ).call render_json_dump( - { workflow_step_options: - ActiveModel::ArraySerializer.new(@workflow_step_options, - each_serializer: DiscourseWorkflow::WorkflowStepOptionSerializer) - }) + { + workflow_step_options: + ActiveModel::ArraySerializer.new( + @workflow_step_options, + each_serializer: DiscourseWorkflow::WorkflowStepOptionSerializer, + ), + }, + ) end def show @@ -28,8 +37,9 @@ def new workflow_step_option = WorkflowStepOption.new(workflow_step_option_params) if workflow_step_option.save render json: { - workflow_step_option: WorkflowStepOptionSerializer.new(workflow_step_option, root: false), - }, + workflow_step_option: + WorkflowStepOptionSerializer.new(workflow_step_option, root: false), + }, status: :created else render_json_error workflow_step_option @@ -39,7 +49,10 @@ def new def create workflow_step_option = WorkflowStepOption.new(workflow_step_option_params) if !workflow_step_option.position.present? - if WorkflowStepOption.count == 0 || WorkflowStepOption.where(workflow_step_id: workflow_step_option.workflow_step_id).count == 0 + if WorkflowStepOption.count == 0 || + WorkflowStepOption.where( + workflow_step_id: workflow_step_option.workflow_step_id, + ).count == 0 workflow_step_option.position = 1 else workflow_step_option.position = @@ -51,8 +64,9 @@ def create end if workflow_step_option.save render json: { - workflow_step_option: WorkflowStepOptionSerializer.new(workflow_step_option, root: false), - }, + workflow_step_option: + WorkflowStepOptionSerializer.new(workflow_step_option, root: false), + }, status: :created else render_json_error workflow_step_option @@ -65,8 +79,9 @@ def edit def update if @workflow_step_option.update(workflow_step_option_params) render json: { - workflow_step_option: WorkflowStepOptionSerializer.new(@workflow_step_option, root: false), - }, + workflow_step_option: + WorkflowStepOptionSerializer.new(@workflow_step_option, root: false), + }, status: :ok else render_json_error @workflow_step_option @@ -97,7 +112,13 @@ def set_workflow_step_option end def workflow_step_option_params - params.require(:workflow_step_option).permit(:position, :workflow_step_id, :workflow_option_id, :target_step_id, :other_attributes...) + params.require(:workflow_step_option).permit( + :position, + :workflow_step_id, + :workflow_option_id, + :target_step_id, + :other_attributes..., + ) end def ensure_admin diff --git a/app/controllers/discourse_workflow/admin/workflow_steps_controller.rb b/app/controllers/discourse_workflow/admin/workflow_steps_controller.rb index 37d2c7b..472a942 100644 --- a/app/controllers/discourse_workflow/admin/workflow_steps_controller.rb +++ b/app/controllers/discourse_workflow/admin/workflow_steps_controller.rb @@ -5,20 +5,29 @@ module Admin class WorkflowStepsController < ::Admin::AdminController requires_plugin ::DiscourseWorkflow::PLUGIN_NAME - before_action :set_workflow, only: [:index, :new, :create] - before_action :set_workflow_step, only: [:show, :edit, :update, :destroy] + before_action :set_workflow, only: %i[index new create] + before_action :set_workflow_step, only: %i[show edit update destroy] def index - if @workflow.present? - @workflow_steps = WorkflowStep.where(workflow_id: @workflow.id).order(:position) - else - @workflow_steps = WorkflowStep.all.order(:position) - end + @workflow_steps = + if @workflow.present? + WorkflowStep.where(workflow_id: @workflow.id).order(:position).to_a + else + WorkflowStep.all.order(:position).to_a + end + ActiveRecord::Associations::Preloader.new( + records: @workflow_steps, + associations: [:category, { workflow_step_options: :workflow_option }], + ).call render_json_dump( - { workflow_steps: - ActiveModel::ArraySerializer.new(@workflow_steps, - each_serializer: DiscourseWorkflow::WorkflowStepSerializer) - }) + { + workflow_steps: + ActiveModel::ArraySerializer.new( + @workflow_steps, + each_serializer: DiscourseWorkflow::WorkflowStepSerializer, + ), + }, + ) end def show @@ -28,8 +37,8 @@ def new workflow_step = WorkflowStep.new(workflow_step_params) if workflow_step.save render json: { - workflow_step: WorkflowStepSerializer.new(workflow_step, root: false), - }, + workflow_step: WorkflowStepSerializer.new(workflow_step, root: false), + }, status: :created else render_json_error workflow_step @@ -39,16 +48,18 @@ def new def create workflow_step = WorkflowStep.new(workflow_step_params) if !workflow_step.position.present? - if WorkflowStep.count == 0 || WorkflowStep.where(workflow_id: workflow_step.workflow_id).count == 0 + if WorkflowStep.count == 0 || + WorkflowStep.where(workflow_id: workflow_step.workflow_id).count == 0 workflow_step.position = 1 else - workflow_step.position = WorkflowStep.where(workflow_id: workflow_step.workflow_id).maximum(:position).to_i + 1 + workflow_step.position = + WorkflowStep.where(workflow_id: workflow_step.workflow_id).maximum(:position).to_i + 1 end end if workflow_step.save render json: { - workflow_step: WorkflowStepSerializer.new(workflow_step, root: false), - }, + workflow_step: WorkflowStepSerializer.new(workflow_step, root: false), + }, status: :created # redirect_to edit_workflow_workflow_step_path(workflow_id: workflow_step.workflow_id, id: workflow_step.id) else @@ -62,8 +73,8 @@ def edit def update if @workflow_step.update(workflow_step_params) render json: { - workflow_step: WorkflowStepSerializer.new(@workflow_step, root: false), - }, + workflow_step: WorkflowStepSerializer.new(@workflow_step, root: false), + }, status: :ok else render_json_error @workflow_step @@ -94,19 +105,17 @@ def set_workflow_step end def workflow_step_params - params - .require(:workflow_step) - .permit( - :workflow_id, - :position, - :name, - :description, - :category_id, - :ai_enabled, - :ai_prompt, - :overdue_days, - :other_attributes..., - ) + params.require(:workflow_step).permit( + :workflow_id, + :position, + :name, + :description, + :category_id, + :ai_enabled, + :ai_prompt, + :overdue_days, + :other_attributes..., + ) end def ensure_admin diff --git a/app/controllers/discourse_workflow/admin/workflows_controller.rb b/app/controllers/discourse_workflow/admin/workflows_controller.rb index de95411..2a8ce8b 100644 --- a/app/controllers/discourse_workflow/admin/workflows_controller.rb +++ b/app/controllers/discourse_workflow/admin/workflows_controller.rb @@ -8,12 +8,22 @@ class WorkflowsController < ::Admin::AdminController before_action :find_workflow, only: %i[edit show update destroy] def index - @workflows = Workflow.order(:enabled).order(:name).order(:id) + @workflows = Workflow.order(:enabled).order(:name).order(:id).to_a + ActiveRecord::Associations::Preloader.new( + records: @workflows, + associations: { + workflow_steps: [:category, { workflow_step_options: :workflow_option }], + }, + ).call render_json_dump( - { workflows: - ActiveModel::ArraySerializer.new(@workflows, - each_serializer: DiscourseWorkflow::WorkflowSerializer) - }) + { + workflows: + ActiveModel::ArraySerializer.new( + @workflows, + each_serializer: DiscourseWorkflow::WorkflowSerializer, + ), + }, + ) end def new @@ -37,8 +47,8 @@ def create def update if @workflow.update(workflow_params) render json: WorkflowSerializer.new(@workflow, root: false) - else - render_json_error @workflow + else + render_json_error @workflow end end diff --git a/app/models/discourse_workflow/workflow.rb b/app/models/discourse_workflow/workflow.rb index ecaf473..0657fd6 100644 --- a/app/models/discourse_workflow/workflow.rb +++ b/app/models/discourse_workflow/workflow.rb @@ -2,7 +2,7 @@ module ::DiscourseWorkflow class Workflow < ActiveRecord::Base - self.table_name = 'workflows' + self.table_name = "workflows" before_validation :generate_unique_slug, if: :slug_generation_required? @@ -21,7 +21,7 @@ class Workflow < ActiveRecord::Base scope :ordered, -> { order("lower(name) ASC") } def kanban_compatible? - steps = workflow_steps.includes(:workflow_step_options).to_a + steps = workflow_steps.to_a return false if steps.blank? positions = steps.map { |step| step.position.to_i } @@ -35,21 +35,24 @@ def kanban_compatible? steps_by_id = steps.index_by(&:id) edges = Hash.new { |hash, key| hash[key] = [] } edge_lookup = {} + step_options_by_step_id = workflow_step_options_by_step_id(steps) steps.each do |step| - step.workflow_step_options.each do |step_option| - target_step_id = step_option.target_step_id - next if target_step_id.blank? - return false if !step_lookup[target_step_id] - - from_position = step.position.to_i - to_position = steps_by_id[target_step_id].position.to_i - edge_key = [from_position, to_position] - return false if edge_lookup[edge_key] - - edge_lookup[edge_key] = true - edges[step.id] << target_step_id - end + step_options_by_step_id + .fetch(step.id, []) + .each do |step_option| + target_step_id = step_option.target_step_id + next if target_step_id.blank? + return false if !step_lookup[target_step_id] + + from_position = step.position.to_i + to_position = steps_by_id[target_step_id].position.to_i + edge_key = [from_position, to_position] + return false if edge_lookup[edge_key] + + edge_lookup[edge_key] = true + edges[step.id] << target_step_id + end end start_step_id = start_steps.first.id @@ -69,41 +72,37 @@ def kanban_compatible? def validation_warnings warnings = [] + steps = workflow_steps.to_a + step_ids = steps.map(&:id) + step_lookup = step_ids.index_with(true) + step_options_by_step_id = workflow_step_options_by_step_id(steps) + step_options = step_options_by_step_id.values.flatten duplicate_step_positions = - workflow_steps - .group(:position) - .having("COUNT(*) > 1") - .pluck(:position) + steps + .map(&:position) .compact + .group_by(&:itself) + .select { |_position, grouped_steps| grouped_steps.length > 1 } + .keys if duplicate_step_positions.present? - warnings << { - code: "duplicate_step_positions", - positions: duplicate_step_positions - } + warnings << { code: "duplicate_step_positions", positions: duplicate_step_positions } end - workflow_step_ids = workflow_steps.pluck(:id) orphan_target_step_options = - WorkflowStepOption - .joins(:workflow_step) - .where(workflow_steps: { workflow_id: id }) - .where.not(target_step_id: workflow_step_ids) + step_options.select do |step_option| + step_option.target_step_id.present? && !step_lookup[step_option.target_step_id] + end - if orphan_target_step_options.exists? + if orphan_target_step_options.present? warnings << { code: "orphan_target_steps", - option_ids: orphan_target_step_options.pluck(:id) + option_ids: orphan_target_step_options.map(&:id), } end - option_slugs = - WorkflowStepOption - .joins(:workflow_step, :workflow_option) - .where(workflow_steps: { workflow_id: id }) - .pluck("workflow_options.slug") - .uniq + option_slugs = workflow_option_slugs(step_options) missing_option_labels = option_slugs.reject do |slug| @@ -111,15 +110,38 @@ def validation_warnings end if missing_option_labels.present? - warnings << { - code: "missing_option_labels", - slugs: missing_option_labels - } + warnings << { code: "missing_option_labels", slugs: missing_option_labels } end warnings end + private + + def workflow_step_options_by_step_id(steps) + step_ids = steps.map(&:id) + return {} if step_ids.blank? + + if steps.all? { |step| step.association(:workflow_step_options).loaded? } + steps.each_with_object({}) { |step, memo| memo[step.id] = step.workflow_step_options } + else + WorkflowStepOption.where(workflow_step_id: step_ids).group_by(&:workflow_step_id) + end + end + + def workflow_option_slugs(step_options) + return [] if step_options.blank? + + if step_options.all? { |step_option| step_option.association(:workflow_option).loaded? } + step_options.map { |step_option| step_option.workflow_option&.slug }.compact.uniq + else + option_ids = step_options.map(&:workflow_option_id).compact.uniq + return [] if option_ids.blank? + + WorkflowOption.where(id: option_ids).pluck(:slug).compact.uniq + end + end + def ensure_name_ascii return if name.blank? if !CGI.unescape(self.name).ascii_only? @@ -128,7 +150,7 @@ def ensure_name_ascii end def generate_unique_slug - base_slug = name.to_s.parameterize(separator: '_') + base_slug = name.to_s.parameterize(separator: "_") slug_candidate = base_slug counter = 2 diff --git a/app/serializers/discourse_workflow/workflow_category_serializer.rb b/app/serializers/discourse_workflow/workflow_category_serializer.rb new file mode 100644 index 0000000..5469deb --- /dev/null +++ b/app/serializers/discourse_workflow/workflow_category_serializer.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +module DiscourseWorkflow + class WorkflowCategorySerializer < ApplicationSerializer + attributes :id, :name, :slug, :color, :text_color, :parent_category_id + end +end diff --git a/app/serializers/discourse_workflow/workflow_serializer.rb b/app/serializers/discourse_workflow/workflow_serializer.rb index 08c05ca..91e1611 100644 --- a/app/serializers/discourse_workflow/workflow_serializer.rb +++ b/app/serializers/discourse_workflow/workflow_serializer.rb @@ -5,16 +5,16 @@ class WorkflowSerializer < ApplicationSerializer root "workflow" attributes :id, - :name, - :description, - :enabled, - :overdue_days, - :show_kanban_tags, - :kanban_compatible, - :workflow_steps_count, - :starting_category_id, - :final_category_id, - :validation_warnings + :name, + :description, + :enabled, + :overdue_days, + :show_kanban_tags, + :kanban_compatible, + :workflow_steps_count, + :starting_category_id, + :final_category_id, + :validation_warnings has_many :workflow_steps, serializer: WorkflowStepSerializer, @@ -22,15 +22,15 @@ class WorkflowSerializer < ApplicationSerializer key: :workflow_steps def workflow_steps_count - object.workflow_steps.count + ordered_workflow_steps.length end def starting_category_id - object.workflow_steps.order(:position).first&.category_id + ordered_workflow_steps.first&.category_id end def final_category_id - object.workflow_steps.order(:position).last&.category_id + ordered_workflow_steps.last&.category_id end def validation_warnings @@ -40,5 +40,11 @@ def validation_warnings def kanban_compatible object.kanban_compatible? end + + private + + def ordered_workflow_steps + @ordered_workflow_steps ||= object.workflow_steps.to_a.sort_by { |step| step.position.to_i } + end end end diff --git a/app/serializers/discourse_workflow/workflow_step_serializer.rb b/app/serializers/discourse_workflow/workflow_step_serializer.rb index 42aed0b..4403151 100644 --- a/app/serializers/discourse_workflow/workflow_step_serializer.rb +++ b/app/serializers/discourse_workflow/workflow_step_serializer.rb @@ -2,26 +2,25 @@ module DiscourseWorkflow class WorkflowStepSerializer < ApplicationSerializer - attributes :id, - :workflow_id, - :category_id, - :position, - :slug, - :name, - :description, - :overdue_days, - :ai_enabled, - :ai_prompt + :workflow_id, + :category_id, + :position, + :slug, + :name, + :description, + :overdue_days, + :ai_enabled, + :ai_prompt has_many :workflow_step_options, serializer: WorkflowStepOptionSerializer, embed: :object, key: :workflow_step_options - has_one :category, serializer: CategorySerializer, embed: :object + has_one :category, serializer: WorkflowCategorySerializer, embed: :object def category - Category.find_by(id: object.category_id) if object.category_id.present? + object.category end end end diff --git a/lib/discourse_workflow/post_notification_handler.rb b/lib/discourse_workflow/post_notification_handler.rb index b38bfee..b44899e 100644 --- a/lib/discourse_workflow/post_notification_handler.rb +++ b/lib/discourse_workflow/post_notification_handler.rb @@ -22,8 +22,7 @@ def handle return false if !post.topic.is_workflow_topic? return false if !post.is_first_post? - workflow_state = - DiscourseWorkflow::WorkflowState.find_by(topic_id: post.topic.id) + workflow_state = DiscourseWorkflow::WorkflowState.find_by(topic_id: post.topic.id) return false if workflow_state.blank? data = { @@ -32,22 +31,17 @@ def handle username: post.user.username, workflow_name: workflow_state.workflow.name, workflow_step_name: workflow_state.workflow_step.name, - topic_title: post.topic.title + topic_title: post.topic.title, } ::CategoryUser - .where( - notification_level: WATCHING_FIRST_POST, - category_id: post.topic.category_id - ) + .where(notification_level: WATCHING_FIRST_POST, category_id: post.topic.category_id) .each do |category_user| - # PostAlerter.create_notification handles many edge cases, such as - # muting, ignoring, double notifications etc. - user = category_user.user - user.notifications.create!( + ::Notification.create!( + user_id: category_user.user_id, notification_type: ::Notification.types[:workflow_topic_arrival], high_priority: true, - data: data.to_json + data: data.to_json, ) end end diff --git a/spec/lib/post_notification_handler_spec.rb b/spec/lib/post_notification_handler_spec.rb index 6eca177..9e01031 100644 --- a/spec/lib/post_notification_handler_spec.rb +++ b/spec/lib/post_notification_handler_spec.rb @@ -49,9 +49,7 @@ .where(notification_type: Notification.types[:workflow_topic_arrival]) .count - expect do - described_class.new(post, []).handle - end.to change { + expect do described_class.new(post, []).handle end.to change { watching_same_category_user .notifications .where(notification_type: Notification.types[:workflow_topic_arrival]) @@ -70,12 +68,44 @@ Fabricate(:post, topic: topic, user: topic_owner) post = Fabricate(:post, topic: topic, user: topic_owner, post_number: 2) - expect do - described_class.new(post, []).handle - end.not_to change { - Notification.where( - notification_type: Notification.types[:workflow_topic_arrival], - ).count + expect do described_class.new(post, []).handle end.not_to change { + Notification.where(notification_type: Notification.types[:workflow_topic_arrival]).count } end + + it "loads watcher users in bulk as watcher counts grow" do + allow(Notification).to receive(:create!) + + base_user_query_count = + track_sql_queries { described_class.new(first_post_with_state, []).handle }.count do |query| + query.start_with?("SELECT") && query.include?('FROM "users"') + end + + 3.times do + user = Fabricate(:user) + CategoryUser.create!( + user_id: user.id, + category_id: workflow_category.id, + notification_level: DiscourseWorkflow::WATCHING_FIRST_POST, + ) + end + + expanded_user_query_count = + track_sql_queries { described_class.new(first_post_with_state, []).handle }.count do |query| + query.start_with?("SELECT") && query.include?('FROM "users"') + end + + expect(expanded_user_query_count).to be <= base_user_query_count + 1 + end + + def first_post_with_state + next_topic = Fabricate(:topic, user: topic_owner, category: workflow_category) + Fabricate( + :workflow_state, + topic_id: next_topic.id, + workflow_id: workflow.id, + workflow_step_id: step_1.id, + ) + Fabricate(:post, topic: next_topic, user: topic_owner) + end end diff --git a/spec/requests/admin/workflow_step_options_controller_spec.rb b/spec/requests/admin/workflow_step_options_controller_spec.rb index 3393ee5..1f519a5 100644 --- a/spec/requests/admin/workflow_step_options_controller_spec.rb +++ b/spec/requests/admin/workflow_step_options_controller_spec.rb @@ -8,20 +8,10 @@ fab!(:category_1, :category) fab!(:category_2, :category) fab!(:step_1) do - Fabricate( - :workflow_step, - workflow_id: workflow.id, - category_id: category_1.id, - position: 1, - ) + Fabricate(:workflow_step, workflow_id: workflow.id, category_id: category_1.id, position: 1) end fab!(:step_2) do - Fabricate( - :workflow_step, - workflow_id: workflow.id, - category_id: category_2.id, - position: 2, - ) + Fabricate(:workflow_step, workflow_id: workflow.id, category_id: category_2.id, position: 2) end fab!(:existing_option) { Fabricate(:workflow_option, slug: "start") } fab!(:new_option) { Fabricate(:workflow_option, slug: "next") } @@ -50,8 +40,33 @@ end.to change { DiscourseWorkflow::WorkflowStepOption.count }.by(1) expect(response.status).to eq(201) - expect(DiscourseWorkflow::WorkflowStepOption.order(:id).last.position).to eq( - 2, + expect(DiscourseWorkflow::WorkflowStepOption.order(:id).last.position).to eq(2) + end + + it "does not add per-option queries when listing step options" do + get "/admin/plugins/discourse-workflow/workflows/#{workflow.id}/workflow_steps/#{step_1.id}/workflow_step_options.json" + base_query_count = + track_sql_queries do + get "/admin/plugins/discourse-workflow/workflows/#{workflow.id}/workflow_steps/#{step_1.id}/workflow_step_options.json" + expect(response.status).to eq(200) + end.count + + extra_option = Fabricate(:workflow_option, slug: "branch") + Fabricate( + :workflow_step_option, + workflow_step_id: step_1.id, + workflow_option_id: extra_option.id, + target_step_id: step_2.id, + position: 2, ) + + get "/admin/plugins/discourse-workflow/workflows/#{workflow.id}/workflow_steps/#{step_1.id}/workflow_step_options.json" + expanded_query_count = + track_sql_queries do + get "/admin/plugins/discourse-workflow/workflows/#{workflow.id}/workflow_steps/#{step_1.id}/workflow_step_options.json" + expect(response.status).to eq(200) + end.count + + expect(expanded_query_count).to eq(base_query_count) end end diff --git a/spec/requests/admin/workflow_steps_controller_spec.rb b/spec/requests/admin/workflow_steps_controller_spec.rb new file mode 100644 index 0000000..53c9b6b --- /dev/null +++ b/spec/requests/admin/workflow_steps_controller_spec.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +require_relative "../../plugin_helper" + +describe DiscourseWorkflow::Admin::WorkflowStepsController do + fab!(:admin) + fab!(:workflow) { Fabricate(:workflow, name: "Workflow Steps Controller Workflow") } + fab!(:category_1, :category) + fab!(:category_2, :category) + fab!(:option) { Fabricate(:workflow_option, slug: "next-step") } + fab!(:step_1) do + Fabricate(:workflow_step, workflow_id: workflow.id, category_id: category_1.id, position: 1) + end + fab!(:step_2) do + Fabricate(:workflow_step, workflow_id: workflow.id, category_id: category_2.id, position: 2) + end + fab!(:step_option_1) do + Fabricate( + :workflow_step_option, + workflow_step_id: step_1.id, + workflow_option_id: option.id, + target_step_id: step_2.id, + position: 1, + ) + end + + before { sign_in(admin) } + + it "does not add per-step queries when listing workflow steps" do + get "/admin/plugins/discourse-workflow/workflows/#{workflow.id}/workflow_steps.json" + base_query_count = + track_sql_queries do + get "/admin/plugins/discourse-workflow/workflows/#{workflow.id}/workflow_steps.json" + expect(response.status).to eq(200) + end.count + + 5.times do |index| + extra_category = Fabricate(:category) + extra_step = + Fabricate( + :workflow_step, + workflow_id: workflow.id, + category_id: extra_category.id, + position: index + 3, + ) + Fabricate( + :workflow_step_option, + workflow_step_id: extra_step.id, + workflow_option_id: option.id, + target_step_id: step_1.id, + position: 1, + ) + end + + get "/admin/plugins/discourse-workflow/workflows/#{workflow.id}/workflow_steps.json" + expanded_query_count = + track_sql_queries do + get "/admin/plugins/discourse-workflow/workflows/#{workflow.id}/workflow_steps.json" + expect(response.status).to eq(200) + end.count + + expect(expanded_query_count).to be <= base_query_count + 2 + end +end diff --git a/spec/requests/admin/workflows_controller_spec.rb b/spec/requests/admin/workflows_controller_spec.rb index d7fedae..486ebd3 100644 --- a/spec/requests/admin/workflows_controller_spec.rb +++ b/spec/requests/admin/workflows_controller_spec.rb @@ -8,20 +8,10 @@ fab!(:category_1, :category) fab!(:category_2, :category) fab!(:step_1) do - Fabricate( - :workflow_step, - workflow_id: workflow.id, - category_id: category_1.id, - position: 1, - ) + Fabricate(:workflow_step, workflow_id: workflow.id, category_id: category_1.id, position: 1) end fab!(:step_2) do - Fabricate( - :workflow_step, - workflow_id: workflow.id, - category_id: category_2.id, - position: 2, - ) + Fabricate(:workflow_step, workflow_id: workflow.id, category_id: category_2.id, position: 2) end before { sign_in(admin) } @@ -53,9 +43,65 @@ it "updates show_kanban_tags on a workflow" do put "/admin/plugins/discourse-workflow/workflows/#{workflow.id}.json", - params: { workflow: { show_kanban_tags: false } } + params: { + workflow: { + show_kanban_tags: false, + }, + } expect(response.status).to eq(200) expect(workflow.reload.show_kanban_tags).to eq(false) end + + it "does not add per-workflow queries when serializing the index" do + option = Fabricate(:workflow_option, slug: "next-query", name: "Next Query") + Fabricate( + :workflow_step_option, + workflow_step_id: step_1.id, + workflow_option_id: option.id, + target_step_id: step_2.id, + ) + + get "/admin/plugins/discourse-workflow/workflows.json" + base_query_count = + track_sql_queries do + get "/admin/plugins/discourse-workflow/workflows.json" + expect(response.status).to eq(200) + end.count + + 5.times do |index| + extra_workflow = Fabricate(:workflow, name: "Controller Workflow #{index + 2}") + category_3 = Fabricate(:category) + category_4 = Fabricate(:category) + extra_step_1 = + Fabricate( + :workflow_step, + workflow_id: extra_workflow.id, + category_id: category_3.id, + position: 1, + ) + extra_step_2 = + Fabricate( + :workflow_step, + workflow_id: extra_workflow.id, + category_id: category_4.id, + position: 2, + ) + Fabricate( + :workflow_step_option, + workflow_step_id: extra_step_1.id, + workflow_option_id: option.id, + target_step_id: extra_step_2.id, + ) + end + + get "/admin/plugins/discourse-workflow/workflows.json" + expanded_query_count = + track_sql_queries do + get "/admin/plugins/discourse-workflow/workflows.json" + expect(response.status).to eq(200) + end.count + + expect(expanded_query_count).to be <= base_query_count + 2 + end end From 8fb215d84032b8394a926c87011e2bceb9a2f437 Mon Sep 17 00:00:00 2001 From: merefield Date: Tue, 24 Feb 2026 11:57:26 +0000 Subject: [PATCH 02/14] PERF: optimize daily stats persistence --- .../discourse_workflow/workflow_stat.rb | 10 ++++-- ...ue_daily_bucket_index_to_workflow_stats.rb | 23 ++++++++++++ lib/discourse_workflow/stats.rb | 36 +++++++++++-------- 3 files changed, 53 insertions(+), 16 deletions(-) create mode 100644 db/migrate/20260223130000_add_unique_daily_bucket_index_to_workflow_stats.rb diff --git a/app/models/discourse_workflow/workflow_stat.rb b/app/models/discourse_workflow/workflow_stat.rb index c3c1b3c..cc9e477 100644 --- a/app/models/discourse_workflow/workflow_stat.rb +++ b/app/models/discourse_workflow/workflow_stat.rb @@ -2,13 +2,18 @@ module ::DiscourseWorkflow class WorkflowStat < ActiveRecord::Base - self.table_name = 'workflow_stats' + self.table_name = "workflow_stats" belongs_to :workflow belongs_to :workflow_step validates :cob_date, presence: true validates :workflow_id, presence: true validates :workflow_step_id, presence: true - validates :count, presence: true, numericality: { only_integer: true, greater_than_or_equal_to: 0 } + validates :count, + presence: true, + numericality: { + only_integer: true, + greater_than_or_equal_to: 0, + } end end @@ -26,6 +31,7 @@ class WorkflowStat < ActiveRecord::Base # # Indexes # +# idx_workflow_stats_daily_workflow_step_unique (cob_date,workflow_id,workflow_step_id) UNIQUE # index_workflow_stats_on_workflow_id (workflow_id) # index_workflow_stats_on_workflow_step_id (workflow_step_id) # diff --git a/db/migrate/20260223130000_add_unique_daily_bucket_index_to_workflow_stats.rb b/db/migrate/20260223130000_add_unique_daily_bucket_index_to_workflow_stats.rb new file mode 100644 index 0000000..47ce65a --- /dev/null +++ b/db/migrate/20260223130000_add_unique_daily_bucket_index_to_workflow_stats.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class AddUniqueDailyBucketIndexToWorkflowStats < ActiveRecord::Migration[7.2] + disable_ddl_transaction! + + INDEX_NAME = "idx_workflow_stats_daily_workflow_step_unique" + + def up + remove_index(:workflow_stats, name: INDEX_NAME, if_exists: true, algorithm: :concurrently) + + add_index( + :workflow_stats, + %i[cob_date workflow_id workflow_step_id], + unique: true, + name: INDEX_NAME, + algorithm: :concurrently, + ) + end + + def down + remove_index(:workflow_stats, name: INDEX_NAME, if_exists: true, algorithm: :concurrently) + end +end diff --git a/lib/discourse_workflow/stats.rb b/lib/discourse_workflow/stats.rb index 04b9274..6f156ea 100644 --- a/lib/discourse_workflow/stats.rb +++ b/lib/discourse_workflow/stats.rb @@ -1,27 +1,35 @@ # frozen_string_literal: true module DiscourseWorkflow class Stats - def calculate_daily_stats return unless SiteSetting.workflow_enabled current_date = Date.current - - ::DiscourseWorkflow::WorkflowStat.where(cob_date: current_date).destroy_all - - Workflow.all.each do |workflow| - workflow.workflow_states.each do |state| - stat = ::DiscourseWorkflow::WorkflowStat.find_or_initialize_by( + now = Time.zone.now + counts_by_workflow_step = + ::DiscourseWorkflow::WorkflowState.group(:workflow_id, :workflow_step_id).count + records = + counts_by_workflow_step.map do |(workflow_id, workflow_step_id), count| + { cob_date: current_date, - workflow_id: workflow.id, - workflow_step_id: state.workflow_step_id - ) - stat.count ||= 0 - stat.count += 1 - stat.save! + workflow_id: workflow_id, + workflow_step_id: workflow_step_id, + count: count, + created_at: now, + updated_at: now, + } end - ::Rails.logger.info("Workflow Daily Stats recorded for: #{workflow.name}") + + # This rebuild strategy assumes a single scheduler execution. + # In standard operation this job should not run concurrently. + ::DiscourseWorkflow::WorkflowStat.transaction do + ::DiscourseWorkflow::WorkflowStat.where(cob_date: current_date.all_day).delete_all + ::DiscourseWorkflow::WorkflowStat.insert_all!(records) if records.present? end + + ::Rails.logger.info( + "Workflow Daily Stats recorded for #{current_date}: #{records.length} step buckets", + ) end end end From b2928f75ef0d2ac422212f2bf6520a21cd8a228e Mon Sep 17 00:00:00 2001 From: merefield Date: Tue, 24 Feb 2026 12:49:19 +0000 Subject: [PATCH 03/14] PERF: keep workflow quick filters in SQL --- .../list_controller_extension.rb | 76 +++++++------------ .../topic_query_extension.rb | 6 +- spec/requests/workflow_list_filters_spec.rb | 20 +++++ 3 files changed, 53 insertions(+), 49 deletions(-) diff --git a/lib/discourse_workflow/list_controller_extension.rb b/lib/discourse_workflow/list_controller_extension.rb index b484f09..a2b8de4 100644 --- a/lib/discourse_workflow/list_controller_extension.rb +++ b/lib/discourse_workflow/list_controller_extension.rb @@ -12,27 +12,24 @@ module ListControllerExtension def workflow list_opts = build_topic_list_options user = list_target_user || current_user - filtered_topic_ids = nil + workflow_topic_ids_scope = DiscourseWorkflow::WorkflowState.all.select(:topic_id).distinct + workflow_filters_applied = false if user.present? && params[:my_categories] == "1" - allowed_category_ids = - Category.topic_create_allowed(Guardian.new(user)).pluck(:id) - - my_category_topic_ids = - DiscourseWorkflow::WorkflowState - .joins(:topic) - .where(topics: { category_id: allowed_category_ids }) - .pluck(:topic_id) - .uniq - - filtered_topic_ids = - filtered_topic_ids ? (filtered_topic_ids & my_category_topic_ids) : my_category_topic_ids + allowed_category_ids = Category.topic_create_allowed(Guardian.new(user)).select(:id) + workflow_topic_ids_scope = + workflow_topic_ids_scope.joins(:topic).where( + topics: { + category_id: allowed_category_ids, + }, + ) + workflow_filters_applied = true end if params[:overdue] == "1" default_overdue_days = SiteSetting.workflow_overdue_days_default.to_i - overdue_topic_ids = - DiscourseWorkflow::WorkflowState + workflow_topic_ids_scope = + workflow_topic_ids_scope .joins(:workflow_step, :workflow) .where( "COALESCE(workflow_steps.overdue_days, workflows.overdue_days, ?) > 0", @@ -42,43 +39,30 @@ def workflow "workflow_states.updated_at <= NOW() - (COALESCE(workflow_steps.overdue_days, workflows.overdue_days, ?) * INTERVAL '1 day')", default_overdue_days, ) - .pluck(:topic_id) - .uniq - - filtered_topic_ids = - filtered_topic_ids ? (filtered_topic_ids & overdue_topic_ids) : overdue_topic_ids + workflow_filters_applied = true elsif params[:overdue_days].present? && params[:overdue_days].to_i > 0 cutoff = params[:overdue_days].to_i.days.ago - overdue_topic_ids = - DiscourseWorkflow::WorkflowState - .where("updated_at <= ?", cutoff) - .pluck(:topic_id) - .uniq - - filtered_topic_ids = - filtered_topic_ids ? (filtered_topic_ids & overdue_topic_ids) : overdue_topic_ids + workflow_topic_ids_scope = + workflow_topic_ids_scope.where("workflow_states.updated_at <= ?", cutoff) + workflow_filters_applied = true end - if params[:workflow_step_position].present? && - params[:workflow_step_position].to_i > 0 - position_topic_ids = - DiscourseWorkflow::WorkflowState - .joins(:workflow_step) - .where(workflow_steps: { position: params[:workflow_step_position].to_i }) - .pluck(:topic_id) - .uniq - - filtered_topic_ids = - filtered_topic_ids ? (filtered_topic_ids & position_topic_ids) : position_topic_ids + if params[:workflow_step_position].present? && params[:workflow_step_position].to_i > 0 + workflow_topic_ids_scope = + workflow_topic_ids_scope.joins(:workflow_step).where( + workflow_steps: { + position: params[:workflow_step_position].to_i, + }, + ) + workflow_filters_applied = true end - if filtered_topic_ids - list_opts[:topic_ids] = filtered_topic_ids.presence || [-1] - end + list_opts[:topic_ids] = workflow_topic_ids_scope if workflow_filters_applied list = TopicQuery.new(user, list_opts).public_send("list_workflow") - list.more_topics_url = url_for(construct_url_with(:next, list_opts)) - list.prev_topics_url = url_for(construct_url_with(:prev, list_opts)) + list_query_opts = list_opts.except(:topic_ids) + list.more_topics_url = url_for(construct_url_with(:next, list_query_opts)) + list.prev_topics_url = url_for(construct_url_with(:prev, list_query_opts)) respond_with_list(list) end @@ -97,9 +81,7 @@ def workflow_charts protected def ensure_discourse_workflow - if !SiteSetting.workflow_enabled - raise Discourse::NotFound - end + raise Discourse::NotFound if !SiteSetting.workflow_enabled end end end diff --git a/lib/discourse_workflow/topic_query_extension.rb b/lib/discourse_workflow/topic_query_extension.rb index 10c35fe..635c295 100644 --- a/lib/discourse_workflow/topic_query_extension.rb +++ b/lib/discourse_workflow/topic_query_extension.rb @@ -4,10 +4,12 @@ module DiscourseWorkflow module TopicQueryExtension def list_workflow create_list(:workflow) do |topics| - topics.joins("INNER JOIN workflow_states + topics.joins( + "INNER JOIN workflow_states ON workflow_states.topic_id = topics.id INNER JOIN workflows - ON workflows.id = workflow_states.workflow_id") + ON workflows.id = workflow_states.workflow_id", + ) end end end diff --git a/spec/requests/workflow_list_filters_spec.rb b/spec/requests/workflow_list_filters_spec.rb index 081633f..1b24b3b 100644 --- a/spec/requests/workflow_list_filters_spec.rb +++ b/spec/requests/workflow_list_filters_spec.rb @@ -136,4 +136,24 @@ expect(topic_list["workflow_kanban_steps"]).to eq([]) expect(topic_list["workflow_kanban_transitions"]).to eq([]) end + + it "does not materialize workflow topic ids when combining quick filters" do + state_a.update_columns(updated_at: 5.days.ago) + + workflow_state_topic_id_plucks = + track_sql_queries do + get "/workflow.json", + params: { + my_categories: "1", + overdue: "1", + workflow_step_position: "1", + } + end.select do |query| + query.match?(/SELECT\s+"workflow_states"\."topic_id"/) && + query.include?('FROM "workflow_states"') + end + + expect(response.status).to eq(200) + expect(workflow_state_topic_id_plucks).to eq([]) + end end From 8d7da3375ad9ccbe1845ecbb6afe222b35d741f3 Mon Sep 17 00:00:00 2001 From: merefield Date: Tue, 24 Feb 2026 12:58:15 +0000 Subject: [PATCH 04/14] PERF: scope chart loading to selected workflow --- .../workflow_charts_controller.rb | 26 +++++++++++-------- .../workflow_charts_controller_spec.rb | 24 +++++++++++++++++ 2 files changed, 39 insertions(+), 11 deletions(-) diff --git a/app/controllers/discourse_workflow/workflow_charts_controller.rb b/app/controllers/discourse_workflow/workflow_charts_controller.rb index bb9960f..15b64f5 100644 --- a/app/controllers/discourse_workflow/workflow_charts_controller.rb +++ b/app/controllers/discourse_workflow/workflow_charts_controller.rb @@ -10,12 +10,7 @@ class WorkflowChartsController < ApplicationController def index weeks = normalized_weeks - workflows = - ::DiscourseWorkflow::Workflow - .where(enabled: true) - .ordered - .includes(workflow_steps: { category: :parent_category }) - selected_workflow = selected_workflow_for(workflows) + selected_workflow = selected_chart_workflow date_range = chart_date_range(weeks) render_json_dump( @@ -55,9 +50,20 @@ def normalized_weeks [requested, 12].min end - def selected_workflow_for(workflows) + def selected_chart_workflow + workflow_scope = ::DiscourseWorkflow::Workflow.where(enabled: true).ordered selected_id = params[:workflow_id].to_i - workflows.find { |workflow| workflow.id == selected_id } || workflows.first + + if selected_id > 0 + selected_workflow = load_chart_workflow(workflow_scope.where(id: selected_id)) + return selected_workflow if selected_workflow.present? + end + + load_chart_workflow(workflow_scope) + end + + def load_chart_workflow(scope) + scope.includes(workflow_steps: { category: :parent_category }).first end def chart_date_range(weeks) @@ -76,9 +82,7 @@ def build_series(workflow, date_range) stats = ::DiscourseWorkflow::WorkflowStat .where(workflow_id: workflow.id, workflow_step_id: step_ids) - .where( - cob_date: date_range.first.beginning_of_day..date_range.last.end_of_day, - ) + .where(cob_date: date_range.first.beginning_of_day..date_range.last.end_of_day) .group("DATE(cob_date)", :workflow_step_id) .sum(:count) counts_by_day_step = diff --git a/spec/requests/workflow_charts_controller_spec.rb b/spec/requests/workflow_charts_controller_spec.rb index 86fdfc8..0909156 100644 --- a/spec/requests/workflow_charts_controller_spec.rb +++ b/spec/requests/workflow_charts_controller_spec.rb @@ -183,6 +183,30 @@ expect(payload).not_to have_key("workflows") end + it "loads chart workflow step data only for the selected workflow" do + sign_in(admin) + + workflow_queries, workflow_steps_queries = + track_sql_queries do + get "/discourse-workflow/charts.json", params: { workflow_id: workflow.id, weeks: 1 } + end.partition { |query| query.include?('FROM "workflows"') } + + workflow_steps_queries.select! do |query| + query.include?('FROM "workflow_steps"') && query.include?('"workflow_steps"."workflow_id"') + end + + unscoped_workflow_query = + workflow_queries.any? do |query| + query.include?('"workflows"."enabled" = TRUE') && !query.include?('"workflows"."id" =') + end + + expect(response.status).to eq(200) + expect(unscoped_workflow_query).to eq(false) + expect(workflow_steps_queries.any? { |query| query.include?(other_workflow.id.to_s) }).to eq( + false, + ) + end + it "supports a one-week horizon when requested" do sign_in(admin) From a8b07c958ee6f31073f4bccb11277e7c15a1d4d6 Mon Sep 17 00:00:00 2001 From: merefield Date: Tue, 24 Feb 2026 13:03:45 +0000 Subject: [PATCH 05/14] PERF: cache workflow metadata include check --- plugin.rb | 30 ++++++++++----------- spec/requests/workflow_list_filters_spec.rb | 14 ++++++++++ 2 files changed, 29 insertions(+), 15 deletions(-) diff --git a/plugin.rb b/plugin.rb index 25babd5..a6334d6 100644 --- a/plugin.rb +++ b/plugin.rb @@ -35,9 +35,7 @@ module ::DiscourseWorkflow SiteSetting.workflow_enabled end - Discourse::Application.routes.prepend do - get "/workflow/charts" => "list#workflow_charts" - end + Discourse::Application.routes.prepend { get "/workflow/charts" => "list#workflow_charts" } Discourse.top_menu_items.push(:workflow) Discourse.anonymous_top_menu_items.push(:workflow) @@ -170,6 +168,12 @@ module ::DiscourseWorkflow end end + add_to_class(:topic_list, :has_workflow_topics?) do + return @has_workflow_topics if defined?(@has_workflow_topics) + + @has_workflow_topics = topics.any? { |topic| topic.workflow_state.present? } + end + add_to_class(:topic_list, :workflow_kanban_compatible) do workflow = workflow_kanban_workflow workflow.present? && workflow.kanban_compatible? @@ -180,13 +184,9 @@ module ::DiscourseWorkflow workflow.present? && workflow.show_kanban_tags != false end - add_to_class(:topic_list, :workflow_single_workflow_id) do - workflow_kanban_workflow&.id - end + add_to_class(:topic_list, :workflow_single_workflow_id) { workflow_kanban_workflow&.id } - add_to_class(:topic_list, :workflow_single_workflow_name) do - workflow_kanban_workflow&.name - end + add_to_class(:topic_list, :workflow_single_workflow_name) { workflow_kanban_workflow&.name } add_to_class(:topic_list, :workflow_kanban_steps) do return [] if !workflow_kanban_compatible @@ -355,7 +355,7 @@ module ::DiscourseWorkflow add_to_serializer( :topic_list, :workflow_kanban_compatible, - include_condition: -> { object.topics.any? { |topic| topic.workflow_state.present? } }, + include_condition: -> { object.has_workflow_topics? }, ) { object.workflow_kanban_compatible } add_to_serializer( @@ -367,31 +367,31 @@ module ::DiscourseWorkflow add_to_serializer( :topic_list, :workflow_single_workflow_id, - include_condition: -> { object.topics.any? { |topic| topic.workflow_state.present? } }, + include_condition: -> { object.has_workflow_topics? }, ) { object.workflow_single_workflow_id } add_to_serializer( :topic_list, :workflow_can_view_charts, - include_condition: -> { object.topics.any? { |topic| topic.workflow_state.present? } }, + include_condition: -> { object.has_workflow_topics? }, ) { DiscourseWorkflow::ChartsPermissions.can_view?(scope.user) } add_to_serializer( :topic_list, :workflow_kanban_show_tags, - include_condition: -> { object.topics.any? { |topic| topic.workflow_state.present? } }, + include_condition: -> { object.has_workflow_topics? }, ) { object.workflow_kanban_show_tags } add_to_serializer( :topic_list, :workflow_kanban_steps, - include_condition: -> { object.topics.any? { |topic| topic.workflow_state.present? } }, + include_condition: -> { object.has_workflow_topics? }, ) { object.workflow_kanban_steps } add_to_serializer( :topic_list, :workflow_kanban_transitions, - include_condition: -> { object.topics.any? { |topic| topic.workflow_state.present? } }, + include_condition: -> { object.has_workflow_topics? }, ) { object.workflow_kanban_transitions } on(:topic_created) do |*params| diff --git a/spec/requests/workflow_list_filters_spec.rb b/spec/requests/workflow_list_filters_spec.rb index 1b24b3b..2b99b20 100644 --- a/spec/requests/workflow_list_filters_spec.rb +++ b/spec/requests/workflow_list_filters_spec.rb @@ -156,4 +156,18 @@ expect(response.status).to eq(200) expect(workflow_state_topic_id_plucks).to eq([]) end + + it "omits workflow metadata when no workflow topics are visible" do + DiscourseWorkflow::WorkflowState.delete_all + + get "/workflow.json" + + topic_list = response.parsed_body["topic_list"] + + expect(topic_list).not_to have_key("workflow_kanban_compatible") + expect(topic_list).not_to have_key("workflow_kanban_workflow_name") + expect(topic_list).not_to have_key("workflow_kanban_steps") + expect(topic_list).not_to have_key("workflow_kanban_transitions") + expect(topic_list).not_to have_key("workflow_can_view_charts") + end end From 290f8e23490f671902598a8fd2a3f4a2550dc40c Mon Sep 17 00:00:00 2001 From: merefield Date: Tue, 24 Feb 2026 13:04:05 +0000 Subject: [PATCH 06/14] PERF: avoid transition lookup round-trips --- .../workflow_action_controller.rb | 8 +- lib/discourse_workflow/transition.rb | 159 ++++++++++-------- spec/lib/transition_spec.rb | 75 ++++++--- 3 files changed, 145 insertions(+), 97 deletions(-) diff --git a/app/controllers/discourse_workflow/workflow_action_controller.rb b/app/controllers/discourse_workflow/workflow_action_controller.rb index 5eb19c2..5ce31e1 100644 --- a/app/controllers/discourse_workflow/workflow_action_controller.rb +++ b/app/controllers/discourse_workflow/workflow_action_controller.rb @@ -7,13 +7,13 @@ class WorkflowActionController < ApplicationController def act topic = Topic.find(params[:topic_id]) guardian.ensure_can_create_topic_on_category!(topic.category_id) - user_id = current_user.id + user = current_user option = params[:option] cooldown_key = nil successful_transition = false - if user_id.present? && option.present? && topic.present? - cooldown_key = "discourse-workflow-transition-#{user_id}-#{topic.id}" + if user.present? && option.present? && topic.present? + cooldown_key = "discourse-workflow-transition-#{user.id}-#{topic.id}" cooldown_acquired = Discourse.redis.set(cooldown_key, "1", ex: 5, nx: true) if !cooldown_acquired @@ -21,7 +21,7 @@ def act return end - successful_transition = Transition.new.transition(user_id, topic, option) + successful_transition = Transition.new.transition(user, topic, option) end Discourse.redis.del(cooldown_key) if !successful_transition && cooldown_key.present? diff --git a/lib/discourse_workflow/transition.rb b/lib/discourse_workflow/transition.rb index 1573d2a..78d4264 100644 --- a/lib/discourse_workflow/transition.rb +++ b/lib/discourse_workflow/transition.rb @@ -2,98 +2,113 @@ module DiscourseWorkflow class Transition - def transition(user_id, topic, option) + def transition(actor, topic, option) success = false workflow_state = - DiscourseWorkflow::WorkflowState - .includes(:workflow, workflow_step: { workflow_step_options: :workflow_option }) - .find_by(topic_id: topic.id) + DiscourseWorkflow::WorkflowState.includes( + :workflow, + { workflow: :workflow_steps }, + { workflow_step: { workflow_step_options: :workflow_option } }, + ).find_by(topic_id: topic.id) return false unless workflow_state && topic current_step = workflow_state.workflow_step return false unless current_step - starting_step_id = current_step.id - starting_step_name = current_step.name - starting_position = current_step.position - starting_category_id = topic.category_id + step_option_by_slug = + current_step.workflow_step_options.index_by do |workflow_step_option| + workflow_step_option.workflow_option&.slug + end + workflow_step_option = step_option_by_slug[option] + return false if workflow_step_option.blank? + + target_step = + workflow_state.workflow.workflow_steps.find do |workflow_step| + workflow_step.id == workflow_step_option.target_step_id + end + return false if target_step.blank? + + user_id, username = resolve_actor(actor) + + starting_step_id = current_step.id + starting_step_name = current_step.name + starting_position = current_step.position + starting_category_id = topic.category_id starting_category_name = topic.category&.name WorkflowState.transaction do - current_step.workflow_step_options.each do |workflow_step_option| - next unless workflow_step_option.workflow_option&.slug == option - - step_option_id = workflow_step_option.id - step_option_name = workflow_step_option.workflow_option.name - step_option_slug = workflow_step_option.workflow_option.slug - - target_step = - DiscourseWorkflow::WorkflowStep.find_by(id: workflow_step_option.target_step_id) - next unless target_step - - # move topic + workflow state - topic.update!(category_id: target_step.category_id) - workflow_state.update!(workflow_step_id: target_step.id) - - if user_id.present? - user = User.find(user_id) - username = user.username - else - user_id = -1 - username = "system" - end - - WorkflowAuditLog.create!( - user_id: user_id, - username: username, - topic_id: topic.id, - topic_title: topic.title, - workflow_id: workflow_state.workflow_id, - workflow_name: workflow_state.workflow.name, - starting_step_id: starting_step_id, - starting_step_name: starting_step_name, - ending_step_id: workflow_state.workflow_step_id, - ending_step_name: workflow_state.workflow_step.name, - starting_category_id: starting_category_id, - starting_category_name: starting_category_name, - ending_category_id: topic.category_id, - ending_category_name: topic.category&.name, - starting_position: starting_position, - ending_position: workflow_state.workflow_step.position, - step_option_id: step_option_id, - step_option_name: step_option_name, - step_option_slug: step_option_slug - ) - - Post.create!( - user_id: user_id, - topic_id: topic.id, - raw: I18n.t( - "discourse_workflow.topic_transition_action_description", - starting_step_name: starting_step_name, - ending_step_name: workflow_state.workflow_step.name, - username: username, - step_option_name: step_option_name - ), - post_type: Post.types[:small_action], - action_code: "workflow_transition" - ) - - success = true - # no need to keep iterating once we matched the option - break - end + step_option_id = workflow_step_option.id + step_option_name = workflow_step_option.workflow_option.name + step_option_slug = workflow_step_option.workflow_option.slug + + # move topic + workflow state + topic.update!(category_id: target_step.category_id) + workflow_state.update!(workflow_step_id: target_step.id) + + WorkflowAuditLog.create!( + user_id: user_id, + username: username, + topic_id: topic.id, + topic_title: topic.title, + workflow_id: workflow_state.workflow_id, + workflow_name: workflow_state.workflow.name, + starting_step_id: starting_step_id, + starting_step_name: starting_step_name, + ending_step_id: workflow_state.workflow_step_id, + ending_step_name: target_step.name, + starting_category_id: starting_category_id, + starting_category_name: starting_category_name, + ending_category_id: topic.category_id, + ending_category_name: topic.category&.name, + starting_position: starting_position, + ending_position: target_step.position, + step_option_id: step_option_id, + step_option_name: step_option_name, + step_option_slug: step_option_slug, + ) + + Post.create!( + user_id: user_id, + topic_id: topic.id, + raw: + I18n.t( + "discourse_workflow.topic_transition_action_description", + starting_step_name: starting_step_name, + ending_step_name: target_step.name, + username: username, + step_option_name: step_option_name, + ), + post_type: Post.types[:small_action], + action_code: "workflow_transition", + ) + + success = true end if success && topic.category_id != starting_category_id ::Jobs::DiscourseWorkflow::TopicArrivalNotifier.perform_async( - { topic_id: topic.id }.as_json + { topic_id: topic.id }.as_json, ) end success end + + private + + def resolve_actor(actor) + if actor.is_a?(::User) + [actor.id, actor.username] + elsif actor.present? + user_id = actor.to_i + username = ::User.where(id: user_id).pick(:username) + raise ActiveRecord::RecordNotFound if username.blank? + [user_id, username] + else + [-1, "system"] + end + end end end diff --git a/spec/lib/transition_spec.rb b/spec/lib/transition_spec.rb index 49212c8..dab4276 100644 --- a/spec/lib/transition_spec.rb +++ b/spec/lib/transition_spec.rb @@ -1,42 +1,63 @@ # frozen_string_literal: true -require_relative '../plugin_helper' +require_relative "../plugin_helper" describe ::DiscourseWorkflow::Transition do - - fab!(:workflow) { + fab!(:workflow) do Fabricate(:workflow, name: "Test Workflow", description: "Test Workflow Description") - } - fab!(:step_1) { - Fabricate(:workflow_step, workflow_id: workflow.id, name: "Step 1", description: "Step 1 Description") - } - fab!(:step_2) { - Fabricate(:workflow_step, workflow_id: workflow.id, name: "Step 2", description: "Step 2 Description") - } + end + fab!(:step_1) do + Fabricate( + :workflow_step, + workflow_id: workflow.id, + name: "Step 1", + description: "Step 1 Description", + ) + end + fab!(:step_2) do + Fabricate( + :workflow_step, + workflow_id: workflow.id, + name: "Step 2", + description: "Step 2 Description", + ) + end fab!(:option_1, :workflow_option) - fab!(:step_option_1) { - Fabricate(:workflow_step_option, workflow_step_id: step_1.id, workflow_option_id: option_1.id, target_step_id: step_2.id) - } + fab!(:step_option_1) do + Fabricate( + :workflow_step_option, + workflow_step_id: step_1.id, + workflow_option_id: option_1.id, + target_step_id: step_2.id, + ) + end fab!(:topic) - fab!(:workflow_state) { - Fabricate(:workflow_state, topic_id: topic.id, workflow_id: workflow.id, workflow_step_id: step_1.id) - } + fab!(:workflow_state) do + Fabricate( + :workflow_state, + topic_id: topic.id, + workflow_id: workflow.id, + workflow_step_id: step_1.id, + ) + end fab!(:user) - fab!(:transition) { - DiscourseWorkflow::Transition.new - } + fab!(:transition) { DiscourseWorkflow::Transition.new } it "creates an audit log entry" do - expect { transition.transition(user.id, topic, option_1.slug) }.to change { ::DiscourseWorkflow::WorkflowAuditLog.count }.by(1) + expect { transition.transition(user.id, topic, option_1.slug) }.to change { + ::DiscourseWorkflow::WorkflowAuditLog.count + }.by(1) end it "updates the topic with a small action Post" do - expect { transition.transition(user.id, topic, option_1.slug) }.to change { ::Post.where(topic_id: topic.id).count }.by(1) + expect { transition.transition(user.id, topic, option_1.slug) }.to change { + ::Post.where(topic_id: topic.id).count + }.by(1) end it "writes a transition action that includes from and to step names" do @@ -47,4 +68,16 @@ expect(small_action_post.raw).to include(step_1.name) expect(small_action_post.raw).to include(step_2.name) end + + it "does not perform per-transition target-step lookup queries" do + sql_queries = track_sql_queries { transition.transition(user, topic, option_1.slug) } + + target_step_lookup_queries = + sql_queries.select do |query| + query.include?('FROM "workflow_steps"') && query.include?('"workflow_steps"."id" =') && + query.include?("LIMIT 1") + end + + expect(target_step_lookup_queries).to eq([]) + end end From 9c9058673024b4f88c7c903fd9dcabb74f08a496 Mon Sep 17 00:00:00 2001 From: merefield Date: Tue, 24 Feb 2026 13:04:48 +0000 Subject: [PATCH 07/14] PERF: index workflow state staleness lookups --- app/models/discourse_workflow/workflow_state.rb | 3 ++- ...00_add_updated_at_index_to_workflow_states.rb | 16 ++++++++++++++++ spec/lib/workflow_state_schema_spec.rb | 11 +++++++++++ 3 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20260223140000_add_updated_at_index_to_workflow_states.rb create mode 100644 spec/lib/workflow_state_schema_spec.rb diff --git a/app/models/discourse_workflow/workflow_state.rb b/app/models/discourse_workflow/workflow_state.rb index 210c08b..0acc49e 100644 --- a/app/models/discourse_workflow/workflow_state.rb +++ b/app/models/discourse_workflow/workflow_state.rb @@ -2,7 +2,7 @@ module ::DiscourseWorkflow class WorkflowState < ActiveRecord::Base - self.table_name = 'workflow_states' + self.table_name = "workflow_states" belongs_to :topic belongs_to :workflow @@ -26,6 +26,7 @@ class WorkflowState < ActiveRecord::Base # index_workflow_states_on_topic_id (topic_id) # index_workflow_states_on_workflow_id (workflow_id) # index_workflow_states_on_workflow_step_id (workflow_step_id) +# idx_workflow_states_updated_at (updated_at) # # Foreign Keys # diff --git a/db/migrate/20260223140000_add_updated_at_index_to_workflow_states.rb b/db/migrate/20260223140000_add_updated_at_index_to_workflow_states.rb new file mode 100644 index 0000000..19c8b6e --- /dev/null +++ b/db/migrate/20260223140000_add_updated_at_index_to_workflow_states.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class AddUpdatedAtIndexToWorkflowStates < ActiveRecord::Migration[7.2] + disable_ddl_transaction! + + INDEX_NAME = "idx_workflow_states_updated_at" + + def up + remove_index(:workflow_states, name: INDEX_NAME, if_exists: true, algorithm: :concurrently) + add_index(:workflow_states, :updated_at, name: INDEX_NAME, algorithm: :concurrently) + end + + def down + remove_index(:workflow_states, name: INDEX_NAME, if_exists: true, algorithm: :concurrently) + end +end diff --git a/spec/lib/workflow_state_schema_spec.rb b/spec/lib/workflow_state_schema_spec.rb new file mode 100644 index 0000000..48904da --- /dev/null +++ b/spec/lib/workflow_state_schema_spec.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +require_relative "../plugin_helper" + +RSpec.describe DiscourseWorkflow::WorkflowState do + it "has an updated_at index for overdue filtering queries" do + index_columns = described_class.connection.indexes(described_class.table_name).map(&:columns) + + expect(index_columns).to include(["updated_at"]) + end +end From 62de1186161ce4d55821bd283d768038f69650b0 Mon Sep 17 00:00:00 2001 From: merefield Date: Tue, 24 Feb 2026 13:14:05 +0000 Subject: [PATCH 08/14] DOCS: update performance roadmap status --- README.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 7907512..2f0b1fb 100644 --- a/README.md +++ b/README.md @@ -226,7 +226,11 @@ Permissioning principle: | SLA | Escalation/reminder notifications | Partial | Overdue visibility exists; automated escalation is next | | Ownership | Discourse Assign integration | Planned | Target is step-entry assignment and auditable ownership changes | | Operations | Bulk workflow transitions from list views | Missing | High-volume queue operation not yet first-class | -| Performance | Eliminate admin serialization N+1 queries | Planned | Preload workflow step/category associations in admin payloads for large workflow edit/list screens | +| Performance | Admin/list/chart/transition query-path N+1 and over-fetch hardening | Implemented | Admin serializers preloaded, workflow quick filters now SQL-scoped, chart loading scoped to selected workflow, transition lookup round-trips reduced, and workflow-state staleness indexing added | +| Performance | Bulk workflow arrival notification fan-out | Planned | Use bulk insert (`insert_all`) for category watcher notifications to reduce per-user insert overhead at high watcher counts | +| Performance | Cache workflow chart payloads | Planned | Add short-lived caching keyed by workflow and period to reduce repeated chart aggregation for frequent refreshes | +| Performance | Cache workflow visualisation payloads | Planned | Cache graph payloads keyed by topic/workflow-state version to avoid rebuilding identical visualisations | +| Performance | Production query-plan validation for workflow filters | Partial | Query shape is now SQL-driven; continue with `EXPLAIN`/index tuning against large production-like datasets | | Reporting | Built-in workflow analytics dashboards | Partial | Data Explorer support exists; admin-native reporting is next | | Lifecycle | Import/export/version workflow definitions | Missing | Useful for staging->production promotion and rollback | | Integration | Event hooks / webhooks / automation integration | Planned | Transition and step events are good integration points | From 5799bd63d8065c2aa8282e1a0dc740f6eb3cfe2d Mon Sep 17 00:00:00 2001 From: merefield Date: Tue, 24 Feb 2026 13:15:26 +0000 Subject: [PATCH 09/14] CHORE: bump patch version --- plugin.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin.rb b/plugin.rb index a6334d6..f87cf0f 100644 --- a/plugin.rb +++ b/plugin.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true # name: discourse-workflow # about: A topic-based workflow engine for Discourse -# version: 0.4.0 +# version: 0.4.1 # authors: Robert Barrow # contact_emails: robert@pavilion.tech # url: https://github.com/merefield/discourse-workflow From bc9eada5cec0217d9476647b01e8a1518662a676 Mon Sep 17 00:00:00 2001 From: merefield Date: Tue, 24 Feb 2026 13:23:39 +0000 Subject: [PATCH 10/14] CHORE: update workflow_stat annotations --- app/models/discourse_workflow/workflow_stat.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/discourse_workflow/workflow_stat.rb b/app/models/discourse_workflow/workflow_stat.rb index cc9e477..c1b5d51 100644 --- a/app/models/discourse_workflow/workflow_stat.rb +++ b/app/models/discourse_workflow/workflow_stat.rb @@ -32,8 +32,8 @@ class WorkflowStat < ActiveRecord::Base # Indexes # # idx_workflow_stats_daily_workflow_step_unique (cob_date,workflow_id,workflow_step_id) UNIQUE -# index_workflow_stats_on_workflow_id (workflow_id) -# index_workflow_stats_on_workflow_step_id (workflow_step_id) +# index_workflow_stats_on_workflow_id (workflow_id) +# index_workflow_stats_on_workflow_step_id (workflow_step_id) # # Foreign Keys # From 8c7451febb81932c225b35201fd3708f6919ed08 Mon Sep 17 00:00:00 2001 From: merefield Date: Tue, 24 Feb 2026 13:30:04 +0000 Subject: [PATCH 11/14] FIX: remove placeholder strong params keys --- .../discourse_workflow/admin/workflow_step_options_controller.rb | 1 - .../discourse_workflow/admin/workflow_steps_controller.rb | 1 - 2 files changed, 2 deletions(-) diff --git a/app/controllers/discourse_workflow/admin/workflow_step_options_controller.rb b/app/controllers/discourse_workflow/admin/workflow_step_options_controller.rb index 784ef1b..7e369b3 100644 --- a/app/controllers/discourse_workflow/admin/workflow_step_options_controller.rb +++ b/app/controllers/discourse_workflow/admin/workflow_step_options_controller.rb @@ -117,7 +117,6 @@ def workflow_step_option_params :workflow_step_id, :workflow_option_id, :target_step_id, - :other_attributes..., ) end diff --git a/app/controllers/discourse_workflow/admin/workflow_steps_controller.rb b/app/controllers/discourse_workflow/admin/workflow_steps_controller.rb index 472a942..96fd11e 100644 --- a/app/controllers/discourse_workflow/admin/workflow_steps_controller.rb +++ b/app/controllers/discourse_workflow/admin/workflow_steps_controller.rb @@ -114,7 +114,6 @@ def workflow_step_params :ai_enabled, :ai_prompt, :overdue_days, - :other_attributes..., ) end From d6d5757b78e80accf0a3094d5c421e8df65d7fca Mon Sep 17 00:00:00 2001 From: merefield Date: Tue, 24 Feb 2026 13:36:23 +0000 Subject: [PATCH 12/14] FIX: ignore invalid workflow states in daily stats --- lib/discourse_workflow/stats.rb | 6 ++++- spec/lib/stats_spec.rb | 43 +++++++++++++++++++++------------ 2 files changed, 33 insertions(+), 16 deletions(-) diff --git a/lib/discourse_workflow/stats.rb b/lib/discourse_workflow/stats.rb index 6f156ea..c7b144c 100644 --- a/lib/discourse_workflow/stats.rb +++ b/lib/discourse_workflow/stats.rb @@ -7,7 +7,11 @@ def calculate_daily_stats current_date = Date.current now = Time.zone.now counts_by_workflow_step = - ::DiscourseWorkflow::WorkflowState.group(:workflow_id, :workflow_step_id).count + ::DiscourseWorkflow::WorkflowState + .where.not(workflow_id: nil) + .where.not(workflow_step_id: nil) + .group(:workflow_id, :workflow_step_id) + .count records = counts_by_workflow_step.map do |(workflow_id, workflow_step_id), count| { diff --git a/spec/lib/stats_spec.rb b/spec/lib/stats_spec.rb index 5dac1eb..d3990e1 100644 --- a/spec/lib/stats_spec.rb +++ b/spec/lib/stats_spec.rb @@ -7,20 +7,10 @@ fab!(:category_1, :category) fab!(:category_2, :category) fab!(:step_1) do - Fabricate( - :workflow_step, - workflow_id: workflow.id, - category_id: category_1.id, - position: 1, - ) + Fabricate(:workflow_step, workflow_id: workflow.id, category_id: category_1.id, position: 1) end fab!(:step_2) do - Fabricate( - :workflow_step, - workflow_id: workflow.id, - category_id: category_2.id, - position: 2, - ) + Fabricate(:workflow_step, workflow_id: workflow.id, category_id: category_2.id, position: 2) end fab!(:topic_1) { Fabricate(:topic, category: category_1) } fab!(:topic_2) { Fabricate(:topic, category: category_1) } @@ -44,8 +34,31 @@ it "calculates daily counts per workflow step from workflow states" do SiteSetting.workflow_enabled = true - expect do - described_class.new.calculate_daily_stats - end.to change { DiscourseWorkflow::WorkflowStat.count }.by(2) + expect do described_class.new.calculate_daily_stats end.to change { + DiscourseWorkflow::WorkflowStat.count + }.by(2) + end + + it "ignores corrupted states with null workflow_id or workflow_step_id" do + SiteSetting.workflow_enabled = true + + state_1.update_columns(workflow_id: nil) + state_2.update_columns(workflow_step_id: nil) + + valid_topic = Fabricate(:topic, category: category_1) + Fabricate( + :workflow_state, + topic_id: valid_topic.id, + workflow_id: workflow.id, + workflow_step_id: step_1.id, + ) + + described_class.new.calculate_daily_stats + + today_stats = DiscourseWorkflow::WorkflowStat.where(cob_date: Date.current) + expect(today_stats.count).to eq(1) + expect(today_stats.first.workflow_id).to eq(workflow.id) + expect(today_stats.first.workflow_step_id).to eq(step_1.id) + expect(today_stats.first.count).to eq(1) end end From eeef2db9a313ec0fc6966afc51972d7b2a1e55a5 Mon Sep 17 00:00:00 2001 From: merefield Date: Tue, 24 Feb 2026 13:39:19 +0000 Subject: [PATCH 13/14] FIX: log target category name in transitions --- lib/discourse_workflow/transition.rb | 6 +++--- spec/lib/transition_spec.rb | 15 ++++++++++++++- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/lib/discourse_workflow/transition.rb b/lib/discourse_workflow/transition.rb index 78d4264..ea24b73 100644 --- a/lib/discourse_workflow/transition.rb +++ b/lib/discourse_workflow/transition.rb @@ -7,8 +7,7 @@ def transition(actor, topic, option) workflow_state = DiscourseWorkflow::WorkflowState.includes( - :workflow, - { workflow: :workflow_steps }, + { workflow: { workflow_steps: :category } }, { workflow_step: { workflow_step_options: :workflow_option } }, ).find_by(topic_id: topic.id) @@ -37,6 +36,7 @@ def transition(actor, topic, option) starting_position = current_step.position starting_category_id = topic.category_id starting_category_name = topic.category&.name + ending_category_name = target_step.category&.name WorkflowState.transaction do step_option_id = workflow_step_option.id @@ -61,7 +61,7 @@ def transition(actor, topic, option) starting_category_id: starting_category_id, starting_category_name: starting_category_name, ending_category_id: topic.category_id, - ending_category_name: topic.category&.name, + ending_category_name: ending_category_name, starting_position: starting_position, ending_position: target_step.position, step_option_id: step_option_id, diff --git a/spec/lib/transition_spec.rb b/spec/lib/transition_spec.rb index dab4276..bef5da0 100644 --- a/spec/lib/transition_spec.rb +++ b/spec/lib/transition_spec.rb @@ -2,6 +2,9 @@ require_relative "../plugin_helper" describe ::DiscourseWorkflow::Transition do + fab!(:category_1, :category) + fab!(:category_2, :category) + fab!(:workflow) do Fabricate(:workflow, name: "Test Workflow", description: "Test Workflow Description") end @@ -9,6 +12,7 @@ Fabricate( :workflow_step, workflow_id: workflow.id, + category_id: category_1.id, name: "Step 1", description: "Step 1 Description", ) @@ -17,6 +21,7 @@ Fabricate( :workflow_step, workflow_id: workflow.id, + category_id: category_2.id, name: "Step 2", description: "Step 2 Description", ) @@ -33,7 +38,7 @@ ) end - fab!(:topic) + fab!(:topic) { Fabricate(:topic, category: category_1) } fab!(:workflow_state) do Fabricate( @@ -80,4 +85,12 @@ expect(target_step_lookup_queries).to eq([]) end + + it "writes ending category name from the target step category" do + transition.transition(user, topic, option_1.slug) + + audit_log = DiscourseWorkflow::WorkflowAuditLog.order(:id).last + expect(audit_log.starting_category_name).to eq(category_1.name) + expect(audit_log.ending_category_name).to eq(category_2.name) + end end From 193691d123853c28abd011ac30d81ffad61f80a6 Mon Sep 17 00:00:00 2001 From: merefield Date: Tue, 24 Feb 2026 13:54:38 +0000 Subject: [PATCH 14/14] UX: stop chart lines at current day when future stats are unavailable --- .../workflow_charts_controller.rb | 6 +++++- .../workflow_charts_controller_spec.rb | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/app/controllers/discourse_workflow/workflow_charts_controller.rb b/app/controllers/discourse_workflow/workflow_charts_controller.rb index 15b64f5..d21d59e 100644 --- a/app/controllers/discourse_workflow/workflow_charts_controller.rb +++ b/app/controllers/discourse_workflow/workflow_charts_controller.rb @@ -77,6 +77,7 @@ def build_series(workflow, date_range) steps = workflow.workflow_steps.sort_by { |step| step.position.to_i } return [] if steps.blank? + today = Date.current step_ids = steps.map(&:id) stats = @@ -97,7 +98,10 @@ def build_series(workflow, date_range) step_name: step.name, step_position: step.position.to_i, color: category&.color || category&.parent_category&.color, - data: date_range.map { |date| counts_by_day_step.fetch([date, step.id], 0) }, + data: + date_range.map do |date| + date > today ? nil : counts_by_day_step.fetch([date, step.id], 0) + end, } end end diff --git a/spec/requests/workflow_charts_controller_spec.rb b/spec/requests/workflow_charts_controller_spec.rb index 0909156..442fdd0 100644 --- a/spec/requests/workflow_charts_controller_spec.rb +++ b/spec/requests/workflow_charts_controller_spec.rb @@ -218,6 +218,24 @@ expect(payload["labels"].length).to eq(7) end + it "returns nil data points for future days so lines stop at the current day" do + freeze_time(Time.zone.parse("2026-02-18 10:00:00 UTC")) do + sign_in(admin) + + get "/discourse-workflow/charts.json", params: { workflow_id: workflow.id, weeks: 1 } + + payload = response.parsed_body + labels = payload["labels"] + future_indexes = labels.each_index.select { |index| Date.parse(labels[index]) > Date.current } + + expect(future_indexes).to be_present + + payload["series"].each do |series| + future_indexes.each { |index| expect(series["data"][index]).to be_nil } + end + end + end + def create_stats_history end_date = Date.current.end_of_week(:saturday) start_date = end_date - 13.days