Commit b7cc03d4 authored by Laura Montemayor's avatar Laura Montemayor Committed by Fatih Acet

Add search term to Sentry::Client#list_issues

That allows to filter Sentry issues by the search term
https://docs.sentry.io/workflow/search/
parent 492775d8
<script> <script>
import { mapActions, mapState, mapGetters } from 'vuex'; import { mapActions, mapState } from 'vuex';
import { import {
GlEmptyState, GlEmptyState,
GlButton, GlButton,
GlLink, GlLink,
GlLoadingIcon, GlLoadingIcon,
GlTable, GlTable,
GlSearchBoxByType, GlSearchBoxByClick,
} from '@gitlab/ui'; } from '@gitlab/ui';
import { visitUrl } from '~/lib/utils/url_utility'; import { visitUrl } from '~/lib/utils/url_utility';
import Icon from '~/vue_shared/components/icon.vue'; import Icon from '~/vue_shared/components/icon.vue';
...@@ -28,7 +28,7 @@ export default { ...@@ -28,7 +28,7 @@ export default {
GlLink, GlLink,
GlLoadingIcon, GlLoadingIcon,
GlTable, GlTable,
GlSearchBoxByType, GlSearchBoxByClick,
Icon, Icon,
TimeAgo, TimeAgo,
}, },
...@@ -64,10 +64,6 @@ export default { ...@@ -64,10 +64,6 @@ export default {
}, },
computed: { computed: {
...mapState('list', ['errors', 'externalUrl', 'loading']), ...mapState('list', ['errors', 'externalUrl', 'loading']),
...mapGetters('list', ['filterErrorsByTitle']),
filteredErrors() {
return this.errorSearchQuery ? this.filterErrorsByTitle(this.errorSearchQuery) : this.errors;
},
}, },
created() { created() {
if (this.errorTrackingEnabled) { if (this.errorTrackingEnabled) {
...@@ -76,6 +72,9 @@ export default { ...@@ -76,6 +72,9 @@ export default {
}, },
methods: { methods: {
...mapActions('list', ['startPolling', 'restartPolling']), ...mapActions('list', ['startPolling', 'restartPolling']),
filterErrors() {
this.startPolling(`${this.indexPath}?search_term=${this.errorSearchQuery}`);
},
trackViewInSentryOptions, trackViewInSentryOptions,
viewDetails(errorId) { viewDetails(errorId) {
visitUrl(`error_tracking/${errorId}/details`); visitUrl(`error_tracking/${errorId}/details`);
...@@ -87,17 +86,15 @@ export default { ...@@ -87,17 +86,15 @@ export default {
<template> <template>
<div> <div>
<div v-if="errorTrackingEnabled"> <div v-if="errorTrackingEnabled">
<div v-if="loading" class="py-3"> <div>
<gl-loading-icon :size="3" />
</div>
<div v-else>
<div class="d-flex flex-row justify-content-around bg-secondary border"> <div class="d-flex flex-row justify-content-around bg-secondary border">
<gl-search-box-by-type <gl-search-box-by-click
v-model="errorSearchQuery" v-model="errorSearchQuery"
class="col-lg-10 m-3 p-0" class="col-lg-10 m-3 p-0"
:placeholder="__('Search or filter results...')" :placeholder="__('Search or filter results...')"
type="search" type="search"
autofocus autofocus
@submit="filterErrors"
/> />
<gl-button <gl-button
v-track-event="trackViewInSentryOptions(externalUrl)" v-track-event="trackViewInSentryOptions(externalUrl)"
...@@ -111,9 +108,14 @@ export default { ...@@ -111,9 +108,14 @@ export default {
</gl-button> </gl-button>
</div> </div>
<div v-if="loading" class="py-3">
<gl-loading-icon size="md" />
</div>
<gl-table <gl-table
v-else
class="mt-3" class="mt-3"
:items="filteredErrors" :items="errors"
:fields="$options.fields" :fields="$options.fields"
:show-empty="true" :show-empty="true"
fixed fixed
......
...@@ -4,7 +4,6 @@ import Vuex from 'vuex'; ...@@ -4,7 +4,6 @@ import Vuex from 'vuex';
import * as listActions from './list/actions'; import * as listActions from './list/actions';
import listMutations from './list/mutations'; import listMutations from './list/mutations';
import listState from './list/state'; import listState from './list/state';
import * as listGetters from './list/getters';
import * as detailsActions from './details/actions'; import * as detailsActions from './details/actions';
import detailsMutations from './details/mutations'; import detailsMutations from './details/mutations';
...@@ -21,7 +20,6 @@ export const createStore = () => ...@@ -21,7 +20,6 @@ export const createStore = () =>
state: listState(), state: listState(),
actions: listActions, actions: listActions,
mutations: listMutations, mutations: listMutations,
getters: listGetters,
}, },
details: { details: {
namespaced: true, namespaced: true,
......
...@@ -7,6 +7,8 @@ import { __, sprintf } from '~/locale'; ...@@ -7,6 +7,8 @@ import { __, sprintf } from '~/locale';
let eTagPoll; let eTagPoll;
export function startPolling({ commit, dispatch }, endpoint) { export function startPolling({ commit, dispatch }, endpoint) {
commit(types.SET_LOADING, true);
eTagPoll = new Poll({ eTagPoll = new Poll({
resource: Service, resource: Service,
method: 'getSentryData', method: 'getSentryData',
......
export const filterErrorsByTitle = state => errorQuery =>
state.errors.filter(error => error.title.match(new RegExp(`${errorQuery}`, 'i')));
export default () => {};
...@@ -44,7 +44,11 @@ class Projects::ErrorTrackingController < Projects::ApplicationController ...@@ -44,7 +44,11 @@ class Projects::ErrorTrackingController < Projects::ApplicationController
private private
def render_index_json def render_index_json
service = ErrorTracking::ListIssuesService.new(project, current_user) service = ErrorTracking::ListIssuesService.new(
project,
current_user,
list_issues_params
)
result = service.execute result = service.execute
return if handle_errors(result) return if handle_errors(result)
...@@ -106,6 +110,10 @@ class Projects::ErrorTrackingController < Projects::ApplicationController ...@@ -106,6 +110,10 @@ class Projects::ErrorTrackingController < Projects::ApplicationController
end end
end end
def list_issues_params
params.permit(:search_term)
end
def list_projects_params def list_projects_params
params.require(:error_tracking_setting).permit([:api_host, :token]) params.require(:error_tracking_setting).permit([:api_host, :token])
end end
......
...@@ -5,6 +5,28 @@ module ErrorTracking ...@@ -5,6 +5,28 @@ module ErrorTracking
DEFAULT_ISSUE_STATUS = 'unresolved' DEFAULT_ISSUE_STATUS = 'unresolved'
DEFAULT_LIMIT = 20 DEFAULT_LIMIT = 20
def execute
return error('Error Tracking is not enabled') unless enabled?
return error('Access denied', :unauthorized) unless can_read?
result = project_error_tracking_setting.list_sentry_issues(
issue_status: issue_status,
limit: limit,
search_term: search_term
)
# our results are not yet ready
unless result
return error('Not ready. Try again later', :no_content)
end
if result[:error].present?
return error(result[:error], http_status_for(result[:error_type]))
end
success(issues: result[:issues])
end
def external_url def external_url
project_error_tracking_setting&.sentry_external_url project_error_tracking_setting&.sentry_external_url
end end
...@@ -26,5 +48,17 @@ module ErrorTracking ...@@ -26,5 +48,17 @@ module ErrorTracking
def limit def limit
params[:limit] || DEFAULT_LIMIT params[:limit] || DEFAULT_LIMIT
end end
def search_term
params[:search_term].presence
end
def enabled?
project_error_tracking_setting&.enabled?
end
def can_read?
can?(current_user, :read_sentry_issue, project)
end
end end
end end
---
title: Search list of Sentry errors by title in GitLab
merge_request: 19439
author:
type: added
...@@ -25,8 +25,12 @@ module Sentry ...@@ -25,8 +25,12 @@ module Sentry
map_to_event(latest_event) map_to_event(latest_event)
end end
def list_issues(issue_status:, limit:) def list_issues(issue_status:, limit:, search_term: '')
issues = get_issues(issue_status: issue_status, limit: limit) issues = get_issues(
issue_status: issue_status,
limit: limit,
search_term: search_term
)
validate_size(issues) validate_size(issues)
...@@ -71,13 +75,14 @@ module Sentry ...@@ -71,13 +75,14 @@ module Sentry
response = handle_request_exceptions do response = handle_request_exceptions do
Gitlab::HTTP.get(url, **request_params.merge(params)) Gitlab::HTTP.get(url, **request_params.merge(params))
end end
handle_response(response) handle_response(response)
end end
def get_issues(issue_status:, limit:) def get_issues(issue_status:, limit:, search_term: '')
query = "is:#{issue_status} #{search_term}".strip
http_get(issues_api_url, query: { http_get(issues_api_url, query: {
query: "is:#{issue_status}", query: query,
limit: limit limit: limit
}) })
end end
......
...@@ -48,15 +48,22 @@ describe Projects::ErrorTrackingController do ...@@ -48,15 +48,22 @@ describe Projects::ErrorTrackingController do
describe 'format json' do describe 'format json' do
let(:list_issues_service) { spy(:list_issues_service) } let(:list_issues_service) { spy(:list_issues_service) }
let(:external_url) { 'http://example.com' } let(:external_url) { 'http://example.com' }
let(:search_term) do
ActionController::Parameters.new(
search_term: 'something'
).permit!
end
context 'no data' do
let(:search_term) do
ActionController::Parameters.new({}).permit!
end
before do before do
expect(ErrorTracking::ListIssuesService) expect(ErrorTracking::ListIssuesService)
.to receive(:new).with(project, user) .to receive(:new).with(project, user, search_term)
.and_return(list_issues_service) .and_return(list_issues_service)
end
context 'no data' do
before do
expect(list_issues_service).to receive(:execute) expect(list_issues_service).to receive(:execute)
.and_return(status: :error, http_status: :no_content) .and_return(status: :error, http_status: :no_content)
end end
...@@ -68,6 +75,41 @@ describe Projects::ErrorTrackingController do ...@@ -68,6 +75,41 @@ describe Projects::ErrorTrackingController do
end end
end end
context 'with a search_term param' do
before do
expect(ErrorTracking::ListIssuesService)
.to receive(:new).with(project, user, search_term)
.and_return(list_issues_service)
end
context 'service result is successful' do
before do
expect(list_issues_service).to receive(:execute)
.and_return(status: :success, issues: [error])
expect(list_issues_service).to receive(:external_url)
.and_return(external_url)
end
let(:error) { build(:error_tracking_error) }
it 'returns a list of errors' do
get :index, params: project_params(format: :json, search_term: 'something')
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('error_tracking/index')
expect(json_response['external_url']).to eq(external_url)
expect(json_response['errors']).to eq([error].as_json)
end
end
end
context 'without a search_term param' do
before do
expect(ErrorTracking::ListIssuesService)
.to receive(:new).with(project, user, {})
.and_return(list_issues_service)
end
context 'service result is successful' do context 'service result is successful' do
before do before do
expect(list_issues_service).to receive(:execute) expect(list_issues_service).to receive(:execute)
...@@ -126,6 +168,7 @@ describe Projects::ErrorTrackingController do ...@@ -126,6 +168,7 @@ describe Projects::ErrorTrackingController do
end end
end end
end end
end
describe 'POST #list_projects' do describe 'POST #list_projects' do
context 'with insufficient permissions' do context 'with insufficient permissions' do
......
import { createLocalVue, shallowMount } from '@vue/test-utils'; import { createLocalVue, shallowMount } from '@vue/test-utils';
import Vuex from 'vuex'; import Vuex from 'vuex';
import ErrorTrackingList from '~/error_tracking/components/error_tracking_list.vue'; import ErrorTrackingList from '~/error_tracking/components/error_tracking_list.vue';
import { GlButton, GlEmptyState, GlLoadingIcon, GlTable, GlLink } from '@gitlab/ui'; import {
GlButton,
GlEmptyState,
GlLoadingIcon,
GlTable,
GlLink,
GlSearchBoxByClick,
} from '@gitlab/ui';
const localVue = createLocalVue(); const localVue = createLocalVue();
localVue.use(Vuex); localVue.use(Vuex);
...@@ -34,8 +41,8 @@ describe('ErrorTrackingList', () => { ...@@ -34,8 +41,8 @@ describe('ErrorTrackingList', () => {
beforeEach(() => { beforeEach(() => {
actions = { actions = {
getSentryData: () => {}, getErrorList: () => {},
startPolling: () => {}, startPolling: jest.fn(),
restartPolling: jest.fn().mockName('restartPolling'), restartPolling: jest.fn().mockName('restartPolling'),
}; };
...@@ -63,13 +70,13 @@ describe('ErrorTrackingList', () => { ...@@ -63,13 +70,13 @@ describe('ErrorTrackingList', () => {
describe('loading', () => { describe('loading', () => {
beforeEach(() => { beforeEach(() => {
store.state.list.loading = true;
mountComponent(); mountComponent();
}); });
it('shows spinner', () => { it('shows spinner', () => {
expect(wrapper.find(GlLoadingIcon).exists()).toBeTruthy(); expect(wrapper.find(GlLoadingIcon).exists()).toBeTruthy();
expect(wrapper.find(GlTable).exists()).toBeFalsy(); expect(wrapper.find(GlTable).exists()).toBeFalsy();
expect(wrapper.find(GlButton).exists()).toBeFalsy();
}); });
}); });
...@@ -85,6 +92,20 @@ describe('ErrorTrackingList', () => { ...@@ -85,6 +92,20 @@ describe('ErrorTrackingList', () => {
expect(wrapper.find(GlTable).exists()).toBeTruthy(); expect(wrapper.find(GlTable).exists()).toBeTruthy();
expect(wrapper.find(GlButton).exists()).toBeTruthy(); expect(wrapper.find(GlButton).exists()).toBeTruthy();
}); });
describe('filtering', () => {
it('shows search box', () => {
expect(wrapper.find(GlSearchBoxByClick).exists()).toBeTruthy();
});
it('makes network request on submit', () => {
expect(actions.startPolling).toHaveBeenCalledTimes(1);
wrapper.find(GlSearchBoxByClick).vm.$emit('submit');
expect(actions.startPolling).toHaveBeenCalledTimes(2);
});
});
}); });
describe('no results', () => { describe('no results', () => {
......
import axios from '~/lib/utils/axios_utils';
import MockAdapter from 'axios-mock-adapter';
import * as actions from '~/error_tracking/store/list/actions';
import * as types from '~/error_tracking/store/list/mutation_types';
describe('error tracking actions', () => {
let mock;
beforeEach(() => {
mock = new MockAdapter(axios);
});
afterEach(() => {
mock.restore();
});
describe('startPolling', () => {
it('commits SET_LOADING', () => {
mock.onGet().reply(200);
const endpoint = '/errors';
const commit = jest.fn();
const state = {};
actions.startPolling({ commit, state }, endpoint);
expect(commit).toHaveBeenCalledWith(types.SET_LOADING, true);
});
});
});
import * as getters from '~/error_tracking/store/list/getters';
describe('Error Tracking getters', () => {
let state;
const mockErrors = [
{ title: 'ActiveModel::MissingAttributeError: missing attribute: encrypted_password' },
{ title: 'Grape::Exceptions::MethodNotAllowed: Grape::Exceptions::MethodNotAllowed' },
{ title: 'NoMethodError: undefined method `sanitize_http_headers=' },
{ title: 'NoMethodError: undefined method `pry' },
];
beforeEach(() => {
state = {
errors: mockErrors,
};
});
describe('search results', () => {
it('should return errors filtered by words in title matching the query', () => {
const filteredErrors = getters.filterErrorsByTitle(state)('NoMethod');
expect(filteredErrors).not.toContainEqual(mockErrors[0]);
expect(filteredErrors.length).toBe(2);
});
it('should not return results if there is no matching query', () => {
const filteredErrors = getters.filterErrorsByTitle(state)('GitLab');
expect(filteredErrors.length).toBe(0);
});
});
});
...@@ -88,12 +88,13 @@ describe Sentry::Client do ...@@ -88,12 +88,13 @@ describe Sentry::Client do
describe '#list_issues' do describe '#list_issues' do
let(:issue_status) { 'unresolved' } let(:issue_status) { 'unresolved' }
let(:limit) { 20 } let(:limit) { 20 }
let(:search_term) { '' }
let(:sentry_api_response) { issues_sample_response } let(:sentry_api_response) { issues_sample_response }
let(:sentry_request_url) { sentry_url + '/issues/?limit=20&query=is:unresolved' } let(:sentry_request_url) { sentry_url + '/issues/?limit=20&query=is:unresolved' }
let!(:sentry_api_request) { stub_sentry_request(sentry_request_url, body: sentry_api_response) } let!(:sentry_api_request) { stub_sentry_request(sentry_request_url, body: sentry_api_response) }
subject { client.list_issues(issue_status: issue_status, limit: limit) } subject { client.list_issues(issue_status: issue_status, limit: limit, search_term: search_term) }
it_behaves_like 'calls sentry api' it_behaves_like 'calls sentry api'
...@@ -202,6 +203,16 @@ describe Sentry::Client do ...@@ -202,6 +203,16 @@ describe Sentry::Client do
end end
it_behaves_like 'maps exceptions' it_behaves_like 'maps exceptions'
context 'when search term is present' do
let(:search_term) { 'NoMethodError'}
let(:sentry_request_url) { "#{sentry_url}/issues/?limit=20&query=is:unresolved NoMethodError" }
it_behaves_like 'calls sentry api'
it_behaves_like 'has correct return type', Gitlab::ErrorTracking::Error
it_behaves_like 'has correct length', 1
end
end end
describe '#list_projects' do describe '#list_projects' do
......
...@@ -5,6 +5,14 @@ require 'spec_helper' ...@@ -5,6 +5,14 @@ require 'spec_helper'
describe ErrorTracking::ListIssuesService do describe ErrorTracking::ListIssuesService do
set(:user) { create(:user) } set(:user) { create(:user) }
set(:project) { create(:project) } set(:project) { create(:project) }
let(:params) { { search_term: 'something' } }
let(:list_sentry_issues_args) do
{
issue_status: 'unresolved',
limit: 20,
search_term: params[:search_term]
}
end
let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project' } let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project' }
let(:token) { 'test-token' } let(:token) { 'test-token' }
...@@ -14,7 +22,7 @@ describe ErrorTracking::ListIssuesService do ...@@ -14,7 +22,7 @@ describe ErrorTracking::ListIssuesService do
create(:project_error_tracking_setting, api_url: sentry_url, token: token, project: project) create(:project_error_tracking_setting, api_url: sentry_url, token: token, project: project)
end end
subject { described_class.new(project, user) } subject { described_class.new(project, user, params) }
before do before do
expect(project).to receive(:error_tracking_setting).at_least(:once).and_return(error_tracking_setting) expect(project).to receive(:error_tracking_setting).at_least(:once).and_return(error_tracking_setting)
...@@ -29,7 +37,9 @@ describe ErrorTracking::ListIssuesService do ...@@ -29,7 +37,9 @@ describe ErrorTracking::ListIssuesService do
before do before do
expect(error_tracking_setting) expect(error_tracking_setting)
.to receive(:list_sentry_issues).and_return(issues: issues) .to receive(:list_sentry_issues)
.with(list_sentry_issues_args)
.and_return(issues: issues)
end end
it 'returns the issues' do it 'returns the issues' do
......
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