From b89dd475b4f53a082852c49ca9bebe33a5eba99a Mon Sep 17 00:00:00 2001 From: Carl Mercier Date: Tue, 28 Jul 2026 10:21:35 -0500 Subject: [PATCH 1/4] refactor(jobs): derive job state and payload columns in one place JobRow.state_for is now the single owner of the finished_at -> annotation -> :unknown rule that JobsQuery and JobDetail each spelled out themselves, and PAYLOAD_COLUMNS replaces the two spellings of "every column except the multi-kilobyte payloads" in JobsQuery. Co-Authored-By: Claude Opus 5 (1M context) --- app/models/flightdeck/job_detail.rb | 9 +-------- app/models/flightdeck/job_row.rb | 10 ++++++++++ app/models/flightdeck/jobs_query.rb | 24 ++++++++---------------- 3 files changed, 19 insertions(+), 24 deletions(-) diff --git a/app/models/flightdeck/job_detail.rb b/app/models/flightdeck/job_detail.rb index 5cf99f4..5072b9f 100644 --- a/app/models/flightdeck/job_detail.rb +++ b/app/models/flightdeck/job_detail.rb @@ -27,7 +27,7 @@ def self.find(id) def initialize(job:, annotation: nil) @job = job - @state = resolve_state(annotation) + @state = JobRow.state_for(job, annotation) @execution = load_execution(annotation) @process = load_process end @@ -140,13 +140,6 @@ def process_label end private - def resolve_state(annotation) - return :finished if job.finished_at.present? - return annotation[:state] if annotation - - :unknown - end - def load_execution(annotation) return nil unless annotation diff --git a/app/models/flightdeck/job_row.rb b/app/models/flightdeck/job_row.rb index fac1175..64f7b11 100644 --- a/app/models/flightdeck/job_row.rb +++ b/app/models/flightdeck/job_row.rb @@ -21,6 +21,16 @@ def model_for(state) name && name.constantize end + # How a job's state is derived, in one place: `finished_at` wins, then + # whichever execution table `annotate` found the job in, then :unknown + # for the transition window where a job is between tables. + def state_for(job, annotation) + return :finished if job.finished_at.present? + return annotation[:state] if annotation + + :unknown + end + # Resolves the state of an already-loaded page of jobs with one # `WHERE job_id IN (page_ids)` lookup per executions table. Returns # { job_id => { state:, execution_id:, process_id: } }. diff --git a/app/models/flightdeck/jobs_query.rb b/app/models/flightdeck/jobs_query.rb index b5621b2..712193c 100644 --- a/app/models/flightdeck/jobs_query.rb +++ b/app/models/flightdeck/jobs_query.rb @@ -90,7 +90,9 @@ def self.state_counts(**filters) end private - JOB_LIST_COLUMNS = -> { SolidQueue::Job.column_names - [ "arguments" ] } + # The columns list queries must never drag along: `arguments` on jobs and + # `error` on failed_executions can each run to kilobytes per row. + PAYLOAD_COLUMNS = %w[arguments error].freeze # Only the filters change what a count means — the cursor and page size do # not, so they are deliberately absent from the key. @@ -124,11 +126,10 @@ def paginated_records relation.select(driving_columns).to_a end - # Never `SELECT *`: it would drag `arguments` (jobs) or `error` - # (failed_executions) through every list query. + # Never `SELECT *`: it would drag a payload column through every list + # query. def driving_columns - names = model.column_names - [ "arguments", "error" ] - names.map { |name| model.arel_table[name] } + (model.column_names - PAYLOAD_COLUMNS).map { |name| model.arel_table[name] } end def rows_from_executions(executions) @@ -159,25 +160,16 @@ def rows_from_jobs(jobs) annotations = state == :finished ? {} : JobRow.annotate(job_ids) jobs.map do |job| - annotation = annotations[job.id] - JobRow.new( job: job, - state: state_for(job, annotation), + state: JobRow.state_for(job, annotations[job.id]), args_preview: previews[job.id] ) end end - def state_for(job, annotation) - return :finished if job.finished_at.present? - return annotation[:state] if annotation - - :unknown - end - def jobs_by_id(job_ids) - SolidQueue::Job.where(id: job_ids).select(JOB_LIST_COLUMNS.call).index_by(&:id) + SolidQueue::Job.where(id: job_ids).select(SolidQueue::Job.column_names - PAYLOAD_COLUMNS).index_by(&:id) end def processes_by_id(process_ids) From 76ea7a8821579081331910b0848b9545665a0d68 Mon Sep 17 00:00:00 2001 From: Carl Mercier Date: Tue, 28 Jul 2026 10:21:46 -0500 Subject: [PATCH 2/4] refactor: write durations through one formatter fd_duration, Overview's tiles and the completion chart's axis each had their own ms/s/m/h case ladder with subtly different rounding. Flightdeck::Duration.humanize is now the one way a number of seconds is written; overview tiles carry display-ready values, so the view no longer switches on the value's type. Co-Authored-By: Claude Opus 5 (1M context) --- app/helpers/flightdeck/jobs_helper.rb | 11 +------ app/models/flightdeck/duration.rb | 23 ++++++++++++++ app/models/flightdeck/metrics/line_chart.rb | 10 ++---- app/models/flightdeck/overview.rb | 31 +++++++------------ app/views/flightdeck/overview/_tiles.html.erb | 2 +- 5 files changed, 40 insertions(+), 37 deletions(-) create mode 100644 app/models/flightdeck/duration.rb diff --git a/app/helpers/flightdeck/jobs_helper.rb b/app/helpers/flightdeck/jobs_helper.rb index 614e2dd..4ceadf4 100644 --- a/app/helpers/flightdeck/jobs_helper.rb +++ b/app/helpers/flightdeck/jobs_helper.rb @@ -54,16 +54,7 @@ def fd_full_time(time) end def fd_duration(seconds) - return "—" if seconds.nil? - - seconds = seconds.to_f.abs - case seconds - when 0...1 then "#{(seconds * 1000).round}ms" - when 1...60 then "#{seconds.round(1)}s" - when 60...3600 then "#{(seconds / 60).round}m" - when 3600...86_400 then "#{(seconds / 3600).round(1)}h" - else "#{(seconds / 86_400).round(1)}d" - end + seconds.nil? ? "—" : Flightdeck::Duration.humanize(seconds) end def fd_ago(time) diff --git a/app/models/flightdeck/duration.rb b/app/models/flightdeck/duration.rb new file mode 100644 index 0000000..0fe1957 --- /dev/null +++ b/app/models/flightdeck/duration.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module Flightdeck + # The one way a number of seconds is written anywhere in Flightdeck — + # list columns, overview tiles and chart axes all speak through this, so + # "how long" can never be spelled three subtly different ways. + module Duration + module_function + + def humanize(seconds) + value = seconds.to_f.abs + case value + when 0...1 then "#{(value * 1000).round}ms" + when 1...60 then "#{trim(value.round(1))}s" + when 60...3600 then "#{(value / 60).round}m" + when 3600...86_400 then "#{trim((value / 3600).round(1))}h" + else "#{trim((value / 86_400).round(1))}d" + end + end + + def trim(number) = number.to_s.sub(/\.0\z/, "") + end +end diff --git a/app/models/flightdeck/metrics/line_chart.rb b/app/models/flightdeck/metrics/line_chart.rb index 98c3dde..d8f5b54 100644 --- a/app/models/flightdeck/metrics/line_chart.rb +++ b/app/models/flightdeck/metrics/line_chart.rb @@ -49,14 +49,10 @@ def any_data? = markers.any? private def raw_max = points.filter_map(&:seconds).max.to_f + # The zero gridline reads "0s" rather than "0ms": it is an axis origin, + # not a measurement. def format_value(value) - case value - when 0 then "0s" - when 0...1 then "#{(value * 1000).round}ms" - when 1...60 then "#{value.round(1).to_s.sub(/\.0\z/, "")}s" - when 60...3600 then "#{(value / 60).round}m" - else "#{(value / 3600).round(1)}h" - end + value.zero? ? "0s" : Flightdeck::Duration.humanize(value) end def point_title(point) diff --git a/app/models/flightdeck/overview.rb b/app/models/flightdeck/overview.rb index 397e888..eb5a12f 100644 --- a/app/models/flightdeck/overview.rb +++ b/app/models/flightdeck/overview.rb @@ -8,6 +8,8 @@ class Overview RECENT_FAILURES = 5 TOP_QUEUES = 6 + # `value` is always display-ready text — a delimited count, a duration, or + # "—" — so the view prints it without asking what kind of number it was. # trend is :up / :down / :flat and says which way the number moved, while # `good` says whether that movement is a good thing — more throughput is # good, more failures are not. @@ -143,7 +145,7 @@ def processed_tile Tile.new( label: "Processed · 24h", - value: processed_24h, + value: number(processed_24h), detail: delta.nil? ? "no prior window to compare" : "#{delta.abs}% vs prior 24h", trend: trend_for(delta), good: delta.nil? || delta >= 0 @@ -155,7 +157,7 @@ def failed_tile Tile.new( label: "Failed · 24h", - value: failed_24h, + value: number(failed_24h), detail: rate.nil? ? "nothing finished yet" : "#{rate}% failure rate", trend: failed_24h.positive? ? :up : :flat, good: failed_24h.zero? @@ -163,24 +165,24 @@ def failed_tile end def ready_tile - Tile.new(label: "Ready now", value: ready_now, detail: "waiting to be claimed", trend: :flat, good: true) + Tile.new(label: "Ready now", value: number(ready_now), detail: "waiting to be claimed", trend: :flat, good: true) end def scheduled_tile detail = if next_scheduled_at.nil? then "nothing scheduled" elsif next_scheduled_at <= now then "due now" - else "next due in #{humanize_duration(next_scheduled_at - now)}" + else "next due in #{Duration.humanize(next_scheduled_at - now)}" end - Tile.new(label: "Scheduled", value: scheduled, detail: detail, trend: :flat, good: true) + Tile.new(label: "Scheduled", value: number(scheduled), detail: detail, trend: :flat, good: true) end def in_progress_tile Tile.new( label: "In progress", - value: in_progress, - unit: slots ? "/ #{ActiveSupport::NumberHelper.number_to_delimited(slots)} slots" : nil, + value: number(in_progress), + unit: slots ? "/ #{number(slots)} slots" : nil, detail: utilization ? "#{utilization}% utilization" : "no worker capacity reported", trend: :flat, good: true @@ -190,28 +192,19 @@ def in_progress_tile def oldest_ready_tile Tile.new( label: "Oldest ready", - value: oldest_ready_age ? humanize_duration(oldest_ready_age) : "—", + value: oldest_ready_age ? Duration.humanize(oldest_ready_age) : "—", detail: oldest_ready_age ? "longest a job has waited" : "nothing waiting", trend: :flat, good: true ) end + def number(value) = ActiveSupport::NumberHelper.number_to_delimited(value) + def trend_for(delta) return :flat if delta.nil? || delta.abs < 0.05 delta.positive? ? :up : :down end - - def humanize_duration(seconds) - seconds = seconds.to_f.abs - case seconds - when 0...1 then "#{(seconds * 1000).round}ms" - when 1...60 then "#{seconds.round(1).to_s.sub(/\.0\z/, "")}s" - when 60...3600 then "#{(seconds / 60).round}m" - when 3600...86_400 then "#{(seconds / 3600).round(1)}h" - else "#{(seconds / 86_400).round(1)}d" - end - end end end diff --git a/app/views/flightdeck/overview/_tiles.html.erb b/app/views/flightdeck/overview/_tiles.html.erb index 71aa06c..368171f 100644 --- a/app/views/flightdeck/overview/_tiles.html.erb +++ b/app/views/flightdeck/overview/_tiles.html.erb @@ -3,7 +3,7 @@
<%= tile.label %>
- <%= tile.value.is_a?(Numeric) ? number_with_delimiter(tile.value) : tile.value %><% + <%= tile.value %><% %><% if tile.unit %><%= tile.unit %><% end %>
From 0c7a9ff028858dae1bd21ff1db45997ba07a3290 Mon Sep 17 00:00:00 2001 From: Carl Mercier Date: Tue, 28 Jul 2026 10:22:00 -0500 Subject: [PATCH 3/4] refactor(views): give polling frames and toast responses one owner The refresh markup was hand-written in every index view and again in four near-identical turbo_stream templates, and the per-frame poll-interval expression was inlined ten times; the copies had already drifted. fd_refresh_frame now owns the frame markup, and one shared refresh.turbo_stream.erb template renders every mutating action's response from a { id:, url:, partial:, locals: } descriptor, replacing the four per-controller templates. Jobs::ActionsController now speaks through the Toasts concern instead of its own parallel success/failure/respond_with machinery, so the toast shape and the level -> flash mapping exist in exactly one place. The unused verb contract method goes with it. Co-Authored-By: Claude Opus 5 (1M context) --- app/controllers/concerns/flightdeck/toasts.rb | 36 +++++--- .../flightdeck/jobs/actions_controller.rb | 83 +++++++++---------- .../flightdeck/jobs/discards_controller.rb | 1 - .../flightdeck/jobs/retries_controller.rb | 1 - .../flightdeck/processes_controller.rb | 11 ++- .../flightdeck/queues_controller.rb | 11 ++- .../flightdeck/recurring_tasks_controller.rb | 9 +- app/helpers/flightdeck/application_helper.rb | 17 ++++ app/helpers/flightdeck/jobs_helper.rb | 6 -- .../jobs/actions/create.turbo_stream.erb | 18 ---- app/views/flightdeck/jobs/index.html.erb | 7 +- app/views/flightdeck/overview/index.html.erb | 49 +++-------- app/views/flightdeck/processes/index.html.erb | 8 +- .../processes/prune.turbo_stream.erb | 15 ---- app/views/flightdeck/queues/index.html.erb | 8 +- .../flightdeck/queues/update.turbo_stream.erb | 15 ---- .../flightdeck/recurring_tasks/index.html.erb | 8 +- .../recurring_tasks/run.turbo_stream.erb | 15 ---- .../shared/refresh.turbo_stream.erb | 14 ++++ 19 files changed, 138 insertions(+), 194 deletions(-) delete mode 100644 app/views/flightdeck/jobs/actions/create.turbo_stream.erb delete mode 100644 app/views/flightdeck/processes/prune.turbo_stream.erb delete mode 100644 app/views/flightdeck/queues/update.turbo_stream.erb delete mode 100644 app/views/flightdeck/recurring_tasks/run.turbo_stream.erb create mode 100644 app/views/flightdeck/shared/refresh.turbo_stream.erb diff --git a/app/controllers/concerns/flightdeck/toasts.rb b/app/controllers/concerns/flightdeck/toasts.rb index 0b14de5..29e9252 100644 --- a/app/controllers/concerns/flightdeck/toasts.rb +++ b/app/controllers/concerns/flightdeck/toasts.rb @@ -1,25 +1,35 @@ # frozen_string_literal: true module Flightdeck - # Shared response shape for the small member actions on the infrastructure - # pages: a toast plus a re-rendered frame over Turbo Streams, or a redirect - # carrying the same sentence in the flash when Turbo is not in play. + # Shared response shape for every mutating action: a toast plus a re-rendered + # polling frame over Turbo Streams, or a redirect carrying the same sentence + # in the flash when Turbo is not in play. + # + # The frame is described as data — `{ id:, url:, partial:, locals: }` — and + # rendered by the one shared template, so every controller's stream response + # is the same markup by construction. module Toasts - extend ActiveSupport::Concern - private - def toast(message, level: :success) - @toast = { message: message, level: level, continuable: false } + def toast(message, level: :success, continuable: false) + @toast = { message: message, level: level, continuable: continuable } end - def respond_with_toast(template, fallback:) + def respond_with_toast(frame, fallback:) respond_to do |format| - format.turbo_stream { render template, formats: :turbo_stream } - format.any do - flash[@toast[:level] == :error ? :alert : :notice] = @toast[:message] - redirect_to fallback - end + format.turbo_stream { render_refresh_stream(frame) } + format.any { redirect_with_toast_flash(fallback) } end end + + def render_refresh_stream(frame) + @frame = frame + render "flightdeck/shared/refresh", formats: :turbo_stream + end + + # The one place that knows how a toast level maps onto the flash. + def redirect_with_toast_flash(fallback) + flash[@toast[:level] == :error ? :alert : :notice] = @toast[:message] + redirect_to fallback + end end end diff --git a/app/controllers/flightdeck/jobs/actions_controller.rb b/app/controllers/flightdeck/jobs/actions_controller.rb index 5d25ce5..b1896ac 100644 --- a/app/controllers/flightdeck/jobs/actions_controller.rb +++ b/app/controllers/flightdeck/jobs/actions_controller.rb @@ -11,17 +11,24 @@ module Jobs # * selected jobs POST /jobs/retry with job_ids[] # * all matching POST /jobs/retry with scope=all + the list filters class ActionsController < Flightdeck::ApplicationController + include Toasts + def create - result = - if params[:id].present? - act_on_single - elsif params[:scope] == "all" - act_on_all_matching - else - act_on_selected + if params[:id].present? + act_on_single + elsif params[:scope] == "all" + act_on_all_matching + else + act_on_selected + end + + respond_to do |format| + format.turbo_stream do + build_refreshed_list + render_refresh_stream(refreshed_frame) end - - respond_with(result) + format.any { redirect_with_toast_flash(list_url) } + end end private @@ -37,7 +44,6 @@ def apply(execution) raise NotImplementedError end - def verb = raise(NotImplementedError) def past_tense = raise(NotImplementedError) # --- scopes ----------------------------------------------------------- @@ -48,12 +54,12 @@ def find_single(job_id) def act_on_single execution = find_single(params[:id]) - return failure("That job is no longer #{blocked_reason}.") unless execution + return toast("That job is no longer #{blocked_reason}.", level: :error) unless execution apply(execution) - success("#{past_tense.capitalize} job ##{params[:id]}.") + toast "#{past_tense.capitalize} job ##{params[:id]}." rescue SolidQueue::Execution::UndiscardableError => error - failure(error.message) + toast error.message, level: :error end # Selected ids are re-checked against the live relation rather than @@ -61,14 +67,15 @@ def act_on_single # may have retried, finished or discarded any of them. def act_on_selected ids = Array(params[:job_ids]).map { |id| Integer(id, exception: false) }.compact.uniq - return failure("Nothing selected.") if ids.empty? + return toast("Nothing selected.", level: :error) if ids.empty? if ids.size > Flightdeck.config.bulk_action_limit - return failure("Select at most #{number_with_delimiter(Flightdeck.config.bulk_action_limit)} jobs.") + return toast("Select at most #{number_with_delimiter(Flightdeck.config.bulk_action_limit)} jobs.", + level: :error) end executions = target_relation.where(job_id: ids).to_a - return failure("Those jobs are no longer #{blocked_reason}.") if executions.empty? + return toast("Those jobs are no longer #{blocked_reason}.", level: :error) if executions.empty? applied = 0 SolidQueue::Record.transaction do @@ -81,13 +88,13 @@ def act_on_selected missing = ids.size - applied message = "#{past_tense.capitalize} #{pluralize_jobs(applied)}." message += " #{missing} had already moved on." if missing.positive? - success(message) + toast message end def act_on_all_matching result = BulkAction.new(relation: target_relation).call { |execution| apply(execution) } - success(bulk_message(result), continuable: result.remaining?) + toast bulk_message(result), continuable: result.remaining? end def bulk_message(result) @@ -101,27 +108,6 @@ def bulk_message(result) # --- responses -------------------------------------------------------- - def success(message, continuable: false) - { message: message, level: :success, continuable: continuable } - end - - def failure(message) - { message: message, level: :error, continuable: false } - end - - def respond_with(result) - @toast = result - - respond_to do |format| - format.turbo_stream do - build_refreshed_list - render "flightdeck/jobs/actions/create", formats: :turbo_stream - end - format.html { redirect_back_to_list(result) } - format.any { redirect_back_to_list(result) } - end - end - # The list the action was launched from, re-queried so the same response # that carries the toast also carries settled rows and counts. def build_refreshed_list @@ -135,14 +121,27 @@ def build_refreshed_list end end + # `target: nil` because the jobs frame is the one frame whose links + # navigate in place (the pager); it matches the index view's frame. + def refreshed_frame + { id: "fd-jobs", target: nil, url: refreshed_list_url, + partial: "flightdeck/jobs/list", + locals: { query: @refreshed_query, rows: @refreshed_rows, groups: @refreshed_groups } } + end + + # URL the refreshed frame should poll, rebuilt from the filters the + # action carried rather than from the POST's own path. + def refreshed_list_url + jobs_path(list_filters.merge(state: @refreshed_state == :all ? nil : @refreshed_state).compact) + end + def refreshed_state state = params[:state].presence&.to_sym || default_state JobsQuery::STATES.include?(state) ? state : :all end - def redirect_back_to_list(result) - flash[result[:level] == :error ? :alert : :notice] = result[:message] - redirect_to jobs_path(list_filters.merge(state: params[:state].presence || default_state)) + def list_url + jobs_path(list_filters.merge(state: params[:state].presence || default_state)) end def default_state = :failed diff --git a/app/controllers/flightdeck/jobs/discards_controller.rb b/app/controllers/flightdeck/jobs/discards_controller.rb index c29036b..5f45b50 100644 --- a/app/controllers/flightdeck/jobs/discards_controller.rb +++ b/app/controllers/flightdeck/jobs/discards_controller.rb @@ -35,7 +35,6 @@ def apply(execution) execution.discard end - def verb = "discard" def past_tense = "discarded" def blocked_reason = "discardable" def default_state = discard_state diff --git a/app/controllers/flightdeck/jobs/retries_controller.rb b/app/controllers/flightdeck/jobs/retries_controller.rb index 7027fab..b6e94ea 100644 --- a/app/controllers/flightdeck/jobs/retries_controller.rb +++ b/app/controllers/flightdeck/jobs/retries_controller.rb @@ -15,7 +15,6 @@ def apply(execution) execution.retry end - def verb = "retry" def past_tense = "retried" def blocked_reason = "failed" end diff --git a/app/controllers/flightdeck/processes_controller.rb b/app/controllers/flightdeck/processes_controller.rb index 6de0330..6ceecda 100644 --- a/app/controllers/flightdeck/processes_controller.rb +++ b/app/controllers/flightdeck/processes_controller.rb @@ -19,7 +19,7 @@ def prune unless node.prunable? toast "#{node.kind} #{node.name} is still sending heartbeats — only unresponsive " \ "processes can be pruned.", level: :error - return respond_with_toast("flightdeck/processes/prune", fallback: processes_path) + return respond_with_toast(processes_frame, fallback: processes_path) end claimed = node.claimed_count @@ -27,7 +27,7 @@ def prune load_registry toast "Pruned #{node.kind} #{node.name}#{released(claimed)}." - respond_with_toast "flightdeck/processes/prune", fallback: processes_path + respond_with_toast processes_frame, fallback: processes_path end private @@ -35,6 +35,11 @@ def load_registry @registry = ProcessRegistry.new end + def processes_frame + { id: "fd-processes", url: processes_path, + partial: "flightdeck/processes/fleet", locals: { registry: @registry } } + end + def released(claimed) return "" if claimed.zero? @@ -43,7 +48,7 @@ def released(claimed) def missing toast "That process is no longer registered.", level: :error - respond_with_toast "flightdeck/processes/prune", fallback: processes_path + respond_with_toast processes_frame, fallback: processes_path end end end diff --git a/app/controllers/flightdeck/queues_controller.rb b/app/controllers/flightdeck/queues_controller.rb index 9bbcaad..7c1f197 100644 --- a/app/controllers/flightdeck/queues_controller.rb +++ b/app/controllers/flightdeck/queues_controller.rb @@ -16,14 +16,14 @@ def pause SolidQueue::Queue.new(@queue_name).pause reload_stats toast "Paused #{@queue_name}. Workers will stop picking up its jobs." - respond_with_toast "flightdeck/queues/update", fallback: queues_path + respond_with_toast queues_frame, fallback: queues_path end def resume SolidQueue::Queue.new(@queue_name).resume reload_stats toast "Resumed #{@queue_name}." - respond_with_toast "flightdeck/queues/update", fallback: queues_path + respond_with_toast queues_frame, fallback: queues_path end private @@ -36,6 +36,11 @@ def reload_stats Flightdeck::Cache.bypass { load_stats } end + def queues_frame + { id: "fd-queues", url: queues_path, + partial: "flightdeck/queues/cards", locals: { stats: @stats, sparklines: @sparklines } } + end + # Only queues Flightdeck is actually showing can be paused. That keeps an # arbitrary parameter from creating pause rows for queues that do not # exist. @@ -45,7 +50,7 @@ def require_known_queue return if @stats.find(@queue_name) toast "Unknown queue #{@queue_name.presence || "(blank)"}.", level: :error - respond_with_toast "flightdeck/queues/update", fallback: queues_path + respond_with_toast queues_frame, fallback: queues_path end end end diff --git a/app/controllers/flightdeck/recurring_tasks_controller.rb b/app/controllers/flightdeck/recurring_tasks_controller.rb index 7de4348..75f73cb 100644 --- a/app/controllers/flightdeck/recurring_tasks_controller.rb +++ b/app/controllers/flightdeck/recurring_tasks_controller.rb @@ -26,7 +26,7 @@ def run "class refused to enqueue.", level: :error end - respond_with_toast "flightdeck/recurring_tasks/run", fallback: recurring_tasks_path + respond_with_toast recurring_frame, fallback: recurring_tasks_path end private @@ -34,13 +34,18 @@ def load_catalog @catalog = RecurringCatalog.new end + def recurring_frame + { id: "fd-recurring", url: recurring_tasks_path, + partial: "flightdeck/recurring_tasks/table", locals: { catalog: @catalog } } + end + def job_reference @enqueued_job_id ? " as job ##{@enqueued_job_id}" : "" end def missing toast "That recurring task no longer exists.", level: :error - respond_with_toast "flightdeck/recurring_tasks/run", fallback: recurring_tasks_path + respond_with_toast recurring_frame, fallback: recurring_tasks_path end end end diff --git a/app/helpers/flightdeck/application_helper.rb b/app/helpers/flightdeck/application_helper.rb index 49de035..ea6c673 100644 --- a/app/helpers/flightdeck/application_helper.rb +++ b/app/helpers/flightdeck/application_helper.rb @@ -2,6 +2,23 @@ module Flightdeck module ApplicationHelper + # The polling every panel lives in: the refresh Stimulus + # controller, its interval, and the URL the frame re-requests. One owner for + # the markup, so the index views and the turbo-stream replacements can never + # drift apart. + # + # `target:` defaults to "_top" because most frames are lists whose links + # leave the frame; the jobs list opts out (`target: nil`) so its pager can + # navigate in place. + def fd_refresh_frame(id, url:, interval: fd_poll_ms, target: "_top", **options, &block) + tag.turbo_frame( + id: id, + target: target, + data: { controller: "refresh", refresh_interval_value: interval, refresh_url_value: url }, + **options, &block + ) + end + # Path to a built asset, resolved through the committed manifest. Returns # nil when assets have not been built yet (fresh checkout / dev), so the # layout can simply omit the tag instead of blowing up. diff --git a/app/helpers/flightdeck/jobs_helper.rb b/app/helpers/flightdeck/jobs_helper.rb index 4ceadf4..5be6c6d 100644 --- a/app/helpers/flightdeck/jobs_helper.rb +++ b/app/helpers/flightdeck/jobs_helper.rb @@ -114,12 +114,6 @@ def fd_error_cell(summary, grouped: false) text.to_s end - # URL the refreshed frame should poll after an action, rebuilt from the - # filters the action carried rather than from the POST's own path. - def fd_refreshed_list_url - jobs_path(list_filters.merge(state: @refreshed_state == :all ? nil : @refreshed_state).compact) - end - def fd_toast_level_class(level) level.to_sym == :error ? "fd-toast error" : "fd-toast" end diff --git a/app/views/flightdeck/jobs/actions/create.turbo_stream.erb b/app/views/flightdeck/jobs/actions/create.turbo_stream.erb deleted file mode 100644 index 3be9ebb..0000000 --- a/app/views/flightdeck/jobs/actions/create.turbo_stream.erb +++ /dev/null @@ -1,18 +0,0 @@ - - - - -<%# Re-render the list the action was launched from, so counts, rows and the - state tabs all settle in the same round trip. A page that has no jobs frame - (the job detail page, say) simply ignores this stream. %> - - - diff --git a/app/views/flightdeck/jobs/index.html.erb b/app/views/flightdeck/jobs/index.html.erb index 07173fe..7562b60 100644 --- a/app/views/flightdeck/jobs/index.html.erb +++ b/app/views/flightdeck/jobs/index.html.erb @@ -5,9 +5,6 @@ <%# The frame re-requests the URL it is showing, so state, filters and cursor all survive a poll. %> - +<%= fd_refresh_frame "fd-jobs", url: fd_current_list_url, target: nil do %> <%= render "flightdeck/jobs/list", query: @query, rows: @rows, groups: @groups %> - +<% end %> diff --git a/app/views/flightdeck/overview/index.html.erb b/app/views/flightdeck/overview/index.html.erb index b5daeb6..5e6c329 100644 --- a/app/views/flightdeck/overview/index.html.erb +++ b/app/views/flightdeck/overview/index.html.erb @@ -1,54 +1,29 @@ <% content_for :page_title, "Overview" %> - +<%= fd_refresh_frame "fd-tiles", url: root_path(range: @window.key) do %> <%= render "flightdeck/overview/tiles", overview: @overview %> - +<% end %>
- + <%= fd_refresh_frame "fd-throughput", url: root_path(range: @window.key), interval: fd_chart_poll_ms do %> <%= render "flightdeck/overview/throughput", series: @series, window: @window %> - + <% end %> - + <%= fd_refresh_frame "fd-completion", url: root_path(range: @window.key), interval: fd_chart_poll_ms do %> <%= render "flightdeck/overview/completion", series: @series, window: @window %> - + <% end %>
- + <%= fd_refresh_frame "fd-overview-queues", url: root_path(range: @window.key) do %> <%= render "flightdeck/overview/queues", overview: @overview %> - + <% end %> - + <%= fd_refresh_frame "fd-overview-failures", url: root_path(range: @window.key) do %> <%= render "flightdeck/overview/failures", overview: @overview %> - + <% end %>
- +<%= fd_refresh_frame "fd-fleet", url: root_path(range: @window.key), class: "fd-block" do %> <%= render "flightdeck/overview/fleet", registry: @overview.registry %> - +<% end %> diff --git a/app/views/flightdeck/processes/index.html.erb b/app/views/flightdeck/processes/index.html.erb index b71942b..385e023 100644 --- a/app/views/flightdeck/processes/index.html.erb +++ b/app/views/flightdeck/processes/index.html.erb @@ -1,9 +1,5 @@ <% content_for :page_title, "Processes" %> - +<%= fd_refresh_frame "fd-processes", url: processes_path do %> <%= render "flightdeck/processes/fleet", registry: @registry %> - +<% end %> diff --git a/app/views/flightdeck/processes/prune.turbo_stream.erb b/app/views/flightdeck/processes/prune.turbo_stream.erb deleted file mode 100644 index b8c4042..0000000 --- a/app/views/flightdeck/processes/prune.turbo_stream.erb +++ /dev/null @@ -1,15 +0,0 @@ - - - - - - - diff --git a/app/views/flightdeck/queues/index.html.erb b/app/views/flightdeck/queues/index.html.erb index 43d9c21..58b4670 100644 --- a/app/views/flightdeck/queues/index.html.erb +++ b/app/views/flightdeck/queues/index.html.erb @@ -1,9 +1,5 @@ <% content_for :page_title, "Queues" %> - +<%= fd_refresh_frame "fd-queues", url: queues_path do %> <%= render "flightdeck/queues/cards", stats: @stats, sparklines: @sparklines %> - +<% end %> diff --git a/app/views/flightdeck/queues/update.turbo_stream.erb b/app/views/flightdeck/queues/update.turbo_stream.erb deleted file mode 100644 index 129d472..0000000 --- a/app/views/flightdeck/queues/update.turbo_stream.erb +++ /dev/null @@ -1,15 +0,0 @@ - - - - - - - diff --git a/app/views/flightdeck/recurring_tasks/index.html.erb b/app/views/flightdeck/recurring_tasks/index.html.erb index e1b84ee..f2cd7bc 100644 --- a/app/views/flightdeck/recurring_tasks/index.html.erb +++ b/app/views/flightdeck/recurring_tasks/index.html.erb @@ -1,9 +1,5 @@ <% content_for :page_title, "Recurring tasks" %> - +<%= fd_refresh_frame "fd-recurring", url: recurring_tasks_path do %> <%= render "flightdeck/recurring_tasks/table", catalog: @catalog %> - +<% end %> diff --git a/app/views/flightdeck/recurring_tasks/run.turbo_stream.erb b/app/views/flightdeck/recurring_tasks/run.turbo_stream.erb deleted file mode 100644 index 4ec9fdd..0000000 --- a/app/views/flightdeck/recurring_tasks/run.turbo_stream.erb +++ /dev/null @@ -1,15 +0,0 @@ - - - - - - - diff --git a/app/views/flightdeck/shared/refresh.turbo_stream.erb b/app/views/flightdeck/shared/refresh.turbo_stream.erb new file mode 100644 index 0000000..d8d618d --- /dev/null +++ b/app/views/flightdeck/shared/refresh.turbo_stream.erb @@ -0,0 +1,14 @@ + + + + +<%# Re-render the frame the action was launched from, so the same round trip + that carries the toast also carries settled rows and counts. A page that + does not have the frame (the job detail page, say) simply ignores this. %> + + + From b6f19cff5dfb2ca05d8fa05a9853231448b878e5 Mon Sep 17 00:00:00 2001 From: Carl Mercier Date: Tue, 28 Jul 2026 10:22:10 -0500 Subject: [PATCH 4/4] chore: remove dead code Deletes methods nothing calls: fd_jobs_url, Assets.reload!, ProcessRegistry.all / #flattened, QueueStats.all and RecurringCatalog.all (tests now build the catalog directly). Co-Authored-By: Claude Opus 5 (1M context) --- app/helpers/flightdeck/jobs_helper.rb | 4 ---- app/models/flightdeck/process_registry.rb | 6 ------ app/models/flightdeck/queue_stats.rb | 2 -- app/models/flightdeck/recurring_catalog.rb | 2 -- lib/flightdeck/assets.rb | 6 ------ test/integration/infrastructure_test.rb | 2 +- test/recurring_catalog_test.rb | 18 +++++++++--------- 7 files changed, 10 insertions(+), 30 deletions(-) diff --git a/app/helpers/flightdeck/jobs_helper.rb b/app/helpers/flightdeck/jobs_helper.rb index 5be6c6d..d897916 100644 --- a/app/helpers/flightdeck/jobs_helper.rb +++ b/app/helpers/flightdeck/jobs_helper.rb @@ -93,10 +93,6 @@ def fd_args_preview(preview, length: 120) Flightdeck::ArgumentsPreview.format(preview, length: length) end - def fd_jobs_url(overrides = {}) - jobs_path(list_filters.merge(state: params[:state].presence).compact.merge(overrides)) - end - # The frame refreshes itself by re-requesting the URL it is showing, so # filters, state tab and cursor all survive a poll. def fd_current_list_url diff --git a/app/models/flightdeck/process_registry.rb b/app/models/flightdeck/process_registry.rb index e79dcf0..5f2b080 100644 --- a/app/models/flightdeck/process_registry.rb +++ b/app/models/flightdeck/process_registry.rb @@ -63,8 +63,6 @@ def config_summary end end - def self.all = new - def nodes @nodes ||= build_nodes end @@ -82,10 +80,6 @@ def tree end end - def flattened - tree.flat_map { |root| [ root, *root.children ] } - end - def dead = nodes.select(&:dead?) def any_dead? = dead.any? def dead_claimed_count = dead.sum(&:claimed_count) diff --git a/app/models/flightdeck/queue_stats.rb b/app/models/flightdeck/queue_stats.rb index 638adc3..2816c91 100644 --- a/app/models/flightdeck/queue_stats.rb +++ b/app/models/flightdeck/queue_stats.rb @@ -31,8 +31,6 @@ def rate_per_hour = completed_in_window def idle? = depth.zero? && completed_in_window.zero? end - def self.all = new.rows - def rows @rows ||= names.sort.map do |name| Row.new( diff --git a/app/models/flightdeck/recurring_catalog.rb b/app/models/flightdeck/recurring_catalog.rb index 188ddf2..1e0814d 100644 --- a/app/models/flightdeck/recurring_catalog.rb +++ b/app/models/flightdeck/recurring_catalog.rb @@ -41,8 +41,6 @@ def failed? = last_status == :failed def ever_run? = last_run_at.present? end - def self.all = new.rows - def rows @rows ||= tasks.map do |task| run = last_runs[task.key] diff --git a/lib/flightdeck/assets.rb b/lib/flightdeck/assets.rb index 874f453..78370a7 100644 --- a/lib/flightdeck/assets.rb +++ b/lib/flightdeck/assets.rb @@ -58,12 +58,6 @@ def content_type_for(file) CONTENT_TYPES.fetch(File.extname(file), "application/octet-stream") end - def reload! - remove_instance_variable(:@manifest) if defined?(@manifest) - remove_instance_variable(:@by_digested_name) if defined?(@by_digested_name) - manifest - end - private def load_manifest path = root.join("manifest.json") diff --git a/test/integration/infrastructure_test.rb b/test/integration/infrastructure_test.rb index 5e4f7b1..408ebcd 100644 --- a/test/integration/infrastructure_test.rb +++ b/test/integration/infrastructure_test.rb @@ -262,7 +262,7 @@ class Flightdeck::RecurringPageTest < FlightdeckIntegrationTest post_fd "/flightdeck/recurring_tasks/#{task.id}/run" assert_equal 1, SolidQueue::RecurringExecution.where(task_key: "sync").count - assert_equal :ok, Flightdeck::RecurringCatalog.all.first.last_status + assert_equal :ok, Flightdeck::RecurringCatalog.new.rows.first.last_status end test "the enqueued job is ready to be picked up" do diff --git a/test/recurring_catalog_test.rb b/test/recurring_catalog_test.rb index e111f6c..88423ea 100644 --- a/test/recurring_catalog_test.rb +++ b/test/recurring_catalog_test.rb @@ -7,7 +7,7 @@ class Flightdeck::RecurringCatalogTest < ActiveSupport::TestCase create_recurring_task(key: "sync", schedule: "*/5 * * * *") create_recurring_task(key: "digest", schedule: "0 23 * * *") - rows = Flightdeck::RecurringCatalog.all + rows = Flightdeck::RecurringCatalog.new.rows assert_equal %w[digest sync], rows.map(&:key) assert_equal "RecurringProbeJob", rows.first.job_class @@ -17,7 +17,7 @@ class Flightdeck::RecurringCatalogTest < ActiveSupport::TestCase test "next run comes from the task's own schedule API" do task = create_recurring_task(key: "hourly", schedule: "0 * * * *") - row = Flightdeck::RecurringCatalog.all.first + row = Flightdeck::RecurringCatalog.new.rows.first assert_equal task.next_time, row.next_run_at assert_operator row.next_run_in, :>, 0 @@ -26,7 +26,7 @@ class Flightdeck::RecurringCatalogTest < ActiveSupport::TestCase test "a task that has never run says so" do create_recurring_task(key: "fresh") - row = Flightdeck::RecurringCatalog.all.first + row = Flightdeck::RecurringCatalog.new.rows.first refute row.ever_run? assert_nil row.last_status @@ -36,7 +36,7 @@ class Flightdeck::RecurringCatalogTest < ActiveSupport::TestCase task = create_recurring_task(key: "sync") record_recurring_run(task, run_at: 10.minutes.ago) - row = Flightdeck::RecurringCatalog.all.first + row = Flightdeck::RecurringCatalog.new.rows.first assert row.ever_run? assert_equal :ok, row.last_status @@ -47,7 +47,7 @@ class Flightdeck::RecurringCatalogTest < ActiveSupport::TestCase task = create_recurring_task(key: "reconcile") record_recurring_run(task, run_at: 10.minutes.ago, failed: true) - row = Flightdeck::RecurringCatalog.all.first + row = Flightdeck::RecurringCatalog.new.rows.first assert_equal :failed, row.last_status assert row.failed? @@ -58,7 +58,7 @@ class Flightdeck::RecurringCatalogTest < ActiveSupport::TestCase record_recurring_run(task, run_at: 2.hours.ago, failed: true) record_recurring_run(task, run_at: 10.minutes.ago) - assert_equal :ok, Flightdeck::RecurringCatalog.all.first.last_status + assert_equal :ok, Flightdeck::RecurringCatalog.new.rows.first.last_status end test "deleting a job cascades its recurring execution away where FKs are enforced" do @@ -68,7 +68,7 @@ class Flightdeck::RecurringCatalogTest < ActiveSupport::TestCase job.delete assert_equal 0, SolidQueue::RecurringExecution.count - refute Flightdeck::RecurringCatalog.all.first.ever_run? + refute Flightdeck::RecurringCatalog.new.rows.first.ever_run? end test "statuses do not leak between tasks" do @@ -77,7 +77,7 @@ class Flightdeck::RecurringCatalogTest < ActiveSupport::TestCase record_recurring_run(ok, run_at: 5.minutes.ago) record_recurring_run(bad, run_at: 5.minutes.ago, failed: true) - by_key = Flightdeck::RecurringCatalog.all.index_by(&:key) + by_key = Flightdeck::RecurringCatalog.new.rows.index_by(&:key) assert_equal :ok, by_key["aaa_ok"].last_status assert_equal :failed, by_key["zzz_failed"].last_status @@ -105,7 +105,7 @@ class Flightdeck::RecurringCatalogPurgedJobTest < ActiveSupport::TestCase assert_nil SolidQueue::Job.find_by(id: job.id) assert_equal 1, SolidQueue::RecurringExecution.count, "the execution row should have survived" - row = Flightdeck::RecurringCatalog.all.first + row = Flightdeck::RecurringCatalog.new.rows.first assert row.ever_run? assert_equal :unknown, row.last_status