Merge branch '37006-fix-open-details-page-in-new-tab' into 'master'

Fix opening Sentry error details in new tab

See merge request gitlab-org/gitlab!20611

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

@@ -8,7 +8,6 @@ import {
   GlTable,
   GlSearchBoxByClick,
 } from '@gitlab/ui';
-import { visitUrl } from '~/lib/utils/url_utility';
 import Icon from '~/vue_shared/components/icon.vue';
 import TimeAgo from '~/vue_shared/components/time_ago_tooltip.vue';
 import { __ } from '~/locale';
@@ -76,8 +75,8 @@ export default {
       this.startPolling(`${this.indexPath}?search_term=${this.errorSearchQuery}`);
     },
     trackViewInSentryOptions,
-    viewDetails(errorId) {
-      visitUrl(`error_tracking/${errorId}/details`);
+    getDetailsLink(errorId) {
+      return `error_tracking/${errorId}/details`;
     },
   },
 };
@@ -129,11 +128,7 @@ export default {
           </template>
           <template slot="error" slot-scope="errors">
             <div class="d-flex flex-column">
-              <gl-link
-                class="d-flex text-dark"
-                target="_blank"
-                @click="viewDetails(errors.item.id)"
-              >
+              <gl-link class="d-flex text-dark" :href="getDetailsLink(errors.item.id)">
                 <strong class="text-truncate">{{ errors.item.title.trim() }}</strong>
               </gl-link>
               <span class="text-secondary text-truncate">

changelogs/unreleased/37006-fix-open-details-page-in-new-tab.yml

+---
+title: Fix opening Sentry error details in new tab
+merge_request: 20611
+author:
+type: fixed

spec/frontend/error_tracking/components/error_tracking_list_spec.js

@@ -9,6 +9,7 @@ import {
   GlLink,
   GlSearchBoxByClick,
 } from '@gitlab/ui';
+import errorsList from './list_mock.json';
 
 const localVue = createLocalVue();
 localVue.use(Vuex);
@@ -18,11 +19,17 @@ describe('ErrorTrackingList', () => {
   let wrapper;
   let actions;
 
+  const findErrorListTable = () => wrapper.find('table');
+  const findErrorListRows = () => wrapper.findAll('tbody tr');
+  const findButton = () => wrapper.find(GlButton);
+  const findLoadingIcon = () => wrapper.find(GlLoadingIcon);
+
   function mountComponent({
     errorTrackingEnabled = true,
     userCanEnableErrorTracking = true,
     stubs = {
       'gl-link': GlLink,
+      'gl-table': GlTable,
     },
   } = {}) {
     wrapper = shallowMount(ErrorTrackingList, {
@@ -47,7 +54,7 @@ describe('ErrorTrackingList', () => {
     };
 
     const state = {
-      errors: [],
+      errors: errorsList,
       loading: true,
     };
 
@@ -75,61 +82,74 @@ describe('ErrorTrackingList', () => {
     });
 
     it('shows spinner', () => {
-      expect(wrapper.find(GlLoadingIcon).exists()).toBeTruthy();
-      expect(wrapper.find(GlTable).exists()).toBeFalsy();
+      expect(findLoadingIcon().exists()).toBe(true);
+      expect(findErrorListTable().exists()).toBe(false);
     });
   });
 
   describe('results', () => {
     beforeEach(() => {
       store.state.list.loading = false;
-
       mountComponent();
     });
 
     it('shows table', () => {
-      expect(wrapper.find(GlLoadingIcon).exists()).toBeFalsy();
-      expect(wrapper.find(GlTable).exists()).toBeTruthy();
-      expect(wrapper.find(GlButton).exists()).toBeTruthy();
+      expect(findLoadingIcon().exists()).toBe(false);
+      expect(findErrorListTable().exists()).toBe(true);
+      expect(findButton().exists()).toBe(true);
+    });
+
+    it('shows list of errors in a table', () => {
+      expect(findErrorListRows().length).toEqual(store.state.list.errors.length);
+    });
+
+    it('each error in a list should have a link to the error page', () => {
+      const errorTitle = wrapper.findAll('tbody tr a');
+
+      errorTitle.wrappers.forEach((_, index) => {
+        expect(errorTitle.at(index).attributes('href')).toEqual(
+          expect.stringMatching(/error_tracking\/\d+\/details$/),
+        );
+      });
     });
 
     describe('filtering', () => {
+      const findSearchBox = () => wrapper.find(GlSearchBoxByClick);
+
       it('shows search box', () => {
-        expect(wrapper.find(GlSearchBoxByClick).exists()).toBeTruthy();
+        expect(findSearchBox().exists()).toBe(true);
       });
 
       it('makes network request on submit', () => {
         expect(actions.startPolling).toHaveBeenCalledTimes(1);
-
-        wrapper.find(GlSearchBoxByClick).vm.$emit('submit');
-
+        findSearchBox().vm.$emit('submit');
         expect(actions.startPolling).toHaveBeenCalledTimes(2);
       });
     });
   });
 
   describe('no results', () => {
+    const findRefreshLink = () => wrapper.find('.js-try-again');
+
     beforeEach(() => {
       store.state.list.loading = false;
+      store.state.list.errors = [];
 
       mountComponent();
     });
 
     it('shows empty table', () => {
-      expect(wrapper.find(GlLoadingIcon).exists()).toBeFalsy();
-      expect(wrapper.find(GlTable).exists()).toBeTruthy();
-      expect(wrapper.find(GlButton).exists()).toBeTruthy();
+      expect(findLoadingIcon().exists()).toBe(false);
+      expect(findErrorListRows().length).toEqual(1);
+      expect(findButton().exists()).toBe(true);
     });
 
     it('shows a message prompting to refresh', () => {
-      const refreshLink = wrapper.vm.$refs.empty.querySelector('a');
-
-      expect(refreshLink.textContent.trim()).toContain('Check again');
+      expect(findRefreshLink().text()).toContain('Check again');
     });
 
     it('restarts polling', () => {
-      wrapper.find('.js-try-again').trigger('click');
-
+      findRefreshLink().trigger('click');
       expect(actions.restartPolling).toHaveBeenCalled();
     });
   });
@@ -140,10 +160,10 @@ describe('ErrorTrackingList', () => {
     });
 
     it('shows empty state', () => {
-      expect(wrapper.find(GlEmptyState).exists()).toBeTruthy();
-      expect(wrapper.find(GlLoadingIcon).exists()).toBeFalsy();
-      expect(wrapper.find(GlTable).exists()).toBeFalsy();
-      expect(wrapper.find(GlButton).exists()).toBeFalsy();
+      expect(wrapper.find(GlEmptyState).exists()).toBe(true);
+      expect(findLoadingIcon().exists()).toBe(false);
+      expect(findErrorListTable().exists()).toBe(false);
+      expect(findButton().exists()).toBe(false);
     });
   });
 

spec/frontend/error_tracking/components/list_mock.json

+[
+  {
+    "id": "1",
+    "title": "PG::ConnectionBad: FATAL",
+    "type": "error",
+    "userCount": 0,
+    "count": "52",
+    "firstSeen": "2019-05-30T07:21:46Z",
+    "lastSeen": "2019-11-06T03:21:39Z"
+  },
+  {
+    "id": "2",
+    "title": "ActiveRecord::StatementInvalid",
+    "type": "error",
+    "userCount": 0,
+    "count": "12",
+    "firstSeen": "2019-10-19T03:53:56Z",
+    "lastSeen": "2019-11-05T03:51:54Z"
+  },
+  {
+    "id": "3",
+    "title": "Command has failed",
+    "type": "default",
+    "userCount": 0,
+    "count": "275",
+    "firstSeen": "2019-02-12T07:22:36Z",
+    "lastSeen": "2019-10-22T03:20:48Z"
+  }
+]
\ No newline at end of file