From 927ae8c0aaa3cf97da764140424d9151a078e270 Mon Sep 17 00:00:00 2001
From: "Hal M. Spitz" <hal.spitz@gmail.com>
Date: Sun, 25 Aug 2024 13:23:11 -0700
Subject: [PATCH] Addresses PR feedback

* Moves defaults handling from MissionControl::Jobs::Application to
  MissionControl::Jobs::Server to minimize impact of this new
  feature on the existing implementation and tests.

  This fixes the unit test failure that I missed in the previous commit.

* Uses @server rather than looking it up via params in the JobsController.
  This allows reverting all changes to the JobsController.
---
 app/controllers/mission_control/jobs/jobs_controller.rb  | 9 ---------
 app/helpers/mission_control/jobs/jobs_helper.rb          | 6 +++---
 .../jobs/jobs/_error_information.html.erb                | 4 ++--
 lib/mission_control/jobs/application.rb                  | 1 -
 lib/mission_control/jobs/server.rb                       | 4 ++--
 5 files changed, 7 insertions(+), 17 deletions(-)

diff --git a/app/controllers/mission_control/jobs/jobs_controller.rb b/app/controllers/mission_control/jobs/jobs_controller.rb
index 513981f0..c9d8e089 100644
--- a/app/controllers/mission_control/jobs/jobs_controller.rb
+++ b/app/controllers/mission_control/jobs/jobs_controller.rb
@@ -2,7 +2,6 @@ class MissionControl::Jobs::JobsController < MissionControl::Jobs::ApplicationCo
   include MissionControl::Jobs::JobScoped, MissionControl::Jobs::JobFilters
 
   skip_before_action :set_job, only: :index
-  before_action :set_backtrace_cleaner, only: :show
 
   def index
     @job_class_names = jobs_with_status.job_class_names
@@ -38,12 +37,4 @@ def filtered_jobs
     def jobs_status
       params[:status].presence&.inquiry
     end
-
-    def set_backtrace_cleaner
-      @backtrace_cleaner = @application.servers[server_id]&.backtrace_cleaner
-    end
-
-    def server_id
-      params[:server_id]
-    end
 end
diff --git a/app/helpers/mission_control/jobs/jobs_helper.rb b/app/helpers/mission_control/jobs/jobs_helper.rb
index ea15dfc7..14219e6c 100644
--- a/app/helpers/mission_control/jobs/jobs_helper.rb
+++ b/app/helpers/mission_control/jobs/jobs_helper.rb
@@ -15,9 +15,9 @@ def clean_backtrace?
     params["clean_backtrace"] == "true"
   end
 
-  def failed_job_backtrace(job)
-    if clean_backtrace? && @backtrace_cleaner
-      @backtrace_cleaner.clean(job.last_execution_error.backtrace).join("\n")
+  def failed_job_backtrace(job, server)
+    if clean_backtrace? && server&.backtrace_cleaner
+      server.backtrace_cleaner.clean(job.last_execution_error.backtrace).join("\n")
     else
       job.last_execution_error.backtrace.join("\n")
     end
diff --git a/app/views/mission_control/jobs/jobs/_error_information.html.erb b/app/views/mission_control/jobs/jobs/_error_information.html.erb
index 419c6b5d..0f61a0c6 100644
--- a/app/views/mission_control/jobs/jobs/_error_information.html.erb
+++ b/app/views/mission_control/jobs/jobs/_error_information.html.erb
@@ -14,9 +14,9 @@
     </tbody>
   </table>
 
-  <% if @backtrace_cleaner %>
+  <% if @server.backtrace_cleaner %>
     <%= render "mission_control/jobs/jobs/failed/backtrace_toggle", application: @application, job: job %>
   <% end %>
 
-  <pre class="is-family-monospace mb-4 backtrace-content"><%= failed_job_backtrace(job) %></pre>
+  <pre class="is-family-monospace mb-4 backtrace-content"><%= failed_job_backtrace(job, @server) %></pre>
 <% end %>
diff --git a/lib/mission_control/jobs/application.rb b/lib/mission_control/jobs/application.rb
index 7fb37174..bd04c81a 100644
--- a/lib/mission_control/jobs/application.rb
+++ b/lib/mission_control/jobs/application.rb
@@ -12,7 +12,6 @@ def initialize(name:)
   def add_servers(queue_adapters_by_name)
     queue_adapters_by_name.each do |name, queue_adapter|
       adapter, cleaner = queue_adapter
-      cleaner ||= MissionControl::Jobs.backtrace_cleaner
 
       servers << MissionControl::Jobs::Server.new(name: name.to_s, queue_adapter: adapter,
         backtrace_cleaner: cleaner, application: self)
diff --git a/lib/mission_control/jobs/server.rb b/lib/mission_control/jobs/server.rb
index fa6dd4c6..970d5f5a 100644
--- a/lib/mission_control/jobs/server.rb
+++ b/lib/mission_control/jobs/server.rb
@@ -6,11 +6,11 @@ class MissionControl::Jobs::Server
 
   attr_reader :name, :queue_adapter, :application, :backtrace_cleaner
 
-  def initialize(name:, queue_adapter:, application:, backtrace_cleaner:)
+  def initialize(name:, queue_adapter:, application:, backtrace_cleaner: nil)
     super(name: name)
     @queue_adapter = queue_adapter
     @application = application
-    @backtrace_cleaner = backtrace_cleaner
+    @backtrace_cleaner = backtrace_cleaner || MissionControl::Jobs.backtrace_cleaner
   end
 
   def activating(&block)