DEV: Remove old problem check system - part 2 (#34403)

We have overhauled the problem check system, which is now database backed. As part of that we marked some existing code as deprecated and slated for removal. This PR removes it.

app/jobs/scheduled/admin_problems.rb

@@ -1,36 +0,0 @@
-# frozen_string_literal: true
-
-module Jobs
-  class AdminProblems < ::Jobs::Scheduled
-    every 30.minutes
-
-    def execute(args)
-      Notification
-        .where(notification_type: Notification.types[:admin_problems])
-        .where("created_at < ?", 7.days.ago)
-        .destroy_all
-
-      return if !persistent_problems?
-
-      notified_user_ids =
-        Notification.where(notification_type: Notification.types[:admin_problems]).pluck(:user_id)
-
-      users = Group[:admins].users.where.not(id: notified_user_ids)
-
-      users.each do |user|
-        Notification.create!(
-          notification_type: Notification.types[:admin_problems],
-          user_id: user.id,
-          data: "{}",
-        )
-      end
-    end
-
-    private
-
-    def persistent_problems?
-      problems_started_at = AdminDashboardData.problems_started_at
-      problems_started_at && problems_started_at < 2.days.ago
-    end
-  end
-end

app/models/admin_dashboard_data.rb

@@ -3,14 +3,9 @@
 class AdminDashboardData
   include StatsCacheable
 
-  cattr_reader :problem_messages, default: []
-
   # kept for backward compatibility
   GLOBAL_REPORTS = []
 
-  PROBLEM_MESSAGE_PREFIX = "admin-problem:"
-  SCHEDULED_PROBLEM_STORAGE_KEY = "admin-found-scheduled-problems-list"
-
   def initialize(opts = {})
     @opts = opts
   end
@@ -23,45 +18,6 @@ class AdminDashboardData
     @json ||= get_json
   end
 
-  def problems
-    problems = []
-    self.class.problem_messages.each do |i18n_key|
-      message = self.class.problem_message_check(i18n_key)
-      problems << ProblemCheck::Problem.new(message) if message.present?
-    end
-    problems.concat(ProblemCheck.realtime.flat_map { |c| c.call(@opts).map(&:to_h) })
-
-    problems.compact!
-
-    if problems.empty?
-      self.class.clear_problems_started
-    else
-      self.class.set_problems_started
-    end
-
-    problems
-  end
-
-  def self.add_problem_check(*syms, &blk)
-    Discourse.deprecate(
-      "`AdminDashboardData#add_problem_check` is deprecated. Implement a class that inherits `ProblemCheck` instead.",
-      drop_from: "3.3",
-    )
-  end
-
-  ##
-  # We call this method in the class definition below
-  # so all of the problem checks in this class are registered on
-  # boot. These problem checks are run when the problems are loaded in
-  # the admin dashboard controller.
-  #
-  # This method also can be used in testing to reset checks between
-  # tests. It will also fire multiple times in development mode because
-  # classes are not cached.
-  def self.reset_problem_checks
-    @@problem_messages = []
-  end
-
   def self.fetch_stats
     new.as_json
   end
@@ -73,55 +29,4 @@ class AdminDashboardData
   def self.stats_cache_key
     "dashboard-data-#{Report::SCHEMA_VERSION}"
   end
-
-  def self.problems_started_key
-    "dash-problems-started-at"
-  end
-
-  def self.set_problems_started
-    existing_time = Discourse.redis.get(problems_started_key)
-    Discourse.redis.setex(problems_started_key, 14.days.to_i, existing_time || Time.zone.now.to_s)
-  end
-
-  def self.clear_problems_started
-    Discourse.redis.del problems_started_key
-  end
-
-  def self.problems_started_at
-    s = Discourse.redis.get(problems_started_key)
-    s ? Time.zone.parse(s) : nil
-  end
-
-  def self.fetch_problems(opts = {})
-    new(opts).problems
-  end
-
-  def self.problem_message_check(i18n_key)
-    if Discourse.redis.get(problem_message_key(i18n_key))
-      I18n.t(i18n_key, base_path: Discourse.base_path)
-    else
-      nil
-    end
-  end
-
-  ##
-  # Arbitrary messages cannot be added here, they must already be defined
-  # in the @problem_messages array which is defined in reset_problem_checks.
-  # The array is iterated over and each key that exists in redis will be added
-  # to the final problems output in #problems.
-  def self.add_problem_message(i18n_key, expire_seconds = nil)
-    if expire_seconds.to_i > 0
-      Discourse.redis.setex problem_message_key(i18n_key), expire_seconds.to_i, 1
-    else
-      Discourse.redis.set problem_message_key(i18n_key), 1
-    end
-  end
-
-  def self.clear_problem_message(i18n_key)
-    Discourse.redis.del problem_message_key(i18n_key)
-  end
-
-  def self.problem_message_key(i18n_key)
-    "#{PROBLEM_MESSAGE_PREFIX}#{i18n_key}"
-  end
 end

spec/jobs/admin_problems_spec.rb

@@ -1,37 +0,0 @@
-# frozen_string_literal: true
-
-RSpec.describe ::Jobs::AdminProblems do
-  fab!(:admin)
-
-  it "creates notification when problems persist for at least 2 days" do
-    Discourse.redis.setex(AdminDashboardData.problems_started_key, 14.days.to_i, Time.zone.now.to_s)
-    expect { described_class.new.execute({}) }.not_to change { Notification.count }
-
-    Discourse.redis.setex(AdminDashboardData.problems_started_key, 14.days.to_i, 3.days.ago)
-    expect { described_class.new.execute({}) }.to change { Notification.count }.by(1)
-  end
-
-  it "does not replace old notification created in last 7 days" do
-    Discourse.redis.setex(AdminDashboardData.problems_started_key, 14.days.to_i, 3.days.ago)
-    expect { described_class.new.execute({}) }.to change { Notification.count }.by(1)
-    old_notification = Notification.last
-
-    expect { described_class.new.execute({}) }.not_to change { Notification.count }
-    new_notification = Notification.last
-
-    expect(old_notification.id).to equal(new_notification.id)
-  end
-
-  it "replace old notification created more than 7 days ago" do
-    Discourse.redis.setex(AdminDashboardData.problems_started_key, 14.days.to_i, 13.days.ago)
-    freeze_time 10.days.ago do
-      expect { described_class.new.execute({}) }.to change { Notification.count }.by(1)
-    end
-    old_notification = Notification.last
-
-    expect { described_class.new.execute({}) }.not_to change { Notification.count }
-    new_notification = Notification.last
-
-    expect(old_notification.id).not_to equal(new_notification.id)
-  end
-end

spec/models/admin_dashboard_data_spec.rb

@@ -1,36 +1,9 @@
 # frozen_string_literal: true
 
 RSpec.describe AdminDashboardData do
-  after do
-    AdminDashboardData.reset_problem_checks
-    Discourse.redis.flushdb
-  end
+  after { Discourse.redis.flushdb }
 
   describe "stats cache" do
     include_examples "stats cacheable"
   end
-
-  describe "#problem_message_check" do
-    let(:key) { "new_key" }
-
-    after { described_class.clear_problem_message(key) }
-
-    it "returns nil if message has not been added" do
-      expect(described_class.problem_message_check(key)).to be_nil
-    end
-
-    it "returns a message if it was added" do
-      described_class.add_problem_message(key)
-      expect(described_class.problem_message_check(key)).to eq(
-        I18n.t(key, base_path: Discourse.base_path),
-      )
-    end
-
-    it "returns a message if it was added with an expiry" do
-      described_class.add_problem_message(key, 300)
-      expect(described_class.problem_message_check(key)).to eq(
-        I18n.t(key, base_path: Discourse.base_path),
-      )
-    end
-  end
 end