From 3137e606539e1209be07c386c8b76a45119e2ea7 Mon Sep 17 00:00:00 2001 From: Ted Johansson Date: Thu, 23 May 2024 09:29:08 +0800 Subject: [PATCH] DEV: Database backed admin notices (#26192) This PR introduces a basic AdminNotice model to store these notices. Admin notices are categorized by their source/type (currently only notices from problem check.) They also have a priority. --- app/controllers/admin/dashboard_controller.rb | 4 +- app/controllers/static_controller.rb | 2 +- app/jobs/regular/run_problem_check.rb | 10 +- app/jobs/scheduled/poll_mailbox.rb | 11 +- app/models/admin_dashboard_data.rb | 9 +- app/models/admin_notice.rb | 33 +++++ app/models/problem_check.rb | 69 +++++++++- app/models/problem_check_tracker.rb | 50 ++++++- app/serializers/admin_notice_serializer.rb | 5 + app/services/problem_check/bad_favicon_url.rb | 5 + .../email_polling_errored_recently.rb | 4 - app/services/problem_check/facebook_config.rb | 4 - app/services/problem_check/failing_emails.rb | 4 - app/services/problem_check/force_https.rb | 6 - app/services/problem_check/github_config.rb | 4 - .../problem_check/google_analytics_version.rb | 6 - .../problem_check/google_oauth2_config.rb | 4 - .../problem_check/group_email_credentials.rb | 19 ++- app/services/problem_check/host_names.rb | 6 - app/services/problem_check/image_magick.rb | 6 - .../problem_check/inline_problem_check.rb | 10 ++ .../problem_check/maxmind_db_configuration.rb | 6 - .../problem_check/missing_mailgun_api_key.rb | 6 - .../problem_check/out_of_date_themes.rb | 2 +- .../problem_check/poll_pop3_auth_error.rb | 5 + .../problem_check/poll_pop3_timeout.rb | 5 + app/services/problem_check/problem.rb | 6 +- app/services/problem_check/rails_env.rb | 4 - app/services/problem_check/ram.rb | 6 - .../problem_check/s3_backup_config.rb | 4 - app/services/problem_check/s3_cdn.rb | 6 - .../problem_check/s3_upload_config.rb | 4 - app/services/problem_check/sidekiq_check.rb | 16 ++- .../problem_check/subfolder_ends_in_slash.rb | 6 - .../problem_check/translation_overrides.rb | 6 - app/services/problem_check/twitter_config.rb | 4 - app/services/problem_check/twitter_login.rb | 6 - .../problem_check/unreachable_themes.rb | 2 +- app/services/problem_check/watched_words.rb | 4 - config/locales/server.en.yml | 65 ++++----- .../20240311015942_create_admin_notices.rb | 14 ++ ...8_add_details_to_problem_check_trackers.rb | 7 + ...19_add_target_to_problem_check_trackers.rb | 7 + ...iguate_problem_check_tracker_uniqueness.rb | 8 ++ spec/fabricators/admin_notice_fabricator.rb | 6 + spec/jobs/poll_mailbox_spec.rb | 8 +- spec/jobs/run_problem_check_spec.rb | 47 +------ spec/models/admin_dashboard_data_spec.rb | 18 --- spec/models/admin_notice_spec.rb | 28 ++++ spec/models/problem_check_tracker_spec.rb | 123 +++++++++++++++++- .../admin/dashboard_controller_spec.rb | 14 +- spec/requests/static_controller_spec.rb | 8 ++ .../group_email_credentials_spec.rb | 6 +- .../maxmind_db_configuration_spec.rb | 2 +- .../problem_check_spec.rb | 46 ++++++- 55 files changed, 512 insertions(+), 264 deletions(-) create mode 100644 app/models/admin_notice.rb create mode 100644 app/serializers/admin_notice_serializer.rb create mode 100644 app/services/problem_check/bad_favicon_url.rb create mode 100644 app/services/problem_check/inline_problem_check.rb create mode 100644 app/services/problem_check/poll_pop3_auth_error.rb create mode 100644 app/services/problem_check/poll_pop3_timeout.rb create mode 100644 db/migrate/20240311015942_create_admin_notices.rb create mode 100644 db/migrate/20240401054228_add_details_to_problem_check_trackers.rb create mode 100644 db/migrate/20240517014119_add_target_to_problem_check_trackers.rb create mode 100644 db/migrate/20240517051933_disambiguate_problem_check_tracker_uniqueness.rb create mode 100644 spec/fabricators/admin_notice_fabricator.rb create mode 100644 spec/models/admin_notice_spec.rb rename spec/{models => services}/problem_check_spec.rb (61%) diff --git a/app/controllers/admin/dashboard_controller.rb b/app/controllers/admin/dashboard_controller.rb index 047694cbd1d..6d2014a1df6 100644 --- a/app/controllers/admin/dashboard_controller.rb +++ b/app/controllers/admin/dashboard_controller.rb @@ -26,7 +26,9 @@ class Admin::DashboardController < Admin::StaffController end def problems - render_json_dump(problems: AdminDashboardData.fetch_problems(check_force_https: request.ssl?)) + ProblemCheck.realtime.run_all + + render json: { problems: serialize_data(AdminNotice.problem.all, AdminNoticeSerializer) } end def new_features diff --git a/app/controllers/static_controller.rb b/app/controllers/static_controller.rb index 945b7866e7c..cd9ba087b08 100644 --- a/app/controllers/static_controller.rb +++ b/app/controllers/static_controller.rb @@ -162,7 +162,7 @@ class StaticController < ApplicationController file&.read || "" rescue => e - AdminDashboardData.add_problem_message("dashboard.bad_favicon_url", 1800) + ProblemCheckTracker[:bad_favicon_url].problem! Rails.logger.debug("Failed to fetch favicon #{favicon.url}: #{e}\n#{e.backtrace}") "" ensure diff --git a/app/jobs/regular/run_problem_check.rb b/app/jobs/regular/run_problem_check.rb index f8468290072..161f0b21180 100644 --- a/app/jobs/regular/run_problem_check.rb +++ b/app/jobs/regular/run_problem_check.rb @@ -15,14 +15,8 @@ module Jobs check = ProblemCheck[identifier] - problems = check.call - raise RetrySignal if problems.present? && retry_count < check.max_retries - - if problems.present? - problems.each { |problem| AdminDashboardData.add_found_scheduled_check_problem(problem) } - ProblemCheckTracker[identifier].problem!(next_run_at: check.perform_every.from_now) - else - ProblemCheckTracker[identifier].no_problem!(next_run_at: check.perform_every.from_now) + check.run do |problems| + raise RetrySignal if problems.present? && retry_count < check.max_retries end rescue RetrySignal Jobs.enqueue_in( diff --git a/app/jobs/scheduled/poll_mailbox.rb b/app/jobs/scheduled/poll_mailbox.rb index 664eb788b53..01389d8fc1c 100644 --- a/app/jobs/scheduled/poll_mailbox.rb +++ b/app/jobs/scheduled/poll_mailbox.rb @@ -68,7 +68,7 @@ module Jobs if count > 3 Discourse.redis.del(POLL_MAILBOX_TIMEOUT_ERROR_KEY) mark_as_errored! - add_admin_dashboard_problem_message("dashboard.poll_pop3_timeout") + track_problem(:poll_pop3_timeout) Discourse.handle_job_exception( e, error_context( @@ -79,7 +79,7 @@ module Jobs end rescue Net::POPAuthenticationError => e mark_as_errored! - add_admin_dashboard_problem_message("dashboard.poll_pop3_auth_error") + track_problem(:poll_pop3_auth_error) Discourse.handle_job_exception(e, error_context(@args, "Signing in to poll incoming emails.")) end @@ -104,11 +104,8 @@ module Jobs Discourse.redis.zadd(POLL_MAILBOX_ERRORS_KEY, now, now.to_s) end - def add_admin_dashboard_problem_message(i18n_key) - AdminDashboardData.add_problem_message( - i18n_key, - SiteSetting.pop3_polling_period_mins.minutes + 5.minutes, - ) + def track_problem(identifier) + ProblemCheckTracker[identifier].problem! end end end diff --git a/app/models/admin_dashboard_data.rb b/app/models/admin_dashboard_data.rb index aa9e26f33ea..cc097be6cfb 100644 --- a/app/models/admin_dashboard_data.rb +++ b/app/models/admin_dashboard_data.rb @@ -3,7 +3,7 @@ class AdminDashboardData include StatsCacheable - cattr_reader :problem_messages + cattr_reader :problem_messages, default: [] # kept for backward compatibility GLOBAL_REPORTS ||= [] @@ -100,13 +100,8 @@ class AdminDashboardData # tests. It will also fire multiple times in development mode because # classes are not cached. def self.reset_problem_checks - @@problem_messages = %w[ - dashboard.bad_favicon_url - dashboard.poll_pop3_timeout - dashboard.poll_pop3_auth_error - ] + @@problem_messages = [] end - reset_problem_checks def self.fetch_stats new.as_json diff --git a/app/models/admin_notice.rb b/app/models/admin_notice.rb new file mode 100644 index 00000000000..9bc8f557a5f --- /dev/null +++ b/app/models/admin_notice.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +class AdminNotice < ActiveRecord::Base + validates :identifier, presence: true + + enum :priority, %i[low high].freeze + enum :subject, %i[problem].freeze + + def message + I18n.t( + "dashboard.#{subject}.#{identifier}", + **details.symbolize_keys.merge(base_path: Discourse.base_path), + ) + end +end + +# == Schema Information +# +# Table name: admin_notices +# +# id :bigint not null, primary key +# subject :integer not null +# priority :integer not null +# identifier :string not null +# details :json not null +# created_at :datetime not null +# updated_at :datetime not null +# +# Indexes +# +# index_admin_notices_on_subject (subject) +# index_admin_notices_on_identifier (identifier) +# diff --git a/app/models/problem_check.rb b/app/models/problem_check.rb index b9056732c7f..43c4c6765aa 100644 --- a/app/models/problem_check.rb +++ b/app/models/problem_check.rb @@ -1,6 +1,26 @@ # frozen_string_literal: true class ProblemCheck + class Collection + include Enumerable + + def initialize(checks) + @checks = checks + end + + def each(...) + checks.each(...) + end + + def run_all + each(&:run) + end + + private + + attr_reader :checks + end + include ActiveSupport::Configurable config_accessor :priority, default: "low", instance_writer: false @@ -33,6 +53,7 @@ class ProblemCheck # Note: This list must come after the `config_accessor` declarations. # CORE_PROBLEM_CHECKS = [ + ProblemCheck::BadFaviconUrl, ProblemCheck::EmailPollingErroredRecently, ProblemCheck::FacebookConfig, ProblemCheck::FailingEmails, @@ -45,6 +66,8 @@ class ProblemCheck ProblemCheck::ImageMagick, ProblemCheck::MissingMailgunApiKey, ProblemCheck::OutOfDateThemes, + ProblemCheck::PollPop3Timeout, + ProblemCheck::PollPop3AuthError, ProblemCheck::RailsEnv, ProblemCheck::Ram, ProblemCheck::S3BackupConfig, @@ -66,15 +89,15 @@ class ProblemCheck end def self.checks - CORE_PROBLEM_CHECKS | DiscoursePluginRegistry.problem_checks + Collection.new(DiscoursePluginRegistry.problem_checks.concat(CORE_PROBLEM_CHECKS)) end def self.scheduled - checks.select(&:scheduled?) + Collection.new(checks.select(&:scheduled?)) end def self.realtime - checks.reject(&:scheduled?) + Collection.new(checks.select(&:realtime?)) end def self.identifier @@ -96,6 +119,10 @@ class ProblemCheck new(data).call end + def self.run(data = {}, &) + new(data).run(&) + end + def initialize(data = {}) @data = OpenStruct.new(data) end @@ -106,9 +133,40 @@ class ProblemCheck raise NotImplementedError end + def run + problems = call + + yield(problems) if block_given? + + next_run_at = perform_every&.from_now + + if problems.empty? + targets.each { |t| tracker(t).no_problem!(next_run_at:) } + else + problems + .uniq(&:target) + .each do |problem| + tracker(problem.target).problem!( + next_run_at:, + details: translation_data.merge(problem.details).merge(base_path: Discourse.base_path), + ) + end + end + + problems + end + private - def problem(override_key = nil, override_data = {}) + def tracker(target = nil) + ProblemCheckTracker[identifier, target] + end + + def targets + [nil] + end + + def problem(override_key: nil, override_data: {}) [ Problem.new( message || @@ -132,8 +190,7 @@ class ProblemCheck end def translation_key - # TODO: Infer a default based on class name, then move translations in locale file. - raise NotImplementedError + "dashboard.problem.#{identifier}" end def translation_data diff --git a/app/models/problem_check_tracker.rb b/app/models/problem_check_tracker.rb index 5899cb57df8..722491b0542 100644 --- a/app/models/problem_check_tracker.rb +++ b/app/models/problem_check_tracker.rb @@ -1,27 +1,42 @@ # frozen_string_literal: true class ProblemCheckTracker < ActiveRecord::Base - validates :identifier, presence: true, uniqueness: true + validates :identifier, presence: true, uniqueness: { scope: :target } validates :blips, presence: true, numericality: { greater_than_or_equal_to: 0 } - def self.[](identifier) - find_or_create_by(identifier:) + scope :failing, -> { where("last_problem_at = last_run_at") } + scope :passing, -> { where("last_success_at = last_run_at") } + + def self.[](identifier, target = nil) + find_or_create_by(identifier:, target:) end def ready_to_run? next_run_at.blank? || next_run_at.past? end - def problem!(next_run_at: nil) + def failing? + last_problem_at == last_run_at + end + + def passing? + last_success_at == last_run_at + end + + def problem!(next_run_at: nil, details: {}) now = Time.current - update!(blips: blips + 1, last_run_at: now, last_problem_at: now, next_run_at:) + update!(blips: blips + 1, details:, last_run_at: now, last_problem_at: now, next_run_at:) + + sound_the_alarm if sound_the_alarm? end def no_problem!(next_run_at: nil) now = Time.current update!(blips: 0, last_run_at: now, last_success_at: now, next_run_at:) + + silence_the_alarm end def check @@ -31,7 +46,26 @@ class ProblemCheckTracker < ActiveRecord::Base private def sound_the_alarm? - blips > check.max_blips + failing? && blips > check.max_blips + end + + def sound_the_alarm + admin_notice.create_with( + priority: check.priority, + details: details.merge(target:), + ).find_or_create_by(identifier:) + end + + def silence_the_alarm + admin_notice.where(identifier:).delete_all + end + + def admin_notice + if target.present? + AdminNotice.problem.where("details->>'target' = ?", target) + else + AdminNotice.problem.where("(details->>'target') IS NULL") + end end end @@ -46,8 +80,10 @@ end # next_run_at :datetime # last_success_at :datetime # last_problem_at :datetime +# details :json +# target :string # # Indexes # -# index_problem_check_trackers_on_identifier (identifier) UNIQUE +# index_problem_check_trackers_on_identifier_and_target (identifier,target) UNIQUE # diff --git a/app/serializers/admin_notice_serializer.rb b/app/serializers/admin_notice_serializer.rb new file mode 100644 index 00000000000..febcb511ae2 --- /dev/null +++ b/app/serializers/admin_notice_serializer.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +class AdminNoticeSerializer < ApplicationSerializer + attributes :priority, :message, :identifier +end diff --git a/app/services/problem_check/bad_favicon_url.rb b/app/services/problem_check/bad_favicon_url.rb new file mode 100644 index 00000000000..81b1d116e5f --- /dev/null +++ b/app/services/problem_check/bad_favicon_url.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +class ProblemCheck::BadFaviconUrl < ProblemCheck::InlineProblemCheck + self.priority = "low" +end diff --git a/app/services/problem_check/email_polling_errored_recently.rb b/app/services/problem_check/email_polling_errored_recently.rb index 2f742b5e5f9..f4c02f8f53f 100644 --- a/app/services/problem_check/email_polling_errored_recently.rb +++ b/app/services/problem_check/email_polling_errored_recently.rb @@ -15,10 +15,6 @@ class ProblemCheck::EmailPollingErroredRecently < ProblemCheck @polling_error_count ||= Jobs::PollMailbox.errors_in_past_24_hours end - def translation_key - "dashboard.email_polling_errored_recently" - end - def translation_data { count: polling_error_count } end diff --git a/app/services/problem_check/facebook_config.rb b/app/services/problem_check/facebook_config.rb index 682297bf07b..931e43d2b6d 100644 --- a/app/services/problem_check/facebook_config.rb +++ b/app/services/problem_check/facebook_config.rb @@ -12,10 +12,6 @@ class ProblemCheck::FacebookConfig < ProblemCheck private - def translation_key - "dashboard.facebook_config_warning" - end - def facebook_credentials_present? SiteSetting.facebook_app_id.present? && SiteSetting.facebook_app_secret.present? end diff --git a/app/services/problem_check/failing_emails.rb b/app/services/problem_check/failing_emails.rb index 4eabd27d682..dbe0cba6507 100644 --- a/app/services/problem_check/failing_emails.rb +++ b/app/services/problem_check/failing_emails.rb @@ -15,10 +15,6 @@ class ProblemCheck::FailingEmails < ProblemCheck @failed_job_count ||= Jobs.num_email_retry_jobs end - def translation_key - "dashboard.failing_emails_warning" - end - def translation_data { num_failed_jobs: failed_job_count } end diff --git a/app/services/problem_check/force_https.rb b/app/services/problem_check/force_https.rb index bbc25387d29..19613fd4ca3 100644 --- a/app/services/problem_check/force_https.rb +++ b/app/services/problem_check/force_https.rb @@ -9,10 +9,4 @@ class ProblemCheck::ForceHttps < ProblemCheck problem end - - private - - def translation_key - "dashboard.force_https_warning" - end end diff --git a/app/services/problem_check/github_config.rb b/app/services/problem_check/github_config.rb index 4f2b4b16784..7ce3bd4e658 100644 --- a/app/services/problem_check/github_config.rb +++ b/app/services/problem_check/github_config.rb @@ -12,10 +12,6 @@ class ProblemCheck::GithubConfig < ProblemCheck private - def translation_key - "dashboard.github_config_warning" - end - def github_credentials_present? SiteSetting.github_client_id.present? && SiteSetting.github_client_secret.present? end diff --git a/app/services/problem_check/google_analytics_version.rb b/app/services/problem_check/google_analytics_version.rb index 2c47cfa25a2..7b5297970e6 100644 --- a/app/services/problem_check/google_analytics_version.rb +++ b/app/services/problem_check/google_analytics_version.rb @@ -8,10 +8,4 @@ class ProblemCheck::GoogleAnalyticsVersion < ProblemCheck problem end - - private - - def translation_key - "dashboard.v3_analytics_deprecated" - end end diff --git a/app/services/problem_check/google_oauth2_config.rb b/app/services/problem_check/google_oauth2_config.rb index 5cea8531300..d149124e79b 100644 --- a/app/services/problem_check/google_oauth2_config.rb +++ b/app/services/problem_check/google_oauth2_config.rb @@ -12,10 +12,6 @@ class ProblemCheck::GoogleOauth2Config < ProblemCheck private - def translation_key - "dashboard.google_oauth2_config_warning" - end - def google_oauth2_credentials_present? SiteSetting.google_oauth2_client_id.present? && SiteSetting.google_oauth2_client_secret.present? end diff --git a/app/services/problem_check/group_email_credentials.rb b/app/services/problem_check/group_email_credentials.rb index 7a2c32612b0..97e78631b8d 100644 --- a/app/services/problem_check/group_email_credentials.rb +++ b/app/services/problem_check/group_email_credentials.rb @@ -7,6 +7,7 @@ # 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.priority = "high" self.perform_every = 30.minutes def call @@ -15,6 +16,10 @@ class ProblemCheck::GroupEmailCredentials < ProblemCheck private + def targets + [*Group.with_smtp_configured.pluck(:name), *Group.with_imap_configured.pluck(:name)] + end + def smtp_errors return [] if !SiteSetting.enable_smtp @@ -52,7 +57,7 @@ class ProblemCheck::GroupEmailCredentials < ProblemCheck rescue *EmailSettingsExceptionHandler::EXPECTED_EXCEPTIONS => err message = I18n.t( - "dashboard.group_email_credentials_warning", + "dashboard.problem.group_email_credentials", { base_path: Discourse.base_path, group_name: group.name, @@ -61,7 +66,17 @@ class ProblemCheck::GroupEmailCredentials < ProblemCheck }, ) - Problem.new(message, priority: "high", identifier: "group_#{group.id}_email_credentials") + Problem.new( + message, + priority: "high", + identifier: "group_email_credentials", + target: group.id, + details: { + group_name: group.name, + group_full_name: group.full_name, + error: EmailSettingsExceptionHandler.friendly_exception_message(err, group.smtp_server), + }, + ) rescue => err Discourse.warn_exception( err, diff --git a/app/services/problem_check/host_names.rb b/app/services/problem_check/host_names.rb index 28520181f4f..a6fb2f5f823 100644 --- a/app/services/problem_check/host_names.rb +++ b/app/services/problem_check/host_names.rb @@ -8,10 +8,4 @@ class ProblemCheck::HostNames < ProblemCheck problem end - - private - - def translation_key - "dashboard.host_names_warning" - end end diff --git a/app/services/problem_check/image_magick.rb b/app/services/problem_check/image_magick.rb index 56131b31adb..4a07e5ef386 100644 --- a/app/services/problem_check/image_magick.rb +++ b/app/services/problem_check/image_magick.rb @@ -9,10 +9,4 @@ class ProblemCheck::ImageMagick < ProblemCheck problem end - - private - - def translation_key - "dashboard.image_magick_warning" - end end diff --git a/app/services/problem_check/inline_problem_check.rb b/app/services/problem_check/inline_problem_check.rb new file mode 100644 index 00000000000..cf1a204c5f0 --- /dev/null +++ b/app/services/problem_check/inline_problem_check.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +class ProblemCheck::InlineProblemCheck < ProblemCheck + def call + # The logic of this problem check is performed inline, so this class is + # purely here to support its configuration. + # + no_problem + end +end diff --git a/app/services/problem_check/maxmind_db_configuration.rb b/app/services/problem_check/maxmind_db_configuration.rb index 579b68c8c61..076d8d16a18 100644 --- a/app/services/problem_check/maxmind_db_configuration.rb +++ b/app/services/problem_check/maxmind_db_configuration.rb @@ -10,10 +10,4 @@ class ProblemCheck::MaxmindDbConfiguration < ProblemCheck no_problem end end - - private - - def translation_key - "dashboard.maxmind_db_configuration_warning" - end end diff --git a/app/services/problem_check/missing_mailgun_api_key.rb b/app/services/problem_check/missing_mailgun_api_key.rb index 7b28fe4d89b..4e399faefb1 100644 --- a/app/services/problem_check/missing_mailgun_api_key.rb +++ b/app/services/problem_check/missing_mailgun_api_key.rb @@ -10,10 +10,4 @@ class ProblemCheck::MissingMailgunApiKey < ProblemCheck problem end - - private - - def translation_key - "dashboard.missing_mailgun_api_key" - end end diff --git a/app/services/problem_check/out_of_date_themes.rb b/app/services/problem_check/out_of_date_themes.rb index 119aa83140e..f298f88df96 100644 --- a/app/services/problem_check/out_of_date_themes.rb +++ b/app/services/problem_check/out_of_date_themes.rb @@ -16,7 +16,7 @@ class ProblemCheck::OutOfDateThemes < ProblemCheck end def message - "#{I18n.t("dashboard.out_of_date_themes")}" + "#{I18n.t("dashboard.problem.out_of_date_themes")}" end def themes_list diff --git a/app/services/problem_check/poll_pop3_auth_error.rb b/app/services/problem_check/poll_pop3_auth_error.rb new file mode 100644 index 00000000000..f3874c9213a --- /dev/null +++ b/app/services/problem_check/poll_pop3_auth_error.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +class ProblemCheck::PollPop3AuthError < ProblemCheck::InlineProblemCheck + self.priority = "low" +end diff --git a/app/services/problem_check/poll_pop3_timeout.rb b/app/services/problem_check/poll_pop3_timeout.rb new file mode 100644 index 00000000000..c0a9a16e6db --- /dev/null +++ b/app/services/problem_check/poll_pop3_timeout.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +class ProblemCheck::PollPop3Timeout < ProblemCheck::InlineProblemCheck + self.priority = "low" +end diff --git a/app/services/problem_check/problem.rb b/app/services/problem_check/problem.rb index 0fa52456b05..03f69ff3eba 100644 --- a/app/services/problem_check/problem.rb +++ b/app/services/problem_check/problem.rb @@ -3,12 +3,14 @@ class ProblemCheck::Problem PRIORITIES = %w[low high].freeze - attr_reader :message, :priority, :identifier + attr_reader :message, :priority, :identifier, :target, :details - def initialize(message, priority: "low", identifier: nil) + def initialize(message, priority: "low", identifier: nil, target: nil, details: {}) @message = message @priority = PRIORITIES.include?(priority) ? priority : "low" @identifier = identifier + @target = target + @details = details end def to_s diff --git a/app/services/problem_check/rails_env.rb b/app/services/problem_check/rails_env.rb index 6f02b6db399..4a5efe0804d 100644 --- a/app/services/problem_check/rails_env.rb +++ b/app/services/problem_check/rails_env.rb @@ -11,10 +11,6 @@ class ProblemCheck::RailsEnv < ProblemCheck private - def translation_key - "dashboard.rails_env_warning" - end - def translation_data { env: Rails.env } end diff --git a/app/services/problem_check/ram.rb b/app/services/problem_check/ram.rb index aebf27bcb22..c0a9f2ff25a 100644 --- a/app/services/problem_check/ram.rb +++ b/app/services/problem_check/ram.rb @@ -11,10 +11,4 @@ class ProblemCheck::Ram < ProblemCheck problem end - - private - - def translation_key - "dashboard.memory_warning" - end end diff --git a/app/services/problem_check/s3_backup_config.rb b/app/services/problem_check/s3_backup_config.rb index 5b2373020df..d455d493d86 100644 --- a/app/services/problem_check/s3_backup_config.rb +++ b/app/services/problem_check/s3_backup_config.rb @@ -18,8 +18,4 @@ class ProblemCheck::S3BackupConfig < ProblemCheck SiteSetting.s3_access_key_id.blank? || SiteSetting.s3_secret_access_key.blank? end - - def translation_key - "dashboard.s3_backup_config_warning" - end end diff --git a/app/services/problem_check/s3_cdn.rb b/app/services/problem_check/s3_cdn.rb index 40fa79df4fb..9dc0b03f0f5 100644 --- a/app/services/problem_check/s3_cdn.rb +++ b/app/services/problem_check/s3_cdn.rb @@ -9,10 +9,4 @@ class ProblemCheck::S3Cdn < ProblemCheck problem end - - private - - def translation_key - "dashboard.s3_cdn_warning" - end end diff --git a/app/services/problem_check/s3_upload_config.rb b/app/services/problem_check/s3_upload_config.rb index 0580436b891..764ce1430c5 100644 --- a/app/services/problem_check/s3_upload_config.rb +++ b/app/services/problem_check/s3_upload_config.rb @@ -18,8 +18,4 @@ class ProblemCheck::S3UploadConfig < ProblemCheck SiteSetting.s3_access_key_id.blank? || SiteSetting.s3_secret_access_key.blank? end - - def translation_key - "dashboard.s3_config_warning" - end end diff --git a/app/services/problem_check/sidekiq_check.rb b/app/services/problem_check/sidekiq_check.rb index 5dd9337df78..424dfd05b70 100644 --- a/app/services/problem_check/sidekiq_check.rb +++ b/app/services/problem_check/sidekiq_check.rb @@ -4,8 +4,20 @@ class ProblemCheck::SidekiqCheck < ProblemCheck self.priority = "low" def call - return problem("dashboard.sidekiq_warning") if jobs_in_queue? && !jobs_performed_recently? - return problem("dashboard.queue_size_warning", queue_size: Jobs.queued) if massive_queue? + if jobs_in_queue? && !jobs_performed_recently? + return problem(override_key: "dashboard.problem.sidekiq") + end + + if massive_queue? + return( + problem( + override_key: "dashboard.problem.queue_size", + override_data: { + queue_size: Jobs.queued, + }, + ) + ) + end no_problem end diff --git a/app/services/problem_check/subfolder_ends_in_slash.rb b/app/services/problem_check/subfolder_ends_in_slash.rb index 8da3d618f1e..49afe7e65f7 100644 --- a/app/services/problem_check/subfolder_ends_in_slash.rb +++ b/app/services/problem_check/subfolder_ends_in_slash.rb @@ -8,10 +8,4 @@ class ProblemCheck::SubfolderEndsInSlash < ProblemCheck problem end - - private - - def translation_key - "dashboard.subfolder_ends_in_slash" - end end diff --git a/app/services/problem_check/translation_overrides.rb b/app/services/problem_check/translation_overrides.rb index d7b22d80da7..ac9bc2beda4 100644 --- a/app/services/problem_check/translation_overrides.rb +++ b/app/services/problem_check/translation_overrides.rb @@ -10,10 +10,4 @@ class ProblemCheck::TranslationOverrides < ProblemCheck problem end - - private - - def translation_key - "dashboard.outdated_translations_warning" - end end diff --git a/app/services/problem_check/twitter_config.rb b/app/services/problem_check/twitter_config.rb index 4566f3460ad..e6d0ca95600 100644 --- a/app/services/problem_check/twitter_config.rb +++ b/app/services/problem_check/twitter_config.rb @@ -12,10 +12,6 @@ class ProblemCheck::TwitterConfig < ProblemCheck private - def translation_key - "dashboard.twitter_config_warning" - end - def twitter_credentials_present? SiteSetting.twitter_consumer_key.present? && SiteSetting.twitter_consumer_secret.present? end diff --git a/app/services/problem_check/twitter_login.rb b/app/services/problem_check/twitter_login.rb index c8df68c9e3b..cfb676814d9 100644 --- a/app/services/problem_check/twitter_login.rb +++ b/app/services/problem_check/twitter_login.rb @@ -2,8 +2,6 @@ class ProblemCheck::TwitterLogin < ProblemCheck self.priority = "high" - - # TODO: Implement. self.perform_every = 24.hours def call @@ -18,8 +16,4 @@ class ProblemCheck::TwitterLogin < ProblemCheck def authenticator @authenticator ||= Auth::TwitterAuthenticator.new end - - def translation_key - "dashboard.twitter_login_warning" - end end diff --git a/app/services/problem_check/unreachable_themes.rb b/app/services/problem_check/unreachable_themes.rb index d5a3a7a279a..708700bdf9a 100644 --- a/app/services/problem_check/unreachable_themes.rb +++ b/app/services/problem_check/unreachable_themes.rb @@ -16,7 +16,7 @@ class ProblemCheck::UnreachableThemes < ProblemCheck end def message - "#{I18n.t("dashboard.unreachable_themes")}" + "#{I18n.t("dashboard.problem.unreachable_themes")}" end def themes_list diff --git a/app/services/problem_check/watched_words.rb b/app/services/problem_check/watched_words.rb index 49aed44ad66..118bb0d9872 100644 --- a/app/services/problem_check/watched_words.rb +++ b/app/services/problem_check/watched_words.rb @@ -11,10 +11,6 @@ class ProblemCheck::WatchedWords < ProblemCheck private - def translation_key - "dashboard.watched_word_regexp_error" - end - def translation_data { action: invalid_regexp_actions.map { |w| "'#{w}'" }.join(", ") } end diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 0cac6c40c42..ecc3a6f7500 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1615,38 +1615,39 @@ en: description: "Top 10 users who have had likes from a wide range of people." dashboard: - twitter_login_warning: 'Twitter login appears to not be working at the moment. Check the credentials in the Site Settings.' - group_email_credentials_warning: 'There was an issue with the email credentials for the group %{group_full_name}. No emails will be sent from the group inbox until this problem is addressed. %{error}' - rails_env_warning: "Your server is running in %{env} mode." - host_names_warning: "Your config/database.yml file is using the default localhost hostname. Update it to use your site's hostname." - sidekiq_warning: 'Sidekiq is not running. Many tasks, like sending emails, are executed asynchronously by Sidekiq. Please ensure at least one Sidekiq process is running. Learn about Sidekiq here.' - queue_size_warning: "The number of queued jobs is %{queue_size}, which is high. This could indicate a problem with the Sidekiq process(es), or you may need to add more Sidekiq workers." - memory_warning: "Your server is running with less than 1 GB of total memory. At least 1 GB of memory is recommended." - maxmind_db_configuration_warning: 'The server has been configured to use MaxMind databases for reverse IP lookups but a valid MaxMind account ID has not been configured which may result in MaxMind databases failing to download in the future. See this guide to learn more.' - google_oauth2_config_warning: 'The server is configured to allow signup and login with Google OAuth2 (enable_google_oauth2_logins), but the client id and client secret values are not set. Go to the Site Settings and update the settings. See this guide to learn more.' - facebook_config_warning: 'The server is configured to allow signup and login with Facebook (enable_facebook_logins), but the app id and app secret values are not set. Go to the Site Settings and update the settings. See this guide to learn more.' - twitter_config_warning: 'The server is configured to allow signup and login with Twitter (enable_twitter_logins), but the key and secret values are not set. Go to the Site Settings and update the settings. See this guide to learn more.' - github_config_warning: 'The server is configured to allow signup and login with GitHub (enable_github_logins), but the client id and secret values are not set. Go to the Site Settings and update the settings. See this guide to learn more.' - s3_config_warning: 'The server is configured to upload files to S3, but at least one the following setting is not set: s3_access_key_id, s3_secret_access_key, s3_use_iam_profile, or s3_upload_bucket. Go to the Site Settings and update the settings. See "How to set up image uploads to S3?" to learn more.' - s3_backup_config_warning: 'The server is configured to upload backups to S3, but at least one the following setting is not set: s3_access_key_id, s3_secret_access_key, s3_use_iam_profile, or s3_backup_bucket. Go to the Site Settings and update the settings. See "How to set up image uploads to S3?" to learn more.' - s3_cdn_warning: 'The server is configured to upload files to S3, but there is no S3 CDN configured. This can lead to expensive S3 costs and slower site performance. See "Using Object Storage for Uploads" to learn more.' - image_magick_warning: 'The server is configured to create thumbnails of large images, but ImageMagick is not installed. Install ImageMagick using your favorite package manager or download the latest release.' - failing_emails_warning: 'There are %{num_failed_jobs} email jobs that failed. Check your app.yml and ensure that the mail server settings are correct. See the failed jobs in Sidekiq.' - subfolder_ends_in_slash: "Your subfolder setup is incorrect; the DISCOURSE_RELATIVE_URL_ROOT ends in a slash." - outdated_translations_warning: "Some of your translation overrides are out of date. Please check your text customizations." - email_polling_errored_recently: - one: "Email polling has generated an error in the past 24 hours. Look at the logs for more details." - other: "Email polling has generated %{count} errors in the past 24 hours. Look at the logs for more details." - missing_mailgun_api_key: "The server is configured to send emails via Mailgun but you haven't provided an API key used to verify the webhook messages." - bad_favicon_url: "The favicon is failing to load. Check your favicon setting in Site Settings." - poll_pop3_timeout: "Connection to the POP3 server is timing out. Incoming email could not be retrieved. Please check your POP3 settings and service provider." - poll_pop3_auth_error: "Connection to the POP3 server is failing with an authentication error. Please check your POP3 settings." - force_https_warning: "Your website is using SSL. But `force_https` is not yet enabled in your site settings." - out_of_date_themes: "Updates are available for the following themes:" - unreachable_themes: "We were unable to check for updates on the following themes:" - watched_word_regexp_error: "The regular expression for %{action} watched words is invalid. Please check your Watched Word settings, or disable the 'watched words regular expressions' site setting." - v3_analytics_deprecated: "Your Discourse is currently using Google Analytics 3, which will no longer be supported after July 2023. Upgrade to Google Analytics 4 now to continue receiving valuable insights and analytics for your website's performance." - category_style_deprecated: "Your Discourse is currently using a deprecated category style which will be removed before the final beta release of Discourse 3.2. Please refer to Moving to a Single Category Style Site Setting for instructions on how to keep your selected category style." + problem: + twitter_login: 'Twitter login appears to not be working at the moment. Check the credentials in the Site Settings.' + group_email_credentials: 'There was an issue with the email credentials for the group %{group_full_name}. No emails will be sent from the group inbox until this problem is addressed. %{error}' + rails_env: "Your server is running in %{env} mode." + host_names: "Your config/database.yml file is using the default localhost hostname. Update it to use your site's hostname." + sidekiq: 'Sidekiq is not running. Many tasks, like sending emails, are executed asynchronously by Sidekiq. Please ensure at least one Sidekiq process is running. Learn about Sidekiq here.' + queue_size: "The number of queued jobs is %{queue_size}, which is high. This could indicate a problem with the Sidekiq process(es), or you may need to add more Sidekiq workers." + ram: "Your server is running with less than 1 GB of total memory. At least 1 GB of memory is recommended." + google_oauth2_config: 'The server is configured to allow signup and login with Google OAuth2 (enable_google_oauth2_logins), but the client id and client secret values are not set. Go to the Site Settings and update the settings. See this guide to learn more.' + facebook_config: 'The server is configured to allow signup and login with Facebook (enable_facebook_logins), but the app id and app secret values are not set. Go to the Site Settings and update the settings. See this guide to learn more.' + twitter_config: 'The server is configured to allow signup and login with Twitter (enable_twitter_logins), but the key and secret values are not set. Go to the Site Settings and update the settings. See this guide to learn more.' + github_config: 'The server is configured to allow signup and login with GitHub (enable_github_logins), but the client id and secret values are not set. Go to the Site Settings and update the settings. See this guide to learn more.' + s3_upload_config: 'The server is configured to upload files to S3, but at least one the following setting is not set: s3_access_key_id, s3_secret_access_key, s3_use_iam_profile, or s3_upload_bucket. Go to the Site Settings and update the settings. See "How to set up image uploads to S3?" to learn more.' + s3_backup_config: 'The server is configured to upload backups to S3, but at least one the following setting is not set: s3_access_key_id, s3_secret_access_key, s3_use_iam_profile, or s3_backup_bucket. Go to the Site Settings and update the settings. See "How to set up image uploads to S3?" to learn more.' + s3_cdn: 'The server is configured to upload files to S3, but there is no S3 CDN configured. This can lead to expensive S3 costs and slower site performance. See "Using Object Storage for Uploads" to learn more.' + image_magick: 'The server is configured to create thumbnails of large images, but ImageMagick is not installed. Install ImageMagick using your favorite package manager or download the latest release.' + failing_emails: 'There are %{num_failed_jobs} email jobs that failed. Check your app.yml and ensure that the mail server settings are correct. See the failed jobs in Sidekiq.' + subfolder_ends_in_slash: "Your subfolder setup is incorrect; the DISCOURSE_RELATIVE_URL_ROOT ends in a slash." + translation_overrides: "Some of your translation overrides are out of date. Please check your text customizations." + email_polling_errored_recently: + one: "Email polling has generated an error in the past 24 hours. Look at the logs for more details." + other: "Email polling has generated %{count} errors in the past 24 hours. Look at the logs for more details." + missing_mailgun_api_key: "The server is configured to send emails via Mailgun but you haven't provided an API key used to verify the webhook messages." + bad_favicon_url: "The favicon is failing to load. Check your favicon setting in Site Settings." + poll_pop3_timeout: "Connection to the POP3 server is timing out. Incoming email could not be retrieved. Please check your POP3 settings and service provider." + poll_pop3_auth_error: "Connection to the POP3 server is failing with an authentication error. Please check your POP3 settings." + force_https: "Your website is using SSL. But `force_https` is not yet enabled in your site settings." + out_of_date_themes: "Updates are available for the following themes:" + unreachable_themes: "We were unable to check for updates on the following themes:" + watched_words: "The regular expression for %{action} watched words is invalid. Please check your Watched Word settings, or disable the 'watched words regular expressions' site setting." + google_analytics_version: "Your Discourse is currently using Google Analytics 3, which will no longer be supported after July 2023. Upgrade to Google Analytics 4 now to continue receiving valuable insights and analytics for your website's performance." + category_style_deprecated: "Your Discourse is currently using a deprecated category style which will be removed before the final beta release of Discourse 3.2. Please refer to Moving to a Single Category Style Site Setting for instructions on how to keep your selected category style." + maxmind_db_configuration: 'The server has been configured to use MaxMind databases for reverse IP lookups but a valid MaxMind account ID has not been configured which may result in MaxMind databases failing to download in the future. See this guide to learn more.' back_from_logster_text: "Back to site" site_settings: diff --git a/db/migrate/20240311015942_create_admin_notices.rb b/db/migrate/20240311015942_create_admin_notices.rb new file mode 100644 index 00000000000..aa6591cf3f9 --- /dev/null +++ b/db/migrate/20240311015942_create_admin_notices.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true +class CreateAdminNotices < ActiveRecord::Migration[7.0] + def change + create_table :admin_notices do |t| + t.integer :subject, null: false, index: true + t.integer :priority, null: false + + t.string :identifier, null: false, index: true + t.json :details, null: false, default: {} + + t.timestamps + end + end +end diff --git a/db/migrate/20240401054228_add_details_to_problem_check_trackers.rb b/db/migrate/20240401054228_add_details_to_problem_check_trackers.rb new file mode 100644 index 00000000000..d498b82e7ff --- /dev/null +++ b/db/migrate/20240401054228_add_details_to_problem_check_trackers.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddDetailsToProblemCheckTrackers < ActiveRecord::Migration[7.0] + def change + add_column :problem_check_trackers, :details, :json, default: {} + end +end diff --git a/db/migrate/20240517014119_add_target_to_problem_check_trackers.rb b/db/migrate/20240517014119_add_target_to_problem_check_trackers.rb new file mode 100644 index 00000000000..0810140965d --- /dev/null +++ b/db/migrate/20240517014119_add_target_to_problem_check_trackers.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddTargetToProblemCheckTrackers < ActiveRecord::Migration[7.0] + def change + add_column :problem_check_trackers, :target, :string, null: true + end +end diff --git a/db/migrate/20240517051933_disambiguate_problem_check_tracker_uniqueness.rb b/db/migrate/20240517051933_disambiguate_problem_check_tracker_uniqueness.rb new file mode 100644 index 00000000000..683e070dbde --- /dev/null +++ b/db/migrate/20240517051933_disambiguate_problem_check_tracker_uniqueness.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +class DisambiguateProblemCheckTrackerUniqueness < ActiveRecord::Migration[7.0] + def change + remove_index :problem_check_trackers, name: "index_problem_check_trackers_on_identifier" + add_index :problem_check_trackers, %i[identifier target], unique: true + end +end diff --git a/spec/fabricators/admin_notice_fabricator.rb b/spec/fabricators/admin_notice_fabricator.rb new file mode 100644 index 00000000000..fa0e23e0d83 --- /dev/null +++ b/spec/fabricators/admin_notice_fabricator.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +Fabricator(:admin_notice) do + priority { "low" } + identifier { "test_notice" } +end diff --git a/spec/jobs/poll_mailbox_spec.rb b/spec/jobs/poll_mailbox_spec.rb index a4ad5aa0537..432712b65da 100644 --- a/spec/jobs/poll_mailbox_spec.rb +++ b/spec/jobs/poll_mailbox_spec.rb @@ -45,9 +45,9 @@ RSpec.describe Jobs::PollMailbox do poller.poll_pop3 - i18n_key = "dashboard.poll_pop3_auth_error" + i18n_key = "dashboard.problem.poll_pop3_auth_error" - expect(AdminDashboardData.problem_message_check(i18n_key)).to eq( + expect(AdminNotice.find_by(identifier: "poll_pop3_auth_error").message).to eq( I18n.t(i18n_key, base_path: Discourse.base_path), ) end @@ -57,9 +57,9 @@ RSpec.describe Jobs::PollMailbox do 4.times { poller.poll_pop3 } - i18n_key = "dashboard.poll_pop3_timeout" + i18n_key = "dashboard.problem.poll_pop3_timeout" - expect(AdminDashboardData.problem_message_check(i18n_key)).to eq( + expect(AdminNotice.find_by(identifier: "poll_pop3_timeout").message).to eq( I18n.t(i18n_key, base_path: Discourse.base_path), ) end diff --git a/spec/jobs/run_problem_check_spec.rb b/spec/jobs/run_problem_check_spec.rb index 56098f7528c..aa905d82aaa 100644 --- a/spec/jobs/run_problem_check_spec.rb +++ b/spec/jobs/run_problem_check_spec.rb @@ -27,49 +27,10 @@ RSpec.describe Jobs::RunProblemCheck do ProblemCheck.send(:remove_const, "TestCheck") end - it "adds the messages to the Redis problems array" do - described_class.new.execute(check_identifier: :test_check) - - problems = AdminDashboardData.load_found_scheduled_check_problems - - expect(problems.map(&:to_s)).to contain_exactly("Big problem", "Yuge problem") - end - end - - context "with multiple problems with the same identifier" do - around do |example| - ProblemCheck::TestCheck = - Class.new(ProblemCheck) do - self.perform_every = 30.minutes - 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 - - stub_const(ProblemCheck, "CORE_PROBLEM_CHECKS", [ProblemCheck::TestCheck], &example) - - ProblemCheck.send(:remove_const, "TestCheck") - end - - 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"]) + it "updates the problem check tracker" do + expect { + described_class.new.execute(check_identifier: :test_check, retry_count: 0) + }.to change { ProblemCheckTracker.failing.count }.by(1) end end diff --git a/spec/models/admin_dashboard_data_spec.rb b/spec/models/admin_dashboard_data_spec.rb index 78119cb60ab..dc660c5b166 100644 --- a/spec/models/admin_dashboard_data_spec.rb +++ b/spec/models/admin_dashboard_data_spec.rb @@ -6,24 +6,6 @@ RSpec.describe AdminDashboardData do Discourse.redis.flushdb end - describe "#fetch_problems" do - describe "adding problem messages" do - it "adds the message and returns it when the problems are fetched" do - AdminDashboardData.add_problem_message("dashboard.bad_favicon_url") - problems = AdminDashboardData.fetch_problems.map(&:to_s) - expect(problems).to include( - I18n.t("dashboard.bad_favicon_url", { base_path: Discourse.base_path }), - ) - end - - it "does not allow adding of arbitrary problem messages, they must exist in AdminDashboardData.problem_messages" do - AdminDashboardData.add_problem_message("errors.messages.invalid") - problems = AdminDashboardData.fetch_problems.map(&:to_s) - expect(problems).not_to include(I18n.t("errors.messages.invalid")) - end - end - end - describe "adding scheduled checks" do it "does not add duplicate problems with the same identifier" do prob1 = ProblemCheck::Problem.new("test problem", identifier: "test") diff --git a/spec/models/admin_notice_spec.rb b/spec/models/admin_notice_spec.rb new file mode 100644 index 00000000000..44af14c60e4 --- /dev/null +++ b/spec/models/admin_notice_spec.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +RSpec.describe AdminNotice do + it { is_expected.to validate_presence_of(:identifier) } + + describe "#message" do + let(:notice) do + Fabricate( + :admin_notice, + identifier: "test", + subject: "problem", + priority: "high", + details: { + thing: "world", + }, + ) + end + + before do + I18n.backend.store_translations( + :en, + { "dashboard" => { "problem" => { "test" => "Something is wrong with the %{thing}" } } }, + ) + end + + it { expect(notice.message).to eq("Something is wrong with the world") } + end +end diff --git a/spec/models/problem_check_tracker_spec.rb b/spec/models/problem_check_tracker_spec.rb index 6d0b3a9b800..60b5f9f4e0e 100644 --- a/spec/models/problem_check_tracker_spec.rb +++ b/spec/models/problem_check_tracker_spec.rb @@ -1,11 +1,13 @@ # 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") } it { expect(record).to validate_presence_of(:identifier) } - it { expect(record).to validate_uniqueness_of(:identifier) } + it { expect(record).to validate_uniqueness_of(:identifier).scoped_to(:target) } it { expect(record).to validate_numericality_of(:blips).is_greater_than_or_equal_to(0) } end @@ -44,14 +46,63 @@ RSpec.describe ProblemCheckTracker do end end + describe "#failing?" do + before { freeze_time } + + let(:problem_tracker) { described_class.new(last_problem_at:, last_run_at:, last_success_at:) } + + context "when the last run passed" do + let(:last_run_at) { 1.minute.ago } + let(:last_success_at) { 1.minute.ago } + let(:last_problem_at) { 11.minutes.ago } + + it { expect(problem_tracker).not_to be_failing } + end + + context "when the last run had a problem" do + let(:last_run_at) { 1.minute.ago } + let(:last_success_at) { 11.minutes.ago } + let(:last_problem_at) { 1.minute.ago } + + it { expect(problem_tracker).to be_failing } + end + end + + describe "#passing?" do + before { freeze_time } + + let(:problem_tracker) { described_class.new(last_problem_at:, last_run_at:, last_success_at:) } + + context "when the last run passed" do + let(:last_run_at) { 1.minute.ago } + let(:last_success_at) { 1.minute.ago } + let(:last_problem_at) { 11.minutes.ago } + + it { expect(problem_tracker).to be_passing } + end + + context "when the last run had a problem" do + let(:last_run_at) { 1.minute.ago } + let(:last_success_at) { 11.minutes.ago } + let(:last_problem_at) { 1.minute.ago } + + it { expect(problem_tracker).not_to be_passing } + end + end + describe "#problem!" do let(:problem_tracker) do - Fabricate(:problem_check_tracker, identifier: "twitter_login", **original_attributes) + Fabricate( + :problem_check_tracker, + identifier: "twitter_login", + target: "foo", + **original_attributes, + ) end let(:original_attributes) do { - blips: 0, + blips:, last_problem_at: 1.week.ago, last_success_at: 24.hours.ago, last_run_at: 24.hours.ago, @@ -59,6 +110,7 @@ RSpec.describe ProblemCheckTracker do } end + let(:blips) { 0 } let(:updated_attributes) { { blips: 1 } } it do @@ -68,6 +120,61 @@ RSpec.describe ProblemCheckTracker do problem_tracker.attributes }.to(hash_including(updated_attributes)) end + + context "when the maximum number of blips have been surpassed" do + let(:blips) { 1 } + + it "sounds the alarm" do + expect { problem_tracker.problem!(next_run_at: 24.hours.from_now) }.to change { + AdminNotice.problem.count + }.by(1) + end + end + + context "when there's an alarm sounding for multi-target trackers" do + let(:blips) { 1 } + + before do + Fabricate( + :admin_notice, + subject: "problem", + identifier: "twitter_login", + details: { + target: target, + }, + ) + end + + context "when the alarm is for a different target" do + let(:target) { "bar" } + + it "sounds the alarm" do + expect { problem_tracker.problem!(next_run_at: 24.hours.from_now) }.to change { + AdminNotice.problem.count + }.by(1) + end + end + + context "when the alarm is for a the same target" do + let(:target) { "foo" } + + it "does not duplicate the alarm" do + expect { problem_tracker.problem!(next_run_at: 24.hours.from_now) }.not_to change { + AdminNotice.problem.count + } + end + end + end + + context "when there are still blips to go" do + let(:blips) { 0 } + + it "does not sound the alarm" do + expect { problem_tracker.problem!(next_run_at: 24.hours.from_now) }.not_to change { + AdminNotice.problem.count + } + end + end end describe "#no_problem!" do @@ -94,5 +201,15 @@ RSpec.describe ProblemCheckTracker do problem_tracker.attributes }.to(hash_including(updated_attributes)) end + + context "when there's an alarm sounding" do + before { Fabricate(:admin_notice, subject: "problem", identifier: "twitter_login") } + + it "silences the alarm" do + expect { problem_tracker.no_problem!(next_run_at: 24.hours.from_now) }.to change { + AdminNotice.problem.count + }.by(-1) + end + end end end diff --git a/spec/requests/admin/dashboard_controller_spec.rb b/spec/requests/admin/dashboard_controller_spec.rb index f333d234c43..48b33877665 100644 --- a/spec/requests/admin/dashboard_controller_spec.rb +++ b/spec/requests/admin/dashboard_controller_spec.rb @@ -98,12 +98,12 @@ RSpec.describe Admin::DashboardController do end describe "#problems" do + before { ProblemCheck.stubs(:realtime).returns(stub(run_all: [])) } + context "when logged in as an admin" do before { sign_in(admin) } context "when there are no problems" do - before { AdminDashboardData.stubs(:fetch_problems).returns([]) } - it "returns an empty array" do get "/admin/dashboard/problems.json" @@ -115,7 +115,8 @@ RSpec.describe Admin::DashboardController do context "when there are problems" do before do - AdminDashboardData.stubs(:fetch_problems).returns(["Not enough awesome", "Too much sass"]) + Fabricate(:admin_notice, subject: "problem", identifier: "foo") + Fabricate(:admin_notice, subject: "problem", identifier: "bar") end it "returns an array of strings" do @@ -123,8 +124,6 @@ RSpec.describe Admin::DashboardController do expect(response.status).to eq(200) json = response.parsed_body expect(json["problems"].size).to eq(2) - expect(json["problems"][0]).to be_a(String) - expect(json["problems"][1]).to be_a(String) end end end @@ -132,7 +131,9 @@ RSpec.describe Admin::DashboardController do context "when logged in as a moderator" do before do sign_in(moderator) - AdminDashboardData.stubs(:fetch_problems).returns(["Not enough awesome", "Too much sass"]) + + Fabricate(:admin_notice, subject: "problem", identifier: "foo") + Fabricate(:admin_notice, subject: "problem", identifier: "bar") end it "returns a list of problems" do @@ -141,7 +142,6 @@ RSpec.describe Admin::DashboardController do expect(response.status).to eq(200) json = response.parsed_body expect(json["problems"].size).to eq(2) - expect(json["problems"]).to contain_exactly("Not enough awesome", "Too much sass") end end diff --git a/spec/requests/static_controller_spec.rb b/spec/requests/static_controller_spec.rb index 01ad41be19e..65060a8393b 100644 --- a/spec/requests/static_controller_spec.rb +++ b/spec/requests/static_controller_spec.rb @@ -54,6 +54,14 @@ RSpec.describe StaticController do expect(response.media_type).to eq("image/png") expect(response.body.bytesize).to eq(upload.filesize) end + + context "when favicon fails to load" do + before { FileHelper.stubs(:download).raises(SocketError) } + + it "creates an admin notice" do + expect { get "/favicon/proxied" }.to change { AdminNotice.problem.count }.by(1) + end + end end end diff --git a/spec/services/problem_check/group_email_credentials_spec.rb b/spec/services/problem_check/group_email_credentials_spec.rb index 5eed6ea23c1..2c2ea73a70f 100644 --- a/spec/services/problem_check/group_email_credentials_spec.rb +++ b/spec/services/problem_check/group_email_credentials_spec.rb @@ -41,7 +41,8 @@ RSpec.describe ProblemCheck::GroupEmailCredentials do expect(described_class.new.call).to contain_exactly( have_attributes( - identifier: "group_#{group2.id}_email_credentials", + identifier: "group_email_credentials", + target: group2.id, 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.", @@ -58,7 +59,8 @@ RSpec.describe ProblemCheck::GroupEmailCredentials do expect(described_class.new.call).to contain_exactly( have_attributes( - identifier: "group_#{group3.id}_email_credentials", + identifier: "group_email_credentials", + target: group3.id, 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.", diff --git a/spec/services/problem_check/maxmind_db_configuration_spec.rb b/spec/services/problem_check/maxmind_db_configuration_spec.rb index ca642e5b961..f5981993139 100644 --- a/spec/services/problem_check/maxmind_db_configuration_spec.rb +++ b/spec/services/problem_check/maxmind_db_configuration_spec.rb @@ -20,7 +20,7 @@ RSpec.describe ProblemCheck::MaxmindDbConfiguration do global_setting :maxmind_license_key, "license_key" expect(check).to have_a_problem.with_priority("low").with_message( - I18n.t("dashboard.maxmind_db_configuration_warning"), + I18n.t("dashboard.problem.maxmind_db_configuration"), ) end end diff --git a/spec/models/problem_check_spec.rb b/spec/services/problem_check_spec.rb similarity index 61% rename from spec/models/problem_check_spec.rb rename to spec/services/problem_check_spec.rb index 55ed6fcf025..3f3c482bf59 100644 --- a/spec/models/problem_check_spec.rb +++ b/spec/services/problem_check_spec.rb @@ -5,16 +5,46 @@ RSpec.describe ProblemCheck do ScheduledCheck = Class.new(described_class) { self.perform_every = 30.minutes } RealtimeCheck = Class.new(described_class) PluginCheck = Class.new(described_class) + FailingCheck = + Class.new(described_class) do + def call + problem + end - stub_const(described_class, "CORE_PROBLEM_CHECKS", [ScheduledCheck, RealtimeCheck], &example) + def translation_key + "failing_check" + end + end + PassingCheck = + Class.new(described_class) do + def call + no_problem + end + + def translation_key + "passing_check" + end + end + + stub_const( + described_class, + "CORE_PROBLEM_CHECKS", + [ScheduledCheck, RealtimeCheck, FailingCheck, PassingCheck], + &example + ) Object.send(:remove_const, ScheduledCheck.name) Object.send(:remove_const, RealtimeCheck.name) Object.send(:remove_const, PluginCheck.name) + Object.send(:remove_const, FailingCheck.name) + Object.send(:remove_const, PassingCheck.name) end let(:scheduled_check) { ScheduledCheck } let(:realtime_check) { RealtimeCheck } + let(:plugin_check) { PluginCheck } + let(:failing_check) { FailingCheck } + let(:passing_check) { PassingCheck } describe ".[]" do it { expect(described_class[:scheduled_check]).to eq(scheduled_check) } @@ -57,13 +87,23 @@ RSpec.describe ProblemCheck do context "when the plugin is enabled" do let(:enabled) { true } - it { expect(described_class.checks).to include(PluginCheck) } + it { expect(described_class.checks).to include(plugin_check) } end context "when the plugin is disabled" do let(:enabled) { false } - it { expect(described_class.checks).not_to include(PluginCheck) } + it { expect(described_class.checks).not_to include(plugin_check) } + end + end + + describe "#run" do + context "when check is failing" do + it { expect { failing_check.run }.to change { ProblemCheckTracker.failing.count }.by(1) } + end + + context "when check is passing" do + it { expect { passing_check.run }.to change { ProblemCheckTracker.passing.count }.by(1) } end end end