diff --git a/app/jobs/scheduled/run_problem_checks.rb b/app/jobs/scheduled/run_problem_checks.rb index eb539e33704..553e647d2cc 100644 --- a/app/jobs/scheduled/run_problem_checks.rb +++ b/app/jobs/scheduled/run_problem_checks.rb @@ -12,12 +12,19 @@ module Jobs def execute(_args) scheduled_checks = ProblemCheckTracker.all.filter_map do |tracker| - tracker.check if tracker.check.scheduled? && tracker.ready_to_run? + tracker.check if eligible_for_this_run?(tracker) end scheduled_checks.each do |check| Jobs.enqueue(:run_problem_check, check_identifier: check.identifier.to_s) end end + + private + + def eligible_for_this_run?(tracker) + tracker.check.present? && tracker.check.enabled? && tracker.check.scheduled? && + tracker.ready_to_run? + end end end diff --git a/app/models/problem_check_tracker.rb b/app/models/problem_check_tracker.rb index 38ffb05bfdc..b9ef0054167 100644 --- a/app/models/problem_check_tracker.rb +++ b/app/models/problem_check_tracker.rb @@ -43,7 +43,14 @@ class ProblemCheckTracker < ActiveRecord::Base end def check - ProblemCheck[identifier] + check = ProblemCheck[identifier] + + return check if check.present? + + silence_the_alarm + destroy + + nil end private diff --git a/spec/jobs/run_problem_checks_spec.rb b/spec/jobs/run_problem_checks_spec.rb index 419fd84776e..bd27d3b8bd3 100644 --- a/spec/jobs/run_problem_checks_spec.rb +++ b/spec/jobs/run_problem_checks_spec.rb @@ -11,18 +11,24 @@ RSpec.describe Jobs::RunProblemChecks do ProblemCheck::NonScheduledCheck = Class.new(ProblemCheck) { def call = [] } + ProblemCheck::DisabledCheck = + Class.new(ProblemCheck) do + self.perform_every = 30.minutes + self.enabled = false + + def call = [] + end + stub_const( ProblemCheck, "CORE_PROBLEM_CHECKS", - [ProblemCheck::ScheduledCheck, ProblemCheck::NonScheduledCheck], + [ProblemCheck::ScheduledCheck, ProblemCheck::NonScheduledCheck, ProblemCheck::DisabledCheck], &example ) - Discourse.redis.flushdb - AdminDashboardData.reset_problem_checks - ProblemCheck.send(:remove_const, "ScheduledCheck") ProblemCheck.send(:remove_const, "NonScheduledCheck") + ProblemCheck.send(:remove_const, "DisabledCheck") end context "when the tracker determines the check is ready to run" do @@ -68,12 +74,29 @@ RSpec.describe Jobs::RunProblemChecks do end end - it "does not schedule non-scheduled checks" do - expect_not_enqueued_with( - job: :run_problem_check, - args: { - check_identifier: "non_scheduled_check", - }, - ) { described_class.new.execute([]) } + context "when dealing with a disabled check" do + before { ProblemCheckTracker.create!(identifier: "disabled_check", next_run_at: nil) } + + it "does not schedule any check" do + expect_not_enqueued_with( + job: :run_problem_check, + args: { + check_identifier: "disabled_check", + }, + ) { described_class.new.execute([]) } + end + end + + context "when dealing with an uninstalled check" do + before { ProblemCheckTracker.create!(identifier: "uninstalled_check", next_run_at: nil) } + + it "does not schedule any check" do + expect_not_enqueued_with( + job: :run_problem_check, + args: { + check_identifier: "uninstalled_check", + }, + ) { described_class.new.execute([]) } + end end end diff --git a/spec/models/problem_check_tracker_spec.rb b/spec/models/problem_check_tracker_spec.rb index b0d93a20786..9b07d47ecb4 100644 --- a/spec/models/problem_check_tracker_spec.rb +++ b/spec/models/problem_check_tracker_spec.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true RSpec.describe ProblemCheckTracker do - before { described_class.any_instance.stubs(:check).returns(stub(max_blips: 1, priority: "low")) } - describe "validations" do let(:record) { described_class.new(identifier: "twitter_login") } @@ -24,6 +22,23 @@ RSpec.describe ProblemCheckTracker do end end + describe "#check" do + before do + Fabricate(:problem_check_tracker, identifier: "twitter_login") + Fabricate(:problem_check_tracker, identifier: "missing_check") + end + + context "when the tracker has a corresponding check" do + it { expect(described_class[:twitter_login].check.new).to be_a(ProblemCheck) } + end + + context "when the checking logic of the tracker has been removed or renamed" do + it do + expect { described_class[:missing_check].check }.to change { described_class.count }.by(-1) + end + end + end + describe "#ready_to_run?" do let(:problem_tracker) { described_class.new(next_run_at:) } @@ -169,7 +184,10 @@ RSpec.describe ProblemCheckTracker do context "when there are still blips to go" do let(:blips) { 0 } + before { ProblemCheck::TwitterLogin.stubs(:max_blips).returns(1) } + it "does not sound the alarm" do + puts ProblemCheck::TwitterLogin.max_blips expect { problem_tracker.problem!(next_run_at: 24.hours.from_now) }.not_to change { AdminNotice.problem.count }