FIX: Update AdminNotice details when problem check tracker changes (#31031)

We have many problem check trackers, and some of them
like `OutOfDateThemes` can have a message which has variable
data in it shown to admins. In this case, a list of themes
that need updating. Currently if you resolve one of these
out of date themes and refresh the list of problems, you
do not see any change.

This is happening because we are only updating the `details`
of the `ProblemCheckTracker` record, not the corresponding
`AdminNotice` record which is what is displayed to the admins
on their dashboard. This commit fixes the issue by updating the
details of the notice at the same time as the problem check
tracker details.
This commit is contained in:
Martin Brennan 2025-01-29 09:44:20 +10:00 committed by GitHub
parent 3196918f4d
commit d8102cb532
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 52 additions and 2 deletions

View File

@ -28,6 +28,8 @@ class ProblemCheckTracker < ActiveRecord::Base
update!(blips: blips + 1, details:, last_run_at: now, last_problem_at: now, next_run_at:)
update_notice_details(details)
sound_the_alarm if sound_the_alarm?
end
@ -55,6 +57,18 @@ class ProblemCheckTracker < ActiveRecord::Base
private
def resolved_target
self.target || ProblemCheck::NO_TARGET
end
def details_with_target(details)
details.merge(target: resolved_target)
end
def update_notice_details(details)
admin_notice.where(identifier:).update_all(details: details_with_target(details))
end
def sound_the_alarm?
failing? && blips > check.max_blips
end
@ -62,7 +76,7 @@ class ProblemCheckTracker < ActiveRecord::Base
def sound_the_alarm
admin_notice.create_with(
priority: check.priority,
details: details.merge(target: target || ProblemCheck::NO_TARGET),
details: details_with_target(self.details),
).find_or_create_by(identifier:)
end
@ -71,7 +85,7 @@ class ProblemCheckTracker < ActiveRecord::Base
end
def admin_notice
AdminNotice.problem.where("details->>'target' = ?", target || ProblemCheck::NO_TARGET)
AdminNotice.problem.where("details->>'target' = ?", resolved_target)
end
end

View File

@ -146,6 +146,42 @@ RSpec.describe ProblemCheckTracker do
end
end
context "when the details of the problem change but the problem remains" do
let(:blips) { 1 }
it "updates the notice" do
original_details = {
themes_list:
"<ul><li><a href=\"/admin/customize/themes/13\">discourse-blank-theme</a></li> <li><a href=\"/admin/customize/themes/31\">Simple Theme</a></li></ul>",
base_path: "",
}
expect do problem_tracker.problem!(details: original_details) end.to change {
AdminNotice.problem.count
}.by(1)
admin_notice = AdminNotice.problem.find_by(identifier: "twitter_login")
expect(
admin_notice.details.merge(target: problem_tracker.target).with_indifferent_access,
).to eq(original_details.merge(target: problem_tracker.target).with_indifferent_access)
new_details = {
themes_list: "<ul><li><a href=\"/admin/customize/themes/31\">Simple Theme</a></li></ul>",
base_path: "",
}
expect do problem_tracker.problem!(details: new_details) end.not_to change {
AdminNotice.problem.count
}
admin_notice.reload
expect(
admin_notice.details.merge(target: problem_tracker.target).with_indifferent_access,
).to eq(new_details.merge(target: problem_tracker.target).with_indifferent_access)
end
end
context "when there's an alarm sounding for multi-target trackers" do
let(:blips) { 1 }