From a72dc2f420ee9f06ee82212ed17a1a0c1d202143 Mon Sep 17 00:00:00 2001 From: Ted Johansson Date: Fri, 23 Feb 2024 11:20:32 +0800 Subject: [PATCH] DEV: Introduce a problem checks API (#25783) Previously, problem checks were all added as either class methods or blocks in AdminDashboardData. Another set of class methods were used to add and run problem checks. As of this PR, problem checks are promoted to first-class citizens. Each problem check receives their own class. This class of course contains the implementation for running the check, but also configuration items like retry strategies (for scheduled checks.) In addition, the parent class ProblemCheck also serves as a registry for checks. For example we can get a list of all existing check classes through ProblemCheck.checks, or just the ones running on a schedule through ProblemCheck.scheduled. After this refactor, the task of adding a new check is significantly simplified. You add a class that inherits ProblemCheck, you implement it, add a test, and you're good to go. --- app/jobs/regular/problem_check.rb | 16 +- app/jobs/scheduled/problem_checks.rb | 9 +- app/models/admin_dashboard_data.rb | 66 +----- lib/group_email_credentials_check.rb | 65 ------ lib/problem_check.rb | 53 +++++ lib/problem_check/group_email_credentials.rb | 74 +++++++ lib/problem_check/problem.rb | 30 +++ spec/jobs/problem_check_spec.rb | 192 ++++++++++-------- spec/jobs/problem_checks_spec.rb | 42 ++-- .../group_email_credentials_spec.rb} | 44 ++-- spec/lib/problem_check_spec.rb | 40 ++++ spec/models/admin_dashboard_data_spec.rb | 43 ---- 12 files changed, 370 insertions(+), 304 deletions(-) delete mode 100644 lib/group_email_credentials_check.rb create mode 100644 lib/problem_check.rb create mode 100644 lib/problem_check/group_email_credentials.rb create mode 100644 lib/problem_check/problem.rb rename spec/lib/{group_email_credentials_check_spec.rb => problem_check/group_email_credentials_spec.rb} (58%) create mode 100644 spec/lib/problem_check_spec.rb diff --git a/app/jobs/regular/problem_check.rb b/app/jobs/regular/problem_check.rb index 715cab58541..f20e0cc5ad0 100644 --- a/app/jobs/regular/problem_check.rb +++ b/app/jobs/regular/problem_check.rb @@ -13,17 +13,23 @@ module Jobs retry_count = args[:retry_count].to_i identifier = args[:check_identifier].to_sym - check = AdminDashboardData.problem_scheduled_check_klasses[identifier] + check = ::ProblemCheck[identifier] - AdminDashboardData.execute_scheduled_check(identifier) do |problems| - raise RetrySignal if retry_count < check.max_retries - end + problems = check.call + raise RetrySignal if problems.present? && retry_count < check.max_retries + + problems.each { |problem| AdminDashboardData.add_found_scheduled_check_problem(problem) } rescue RetrySignal Jobs.enqueue_in( - check.retry_wait, + check.retry_after, :problem_check, args.merge(retry_count: retry_count + 1).stringify_keys, ) + rescue StandardError => err + Discourse.warn_exception( + err, + message: "A scheduled admin dashboard problem check (#{identifier}) errored.", + ) end end end diff --git a/app/jobs/scheduled/problem_checks.rb b/app/jobs/scheduled/problem_checks.rb index c6540b2c542..8950138faf5 100644 --- a/app/jobs/scheduled/problem_checks.rb +++ b/app/jobs/scheduled/problem_checks.rb @@ -2,8 +2,8 @@ module Jobs # This job runs all of the scheduled problem checks for the admin dashboard - # on a regular basis. To add a problem check for this scheduled job run - # call AdminDashboardData.add_scheduled_problem_check + # on a regular basis. To add a problem check, add a new class that inherits + # the `ProblemCheck` base class. class ProblemChecks < ::Jobs::Scheduled sidekiq_options retry: false @@ -13,7 +13,10 @@ module Jobs # This way if the problems have been solved in the meantime, then they will # not be re-added by the relevant checker, and will be cleared. AdminDashboardData.clear_found_scheduled_check_problems - AdminDashboardData.execute_scheduled_checks + + ::ProblemCheck.scheduled.each do |check| + Jobs.enqueue(:problem_check, check_identifier: check.identifier.to_s) + end end end end diff --git a/app/models/admin_dashboard_data.rb b/app/models/admin_dashboard_data.rb index 9bacaff5de4..9bec82b49ad 100644 --- a/app/models/admin_dashboard_data.rb +++ b/app/models/admin_dashboard_data.rb @@ -3,11 +3,7 @@ class AdminDashboardData include StatsCacheable - cattr_reader :problem_syms, - :problem_blocks, - :problem_messages, - :problem_scheduled_check_blocks, - :problem_scheduled_check_klasses + cattr_reader :problem_syms, :problem_blocks, :problem_messages class Problem VALID_PRIORITIES = %w[low high].freeze @@ -84,11 +80,6 @@ class AdminDashboardData @@problem_blocks << blk if blk end - def self.add_scheduled_problem_check(check_identifier, klass = nil, &blk) - @@problem_scheduled_check_klasses[check_identifier] = klass - @@problem_scheduled_check_blocks[check_identifier] = blk - end - def self.add_found_scheduled_check_problem(problem) problems = load_found_scheduled_check_problems if problem.identifier.present? @@ -129,57 +120,6 @@ class AdminDashboardData end end - def self.register_default_scheduled_problem_checks - add_scheduled_problem_check(:group_smtp_credentials, GroupEmailCredentialsCheck) do - problems = GroupEmailCredentialsCheck.run - problems.map do |p| - problem_message = - I18n.t( - "dashboard.group_email_credentials_warning", - { - base_path: Discourse.base_path, - group_name: p[:group_name], - group_full_name: p[:group_full_name], - error: p[:message], - }, - ) - Problem.new( - problem_message, - priority: "high", - identifier: "group_#{p[:group_id]}_email_credentials", - ) - end - end - end - - def self.execute_scheduled_checks - problem_scheduled_check_blocks.keys.each do |check_identifier| - Jobs.enqueue(:problem_check, check_identifier: check_identifier.to_s) - end - end - - def self.execute_scheduled_check(identifier) - check = problem_scheduled_check_blocks[identifier] - - problems = instance_exec(&check) - - yield(problems) if block_given? && problems.present? - - Array - .wrap(problems) - .compact - .each do |problem| - next if !problem.is_a?(Problem) - - add_found_scheduled_check_problem(problem) - end - rescue StandardError => err - Discourse.warn_exception( - err, - message: "A scheduled admin dashboard problem check (#{identifier}) errored.", - ) - end - ## # We call this method in the class definition below # so all of the problem checks in this class are registered on @@ -192,8 +132,6 @@ class AdminDashboardData def self.reset_problem_checks @@problem_syms = [] @@problem_blocks = [] - @@problem_scheduled_check_blocks = {} - @@problem_scheduled_check_klasses = {} @@problem_messages = %w[ dashboard.bad_favicon_url @@ -222,8 +160,6 @@ class AdminDashboardData :translation_overrides_check, :ember_version_check - register_default_scheduled_problem_checks - add_problem_check { sidekiq_check || queue_size_check } end reset_problem_checks diff --git a/lib/group_email_credentials_check.rb b/lib/group_email_credentials_check.rb deleted file mode 100644 index f9e9ced842a..00000000000 --- a/lib/group_email_credentials_check.rb +++ /dev/null @@ -1,65 +0,0 @@ -# frozen_string_literal: true - -## -# If group SMTP or IMAP has been configured, we want to make sure the -# credentials are always valid otherwise emails will not be sending out -# from group inboxes. This check is run as part of scheduled AdminDashboardData -# problem checks, and if any credentials have issues they will show up on -# the admin dashboard as a high priority issue. -class GroupEmailCredentialsCheck - def self.max_retries = 2 - def self.retry_wait = 30.seconds - - def self.run - errors = [] - - if SiteSetting.enable_smtp - Group.with_smtp_configured.find_each do |group| - errors << try_validate(group) do - EmailSettingsValidator.validate_smtp( - host: group.smtp_server, - port: group.smtp_port, - username: group.email_username, - password: group.email_password, - ) - end - end - end - - if SiteSetting.enable_imap - Group.with_imap_configured.find_each do |group| - errors << try_validate(group) do - EmailSettingsValidator.validate_imap( - host: group.smtp_server, - port: group.smtp_port, - username: group.email_username, - password: group.email_password, - ) - end - end - end - - errors.compact - end - - def self.try_validate(group, &blk) - begin - blk.call - nil - rescue *EmailSettingsExceptionHandler::EXPECTED_EXCEPTIONS => err - { - group_id: group.id, - group_name: group.name, - group_full_name: group.full_name, - message: EmailSettingsExceptionHandler.friendly_exception_message(err, group.smtp_server), - } - rescue => err - Discourse.warn_exception( - err, - message: - "Unexpected error when checking SMTP credentials for group #{group.id} (#{group.name}).", - ) - nil - end - end -end diff --git a/lib/problem_check.rb b/lib/problem_check.rb new file mode 100644 index 00000000000..8970240e375 --- /dev/null +++ b/lib/problem_check.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +class ProblemCheck + include ActiveSupport::Configurable + + # Determines if the check should be performed at a regular interval, and if + # so how often. If left blank, the check will be performed every time the + # admin dashboard is loaded, or the data is otherwise requested. + # + config_accessor :perform_every, default: nil, instance_writer: false + + # How many times the check should retry before registering a problem. Only + # works for scheduled checks. + # + config_accessor :max_retries, default: 2, instance_writer: false + + # The retry delay after a failed check. Only works for scheduled checks with + # more than one retry configured. + # + config_accessor :retry_after, default: 30.seconds, instance_writer: false + + def self.[](key) + key = key.to_sym + + checks.find { |c| c.identifier == key } + end + + def self.checks + descendants + end + + def self.scheduled + checks.select(&:scheduled?) + end + + def self.identifier + name.demodulize.underscore.to_sym + end + delegate :identifier, to: :class + + def self.scheduled? + perform_every.present? + end + delegate :scheduled?, to: :class + + def self.call + new.call + end + + def call + raise NotImplementedError + end +end diff --git a/lib/problem_check/group_email_credentials.rb b/lib/problem_check/group_email_credentials.rb new file mode 100644 index 00000000000..7a2c32612b0 --- /dev/null +++ b/lib/problem_check/group_email_credentials.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +## +# If group SMTP or IMAP has been configured, we want to make sure the +# credentials are always valid otherwise emails will not be sending out +# from group inboxes. This check is run as part of scheduled admin +# problem checks, and if any credentials have issues they will show up on +# the admin dashboard as a high priority issue. +class ProblemCheck::GroupEmailCredentials < ProblemCheck + self.perform_every = 30.minutes + + def call + [*smtp_errors, *imap_errors] + end + + private + + def smtp_errors + return [] if !SiteSetting.enable_smtp + + Group.with_smtp_configured.find_each.filter_map do |group| + try_validate(group) do + EmailSettingsValidator.validate_smtp( + host: group.smtp_server, + port: group.smtp_port, + username: group.email_username, + password: group.email_password, + ) + end + end + end + + def imap_errors + return [] if !SiteSetting.enable_imap + + Group.with_imap_configured.find_each.filter_map do |group| + try_validate(group) do + EmailSettingsValidator.validate_imap( + host: group.imap_server, + port: group.imap_port, + username: group.email_username, + password: group.email_password, + ) + end + end + end + + def try_validate(group, &blk) + begin + blk.call + nil + rescue *EmailSettingsExceptionHandler::EXPECTED_EXCEPTIONS => err + message = + I18n.t( + "dashboard.group_email_credentials_warning", + { + base_path: Discourse.base_path, + group_name: group.name, + group_full_name: group.full_name, + error: EmailSettingsExceptionHandler.friendly_exception_message(err, group.smtp_server), + }, + ) + + Problem.new(message, priority: "high", identifier: "group_#{group.id}_email_credentials") + rescue => err + Discourse.warn_exception( + err, + message: + "Unexpected error when checking SMTP credentials for group #{group.id} (#{group.name}).", + ) + nil + end + end +end diff --git a/lib/problem_check/problem.rb b/lib/problem_check/problem.rb new file mode 100644 index 00000000000..0fa52456b05 --- /dev/null +++ b/lib/problem_check/problem.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +class ProblemCheck::Problem + PRIORITIES = %w[low high].freeze + + attr_reader :message, :priority, :identifier + + def initialize(message, priority: "low", identifier: nil) + @message = message + @priority = PRIORITIES.include?(priority) ? priority : "low" + @identifier = identifier + end + + def to_s + @message + end + + def to_h + { message: message, priority: priority, identifier: identifier } + end + alias_method :attributes, :to_h + + def self.from_h(h) + h = h.with_indifferent_access + + return if h[:message].blank? + + new(h[:message], priority: h[:priority], identifier: h[:identifier]) + end +end diff --git a/spec/jobs/problem_check_spec.rb b/spec/jobs/problem_check_spec.rb index 80108ad593d..8f4d9d60b5f 100644 --- a/spec/jobs/problem_check_spec.rb +++ b/spec/jobs/problem_check_spec.rb @@ -3,98 +3,128 @@ RSpec.describe Jobs::ProblemCheck do after do Discourse.redis.flushdb - AdminDashboardData.reset_problem_checks + + ::ProblemCheck.send(:remove_const, "TestCheck") end - class TestCheck - def self.max_retries = 0 - def self.retry_wait = 30.seconds - end + context "when there are problems" do + before do + ::ProblemCheck::TestCheck = + Class.new(::ProblemCheck) do + self.max_retries = 0 - it "runs the scheduled problem check that has been added and adds the messages to the load_found_scheduled_check_problems array" do - AdminDashboardData.add_scheduled_problem_check(:test_identifier, TestCheck) do - AdminDashboardData::Problem.new("big problem") + def call + [ + ::ProblemCheck::Problem.new("Big problem"), + ::ProblemCheck::Problem.new( + "Yuge problem", + priority: "high", + identifier: "config_is_a_mess", + ), + ] + end + end end - described_class.new.execute(check_identifier: :test_identifier) - problems = AdminDashboardData.load_found_scheduled_check_problems - expect(problems.count).to eq(1) - expect(problems.first).to be_a(AdminDashboardData::Problem) - expect(problems.first.to_s).to eq("big problem") - end + it "adds the messages to the Redis problems array" do + described_class.new.execute(check_identifier: :test_check) - it "can handle the problem check returning multiple problems" do - AdminDashboardData.add_scheduled_problem_check(:test_identifier, TestCheck) do - [ - AdminDashboardData::Problem.new("big problem"), - AdminDashboardData::Problem.new( - "yuge problem", - priority: "high", - identifier: "config_is_a_mess", - ), - ] - end + problems = AdminDashboardData.load_found_scheduled_check_problems - described_class.new.execute(check_identifier: :test_identifier) - problems = AdminDashboardData.load_found_scheduled_check_problems - expect(problems.map(&:to_s)).to match_array(["big problem", "yuge problem"]) - end - - it "does not add the same problem twice if the identifier already exists" do - AdminDashboardData.add_scheduled_problem_check(:test_identifier, TestCheck) do - [ - AdminDashboardData::Problem.new( - "yuge problem", - priority: "high", - identifier: "config_is_a_mess", - ), - AdminDashboardData::Problem.new( - "nasty problem", - priority: "high", - identifier: "config_is_a_mess", - ), - ] - end - - described_class.new.execute(check_identifier: :test_identifier) - problems = AdminDashboardData.load_found_scheduled_check_problems - expect(problems.map(&:to_s)).to match_array(["yuge problem"]) - end - - it "schedules a retry if there are attempts remaining" do - AdminDashboardData.add_scheduled_problem_check(:test_identifier, TestCheck) do - AdminDashboardData::Problem.new("big problem") - end - - TestCheck.stubs(:max_retries).returns(1) - - expect_enqueued_with( - job: :problem_check, - args: { - check_identifier: :test_identifier, - retry_count: 1, - }, - ) { described_class.new.execute(check_identifier: :test_identifier) } - end - - it "does not schedule a retry if there are no more attempts remaining" do - AdminDashboardData.add_scheduled_problem_check(:test_identifier, TestCheck) do - AdminDashboardData::Problem.new("big problem") - end - - TestCheck.stubs(:max_retries).returns(1) - - expect_not_enqueued_with(job: :problem_check) do - described_class.new.execute(check_identifier: :test_identifier, retry_count: 1) + expect(problems.map(&:to_s)).to contain_exactly("Big problem", "Yuge problem") end end - it "handles errors from a troublesome check" do - AdminDashboardData.add_scheduled_problem_check(:test_identifier, TestCheck) do - raise StandardError.new("something went wrong") + context "with multiple problems with the same identifier" do + before do + ::ProblemCheck::TestCheck = + Class.new(::ProblemCheck) do + self.max_retries = 0 + + def call + [ + ::ProblemCheck::Problem.new( + "Yuge problem", + priority: "high", + identifier: "config_is_a_mess", + ), + ::ProblemCheck::Problem.new( + "Nasty problem", + priority: "high", + identifier: "config_is_a_mess", + ), + ] + end + end end - described_class.new.execute(check_identifier: :test_identifier) - expect(AdminDashboardData.load_found_scheduled_check_problems.count).to eq(0) + it "does not add the same problem twice" do + described_class.new.execute(check_identifier: :test_check) + + problems = AdminDashboardData.load_found_scheduled_check_problems + + expect(problems.map(&:to_s)).to match_array(["Yuge problem"]) + end + end + + context "when there are retries remaining" do + before do + ::ProblemCheck::TestCheck = + Class.new(::ProblemCheck) do + self.max_retries = 2 + + def call + [::ProblemCheck::Problem.new("Yuge problem")] + end + end + end + + it "schedules a retry" do + expect_enqueued_with( + job: :problem_check, + args: { + check_identifier: :test_check, + retry_count: 1, + }, + ) { described_class.new.execute(check_identifier: :test_check) } + end + end + + context "when there are no retries remaining" do + before do + ::ProblemCheck::TestCheck = + Class.new(::ProblemCheck) do + self.max_retries = 1 + + def call + [::ProblemCheck::Problem.new("Yuge problem")] + end + end + end + + it "does not schedule a retry" do + expect_not_enqueued_with(job: :problem_check) do + described_class.new.execute(check_identifier: :test_identifier, retry_count: 1) + end + end + end + + context "when the check unexpectedly errors out" do + before do + ::ProblemCheck::TestCheck = + Class.new(::ProblemCheck) do + self.max_retries = 1 + + def call + raise StandardError.new("Something went wrong") + end + end + end + + it "does not add a problem to the Redis array" do + described_class.new.execute(check_identifier: :test_check) + + expect(AdminDashboardData.load_found_scheduled_check_problems).to be_empty + end end end diff --git a/spec/jobs/problem_checks_spec.rb b/spec/jobs/problem_checks_spec.rb index 38b471dc5e2..2e1c30b6213 100644 --- a/spec/jobs/problem_checks_spec.rb +++ b/spec/jobs/problem_checks_spec.rb @@ -1,31 +1,37 @@ # frozen_string_literal: true RSpec.describe Jobs::ProblemChecks do - before { Jobs.run_immediately! } + before do + ::ProblemCheck::ScheduledCheck = + Class.new(ProblemCheck) do + self.perform_every = 30.minutes + + def call = [] + end + + ::ProblemCheck::NonScheduledCheck = Class.new(ProblemCheck) { def call = [] } + end after do Discourse.redis.flushdb AdminDashboardData.reset_problem_checks + + ProblemCheck.send(:remove_const, "ScheduledCheck") + ProblemCheck.send(:remove_const, "NonScheduledCheck") end - class TestCheck - def self.max_retries = 0 - def self.retry_wait = 0.seconds - end - - it "starts with a blank slate every time the checks are run to avoid duplicate problems and to clear no longer firing problems" do - problem_should_fire = true - AdminDashboardData.reset_problem_checks - AdminDashboardData.add_scheduled_problem_check(:test_identifier, TestCheck) do - if problem_should_fire - problem_should_fire = false - AdminDashboardData::Problem.new("yuge problem", priority: "high") - end + it "schedules the individual scheduled checks" do + expect_enqueued_with(job: :problem_check, args: { check_identifier: "scheduled_check" }) do + described_class.new.execute([]) end + end - described_class.new.execute(nil) - expect(AdminDashboardData.load_found_scheduled_check_problems.count).to eq(1) - described_class.new.execute(nil) - expect(AdminDashboardData.load_found_scheduled_check_problems.count).to eq(0) + it "does not schedule non-scheduled checks" do + expect_not_enqueued_with( + job: :problem_check, + args: { + check_identifier: "non_scheduled_check", + }, + ) { described_class.new.execute([]) } end end diff --git a/spec/lib/group_email_credentials_check_spec.rb b/spec/lib/problem_check/group_email_credentials_spec.rb similarity index 58% rename from spec/lib/group_email_credentials_check_spec.rb rename to spec/lib/problem_check/group_email_credentials_spec.rb index 603c1a44df3..3d3a74d9cfa 100644 --- a/spec/lib/group_email_credentials_check_spec.rb +++ b/spec/lib/problem_check/group_email_credentials_spec.rb @@ -3,16 +3,16 @@ require "net/smtp" require "net/imap" -RSpec.describe GroupEmailCredentialsCheck do +RSpec.describe ProblemCheck::GroupEmailCredentials do fab!(:group1) { Fabricate(:group) } fab!(:group2) { Fabricate(:smtp_group) } fab!(:group3) { Fabricate(:imap_group) } - describe "#run" do + describe "#call" do it "does nothing if SMTP is disabled for the site" do expect_no_validate_any SiteSetting.enable_smtp = false - expect(described_class.run).to eq([]) + expect(described_class.new.call).to eq([]) end context "with smtp and imap enabled for the site" do @@ -25,10 +25,10 @@ RSpec.describe GroupEmailCredentialsCheck do expect_no_validate_any group2.update!(smtp_enabled: false) group3.update!(smtp_enabled: false, imap_enabled: false) - expect(described_class.run).to eq([]) + expect(described_class.new.call).to eq([]) end - it "returns an error message and the group ID if the group's SMTP settings error" do + it "returns a problem with the group's SMTP settings error" do EmailSettingsValidator .expects(:validate_smtp) .raises(Net::SMTPAuthenticationError.new("bad credentials")) @@ -37,15 +37,13 @@ RSpec.describe GroupEmailCredentialsCheck do .at_least_once EmailSettingsValidator.stubs(:validate_imap).returns(true) - expect(described_class.run).to eq( - [ - { - group_full_name: group2.full_name, - group_name: group2.name, - group_id: group2.id, - message: I18n.t("email_settings.smtp_authentication_error"), - }, - ], + expect(described_class.new.call).to contain_exactly( + have_attributes( + identifier: "group_#{group2.id}_email_credentials", + priority: "high", + message: + "There was an issue with the email credentials for the group . No emails will be sent from the group inbox until this problem is addressed. There was an issue with the SMTP credentials provided, check the username and password and try again.", + ), ) end @@ -56,15 +54,13 @@ RSpec.describe GroupEmailCredentialsCheck do .raises(Net::IMAP::NoResponseError.new(stub(data: stub(text: "Invalid credentials")))) .once - expect(described_class.run).to eq( - [ - { - group_full_name: group3.full_name, - group_name: group3.name, - group_id: group3.id, - message: I18n.t("email_settings.imap_authentication_error"), - }, - ], + expect(described_class.new.call).to contain_exactly( + have_attributes( + identifier: "group_#{group3.id}_email_credentials", + priority: "high", + message: + "There was an issue with the email credentials for the group . No emails will be sent from the group inbox until this problem is addressed. There was an issue with the IMAP credentials provided, check the username and password and try again.", + ), ) end @@ -73,7 +69,7 @@ RSpec.describe GroupEmailCredentialsCheck do EmailSettingsValidator.stubs(:validate_smtp).returns(true) EmailSettingsValidator.expects(:validate_imap).never - expect(described_class.run).to eq([]) + expect(described_class.new.call).to eq([]) end end end diff --git a/spec/lib/problem_check_spec.rb b/spec/lib/problem_check_spec.rb new file mode 100644 index 00000000000..9df52fb3a62 --- /dev/null +++ b/spec/lib/problem_check_spec.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +RSpec.describe ProblemCheck do + # rubocop:disable RSpec/BeforeAfterAll + before(:all) do + ScheduledCheck = Class.new(described_class) { self.perform_every = 30.minutes } + UnscheduledCheck = Class.new(described_class) + end + + after(:all) do + Object.send(:remove_const, ScheduledCheck.name) + Object.send(:remove_const, UnscheduledCheck.name) + end + # rubocop:enable RSpec/BeforeAfterAll + + let(:scheduled_check) { ScheduledCheck } + let(:unscheduled_check) { UnscheduledCheck } + + describe ".[]" do + it { expect(described_class[:scheduled_check]).to eq(scheduled_check) } + it { expect(described_class[:foo]).to eq(nil) } + end + + describe ".identifier" do + it { expect(scheduled_check.identifier).to eq(:scheduled_check) } + end + + describe ".checks" do + it { expect(described_class.checks).to contain_exactly(scheduled_check, unscheduled_check) } + end + + describe ".scheduled" do + it { expect(described_class.scheduled).to contain_exactly(scheduled_check) } + end + + describe ".scheduled?" do + it { expect(scheduled_check).to be_scheduled } + it { expect(unscheduled_check).to_not be_scheduled } + end +end diff --git a/spec/models/admin_dashboard_data_spec.rb b/spec/models/admin_dashboard_data_spec.rb index be87adfa960..fd5d675a9ea 100644 --- a/spec/models/admin_dashboard_data_spec.rb +++ b/spec/models/admin_dashboard_data_spec.rb @@ -82,49 +82,6 @@ RSpec.describe AdminDashboardData do include_examples "stats cacheable" end - describe ".execute_scheduled_checks" do - let(:blk) { -> {} } - - before { AdminDashboardData.add_scheduled_problem_check(:foo, &blk) } - after { AdminDashboardData.reset_problem_checks } - - it do - expect_enqueued_with(job: :problem_check, args: { check_identifier: :foo }) do - AdminDashboardData.execute_scheduled_checks - end - end - end - - describe ".execute_scheduled_check" do - context "when problems are found" do - let(:blk) { -> { self::Problem.new("Problem") } } - - before do - AdminDashboardData.add_scheduled_problem_check(:foo, &blk) - AdminDashboardData.expects(:add_found_scheduled_check_problem).once - end - - after { AdminDashboardData.reset_problem_checks } - - it do - expect(described_class.execute_scheduled_check(:foo)).to all(be_a(described_class::Problem)) - end - end - - context "when check errors out" do - let(:blk) { -> { raise StandardError } } - - before do - AdminDashboardData.add_scheduled_problem_check(:foo, &blk) - Discourse.expects(:warn_exception).once - end - - after { AdminDashboardData.reset_problem_checks } - - it { expect(described_class.execute_scheduled_check(:foo)).to eq(nil) } - end - end - describe "#problem_message_check" do let(:key) { AdminDashboardData.problem_messages.first }