Commit a5f3d391 authored by Allison Browne's avatar Allison Browne Committed by Mayra Cabrera

Allow filtering by issue_status in issues service

Refactor issue services to allow a return with api formatted error
based on the results of the response.
parent 06fcdc01
......@@ -30,7 +30,7 @@ class Projects::ErrorTrackingController < Projects::ErrorTracking::BaseControlle
service = ErrorTracking::IssueUpdateService.new(project, current_user, issue_update_params)
result = service.execute
return if handle_errors(result)
return if render_errors(result)
render json: {
result: result
......@@ -47,7 +47,7 @@ class Projects::ErrorTrackingController < Projects::ErrorTracking::BaseControlle
)
result = service.execute
return if handle_errors(result)
return if render_errors(result)
render json: {
errors: serialize_errors(result[:issues]),
......@@ -60,14 +60,14 @@ class Projects::ErrorTrackingController < Projects::ErrorTracking::BaseControlle
service = ErrorTracking::IssueDetailsService.new(project, current_user, issue_details_params)
result = service.execute
return if handle_errors(result)
return if render_errors(result)
render json: {
error: serialize_detailed_error(result[:issue])
}
end
def handle_errors(result)
def render_errors(result)
unless result[:status] == :success
render json: { message: result[:message] },
status: result[:http_status] || :bad_request
......@@ -75,7 +75,7 @@ class Projects::ErrorTrackingController < Projects::ErrorTracking::BaseControlle
end
def list_issues_params
params.permit(:search_term, :sort, :cursor)
params.permit(:search_term, :sort, :cursor, :issue_status)
end
def issue_update_params
......
......@@ -89,7 +89,9 @@ module ErrorTracking
end
def list_sentry_projects
{ projects: sentry_client.projects }
handle_exceptions do
{ projects: sentry_client.projects }
end
end
def issue_details(opts = {})
......
......@@ -3,21 +3,9 @@
module ErrorTracking
class BaseService < ::BaseService
def execute
unauthorized = check_permissions
return unauthorized if unauthorized
begin
response = perform
rescue Sentry::Client::Error => e
return error(e.message, :bad_request)
rescue Sentry::Client::MissingKeysError => e
return error(e.message, :internal_server_error)
end
errors = parse_errors(response)
return errors if errors
success(parse_response(response))
perform
end
private
......@@ -27,12 +15,21 @@ module ErrorTracking
"#{self.class} does not implement #{__method__}"
end
def compose_response(response, &block)
errors = parse_errors(response)
return errors if errors
yield if block_given?
success(parse_response(response))
end
def parse_response(response)
raise NotImplementedError,
"#{self.class} does not implement #{__method__}"
end
def check_permissions
def unauthorized
return error('Error Tracking is not enabled') unless enabled?
return error('Access denied', :unauthorized) unless can_read?
end
......
......@@ -5,7 +5,9 @@ module ErrorTracking
private
def perform
project_error_tracking_setting.issue_details(issue_id: params[:issue_id])
response = project_error_tracking_setting.issue_details(issue_id: params[:issue_id])
compose_response(response)
end
def parse_response(response)
......
......@@ -5,7 +5,9 @@ module ErrorTracking
private
def perform
project_error_tracking_setting.issue_latest_event(issue_id: params[:issue_id])
response = project_error_tracking_setting.issue_latest_event(issue_id: params[:issue_id])
compose_response(response)
end
def parse_response(response)
......
......@@ -7,21 +7,15 @@ module ErrorTracking
private
def perform
response = fetch
response = project_error_tracking_setting.update_issue(
issue_id: params[:issue_id],
params: update_params
)
unless parse_errors(response).present?
compose_response(response) do
response[:closed_issue_iid] = update_related_issue&.iid
project_error_tracking_setting.expire_issues_cache
end
response
end
def fetch
project_error_tracking_setting.update_issue(
issue_id: params[:issue_id],
params: update_params
)
end
def update_related_issue
......@@ -74,7 +68,7 @@ module ErrorTracking
}
end
def check_permissions
def unauthorized
return error('Error Tracking is not enabled') unless enabled?
return error('Access denied', :unauthorized) unless can_update?
end
......
......@@ -6,6 +6,13 @@ module ErrorTracking
DEFAULT_LIMIT = 20
DEFAULT_SORT = 'last_seen'
# Sentry client supports 'muted' and 'assigned' but GitLab does not
ISSUE_STATUS_VALUES = %w[
resolved
unresolved
ignored
].freeze
def external_url
project_error_tracking_setting&.sentry_external_url
end
......@@ -13,19 +20,31 @@ module ErrorTracking
private
def perform
project_error_tracking_setting.list_sentry_issues(
return invalid_status_error unless valid_status?
response = project_error_tracking_setting.list_sentry_issues(
issue_status: issue_status,
limit: limit,
search_term: params[:search_term].presence,
sort: sort,
cursor: params[:cursor].presence
)
compose_response(response)
end
def parse_response(response)
response.slice(:issues, :pagination)
end
def invalid_status_error
error('Bad Request: Invalid issue_status', http_status_for(:bad_Request))
end
def valid_status?
ISSUE_STATUS_VALUES.include?(issue_status)
end
def issue_status
params[:issue_status] || DEFAULT_ISSUE_STATUS
end
......
......@@ -2,18 +2,16 @@
module ErrorTracking
class ListProjectsService < ErrorTracking::BaseService
def execute
private
def perform
unless project_error_tracking_setting.valid?
return error(project_error_tracking_setting.errors.full_messages.join(', '), :bad_request)
end
super
end
response = project_error_tracking_setting.list_sentry_projects
private
def perform
project_error_tracking_setting.list_sentry_projects
compose_response(response)
end
def parse_response(response)
......
# frozen_string_literal: true
require 'spec_helper'
describe ErrorTracking::BaseService do
describe '#compose_response' do
let(:project) { double('project') }
let(:user) { double('user') }
let(:service) { described_class.new(project, user) }
it 'returns bad_request error when response has an error key' do
data = { error: 'Unexpected Error' }
result = service.send(:compose_response, data)
expect(result[:status]).to be(:error)
expect(result[:message]).to be('Unexpected Error')
expect(result[:http_status]).to be(:bad_request)
end
it 'returns server error when response has missing key error_type' do
data = { error: 'Unexpected Error', error_type: ErrorTracking::ProjectErrorTrackingSetting::SENTRY_API_ERROR_TYPE_MISSING_KEYS }
result = service.send(:compose_response, data)
expect(result[:status]).to be(:error)
expect(result[:message]).to be('Unexpected Error')
expect(result[:http_status]).to be(:internal_server_error)
end
it 'returns no content when response is nil' do
data = nil
result = service.send(:compose_response, data)
expect(result[:status]).to be(:error)
expect(result[:message]).to be('Not ready. Try again later')
expect(result[:http_status]).to be(:no_content)
end
context 'when result has no errors key' do
let(:data) { { thing: :cat } }
it 'raises NotImplementedError' do
expect { service.send(:compose_response, data) }
.to raise_error(NotImplementedError)
end
context 'when parse_response is implemented' do
before do
expect(service).to receive(:parse_response) do |response|
{ animal: response[:thing] }
end
end
it 'returns successful response' do
result = service.send(:compose_response, data)
expect(result[:animal]).to eq(:cat)
expect(result[:status]).to eq(:success)
end
it 'returns successful response with changes from passed block' do
result = service.send(:compose_response, data) do
data[:thing] = :fish
end
expect(result[:animal]).to eq(:fish)
expect(result[:status]).to eq(:success)
end
end
end
end
end
......@@ -20,21 +20,31 @@ describe ErrorTracking::ListIssuesService do
describe '#execute' do
context 'with authorized user' do
context 'when list_sentry_issues returns issues' do
let(:issues) { [:list, :of, :issues] }
before do
expect(error_tracking_setting)
.to receive(:list_sentry_issues)
.with(list_sentry_issues_args)
.and_return(issues: issues, pagination: {})
end
let(:issues) { [] }
described_class::ISSUE_STATUS_VALUES.each do |status|
it "returns the issues with #{status} issue_status" do
params[:issue_status] = status
list_sentry_issues_args[:issue_status] = status
expect_list_sentry_issues_with(list_sentry_issues_args)
it 'returns the issues' do
expect(result).to eq(status: :success, pagination: {}, issues: issues)
end
end
it 'returns the issues with no issue_status' do
expect_list_sentry_issues_with(list_sentry_issues_args)
expect(result).to eq(status: :success, pagination: {}, issues: issues)
end
it 'returns bad request for an issue_status not on the whitelist' do
params[:issue_status] = 'assigned'
expect(error_tracking_setting).not_to receive(:list_sentry_issues)
expect(result).to eq(message: "Bad Request: Invalid issue_status", status: :error, http_status: :bad_request)
end
include_examples 'error tracking service data not ready', :list_sentry_issues
include_examples 'error tracking service sentry error handling', :list_sentry_issues
include_examples 'error tracking service http status handling', :list_sentry_issues
......@@ -52,3 +62,10 @@ describe ErrorTracking::ListIssuesService do
end
end
end
def expect_list_sentry_issues_with(list_sentry_issues_args)
expect(error_tracking_setting)
.to receive(:list_sentry_issues)
.with(list_sentry_issues_args)
.and_return(issues: [], pagination: {})
end
......@@ -63,32 +63,6 @@ describe ErrorTracking::ListProjectsService do
end
end
context 'sentry client raises exception' do
context 'Sentry::Client::Error' do
before do
expect(error_tracking_setting).to receive(:list_sentry_projects)
.and_raise(Sentry::Client::Error, 'Sentry response status code: 500')
end
it 'returns error response' do
expect(result[:message]).to eq('Sentry response status code: 500')
expect(result[:http_status]).to eq(:bad_request)
end
end
context 'Sentry::Client::MissingKeysError' do
before do
expect(error_tracking_setting).to receive(:list_sentry_projects)
.and_raise(Sentry::Client::MissingKeysError, 'Sentry API response is missing keys. key not found: "id"')
end
it 'returns error response' do
expect(result[:message]).to eq('Sentry API response is missing keys. key not found: "id"')
expect(result[:http_status]).to eq(:internal_server_error)
end
end
end
context 'with invalid url' do
let(:params) do
ActionController::Parameters.new(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment