From d26d45540e9395cd1be1063e158fab14df4bc402 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lo=C3=AFc=20Guitaut?= <loic@discourse.org>
Date: Wed, 28 Aug 2024 14:54:10 +0200
Subject: [PATCH] DEV: Use `run_successfully` matcher in service specs

---
 app/services/update_site_setting.rb           |  4 -
 .../chat/add_users_to_channel_spec.rb         |  2 +
 .../chat/auto_join_channel_batch_spec.rb      |  4 +
 .../chat/create_category_channel_spec.rb      |  2 +
 .../create_direct_message_channel_spec.rb     |  3 +
 .../spec/services/chat/create_message_spec.rb | 14 ++-
 .../spec/services/chat/create_thread_spec.rb  |  4 +-
 .../spec/services/chat/flag_message_spec.rb   |  2 +
 .../chat/invite_users_to_channel_spec.rb      |  4 +-
 .../spec/services/chat/leave_channel_spec.rb  | 12 +++
 .../chat/list_channel_messages_spec.rb        | 24 +++++-
 .../chat/list_channel_thread_messages_spec.rb | 12 +--
 .../services/chat/list_user_channels_spec.rb  |  2 +
 .../chat/lookup_channel_threads_spec.rb       |  2 +-
 .../spec/services/chat/lookup_thread_spec.rb  |  6 +-
 .../services/chat/lookup_user_threads_spec.rb | 59 ++++---------
 .../chat/mark_all_user_channels_read_spec.rb  | 10 +--
 .../mark_thread_title_prompt_seen_spec.rb     |  4 +-
 .../services/chat/restore_message_spec.rb     |  4 +-
 .../services/chat/search_chatable_spec.rb     |  4 +-
 .../chat/stop_message_streaming_spec.rb       | 14 +--
 .../spec/services/chat/trash_channel_spec.rb  |  4 +-
 .../spec/services/chat/trash_message_spec.rb  |  4 +-
 .../spec/services/chat/trash_messages_spec.rb |  4 +-
 .../services/chat/unfollow_channel_spec.rb    |  4 +
 .../spec/services/chat/update_channel_spec.rb |  4 +-
 .../chat/update_channel_status_spec.rb        |  4 +-
 .../spec/services/chat/update_message_spec.rb |  4 +-
 ...pdate_thread_notification_settings_spec.rb |  4 +-
 .../spec/services/chat/update_thread_spec.rb  |  4 +-
 .../update_user_channel_last_read_spec.rb     |  4 +-
 .../chat/update_user_thread_last_read_spec.rb |  4 +-
 .../spec/services/chat/upsert_draft_spec.rb   |  2 +
 spec/services/flags/create_flag_spec.rb       |  5 +-
 spec/services/flags/destroy_flag_spec.rb      |  6 +-
 spec/services/flags/reorder_flag_spec.rb      |  4 +-
 spec/services/flags/toggle_flag_spec.rb       |  4 +-
 spec/services/flags/update_flag_spec.rb       |  8 +-
 spec/services/update_site_setting_spec.rb     | 85 ++++++++++---------
 spec/support/service_matchers.rb              | 20 ++++-
 40 files changed, 189 insertions(+), 181 deletions(-)

diff --git a/app/services/update_site_setting.rb b/app/services/update_site_setting.rb
index be896e4767d..0b4f28ff1f3 100644
--- a/app/services/update_site_setting.rb
+++ b/app/services/update_site_setting.rb
@@ -4,14 +4,10 @@ class UpdateSiteSetting
   include Service::Base
 
   policy :current_user_is_admin
-
   contract
-
   step :convert_name_to_sym
-
   policy :setting_is_visible
   policy :setting_is_configurable
-
   step :cleanup_value
   step :save
 
diff --git a/plugins/chat/spec/services/chat/add_users_to_channel_spec.rb b/plugins/chat/spec/services/chat/add_users_to_channel_spec.rb
index 3914f37943b..b825e1dae03 100644
--- a/plugins/chat/spec/services/chat/add_users_to_channel_spec.rb
+++ b/plugins/chat/spec/services/chat/add_users_to_channel_spec.rb
@@ -28,6 +28,8 @@ RSpec.describe Chat::AddUsersToChannel do
     context "when all steps pass" do
       before { channel.add(current_user) }
 
+      it { is_expected.to run_successfully }
+
       it "fetches users to add" do
         expect(result.target_users.map(&:username)).to contain_exactly(*users.map(&:username))
       end
diff --git a/plugins/chat/spec/services/chat/auto_join_channel_batch_spec.rb b/plugins/chat/spec/services/chat/auto_join_channel_batch_spec.rb
index 26f47d4ee68..37853130c3e 100644
--- a/plugins/chat/spec/services/chat/auto_join_channel_batch_spec.rb
+++ b/plugins/chat/spec/services/chat/auto_join_channel_batch_spec.rb
@@ -86,6 +86,8 @@ describe Chat::AutoJoinChannelBatch do
         context "when more than one membership is created" do
           let(:user_ids) { Fabricate.times(2, :user).map(&:id) }
 
+          it { is_expected.to run_successfully }
+
           it "does not recalculate user count" do
             ::Chat::ChannelMembershipManager.any_instance.expects(:recalculate_user_count).never
             result
@@ -101,6 +103,8 @@ describe Chat::AutoJoinChannelBatch do
         end
 
         context "when only one membership is created" do
+          it { is_expected.to run_successfully }
+
           it "recalculates user count" do
             ::Chat::ChannelMembershipManager.any_instance.expects(:recalculate_user_count).once
             result
diff --git a/plugins/chat/spec/services/chat/create_category_channel_spec.rb b/plugins/chat/spec/services/chat/create_category_channel_spec.rb
index ea0fc252a15..5fbc574bbd1 100644
--- a/plugins/chat/spec/services/chat/create_category_channel_spec.rb
+++ b/plugins/chat/spec/services/chat/create_category_channel_spec.rb
@@ -60,6 +60,8 @@ RSpec.describe Chat::CreateCategoryChannel do
       end
 
       context "when all steps pass" do
+        it { is_expected.to run_successfully }
+
         it "creates the channel" do
           expect { result }.to change { Chat::Channel.count }.by(1)
           expect(result.channel).to have_attributes(
diff --git a/plugins/chat/spec/services/chat/create_direct_message_channel_spec.rb b/plugins/chat/spec/services/chat/create_direct_message_channel_spec.rb
index 844247d40d2..0b80c0f6ab8 100644
--- a/plugins/chat/spec/services/chat/create_direct_message_channel_spec.rb
+++ b/plugins/chat/spec/services/chat/create_direct_message_channel_spec.rb
@@ -17,6 +17,7 @@ RSpec.describe Chat::CreateDirectMessageChannel do
         expect(contract.target_usernames).to eq(%w[lechuck elaine])
       end
     end
+
     context "when the target_groups argument is a string" do
       let(:params) { { target_groups: "admins,moderators" } }
 
@@ -42,6 +43,8 @@ RSpec.describe Chat::CreateDirectMessageChannel do
     let(:params) { { guardian: guardian, target_usernames: target_usernames, name: name } }
 
     context "when all steps pass" do
+      it { is_expected.to run_successfully }
+
       it "sets the service result as successful" do
         expect(result).to be_a_success
       end
diff --git a/plugins/chat/spec/services/chat/create_message_spec.rb b/plugins/chat/spec/services/chat/create_message_spec.rb
index 51b4172e4ef..5ed68b2e221 100644
--- a/plugins/chat/spec/services/chat/create_message_spec.rb
+++ b/plugins/chat/spec/services/chat/create_message_spec.rb
@@ -237,7 +237,7 @@ RSpec.describe Chat::CreateMessage do
           context "when user is a bot" do
             fab!(:user) { Discourse.system_user }
 
-            it { is_expected.to be_a_success }
+            it { is_expected.to run_successfully }
           end
 
           context "when membership is enforced" do
@@ -248,7 +248,7 @@ RSpec.describe Chat::CreateMessage do
               params[:enforce_membership] = true
             end
 
-            it { is_expected.to be_a_success }
+            it { is_expected.to run_successfully }
           end
 
           context "when user can join channel" do
@@ -291,6 +291,8 @@ RSpec.describe Chat::CreateMessage do
                       it_behaves_like "creating a new message"
                       it_behaves_like "a message in a thread"
 
+                      it { is_expected.to run_successfully }
+
                       it "assigns the thread to the new message" do
                         expect(message).to have_attributes(
                           in_reply_to: an_object_having_attributes(thread: thread),
@@ -312,6 +314,8 @@ RSpec.describe Chat::CreateMessage do
                         let(:original_user) { reply_to.user }
                       end
 
+                      it { is_expected.to run_successfully }
+
                       it "creates a new thread" do
                         expect { result }.to change { Chat::Thread.count }.by(1)
                         expect(message).to have_attributes(
@@ -389,6 +393,8 @@ RSpec.describe Chat::CreateMessage do
                         it_behaves_like "creating a new message"
                         it_behaves_like "a message in a thread"
 
+                        it { is_expected.to run_successfully }
+
                         it "does not publish the thread" do
                           Chat::Publisher.expects(:publish_thread_created!).never
                           result
@@ -400,6 +406,8 @@ RSpec.describe Chat::CreateMessage do
                       it_behaves_like "creating a new message"
                       it_behaves_like "a message in a thread"
 
+                      it { is_expected.to run_successfully }
+
                       it "does not publish the thread" do
                         Chat::Publisher.expects(:publish_thread_created!).never
                         result
@@ -418,6 +426,8 @@ RSpec.describe Chat::CreateMessage do
                   context "when message is valid" do
                     it_behaves_like "creating a new message"
 
+                    it { is_expected.to run_successfully }
+
                     it "updates membership last_read_message attribute" do
                       expect { result }.to change { membership.reload.last_read_message }
                     end
diff --git a/plugins/chat/spec/services/chat/create_thread_spec.rb b/plugins/chat/spec/services/chat/create_thread_spec.rb
index c7cb6cc9d43..e2479837901 100644
--- a/plugins/chat/spec/services/chat/create_thread_spec.rb
+++ b/plugins/chat/spec/services/chat/create_thread_spec.rb
@@ -25,9 +25,7 @@ RSpec.describe Chat::CreateThread do
     end
 
     context "when all steps pass" do
-      it "sets the service result as successful" do
-        expect(result).to be_a_success
-      end
+      it { is_expected.to run_successfully }
 
       it "creates a thread" do
         result
diff --git a/plugins/chat/spec/services/chat/flag_message_spec.rb b/plugins/chat/spec/services/chat/flag_message_spec.rb
index 499c2183053..83b58bebaf1 100644
--- a/plugins/chat/spec/services/chat/flag_message_spec.rb
+++ b/plugins/chat/spec/services/chat/flag_message_spec.rb
@@ -44,6 +44,8 @@ RSpec.describe Chat::FlagMessage do
     context "when all steps pass" do
       fab!(:current_user) { Fabricate(:admin) }
 
+      it { is_expected.to run_successfully }
+
       it "flags the message" do
         expect { result }.to change { Reviewable.count }.by(1)
 
diff --git a/plugins/chat/spec/services/chat/invite_users_to_channel_spec.rb b/plugins/chat/spec/services/chat/invite_users_to_channel_spec.rb
index 2dce1cfef0b..0d0dafdcbae 100644
--- a/plugins/chat/spec/services/chat/invite_users_to_channel_spec.rb
+++ b/plugins/chat/spec/services/chat/invite_users_to_channel_spec.rb
@@ -29,9 +29,7 @@ RSpec.describe Chat::InviteUsersToChannel do
   end
 
   context "when all steps pass" do
-    it "sets the service result as successful" do
-      expect(result).to run_service_successfully
-    end
+    it { is_expected.to run_successfully }
 
     it "creates the notifications for allowed users" do
       result
diff --git a/plugins/chat/spec/services/chat/leave_channel_spec.rb b/plugins/chat/spec/services/chat/leave_channel_spec.rb
index c28d95154de..37b33d21b2f 100644
--- a/plugins/chat/spec/services/chat/leave_channel_spec.rb
+++ b/plugins/chat/spec/services/chat/leave_channel_spec.rb
@@ -28,6 +28,8 @@ RSpec.describe Chat::LeaveChannel do
             Chat::Channel.ensure_consistency!
           end
 
+          it { is_expected.to run_successfully }
+
           it "unfollows the channel" do
             membership = channel_1.membership_for(current_user)
 
@@ -40,6 +42,8 @@ RSpec.describe Chat::LeaveChannel do
         end
 
         context "with no existing membership" do
+          it { is_expected.to run_successfully }
+
           it "does nothing" do
             expect { result }.to_not change { Chat::UserChatChannelMembership }
           end
@@ -54,6 +58,8 @@ RSpec.describe Chat::LeaveChannel do
 
           before { Chat::Channel.ensure_consistency! }
 
+          it { is_expected.to run_successfully }
+
           it "leaves the channel" do
             membership = channel_1.membership_for(current_user)
 
@@ -71,6 +77,8 @@ RSpec.describe Chat::LeaveChannel do
         end
 
         context "with no existing membership" do
+          it { is_expected.to run_successfully }
+
           it "does nothing" do
             expect { result }.to_not change { Chat::UserChatChannelMembership }
           end
@@ -85,6 +93,8 @@ RSpec.describe Chat::LeaveChannel do
 
           before { Chat::Channel.ensure_consistency! }
 
+          it { is_expected.to run_successfully }
+
           it "unfollows the channel" do
             membership = channel_1.membership_for(current_user)
 
@@ -100,6 +110,8 @@ RSpec.describe Chat::LeaveChannel do
         end
 
         context "with no existing membership" do
+          it { is_expected.to run_successfully }
+
           it "does nothing" do
             expect { result }.to_not change { Chat::UserChatChannelMembership }
           end
diff --git a/plugins/chat/spec/services/chat/list_channel_messages_spec.rb b/plugins/chat/spec/services/chat/list_channel_messages_spec.rb
index ce210e7c3aa..ffdad6ec100 100644
--- a/plugins/chat/spec/services/chat/list_channel_messages_spec.rb
+++ b/plugins/chat/spec/services/chat/list_channel_messages_spec.rb
@@ -29,6 +29,8 @@ RSpec.describe Chat::ListChannelMessages do
     end
 
     context "when channel exists" do
+      it { is_expected.to run_successfully }
+
       it "finds the correct channel" do
         expect(result.channel).to eq(channel)
       end
@@ -37,6 +39,8 @@ RSpec.describe Chat::ListChannelMessages do
 
   context "when fetch_eventual_membership" do
     context "when user has membership" do
+      it { is_expected.to run_successfully }
+
       it "finds the correct membership" do
         expect(result.membership).to eq(channel.membership_for(user))
       end
@@ -45,6 +49,8 @@ RSpec.describe Chat::ListChannelMessages do
     context "when user has no membership" do
       before { channel.membership_for(user).destroy! }
 
+      it { is_expected.to run_successfully }
+
       it "finds no membership" do
         expect(result.membership).to be_blank
       end
@@ -86,7 +92,7 @@ RSpec.describe Chat::ListChannelMessages do
 
   context "when target_message_exists" do
     context "when no target_message_id is given" do
-      it { is_expected.to be_a_success }
+      it { is_expected.to run_successfully }
     end
 
     context "when target message is not found" do
@@ -99,7 +105,7 @@ RSpec.describe Chat::ListChannelMessages do
       fab!(:target_message) { Fabricate(:chat_message, chat_channel: channel) }
       let(:optional_params) { { target_message_id: target_message.id } }
 
-      it { is_expected.to be_a_success }
+      it { is_expected.to run_successfully }
     end
 
     context "when target message is trashed" do
@@ -117,13 +123,13 @@ RSpec.describe Chat::ListChannelMessages do
       context "when user is the message creator" do
         fab!(:target_message) { Fabricate(:chat_message, chat_channel: channel, user: user) }
 
-        it { is_expected.to be_a_success }
+        it { is_expected.to run_successfully }
       end
 
       context "when user is admin" do
         fab!(:user) { Fabricate(:admin) }
 
-        it { is_expected.to be_a_success }
+        it { is_expected.to run_successfully }
       end
     end
   end
@@ -132,6 +138,8 @@ RSpec.describe Chat::ListChannelMessages do
     context "with no params" do
       fab!(:messages) { Fabricate.times(20, :chat_message, chat_channel: channel) }
 
+      it { is_expected.to be_a_success }
+
       it "returns messages" do
         expect(result.can_load_more_past).to eq(false)
         expect(result.can_load_more_future).to eq(false)
@@ -149,6 +157,8 @@ RSpec.describe Chat::ListChannelMessages do
 
       let(:optional_params) { { target_date: 2.days.ago } }
 
+      it { is_expected.to be_a_success }
+
       it "includes past and future messages" do
         expect(result.messages).to eq([past_message, future_message])
       end
@@ -164,6 +174,8 @@ RSpec.describe Chat::ListChannelMessages do
         thread_1.add(user)
       end
 
+      it { is_expected.to be_a_success }
+
       it "returns tracking" do
         Fabricate(:chat_message, chat_channel: channel, thread: thread_1)
 
@@ -175,6 +187,8 @@ RSpec.describe Chat::ListChannelMessages do
       context "when thread is forced" do
         before { thread_1.update!(force: true) }
 
+        it { is_expected.to be_a_success }
+
         it "returns tracking" do
           Fabricate(:chat_message, chat_channel: channel, thread: thread_1)
 
@@ -193,6 +207,8 @@ RSpec.describe Chat::ListChannelMessages do
         thread_1.add(user)
       end
 
+      it { is_expected.to be_a_success }
+
       it "returns tracking" do
         Fabricate(:chat_message, chat_channel: channel, thread: thread_1)
 
diff --git a/plugins/chat/spec/services/chat/list_channel_thread_messages_spec.rb b/plugins/chat/spec/services/chat/list_channel_thread_messages_spec.rb
index 62ab38b7023..059a93b7f01 100644
--- a/plugins/chat/spec/services/chat/list_channel_thread_messages_spec.rb
+++ b/plugins/chat/spec/services/chat/list_channel_thread_messages_spec.rb
@@ -29,6 +29,8 @@ RSpec.describe Chat::ListChannelThreadMessages do
     end
 
     context "when thread exists" do
+      it { is_expected.to run_successfully }
+
       it "finds the correct channel" do
         expect(result.thread).to eq(thread)
       end
@@ -44,7 +46,7 @@ RSpec.describe Chat::ListChannelThreadMessages do
       context "with system user" do
         fab!(:user) { Discourse.system_user }
 
-        it { is_expected.to be_a_success }
+        it { is_expected.to run_successfully }
       end
     end
   end
@@ -80,7 +82,7 @@ RSpec.describe Chat::ListChannelThreadMessages do
 
   context "when target_message_exists" do
     context "when no target_message_id is given" do
-      it { is_expected.to be_a_success }
+      it { is_expected.to run_successfully }
     end
 
     context "when target message is not found" do
@@ -95,7 +97,7 @@ RSpec.describe Chat::ListChannelThreadMessages do
       end
       let(:optional_params) { { target_message_id: target_message.id } }
 
-      it { is_expected.to be_a_success }
+      it { is_expected.to run_successfully }
     end
 
     context "when target message is trashed" do
@@ -115,13 +117,13 @@ RSpec.describe Chat::ListChannelThreadMessages do
           Fabricate(:chat_message, chat_channel: thread.channel, thread: thread, user: user)
         end
 
-        it { is_expected.to be_a_success }
+        it { is_expected.to run_successfully }
       end
 
       context "when user is admin" do
         fab!(:user) { Fabricate(:admin) }
 
-        it { is_expected.to be_a_success }
+        it { is_expected.to run_successfully }
       end
     end
   end
diff --git a/plugins/chat/spec/services/chat/list_user_channels_spec.rb b/plugins/chat/spec/services/chat/list_user_channels_spec.rb
index 2a94b39a5ea..8af4914a1e6 100644
--- a/plugins/chat/spec/services/chat/list_user_channels_spec.rb
+++ b/plugins/chat/spec/services/chat/list_user_channels_spec.rb
@@ -11,6 +11,8 @@ RSpec.describe Chat::ListUserChannels do
 
   before { channel_1.add(current_user) }
 
+  it { is_expected.to run_successfully }
+
   it "returns the structured data" do
     expect(result.structured[:post_allowed_category_ids]).to eq(nil)
     expect(result.structured[:unread_thread_overview]).to eq({})
diff --git a/plugins/chat/spec/services/chat/lookup_channel_threads_spec.rb b/plugins/chat/spec/services/chat/lookup_channel_threads_spec.rb
index a0618336903..05507470519 100644
--- a/plugins/chat/spec/services/chat/lookup_channel_threads_spec.rb
+++ b/plugins/chat/spec/services/chat/lookup_channel_threads_spec.rb
@@ -125,7 +125,7 @@ RSpec.describe ::Chat::LookupChannelThreads do
     describe "model - threads" do
       before { channel_1.add(current_user) }
 
-      it { is_expected.to be_a_success }
+      it { is_expected.to run_successfully }
 
       it "orders threads by the last reply created_at timestamp" do
         [
diff --git a/plugins/chat/spec/services/chat/lookup_thread_spec.rb b/plugins/chat/spec/services/chat/lookup_thread_spec.rb
index 873d30e0a4f..65d9c190d96 100644
--- a/plugins/chat/spec/services/chat/lookup_thread_spec.rb
+++ b/plugins/chat/spec/services/chat/lookup_thread_spec.rb
@@ -19,9 +19,7 @@ RSpec.describe Chat::LookupThread do
     let(:params) { { guardian: guardian, thread_id: thread.id, channel_id: thread.channel_id } }
 
     context "when all steps pass" do
-      it "sets the service result as successful" do
-        expect(result).to be_a_success
-      end
+      it { is_expected.to run_successfully }
 
       it "fetches the thread" do
         expect(result.thread).to eq(thread)
@@ -60,7 +58,7 @@ RSpec.describe Chat::LookupThread do
       context "when thread is forced" do
         before { thread.update!(force: true) }
 
-        it { is_expected.to be_a_success }
+        it { is_expected.to run_successfully }
       end
     end
   end
diff --git a/plugins/chat/spec/services/chat/lookup_user_threads_spec.rb b/plugins/chat/spec/services/chat/lookup_user_threads_spec.rb
index 940b715efb8..a20758af42d 100644
--- a/plugins/chat/spec/services/chat/lookup_user_threads_spec.rb
+++ b/plugins/chat/spec/services/chat/lookup_user_threads_spec.rb
@@ -5,6 +5,7 @@ RSpec.describe ::Chat::LookupUserThreads do
 
   fab!(:current_user) { Fabricate(:user) }
   fab!(:channel_1) { Fabricate(:chat_channel, threading_enabled: true) }
+  fab!(:thread_1) { Fabricate(:chat_thread, channel: channel_1, with_replies: 1) }
 
   let(:guardian) { Guardian.new(current_user) }
   let(:channel_id) { channel_1.id }
@@ -12,15 +13,15 @@ RSpec.describe ::Chat::LookupUserThreads do
   let(:offset) { 0 }
   let(:params) { { guardian: guardian, limit: limit, offset: offset } }
 
-  before { channel_1.add(current_user) }
+  before do
+    channel_1.add(current_user)
+    thread_1.add(current_user)
+  end
 
   context "when all steps pass" do
-    it "returns threads" do
-      thread_1 =
-        Fabricate(:chat_thread, channel: channel_1, with_replies: 1).tap do |thread|
-          thread.add(current_user)
-        end
+    it { is_expected.to run_successfully }
 
+    it "returns threads" do
       expect(result.threads).to eq([thread_1])
     end
 
@@ -65,6 +66,7 @@ RSpec.describe ::Chat::LookupUserThreads do
     it "can offset" do
       params[:offset] = 1
 
+      Chat::Thread.destroy_all
       threads =
         Fabricate
           .times(2, :chat_thread, channel: channel_1, with_replies: 1)
@@ -77,21 +79,16 @@ RSpec.describe ::Chat::LookupUserThreads do
     it "has a min offset" do
       params[:offset] = -99
 
-      threads =
-        Fabricate
-          .times(2, :chat_thread, channel: channel_1, with_replies: 1)
-          .each { |thread| thread.add(current_user) }
+      Chat::Thread.destroy_all
+      Fabricate
+        .times(2, :chat_thread, channel: channel_1, with_replies: 1)
+        .each { |thread| thread.add(current_user) }
 
       # 0 because we sort by last_message.created_at, so the last created thread is the first one
       expect(result.threads.length).to eq(2)
     end
 
     it "fetches tracking" do
-      thread_1 =
-        Fabricate(:chat_thread, channel: channel_1, with_replies: 1).tap do |thread|
-          thread.add(current_user)
-        end
-
       expect(result.tracking).to eq(
         ::Chat::TrackingStateReportQuery.call(
           guardian: current_user.guardian,
@@ -102,58 +99,39 @@ RSpec.describe ::Chat::LookupUserThreads do
     end
 
     it "fetches memberships" do
-      thread_1 =
-        Fabricate(:chat_thread, channel: channel_1, with_replies: 1).tap do |thread|
-          thread.add(current_user)
-        end
-
       expect(result.memberships).to eq([thread_1.membership_for(current_user)])
     end
 
     it "fetches participants" do
-      thread_1 =
-        Fabricate(:chat_thread, channel: channel_1, with_replies: 1).tap do |thread|
-          thread.add(current_user)
-        end
-
       expect(result.participants).to eq(
         ::Chat::ThreadParticipantQuery.call(thread_ids: [thread_1.id]),
       )
     end
 
     it "builds a load_more_url" do
-      Fabricate(:chat_thread, channel: channel_1, with_replies: 1).tap do |thread|
-        thread.add(current_user)
-      end
-
       expect(result.load_more_url).to eq("/chat/api/me/threads?limit=10&offset=10")
     end
   end
 
   it "doesn't return threads with no replies" do
-    thread_1 = Fabricate(:chat_thread, channel: channel_1)
-    thread_1.add(current_user)
+    Fabricate(:chat_thread, channel: channel_1).tap { _1.add(current_user) }
 
-    expect(result.threads).to eq([])
+    expect(result.threads).to eq([thread_1])
   end
 
   it "doesn't return threads with no membership" do
-    thread_1 = Fabricate(:chat_thread, channel: channel_1, with_replies: 1)
+    Fabricate(:chat_thread, channel: channel_1, with_replies: 1)
 
-    expect(result.threads).to eq([])
+    expect(result.threads).to eq([thread_1])
   end
 
   it "doesn't return threads when the channel has not threading enabled" do
     channel_1.update!(threading_enabled: false)
-    thread_1 = Fabricate(:chat_thread, channel: channel_1, with_replies: 1)
-    thread_1.add(current_user)
 
     expect(result.threads).to eq([])
   end
 
   it "doesn't return muted threads" do
-    thread_1 = Fabricate(:chat_thread, channel: channel_1, with_replies: 1)
-    thread_1.add(current_user)
     thread_1.membership_for(current_user).update!(
       notification_level: ::Chat::UserChatThreadMembership.notification_levels[:muted],
     )
@@ -163,16 +141,11 @@ RSpec.describe ::Chat::LookupUserThreads do
 
   it "doesn't return threads when the channel it not open" do
     channel_1.update!(status: Chat::Channel.statuses[:closed])
-    thread_1 = Fabricate(:chat_thread, channel: channel_1, with_replies: 1)
-    thread_1.add(current_user)
 
     expect(result.threads).to eq([])
   end
 
   it "returns threads from muted channels" do
-    thread_1 = Fabricate(:chat_thread, channel: channel_1, with_replies: 1)
-    thread_1.add(current_user)
-
     channel_1.membership_for(current_user).update!(muted: true)
 
     expect(result.threads).to eq([thread_1])
diff --git a/plugins/chat/spec/services/chat/mark_all_user_channels_read_spec.rb b/plugins/chat/spec/services/chat/mark_all_user_channels_read_spec.rb
index 346de0648a1..f7b1643618d 100644
--- a/plugins/chat/spec/services/chat/mark_all_user_channels_read_spec.rb
+++ b/plugins/chat/spec/services/chat/mark_all_user_channels_read_spec.rb
@@ -56,12 +56,10 @@ RSpec.describe Chat::MarkAllUserChannelsRead do
     context "when the user has no memberships" do
       let(:guardian) { Guardian.new(Fabricate(:user)) }
 
-      it "sets the service result as successful" do
-        expect(result).to be_a_success
-      end
+      it { is_expected.to run_successfully }
 
       it "returns the updated_memberships in context" do
-        expect(result.updated_memberships).to eq([])
+        expect(result.updated_memberships).to be_empty
       end
     end
 
@@ -96,9 +94,7 @@ RSpec.describe Chat::MarkAllUserChannelsRead do
         )
       end
 
-      it "sets the service result as successful" do
-        expect(result).to be_a_success
-      end
+      it { is_expected.to run_successfully }
 
       it "updates the last_read_message_ids" do
         result
diff --git a/plugins/chat/spec/services/chat/mark_thread_title_prompt_seen_spec.rb b/plugins/chat/spec/services/chat/mark_thread_title_prompt_seen_spec.rb
index 76cb6e9afe2..49b7b28cbff 100644
--- a/plugins/chat/spec/services/chat/mark_thread_title_prompt_seen_spec.rb
+++ b/plugins/chat/spec/services/chat/mark_thread_title_prompt_seen_spec.rb
@@ -23,9 +23,7 @@ RSpec.describe Chat::MarkThreadTitlePromptSeen do
     before { thread.update!(last_message: last_reply) }
 
     context "when all steps pass" do
-      it "sets the service result as successful" do
-        expect(result).to be_a_success
-      end
+      it { is_expected.to run_successfully }
 
       context "when the user is a member of the thread" do
         fab!(:membership) { thread.add(current_user) }
diff --git a/plugins/chat/spec/services/chat/restore_message_spec.rb b/plugins/chat/spec/services/chat/restore_message_spec.rb
index e7ce41d715c..3f8e4452f3c 100644
--- a/plugins/chat/spec/services/chat/restore_message_spec.rb
+++ b/plugins/chat/spec/services/chat/restore_message_spec.rb
@@ -41,9 +41,7 @@ RSpec.describe Chat::RestoreMessage do
       end
 
       context "when the user has permission to restore" do
-        it "sets the service result as successful" do
-          expect(result).to be_a_success
-        end
+        it { is_expected.to run_successfully }
 
         it "restores the message" do
           result
diff --git a/plugins/chat/spec/services/chat/search_chatable_spec.rb b/plugins/chat/spec/services/chat/search_chatable_spec.rb
index 116d9d50fe5..943f4723103 100644
--- a/plugins/chat/spec/services/chat/search_chatable_spec.rb
+++ b/plugins/chat/spec/services/chat/search_chatable_spec.rb
@@ -43,9 +43,7 @@ RSpec.describe Chat::SearchChatable do
     end
 
     context "when all steps pass" do
-      it "sets the service result as successful" do
-        expect(result).to be_a_success
-      end
+      it { is_expected.to run_successfully }
 
       it "cleans the term" do
         params[:term] = "#bob"
diff --git a/plugins/chat/spec/services/chat/stop_message_streaming_spec.rb b/plugins/chat/spec/services/chat/stop_message_streaming_spec.rb
index b3d1e3a3378..2a006a88235 100644
--- a/plugins/chat/spec/services/chat/stop_message_streaming_spec.rb
+++ b/plugins/chat/spec/services/chat/stop_message_streaming_spec.rb
@@ -19,7 +19,7 @@ RSpec.describe Chat::StopMessageStreaming do
     context "with valid params" do
       fab!(:current_user) { Fabricate(:admin) }
 
-      it { is_expected.to be_a_success }
+      it { is_expected.to run_successfully }
 
       it "updates the streaming attribute to false" do
         expect { result }.to change { message_1.reload.streaming }.to eq(false)
@@ -42,7 +42,7 @@ RSpec.describe Chat::StopMessageStreaming do
       context "when the user is a bot" do
         fab!(:current_user) { Discourse.system_user }
 
-        it { is_expected.to be_a_success }
+        it { is_expected.to run_successfully }
       end
     end
 
@@ -69,7 +69,7 @@ RSpec.describe Chat::StopMessageStreaming do
 
         before { params[:message_id] = reply.id }
 
-        it { is_expected.to be_a_success }
+        it { is_expected.to run_successfully }
       end
 
       context "when the OM is not from current user" do
@@ -89,13 +89,13 @@ RSpec.describe Chat::StopMessageStreaming do
         context "when current user is a bot" do
           fab!(:current_user) { Discourse.system_user }
 
-          it { is_expected.to be_a_success }
+          it { is_expected.to run_successfully }
         end
 
         context "when current user is an admin" do
           fab!(:current_user) { Fabricate(:admin) }
 
-          it { is_expected.to be_a_success }
+          it { is_expected.to run_successfully }
         end
       end
     end
@@ -112,13 +112,13 @@ RSpec.describe Chat::StopMessageStreaming do
       context "when current user is a bot" do
         fab!(:current_user) { Discourse.system_user }
 
-        it { is_expected.to be_a_success }
+        it { is_expected.to run_successfully }
       end
 
       context "when current user is an admin" do
         fab!(:current_user) { Fabricate(:admin) }
 
-        it { is_expected.to be_a_success }
+        it { is_expected.to run_successfully }
       end
     end
   end
diff --git a/plugins/chat/spec/services/chat/trash_channel_spec.rb b/plugins/chat/spec/services/chat/trash_channel_spec.rb
index 120394e9d8b..d5fa1009636 100644
--- a/plugins/chat/spec/services/chat/trash_channel_spec.rb
+++ b/plugins/chat/spec/services/chat/trash_channel_spec.rb
@@ -25,9 +25,7 @@ RSpec.describe(Chat::TrashChannel) do
     context "when user is allowed to perform the action" do
       fab!(:current_user) { Fabricate(:admin) }
 
-      it "sets the service result as successful" do
-        expect(result).to be_a_success
-      end
+      it { is_expected.to run_successfully }
 
       it "trashes the channel" do
         expect(result[:channel]).to be_trashed
diff --git a/plugins/chat/spec/services/chat/trash_message_spec.rb b/plugins/chat/spec/services/chat/trash_message_spec.rb
index a1f6a72c6fb..b7bc327a458 100644
--- a/plugins/chat/spec/services/chat/trash_message_spec.rb
+++ b/plugins/chat/spec/services/chat/trash_message_spec.rb
@@ -34,9 +34,7 @@ RSpec.describe Chat::TrashMessage do
       end
 
       context "when the user has permission to delete" do
-        it "sets the service result as successful" do
-          expect(result).to be_a_success
-        end
+        it { is_expected.to run_successfully }
 
         it "trashes the message" do
           result
diff --git a/plugins/chat/spec/services/chat/trash_messages_spec.rb b/plugins/chat/spec/services/chat/trash_messages_spec.rb
index 034fa33a866..10ac0e19c9d 100644
--- a/plugins/chat/spec/services/chat/trash_messages_spec.rb
+++ b/plugins/chat/spec/services/chat/trash_messages_spec.rb
@@ -40,9 +40,7 @@ RSpec.describe Chat::TrashMessages do
       end
 
       context "when the user has permission to delete" do
-        it "sets the service result as successful" do
-          expect(result).to be_a_success
-        end
+        it { is_expected.to run_successfully }
 
         it "trashes the messages" do
           result
diff --git a/plugins/chat/spec/services/chat/unfollow_channel_spec.rb b/plugins/chat/spec/services/chat/unfollow_channel_spec.rb
index 71aacd317c0..3199852f132 100644
--- a/plugins/chat/spec/services/chat/unfollow_channel_spec.rb
+++ b/plugins/chat/spec/services/chat/unfollow_channel_spec.rb
@@ -24,6 +24,8 @@ RSpec.describe Chat::UnfollowChannel do
       context "with existing membership" do
         before { channel_1.add(current_user) }
 
+        it { is_expected.to run_successfully }
+
         it "unfollows the channel" do
           membership = channel_1.membership_for(current_user)
 
@@ -32,6 +34,8 @@ RSpec.describe Chat::UnfollowChannel do
       end
 
       context "with no existing membership" do
+        it { is_expected.to run_successfully }
+
         it "does nothing" do
           expect { result }.to_not change { Chat::UserChatChannelMembership }
         end
diff --git a/plugins/chat/spec/services/chat/update_channel_spec.rb b/plugins/chat/spec/services/chat/update_channel_spec.rb
index 92729aa778f..2f6f49cb536 100644
--- a/plugins/chat/spec/services/chat/update_channel_spec.rb
+++ b/plugins/chat/spec/services/chat/update_channel_spec.rb
@@ -31,9 +31,7 @@ RSpec.describe Chat::UpdateChannel do
           .first
       end
 
-      it "sets the service result as successful" do
-        expect(result).to be_a_success
-      end
+      it { is_expected.to run_successfully }
 
       it "updates the channel accordingly" do
         result
diff --git a/plugins/chat/spec/services/chat/update_channel_status_spec.rb b/plugins/chat/spec/services/chat/update_channel_status_spec.rb
index e5ac9b92a8a..b3ccccec7d6 100644
--- a/plugins/chat/spec/services/chat/update_channel_status_spec.rb
+++ b/plugins/chat/spec/services/chat/update_channel_status_spec.rb
@@ -42,9 +42,7 @@ RSpec.describe(Chat::UpdateChannelStatus) do
   context "when status is allowed" do
     let(:status) { "closed" }
 
-    it "sets the service result as successful" do
-      expect(result).to be_a_success
-    end
+    it { is_expected.to run_successfully }
 
     it "changes the status" do
       result
diff --git a/plugins/chat/spec/services/chat/update_message_spec.rb b/plugins/chat/spec/services/chat/update_message_spec.rb
index 6b39f5d9aa9..edda8fa1b8e 100644
--- a/plugins/chat/spec/services/chat/update_message_spec.rb
+++ b/plugins/chat/spec/services/chat/update_message_spec.rb
@@ -879,9 +879,7 @@ RSpec.describe Chat::UpdateMessage do
     end
 
     context "when all steps pass" do
-      it "sets the service result as successful" do
-        expect(result).to run_service_successfully
-      end
+      it { is_expected.to run_successfully }
 
       it "updates the message" do
         expect(result.message.message).to eq("new")
diff --git a/plugins/chat/spec/services/chat/update_thread_notification_settings_spec.rb b/plugins/chat/spec/services/chat/update_thread_notification_settings_spec.rb
index 671dd9d088b..c026504b399 100644
--- a/plugins/chat/spec/services/chat/update_thread_notification_settings_spec.rb
+++ b/plugins/chat/spec/services/chat/update_thread_notification_settings_spec.rb
@@ -29,9 +29,7 @@ RSpec.describe Chat::UpdateThreadNotificationSettings do
     before { thread.update!(last_message: last_reply) }
 
     context "when all steps pass" do
-      it "sets the service result as successful" do
-        expect(result).to be_a_success
-      end
+      it { is_expected.to run_successfully }
 
       context "when the user is a member of the thread" do
         fab!(:membership) { thread.add(current_user) }
diff --git a/plugins/chat/spec/services/chat/update_thread_spec.rb b/plugins/chat/spec/services/chat/update_thread_spec.rb
index 6d69765b77e..5be66c740f2 100644
--- a/plugins/chat/spec/services/chat/update_thread_spec.rb
+++ b/plugins/chat/spec/services/chat/update_thread_spec.rb
@@ -19,9 +19,7 @@ RSpec.describe Chat::UpdateThread do
     let(:params) { { guardian: guardian, thread_id: thread.id, title: title } }
 
     context "when all steps pass" do
-      it "sets the service result as successful" do
-        expect(result).to be_a_success
-      end
+      it { is_expected.to run_successfully }
 
       it "updates the title of the thread" do
         result
diff --git a/plugins/chat/spec/services/chat/update_user_channel_last_read_spec.rb b/plugins/chat/spec/services/chat/update_user_channel_last_read_spec.rb
index 854890c0131..9122305685a 100644
--- a/plugins/chat/spec/services/chat/update_user_channel_last_read_spec.rb
+++ b/plugins/chat/spec/services/chat/update_user_channel_last_read_spec.rb
@@ -91,9 +91,7 @@ RSpec.describe Chat::UpdateUserChannelLastRead do
           )
         end
 
-        it "sets the service result as successful" do
-          expect(result).to be_a_success
-        end
+        it { is_expected.to run_successfully }
 
         it "updates the last_read message id" do
           expect { result }.to change { membership.reload.last_read_message_id }.to(message_1.id)
diff --git a/plugins/chat/spec/services/chat/update_user_thread_last_read_spec.rb b/plugins/chat/spec/services/chat/update_user_thread_last_read_spec.rb
index e94a8de922d..5802478dd60 100644
--- a/plugins/chat/spec/services/chat/update_user_thread_last_read_spec.rb
+++ b/plugins/chat/spec/services/chat/update_user_thread_last_read_spec.rb
@@ -55,9 +55,7 @@ RSpec.describe Chat::UpdateUserThreadLastRead do
     end
 
     context "when params are valid" do
-      it "sets the service result as successful" do
-        expect(result).to be_a_success
-      end
+      it { is_expected.to run_successfully }
 
       it "publishes new last read to clients" do
         messages = MessageBus.track_publish { result }
diff --git a/plugins/chat/spec/services/chat/upsert_draft_spec.rb b/plugins/chat/spec/services/chat/upsert_draft_spec.rb
index b4fd12f3286..5727730c255 100644
--- a/plugins/chat/spec/services/chat/upsert_draft_spec.rb
+++ b/plugins/chat/spec/services/chat/upsert_draft_spec.rb
@@ -29,6 +29,8 @@ RSpec.describe Chat::UpsertDraft do
     end
 
     context "when all steps pass" do
+      it { is_expected.to run_successfully }
+
       it "creates draft if data provided and not existing draft" do
         params[:data] = MultiJson.dump(message: "a")
 
diff --git a/spec/services/flags/create_flag_spec.rb b/spec/services/flags/create_flag_spec.rb
index 2a411697d57..620cd9bbce6 100644
--- a/spec/services/flags/create_flag_spec.rb
+++ b/spec/services/flags/create_flag_spec.rb
@@ -69,11 +69,10 @@ RSpec.describe(Flags::CreateFlag) do
         OpenStruct.new(enabled?: true),
       )
     end
+
     after { Flag.destroy_by(name: "custom flag name") }
 
-    it "sets the service result as successful" do
-      expect(result).to be_a_success
-    end
+    it { is_expected.to run_successfully }
 
     it "creates the flag" do
       result
diff --git a/spec/services/flags/destroy_flag_spec.rb b/spec/services/flags/destroy_flag_spec.rb
index cfc2e8b4238..6519e363ddd 100644
--- a/spec/services/flags/destroy_flag_spec.rb
+++ b/spec/services/flags/destroy_flag_spec.rb
@@ -1,10 +1,10 @@
 # frozen_string_literal: true
 
 RSpec.describe(Flags::DestroyFlag) do
-  fab!(:flag)
-
   subject(:result) { described_class.call(id: flag.id, guardian: current_user.guardian) }
 
+  fab!(:flag)
+
   after { flag.destroy }
 
   context "when user is not allowed to perform the action" do
@@ -16,6 +16,8 @@ RSpec.describe(Flags::DestroyFlag) do
   context "when user is allowed to perform the action" do
     fab!(:current_user) { Fabricate(:admin) }
 
+    it { is_expected.to run_successfully }
+
     it "sets the service result as successful" do
       expect(result).to be_a_success
     end
diff --git a/spec/services/flags/reorder_flag_spec.rb b/spec/services/flags/reorder_flag_spec.rb
index 1eff5f97d4a..cf49a1dcb40 100644
--- a/spec/services/flags/reorder_flag_spec.rb
+++ b/spec/services/flags/reorder_flag_spec.rb
@@ -35,9 +35,7 @@ RSpec.describe(Flags::ReorderFlag) do
       described_class.call(flag_id: flag.id, guardian: current_user.guardian, direction: "down")
     end
 
-    it "sets the service result as successful" do
-      expect(result).to be_a_success
-    end
+    it { is_expected.to run_successfully }
 
     it "moves the flag" do
       expect(Flag.order(:position).map(&:name)).to eq(
diff --git a/spec/services/flags/toggle_flag_spec.rb b/spec/services/flags/toggle_flag_spec.rb
index 4eba4502b58..fea69c40527 100644
--- a/spec/services/flags/toggle_flag_spec.rb
+++ b/spec/services/flags/toggle_flag_spec.rb
@@ -16,9 +16,7 @@ RSpec.describe(Flags::ToggleFlag) do
 
     after { flag.reload.update!(enabled: true) }
 
-    it "sets the service result as successful" do
-      expect(result).to be_a_success
-    end
+    it { is_expected.to run_successfully }
 
     it "toggles the flag" do
       expect(result[:flag].enabled).to be false
diff --git a/spec/services/flags/update_flag_spec.rb b/spec/services/flags/update_flag_spec.rb
index 8292c05ae45..3fb7faacf15 100644
--- a/spec/services/flags/update_flag_spec.rb
+++ b/spec/services/flags/update_flag_spec.rb
@@ -1,8 +1,6 @@
 # frozen_string_literal: true
 
 RSpec.describe(Flags::UpdateFlag) do
-  fab!(:flag)
-
   subject(:result) do
     described_class.call(
       id: flag.id,
@@ -15,6 +13,8 @@ RSpec.describe(Flags::UpdateFlag) do
     )
   end
 
+  fab!(:flag)
+
   after { flag.destroy }
 
   let(:name) { "edited custom flag name" }
@@ -67,9 +67,7 @@ RSpec.describe(Flags::UpdateFlag) do
   context "when user is allowed to perform the action" do
     fab!(:current_user) { Fabricate(:admin) }
 
-    it "sets the service result as successful" do
-      expect(result).to be_a_success
-    end
+    it { is_expected.to run_successfully }
 
     it "updates the flag" do
       result
diff --git a/spec/services/update_site_setting_spec.rb b/spec/services/update_site_setting_spec.rb
index 7040b6dea19..fda557efc8f 100644
--- a/spec/services/update_site_setting_spec.rb
+++ b/spec/services/update_site_setting_spec.rb
@@ -1,71 +1,74 @@
 # frozen_string_literal: true
 
-describe(UpdateSiteSetting) do
-  fab!(:admin)
+RSpec.describe UpdateSiteSetting do
+  subject(:result) { described_class.call(params) }
 
-  def call_service(name, value, user: admin, allow_changing_hidden: false)
-    described_class.call(
-      setting_name: name,
-      new_value: value,
-      guardian: user.guardian,
-      allow_changing_hidden:,
-    )
+  describe described_class::Contract, type: :model do
+    subject(:contract) { described_class.new }
+
+    it { is_expected.to validate_presence_of :setting_name }
   end
 
-  context "when setting_name is blank" do
-    it "fails the service contract" do
-      expect(call_service(nil, "blah whatever")).to fail_a_contract
+  fab!(:admin)
+  let(:params) { { setting_name:, new_value:, guardian:, allow_changing_hidden: } }
+  let(:setting_name) { :title }
+  let(:new_value) { "blah whatever" }
+  let(:guardian) { admin.guardian }
+  let(:allow_changing_hidden) { false }
 
-      expect(call_service(:"", "blah whatever")).to fail_a_contract
-    end
+  context "when setting_name is blank" do
+    let(:setting_name) { nil }
+
+    it { is_expected.to fail_a_contract }
   end
 
   context "when a non-admin user tries to change a setting" do
-    it "fails the current_user_is_admin policy" do
-      expect(call_service(:title, "some new title", user: Fabricate(:moderator))).to fail_a_policy(
-        :current_user_is_admin,
-      )
-      expect(SiteSetting.title).not_to eq("some new title")
-    end
+    let(:guardian) { Guardian.new }
+
+    it { is_expected.to fail_a_policy(:current_user_is_admin) }
   end
 
   context "when the user changes a hidden setting" do
+    let(:setting_name) { :max_category_nesting }
+    let(:new_value) { 3 }
+
     context "when allow_changing_hidden is false" do
-      it "fails the setting_is_visible policy" do
-        expect(call_service(:max_category_nesting, 3)).to fail_a_policy(:setting_is_visible)
-        expect(SiteSetting.max_category_nesting).not_to eq(3)
-      end
+      it { is_expected.to fail_a_policy(:setting_is_visible) }
     end
 
     context "when allow_changing_hidden is true" do
+      let(:allow_changing_hidden) { true }
+
+      it { is_expected.to run_successfully }
+
       it "updates the specified setting" do
-        expect(call_service(:max_category_nesting, 3, allow_changing_hidden: true)).to be_success
-        expect(SiteSetting.max_category_nesting).to eq(3)
+        expect { result }.to change { SiteSetting.max_category_nesting }.to(3)
       end
     end
   end
 
   context "when the user changes a visible setting" do
+    let(:new_value) { "hello this is title" }
+
+    it { is_expected.to run_successfully }
+
     it "updates the specified setting" do
-      expect(call_service(:title, "hello this is title")).to be_success
-      expect(SiteSetting.title).to eq("hello this is title")
-    end
-
-    it "cleans up the new setting value before using it" do
-      expect(call_service(:suggested_topics, "308viu")).to be_success
-      expect(SiteSetting.suggested_topics).to eq(308)
-
-      expect(call_service(:max_image_size_kb, "8zf843")).to be_success
-      expect(SiteSetting.max_image_size_kb).to eq(8843)
+      expect { result }.to change { SiteSetting.title }.to(new_value)
     end
 
     it "creates an entry in the staff action logs" do
-      expect do expect(call_service(:max_image_size_kb, 44_543)).to be_success end.to change {
-        UserHistory.where(
-          action: UserHistory.actions[:change_site_setting],
-          subject: "max_image_size_kb",
-        ).count
+      expect { result }.to change {
+        UserHistory.where(action: UserHistory.actions[:change_site_setting], subject: "title").count
       }.by(1)
     end
+
+    context "when value needs cleanup" do
+      let(:setting_name) { :max_image_size_kb }
+      let(:new_value) { "8zf843" }
+
+      it "cleans up the new setting value before using it" do
+        expect { result }.to change { SiteSetting.max_image_size_kb }.to(8843)
+      end
+    end
   end
 end
diff --git a/spec/support/service_matchers.rb b/spec/support/service_matchers.rb
index 7ef1571af40..d2945b8e353 100644
--- a/spec/support/service_matchers.rb
+++ b/spec/support/service_matchers.rb
@@ -10,8 +10,24 @@ module ServiceMatchers
     end
 
     def failure_message
+      message = "Expected the service to succeed but it failed."
+      error_message_with_inspection(message)
+    end
+
+    def failure_message_when_negated
+      message = "Expected the service to fail but it succeeded."
+      error_message_with_inspection(message)
+    end
+
+    def description
+      "run the service successfully"
+    end
+
+    private
+
+    def error_message_with_inspection(message)
       inspector = StepsInspector.new(result)
-      "Expected to run the service sucessfully but failed:\n\n#{inspector.inspect}\n\n#{inspector.error}"
+      "#{message}\n\n#{inspector.inspect}\n\n#{inspector.error}"
     end
   end
 
@@ -137,7 +153,7 @@ module ServiceMatchers
     FailStep.new(name)
   end
 
-  def run_service_successfully
+  def run_successfully
     RunServiceSuccessfully.new
   end