Merge branch 'refactor-error-tracking-controller-extract-stack-trace' into 'master'

Extract stack trace from ErrorTracking controller

See merge request gitlab-org/gitlab!22158

app/controllers/projects/error_tracking/stack_traces_controller.rb

+# frozen_string_literal: true
+
+module Projects
+  module ErrorTracking
+    class StackTracesController < Projects::ApplicationController
+      respond_to :json
+
+      before_action :authorize_read_sentry_issue!
+
+      def index
+        result = fetch_latest_event_issue
+
+        if result[:status] == :success
+          result_with_syntax_highlight = Gitlab::ErrorTracking::StackTraceHighlightDecorator.decorate(result[:latest_event])
+
+          render json: { error: serialize_error_event(result_with_syntax_highlight) }
+        else
+          render json: { message: result[:message] }, status: result.fetch(:http_status, :bad_request)
+        end
+      end
+
+      private
+
+      def fetch_latest_event_issue
+        ::ErrorTracking::IssueLatestEventService
+          .new(project, current_user, issue_id: params[:issue_id])
+          .execute
+      end
+
+      def serialize_error_event(event)
+        ::ErrorTracking::ErrorEventSerializer
+          .new(project: project, user: current_user)
+          .represent(event)
+      end
+    end
+  end
+end

app/controllers/projects/error_tracking_controller.rb

@@ -2,7 +2,7 @@
 
 class Projects::ErrorTrackingController < Projects::ApplicationController
   before_action :authorize_read_sentry_issue!
-  before_action :set_issue_id, only: [:details, :stack_trace]
+  before_action :set_issue_id, only: :details
 
   POLLING_INTERVAL = 10_000
 
@@ -25,14 +25,6 @@ class Projects::ErrorTrackingController < Projects::ApplicationController
     end
   end
 
-  def stack_trace
-    respond_to do |format|
-      format.json do
-        render_issue_stack_trace_json
-      end
-    end
-  end
-
   private
 
   def render_index_json
@@ -63,19 +55,6 @@ class Projects::ErrorTrackingController < Projects::ApplicationController
     }
   end
 
-  def render_issue_stack_trace_json
-    service = ErrorTracking::IssueLatestEventService.new(project, current_user, issue_details_params)
-    result = service.execute
-
-    return if handle_errors(result)
-
-    result_with_syntax_highlight = Gitlab::ErrorTracking::StackTraceHighlightDecorator.decorate(result[:latest_event])
-
-    render json: {
-      error: serialize_error_event(result_with_syntax_highlight)
-    }
-  end
-
   def handle_errors(result)
     unless result[:status] == :success
       render json: { message: result[:message] },
@@ -110,10 +89,4 @@ class Projects::ErrorTrackingController < Projects::ApplicationController
       .new(project: project, user: current_user)
       .represent(error)
   end
-
-  def serialize_error_event(event)
-    ErrorTracking::ErrorEventSerializer
-      .new(project: project, user: current_user)
-      .represent(event)
-  end
 end

config/routes/project.rb

@@ -269,7 +269,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
               to: 'error_tracking#details',
               as: 'details'
             get ':issue_id/stack_trace',
-              to: 'error_tracking#stack_trace',
+              to: 'error_tracking/stack_traces#index',
               as: 'stack_trace'
           end
         end

spec/controllers/projects/error_tracking/stack_traces_controller_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Projects::ErrorTracking::StackTracesController do
+  let_it_be(:project) { create(:project) }
+  let_it_be(:user) { create(:user) }
+
+  before do
+    sign_in(user)
+    project.add_maintainer(user)
+  end
+
+  describe 'GET #index' do
+    let(:issue_id) { 1234 }
+    let(:issue_stack_trace_service) { spy(:issue_stack_trace_service) }
+
+    subject(:get_stack_trace) do
+      get :index, params: { namespace_id: project.namespace, project_id: project, issue_id: issue_id, format: :json }
+    end
+
+    before do
+      expect(ErrorTracking::IssueLatestEventService)
+        .to receive(:new).with(project, user, issue_id: issue_id.to_s)
+        .and_return(issue_stack_trace_service)
+      expect(issue_stack_trace_service).to receive(:execute).and_return(service_response)
+
+      get_stack_trace
+    end
+
+    context 'awaiting data' do
+      let(:service_response) { { status: :error, http_status: :no_content }}
+
+      it 'responds with no data' do
+        expect(response).to have_gitlab_http_status(:no_content)
+      end
+    end
+
+    context 'service result is successful' do
+      let(:service_response) { { status: :success, latest_event: error_event } }
+      let(:error_event) { build(:error_tracking_error_event) }
+
+      it 'responds with success' do
+        expect(response).to have_gitlab_http_status(:ok)
+      end
+
+      it 'responds with error' do
+        expect(response).to match_response_schema('error_tracking/issue_stack_trace')
+      end
+
+      it 'highlights stack trace source code' do
+        expect(json_response['error']).to eq(
+          Gitlab::ErrorTracking::StackTraceHighlightDecorator.decorate(error_event).as_json
+        )
+      end
+    end
+
+    context 'service result is erroneous' do
+      let(:error_message) { 'error message' }
+
+      context 'without http_status' do
+        let(:service_response) { { status: :error, message: error_message } }
+
+        it 'responds with bad request' do
+          expect(response).to have_gitlab_http_status(:bad_request)
+        end
+
+        it 'responds with error message' do
+          expect(json_response['message']).to eq(error_message)
+        end
+      end
+
+      context 'with explicit http_status' do
+        let(:http_status) { :no_content }
+        let(:service_response) { { status: :error, message: error_message, http_status: http_status } }
+
+        it 'responds with custom http status' do
+          expect(response).to have_gitlab_http_status(http_status)
+        end
+
+        it 'responds with error message' do
+          expect(json_response['message']).to eq(error_message)
+        end
+      end
+    end
+  end
+end

spec/controllers/projects/error_tracking_controller_spec.rb

@@ -266,102 +266,6 @@ describe Projects::ErrorTrackingController do
     end
   end
 
-  describe 'GET #stack_trace' do
-    let_it_be(:issue_id) { 1234 }
-
-    let(:issue_stack_trace_service) { spy(:issue_stack_trace_service) }
-
-    let(:permitted_params) do
-      ActionController::Parameters.new(
-        { issue_id: issue_id.to_s }
-      ).permit!
-    end
-
-    subject(:get_stack_trace) do
-      get :stack_trace, params: issue_params(issue_id: issue_id, format: :json)
-    end
-
-    before do
-      expect(ErrorTracking::IssueLatestEventService)
-        .to receive(:new).with(project, user, permitted_params)
-        .and_return(issue_stack_trace_service)
-    end
-
-    describe 'format json' do
-      context 'awaiting data' do
-        before do
-          expect(issue_stack_trace_service).to receive(:execute)
-            .and_return(status: :error, http_status: :no_content)
-        end
-
-        it 'returns no data' do
-          get_stack_trace
-
-          expect(response).to have_gitlab_http_status(:no_content)
-        end
-      end
-
-      context 'service result is successful' do
-        before do
-          expect(issue_stack_trace_service).to receive(:execute)
-            .and_return(status: :success, latest_event: error_event)
-
-          get_stack_trace
-        end
-
-        let(:error_event) { build(:error_tracking_error_event) }
-
-        it 'returns an error' do
-          expect(response).to have_gitlab_http_status(:ok)
-          expect(response).to match_response_schema('error_tracking/issue_stack_trace')
-        end
-
-        it 'highlights stack trace source code' do
-          expect(json_response['error']).to eq(
-            Gitlab::ErrorTracking::StackTraceHighlightDecorator.decorate(error_event).as_json
-          )
-        end
-      end
-
-      context 'service result is erroneous' do
-        let(:error_message) { 'error message' }
-
-        context 'without http_status' do
-          before do
-            expect(issue_stack_trace_service).to receive(:execute)
-              .and_return(status: :error, message: error_message)
-          end
-
-          it 'returns 400 with message' do
-            get_stack_trace
-
-            expect(response).to have_gitlab_http_status(:bad_request)
-            expect(json_response['message']).to eq(error_message)
-          end
-        end
-
-        context 'with explicit http_status' do
-          let(:http_status) { :no_content }
-
-          before do
-            expect(issue_stack_trace_service).to receive(:execute).and_return(
-              status: :error,
-              message: error_message,
-              http_status: http_status
-            )
-          end
-
-          it 'returns http_status with message' do
-            get_stack_trace
-
-            expect(response).to have_gitlab_http_status(http_status)
-            expect(json_response['message']).to eq(error_message)
-          end
-        end
-      end
-    end
-  end
-
   private
 
   def issue_params(opts = {})