Merge branch '197137-remove-REST-for-Sentry-error-details' into 'master'

Remove REST call and related logic for error details page

See merge request gitlab-org/gitlab!23980

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

@@ -54,10 +54,6 @@ export default {
       type: String,
       required: true,
     },
-    issueDetailsPath: {
-      type: String,
-      required: true,
-    },
     issueStackTracePath: {
       type: String,
       required: true,
@@ -72,7 +68,7 @@ export default {
     },
   },
   apollo: {
-    GQLerror: {
+    error: {
       query,
       variables() {
         return {
@@ -81,19 +77,19 @@ export default {
         };
       },
       pollInterval: 2000,
-      update: data => data.project.sentryDetailedError,
+      update: data => data.project.sentryErrors.detailedError,
       error: () => createFlash(__('Failed to load error details from Sentry.')),
       result(res) {
-        if (res.data.project?.sentryDetailedError) {
-          this.$apollo.queries.GQLerror.stopPolling();
-          this.setStatus(this.GQLerror.status);
+        if (res.data.project?.sentryErrors?.detailedError) {
+          this.$apollo.queries.error.stopPolling();
+          this.setStatus(this.error.status);
         }
       },
     },
   },
   data() {
     return {
-      GQLerror: null,
+      error: null,
       issueCreationInProgress: false,
       isAlertVisible: false,
       closedIssueId: null,
@@ -101,8 +97,6 @@ export default {
   },
   computed: {
     ...mapState('details', [
-      'error',
-      'loading',
       'loadingStacktrace',
       'stacktraceData',
       'updatingResolveStatus',
@@ -114,28 +108,23 @@ export default {
       return sprintf(
         __('Reported %{timeAgo} by %{reportedBy}'),
         {
-          reportedBy: `<strong>${this.GQLerror.culprit}</strong>`,
+          reportedBy: `<strong>${this.error.culprit}</strong>`,
           timeAgo: this.timeFormatted(this.stacktraceData.date_received),
         },
         false,
       );
     },
     firstReleaseLink() {
-      return `${this.error.external_base_url}/releases/${this.GQLerror.firstReleaseShortVersion}`;
+      return `${this.error.externalBaseUrl}/releases/${this.error.firstReleaseShortVersion}`;
     },
     lastReleaseLink() {
-      return `${this.error.external_base_url}releases/${this.GQLerror.lastReleaseShortVersion}`;
-    },
-    showDetails() {
-      return Boolean(
-        !this.loading && !this.$apollo.queries.GQLerror.loading && this.error && this.GQLerror,
-      );
+      return `${this.error.externalBaseUrl}/releases/${this.error.lastReleaseShortVersion}`;
     },
     showStacktrace() {
-      return Boolean(!this.loadingStacktrace && this.stacktrace && this.stacktrace.length);
+      return Boolean(this.stacktrace?.length);
     },
     issueTitle() {
-      return this.GQLerror.title;
+      return this.error.title;
     },
     issueDescription() {
       return sprintf(
@@ -144,13 +133,13 @@ export default {
         ),
         {
           description: '# Error Details:\n',
-          errorUrl: `${this.GQLerror.externalUrl}\n`,
-          firstSeen: `\n${this.GQLerror.firstSeen}\n`,
-          lastSeen: `${this.GQLerror.lastSeen}\n`,
-          countLabel: n__('- Event', '- Events', this.GQLerror.count),
-          count: `${this.GQLerror.count}\n`,
-          userCountLabel: n__('- User', '- Users', this.GQLerror.userCount),
-          userCount: `${this.GQLerror.userCount}\n`,
+          errorUrl: `${this.error.externalUrl}\n`,
+          firstSeen: `\n${this.error.firstSeen}\n`,
+          lastSeen: `${this.error.lastSeen}\n`,
+          countLabel: n__('- Event', '- Events', this.error.count),
+          count: `${this.error.count}\n`,
+          userCountLabel: n__('- User', '- Users', this.error.userCount),
+          userCount: `${this.error.userCount}\n`,
         },
         false,
       );
@@ -171,12 +160,10 @@ export default {
     },
   },
   mounted() {
-    this.startPollingDetails(this.issueDetailsPath);
     this.startPollingStacktrace(this.issueStackTracePath);
   },
   methods: {
     ...mapActions('details', [
-      'startPollingDetails',
       'startPollingStacktrace',
       'updateStatus',
       'setStatus',
@@ -214,10 +201,10 @@ export default {
 
 <template>
   <div>
-    <div v-if="$apollo.queries.GQLerror.loading || loading" class="py-3">
+    <div v-if="$apollo.queries.error.loading" class="py-3">
       <gl-loading-icon :size="3" />
     </div>
-    <div v-else-if="showDetails" class="error-details">
+    <div v-else-if="error" class="error-details">
       <gl-alert v-if="isAlertVisible" @dismiss="isAlertVisible = false">
         <gl-sprintf
           :message="
@@ -232,7 +219,7 @@ export default {
 
       <div class="top-area align-items-center justify-content-between py-3">
         <span v-if="!loadingStacktrace && stacktrace" v-html="reported"></span>
-        <div class="d-inline-flex">
+        <div class="d-inline-flex ml-lg-auto">
           <loading-button
             :label="ignoreBtnLabel"
             :loading="updatingIgnoreStatus"
@@ -247,10 +234,10 @@ export default {
             @click="onResolveStatusUpdate"
           />
           <gl-button
-            v-if="error.gitlab_issue"
+            v-if="error.gitlabIssuePath"
             class="ml-2"
             data-qa-selector="view_issue_button"
-            :href="error.gitlab_issue"
+            :href="error.gitlabIssuePath"
             variant="success"
           >
             {{ __('View issue') }}
@@ -264,13 +251,13 @@ export default {
             <gl-form-input class="hidden" name="issue[title]" :value="issueTitle" />
             <input name="issue[description]" :value="issueDescription" type="hidden" />
             <gl-form-input
-              :value="GQLerror.sentryId"
+              :value="error.sentryId"
               class="hidden"
               name="issue[sentry_issue_attributes][sentry_issue_identifier]"
             />
             <gl-form-input :value="csrfToken" class="hidden" name="authenticity_token" />
             <loading-button
-              v-if="!error.gitlab_issue"
+              v-if="!error.gitlabIssuePath"
               class="btn-success"
               :label="__('Create issue')"
               :loading="issueCreationInProgress"
@@ -281,8 +268,8 @@ export default {
         </div>
       </div>
       <div>
-        <tooltip-on-truncate :title="GQLerror.title" truncate-target="child" placement="top">
-          <h2 class="text-truncate">{{ GQLerror.title }}</h2>
+        <tooltip-on-truncate :title="error.title" truncate-target="child" placement="top">
+          <h2 class="text-truncate">{{ error.title }}</h2>
         </tooltip-on-truncate>
         <template v-if="error.tags">
           <gl-badge
@@ -297,53 +284,51 @@ export default {
           </gl-badge>
         </template>
         <ul>
-          <li v-if="GQLerror.gitlabCommit">
+          <li v-if="error.gitlabCommit">
             <strong class="bold">{{ __('GitLab commit') }}:</strong>
-            <gl-link :href="GQLerror.gitlabCommitPath">
-              <span>{{ GQLerror.gitlabCommit.substr(0, 10) }}</span>
+            <gl-link :href="error.gitlabCommitPath">
+              <span>{{ error.gitlabCommit.substr(0, 10) }}</span>
             </gl-link>
           </li>
-          <li v-if="error.gitlab_issue">
+          <li v-if="error.gitlabIssuePath">
             <strong class="bold">{{ __('GitLab Issue') }}:</strong>
-            <gl-link :href="error.gitlab_issue">
-              <span>{{ error.gitlab_issue }}</span>
+            <gl-link :href="error.gitlabIssuePath">
+              <span>{{ error.gitlabIssuePath }}</span>
             </gl-link>
           </li>
           <li>
             <strong class="bold">{{ __('Sentry event') }}:</strong>
             <gl-link
-              v-track-event="trackClickErrorLinkToSentryOptions(GQLerror.externalUrl)"
+              v-track-event="trackClickErrorLinkToSentryOptions(error.externalUrl)"
               class="d-inline-flex align-items-center"
-              :href="GQLerror.externalUrl"
+              :href="error.externalUrl"
               target="_blank"
             >
-              <span class="text-truncate">{{ GQLerror.externalUrl }}</span>
+              <span class="text-truncate">{{ error.externalUrl }}</span>
               <icon name="external-link" class="ml-1 flex-shrink-0" />
             </gl-link>
           </li>
-          <li v-if="GQLerror.firstReleaseShortVersion">
+          <li v-if="error.firstReleaseShortVersion">
             <strong class="bold">{{ __('First seen') }}:</strong>
-            {{ formatDate(GQLerror.firstSeen) }}
+            {{ formatDate(error.firstSeen) }}
             <gl-link :href="firstReleaseLink" target="_blank">
-              <span>
-                {{ __('Release') }}: {{ GQLerror.firstReleaseShortVersion.substr(0, 10) }}
-              </span>
+              <span>{{ __('Release') }}: {{ error.firstReleaseShortVersion.substr(0, 10) }}</span>
             </gl-link>
           </li>
-          <li v-if="GQLerror.lastReleaseShortVersion">
+          <li v-if="error.lastReleaseShortVersion">
             <strong class="bold">{{ __('Last seen') }}:</strong>
-            {{ formatDate(GQLerror.lastSeen) }}
+            {{ formatDate(error.lastSeen) }}
             <gl-link :href="lastReleaseLink" target="_blank">
-              <span>{{ __('Release') }}: {{ GQLerror.lastReleaseShortVersion.substr(0, 10) }}</span>
+              <span>{{ __('Release') }}: {{ error.lastReleaseShortVersion.substr(0, 10) }}</span>
             </gl-link>
           </li>
           <li>
             <strong class="bold">{{ __('Events') }}:</strong>
-            <span>{{ GQLerror.count }}</span>
+            <span>{{ error.count }}</span>
           </li>
           <li>
             <strong class="bold">{{ __('Users') }}:</strong>
-            <span>{{ GQLerror.userCount }}</span>
+            <span>{{ error.userCount }}</span>
           </li>
         </ul>
 
@@ -351,7 +336,7 @@ export default {
           <gl-loading-icon :size="3" />
         </div>
 
-        <template v-if="showStacktrace">
+        <template v-else-if="showStacktrace">
           <h3 class="my-4">{{ __('Stack trace') }}</h3>
           <stacktrace :entries="stacktrace" />
         </template>

app/assets/javascripts/error_tracking/details.js

@@ -26,7 +26,6 @@ export default () => {
         issueId,
         projectPath,
         issueUpdatePath,
-        issueDetailsPath,
         issueStackTracePath,
         projectIssuesPath,
       } = domEl.dataset;
@@ -36,7 +35,6 @@ export default () => {
           issueId,
           projectPath,
           issueUpdatePath,
-          issueDetailsPath,
           issueStackTracePath,
           projectIssuesPath,
           csrfToken: csrf.token,

app/assets/javascripts/error_tracking/queries/details.query.graphql

 query errorDetails($fullPath: ID!, $errorId: ID!) {
     project(fullPath: $fullPath) {
-        sentryDetailedError(id: $errorId) {
+        sentryErrors {
+            detailedError(id: $errorId) {
                 id
                 sentryId
                 title
@@ -11,11 +12,18 @@ query errorDetails($fullPath: ID!, $errorId: ID!) {
                 lastSeen
                 message
                 culprit
+                tags {
+                    level
+                    logger
+                }
                 externalUrl
+                externalBaseUrl
                 firstReleaseShortVersion
                 lastReleaseShortVersion
                 gitlabCommit
                 gitlabCommitPath
+                gitlabIssuePath
+            }
         }
     }
 }

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

@@ -5,36 +5,11 @@ import Poll from '~/lib/utils/poll';
 import { __ } from '~/locale';
 
 let stackTracePoll;
-let detailPoll;
 
 const stopPolling = poll => {
   if (poll) poll.stop();
 };
 
-export function startPollingDetails({ commit }, endpoint) {
-  detailPoll = new Poll({
-    resource: service,
-    method: 'getSentryData',
-    data: { endpoint },
-    successCallback: ({ data }) => {
-      if (!data) {
-        return;
-      }
-
-      commit(types.SET_ERROR, data.error);
-      commit(types.SET_LOADING, false);
-
-      stopPolling(detailPoll);
-    },
-    errorCallback: () => {
-      commit(types.SET_LOADING, false);
-      createFlash(__('Failed to load error details from Sentry.'));
-    },
-  });
-
-  detailPoll.makeRequest();
-}
-
 export function startPollingStacktrace({ commit }, endpoint) {
   stackTracePoll = new Poll({
     resource: service,

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

-export const SET_ERROR = 'SET_ERRORS';
-export const SET_LOADING = 'SET_LOADING';
 export const SET_LOADING_STACKTRACE = 'SET_LOADING_STACKTRACE';
 export const SET_STACKTRACE_DATA = 'SET_STACKTRACE_DATA';

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

 import * as types from './mutation_types';
 
 export default {
-  [types.SET_ERROR](state, data) {
-    state.error = data;
-  },
-  [types.SET_LOADING](state, loading) {
-    state.loading = loading;
-  },
   [types.SET_LOADING_STACKTRACE](state, data) {
     state.loadingStacktrace = data;
   },

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

 export default () => ({
-  error: {},
   stacktraceData: {},
-  loading: true,
   loadingStacktrace: true,
   updatingResolveStatus: false,
   updatingIgnoreStatus: false,

app/helpers/projects/error_tracking_helper.rb

@@ -22,7 +22,6 @@ module Projects::ErrorTrackingHelper
     {
       'issue-id' => issue_id,
       'project-path' => project.full_path,
-      'issue-details-path' => details_project_error_tracking_index_path(*opts),
       'issue-update-path' => update_project_error_tracking_index_path(*opts),
       'project-issues-path' => project_issues_path(project),
       'issue-stack-trace-path' => stack_trace_project_error_tracking_index_path(*opts)

spec/frontend/error_tracking/components/error_details_spec.js

@@ -37,14 +37,13 @@ describe('ErrorDetails', () => {
         projectPath: '/root/gitlab-test',
         listPath: '/error_tracking',
         issueUpdatePath: '/123',
-        issueDetailsPath: '/123/details',
         issueStackTracePath: '/stacktrace',
         projectIssuesPath: '/test-project/issues/',
         csrfToken: 'fakeToken',
       },
     });
     wrapper.setData({
-      GQLerror: {
+      error: {
         id: 'gid://gitlab/Gitlab::ErrorTracking::DetailedError/129381',
         sentryId: 129381,
         title: 'Issue title',
@@ -59,7 +58,6 @@ describe('ErrorDetails', () => {
 
   beforeEach(() => {
     actions = {
-      startPollingDetails: () => {},
       startPollingStacktrace: () => {},
       updateIgnoreStatus: jest.fn(),
       updateResolveStatus: jest.fn().mockResolvedValue({ closed_issue_iid: 1 }),
@@ -71,8 +69,6 @@ describe('ErrorDetails', () => {
     };
 
     const state = {
-      error: {},
-      loading: true,
       stacktraceData: {},
       loadingStacktrace: true,
     };
@@ -93,7 +89,7 @@ describe('ErrorDetails', () => {
       $apollo: {
         query,
         queries: {
-          GQLerror: {
+          error: {
             loading: true,
             stopPolling: jest.fn(),
           },
@@ -122,9 +118,7 @@ describe('ErrorDetails', () => {
 
   describe('Error details', () => {
     beforeEach(() => {
-      store.state.details.loading = false;
-      store.state.details.error.id = 1;
-      mocks.$apollo.queries.GQLerror.loading = false;
+      mocks.$apollo.queries.error.loading = false;
       mountComponent();
     });
 
@@ -138,16 +132,22 @@ describe('ErrorDetails', () => {
 
     describe('Badges', () => {
       it('should show language and error level badges', () => {
-        store.state.details.error.tags = { level: 'error', logger: 'ruby' };
-        mountComponent();
+        wrapper.setData({
+          error: {
+            tags: { level: 'error', logger: 'ruby' },
+          },
+        });
         return wrapper.vm.$nextTick().then(() => {
           expect(wrapper.findAll(GlBadge).length).toBe(2);
         });
       });
 
       it('should NOT show the badge if the tag is not present', () => {
-        store.state.details.error.tags = { level: 'error' };
-        mountComponent();
+        wrapper.setData({
+          error: {
+            tags: { level: 'error' },
+          },
+        });
         return wrapper.vm.$nextTick().then(() => {
           expect(wrapper.findAll(GlBadge).length).toBe(1);
         });
@@ -156,8 +156,11 @@ describe('ErrorDetails', () => {
       it.each(Object.keys(severityLevel))(
         'should set correct severity level variant for %s badge',
         level => {
-          store.state.details.error.tags = { level: severityLevel[level] };
-          mountComponent();
+          wrapper.setData({
+            error: {
+              tags: { level: severityLevel[level] },
+            },
+          });
           return wrapper.vm.$nextTick().then(() => {
             expect(wrapper.find(GlBadge).attributes('variant')).toEqual(
               severityLevelVariant[severityLevel[level]],
@@ -167,8 +170,11 @@ describe('ErrorDetails', () => {
       );
 
       it('should fallback for ERROR severityLevelVariant when severityLevel is unknown', () => {
-        store.state.details.error.tags = { level: 'someNewErrorLevel' };
-        mountComponent();
+        wrapper.setData({
+          error: {
+            tags: { level: 'someNewErrorLevel' },
+          },
+        });
         return wrapper.vm.$nextTick().then(() => {
           expect(wrapper.find(GlBadge).attributes('variant')).toEqual(
             severityLevelVariant[severityLevel.ERROR],
@@ -180,7 +186,6 @@ describe('ErrorDetails', () => {
     describe('Stacktrace', () => {
       it('should show stacktrace', () => {
         store.state.details.loadingStacktrace = false;
-        mountComponent();
         return wrapper.vm.$nextTick().then(() => {
           expect(wrapper.find(GlLoadingIcon).exists()).toBe(false);
           expect(wrapper.find(Stacktrace).exists()).toBe(true);
@@ -190,11 +195,12 @@ describe('ErrorDetails', () => {
       it('should NOT show stacktrace if no entries', () => {
         store.state.details.loadingStacktrace = false;
         store.getters = { 'details/sentryUrl': () => 'sentry.io', 'details/stacktrace': () => [] };
-        mountComponent();
+        return wrapper.vm.$nextTick().then(() => {
           expect(wrapper.find(GlLoadingIcon).exists()).toBe(false);
           expect(wrapper.find(Stacktrace).exists()).toBe(false);
         });
       });
+    });
 
     describe('When a user clicks the create issue button', () => {
       beforeEach(() => {
@@ -331,19 +337,18 @@ describe('ErrorDetails', () => {
     });
 
     describe('GitLab issue link', () => {
-      const gitlabIssue = 'https://gitlab.example.com/issues/1';
-      const findGitLabLink = () => wrapper.find(`[href="${gitlabIssue}"]`);
+      const gitlabIssuePath = 'https://gitlab.example.com/issues/1';
+      const findGitLabLink = () => wrapper.find(`[href="${gitlabIssuePath}"]`);
       const findCreateIssueButton = () => wrapper.find('[data-qa-selector="create_issue_button"]');
       const findViewIssueButton = () => wrapper.find('[data-qa-selector="view_issue_button"]');
 
       describe('is present', () => {
         beforeEach(() => {
-          store.state.details.loading = false;
-          store.state.details.error = {
-            id: 1,
-            gitlab_issue: gitlabIssue,
-          };
-          mountComponent();
+          wrapper.setData({
+            error: {
+              gitlabIssuePath,
+            },
+          });
         });
 
         it('should display the View issue button', () => {
@@ -361,12 +366,11 @@ describe('ErrorDetails', () => {
 
       describe('is not present', () => {
         beforeEach(() => {
-          store.state.details.loading = false;
-          store.state.details.error = {
-            id: 1,
-            gitlab_issue: null,
-          };
-          mountComponent();
+          wrapper.setData({
+            error: {
+              gitlabIssuePath: null,
+            },
+          });
         });
 
         it('should not display the View issue button', () => {
@@ -390,9 +394,9 @@ describe('ErrorDetails', () => {
       const findGitLabCommitLink = () => wrapper.find(`[href$="${gitlabCommitPath}"]`);
 
       it('should display a link', () => {
-        mocks.$apollo.queries.GQLerror.loading = false;
+        mocks.$apollo.queries.error.loading = false;
         wrapper.setData({
-          GQLerror: {
+          error: {
             gitlabCommit,
             gitlabCommitPath,
           },
@@ -403,9 +407,9 @@ describe('ErrorDetails', () => {
       });
 
       it('should not display a link', () => {
-        mocks.$apollo.queries.GQLerror.loading = false;
+        mocks.$apollo.queries.error.loading = false;
         wrapper.setData({
-          GQLerror: {
+          error: {
             gitlabCommit: null,
           },
         });

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

@@ -26,53 +26,6 @@ describe('Sentry error details store actions', () => {
     }
   });
 
-  describe('startPollingDetails', () => {
-    const endpoint = '123/details';
-    it('should commit SET_ERROR with received response', done => {
-      const payload = { error: { id: 1 } };
-      mockedAdapter.onGet().reply(200, payload);
-      testAction(
-        actions.startPollingDetails,
-        { endpoint },
-        {},
-        [
-          { type: types.SET_ERROR, payload: payload.error },
-          { type: types.SET_LOADING, payload: false },
-        ],
-        [],
-        () => {
-          done();
-        },
-      );
-    });
-
-    it('should show flash on API error', done => {
-      mockedAdapter.onGet().reply(400);
-
-      testAction(
-        actions.startPollingDetails,
-        { endpoint },
-        {},
-        [{ type: types.SET_LOADING, payload: false }],
-        [],
-        () => {
-          expect(createFlash).toHaveBeenCalledTimes(1);
-          done();
-        },
-      );
-    });
-
-    it('should not restart polling when receiving an empty 204 response', done => {
-      mockedRestart = jest.spyOn(Poll.prototype, 'restart');
-      mockedAdapter.onGet().reply(204);
-
-      testAction(actions.startPollingDetails, { endpoint }, {}, [], [], () => {
-        expect(mockedRestart).toHaveBeenCalledTimes(0);
-        done();
-      });
-    });
-  });
-
   describe('startPollingStacktrace', () => {
     const endpoint = '123/stacktrace';
     it('should commit SET_ERROR with received response', done => {

spec/helpers/projects/error_tracking_helper_spec.rb

@@ -83,7 +83,6 @@ describe Projects::ErrorTrackingHelper do
   describe '#error_details_data' do
     let(:issue_id) { 1234 }
     let(:route_params) { [project.owner, project, issue_id, { format: :json }] }
-    let(:details_path) { details_namespace_project_error_tracking_index_path(*route_params) }
     let(:project_path) { project.full_path }
     let(:stack_trace_path) { stack_trace_namespace_project_error_tracking_index_path(*route_params) }
     let(:issues_path) { project_issues_path(project) }
@@ -98,10 +97,6 @@ describe Projects::ErrorTrackingHelper do
       expect(result['project-path']).to eq project_path
     end
 
-    it 'returns the correct details path' do
-      expect(result['issue-details-path']).to eq details_path
-    end
-
     it 'returns the correct stack trace path' do
       expect(result['issue-stack-trace-path']).to eq stack_trace_path
     end