Merge branch '34068-sort-error-list' into 'master'

Sort Sentry error list by first seen, last seen or frequency

See merge request gitlab-org/gitlab!21250

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

@@ -17,8 +17,6 @@ import AccessorUtils from '~/lib/utils/accessor';
 import Icon from '~/vue_shared/components/icon.vue';
 import TimeAgo from '~/vue_shared/components/time_ago_tooltip.vue';
 import { __ } from '~/locale';
-import TrackEventDirective from '~/vue_shared/directives/track_event';
-import { trackViewInSentryOptions } from '../utils';
 
 export default {
   fields: [
@@ -27,6 +25,11 @@ export default {
     { key: 'users', label: __('Users') },
     { key: 'lastSeen', label: __('Last seen'), thClass: 'w-15p' },
   ],
+  sortFields: {
+    last_seen: __('Last Seen'),
+    first_seen: __('First Seen'),
+    frequency: __('Frequency'),
+  },
   components: {
     GlEmptyState,
     GlButton,
@@ -43,7 +46,6 @@ export default {
   },
   directives: {
     GlTooltip: GlTooltipDirective,
-    TrackEvent: TrackEventDirective,
   },
   props: {
     indexPath: {
@@ -74,45 +76,47 @@ export default {
     };
   },
   computed: {
-    ...mapState('list', ['errors', 'externalUrl', 'loading', 'recentSearches']),
+    ...mapState('list', ['errors', 'loading', 'searchQuery', 'sortField', 'recentSearches']),
   },
   created() {
     if (this.errorTrackingEnabled) {
-      this.startPolling(this.indexPath);
+      this.setEndpoint(this.indexPath);
+      this.startPolling();
     }
   },
   methods: {
     ...mapActions('list', [
       'startPolling',
       'restartPolling',
+      'setEndpoint',
+      'searchByQuery',
+      'sortByField',
       'addRecentSearch',
       'clearRecentSearches',
       'loadRecentSearches',
       'setIndexPath',
     ]),
-    filterErrors() {
-      const searchTerm = this.errorSearchQuery.trim();
-      this.addRecentSearch(searchTerm);
-
-      this.startPolling(`${this.indexPath}?search_term=${searchTerm}`);
-    },
     setSearchText(text) {
       this.errorSearchQuery = text;
-      this.filterErrors();
+      this.searchByQuery(text);
     },
-    trackViewInSentryOptions,
     getDetailsLink(errorId) {
       return `error_tracking/${errorId}/details`;
     },
+    isCurrentSortField(field) {
+      return field === this.sortField;
+    },
   },
 };
 </script>
 
 <template>
-  <div>
+  <div class="error-list">
     <div v-if="errorTrackingEnabled">
-      <div class="d-flex flex-row justify-content-around bg-secondary border p-3">
-        <div class="filtered-search-box">
+      <div
+        class="d-flex flex-row justify-content-around align-items-center bg-secondary border mt-2"
+      >
+        <div class="filtered-search-box flex-grow-1 my-3 ml-3 mr-2">
           <gl-dropdown
             :text="__('Recent searches')"
             class="filtered-search-history-dropdown-wrapper d-none d-md-block"
@@ -143,7 +147,7 @@ export default {
               :disabled="loading"
               :placeholder="__('Search or filter results…')"
               autofocus
-              @keyup.enter.native="filterErrors"
+              @keyup.enter.native="searchByQuery(errorSearchQuery)"
             />
           </div>
           <div class="gl-search-box-by-type-right-icons">
@@ -160,16 +164,28 @@ export default {
           </div>
         </div>
 
-        <gl-button
-          v-track-event="trackViewInSentryOptions(externalUrl)"
-          class="ml-3"
-          variant="primary"
-          :href="externalUrl"
-          target="_blank"
+        <gl-dropdown
+          :text="$options.sortFields[sortField]"
+          left
+          :disabled="loading"
+          class="mr-3"
+          menu-class="sort-dropdown"
         >
-          {{ __('View in Sentry') }}
-          <icon name="external-link" class="flex-shrink-0" />
-        </gl-button>
+          <gl-dropdown-item
+            v-for="(label, field) in $options.sortFields"
+            :key="field"
+            @click="sortByField(field)"
+          >
+            <span class="d-flex">
+              <icon
+                class="flex-shrink-0 append-right-4"
+                :class="{ invisible: !isCurrentSortField(field) }"
+                name="mobile-issue-close"
+              />
+              {{ label }}
+            </span>
+          </gl-dropdown-item>
+        </gl-dropdown>
       </div>
 
       <div v-if="loading" class="py-3">

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

 import axios from '~/lib/utils/axios_utils';
 
 export default {
-  getSentryData({ endpoint }) {
-    return axios.get(endpoint);
+  getSentryData({ endpoint, params }) {
+    return axios.get(endpoint, { params });
   },
 };

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

@@ -6,19 +6,24 @@ import { __, sprintf } from '~/locale';
 
 let eTagPoll;
 
-export function startPolling({ commit, dispatch }, endpoint) {
+export function startPolling({ state, commit, dispatch }) {
   commit(types.SET_LOADING, true);
 
   eTagPoll = new Poll({
     resource: Service,
     method: 'getSentryData',
-    data: { endpoint },
+    data: {
+      endpoint: state.endpoint,
+      params: {
+        search_term: state.searchQuery,
+        sort: state.sortField,
+      },
+    },
     successCallback: ({ data }) => {
       if (!data) {
         return;
       }
       commit(types.SET_ERRORS, data.errors);
-      commit(types.SET_EXTERNAL_URL, data.external_url);
       commit(types.SET_LOADING, false);
       dispatch('stopPolling');
     },
@@ -45,7 +50,6 @@ export const stopPolling = () => {
 
 export function restartPolling({ commit }) {
   commit(types.SET_ERRORS, []);
-  commit(types.SET_EXTERNAL_URL, '');
   commit(types.SET_LOADING, true);
 
   if (eTagPoll) eTagPoll.restart();
@@ -67,4 +71,22 @@ export function clearRecentSearches({ commit }) {
   commit(types.CLEAR_RECENT_SEARCHES);
 }
 
+export const searchByQuery = ({ commit, dispatch }, query) => {
+  const searchQuery = query.trim();
+  commit(types.SET_SEARCH_QUERY, searchQuery);
+  commit(types.ADD_RECENT_SEARCH, searchQuery);
+  dispatch('stopPolling');
+  dispatch('startPolling');
+};
+
+export const sortByField = ({ commit, dispatch }, field) => {
+  commit(types.SET_SORT_FIELD, field);
+  dispatch('stopPolling');
+  dispatch('startPolling');
+};
+
+export const setEndpoint = ({ commit }, endpoint) => {
+  commit(types.SET_ENDPOINT, endpoint);
+};
+
 export default () => {};

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

 export const SET_ERRORS = 'SET_ERRORS';
-export const SET_EXTERNAL_URL = 'SET_EXTERNAL_URL';
 export const SET_INDEX_PATH = 'SET_INDEX_PATH';
 export const SET_LOADING = 'SET_LOADING';
 export const ADD_RECENT_SEARCH = 'ADD_RECENT_SEARCH';
 export const CLEAR_RECENT_SEARCHES = 'CLEAR_RECENT_SEARCHES';
 export const LOAD_RECENT_SEARCHES = 'LOAD_RECENT_SEARCHES';
+export const SET_ENDPOINT = 'SET_ENDPOINT';
+export const SET_SORT_FIELD = 'SET_SORT_FIELD';
+export const SET_SEARCH_QUERY = 'SET_SEARCH_QUERY';

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

@@ -6,9 +6,6 @@ export default {
   [types.SET_ERRORS](state, data) {
     state.errors = convertObjectPropsToCamelCase(data, { deep: true });
   },
-  [types.SET_EXTERNAL_URL](state, url) {
-    state.externalUrl = url;
-  },
   [types.SET_LOADING](state, loading) {
     state.loading = loading;
   },
@@ -47,4 +44,13 @@ export default {
       throw e;
     }
   },
+  [types.SET_SORT_FIELD](state, field) {
+    state.sortField = field;
+  },
+  [types.SET_SEARCH_QUERY](state, query) {
+    state.searchQuery = query;
+  },
+  [types.SET_ENDPOINT](state, endpoint) {
+    state.endpoint = endpoint;
+  },
 };

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

 export default () => ({
   errors: [],
-  externalUrl: '',
   loading: true,
+  endpoint: null,
+  sortField: 'last_seen',
+  searchQuery: null,
   indexPath: '',
   recentSearches: [],
 });

app/assets/javascripts/error_tracking/utils.js

-/* eslint-disable @gitlab/i18n/no-non-i18n-strings */
-
-/**
- * Tracks snowplow event when user clicks View in Sentry btn
- * @param {String}  externalUrl that will be send as a property for the event
- */
-export const trackViewInSentryOptions = url => ({
-  category: 'Error Tracking',
-  action: 'click_view_in_sentry',
-  label: 'External Url',
-  property: url,
-});
+/* eslint-disable @gitlab/i18n/no-non-i18n-strings, import/prefer-default-export */
 
 /**
  * Tracks snowplow event when User clicks on error link to Sentry

app/assets/stylesheets/pages/error_tracking_list.scss

+.error-list {
+  .sort-dropdown {
+    min-width: auto;
+  }
+}

changelogs/unreleased/34068-sort-error-list.yml

+---
+title: Sort Sentry error list by first seen, last seen or frequency
+merge_request: 21250
+author:
+type: added

locale/gitlab.pot

@@ -7735,6 +7735,9 @@ msgstr ""
 msgid "Finished"
 msgstr ""
 
+msgid "First Seen"
+msgstr ""
+
 msgid "First day of the week"
 msgstr ""
 
@@ -7861,6 +7864,9 @@ msgstr ""
 msgid "Free Trial of GitLab.com Gold"
 msgstr ""
 
+msgid "Frequency"
+msgstr ""
+
 msgid "Friday"
 msgstr ""
 
@@ -10201,6 +10207,9 @@ msgstr ""
 msgid "Last Pipeline"
 msgstr ""
 
+msgid "Last Seen"
+msgstr ""
+
 msgid "Last accessed on"
 msgstr ""
 
@@ -19762,9 +19771,6 @@ msgstr ""
 msgid "View group labels"
 msgstr ""
 
-msgid "View in Sentry"
-msgstr ""
-
 msgid "View it on GitLab"
 msgstr ""
 

spec/frontend/error_tracking/components/error_tracking_list_spec.js

 import { createLocalVue, shallowMount } from '@vue/test-utils';
 import Vuex from 'vuex';
 import {
-  GlButton,
   GlEmptyState,
   GlLoadingIcon,
   GlTable,
@@ -24,7 +23,9 @@ describe('ErrorTrackingList', () => {
 
   const findErrorListTable = () => wrapper.find('table');
   const findErrorListRows = () => wrapper.findAll('tbody tr');
-  const findButton = () => wrapper.find(GlButton);
+  const findSortDropdown = () => wrapper.find('.sort-dropdown');
+  const findRecentSearchesDropdown = () =>
+    wrapper.find('.filtered-search-history-dropdown-wrapper');
   const findLoadingIcon = () => wrapper.find(GlLoadingIcon);
 
   function mountComponent({
@@ -33,6 +34,8 @@ describe('ErrorTrackingList', () => {
     stubs = {
       'gl-link': GlLink,
       'gl-table': GlTable,
+      'gl-dropdown': GlDropdown,
+      'gl-dropdown-item': GlDropdownItem,
     },
   } = {}) {
     wrapper = shallowMount(ErrorTrackingList, {
@@ -46,6 +49,9 @@ describe('ErrorTrackingList', () => {
         illustrationPath: 'illustration/path',
       },
       stubs,
+      data() {
+        return { errorSearchQuery: 'search' };
+      },
     });
   }
 
@@ -58,6 +64,9 @@ describe('ErrorTrackingList', () => {
       loadRecentSearches: jest.fn(),
       setIndexPath: jest.fn(),
       clearRecentSearches: jest.fn(),
+      setEndpoint: jest.fn(),
+      searchByQuery: jest.fn(),
+      sortByField: jest.fn(),
     };
 
     const state = createListState();
@@ -101,7 +110,7 @@ describe('ErrorTrackingList', () => {
     it('shows table', () => {
       expect(findLoadingIcon().exists()).toBe(false);
       expect(findErrorListTable().exists()).toBe(true);
-      expect(findButton().exists()).toBe(true);
+      expect(findSortDropdown().exists()).toBe(true);
     });
 
     it('shows list of errors in a table', () => {
@@ -121,16 +130,20 @@ describe('ErrorTrackingList', () => {
     describe('filtering', () => {
       const findSearchBox = () => wrapper.find(GlFormInput);
 
-      it('shows search box', () => {
+      it('shows search box & sort dropdown', () => {
         expect(findSearchBox().exists()).toBe(true);
+        expect(findSortDropdown().exists()).toBe(true);
       });
 
-      it('makes network request on submit', () => {
-        expect(actions.startPolling).toHaveBeenCalledTimes(1);
-
+      it('it searches by query', () => {
         findSearchBox().trigger('keyup.enter');
+        expect(actions.searchByQuery.mock.calls[0][1]).toEqual(wrapper.vm.errorSearchQuery);
+      });
 
-        expect(actions.startPolling).toHaveBeenCalledTimes(2);
+      it('it sorts by fields', () => {
+        const findSortItem = () => wrapper.find('.dropdown-item');
+        findSortItem().trigger('click');
+        expect(actions.sortByField).toHaveBeenCalled();
       });
     });
   });
@@ -148,7 +161,7 @@ describe('ErrorTrackingList', () => {
     it('shows empty table', () => {
       expect(findLoadingIcon().exists()).toBe(false);
       expect(findErrorListRows().length).toEqual(1);
-      expect(findButton().exists()).toBe(true);
+      expect(findSortDropdown().exists()).toBe(true);
     });
 
     it('shows a message prompting to refresh', () => {
@@ -170,7 +183,7 @@ describe('ErrorTrackingList', () => {
       expect(wrapper.find(GlEmptyState).exists()).toBe(true);
       expect(findLoadingIcon().exists()).toBe(false);
       expect(findErrorListTable().exists()).toBe(false);
-      expect(findButton().exists()).toBe(false);
+      expect(findSortDropdown().exists()).toBe(false);
     });
   });
 
@@ -201,13 +214,13 @@ describe('ErrorTrackingList', () => {
     it('shows empty message', () => {
       store.state.list.recentSearches = [];
 
-      expect(wrapper.find(GlDropdown).text()).toBe("You don't have any recent searches");
+      expect(findRecentSearchesDropdown().text()).toContain("You don't have any recent searches");
     });
 
     it('shows items', () => {
       store.state.list.recentSearches = ['great', 'search'];
 
-      const dropdownItems = wrapper.findAll(GlDropdownItem);
+      const dropdownItems = wrapper.findAll('.filtered-search-box li');
 
       expect(dropdownItems.length).toBe(3);
       expect(dropdownItems.at(0).text()).toBe('great');

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

-import MockAdapter from 'axios-mock-adapter';
 import axios from '~/lib/utils/axios_utils';
+import MockAdapter from 'axios-mock-adapter';
+import testAction from 'helpers/vuex_action_helper';
+import httpStatusCodes from '~/lib/utils/http_status';
+import createFlash from '~/flash';
 import * as actions from '~/error_tracking/store/list/actions';
 import * as types from '~/error_tracking/store/list/mutation_types';
 
+jest.mock('~/flash.js');
+
 describe('error tracking actions', () => {
   let mock;
 
@@ -15,15 +20,97 @@ describe('error tracking actions', () => {
   });
 
   describe('startPolling', () => {
-    it('commits SET_LOADING', () => {
-      mock.onGet().reply(200);
-      const endpoint = '/errors';
-      const commit = jest.fn();
-      const state = {};
+    it('should start polling for data', done => {
+      const payload = { errors: [{ id: 1 }, { id: 2 }] };
+
+      mock.onGet().reply(httpStatusCodes.OK, payload);
+      testAction(
+        actions.startPolling,
+        {},
+        {},
+        [
+          { type: types.SET_LOADING, payload: true },
+          { type: types.SET_ERRORS, payload: payload.errors },
+          { type: types.SET_LOADING, payload: false },
+        ],
+        [{ type: 'stopPolling' }],
+        () => {
+          done();
+        },
+      );
+    });
+
+    it('should show flash on API error', done => {
+      mock.onGet().reply(httpStatusCodes.BAD_REQUEST);
+
+      testAction(
+        actions.startPolling,
+        {},
+        {},
+        [{ type: types.SET_LOADING, payload: true }, { type: types.SET_LOADING, payload: false }],
+        [],
+        () => {
+          expect(createFlash).toHaveBeenCalledTimes(1);
+          done();
+        },
+      );
+    });
+  });
+
+  describe('restartPolling', () => {
+    it('should restart polling', () => {
+      testAction(
+        actions.restartPolling,
+        {},
+        {},
+        [{ type: types.SET_ERRORS, payload: [] }, { type: types.SET_LOADING, payload: true }],
+        [],
+      );
+    });
+  });
+
+  describe('searchByQuery', () => {
+    it('should search by query', () => {
+      const query = 'search';
+
+      testAction(
+        actions.searchByQuery,
+        query,
+        {},
+        [
+          { type: types.SET_SEARCH_QUERY, payload: query },
+          { type: types.ADD_RECENT_SEARCH, payload: query },
+        ],
+        [{ type: 'stopPolling' }, { type: 'startPolling' }],
+      );
+    });
+  });
+
+  describe('sortByField', () => {
+    it('should search by query', () => {
+      const field = 'frequency';
+
+      testAction(
+        actions.sortByField,
+        { field },
+        {},
+        [{ type: types.SET_SORT_FIELD, payload: { field } }],
+        [{ type: 'stopPolling' }, { type: 'startPolling' }],
+      );
+    });
+  });
 
-      actions.startPolling({ commit, state }, endpoint);
+  describe('setEnpoint', () => {
+    it('should set search endpoint', () => {
+      const endpoint = 'https://sentry.io';
 
-      expect(commit).toHaveBeenCalledWith(types.SET_LOADING, true);
+      testAction(
+        actions.setEndpoint,
+        { endpoint },
+        {},
+        [{ type: types.SET_ENDPOINT, payload: { endpoint } }],
+        [],
+      );
     });
   });
 });

spec/frontend/error_tracking/utils_spec.js

@@ -3,17 +3,6 @@ import * as errorTrackingUtils from '~/error_tracking/utils';
 const externalUrl = 'https://sentry.io/organizations/test-sentry-nk/issues/1/?project=1';
 
 describe('Error Tracking Events', () => {
-  describe('trackViewInSentryOptions', () => {
-    it('should return correct event options', () => {
-      expect(errorTrackingUtils.trackViewInSentryOptions(externalUrl)).toEqual({
-        category: 'Error Tracking',
-        action: 'click_view_in_sentry',
-        label: 'External Url',
-        property: externalUrl,
-      });
-    });
-  });
-
   describe('trackClickErrorLinkToSentryOptions', () => {
     it('should return correct event options', () => {
       expect(errorTrackingUtils.trackClickErrorLinkToSentryOptions(externalUrl)).toEqual({