From 7ecb258b83aaf88cb6dde8cb1957b69d7dc5f509 Mon Sep 17 00:00:00 2001 From: Ted Johansson Date: Mon, 7 Oct 2024 12:29:33 +0800 Subject: [PATCH] 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. --- app/services/admin_notices/dismiss.rb | 6 +++++- spec/services/admin_notices/dismiss_spec.rb | 8 ++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/app/services/admin_notices/dismiss.rb b/app/services/admin_notices/dismiss.rb index 93e57d048d8..8d6215eeea1 100644 --- a/app/services/admin_notices/dismiss.rb +++ b/app/services/admin_notices/dismiss.rb @@ -3,7 +3,7 @@ class AdminNotices::Dismiss include Service::Base - model :admin_notice + model :admin_notice, optional: true policy :invalid_access @@ -23,10 +23,14 @@ class AdminNotices::Dismiss 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? + ProblemCheckTracker.find_by(identifier: admin_notice.identifier)&.reset end end diff --git a/spec/services/admin_notices/dismiss_spec.rb b/spec/services/admin_notices/dismiss_spec.rb index 758aba9d1ca..d16f439d0c7 100644 --- a/spec/services/admin_notices/dismiss_spec.rb +++ b/spec/services/admin_notices/dismiss_spec.rb @@ -12,6 +12,14 @@ RSpec.describe(AdminNotices::Dismiss) do it { is_expected.to fail_a_policy(:invalid_access) } 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 fab!(:current_user) { Fabricate(:admin) }