Merge branch 'group-security-dashboard-action-button-improvements' into 'master'

Adds a few UX tweaks after a call with @andyvolpe

See merge request gitlab-org/gitlab-ee!8217

ee/app/assets/javascripts/security_dashboard/components/app.vue

@@ -54,14 +54,14 @@ export default {
         title: s__('Security Reports|At this time, the security dashboard only supports SAST.'),
         content: `
           <a
-            title="${s__('Security Reports|Security Dashboard Documentation')}"
+            title="${s__('Security Reports|Security dashboard documentation')}"
             href="${this.dashboardDocumentation}"
             target="_blank"
             rel="noopener
             noreferrer"
           >
             <span class="vertical-align-middle">${s__(
-              'Security Reports|Security Dashboard Documentation',
+              'Security Reports|Security dashboard documentation',
             )}</span>
             ${spriteIcon('external-link', 's16 vertical-align-middle')}
           </a>
@@ -82,7 +82,7 @@ export default {
       'fetchVulnerabilitiesCount',
       'createIssue',
       'dismissVulnerability',
-      'undoDismissal',
+      'revertDismissal',
     ]),
   },
 };
@@ -123,7 +123,7 @@ export default {
       :can-create-feedback-permission="true"
       @createNewIssue="createIssue({ vulnerability: modal.vulnerability })"
       @dismissIssue="dismissVulnerability({ vulnerability: modal.vulnerability })"
-      @revertDismissIssue="undoDismissal({ vulnerability: modal.vulnerability })"
+      @revertDismissIssue="revertDismissal({ vulnerability: modal.vulnerability })"
     />
   </div>
 </template>

ee/app/assets/javascripts/security_dashboard/components/vulnerability_action_buttons.vue

@@ -42,7 +42,7 @@ export default {
       'openModal',
       'createIssue',
       'dismissVulnerability',
-      'undoDismissal',
+      'revertDismissal',
     ]),
     handleCreateIssue() {
       const { vulnerability } = this;
@@ -52,9 +52,9 @@ export default {
       const { vulnerability } = this;
       this.dismissVulnerability({ vulnerability, flashError: true });
     },
-    handleUndoDismissal() {
+    handleRevertDismissal() {
       const { vulnerability } = this;
-      this.undoDismissal({ vulnerability, flashError: true });
+      this.revertDismissal({ vulnerability, flashError: true });
     },
   },
 };
@@ -67,7 +67,7 @@ export default {
       v-gl-tooltip
       :aria-label="s__('Security Reports|More info')"
       :title="s__('Security Reports|More info')"
-      class="btn btn-secondary js-more-info"
+      class="btn btn-secondary btn-icon js-more-info"
       type="button"
       @click="openModal({ vulnerability })"
     >
@@ -77,10 +77,10 @@ export default {
       v-if="canCreateIssue"
       key="create-issue"
       v-gl-tooltip
-      :aria-label="s__('Security Reports|New Issue')"
+      :aria-label="s__('Security Reports|Create issue')"
       :loading="isCreatingIssue"
-      :title="s__('Security Reports|New Issue')"
-      container-class="btn btn-inverted btn-success js-create-issue"
+      :title="s__('Security Reports|Create issue')"
+      container-class="btn btn-inverted btn-success prepend-left-8 js-create-issue"
       type="button"
       @click="handleCreateIssue"
     >
@@ -89,21 +89,21 @@ export default {
     <template v-if="canDismissVulnerability">
       <loading-button
         v-if="isDismissed"
-        key="undo-dismissal"
-        :label="s__('Security Reports|Undo Dismissal')"
+        key="revert-dismissal"
+        :label="s__('Security Reports|Revert dismissal')"
         :loading="isDismissingVulnerability"
-        container-class="btn btn-inverted btn-warning js-undo-dismissal"
+        container-class="btn btn-inverted btn-warning prepend-left-8 js-revert-dismissal"
         type="button"
-        @click="handleUndoDismissal"
+        @click="handleRevertDismissal"
       />
       <loading-button
         v-else
         key="dismiss-vulnerability"
         v-gl-tooltip
-        :aria-label="s__('Security Reports|Dismiss Vulnerability')"
+        :aria-label="s__('Security Reports|Dismiss vulnerability')"
         :loading="isDismissingVulnerability"
-        :title="s__('Security Reports|Dismiss Vulnerability')"
-        container-class="btn btn-inverted btn-warning js-dismiss-vulnerability"
+        :title="s__('Security Reports|Dismiss vulnerability')"
+        container-class="btn btn-inverted btn-warning prepend-left-8 js-dismiss-vulnerability"
         type="button"
         @click="handleDismissVulnerability"
       >

ee/app/assets/javascripts/security_dashboard/store/modules/vulnerabilities/actions.js

@@ -167,37 +167,37 @@ export const receiveDismissVulnerabilityError = ({ commit }, { flashError }) =>
   }
 };
 
-export const undoDismissal = ({ dispatch }, { vulnerability, flashError }) => {
+export const revertDismissal = ({ dispatch }, { vulnerability, flashError }) => {
   const { vulnerability_feedback_url, dismissal_feedback } = vulnerability;
   // eslint-disable-next-line camelcase
   const url = `${vulnerability_feedback_url}/${dismissal_feedback.id}`;
 
-  dispatch('requestUndoDismissal');
+  dispatch('requestRevertDismissal');
 
   axios
     .delete(url)
     .then(() => {
       const { id } = vulnerability;
-      dispatch('receiveUndoDismissalSuccess', { id });
+      dispatch('receiveRevertDismissalSuccess', { id });
     })
     .catch(() => {
-      dispatch('receiveUndoDismissalError', { flashError });
+      dispatch('receiveRevertDismissalError', { flashError });
     });
 };
 
-export const requestUndoDismissal = ({ commit }) => {
-  commit(types.REQUEST_UNDO_DISMISSAL);
+export const requestRevertDismissal = ({ commit }) => {
+  commit(types.REQUEST_REVERT_DISMISSAL);
 };
 
-export const receiveUndoDismissalSuccess = ({ commit }, payload) => {
-  commit(types.RECEIVE_UNDO_DISMISSAL_SUCCESS, payload);
+export const receiveRevertDismissalSuccess = ({ commit }, payload) => {
+  commit(types.RECEIVE_REVERT_DISMISSAL_SUCCESS, payload);
 };
 
-export const receiveUndoDismissalError = ({ commit }, { flashError }) => {
-  commit(types.RECEIVE_UNDO_DISMISSAL_ERROR);
+export const receiveRevertDismissalError = ({ commit }, { flashError }) => {
+  commit(types.RECEIVE_REVERT_DISMISSAL_ERROR);
   if (flashError) {
     createFlash(
-      s__('Security Reports|There was an error undoing this dismissal.'),
+      s__('Security Reports|There was an error reverting this dismissal.'),
       'alert',
       document.querySelector('.ci-table'),
     );

ee/app/assets/javascripts/security_dashboard/store/modules/vulnerabilities/mutation_types.js

@@ -18,6 +18,6 @@ export const REQUEST_DISMISS_VULNERABILITY = 'REQUEST_DISMISS_VULNERABILITY';
 export const RECEIVE_DISMISS_VULNERABILITY_SUCCESS = 'RECEIVE_DISMISS_VULNERABILITY_SUCCESS';
 export const RECEIVE_DISMISS_VULNERABILITY_ERROR = 'RECEIVE_DISMISS_VULNERABILITY_ERROR';
 
-export const REQUEST_UNDO_DISMISSAL = 'REQUEST_UNDO_DISMISSAL';
-export const RECEIVE_UNDO_DISMISSAL_SUCCESS = 'RECEIVE_UNDO_DISMISSAL_SUCCESS';
-export const RECEIVE_UNDO_DISMISSAL_ERROR = 'RECEIVE_UNDO_DISMISSAL_ERROR';
+export const REQUEST_REVERT_DISMISSAL = 'REQUEST_REVERT_DISMISSAL';
+export const RECEIVE_REVERT_DISMISSAL_SUCCESS = 'RECEIVE_REVERT_DISMISSAL_SUCCESS';
+export const RECEIVE_REVERT_DISMISSAL_ERROR = 'RECEIVE_REVERT_DISMISSAL_ERROR';

ee/app/assets/javascripts/security_dashboard/store/modules/vulnerabilities/mutations.js

@@ -116,25 +116,25 @@ export default {
       s__('Security Reports|There was an error dismissing the vulnerability.'),
     );
   },
-  [types.REQUEST_UNDO_DISMISSAL](state) {
+  [types.REQUEST_REVERT_DISMISSAL](state) {
     state.isDismissingVulnerability = true;
     Vue.set(state.modal, 'isDismissingVulnerability', true);
     Vue.set(state.modal, 'error', null);
   },
-  [types.RECEIVE_UNDO_DISMISSAL_SUCCESS](state, payload) {
+  [types.RECEIVE_REVERT_DISMISSAL_SUCCESS](state, payload) {
     const vulnerability = state.vulnerabilities.find(vuln => vuln.id === payload.id);
     vulnerability.dismissal_feedback = null;
     state.isDismissingVulnerability = false;
     Vue.set(state.modal, 'isDismissingVulnerability', false);
     Vue.set(state.modal.vulnerability, 'isDismissed', false);
   },
-  [types.RECEIVE_UNDO_DISMISSAL_ERROR](state) {
+  [types.RECEIVE_REVERT_DISMISSAL_ERROR](state) {
     state.isDismissingVulnerability = false;
     Vue.set(state.modal, 'isDismissingVulnerability', false);
     Vue.set(
       state.modal,
       'error',
-      s__('Security Reports|There was an error undoing the dismissal.'),
+      s__('Security Reports|There was an error reverting the dismissal.'),
     );
   },
 };

ee/changelogs/unreleased/group-security-dashboard-action-button-improvements.yml

+---
+title: UX improvements for the group security dashboard
+merge_request: 8217
+author:
+type: other

ee/spec/javascripts/security_dashboard/components/vulnerability_action_buttons_spec.js

@@ -124,10 +124,12 @@ describe('Security Dashboard Action Buttons', () => {
     });
   });
 
-  describe('with a vulnerbility that has been dismissed', () => {
+  describe('with a vulnerability that has been dismissed', () => {
     beforeEach(() => {
       props = {
         vulnerability: mockDataVulnerabilities[2],
+        canDismissVulnerability: true,
+        isDismissed: true,
       };
       vm = mountComponentWithStore(Component, { store, props });
     });
@@ -136,12 +138,16 @@ describe('Security Dashboard Action Buttons', () => {
       vm.$destroy();
     });
 
-    it('should only render one button', () => {
-      expect(vm.$el.querySelectorAll('.btn')).toHaveLength(1);
+    it('should render two buttons', () => {
+      expect(vm.$el.querySelectorAll('.btn')).toHaveLength(2);
     });
 
     it('should not render the dismiss vulnerability button', () => {
       expect(vm.$el.querySelector('.js-dismiss-vulnerability')).toBeNull();
     });
+
+    it('should render the revert dismissal button', () => {
+      expect(vm.$el.querySelector('.js-revert-dismissal')).not.toBeNull();
+    });
   });
 });

ee/spec/javascripts/security_dashboard/store/vulnerabilities/actions_spec.js

@@ -524,8 +524,8 @@ describe('vulnerability dismissal', () => {
   });
 });
 
-describe('undo vulnerability dismissal', () => {
-  describe('undoDismissal', () => {
+describe('revert vulnerability dismissal', () => {
+  describe('revertDismissal', () => {
     const vulnerability = mockDataVulnerabilities[2];
     const url = `${vulnerability.vulnerability_feedback_url}/${
       vulnerability.dismissal_feedback.id
@@ -547,13 +547,13 @@ describe('undo vulnerability dismissal', () => {
 
       it('should dispatch the request and success actions', done => {
         testAction(
-          actions.undoDismissal,
+          actions.revertDismissal,
           { vulnerability },
           {},
           [],
           [
-            { type: 'requestUndoDismissal' },
-            { type: 'receiveUndoDismissalSuccess', payload: { id: vulnerability.id } },
+            { type: 'requestRevertDismissal' },
+            { type: 'receiveRevertDismissalSuccess', payload: { id: vulnerability.id } },
           ],
           done,
         );
@@ -569,13 +569,13 @@ describe('undo vulnerability dismissal', () => {
         const flashError = false;
 
         testAction(
-          actions.undoDismissal,
+          actions.revertDismissal,
           { vulnerability, flashError },
           {},
           [],
           [
-            { type: 'requestUndoDismissal' },
-            { type: 'receiveUndoDismissalError', payload: { flashError: false } },
+            { type: 'requestRevertDismissal' },
+            { type: 'receiveRevertDismissalError', payload: { flashError: false } },
           ],
           done,
         );
@@ -583,18 +583,18 @@ describe('undo vulnerability dismissal', () => {
     });
   });
 
-  describe('receiveUndoDismissalSuccess', () => {
+  describe('receiveRevertDismissalSuccess', () => {
     it('should commit the success mutation', done => {
       const state = initialState;
       const data = mockDataVulnerabilities[0];
 
       testAction(
-        actions.receiveUndoDismissalSuccess,
+        actions.receiveRevertDismissalSuccess,
         { data },
         state,
         [
           {
-            type: types.RECEIVE_UNDO_DISMISSAL_SUCCESS,
+            type: types.RECEIVE_REVERT_DISMISSAL_SUCCESS,
             payload: { data },
           },
         ],
@@ -604,30 +604,30 @@ describe('undo vulnerability dismissal', () => {
     });
   });
 
-  describe('receiveUndoDismissalError', () => {
+  describe('receiveRevertDismissalError', () => {
     it('should commit the error mutation', done => {
       const state = initialState;
 
       testAction(
-        actions.receiveUndoDismissalError,
+        actions.receiveRevertDismissalError,
         {},
         state,
-        [{ type: types.RECEIVE_UNDO_DISMISSAL_ERROR }],
+        [{ type: types.RECEIVE_REVERT_DISMISSAL_ERROR }],
         [],
         done,
       );
     });
   });
 
-  describe('requestUndoDismissal', () => {
+  describe('requestRevertDismissal', () => {
     it('should commit the request mutation', done => {
       const state = initialState;
 
       testAction(
-        actions.requestUndoDismissal,
+        actions.requestRevertDismissal,
         {},
         state,
-        [{ type: types.REQUEST_UNDO_DISMISSAL }],
+        [{ type: types.REQUEST_REVERT_DISMISSAL }],
         [],
         done,
       );

ee/spec/javascripts/security_dashboard/store/vulnerabilities/mutations_spec.js

@@ -360,12 +360,12 @@ describe('vulnerabilities module mutations', () => {
     });
   });
 
-  describe('REQUEST_UNDO_DISMISSAL', () => {
+  describe('REQUEST_REVERT_DISMISSAL', () => {
     let state;
 
     beforeEach(() => {
       state = createState();
-      mutations[types.REQUEST_UNDO_DISMISSAL](state);
+      mutations[types.REQUEST_REVERT_DISMISSAL](state);
     });
 
     it('should set isDismissingVulnerability to true', () => {
@@ -381,7 +381,7 @@ describe('vulnerabilities module mutations', () => {
     });
   });
 
-  describe('RECEIVE_UNDO_DISMISSAL_SUCCESS', () => {
+  describe('RECEIVE_REVERT_DISMISSAL_SUCCESS', () => {
     let state;
     let payload;
     let vulnerability;
@@ -391,7 +391,7 @@ describe('vulnerabilities module mutations', () => {
       state.vulnerabilities = mockData;
       [vulnerability] = mockData;
       payload = { id: vulnerability.id };
-      mutations[types.RECEIVE_UNDO_DISMISSAL_SUCCESS](state, payload);
+      mutations[types.RECEIVE_REVERT_DISMISSAL_SUCCESS](state, payload);
     });
 
     it('should set the dismissal feedback on the passed vulnerability', () => {
@@ -411,12 +411,12 @@ describe('vulnerabilities module mutations', () => {
     });
   });
 
-  describe('RECEIVE_UNDO_DISMISSAL_ERROR', () => {
+  describe('RECEIVE_REVERT_DISMISSAL_ERROR', () => {
     let state;
 
     beforeEach(() => {
       state = createState();
-      mutations[types.RECEIVE_UNDO_DISMISSAL_ERROR](state);
+      mutations[types.RECEIVE_REVERT_DISMISSAL_ERROR](state);
     });
 
     it('should set isDismissingVulnerability to false', () => {
@@ -428,7 +428,7 @@ describe('vulnerabilities module mutations', () => {
     });
 
     it('should set the error state on the modal', () => {
-      expect(state.modal.error).toEqual('There was an error undoing the dismissal.');
+      expect(state.modal.error).toEqual('There was an error reverting the dismissal.');
     });
   });
 });

locale/gitlab.pot

@@ -7166,16 +7166,19 @@ msgstr ""
 msgid "Security Reports|At this time, the security dashboard only supports SAST."
 msgstr ""
 
-msgid "Security Reports|Dismiss Vulnerability"
+msgid "Security Reports|Create issue"
+msgstr ""
+
+msgid "Security Reports|Dismiss vulnerability"
 msgstr ""
 
 msgid "Security Reports|More info"
 msgstr ""
 
-msgid "Security Reports|New Issue"
+msgid "Security Reports|Revert dismissal"
 msgstr ""
 
-msgid "Security Reports|Security Dashboard Documentation"
+msgid "Security Reports|Security dashboard documentation"
 msgstr ""
 
 msgid "Security Reports|There was an error creating the issue."
@@ -7184,13 +7187,10 @@ msgstr ""
 msgid "Security Reports|There was an error dismissing the vulnerability."
 msgstr ""
 
-msgid "Security Reports|There was an error undoing the dismissal."
-msgstr ""
-
-msgid "Security Reports|There was an error undoing this dismissal."
+msgid "Security Reports|There was an error reverting the dismissal."
 msgstr ""
 
-msgid "Security Reports|Undo Dismissal"
+msgid "Security Reports|There was an error reverting this dismissal."
 msgstr ""
 
 msgid "SecurityDashboard| The security dashboard displays the latest security report. Use it to find and fix vulnerabilities."