From 106b60928b3af8c510360fb692dfbe5fb560b4ab Mon Sep 17 00:00:00 2001
From: samdbeckham <sbeckham@gitlab.com>
Date: Thu, 29 Aug 2019 13:07:07 +0100
Subject: [PATCH 1/4] Moves MR reports logic for SAST to the backend

- Copies over all the changes from the similar MRs for DAST, dependency
  and container scanning.
- Everything is behind an inactive feature flag.

See https://gitlab.com/gitlab-org/gitlab-ee/issues/12002 for more
information.
---
 .../grouped_security_reports_app.vue          |  16 ++-
 .../security_reports/store/actions.js         |  25 +---
 .../store/modules/sast/actions.js             |  28 ++++
 .../store/modules/sast/mutation_types.js      |   8 +-
 .../store/modules/sast/mutations.js           |  15 +-
 .../store/modules/sast/state.js               |   1 +
 .../security_reports/store/utils.js           |  32 +++++
 .../store/modules/sast/actions_spec.js        | 128 +++++++++++++++++-
 .../store/modules/sast/mutations_spec.js      |  42 ++++++
 .../grouped_security_reports_app_spec.js      |  47 +++++++
 10 files changed, 311 insertions(+), 31 deletions(-)

diff --git a/ee/app/assets/javascripts/vue_shared/security_reports/grouped_security_reports_app.vue b/ee/app/assets/javascripts/vue_shared/security_reports/grouped_security_reports_app.vue
index eb355437a8f83c..78e86a3702eb3e 100644
--- a/ee/app/assets/javascripts/vue_shared/security_reports/grouped_security_reports_app.vue
+++ b/ee/app/assets/javascripts/vue_shared/security_reports/grouped_security_reports_app.vue
@@ -181,6 +181,11 @@ export default {
     shouldRenderDast() {
       const { head, diffEndpoint } = this.dast.paths;
 
+      return head || diffEndpoint;
+    },
+    shouldRenderSast() {
+      const { head, diffEndpoint } = this.sast.paths;
+
       return head || diffEndpoint;
     },
   },
@@ -202,7 +207,12 @@ export default {
     this.setCanCreateIssuePermission(this.canCreateIssue);
     this.setCanCreateFeedbackPermission(this.canCreateFeedback);
 
-    if (this.sastHeadPath) {
+    const sastDiffEndpoint = gl && gl.mrWidgetData && gl.mrWidgetData.sast_comparison_path;
+
+    if (gon.features && gon.features.sastMergeRequestReportApi && sastDiffEndpoint) {
+      this.setSastDiffEndpoint(sastDiffEndpoint);
+      this.fetchSastDiff();
+    } else if (this.sastHeadPath) {
       this.setSastHeadPath(this.sastHeadPath);
 
       if (this.sastBasePath) {
@@ -307,7 +317,9 @@ export default {
     ...mapActions('sast', {
       setSastHeadPath: 'setHeadPath',
       setSastBasePath: 'setBasePath',
+      setSastDiffEndpoint: 'setDiffEndpoint',
       fetchSastReports: 'fetchReports',
+      fetchSastDiff: 'fetchDiff',
     }),
   },
 };
@@ -333,7 +345,7 @@ export default {
     </div>
 
     <div slot="body" class="mr-widget-grouped-section report-block">
-      <template v-if="sastHeadPath">
+      <template v-if="shouldRenderSast">
         <summary-row
           :summary="groupedSastText"
           :status-icon="sastStatusIcon"
diff --git a/ee/app/assets/javascripts/vue_shared/security_reports/store/actions.js b/ee/app/assets/javascripts/vue_shared/security_reports/store/actions.js
index c10c31b6a68bba..9f4ad258f492f8 100644
--- a/ee/app/assets/javascripts/vue_shared/security_reports/store/actions.js
+++ b/ee/app/assets/javascripts/vue_shared/security_reports/store/actions.js
@@ -4,8 +4,7 @@ import { s__ } from '~/locale';
 import { visitUrl } from '~/lib/utils/url_utility';
 import * as types from './mutation_types';
 import downloadPatchHelper from './utils/download_patch_helper';
-import Poll from '~/lib/utils/poll';
-import httpStatusCodes from '~/lib/utils/http_status';
+import { pollUntilComplete } from './utils';
 
 /**
  * A lot of this file has duplicate actions to
@@ -18,28 +17,6 @@ import httpStatusCodes from '~/lib/utils/http_status';
 
 const hideModal = () => $('#modal-mrwidget-security-issue').modal('hide');
 
-const pollUntilComplete = endpoint =>
-  new Promise((resolve, reject) => {
-    const eTagPoll = new Poll({
-      resource: {
-        getReports(url) {
-          return axios.get(url);
-        },
-      },
-      data: endpoint,
-      method: 'getReports',
-      successCallback: response => {
-        if (response.status === httpStatusCodes.OK) {
-          resolve(response);
-          eTagPoll.stop();
-        }
-      },
-      errorCallback: reject,
-    });
-
-    eTagPoll.makeRequest();
-  });
-
 export const setHeadBlobPath = ({ commit }, blobPath) => commit(types.SET_HEAD_BLOB_PATH, blobPath);
 
 export const setBaseBlobPath = ({ commit }, blobPath) => commit(types.SET_BASE_BLOB_PATH, blobPath);
diff --git a/ee/app/assets/javascripts/vue_shared/security_reports/store/modules/sast/actions.js b/ee/app/assets/javascripts/vue_shared/security_reports/store/modules/sast/actions.js
index 10b912ab038008..0e5da88ee3c44d 100644
--- a/ee/app/assets/javascripts/vue_shared/security_reports/store/modules/sast/actions.js
+++ b/ee/app/assets/javascripts/vue_shared/security_reports/store/modules/sast/actions.js
@@ -1,10 +1,13 @@
 import axios from '~/lib/utils/axios_utils';
 import * as types from './mutation_types';
+import { pollUntilComplete } from '../../utils';
 
 export const setHeadPath = ({ commit }, path) => commit(types.SET_HEAD_PATH, path);
 
 export const setBasePath = ({ commit }, path) => commit(types.SET_BASE_PATH, path);
 
+export const setDiffEndpoint = ({ commit }, path) => commit(types.SET_DIFF_ENDPOINT, path);
+
 export const requestReports = ({ commit }) => commit(types.REQUEST_REPORTS);
 
 export const receiveReports = ({ commit }, response) => commit(types.RECEIVE_REPORTS, response);
@@ -44,4 +47,29 @@ export const fetchReports = ({ state, rootState, dispatch }) => {
 export const updateVulnerability = ({ commit }, vulnerability) =>
   commit(types.UPDATE_VULNERABILITY, vulnerability);
 
+export const receiveDiffSuccess = ({ commit }, response) =>
+  commit(types.RECEIVE_DIFF_SUCCESS, response);
+
+export const fetchDiff = ({ state, rootState, dispatch }) => {
+  dispatch('requestReports');
+
+  return Promise.all([
+    pollUntilComplete(state.paths.diffEndpoint),
+    axios.get(rootState.vulnerabilityFeedbackPath, {
+      params: {
+        category: 'sast',
+      },
+    }),
+  ])
+    .then(values => {
+      dispatch('receiveDiffSuccess', {
+        diff: values[0].data,
+        enrichData: values[1].data,
+      });
+    })
+    .catch(() => {
+      dispatch('receiveError');
+    });
+};
+
 export default () => {};
diff --git a/ee/app/assets/javascripts/vue_shared/security_reports/store/modules/sast/mutation_types.js b/ee/app/assets/javascripts/vue_shared/security_reports/store/modules/sast/mutation_types.js
index 9ff696eb869931..6cdba6991a46ee 100644
--- a/ee/app/assets/javascripts/vue_shared/security_reports/store/modules/sast/mutation_types.js
+++ b/ee/app/assets/javascripts/vue_shared/security_reports/store/modules/sast/mutation_types.js
@@ -1,6 +1,8 @@
-export const SET_HEAD_PATH = 'SET_HEAD_PATH';
-export const SET_BASE_PATH = 'SET_BASE_PATH';
-export const REQUEST_REPORTS = 'REQUEST_REPORTS';
+export const RECEIVE_DIFF_SUCCESS = 'RECEIVE_DIFF_SUCCESS';
 export const RECEIVE_REPORTS = 'RECEIVE_REPORTS';
 export const RECEIVE_REPORTS_ERROR = 'RECEIVE_REPORTS_ERROR';
+export const REQUEST_REPORTS = 'REQUEST_REPORTS';
+export const SET_BASE_PATH = 'SET_BASE_PATH';
+export const SET_DIFF_ENDPOINT = 'SET_DIFF_ENDPOINT';
+export const SET_HEAD_PATH = 'SET_HEAD_PATH';
 export const UPDATE_VULNERABILITY = 'UPDATE_VULNERABILITY';
diff --git a/ee/app/assets/javascripts/vue_shared/security_reports/store/modules/sast/mutations.js b/ee/app/assets/javascripts/vue_shared/security_reports/store/modules/sast/mutations.js
index d044082d08b9b0..0c02d79d37d118 100644
--- a/ee/app/assets/javascripts/vue_shared/security_reports/store/modules/sast/mutations.js
+++ b/ee/app/assets/javascripts/vue_shared/security_reports/store/modules/sast/mutations.js
@@ -1,6 +1,6 @@
 import Vue from 'vue';
 import * as types from './mutation_types';
-import { parseSastIssues, findIssueIndex } from '../../utils';
+import { parseSastIssues, findIssueIndex, parseDiff } from '../../utils';
 import filterByKey from '../../utils/filter_by_key';
 
 export default {
@@ -12,6 +12,10 @@ export default {
     Vue.set(state.paths, 'base', path);
   },
 
+  [types.SET_DIFF_ENDPOINT](state, path) {
+    Vue.set(state.paths, 'diffEndpoint', path);
+  },
+
   [types.REQUEST_REPORTS](state) {
     state.isLoading = true;
   },
@@ -55,6 +59,15 @@ export default {
     }
   },
 
+  [types.RECEIVE_DIFF_SUCCESS](state, { diff, enrichData }) {
+    const { added, fixed, existing } = parseDiff(diff, enrichData);
+
+    state.isLoading = false;
+    state.newIssues = added;
+    state.resolvedIssues = fixed;
+    state.allIssues = existing;
+  },
+
   [types.RECEIVE_REPORTS_ERROR](state) {
     state.isLoading = false;
     state.hasError = true;
diff --git a/ee/app/assets/javascripts/vue_shared/security_reports/store/modules/sast/state.js b/ee/app/assets/javascripts/vue_shared/security_reports/store/modules/sast/state.js
index f02b626e4f0453..02693c4e6026d2 100644
--- a/ee/app/assets/javascripts/vue_shared/security_reports/store/modules/sast/state.js
+++ b/ee/app/assets/javascripts/vue_shared/security_reports/store/modules/sast/state.js
@@ -2,6 +2,7 @@ export default () => ({
   paths: {
     head: null,
     base: null,
+    diffEndpoint: null,
   },
 
   isLoading: false,
diff --git a/ee/app/assets/javascripts/vue_shared/security_reports/store/utils.js b/ee/app/assets/javascripts/vue_shared/security_reports/store/utils.js
index b119be8839937d..e21d3930958d82 100644
--- a/ee/app/assets/javascripts/vue_shared/security_reports/store/utils.js
+++ b/ee/app/assets/javascripts/vue_shared/security_reports/store/utils.js
@@ -1,7 +1,10 @@
 import sha1 from 'sha1';
 import _ from 'underscore';
+import axios from 'axios';
 import { stripHtml } from '~/lib/utils/text_utility';
 import { n__, s__, sprintf } from '~/locale';
+import Poll from '~/lib/utils/poll';
+import httpStatusCodes from '~/lib/utils/http_status';
 
 /**
  * Returns the index of an issue in given list
@@ -441,3 +444,32 @@ export const parseDiff = (diff, enrichData) => {
     existing: diff.existing ? diff.existing.map(enrichVulnerability) : [],
   };
 };
+
+/**
+ * Polls an endpoint untill it returns either an error or a 200.
+ * Resolving or rejecting the promise accordingly.
+ *
+ * @param {String} endpoint the endpoint to poll.
+ * @returns {Promise}
+ */
+export const pollUntilComplete = endpoint =>
+  new Promise((resolve, reject) => {
+    const eTagPoll = new Poll({
+      resource: {
+        getReports(url) {
+          return axios.get(url);
+        },
+      },
+      data: endpoint,
+      method: 'getReports',
+      successCallback: response => {
+        if (response.status === httpStatusCodes.OK) {
+          resolve(response);
+          eTagPoll.stop();
+        }
+      },
+      errorCallback: reject,
+    });
+
+    eTagPoll.makeRequest();
+  });
diff --git a/ee/spec/frontend/vue_shared/security_reports/store/modules/sast/actions_spec.js b/ee/spec/frontend/vue_shared/security_reports/store/modules/sast/actions_spec.js
index 617a20c61a5c5e..6c2273a6135d03 100644
--- a/ee/spec/frontend/vue_shared/security_reports/store/modules/sast/actions_spec.js
+++ b/ee/spec/frontend/vue_shared/security_reports/store/modules/sast/actions_spec.js
@@ -8,11 +8,13 @@ import * as actions from 'ee/vue_shared/security_reports/store/modules/sast/acti
 
 const headPath = 'head-path.json';
 const basePath = 'base-path.json';
+const diffEndpoint = 'diff-endpoint.json';
 const blobPath = 'blob-path.json';
 const reports = {
   base: 'base',
   head: 'head',
   enrichData: 'enrichData',
+  diff: 'diff',
 };
 const error = 'Something went wrong';
 const issue = {};
@@ -62,6 +64,24 @@ describe('sast report actions', () => {
     });
   });
 
+  describe('setDiffEndpoint', () => {
+    it(`should commit ${types.SET_DIFF_ENDPOINT} with the correct path`, done => {
+      testAction(
+        actions.setDiffEndpoint,
+        diffEndpoint,
+        state,
+        [
+          {
+            type: types.SET_DIFF_ENDPOINT,
+            payload: diffEndpoint,
+          },
+        ],
+        [],
+        done,
+      );
+    });
+  });
+
   describe('requestReports', () => {
     it(`should commit ${types.REQUEST_REPORTS}`, done => {
       testAction(actions.requestReports, {}, state, [{ type: types.REQUEST_REPORTS }], [], done);
@@ -129,6 +149,8 @@ describe('sast report actions', () => {
       });
 
       it('should dispatch the `receiveReports` action', done => {
+        const { head, base, enrichData } = reports;
+
         testAction(
           actions.fetchReports,
           {},
@@ -140,7 +162,7 @@ describe('sast report actions', () => {
               type: 'receiveReports',
               payload: {
                 blobPath,
-                reports,
+                reports: { head, base, enrichData },
               },
             },
           ],
@@ -173,6 +195,110 @@ describe('sast report actions', () => {
     });
   });
 
+  describe('receiveDiffSuccess', () => {
+    it(`should commit ${types.RECEIVE_DIFF_SUCCESS} with the correct response`, done => {
+      testAction(
+        actions.receiveDiffSuccess,
+        reports,
+        state,
+        [
+          {
+            type: types.RECEIVE_DIFF_SUCCESS,
+            payload: reports,
+          },
+        ],
+        [],
+        done,
+      );
+    });
+  });
+
+  describe('fetchDiff', () => {
+    let mock;
+
+    beforeEach(() => {
+      mock = new MockAdapter(axios);
+      state.paths.diffEndpoint = diffEndpoint;
+    });
+
+    afterEach(() => {
+      mock.restore();
+    });
+
+    describe('when everything goes according to plan', () => {
+      beforeEach(() => {
+        mock
+          .onGet(diffEndpoint)
+          .replyOnce(200, reports.diff)
+          .onGet(vulnerabilityFeedbackPath)
+          .replyOnce(200, reports.enrichData);
+      });
+
+      it('should dispatch the `receiveDiffSuccess` action', done => {
+        const { diff, enrichData } = reports;
+        testAction(
+          actions.fetchDiff,
+          {},
+          { ...rootState, ...state },
+          [],
+          [
+            { type: 'requestReports' },
+            {
+              type: 'receiveDiffSuccess',
+              payload: {
+                diff,
+                enrichData,
+              },
+            },
+          ],
+          done,
+        );
+      });
+    });
+
+    describe('when the vulnerability feedback endpoint fails', () => {
+      beforeEach(() => {
+        mock
+          .onGet(diffEndpoint)
+          .replyOnce(200, reports.diff)
+          .onGet(vulnerabilityFeedbackPath)
+          .replyOnce(404);
+      });
+
+      it('should dispatch the `receiveError` action', done => {
+        testAction(
+          actions.fetchDiff,
+          {},
+          { ...rootState, ...state },
+          [],
+          [{ type: 'requestReports' }, { type: 'receiveError' }],
+          done,
+        );
+      });
+    });
+
+    describe('when the diff endpoint fails', () => {
+      beforeEach(() => {
+        mock
+          .onGet(diffEndpoint)
+          .replyOnce(404)
+          .onGet(vulnerabilityFeedbackPath)
+          .replyOnce(200, reports.enrichData);
+      });
+
+      it('should dispatch the `receiveError` action', done => {
+        testAction(
+          actions.fetchDiff,
+          {},
+          { ...rootState, ...state },
+          [],
+          [{ type: 'requestReports' }, { type: 'receiveError' }],
+          done,
+        );
+      });
+    });
+  });
+
   describe('updateVulnerability', () => {
     it(`should commit ${types.UPDATE_VULNERABILITY} with the correct response`, done => {
       testAction(
diff --git a/ee/spec/frontend/vue_shared/security_reports/store/modules/sast/mutations_spec.js b/ee/spec/frontend/vue_shared/security_reports/store/modules/sast/mutations_spec.js
index 6b00475371e8d4..ef6abf020cc849 100644
--- a/ee/spec/frontend/vue_shared/security_reports/store/modules/sast/mutations_spec.js
+++ b/ee/spec/frontend/vue_shared/security_reports/store/modules/sast/mutations_spec.js
@@ -28,6 +28,14 @@ describe('sast module mutations', () => {
     });
   });
 
+  describe(types.SET_DIFF_ENDPOINT, () => {
+    it('should set the SAST diff endpoint', () => {
+      mutations[types.SET_DIFF_ENDPOINT](state, path);
+
+      expect(state.paths.diffEndpoint).toBe(path);
+    });
+  });
+
   describe(types.REQUEST_REPORTS, () => {
     it('should set the `isLoading` status to `true`', () => {
       mutations[types.REQUEST_REPORTS](state);
@@ -177,4 +185,38 @@ describe('sast module mutations', () => {
       });
     });
   });
+
+  describe(types.RECEIVE_DIFF_SUCCESS, () => {
+    beforeEach(() => {
+      const reports = {
+        diff: {
+          added: [
+            createIssue({ cve: 'CVE-1' }),
+            createIssue({ cve: 'CVE-2' }),
+            createIssue({ cve: 'CVE-3' }),
+          ],
+          fixed: [createIssue({ cve: 'CVE-4' }), createIssue({ cve: 'CVE-5' })],
+          existing: [createIssue({ cve: 'CVE-6' })],
+        },
+      };
+      state.isLoading = true;
+      mutations[types.RECEIVE_DIFF_SUCCESS](state, reports);
+    });
+
+    it('should set the `isLoading` status to `false`', () => {
+      expect(state.isLoading).toBe(false);
+    });
+
+    it('should have the relevant `new` issues', () => {
+      expect(state.newIssues.length).toBe(3);
+    });
+
+    it('should have the relevant `resolved` issues', () => {
+      expect(state.resolvedIssues.length).toBe(2);
+    });
+
+    it('should have the relevant `all` issues', () => {
+      expect(state.allIssues.length).toBe(1);
+    });
+  });
 });
diff --git a/ee/spec/javascripts/vue_shared/security_reports/grouped_security_reports_app_spec.js b/ee/spec/javascripts/vue_shared/security_reports/grouped_security_reports_app_spec.js
index 6aea0c34a7d6bd..f0c953a230babe 100644
--- a/ee/spec/javascripts/vue_shared/security_reports/grouped_security_reports_app_spec.js
+++ b/ee/spec/javascripts/vue_shared/security_reports/grouped_security_reports_app_spec.js
@@ -414,5 +414,52 @@ describe('Grouped security reports app', () => {
         expect(vm.$el.textContent).toContain('DAST detected 1 new, and 2 fixed vulnerabilities');
       });
     });
+
+    describe('sast reports', () => {
+      const sastEndpoint = 'sast.json';
+
+      beforeEach(done => {
+        gon.features = gon.features || {};
+        gon.features.sastMergeRequestReportApi = true;
+
+        gl.mrWidgetData = gl.mrWidgetData || {};
+        gl.mrWidgetData.sast_comparison_path = sastEndpoint;
+
+        mock.onGet(sastEndpoint).reply(200, {
+          added: [dockerReport.vulnerabilities[0]],
+          fixed: [dockerReport.vulnerabilities[1], dockerReport.vulnerabilities[2]],
+          existing: [dockerReport.vulnerabilities[2]],
+        });
+
+        mock.onGet('vulnerability_feedback_path.json').reply(200, []);
+
+        vm = mountComponent(Component, {
+          headBlobPath: 'path',
+          baseBlobPath: 'path',
+          sastHelpPath: 'path',
+          sastContainerHelpPath: 'path',
+          dastHelpPath: 'path',
+          dependencyScanningHelpPath: 'path',
+          vulnerabilityFeedbackPath: 'vulnerability_feedback_path.json',
+          vulnerabilityFeedbackHelpPath: 'path',
+          pipelineId: 123,
+          canCreateIssue: true,
+          canCreateMergeRequest: true,
+          canDismissVulnerability: true,
+        });
+
+        waitForMutation(vm.$store, `sast/${sastTypes.RECEIVE_DIFF_SUCCESS}`)
+          .then(done)
+          .catch(done.fail);
+      });
+
+      it('should set setSastDiffEndpoint', () => {
+        expect(vm.sast.paths.diffEndpoint).toEqual(sastEndpoint);
+      });
+
+      it('should call `fetchSastDiff`', () => {
+        expect(vm.$el.textContent).toContain('SAST detected 1 new, and 2 fixed vulnerabilities');
+      });
+    });
   });
 });
-- 
GitLab


From dc8fc08d6ae0f4aff5d24c0b55b0065a698c4279 Mon Sep 17 00:00:00 2001
From: Mark Florian <mflorian@gitlab.com>
Date: Tue, 3 Sep 2019 16:10:29 +0100
Subject: [PATCH 2/4] Apply review suggestsions from @filipa

This adds `RECEIVE_.*_DIFF_ERROR` mutations and associated actions to
the security reports store, to be more consistent with the Vuex
guidelines.
---
 .../store/modules/sast/actions.js             |  5 +++-
 .../store/modules/sast/mutation_types.js      |  1 +
 .../store/modules/sast/mutations.js           |  5 ++++
 .../store/modules/sast/actions_spec.js        | 24 ++++++++++++++++---
 .../store/modules/sast/mutations_spec.js      | 15 ++++++++++++
 5 files changed, 46 insertions(+), 4 deletions(-)

diff --git a/ee/app/assets/javascripts/vue_shared/security_reports/store/modules/sast/actions.js b/ee/app/assets/javascripts/vue_shared/security_reports/store/modules/sast/actions.js
index 0e5da88ee3c44d..36960a3a1332b4 100644
--- a/ee/app/assets/javascripts/vue_shared/security_reports/store/modules/sast/actions.js
+++ b/ee/app/assets/javascripts/vue_shared/security_reports/store/modules/sast/actions.js
@@ -50,6 +50,9 @@ export const updateVulnerability = ({ commit }, vulnerability) =>
 export const receiveDiffSuccess = ({ commit }, response) =>
   commit(types.RECEIVE_DIFF_SUCCESS, response);
 
+export const receiveDiffError = ({ commit }, response) =>
+  commit(types.RECEIVE_DIFF_ERROR, response);
+
 export const fetchDiff = ({ state, rootState, dispatch }) => {
   dispatch('requestReports');
 
@@ -68,7 +71,7 @@ export const fetchDiff = ({ state, rootState, dispatch }) => {
       });
     })
     .catch(() => {
-      dispatch('receiveError');
+      dispatch('receiveDiffError');
     });
 };
 
diff --git a/ee/app/assets/javascripts/vue_shared/security_reports/store/modules/sast/mutation_types.js b/ee/app/assets/javascripts/vue_shared/security_reports/store/modules/sast/mutation_types.js
index 6cdba6991a46ee..4897f58789de44 100644
--- a/ee/app/assets/javascripts/vue_shared/security_reports/store/modules/sast/mutation_types.js
+++ b/ee/app/assets/javascripts/vue_shared/security_reports/store/modules/sast/mutation_types.js
@@ -1,4 +1,5 @@
 export const RECEIVE_DIFF_SUCCESS = 'RECEIVE_DIFF_SUCCESS';
+export const RECEIVE_DIFF_ERROR = 'RECEIVE_DIFF_ERROR';
 export const RECEIVE_REPORTS = 'RECEIVE_REPORTS';
 export const RECEIVE_REPORTS_ERROR = 'RECEIVE_REPORTS_ERROR';
 export const REQUEST_REPORTS = 'REQUEST_REPORTS';
diff --git a/ee/app/assets/javascripts/vue_shared/security_reports/store/modules/sast/mutations.js b/ee/app/assets/javascripts/vue_shared/security_reports/store/modules/sast/mutations.js
index 0c02d79d37d118..ff8a0b5f752127 100644
--- a/ee/app/assets/javascripts/vue_shared/security_reports/store/modules/sast/mutations.js
+++ b/ee/app/assets/javascripts/vue_shared/security_reports/store/modules/sast/mutations.js
@@ -68,6 +68,11 @@ export default {
     state.allIssues = existing;
   },
 
+  [types.RECEIVE_DIFF_ERROR](state) {
+    state.isLoading = false;
+    state.hasError = true;
+  },
+
   [types.RECEIVE_REPORTS_ERROR](state) {
     state.isLoading = false;
     state.hasError = true;
diff --git a/ee/spec/frontend/vue_shared/security_reports/store/modules/sast/actions_spec.js b/ee/spec/frontend/vue_shared/security_reports/store/modules/sast/actions_spec.js
index 6c2273a6135d03..23b29956ed4239 100644
--- a/ee/spec/frontend/vue_shared/security_reports/store/modules/sast/actions_spec.js
+++ b/ee/spec/frontend/vue_shared/security_reports/store/modules/sast/actions_spec.js
@@ -213,6 +213,24 @@ describe('sast report actions', () => {
     });
   });
 
+  describe('receiveDiffError', () => {
+    it(`should commit ${types.RECEIVE_DIFF_ERROR} with the correct response`, done => {
+      testAction(
+        actions.receiveDiffError,
+        error,
+        state,
+        [
+          {
+            type: types.RECEIVE_DIFF_ERROR,
+            payload: error,
+          },
+        ],
+        [],
+        done,
+      );
+    });
+  });
+
   describe('fetchDiff', () => {
     let mock;
 
@@ -271,7 +289,7 @@ describe('sast report actions', () => {
           {},
           { ...rootState, ...state },
           [],
-          [{ type: 'requestReports' }, { type: 'receiveError' }],
+          [{ type: 'requestReports' }, { type: 'receiveDiffError' }],
           done,
         );
       });
@@ -286,13 +304,13 @@ describe('sast report actions', () => {
           .replyOnce(200, reports.enrichData);
       });
 
-      it('should dispatch the `receiveError` action', done => {
+      it('should dispatch the `receiveDiffError` action', done => {
         testAction(
           actions.fetchDiff,
           {},
           { ...rootState, ...state },
           [],
-          [{ type: 'requestReports' }, { type: 'receiveError' }],
+          [{ type: 'requestReports' }, { type: 'receiveDiffError' }],
           done,
         );
       });
diff --git a/ee/spec/frontend/vue_shared/security_reports/store/modules/sast/mutations_spec.js b/ee/spec/frontend/vue_shared/security_reports/store/modules/sast/mutations_spec.js
index ef6abf020cc849..a02277c7cf3204 100644
--- a/ee/spec/frontend/vue_shared/security_reports/store/modules/sast/mutations_spec.js
+++ b/ee/spec/frontend/vue_shared/security_reports/store/modules/sast/mutations_spec.js
@@ -219,4 +219,19 @@ describe('sast module mutations', () => {
       expect(state.allIssues.length).toBe(1);
     });
   });
+
+  describe(types.RECEIVE_DIFF_ERROR, () => {
+    beforeEach(() => {
+      state.isLoading = true;
+      mutations[types.RECEIVE_DIFF_ERROR](state);
+    });
+
+    it('should set the `isLoading` status to `false`', () => {
+      expect(state.isLoading).toBe(false);
+    });
+
+    it('should set the `hasError` status to `true`', () => {
+      expect(state.hasError).toBe(true);
+    });
+  });
 });
-- 
GitLab


From df4c12f605ca98584c251d11fb6a0cae30d49484 Mon Sep 17 00:00:00 2001
From: Mark Florian <mflorian@gitlab.com>
Date: Mon, 9 Sep 2019 12:07:25 +0100
Subject: [PATCH 3/4] Improve descriptions in specs

---
 .../security_reports/store/modules/sast/actions_spec.js         | 2 +-
 .../security_reports/grouped_security_reports_app_spec.js       | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/ee/spec/frontend/vue_shared/security_reports/store/modules/sast/actions_spec.js b/ee/spec/frontend/vue_shared/security_reports/store/modules/sast/actions_spec.js
index 23b29956ed4239..00826c6424a2c4 100644
--- a/ee/spec/frontend/vue_shared/security_reports/store/modules/sast/actions_spec.js
+++ b/ee/spec/frontend/vue_shared/security_reports/store/modules/sast/actions_spec.js
@@ -243,7 +243,7 @@ describe('sast report actions', () => {
       mock.restore();
     });
 
-    describe('when everything goes according to plan', () => {
+    describe('when diff and vulnerability feedback endpoints respond successfully', () => {
       beforeEach(() => {
         mock
           .onGet(diffEndpoint)
diff --git a/ee/spec/javascripts/vue_shared/security_reports/grouped_security_reports_app_spec.js b/ee/spec/javascripts/vue_shared/security_reports/grouped_security_reports_app_spec.js
index f0c953a230babe..d69ec7d047f95f 100644
--- a/ee/spec/javascripts/vue_shared/security_reports/grouped_security_reports_app_spec.js
+++ b/ee/spec/javascripts/vue_shared/security_reports/grouped_security_reports_app_spec.js
@@ -314,7 +314,7 @@ describe('Grouped security reports app', () => {
         expect(vm.sastContainer.paths.diffEndpoint).toEqual(sastContainerEndpoint);
       });
 
-      it('should call `fetchSastContainerDiff`', () => {
+      it('should diplay the correct numbers of vulnerabilities', () => {
         expect(vm.$el.textContent).toContain(
           'Container scanning detected 2 new, and 1 fixed vulnerabilities',
         );
-- 
GitLab


From 47b5b1db5fa99589fa3c165351d2ce8bd11d4ce3 Mon Sep 17 00:00:00 2001
From: Mark Florian <mflorian@gitlab.com>
Date: Mon, 9 Sep 2019 18:09:06 +0100
Subject: [PATCH 4/4] Improve test descriptions

---
 .../security_reports/grouped_security_reports_app_spec.js | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/ee/spec/javascripts/vue_shared/security_reports/grouped_security_reports_app_spec.js b/ee/spec/javascripts/vue_shared/security_reports/grouped_security_reports_app_spec.js
index d69ec7d047f95f..d8fd29377944f8 100644
--- a/ee/spec/javascripts/vue_shared/security_reports/grouped_security_reports_app_spec.js
+++ b/ee/spec/javascripts/vue_shared/security_reports/grouped_security_reports_app_spec.js
@@ -314,7 +314,7 @@ describe('Grouped security reports app', () => {
         expect(vm.sastContainer.paths.diffEndpoint).toEqual(sastContainerEndpoint);
       });
 
-      it('should diplay the correct numbers of vulnerabilities', () => {
+      it('should display the correct numbers of vulnerabilities', () => {
         expect(vm.$el.textContent).toContain(
           'Container scanning detected 2 new, and 1 fixed vulnerabilities',
         );
@@ -362,7 +362,7 @@ describe('Grouped security reports app', () => {
         expect(vm.dependencyScanning.paths.diffEndpoint).toEqual(dependencyScanningEndpoint);
       });
 
-      it('should call `fetchDependencyScanningDiff`', () => {
+      it('should display the correct numbers of vulnerabilities', () => {
         expect(vm.$el.textContent).toContain(
           'Dependency scanning detected 2 new, and 1 fixed vulnerabilities',
         );
@@ -410,7 +410,7 @@ describe('Grouped security reports app', () => {
         expect(vm.dast.paths.diffEndpoint).toEqual(dastEndpoint);
       });
 
-      it('should call `fetchDastDiff`', () => {
+      it('should display the correct numbers of vulnerabilities', () => {
         expect(vm.$el.textContent).toContain('DAST detected 1 new, and 2 fixed vulnerabilities');
       });
     });
@@ -457,7 +457,7 @@ describe('Grouped security reports app', () => {
         expect(vm.sast.paths.diffEndpoint).toEqual(sastEndpoint);
       });
 
-      it('should call `fetchSastDiff`', () => {
+      it('should display the correct numbers of vulnerabilities', () => {
         expect(vm.$el.textContent).toContain('SAST detected 1 new, and 2 fixed vulnerabilities');
       });
     });
-- 
GitLab