Merge branch '196881-reverse-actions-for-status-update' into 'master'

Reverse actions for ignoring/resolving errors

See merge request gitlab-org/gitlab!23516

app/assets/javascripts/error_tracking/components/constants.js

@@ -13,3 +13,9 @@ export const severityLevelVariant = {
   [severityLevel.INFO]: 'info',
   [severityLevel.DEBUG]: 'light',
 };
+
+export const errorStatus = {
+  IGNORED: 'ignored',
+  RESOLVED: 'resolved',
+  UNRESOLVED: 'unresolved',
+};

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

@@ -11,7 +11,7 @@ import Stacktrace from './stacktrace.vue';
 import TrackEventDirective from '~/vue_shared/directives/track_event';
 import timeagoMixin from '~/vue_shared/mixins/timeago';
 import { trackClickErrorLinkToSentryOptions } from '../utils';
-import { severityLevel, severityLevelVariant } from './constants';
+import { severityLevel, severityLevelVariant, errorStatus } from './constants';
 
 import query from '../queries/details.query.graphql';
 
@@ -32,10 +32,6 @@ export default {
   },
   mixins: [timeagoMixin],
   props: {
-    listPath: {
-      type: String,
-      required: true,
-    },
     issueUpdatePath: {
       type: String,
       required: true,
@@ -80,6 +76,7 @@ export default {
       result(res) {
         if (res.data.project?.sentryDetailedError) {
           this.$apollo.queries.GQLerror.stopPolling();
+          this.setStatus(this.GQLerror.status);
         }
       },
     },
@@ -98,6 +95,7 @@ export default {
       'stacktraceData',
       'updatingResolveStatus',
       'updatingIgnoreStatus',
+      'errorStatus',
     ]),
     ...mapGetters('details', ['stacktrace']),
     reported() {
@@ -153,20 +151,40 @@ export default {
         severityLevelVariant[this.error.tags.level] || severityLevelVariant[severityLevel.ERROR]
       );
     },
+    ignoreBtnLabel() {
+      return this.errorStatus !== errorStatus.IGNORED ? __('Ignore') : __('Undo ignore');
+    },
+    resolveBtnLabel() {
+      return this.errorStatus !== errorStatus.RESOLVED ? __('Resolve') : __('Unresolve');
+    },
   },
   mounted() {
     this.startPollingDetails(this.issueDetailsPath);
     this.startPollingStacktrace(this.issueStackTracePath);
   },
   methods: {
-    ...mapActions('details', ['startPollingDetails', 'startPollingStacktrace', 'updateStatus']),
+    ...mapActions('details', [
+      'startPollingDetails',
+      'startPollingStacktrace',
+      'updateStatus',
+      'setStatus',
+      'updateResolveStatus',
+      'updateIgnoreStatus',
+    ]),
     trackClickErrorLinkToSentryOptions,
     createIssue() {
       this.issueCreationInProgress = true;
       this.$refs.sentryIssueForm.submit();
     },
-    updateIssueStatus(status) {
-      this.updateStatus({ endpoint: this.issueUpdatePath, redirectUrl: this.listPath, status });
+    onIgnoreStatusUpdate() {
+      const status =
+        this.errorStatus === errorStatus.IGNORED ? errorStatus.UNRESOLVED : errorStatus.IGNORED;
+      this.updateIgnoreStatus({ endpoint: this.issueUpdatePath, status });
+    },
+    onResolveStatusUpdate() {
+      const status =
+        this.errorStatus === errorStatus.RESOLVED ? errorStatus.UNRESOLVED : errorStatus.RESOLVED;
+      this.updateResolveStatus({ endpoint: this.issueUpdatePath, status });
     },
     formatDate(date) {
       return `${this.timeFormatted(date)} (${dateFormat(date, 'UTC:yyyy-mm-dd h:MM:ssTT Z')})`;
@@ -185,15 +203,17 @@ export default {
         <span v-if="!loadingStacktrace && stacktrace" v-html="reported"></span>
         <div class="d-inline-flex">
           <loading-button
-            :label="__('Ignore')"
+            :label="ignoreBtnLabel"
             :loading="updatingIgnoreStatus"
-            @click="updateIssueStatus('ignored')"
+            data-qa-selector="update_ignore_status_button"
+            @click="onIgnoreStatusUpdate"
           />
           <loading-button
             class="btn-outline-info ml-2"
-            :label="__('Resolve')"
+            :label="resolveBtnLabel"
             :loading="updatingResolveStatus"
-            @click="updateIssueStatus('resolved')"
+            data-qa-selector="update_resolve_status_button"
+            @click="onResolveStatusUpdate"
           />
           <gl-button
             v-if="error.gitlab_issue"

app/assets/javascripts/error_tracking/details.js

@@ -25,7 +25,6 @@ export default () => {
       const {
         issueId,
         projectPath,
-        listPath,
         issueUpdatePath,
         issueDetailsPath,
         issueStackTracePath,
@@ -36,7 +35,6 @@ export default () => {
         props: {
           issueId,
           projectPath,
-          listPath,
           issueUpdatePath,
           issueDetailsPath,
           issueStackTracePath,

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

@@ -6,6 +6,7 @@ query errorDetails($fullPath: ID!, $errorId: ID!) {
             title
             userCount
             count
+            status
             firstSeen
             lastSeen
             message

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

@@ -4,16 +4,33 @@ import createFlash from '~/flash';
 import { visitUrl } from '~/lib/utils/url_utility';
 import { __ } from '~/locale';
 
-export function updateStatus({ commit }, { endpoint, redirectUrl, status }) {
-  const type =
-    status === 'resolved' ? types.SET_UPDATING_RESOLVE_STATUS : types.SET_UPDATING_IGNORE_STATUS;
-  commit(type, true);
+export const setStatus = ({ commit }, status) => {
+  commit(types.SET_ERROR_STATUS, status.toLowerCase());
+};
 
-  return service
+export const updateStatus = ({ commit }, { endpoint, redirectUrl, status }) =>
+  service
     .updateErrorStatus(endpoint, status)
-    .then(() => visitUrl(redirectUrl))
-    .catch(() => createFlash(__('Failed to update issue status')))
-    .finally(() => commit(type, false));
-}
+    .then(() => {
+      if (redirectUrl) visitUrl(redirectUrl);
+      commit(types.SET_ERROR_STATUS, status);
+    })
+    .catch(() => createFlash(__('Failed to update issue status')));
+
+export const updateResolveStatus = ({ commit, dispatch }, params) => {
+  commit(types.SET_UPDATING_RESOLVE_STATUS, true);
+
+  return dispatch('updateStatus', params).finally(() => {
+    commit(types.SET_UPDATING_RESOLVE_STATUS, false);
+  });
+};
+
+export const updateIgnoreStatus = ({ commit, dispatch }, params) => {
+  commit(types.SET_UPDATING_IGNORE_STATUS, true);
+
+  return dispatch('updateStatus', params).finally(() => {
+    commit(types.SET_UPDATING_IGNORE_STATUS, false);
+  });
+};
 
 export default () => {};

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

@@ -5,4 +5,5 @@ export default () => ({
   loadingStacktrace: true,
   updatingResolveStatus: false,
   updatingIgnoreStatus: false,
+  errorStatus: '',
 });

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

 export const SET_UPDATING_RESOLVE_STATUS = 'SET_UPDATING_RESOLVE_STATUS';
 export const SET_UPDATING_IGNORE_STATUS = 'SET_UPDATING_IGNORE_STATUS';
+export const SET_ERROR_STATUS = 'SET_ERROR_STATUS';

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

@@ -7,4 +7,7 @@ export default {
   [types.SET_UPDATING_RESOLVE_STATUS](state, updating) {
     state.updatingResolveStatus = updating;
   },
+  [types.SET_ERROR_STATUS](state, status) {
+    state.errorStatus = status;
+  },
 };

app/helpers/projects/error_tracking_helper.rb

@@ -22,7 +22,6 @@ module Projects::ErrorTrackingHelper
     {
       'issue-id' => issue_id,
       'project-path' => project.full_path,
-      'list-path' => project_error_tracking_index_path(project),
       '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),

changelogs/unreleased/196881-reverse-actions-for-status-update.yml

+---
+title: Reverse actions for resolve/ignore Sentry issue
+merge_request: 23516
+author:
+type: added

locale/gitlab.pot

@@ -20259,6 +20259,9 @@ msgstr ""
 msgid "Undo"
 msgstr ""
 
+msgid "Undo ignore"
+msgstr ""
+
 msgid "Unfortunately, your email message to GitLab could not be processed."
 msgstr ""
 
@@ -20310,6 +20313,9 @@ msgstr ""
 msgid "Unmarks this %{noun} as Work In Progress."
 msgstr ""
 
+msgid "Unresolve"
+msgstr ""
+
 msgid "Unresolve discussion"
 msgstr ""
 

spec/frontend/error_tracking/components/error_details_spec.js

 import { createLocalVue, shallowMount } from '@vue/test-utils';
 import Vuex from 'vuex';
+import { __ } from '~/locale';
 import { GlLoadingIcon, GlLink, GlBadge, GlFormInput } from '@gitlab/ui';
 import LoadingButton from '~/vue_shared/components/loading_button.vue';
 import Stacktrace from '~/error_tracking/components/stacktrace.vue';
 import ErrorDetails from '~/error_tracking/components/error_details.vue';
-import { severityLevel, severityLevelVariant } from '~/error_tracking/components/constants';
+import {
+  severityLevel,
+  severityLevelVariant,
+  errorStatus,
+} from '~/error_tracking/components/constants';
 
 const localVue = createLocalVue();
 localVue.use(Vuex);
@@ -56,6 +61,8 @@ describe('ErrorDetails', () => {
     actions = {
       startPollingDetails: () => {},
       startPollingStacktrace: () => {},
+      updateIgnoreStatus: jest.fn(),
+      updateResolveStatus: jest.fn(),
     };
 
     getters = {
@@ -219,6 +226,96 @@ describe('ErrorDetails', () => {
       });
     });
 
+    describe('Status update', () => {
+      const findUpdateIgnoreStatusButton = () =>
+        wrapper.find('[data-qa-selector="update_ignore_status_button"]');
+      const findUpdateResolveStatusButton = () =>
+        wrapper.find('[data-qa-selector="update_resolve_status_button"]');
+
+      afterEach(() => {
+        actions.updateIgnoreStatus.mockClear();
+        actions.updateResolveStatus.mockClear();
+      });
+
+      describe('when error is unresolved', () => {
+        beforeEach(() => {
+          store.state.details.errorStatus = errorStatus.UNRESOLVED;
+          mountComponent();
+        });
+
+        it('displays Ignore and Resolve buttons', () => {
+          expect(findUpdateIgnoreStatusButton().text()).toBe(__('Ignore'));
+          expect(findUpdateResolveStatusButton().text()).toBe(__('Resolve'));
+        });
+
+        it('marks error as ignored when ignore button is clicked', () => {
+          findUpdateIgnoreStatusButton().trigger('click');
+          expect(actions.updateIgnoreStatus.mock.calls[0][1]).toEqual(
+            expect.objectContaining({ status: errorStatus.IGNORED }),
+          );
+        });
+
+        it('marks error as resolved when resolve button is clicked', () => {
+          findUpdateResolveStatusButton().trigger('click');
+          expect(actions.updateResolveStatus.mock.calls[0][1]).toEqual(
+            expect.objectContaining({ status: errorStatus.RESOLVED }),
+          );
+        });
+      });
+
+      describe('when error is ignored', () => {
+        beforeEach(() => {
+          store.state.details.errorStatus = errorStatus.IGNORED;
+          mountComponent();
+        });
+
+        it('displays Undo Ignore and Resolve buttons', () => {
+          expect(findUpdateIgnoreStatusButton().text()).toBe(__('Undo ignore'));
+          expect(findUpdateResolveStatusButton().text()).toBe(__('Resolve'));
+        });
+
+        it('marks error as unresolved when ignore button is clicked', () => {
+          findUpdateIgnoreStatusButton().trigger('click');
+          expect(actions.updateIgnoreStatus.mock.calls[0][1]).toEqual(
+            expect.objectContaining({ status: errorStatus.UNRESOLVED }),
+          );
+        });
+
+        it('marks error as resolved when resolve button is clicked', () => {
+          findUpdateResolveStatusButton().trigger('click');
+          expect(actions.updateResolveStatus.mock.calls[0][1]).toEqual(
+            expect.objectContaining({ status: errorStatus.RESOLVED }),
+          );
+        });
+      });
+
+      describe('when error is resolved', () => {
+        beforeEach(() => {
+          store.state.details.errorStatus = errorStatus.RESOLVED;
+          mountComponent();
+        });
+
+        it('displays Ignore and Unresolve buttons', () => {
+          expect(findUpdateIgnoreStatusButton().text()).toBe(__('Ignore'));
+          expect(findUpdateResolveStatusButton().text()).toBe(__('Unresolve'));
+        });
+
+        it('marks error as ignored when ignore button is clicked', () => {
+          findUpdateIgnoreStatusButton().trigger('click');
+          expect(actions.updateIgnoreStatus.mock.calls[0][1]).toEqual(
+            expect.objectContaining({ status: errorStatus.IGNORED }),
+          );
+        });
+
+        it('marks error as unresolved when unresolve button is clicked', () => {
+          findUpdateResolveStatusButton().trigger('click');
+          expect(actions.updateResolveStatus.mock.calls[0][1]).toEqual(
+            expect.objectContaining({ status: errorStatus.UNRESOLVED }),
+          );
+        });
+      });
+    });
+
     describe('GitLab issue link', () => {
       const gitlabIssue = 'https://gitlab.example.com/issues/1';
       const findGitLabLink = () => wrapper.find(`[href="${gitlabIssue}"]`);

spec/frontend/error_tracking/store/actions_spec.js

@@ -10,6 +10,8 @@ jest.mock('~/flash.js');
 jest.mock('~/lib/utils/url_utility');
 
 let mock;
+const commit = jest.fn();
+const dispatch = jest.fn().mockResolvedValue();
 
 describe('Sentry common store actions', () => {
   beforeEach(() => {
@@ -20,26 +22,22 @@ describe('Sentry common store actions', () => {
     mock.restore();
     createFlash.mockClear();
   });
+  const endpoint = '123/stacktrace';
+  const redirectUrl = '/list';
+  const status = 'resolved';
+  const params = { endpoint, redirectUrl, status };
 
   describe('updateStatus', () => {
-    const endpoint = '123/stacktrace';
-    const redirectUrl = '/list';
-    const status = 'resolved';
-
     it('should handle successful status update', done => {
       mock.onPut().reply(200, {});
       testAction(
         actions.updateStatus,
-        { endpoint, redirectUrl, status },
+        params,
         {},
         [
           {
-            payload: true,
-            type: types.SET_UPDATING_RESOLVE_STATUS,
-          },
-          {
-            payload: false,
-            type: 'SET_UPDATING_RESOLVE_STATUS',
+            payload: 'resolved',
+            type: types.SET_ERROR_STATUS,
           },
         ],
         [],
@@ -52,27 +50,29 @@ describe('Sentry common store actions', () => {
 
     it('should handle unsuccessful status update', done => {
       mock.onPut().reply(400, {});
-      testAction(
-        actions.updateStatus,
-        { endpoint, redirectUrl, status },
-        {},
-        [
-          {
-            payload: true,
-            type: types.SET_UPDATING_RESOLVE_STATUS,
-          },
-          {
-            payload: false,
-            type: types.SET_UPDATING_RESOLVE_STATUS,
-          },
-        ],
-        [],
-        () => {
-          expect(visitUrl).not.toHaveBeenCalled();
-          expect(createFlash).toHaveBeenCalledTimes(1);
-          done();
-        },
-      );
+      testAction(actions.updateStatus, params, {}, [], [], () => {
+        expect(visitUrl).not.toHaveBeenCalled();
+        expect(createFlash).toHaveBeenCalledTimes(1);
+        done();
+      });
     });
   });
+
+  describe('updateResolveStatus', () => {
+    it('handles status update', () =>
+      actions.updateResolveStatus({ commit, dispatch }, params).then(() => {
+        expect(commit).toHaveBeenCalledWith(types.SET_UPDATING_RESOLVE_STATUS, true);
+        expect(commit).toHaveBeenCalledWith(types.SET_UPDATING_RESOLVE_STATUS, false);
+        expect(dispatch).toHaveBeenCalledWith('updateStatus', params);
+      }));
+  });
+
+  describe('updateIgnoreStatus', () => {
+    it('handles status update', () =>
+      actions.updateIgnoreStatus({ commit, dispatch }, params).then(() => {
+        expect(commit).toHaveBeenCalledWith(types.SET_UPDATING_IGNORE_STATUS, true);
+        expect(commit).toHaveBeenCalledWith(types.SET_UPDATING_IGNORE_STATUS, false);
+        expect(dispatch).toHaveBeenCalledWith('updateStatus', params);
+      }));
+  });
 });

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(:list_path) { project_error_tracking_index_path(project) }
     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) }
@@ -91,10 +90,6 @@ describe Projects::ErrorTrackingHelper do
 
     let(:result) { helper.error_details_data(project, issue_id) }
 
-    it 'returns the correct list path' do
-      expect(result['list-path']).to eq list_path
-    end
-
     it 'returns the correct issue id' do
       expect(result['issue-id']).to eq issue_id
     end