FIX: Support idempotent admin notice dismissal (#29099)

If you have the admin dashboard open, and one of the admin notices listed has already been dismissed (e.g. in another tab, or by another admin) we would show an ugly "FAILED" modal.

This change makes the admin dismiss endpoint idempotent. If the admin notice is already destroyed, then respond with 200. This will also correctly remove it from the list in the front-end.
This commit is contained in:
Ted Johansson 2024-10-07 12:29:33 +08:00 committed by GitHub
parent 4ba8d3b76b
commit 7ecb258b83
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 13 additions and 1 deletions

View File

@ -3,7 +3,7 @@
class AdminNotices::Dismiss class AdminNotices::Dismiss
include Service::Base include Service::Base
model :admin_notice model :admin_notice, optional: true
policy :invalid_access policy :invalid_access
@ -23,10 +23,14 @@ class AdminNotices::Dismiss
end end
def destroy(admin_notice:) def destroy(admin_notice:)
return if admin_notice.blank?
admin_notice.destroy! admin_notice.destroy!
end end
def reset_problem_check(admin_notice:) def reset_problem_check(admin_notice:)
return if admin_notice.blank?
ProblemCheckTracker.find_by(identifier: admin_notice.identifier)&.reset ProblemCheckTracker.find_by(identifier: admin_notice.identifier)&.reset
end end
end end

View File

@ -12,6 +12,14 @@ RSpec.describe(AdminNotices::Dismiss) do
it { is_expected.to fail_a_policy(:invalid_access) } it { is_expected.to fail_a_policy(:invalid_access) }
end end
context "when the admin notice has already been dismissed" do
fab!(:current_user) { Fabricate(:admin) }
before { admin_notice.destroy! }
it { is_expected.to run_successfully }
end
context "when user is allowed to perform the action" do context "when user is allowed to perform the action" do
fab!(:current_user) { Fabricate(:admin) } fab!(:current_user) { Fabricate(:admin) }