Merge branch 'fe-search-list-of-sentry-errors' into 'master'

Reimplement search list of sentry errors

See merge request gitlab-org/gitlab!19666

app/assets/javascripts/error_tracking/components/error_tracking_list.vue

 <script>
-import { mapActions, mapState, mapGetters } from 'vuex';
+import { mapActions, mapState } from 'vuex';
 import {
   GlEmptyState,
   GlButton,
   GlLink,
   GlLoadingIcon,
   GlTable,
-  GlSearchBoxByType,
+  GlSearchBoxByClick,
 } from '@gitlab/ui';
 import { visitUrl } from '~/lib/utils/url_utility';
 import Icon from '~/vue_shared/components/icon.vue';
@@ -28,7 +28,7 @@ export default {
     GlLink,
     GlLoadingIcon,
     GlTable,
-    GlSearchBoxByType,
+    GlSearchBoxByClick,
     Icon,
     TimeAgo,
   },
@@ -64,10 +64,6 @@ export default {
   },
   computed: {
     ...mapState('list', ['errors', 'externalUrl', 'loading']),
-    ...mapGetters('list', ['filterErrorsByTitle']),
-    filteredErrors() {
-      return this.errorSearchQuery ? this.filterErrorsByTitle(this.errorSearchQuery) : this.errors;
-    },
   },
   created() {
     if (this.errorTrackingEnabled) {
@@ -76,6 +72,9 @@ export default {
   },
   methods: {
     ...mapActions('list', ['startPolling', 'restartPolling']),
+    filterErrors() {
+      this.startPolling(`${this.indexPath}?search_term=${this.errorSearchQuery}`);
+    },
     trackViewInSentryOptions,
     viewDetails(errorId) {
       visitUrl(`error_tracking/${errorId}/details`);
@@ -87,17 +86,15 @@ export default {
 <template>
   <div>
     <div v-if="errorTrackingEnabled">
-      <div v-if="loading" class="py-3">
-        <gl-loading-icon :size="3" />
-      </div>
-      <div v-else>
+      <div>
         <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"
             class="col-lg-10 m-3 p-0"
             :placeholder="__('Search or filter results...')"
             type="search"
             autofocus
+            @submit="filterErrors"
           />
           <gl-button
             v-track-event="trackViewInSentryOptions(externalUrl)"
@@ -111,9 +108,14 @@ export default {
           </gl-button>
         </div>
 
+        <div v-if="loading" class="py-3">
+          <gl-loading-icon size="md" />
+        </div>
+
         <gl-table
+          v-else
           class="mt-3"
-          :items="filteredErrors"
+          :items="errors"
           :fields="$options.fields"
           :show-empty="true"
           fixed

app/assets/javascripts/error_tracking/store/index.js

@@ -4,7 +4,6 @@ import Vuex from 'vuex';
 import * as listActions from './list/actions';
 import listMutations from './list/mutations';
 import listState from './list/state';
-import * as listGetters from './list/getters';
 
 import * as detailsActions from './details/actions';
 import detailsMutations from './details/mutations';
@@ -21,7 +20,6 @@ export const createStore = () =>
         state: listState(),
         actions: listActions,
         mutations: listMutations,
-        getters: listGetters,
       },
       details: {
         namespaced: true,

app/assets/javascripts/error_tracking/store/list/actions.js

@@ -7,6 +7,8 @@ import { __, sprintf } from '~/locale';
 let eTagPoll;
 
 export function startPolling({ commit, dispatch }, endpoint) {
+  commit(types.SET_LOADING, true);
+
   eTagPoll = new Poll({
     resource: Service,
     method: 'getSentryData',

app/assets/javascripts/error_tracking/store/list/getters.js

-export const filterErrorsByTitle = state => errorQuery =>
-  state.errors.filter(error => error.title.match(new RegExp(`${errorQuery}`, 'i')));
-
-export default () => {};

app/controllers/projects/error_tracking_controller.rb

@@ -44,7 +44,11 @@ class Projects::ErrorTrackingController < Projects::ApplicationController
   private
 
   def render_index_json
-    service = ErrorTracking::ListIssuesService.new(project, current_user)
+    service = ErrorTracking::ListIssuesService.new(
+      project,
+      current_user,
+      list_issues_params
+    )
     result = service.execute
 
     return if handle_errors(result)
@@ -106,6 +110,10 @@ class Projects::ErrorTrackingController < Projects::ApplicationController
     end
   end
 
+  def list_issues_params
+    params.permit(:search_term)
+  end
+
   def list_projects_params
     params.require(:error_tracking_setting).permit([:api_host, :token])
   end

app/services/error_tracking/list_issues_service.rb

@@ -5,6 +5,28 @@ module ErrorTracking
     DEFAULT_ISSUE_STATUS = 'unresolved'
     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
       project_error_tracking_setting&.sentry_external_url
     end
@@ -26,5 +48,17 @@ module ErrorTracking
     def limit
       params[:limit] || DEFAULT_LIMIT
     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

changelogs/unreleased/search-list-of-sentry-errors.yml

+---
+title: Search list of Sentry errors by title in GitLab
+merge_request: 19439
+author:
+type: added

lib/sentry/client.rb

@@ -25,8 +25,12 @@ module Sentry
       map_to_event(latest_event)
     end
 
-    def list_issues(issue_status:, limit:)
-      issues = get_issues(issue_status: issue_status, limit: limit)
+    def list_issues(issue_status:, limit:, search_term: '')
+      issues = get_issues(
+        issue_status: issue_status,
+        limit: limit,
+        search_term: search_term
+      )
 
       validate_size(issues)
 
@@ -71,13 +75,14 @@ module Sentry
       response = handle_request_exceptions do
         Gitlab::HTTP.get(url, **request_params.merge(params))
       end
-
       handle_response(response)
     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: {
-        query: "is:#{issue_status}",
+        query: query,
         limit: limit
       })
     end

spec/controllers/projects/error_tracking_controller_spec.rb

@@ -48,15 +48,22 @@ describe Projects::ErrorTrackingController do
     describe 'format json' do
       let(:list_issues_service) { spy(:list_issues_service) }
       let(:external_url) { 'http://example.com' }
-
-      before do
-        expect(ErrorTracking::ListIssuesService)
-          .to receive(:new).with(project, user)
-          .and_return(list_issues_service)
+      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
+          expect(ErrorTracking::ListIssuesService)
+            .to receive(:new).with(project, user, search_term)
+            .and_return(list_issues_service)
+
           expect(list_issues_service).to receive(:execute)
             .and_return(status: :error, http_status: :no_content)
         end
@@ -68,59 +75,95 @@ describe Projects::ErrorTrackingController do
         end
       end
 
-      context 'service result is successful' do
+      context 'with a search_term param' 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)
+          expect(ErrorTracking::ListIssuesService)
+            .to receive(:new).with(project, user, search_term)
+            .and_return(list_issues_service)
         end
 
-        let(:error) { build(:error_tracking_error) }
+        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
 
-        it 'returns a list of errors' do
-          get :index, params: project_params(format: :json)
+          let(:error) { build(:error_tracking_error) }
 
-          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)
+          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 'service result is erroneous' do
-        let(:error_message) { 'error message' }
+      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 'without http_status' do
+        context 'service result is successful' do
           before do
             expect(list_issues_service).to receive(:execute)
-              .and_return(status: :error, message: error_message)
+              .and_return(status: :success, issues: [error])
+            expect(list_issues_service).to receive(:external_url)
+              .and_return(external_url)
           end
 
-          it 'returns 400 with message' do
+          let(:error) { build(:error_tracking_error) }
+
+          it 'returns a list of errors' do
             get :index, params: project_params(format: :json)
 
-            expect(response).to have_gitlab_http_status(:bad_request)
-            expect(json_response['message']).to eq(error_message)
+            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
 
-        context 'with explicit http_status' do
-          let(:http_status) { :no_content }
+        context 'service result is erroneous' do
+          let(:error_message) { 'error message' }
 
-          before do
-            expect(list_issues_service).to receive(:execute).and_return(
-              status: :error,
-              message: error_message,
-              http_status: http_status
-            )
+          context 'without http_status' do
+            before do
+              expect(list_issues_service).to receive(:execute)
+                .and_return(status: :error, message: error_message)
+            end
+
+            it 'returns 400 with message' do
+              get :index, params: project_params(format: :json)
+
+              expect(response).to have_gitlab_http_status(:bad_request)
+              expect(json_response['message']).to eq(error_message)
+            end
           end
 
-          it 'returns http_status with message' do
-            get :index, params: project_params(format: :json)
+          context 'with explicit http_status' do
+            let(:http_status) { :no_content }
 
-            expect(response).to have_gitlab_http_status(http_status)
-            expect(json_response['message']).to eq(error_message)
+            before do
+              expect(list_issues_service).to receive(:execute).and_return(
+                status: :error,
+                message: error_message,
+                http_status: http_status
+              )
+            end
+
+            it 'returns http_status with message' do
+              get :index, params: project_params(format: :json)
+
+              expect(response).to have_gitlab_http_status(http_status)
+              expect(json_response['message']).to eq(error_message)
+            end
           end
         end
       end

spec/frontend/error_tracking/components/error_tracking_list_spec.js

 import { createLocalVue, shallowMount } from '@vue/test-utils';
 import Vuex from 'vuex';
 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();
 localVue.use(Vuex);
@@ -34,8 +41,8 @@ describe('ErrorTrackingList', () => {
 
   beforeEach(() => {
     actions = {
-      getSentryData: () => {},
-      startPolling: () => {},
+      getErrorList: () => {},
+      startPolling: jest.fn(),
       restartPolling: jest.fn().mockName('restartPolling'),
     };
 
@@ -63,13 +70,13 @@ describe('ErrorTrackingList', () => {
 
   describe('loading', () => {
     beforeEach(() => {
+      store.state.list.loading = true;
       mountComponent();
     });
 
     it('shows spinner', () => {
       expect(wrapper.find(GlLoadingIcon).exists()).toBeTruthy();
       expect(wrapper.find(GlTable).exists()).toBeFalsy();
-      expect(wrapper.find(GlButton).exists()).toBeFalsy();
     });
   });
 
@@ -85,6 +92,20 @@ describe('ErrorTrackingList', () => {
       expect(wrapper.find(GlTable).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', () => {

spec/frontend/error_tracking/store/list/actions_spec.js

+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);
+    });
+  });
+});

spec/frontend/error_tracking/store/list/getters_spec.js

-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);
-    });
-  });
-});

spec/lib/sentry/client_spec.rb

@@ -88,12 +88,13 @@ describe Sentry::Client do
   describe '#list_issues' do
     let(:issue_status) { 'unresolved' }
     let(:limit) { 20 }
+    let(:search_term) { '' }
     let(:sentry_api_response) { issues_sample_response }
     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) }
 
-    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'
 
@@ -202,6 +203,16 @@ describe Sentry::Client do
     end
 
     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
 
   describe '#list_projects' do

spec/services/error_tracking/list_issues_service_spec.rb

@@ -5,6 +5,14 @@ require 'spec_helper'
 describe ErrorTracking::ListIssuesService do
   set(:user) { create(:user) }
   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(:token) { 'test-token' }
@@ -14,7 +22,7 @@ describe ErrorTracking::ListIssuesService do
     create(:project_error_tracking_setting, api_url: sentry_url, token: token, project: project)
   end
 
-  subject { described_class.new(project, user) }
+  subject { described_class.new(project, user, params) }
 
   before do
     expect(project).to receive(:error_tracking_setting).at_least(:once).and_return(error_tracking_setting)
@@ -29,7 +37,9 @@ describe ErrorTracking::ListIssuesService do
 
         before do
           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
 
         it 'returns the issues' do