mirror of
https://github.com/discourse/discourse.git
synced 2026-08-11 02:59:07 +08:00
When dismissing an admin notice, the service would look up the
associated `ProblemCheckTracker` by identifier only. This could find a
tracker with a `NULL` target, which then fails validation on update
since target is now required (as of 8ca5fb706a).
The root issue is that `problem_check_trackers.target` was added with
`null: true` and while we later added a default value of `"__NULL__"`
and a Ruby validation, we never enforced `NOT NULL` at the database
level. This allowed records with `NULL` targets to persist or be
recreated through code paths that explicitly passed nil.
This commit:
- Updates the dismiss service to look up trackers by both identifier AND
target (extracted from the admin notice's details)
- Removes any remaining records with `NULL` targets
- Adds a `NOT NULL` constraint to prevent this from recurring
Ref - https://meta.discourse.org/t/392248
43 lines
800 B
Ruby
Vendored
43 lines
800 B
Ruby
Vendored
# frozen_string_literal: true
|
|
|
|
class AdminNotices::Dismiss
|
|
include Service::Base
|
|
|
|
policy :invalid_access
|
|
|
|
params do
|
|
attribute :id, :integer
|
|
|
|
validates :id, presence: true
|
|
end
|
|
|
|
model :admin_notice, optional: true
|
|
|
|
transaction do
|
|
step :destroy
|
|
step :reset_problem_check
|
|
end
|
|
|
|
private
|
|
|
|
def invalid_access(guardian:)
|
|
guardian.is_admin?
|
|
end
|
|
|
|
def fetch_admin_notice(params:)
|
|
AdminNotice.find_by(id: params.id)
|
|
end
|
|
|
|
def destroy(admin_notice:)
|
|
return if admin_notice.blank?
|
|
|
|
admin_notice.destroy!
|
|
end
|
|
|
|
def reset_problem_check(admin_notice:)
|
|
return if admin_notice.blank?
|
|
|
|
target = admin_notice.details&.dig("target") || ProblemCheck::NO_TARGET
|
|
ProblemCheckTracker.find_by(identifier: admin_notice.identifier, target:)&.reset
|
|
end
|
|
end
|