FIX: Make problem check tracker unique constraint work on PG13 (#29272)

In #29169 we added a NULLS NOT DISTINCT option to the unique index on problem_check_trackers. This is to enforce uniqueness even when the target is NULL. (Postgres considers all NULLs to be distinct by default.)

However, this only works in PG15. In PG13 it does nothing.

This commit adds a default dummy string value __NULL__ to target. Since it's a string, PG13 will be able to correctly identify duplicate records.

app/models/problem_check.rb

@@ -90,6 +90,10 @@ class ProblemCheck
     ProblemCheck::WatchedWords,
   ].freeze
 
+  # To enforce the unique constraint in Postgres <15 we need a dummy
+  # value, since the index considers NULLs to be distinct.
+  NO_TARGET = "__NULL__"
+
   def self.[](key)
     key = key.to_sym
 
@@ -176,12 +180,12 @@ class ProblemCheck
 
   private
 
-  def tracker(target = nil)
+  def tracker(target = NO_TARGET)
     ProblemCheckTracker[identifier, target]
   end
 
   def targets
-    [nil]
+    [NO_TARGET]
   end
 
   def problem(override_key: nil, override_data: {})

app/models/problem_check_tracker.rb

@@ -7,7 +7,7 @@ class ProblemCheckTracker < ActiveRecord::Base
   scope :failing, -> { where("last_problem_at = last_run_at") }
   scope :passing, -> { where("last_success_at = last_run_at") }
 
-  def self.[](identifier, target = nil)
+  def self.[](identifier, target = ProblemCheck::NO_TARGET)
     find_or_create_by(identifier:, target:)
   end
 
@@ -71,11 +71,7 @@ class ProblemCheckTracker < ActiveRecord::Base
   end
 
   def admin_notice
-    if target.present?
+    AdminNotice.problem.where("details->>'target' = ?", target || ProblemCheck::NO_TARGET)
-      AdminNotice.problem.where("details->>'target' = ?", target)
-    else
-      AdminNotice.problem.where("(details->>'target') IS NULL")
-    end
   end
 end
 
@@ -91,7 +87,7 @@ end
 #  last_success_at :datetime
 #  last_problem_at :datetime
 #  details         :json
-#  target          :string
+#  target          :string           default("__NULL__")
 #
 # Indexes
 #

db/migrate/20241018031851_add_default_value_to_problem_check_trackers_target.rb

@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+class AddDefaultValueToProblemCheckTrackersTarget < ActiveRecord::Migration[7.1]
+  def up
+    change_column_default :problem_check_trackers, :target, "__NULL__"
+
+    execute(<<~SQL)
+      UPDATE problem_check_trackers
+      SET target='__NULL__'
+      WHERE target IS NULL
+    SQL
+  end
+
+  def down
+    change_column_default :problem_check_trackers, :target, nil
+
+    execute(<<~SQL)
+      UPDATE problem_check_trackers
+      SET target = NULL
+      WHERE target = '__NULL__'
+    SQL
+  end
+end

spec/models/problem_check_tracker_spec.rb

@@ -187,7 +187,6 @@ RSpec.describe ProblemCheckTracker do
       before { ProblemCheck::TwitterLogin.stubs(:max_blips).returns(1) }
 
       it "does not sound the alarm" do
-        puts ProblemCheck::TwitterLogin.max_blips
         expect { problem_tracker.problem!(next_run_at: 24.hours.from_now) }.not_to change {
           AdminNotice.problem.count
         }
@@ -221,7 +220,7 @@ RSpec.describe ProblemCheckTracker do
     end
 
     context "when there's an alarm sounding" do
-      before { Fabricate(:admin_notice, subject: "problem", identifier: "twitter_login") }
+      before { problem_tracker.problem! }
 
       it "silences the alarm" do
         expect { problem_tracker.no_problem!(next_run_at: 24.hours.from_now) }.to change {