diff --git a/README.md b/README.md index 109d32b7..47beb1f1 100644 --- a/README.md +++ b/README.md @@ -1353,6 +1353,16 @@ enabling this feature. See [upgrading](#upgrading) for more information. MaintenanceTasks.serialize_cursors_as_json = true ``` +#### Configure staleness threshold + +To specify a staleness threshold date interval which will mark task runs as stale, you can +configure `MaintenanceTasks.task_staleness_threshold`. Tasks that have last run +successfully after this threshold will be marked stale. + +The value for `MaintenanceTasks.task_staleness_threshold` must be an +`ActiveSupport::Duration`. If no value is specified, it will default to 30 days. This can be disabled +by setting `MaintenanceTasks.task_staleness_threshold` to `false`. + ## Upgrading Use bundler to check for and upgrade to newer versions. After installing a new diff --git a/app/helpers/maintenance_tasks/tasks_helper.rb b/app/helpers/maintenance_tasks/tasks_helper.rb index 6f1eff21..7ed77c22 100644 --- a/app/helpers/maintenance_tasks/tasks_helper.rb +++ b/app/helpers/maintenance_tasks/tasks_helper.rb @@ -62,7 +62,7 @@ def progress(run) def status_tag(status) tag.span( status.capitalize, - class: ["tag", "has-text-weight-medium", "pr-2", "mr-4"] + STATUS_COLOURS.fetch(status), + class: ["tag", "has-text-weight-medium", "px-2", "mx-4"] + STATUS_COLOURS.fetch(status), ) end diff --git a/app/models/concerns/maintenance_tasks/run_concern.rb b/app/models/concerns/maintenance_tasks/run_concern.rb index 132c9aa1..b936c6dc 100644 --- a/app/models/concerns/maintenance_tasks/run_concern.rb +++ b/app/models/concerns/maintenance_tasks/run_concern.rb @@ -471,6 +471,17 @@ def configure_cursor_encoding! self.cursor_is_json = true end + # Returns whether the run is stale based on the staleness threshold. + # + # @return [Boolean] + def stale? + return false unless MaintenanceTasks.task_staleness_threshold.present? + return false unless succeeded? + return false unless ended_at.present? + + ended_at < MaintenanceTasks.task_staleness_threshold.ago + end + private def instrument_status_change diff --git a/app/models/maintenance_tasks/task_data_index.rb b/app/models/maintenance_tasks/task_data_index.rb index 51970103..373ae86b 100644 --- a/app/models/maintenance_tasks/task_data_index.rb +++ b/app/models/maintenance_tasks/task_data_index.rb @@ -64,6 +64,15 @@ def initialize(name, related_run = nil) alias_method :to_s, :name + # Delegates to the related run's stale? method when available. + # + # @return [Boolean] whether the related run is stale. + def stale? + return false unless related_run.present? + + related_run.stale? + end + # Returns the status of the latest active or completed Run, if present. # If the Task does not have any Runs, the Task status is `new`. # diff --git a/app/views/maintenance_tasks/tasks/_task.html.erb b/app/views/maintenance_tasks/tasks/_task.html.erb index dfd193ea..d04c4b0f 100644 --- a/app/views/maintenance_tasks/tasks/_task.html.erb +++ b/app/views/maintenance_tasks/tasks/_task.html.erb @@ -1,10 +1,22 @@
-

+

<%= link_to task, task_path(task) %> <%= status_tag(task.status) %>

<% if (run = task.related_run) %> + <% if task.stale? %> +
+ + + + +

+ This task last ran <%= MaintenanceTasks.task_staleness_threshold.inspect %> ago. Consider removing it as it may be stale. +

+
+ <% end %> +
<%= time_tag run.created_at, title: run.created_at.utc %>
diff --git a/lib/maintenance_tasks.rb b/lib/maintenance_tasks.rb index 059315d4..71b019d8 100644 --- a/lib/maintenance_tasks.rb +++ b/lib/maintenance_tasks.rb @@ -137,6 +137,14 @@ module MaintenanceTasks # @return [Boolean] whether or not to store cursor values as JSON. mattr_accessor :serialize_cursors_as_json, default: false + # @!attribute task_staleness_threshold + # @scope class + # The threshold after which a task is considered stale. + # Defaults to 30 days. Can be disabled by setting this to `false`. + # + # @return [ActiveSupport::Duration, false] time interval after which a task is considered stale. + mattr_accessor :task_staleness_threshold, default: 30.days + class << self DEPRECATION_MESSAGE = "MaintenanceTasks.error_handler is deprecated and will be removed in the 3.0 release. " \ "Instead, reports will be sent to the Rails error reporter. Do not set a handler and subscribe " \ diff --git a/test/dummy/app/tasks/maintenance/stale_task.rb b/test/dummy/app/tasks/maintenance/stale_task.rb new file mode 100644 index 00000000..0a640e76 --- /dev/null +++ b/test/dummy/app/tasks/maintenance/stale_task.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module Maintenance + class StaleTask < MaintenanceTasks::Task + def collection + [1, 2] + end + + def process(number) + Rails.logger.debug("This task is stale") + end + end +end diff --git a/test/fixtures/maintenance_tasks/runs.yml b/test/fixtures/maintenance_tasks/runs.yml index 97095abb..9e11b45d 100644 --- a/test/fixtures/maintenance_tasks/runs.yml +++ b/test/fixtures/maintenance_tasks/runs.yml @@ -69,8 +69,13 @@ import_posts_task_succeeded_old: task_name: Maintenance::ImportPostsTask tick_count: 10 tick_total: 10 + +stale_task: + task_name: Maintenance::StaleTask + tick_count: 10 + tick_total: 10 job_id: '123abc' status: 'succeeded' - created_at: '01 Jan 2020 00:20:00' - started_at: '01 Jan 2020 00:40:36' - ended_at: '01 Jan 2020 00:52:19' + created_at: '03 Mar 2026 00:20:00' + started_at: '03 Mar 2026 00:40:36' + ended_at: '03 Mar 2026 00:52:19' diff --git a/test/helpers/maintenance_tasks/tasks_helper_test.rb b/test/helpers/maintenance_tasks/tasks_helper_test.rb index 9244cd0d..d06263b2 100644 --- a/test/helpers/maintenance_tasks/tasks_helper_test.rb +++ b/test/helpers/maintenance_tasks/tasks_helper_test.rb @@ -50,7 +50,7 @@ class TasksHelperTest < ActionView::TestCase end test "#status_tag renders a span with the appropriate tag based on status" do - expected = 'Pausing' + expected = 'Pausing' assert_equal expected, status_tag("pausing") end diff --git a/test/models/maintenance_tasks/run_test.rb b/test/models/maintenance_tasks/run_test.rb index 6a28330e..02a6fd88 100644 --- a/test/models/maintenance_tasks/run_test.rb +++ b/test/models/maintenance_tasks/run_test.rb @@ -773,6 +773,50 @@ class RunTest < ActiveSupport::TestCase end end + test "#stale? returns `false` when the run is not succeeded" do + MaintenanceTasks.with(task_staleness_threshold: 1.day) do + run = Run.create!(task_name: "Maintenance::UpdatePostsTask", ended_at: 2.days.ago, status: :running) + refute run.stale? + end + end + + test "#stale? returns `false` when the staleness threshold is disabled" do + MaintenanceTasks.with(task_staleness_threshold: false) do + run = Run.create!(task_name: "Maintenance::UpdatePostsTask", ended_at: 2.days.ago, status: :succeeded) + refute run.stale? + end + end + + test "#stale? returns `true` when the run is succeeded and beyond the default staleness threshold" do + run = Run.create!( + task_name: "Maintenance::UpdatePostsTask", + ended_at: (MaintenanceTasks.task_staleness_threshold + 1.day).ago, + status: :succeeded, + ) + assert run.stale? + end + + test "#stale? returns `true` when the run is succeeded and beyond the staleness threshold" do + MaintenanceTasks.with(task_staleness_threshold: 1.day) do + run = Run.create!(task_name: "Maintenance::UpdatePostsTask", ended_at: 2.days.ago, status: :succeeded) + assert run.stale? + end + end + + test "#stale? returns `false` when the run is succeeded and within the staleness threshold" do + MaintenanceTasks.with(task_staleness_threshold: 2.day) do + run = Run.create!(task_name: "Maintenance::UpdatePostsTask", ended_at: 1.day.ago, status: :succeeded) + refute run.stale? + end + end + + test "#stale? returns `false` when the run is nil" do + MaintenanceTasks.with(task_staleness_threshold: 1.day) do + run = Run.new + refute run.stale? + end + end + private def expected_notification(run) diff --git a/test/models/maintenance_tasks/task_data_index_test.rb b/test/models/maintenance_tasks/task_data_index_test.rb index a960155b..07384765 100644 --- a/test/models/maintenance_tasks/task_data_index_test.rb +++ b/test/models/maintenance_tasks/task_data_index_test.rb @@ -22,6 +22,7 @@ class TaskDataIndexTest < ActiveSupport::TestCase # duplicate due to fixtures containing two active runs of this task "Maintenance::NoCollectionTask", "Maintenance::ParamsTask", + "Maintenance::StaleTask", "Maintenance::TestTask", "Maintenance::UpdatePostsInBatchesTask", "Maintenance::UpdatePostsModulePrependedTask", @@ -51,6 +52,25 @@ class TaskDataIndexTest < ActiveSupport::TestCase assert_equal "Maintenance::UpdatePostsTask", task_data.to_s end + test "#stale? returns `true` for tasks outside of the staleness threshold for the related_run" do + MaintenanceTasks.with(task_staleness_threshold: 1.day) do + run = Run.create!( + task_name: "Maintenance::UpdatePostsTask", + ended_at: 2.days.ago, + status: :succeeded, + ) + task_data = TaskDataIndex.new("Maintenance::UpdatePostsTask", run) + assert task_data.stale? + end + end + + test "#stale? returns `false` for tasks with no related run" do + MaintenanceTasks.with(task_staleness_threshold: 1.day) do + task_data = TaskDataIndex.new("Maintenance::UpdatePostsTask", nil) + refute task_data.stale? + end + end + test "#status is new when Task does not have any Runs" do task_data = TaskDataIndex.new("Maintenance::UpdatePostsTask") assert_equal "new", task_data.status diff --git a/test/models/maintenance_tasks/task_test.rb b/test/models/maintenance_tasks/task_test.rb index 572e69d5..776a6e80 100644 --- a/test/models/maintenance_tasks/task_test.rb +++ b/test/models/maintenance_tasks/task_test.rb @@ -20,6 +20,7 @@ class TaskTest < ActiveSupport::TestCase "Maintenance::Nested::NestedTask", "Maintenance::NoCollectionTask", "Maintenance::ParamsTask", + "Maintenance::StaleTask", "Maintenance::TestTask", "Maintenance::UpdatePostsInBatchesTask", "Maintenance::UpdatePostsModulePrependedTask", diff --git a/test/system/maintenance_tasks/tasks_test.rb b/test/system/maintenance_tasks/tasks_test.rb index 5e973d87..e1cee68a 100644 --- a/test/system/maintenance_tasks/tasks_test.rb +++ b/test/system/maintenance_tasks/tasks_test.rb @@ -40,6 +40,7 @@ class TasksTest < ApplicationSystemTestCase "Maintenance::UpdatePostsThrottledTask New", "Completed Tasks", "Maintenance::ImportPostsTask Succeeded", + "Maintenance::StaleTask Succeeded", ] assert_equal expected, page.all("h3").map(&:text) @@ -71,6 +72,18 @@ class TasksTest < ApplicationSystemTestCase assert_text(/January 01, 2020 01:00 Succeeded #\d/) end + test "show a Task with stale run" do + travel_to(maintenance_tasks_runs(:stale_task).ended_at + 2.days) do + MaintenanceTasks.with(task_staleness_threshold: 1.day) do + visit maintenance_tasks_path + + within page.find("a", text: "Maintenance::StaleTask").find(:xpath, "..").sibling(".has-text-warning") do |element| + assert_text "This task last ran 1 day ago. Consider removing it as it may be stale." + end + end + end + end + test "task with attributes renders default values on the form" do visit maintenance_tasks_path