From f79dd5c8b5d0932a6211b329e65ed3ccfcac9638 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lo=C3=AFc=20Guitaut?= <loic@discourse.org>
Date: Mon, 21 Oct 2024 15:37:02 +0200
Subject: [PATCH] DEV: Stop injecting a service result object in the caller
 object

Currently, when calling a service with its block form, a `#result`
method is automatically created on the caller object. Even if it never
clashed so far, this could happen.

This patch removes that method, and instead use a more classical way of
doing things: the result object is now provided as an argument to the
main block. This means if we need to access the result object in an
outcome block, it will be done like this from now on:
```ruby
MyService.call(params) do |result|
  on_success do
    # do something with the result object
    do_something(result)
  end
end
```

In the same vein, this patch introduces the ability to match keys from
the result object in the outcome blocks, like we already do with step
definitions in a service. For example:
```ruby
on_success do |model:, contract:|
  do_something(model, contract)
end
```
Instead of
```ruby
on_success do
  do_something(result.model, result.contract)
end
```
---
 .../admin/config/flags_controller.rb          |  8 +-
 .../admin/site_settings_controller.rb         |  4 +-
 app/controllers/admin/users_controller.rb     | 18 ++--
 app/services/experiments/toggle.rb            |  3 -
 lib/service/runner.rb                         | 14 +--
 .../chat/api/channel_messages_controller.rb   |  8 +-
 .../api/channel_thread_messages_controller.rb |  2 +-
 .../chat/api/channel_threads_controller.rb    | 37 ++++----
 ..._user_notifications_settings_controller.rb | 12 +--
 ...rrent_user_title_prompt_seen_controller.rb | 12 +--
 .../chat/api/channels_controller.rb           | 23 ++---
 ...rent_user_membership_follows_controller.rb | 12 +--
 .../chat/api/channels_read_controller.rb      |  4 +-
 .../chat/api/channels_status_controller.rb    |  2 +-
 .../chat/api/chatables_controller.rb          |  2 +-
 .../api/current_user_channels_controller.rb   |  6 +-
 .../api/current_user_threads_controller.rb    | 18 ++--
 .../chat/api/direct_messages_controller.rb    |  9 +-
 .../regular/chat/auto_join_channel_batch.rb   |  4 +-
 plugins/chat/lib/chat_sdk/channel.rb          |  2 +-
 plugins/chat/lib/chat_sdk/message.rb          |  4 +-
 plugins/chat/lib/chat_sdk/thread.rb           |  4 +-
 plugins/chat/spec/plugin_helper.rb            | 86 +++++++++----------
 .../chat/api/current_user_threads_spec.rb     | 12 ++-
 spec/lib/service/runner_spec.rb               | 50 ++++++-----
 25 files changed, 168 insertions(+), 188 deletions(-)

diff --git a/app/controllers/admin/config/flags_controller.rb b/app/controllers/admin/config/flags_controller.rb
index 6e46a7e0857..0d83da70da9 100644
--- a/app/controllers/admin/config/flags_controller.rb
+++ b/app/controllers/admin/config/flags_controller.rb
@@ -27,9 +27,9 @@ class Admin::Config::FlagsController < Admin::AdminController
 
   def create
     Flags::CreateFlag.call(service_params) do
-      on_success do
+      on_success do |flag:|
         Discourse.request_refresh!
-        render json: result.flag, serializer: FlagSerializer, used_flag_ids: Flag.used_flag_ids
+        render json: flag, serializer: FlagSerializer, used_flag_ids: Flag.used_flag_ids
       end
       on_failure { render(json: failed_json, status: 422) }
       on_failed_policy(:invalid_access) { raise Discourse::InvalidAccess }
@@ -42,9 +42,9 @@ class Admin::Config::FlagsController < Admin::AdminController
 
   def update
     Flags::UpdateFlag.call(service_params) do
-      on_success do
+      on_success do |flag:|
         Discourse.request_refresh!
-        render json: result.flag, serializer: FlagSerializer, used_flag_ids: Flag.used_flag_ids
+        render json: flag, serializer: FlagSerializer, used_flag_ids: Flag.used_flag_ids
       end
       on_failure { render(json: failed_json, status: 422) }
       on_model_not_found(:message) { raise Discourse::NotFound }
diff --git a/app/controllers/admin/site_settings_controller.rb b/app/controllers/admin/site_settings_controller.rb
index eb86c9789d2..cd80e41d669 100644
--- a/app/controllers/admin/site_settings_controller.rb
+++ b/app/controllers/admin/site_settings_controller.rb
@@ -40,9 +40,9 @@ class Admin::SiteSettingsController < Admin::AdminController
     previous_value = value_or_default(SiteSetting.get(id)) if update_existing_users
 
     SiteSetting::Update.call(service_params.merge(setting_name: id, new_value: value)) do
-      on_success do
+      on_success do |contract:|
         if update_existing_users
-          SiteSettingUpdateExistingUsers.call(id, result.contract.new_value, previous_value)
+          SiteSettingUpdateExistingUsers.call(id, contract.new_value, previous_value)
         end
         render body: nil
       end
diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb
index 8a2f668414d..ca0223dd5b0 100644
--- a/app/controllers/admin/users_controller.rb
+++ b/app/controllers/admin/users_controller.rb
@@ -121,13 +121,13 @@ class Admin::UsersController < Admin::StaffController
 
   def suspend
     User::Suspend.call(service_params) do
-      on_success do
+      on_success do |contract:, user:, full_reason:|
         render_json_dump(
           suspension: {
-            suspend_reason: result.contract.reason,
-            full_suspend_reason: result.full_reason,
-            suspended_till: result.user.suspended_till,
-            suspended_at: result.user.suspended_at,
+            suspend_reason: contract.reason,
+            full_suspend_reason: full_reason,
+            suspended_till: user.suspended_till,
+            suspended_at: user.suspended_at,
             suspended_by: BasicUserSerializer.new(current_user, root: false).as_json,
           },
         )
@@ -316,13 +316,13 @@ class Admin::UsersController < Admin::StaffController
 
   def silence
     User::Silence.call(service_params) do
-      on_success do
+      on_success do |full_reason:, user:|
         render_json_dump(
           silence: {
             silenced: true,
-            silence_reason: result.full_reason,
-            silenced_till: result.user.silenced_till,
-            silenced_at: result.user.silenced_at,
+            silence_reason: full_reason,
+            silenced_till: user.silenced_till,
+            silenced_at: user.silenced_at,
             silenced_by: BasicUserSerializer.new(current_user, root: false).as_json,
           },
         )
diff --git a/app/services/experiments/toggle.rb b/app/services/experiments/toggle.rb
index 3746b82f53a..84b6b709016 100644
--- a/app/services/experiments/toggle.rb
+++ b/app/services/experiments/toggle.rb
@@ -4,14 +4,11 @@ class Experiments::Toggle
   include Service::Base
 
   policy :current_user_is_admin
-
   contract do
     attribute :setting_name, :string
     validates :setting_name, presence: true
   end
-
   policy :setting_is_available
-
   transaction { step :toggle }
 
   private
diff --git a/lib/service/runner.rb b/lib/service/runner.rb
index 794a3cc4c36..768aa5192e4 100644
--- a/lib/service/runner.rb
+++ b/lib/service/runner.rb
@@ -97,8 +97,6 @@ class Service::Runner
   # @!visibility private
   attr_reader :service, :object, :dependencies
 
-  delegate :result, to: :object
-
   # @!visibility private
   def initialize(service, object, dependencies)
     @service = service
@@ -117,8 +115,7 @@ class Service::Runner
 
   # @!visibility private
   def call(&block)
-    instance_eval(&block)
-    setup_and_run_service
+    instance_exec(result, &block)
     # Always have `on_failure` as the last action
     (
       actions
@@ -132,12 +129,8 @@ class Service::Runner
 
   attr_reader :actions
 
-  def setup_and_run_service
-    runner = self
-    object.instance_eval do
-      def result = @_result
-      @_result = runner.service.call(runner.dependencies)
-    end
+  def result
+    @result ||= service.call(dependencies)
   end
 
   def failure_for?(key)
@@ -151,6 +144,7 @@ class Service::Runner
       -> do
         object.instance_exec(
           result[[*action[:key], args.first || action[:default_name]].join(".")],
+          **result.slice(*block.parameters.filter_map { _1.last if _1.first == :keyreq }),
           &block
         )
       end,
diff --git a/plugins/chat/app/controllers/chat/api/channel_messages_controller.rb b/plugins/chat/app/controllers/chat/api/channel_messages_controller.rb
index e706614ec55..d988ff628b7 100644
--- a/plugins/chat/app/controllers/chat/api/channel_messages_controller.rb
+++ b/plugins/chat/app/controllers/chat/api/channel_messages_controller.rb
@@ -2,7 +2,7 @@
 
 class Chat::Api::ChannelMessagesController < Chat::ApiController
   def index
-    ::Chat::ListChannelMessages.call(service_params) do
+    ::Chat::ListChannelMessages.call(service_params) do |result|
       on_success { render_serialized(result, ::Chat::MessagesSerializer, root: false) }
       on_failure { render(json: failed_json, status: 422) }
       on_failed_policy(:can_view_channel) { raise Discourse::InvalidAccess }
@@ -52,7 +52,7 @@ class Chat::Api::ChannelMessagesController < Chat::ApiController
 
   def update
     Chat::UpdateMessage.call(service_params) do
-      on_success { render json: success_json.merge(message_id: result[:message].id) }
+      on_success { |message:| render json: success_json.merge(message_id: message.id) }
       on_failure { render(json: failed_json, status: 422) }
       on_model_not_found(:message) { raise Discourse::NotFound }
       on_model_errors(:message) do |model|
@@ -69,7 +69,9 @@ class Chat::Api::ChannelMessagesController < Chat::ApiController
 
     # users can't force a thread through JSON API
     Chat::CreateMessage.call(service_params.merge(force_thread: false)) do
-      on_success { render json: success_json.merge(message_id: result[:message_instance].id) }
+      on_success do |message_instance:|
+        render json: success_json.merge(message_id: message_instance.id)
+      end
       on_failure { render(json: failed_json, status: 422) }
       on_failed_policy(:no_silenced_user) { raise Discourse::InvalidAccess }
       on_model_not_found(:channel) { raise Discourse::NotFound }
diff --git a/plugins/chat/app/controllers/chat/api/channel_thread_messages_controller.rb b/plugins/chat/app/controllers/chat/api/channel_thread_messages_controller.rb
index 27fc14ce4b2..d5b4e5a4331 100644
--- a/plugins/chat/app/controllers/chat/api/channel_thread_messages_controller.rb
+++ b/plugins/chat/app/controllers/chat/api/channel_thread_messages_controller.rb
@@ -2,7 +2,7 @@
 
 class Chat::Api::ChannelThreadMessagesController < Chat::ApiController
   def index
-    ::Chat::ListChannelThreadMessages.call(service_params) do
+    ::Chat::ListChannelThreadMessages.call(service_params) do |result|
       on_success do
         render_serialized(
           result,
diff --git a/plugins/chat/app/controllers/chat/api/channel_threads_controller.rb b/plugins/chat/app/controllers/chat/api/channel_threads_controller.rb
index 52d4daf97f7..491b14911da 100644
--- a/plugins/chat/app/controllers/chat/api/channel_threads_controller.rb
+++ b/plugins/chat/app/controllers/chat/api/channel_threads_controller.rb
@@ -3,16 +3,16 @@
 class Chat::Api::ChannelThreadsController < Chat::ApiController
   def index
     ::Chat::LookupChannelThreads.call(service_params) do
-      on_success do
+      on_success do |threads:, channel:, tracking:, memberships:, load_more_url:, participants:|
         render_serialized(
           ::Chat::ThreadsView.new(
             user: guardian.user,
-            threads: result.threads,
-            channel: result.channel,
-            tracking: result.tracking,
-            memberships: result.memberships,
-            load_more_url: result.load_more_url,
-            threads_participants: result.participants,
+            threads_participants: participants,
+            threads:,
+            channel:,
+            tracking:,
+            memberships:,
+            load_more_url:,
           ),
           ::Chat::ThreadListSerializer,
           root: false,
@@ -31,15 +31,15 @@ class Chat::Api::ChannelThreadsController < Chat::ApiController
 
   def show
     ::Chat::LookupThread.call(service_params) do
-      on_success do
+      on_success do |thread:, membership:, participants:|
         render_serialized(
-          result.thread,
+          thread,
           ::Chat::ThreadSerializer,
           root: "thread",
-          membership: result.membership,
           include_thread_preview: true,
           include_thread_original_message: true,
-          participants: result.participants,
+          membership:,
+          participants:,
         )
       end
       on_failed_policy(:invalid_access) { raise Discourse::InvalidAccess }
@@ -58,8 +58,8 @@ class Chat::Api::ChannelThreadsController < Chat::ApiController
       on_failed_policy(:can_view_channel) { raise Discourse::InvalidAccess }
       on_failed_policy(:can_edit_thread) { raise Discourse::InvalidAccess }
       on_model_not_found(:thread) { raise Discourse::NotFound }
-      on_failed_step(:update) do
-        render json: failed_json.merge(errors: [result["result.step.update"].error]), status: 422
+      on_failed_step(:update) do |step|
+        render json: failed_json.merge(errors: [step.error]), status: 422
       end
       on_success { render(json: success_json) }
       on_failure { render(json: failed_json, status: 422) }
@@ -71,20 +71,19 @@ class Chat::Api::ChannelThreadsController < Chat::ApiController
 
   def create
     ::Chat::CreateThread.call(service_params) do
-      on_success do
+      on_success do |thread:, membership:|
         render_serialized(
-          result.thread,
+          thread,
           ::Chat::ThreadSerializer,
           root: false,
-          membership: result.membership,
           include_thread_original_message: true,
+          membership:,
         )
       end
       on_failed_policy(:threading_enabled_for_channel) { raise Discourse::NotFound }
       on_failed_policy(:can_view_channel) { raise Discourse::InvalidAccess }
-      on_failed_step(:create_thread) do
-        render json: failed_json.merge(errors: [result["result.step.create_thread"].error]),
-               status: 422
+      on_failed_step(:create_thread) do |step|
+        render json: failed_json.merge(errors: [step.error]), status: 422
       end
       on_failure { render(json: failed_json, status: 422) }
       on_failed_contract do |contract|
diff --git a/plugins/chat/app/controllers/chat/api/channel_threads_current_user_notifications_settings_controller.rb b/plugins/chat/app/controllers/chat/api/channel_threads_current_user_notifications_settings_controller.rb
index 7880827d19b..abce511e4e4 100644
--- a/plugins/chat/app/controllers/chat/api/channel_threads_current_user_notifications_settings_controller.rb
+++ b/plugins/chat/app/controllers/chat/api/channel_threads_current_user_notifications_settings_controller.rb
@@ -3,17 +3,13 @@
 class Chat::Api::ChannelThreadsCurrentUserNotificationsSettingsController < Chat::ApiController
   def update
     Chat::UpdateThreadNotificationSettings.call(service_params) do
+      on_success do |membership:|
+        render_serialized(membership, Chat::BaseThreadMembershipSerializer, root: "membership")
+      end
+      on_failure { render(json: failed_json, status: 422) }
       on_failed_policy(:threading_enabled_for_channel) { raise Discourse::NotFound }
       on_failed_policy(:can_view_channel) { raise Discourse::InvalidAccess }
       on_model_not_found(:thread) { raise Discourse::NotFound }
-      on_success do
-        render_serialized(
-          result.membership,
-          Chat::BaseThreadMembershipSerializer,
-          root: "membership",
-        )
-      end
-      on_failure { render(json: failed_json, status: 422) }
       on_failed_contract do |contract|
         render(json: failed_json.merge(errors: contract.errors.full_messages), status: 400)
       end
diff --git a/plugins/chat/app/controllers/chat/api/channel_threads_current_user_title_prompt_seen_controller.rb b/plugins/chat/app/controllers/chat/api/channel_threads_current_user_title_prompt_seen_controller.rb
index b37fd1a4bc3..6a740e9a5b5 100644
--- a/plugins/chat/app/controllers/chat/api/channel_threads_current_user_title_prompt_seen_controller.rb
+++ b/plugins/chat/app/controllers/chat/api/channel_threads_current_user_title_prompt_seen_controller.rb
@@ -3,17 +3,13 @@
 class Chat::Api::ChannelThreadsCurrentUserTitlePromptSeenController < Chat::ApiController
   def update
     Chat::MarkThreadTitlePromptSeen.call(service_params) do
+      on_success do |membership:|
+        render_serialized(membership, Chat::BaseThreadMembershipSerializer, root: "membership")
+      end
+      on_failure { render(json: failed_json, status: 422) }
       on_failed_policy(:threading_enabled_for_channel) { raise Discourse::NotFound }
       on_failed_policy(:can_view_channel) { raise Discourse::InvalidAccess }
       on_model_not_found(:thread) { raise Discourse::NotFound }
-      on_success do
-        render_serialized(
-          result.membership,
-          Chat::BaseThreadMembershipSerializer,
-          root: "membership",
-        )
-      end
-      on_failure { render(json: failed_json, status: 422) }
       on_failed_contract do |contract|
         render(json: failed_json.merge(errors: contract.errors.full_messages), status: 400)
       end
diff --git a/plugins/chat/app/controllers/chat/api/channels_controller.rb b/plugins/chat/app/controllers/chat/api/channels_controller.rb
index 69a0058bf1b..cd5f203f769 100644
--- a/plugins/chat/app/controllers/chat/api/channels_controller.rb
+++ b/plugins/chat/app/controllers/chat/api/channels_controller.rb
@@ -57,24 +57,19 @@ class Chat::Api::ChannelsController < Chat::ApiController
     Chat::CreateCategoryChannel.call(
       service_params.merge(channel_params.merge(category_id: channel_params[:chatable_id])),
     ) do
-      on_success do
-        render_serialized(
-          result.channel,
-          Chat::ChannelSerializer,
-          root: "channel",
-          membership: result.membership,
-        )
+      on_success do |channel:, membership:|
+        render_serialized(channel, Chat::ChannelSerializer, root: "channel", membership:)
       end
       on_model_not_found(:category) { raise ActiveRecord::RecordNotFound }
       on_failed_policy(:can_create_channel) { raise Discourse::InvalidAccess }
       on_failed_policy(:category_channel_does_not_exist) do
         raise Discourse::InvalidParameters.new(I18n.t("chat.errors.channel_exists_for_category"))
       end
-      on_model_errors(:channel) do
-        render_json_error(result.channel, type: :record_invalid, status: 422)
+      on_model_errors(:channel) do |model|
+        render_json_error(model, type: :record_invalid, status: 422)
       end
-      on_model_errors(:membership) do
-        render_json_error(result.membership, type: :record_invalid, status: 422)
+      on_model_errors(:membership) do |model|
+        render_json_error(model, type: :record_invalid, status: 422)
       end
       on_failure { render(json: failed_json, status: 422) }
       on_failed_contract do |contract|
@@ -101,12 +96,12 @@ class Chat::Api::ChannelsController < Chat::ApiController
     end
 
     Chat::UpdateChannel.call(service_params.merge(params_to_edit)) do
-      on_success do
+      on_success do |channel:|
         render_serialized(
-          result.channel,
+          channel,
           Chat::ChannelSerializer,
           root: "channel",
-          membership: result.channel.membership_for(current_user),
+          membership: channel.membership_for(current_user),
         )
       end
       on_model_not_found(:channel) { raise ActiveRecord::RecordNotFound }
diff --git a/plugins/chat/app/controllers/chat/api/channels_current_user_membership_follows_controller.rb b/plugins/chat/app/controllers/chat/api/channels_current_user_membership_follows_controller.rb
index b1da98b2773..bc17163fd73 100644
--- a/plugins/chat/app/controllers/chat/api/channels_current_user_membership_follows_controller.rb
+++ b/plugins/chat/app/controllers/chat/api/channels_current_user_membership_follows_controller.rb
@@ -3,18 +3,14 @@
 class Chat::Api::ChannelsCurrentUserMembershipFollowsController < Chat::Api::ChannelsController
   def destroy
     Chat::UnfollowChannel.call(service_params) do
-      on_success do
-        render_serialized(
-          result.membership,
-          Chat::UserChannelMembershipSerializer,
-          root: "membership",
-        )
+      on_success do |membership:|
+        render_serialized(membership, Chat::UserChannelMembershipSerializer, root: "membership")
       end
-      on_model_not_found(:channel) { raise Discourse::NotFound }
-      on_failure { render(json: failed_json, status: 422) }
       on_failed_contract do |contract|
         render(json: failed_json.merge(errors: contract.errors.full_messages), status: 400)
       end
+      on_model_not_found(:channel) { raise Discourse::NotFound }
+      on_failure { render(json: failed_json, status: 422) }
     end
   end
 end
diff --git a/plugins/chat/app/controllers/chat/api/channels_read_controller.rb b/plugins/chat/app/controllers/chat/api/channels_read_controller.rb
index 2549d15a3e8..a02a21a77dc 100644
--- a/plugins/chat/app/controllers/chat/api/channels_read_controller.rb
+++ b/plugins/chat/app/controllers/chat/api/channels_read_controller.rb
@@ -20,9 +20,7 @@ class Chat::Api::ChannelsReadController < Chat::ApiController
 
   def update_all
     Chat::MarkAllUserChannelsRead.call(service_params) do
-      on_success do
-        render(json: success_json.merge(updated_memberships: result.updated_memberships))
-      end
+      on_success { |updated_memberships:| render(json: success_json.merge(updated_memberships:)) }
       on_failure { render(json: failed_json, status: 422) }
     end
   end
diff --git a/plugins/chat/app/controllers/chat/api/channels_status_controller.rb b/plugins/chat/app/controllers/chat/api/channels_status_controller.rb
index 6f83c3223e8..cd97271c6d0 100644
--- a/plugins/chat/app/controllers/chat/api/channels_status_controller.rb
+++ b/plugins/chat/app/controllers/chat/api/channels_status_controller.rb
@@ -3,7 +3,7 @@
 class Chat::Api::ChannelsStatusController < Chat::Api::ChannelsController
   def update
     Chat::UpdateChannelStatus.call(service_params) do
-      on_success { render_serialized(result.channel, Chat::ChannelSerializer, root: "channel") }
+      on_success { |channel:| render_serialized(channel, Chat::ChannelSerializer, root: "channel") }
       on_model_not_found(:channel) { raise ActiveRecord::RecordNotFound }
       on_failed_policy(:check_channel_permission) { raise Discourse::InvalidAccess }
       on_failure { render(json: failed_json, status: 422) }
diff --git a/plugins/chat/app/controllers/chat/api/chatables_controller.rb b/plugins/chat/app/controllers/chat/api/chatables_controller.rb
index e59a0bb729d..166f6b660b9 100644
--- a/plugins/chat/app/controllers/chat/api/chatables_controller.rb
+++ b/plugins/chat/app/controllers/chat/api/chatables_controller.rb
@@ -4,7 +4,7 @@ class Chat::Api::ChatablesController < Chat::ApiController
   before_action :ensure_logged_in
 
   def index
-    ::Chat::SearchChatable.call(service_params) do
+    ::Chat::SearchChatable.call(service_params) do |result|
       on_success { render_serialized(result, ::Chat::ChatablesSerializer, root: false) }
       on_failure { render(json: failed_json, status: 422) }
       on_failed_contract do |contract|
diff --git a/plugins/chat/app/controllers/chat/api/current_user_channels_controller.rb b/plugins/chat/app/controllers/chat/api/current_user_channels_controller.rb
index 2e14301cf74..d87955a5c24 100644
--- a/plugins/chat/app/controllers/chat/api/current_user_channels_controller.rb
+++ b/plugins/chat/app/controllers/chat/api/current_user_channels_controller.rb
@@ -3,12 +3,12 @@
 class Chat::Api::CurrentUserChannelsController < Chat::ApiController
   def index
     Chat::ListUserChannels.call(service_params) do
-      on_success do
+      on_success do |structured:, post_allowed_category_ids:|
         render_serialized(
-          result.structured,
+          structured,
           Chat::ChannelIndexSerializer,
           root: false,
-          post_allowed_category_ids: result.post_allowed_category_ids,
+          post_allowed_category_ids: post_allowed_category_ids,
         )
       end
       on_failure { render(json: failed_json, status: 422) }
diff --git a/plugins/chat/app/controllers/chat/api/current_user_threads_controller.rb b/plugins/chat/app/controllers/chat/api/current_user_threads_controller.rb
index d781b526718..5fdaf983455 100644
--- a/plugins/chat/app/controllers/chat/api/current_user_threads_controller.rb
+++ b/plugins/chat/app/controllers/chat/api/current_user_threads_controller.rb
@@ -3,26 +3,26 @@
 class Chat::Api::CurrentUserThreadsController < Chat::ApiController
   def index
     ::Chat::LookupUserThreads.call(service_params) do
-      on_success do
+      on_success do |threads:, tracking:, memberships:, load_more_url:, participants:|
         render_serialized(
           ::Chat::ThreadsView.new(
             user: guardian.user,
-            threads: result.threads,
-            channel: result.channel,
-            tracking: result.tracking,
-            memberships: result.memberships,
-            load_more_url: result.load_more_url,
-            threads_participants: result.participants,
+            threads_participants: participants,
+            channel: nil,
+            threads:,
+            tracking:,
+            memberships:,
+            load_more_url:,
           ),
           ::Chat::ThreadListSerializer,
           root: false,
         )
       end
-      on_model_not_found(:threads) { render json: success_json.merge(threads: []) }
-      on_failure { render(json: failed_json, status: 422) }
       on_failed_contract do |contract|
         render(json: failed_json.merge(errors: contract.errors.full_messages), status: 400)
       end
+      on_model_not_found(:threads) { render json: success_json.merge(threads: []) }
+      on_failure { render(json: failed_json, status: 422) }
     end
   end
 end
diff --git a/plugins/chat/app/controllers/chat/api/direct_messages_controller.rb b/plugins/chat/app/controllers/chat/api/direct_messages_controller.rb
index 63c98f973d6..66b756f1be8 100644
--- a/plugins/chat/app/controllers/chat/api/direct_messages_controller.rb
+++ b/plugins/chat/app/controllers/chat/api/direct_messages_controller.rb
@@ -3,14 +3,7 @@
 class Chat::Api::DirectMessagesController < Chat::ApiController
   def create
     Chat::CreateDirectMessageChannel.call(service_params) do
-      on_success do
-        render_serialized(
-          result.channel,
-          Chat::ChannelSerializer,
-          root: "channel",
-          membership: result.membership,
-        )
-      end
+      on_success { |channel:| render_serialized(channel, Chat::ChannelSerializer, root: "channel") }
       on_model_not_found(:target_users) { raise ActiveRecord::RecordNotFound }
       on_failed_policy(:satisfies_dms_max_users_limit) do |policy|
         render_json_dump({ error: policy.reason }, status: 400)
diff --git a/plugins/chat/app/jobs/regular/chat/auto_join_channel_batch.rb b/plugins/chat/app/jobs/regular/chat/auto_join_channel_batch.rb
index fb3f7e2166a..c7c5d6014d0 100644
--- a/plugins/chat/app/jobs/regular/chat/auto_join_channel_batch.rb
+++ b/plugins/chat/app/jobs/regular/chat/auto_join_channel_batch.rb
@@ -9,8 +9,8 @@ module Jobs
           on_failed_contract do |contract|
             Rails.logger.error(contract.errors.full_messages.join(", "))
           end
-          on_model_not_found(:channel) do
-            Rails.logger.error("Channel not found (id=#{result.contract.channel_id})")
+          on_model_not_found(:channel) do |contract:|
+            Rails.logger.error("Channel not found (id=#{contract.channel_id})")
           end
         end
       end
diff --git a/plugins/chat/lib/chat_sdk/channel.rb b/plugins/chat/lib/chat_sdk/channel.rb
index 1bf85714a1f..5a0b914b3b2 100644
--- a/plugins/chat/lib/chat_sdk/channel.rb
+++ b/plugins/chat/lib/chat_sdk/channel.rb
@@ -17,7 +17,7 @@ module ChatSDK
 
     def messages(channel_id:, guardian:, **params)
       Chat::ListChannelMessages.call(channel_id:, guardian:, **params, direction: "future") do
-        on_success { result.messages }
+        on_success { |messages:| messages }
         on_failure { raise "Unexpected error" }
         on_failed_policy(:can_view_channel) { raise "Guardian can't view channel" }
         on_failed_policy(:target_message_exists) { raise "Target message doesn't exist" }
diff --git a/plugins/chat/lib/chat_sdk/message.rb b/plugins/chat/lib/chat_sdk/message.rb
index 64079713239..952b9fe13ae 100644
--- a/plugins/chat/lib/chat_sdk/message.rb
+++ b/plugins/chat/lib/chat_sdk/message.rb
@@ -90,7 +90,7 @@ module ChatSDK
 
     def stop_stream(message_id:, guardian:)
       Chat::StopMessageStreaming.call(message_id:, guardian:) do
-        on_success { result.message }
+        on_success { |message:| message }
         on_model_not_found(:message) { raise "Couldn't find message with id: `#{message_id}`" }
         on_model_not_found(:membership) do
           raise "Couldn't find membership for user with id: `#{guardian.user.id}`"
@@ -145,7 +145,7 @@ module ChatSDK
             raise "User with id: `#{guardian.user.id}` can't join this channel"
           end
           on_failed_contract { |contract| raise contract.errors.full_messages.join(", ") }
-          on_success { result.message_instance }
+          on_success { |message_instance:| message_instance }
           on_failure { raise "Unexpected error" }
         end
 
diff --git a/plugins/chat/lib/chat_sdk/thread.rb b/plugins/chat/lib/chat_sdk/thread.rb
index a8bfedd1cbf..9e038728a07 100644
--- a/plugins/chat/lib/chat_sdk/thread.rb
+++ b/plugins/chat/lib/chat_sdk/thread.rb
@@ -75,7 +75,7 @@ module ChatSDK
 
     def messages(thread_id:, guardian:, direction: "future", **params)
       Chat::ListChannelThreadMessages.call(thread_id:, guardian:, direction:, **params) do
-        on_success { result.messages }
+        on_success { |messages:| messages }
         on_failed_policy(:can_view_thread) { raise "Guardian can't view thread" }
         on_failed_policy(:target_message_exists) { raise "Target message doesn't exist" }
         on_failure { raise "Unexpected error" }
@@ -96,7 +96,7 @@ module ChatSDK
           raise "Threading is not enabled for this channel"
         end
         on_failed_contract { |contract| raise contract.errors.full_messages.join(", ") }
-        on_success { result.thread_instance }
+        on_success { |thread:| thread }
         on_failure { raise "Unexpected error" }
       end
     end
diff --git a/plugins/chat/spec/plugin_helper.rb b/plugins/chat/spec/plugin_helper.rb
index 9c29fd94aec..a15918f73d7 100644
--- a/plugins/chat/spec/plugin_helper.rb
+++ b/plugins/chat/spec/plugin_helper.rb
@@ -67,49 +67,49 @@ module ChatSpecHelpers
   end
 
   def update_message!(message, text: nil, user: Discourse.system_user, upload_ids: nil)
-    result =
-      Chat::UpdateMessage.call(
-        guardian: user.guardian,
-        message_id: message.id,
-        upload_ids: upload_ids,
-        message: text,
-        process_inline: true,
-      )
-    service_failed!(result) if result.failure?
-    result.message_instance
+    Chat::UpdateMessage.call(
+      guardian: user.guardian,
+      message_id: message.id,
+      upload_ids: upload_ids,
+      message: text,
+      process_inline: true,
+    ) do |result|
+      on_success { result.message_instance }
+      on_failure { service_failed!(result) }
+    end
   end
 
   def trash_message!(message, user: Discourse.system_user)
-    result =
-      Chat::TrashMessage.call(
-        message_id: message.id,
-        channel_id: message.chat_channel_id,
-        guardian: user.guardian,
-      )
-    service_failed!(result) if result.failure?
-    result
+    Chat::TrashMessage.call(
+      message_id: message.id,
+      channel_id: message.chat_channel_id,
+      guardian: user.guardian,
+    ) do |result|
+      on_success { result }
+      on_failure { service_failed!(result) }
+    end
   end
 
   def restore_message!(message, user: Discourse.system_user)
-    result =
-      Chat::RestoreMessage.call(
-        message_id: message.id,
-        channel_id: message.chat_channel_id,
-        guardian: user.guardian,
-      )
-    service_failed!(result) if result.failure?
-    result
+    Chat::RestoreMessage.call(
+      message_id: message.id,
+      channel_id: message.chat_channel_id,
+      guardian: user.guardian,
+    ) do |result|
+      on_success { result }
+      on_failure { service_failed!(result) }
+    end
   end
 
   def add_users_to_channel(users, channel, user: Discourse.system_user)
-    result =
-      ::Chat::AddUsersToChannel.call(
-        guardian: user.guardian,
-        channel_id: channel.id,
-        usernames: Array(users).map(&:username),
-      )
-    service_failed!(result) if result.failure?
-    result
+    ::Chat::AddUsersToChannel.call(
+      guardian: user.guardian,
+      channel_id: channel.id,
+      usernames: Array(users).map(&:username),
+    ) do |result|
+      on_success { result }
+      on_failure { service_failed!(result) }
+    end
   end
 
   def create_draft(channel, thread: nil, user: Discourse.system_user, data: { message: "draft" })
@@ -119,15 +119,15 @@ module ChatSpecHelpers
       end
     end
 
-    result =
-      ::Chat::UpsertDraft.call(
-        guardian: user.guardian,
-        channel_id: channel.id,
-        thread_id: thread&.id,
-        data: data.to_json,
-      )
-    service_failed!(result) if result.failure?
-    result
+    ::Chat::UpsertDraft.call(
+      guardian: user.guardian,
+      channel_id: channel.id,
+      thread_id: thread&.id,
+      data: data.to_json,
+    ) do |result|
+      on_success { result }
+      on_failure { service_failed!(result) }
+    end
   end
 end
 
diff --git a/plugins/chat/spec/requests/chat/api/current_user_threads_spec.rb b/plugins/chat/spec/requests/chat/api/current_user_threads_spec.rb
index 0dc0230879e..02a1f5867df 100644
--- a/plugins/chat/spec/requests/chat/api/current_user_threads_spec.rb
+++ b/plugins/chat/spec/requests/chat/api/current_user_threads_spec.rb
@@ -11,10 +11,20 @@ describe Chat::Api::CurrentUserThreadsController do
 
   describe "#index" do
     describe "success" do
+      let!(:thread) do
+        Fabricate(
+          :chat_thread,
+          original_message_user: current_user,
+          with_replies: 2,
+          use_service: true,
+        )
+      end
+
       it "works" do
         get "/chat/api/me/threads"
 
-        expect(response.status).to eq(200)
+        expect(response).to have_http_status :ok
+        expect(response.parsed_body[:threads]).not_to be_empty
       end
     end
 
diff --git a/spec/lib/service/runner_spec.rb b/spec/lib/service/runner_spec.rb
index b9d98c66b03..7128b82b11f 100644
--- a/spec/lib/service/runner_spec.rb
+++ b/spec/lib/service/runner_spec.rb
@@ -150,10 +150,13 @@ RSpec.describe Service::Runner do
   describe ".call" do
     subject(:runner) { described_class.call(service, &actions_block) }
 
-    let(:result) { object.result }
     let(:actions_block) { object.instance_eval(actions) }
-    let(:service) { SuccessService }
-    let(:actions) { "proc {}" }
+    let(:service) { SuccessWithModelService }
+    let(:actions) { <<-BLOCK }
+        proc do |result|
+          on_success { |fake_model:| [result, fake_model] }
+        end
+      BLOCK
     let(:object) do
       Class
         .new(ApplicationController) do
@@ -171,10 +174,12 @@ RSpec.describe Service::Runner do
         .new
     end
 
-    it "runs the provided service in the context of a controller" do
-      runner
-      expect(result).to be_a Service::Base::Context
-      expect(result).to be_a_success
+    it "allows access to the result object" do
+      expect(runner.first).to be_a Service::Base::Context
+    end
+
+    it "allows using keyword args in blocks" do
+      expect(runner.last).to eq :model_found
     end
 
     context "when using the on_success action" do
@@ -241,7 +246,7 @@ RSpec.describe Service::Runner do
 
         context "when using the block argument" do
           let(:actions) { <<-BLOCK }
-              proc do
+              proc do |result|
                 on_failed_policy(:test) { |policy| policy == result["result.policy.test"] }
               end
             BLOCK
@@ -279,7 +284,7 @@ RSpec.describe Service::Runner do
 
         context "when using the block argument" do
           let(:actions) { <<-BLOCK }
-              proc do
+              proc do |result|
                 on_failed_contract { |contract| contract == result["result.contract.default"] }
               end
             BLOCK
@@ -301,8 +306,9 @@ RSpec.describe Service::Runner do
 
     context "when using the on_model_not_found action" do
       let(:actions) { <<-BLOCK }
-          proc do
-            on_model_not_found(:fake_model) { :no_model }
+          proc do |result|
+            on_success { [result] }
+            on_model_not_found(:fake_model) { [:no_model, result] }
           end
         BLOCK
 
@@ -311,7 +317,7 @@ RSpec.describe Service::Runner do
           let(:service) { FailureWithOptionalModelService }
 
           it "does not run the provided block" do
-            expect(runner).not_to eq :no_model
+            expect(runner).not_to include :no_model
           end
         end
 
@@ -320,13 +326,13 @@ RSpec.describe Service::Runner do
 
           context "when not using the block argument" do
             it "runs the provided block" do
-              expect(runner).to eq :no_model
+              expect(runner).to include :no_model
             end
           end
 
           context "when using the block argument" do
             let(:actions) { <<-BLOCK }
-                proc do
+                proc do |result|
                   on_model_not_found(:fake_model) { |model| model == result["result.model.fake_model"] }
                 end
               BLOCK
@@ -341,7 +347,7 @@ RSpec.describe Service::Runner do
           let(:service) { SuccessWithModelService }
 
           it "does not run the provided block" do
-            expect(runner).not_to eq :no_model
+            expect(runner).not_to include :no_model
           end
         end
       end
@@ -351,7 +357,7 @@ RSpec.describe Service::Runner do
           let(:service) { FailureWithCollectionModelService }
 
           it "runs the provided block" do
-            expect(runner).to eq :no_model
+            expect(runner).to include :no_model
           end
         end
 
@@ -359,7 +365,7 @@ RSpec.describe Service::Runner do
           let(:service) { SuccessWithCollectionModelService }
 
           it "does not run the provided block" do
-            expect(runner).not_to eq :no_model
+            expect(runner).not_to include :no_model
           end
         end
       end
@@ -371,23 +377,21 @@ RSpec.describe Service::Runner do
           before { Fabricate(:user) }
 
           it "does not run the provided block" do
-            expect(runner).not_to eq :no_model
+            expect(runner).not_to include :no_model
           end
 
           it "does not fetch records from the relation" do
-            runner
-            expect(result[:fake_model]).not_to be_loaded
+            expect(runner.last[:fake_model]).not_to be_loaded
           end
         end
 
         context "when the service fails" do
           it "runs the provided block" do
-            expect(runner).to eq :no_model
+            expect(runner).to include :no_model
           end
 
           it "does not fetch records from the relation" do
-            runner
-            expect(result[:fake_model]).not_to be_loaded
+            expect(runner.last[:fake_model]).not_to be_loaded
           end
         end
       end