[ci skip] Changes after review

ee/app/assets/javascripts/vue_shared/security_reports/store/actions.js

@@ -3,9 +3,7 @@ import * as types from './mutation_types';
 
 export const setHeadBlobPath = ({ commit }, blobPath) => commit(types.SET_HEAD_BLOB_PATH, blobPath);
 
-export const setBaseBlobPath = ({ commit }, blobPath) => {
-  commit(types.SET_BASE_BLOB_PATH, blobPath);
-};
+export const setBaseBlobPath = ({ commit }, blobPath) => commit(types.SET_BASE_BLOB_PATH, blobPath);
 
 /**
  * SAST
@@ -28,29 +26,19 @@ export const fetchSastReports = ({ state, dispatch }) => {
 
   dispatch('requestSastReports');
 
-  if (head && base) {
-    Promise.all([axios.get(head), axios.get(base)])
+  Promise.all([
+    head ? axios.get(head) : Promise.resolve(),
+    base ? axios.get(base) : Promise.resolve(),
+  ])
   .then(values => {
     dispatch('receiveSastReports', {
-          head: values[0].data,
-          base: values[1].data,
-        });
-      })
-      .catch(() => {
-        dispatch('receiveSastError');
-      });
-  } else if (head && !base) {
-    axios
-      .get(head)
-      .then(response => {
-        dispatch('receiveSastReports', {
-          head: response.data,
+      head: values[0] ? values[0].data : null,
+      base: values[1] ? values[1].data : null,
     });
   })
   .catch(() => {
     dispatch('receiveSastError');
   });
-  }
 };
 
 /**
@@ -77,29 +65,19 @@ export const fetchSastContainerReports = ({ state, dispatch }) => {
 
   dispatch('requestSastContainerReports');
 
-  if (head && base) {
-    Promise.all([axios.get(head), axios.get(base)])
+  Promise.all([
+    head ? axios.get(head) : Promise.resolve(),
+    base ? axios.get(base) : Promise.resolve(),
+  ])
   .then(values => {
     dispatch('receiveSastContainerReports', {
-          head: values[0].data,
-          base: values[1].data,
-        });
-      })
-      .catch(() => {
-        dispatch('receiveSastContainerError');
-      });
-  } else {
-    axios
-      .get(head)
-      .then(response => {
-        dispatch('receiveSastContainerReports', {
-          head: response.data,
+      head: values[0] ? values[0].data : null,
+      base: values[1] ? values[1].data : null,
     });
   })
   .catch(() => {
     dispatch('receiveSastContainerError');
   });
-  }
 };
 
 /**
@@ -122,29 +100,19 @@ export const fetchDastReports = ({ state, dispatch }) => {
 
   dispatch('requestDastReports');
 
-  if (head && base) {
-    Promise.all([axios.get(head), axios.get(base)])
+  Promise.all([
+    head ? axios.get(head) : Promise.resolve(),
+    base ? axios.get(base) : Promise.resolve(),
+  ])
   .then(values => {
     dispatch('receiveDastReports', {
-          head: values[0].data,
-          base: values[1].data,
+      head: values[0] ? values[0].data : null,
+      base: values[1] ? values[1].data : null,
     });
   })
   .catch(() => {
     dispatch('receiveDastError');
   });
-  } else {
-    axios
-      .get(head)
-      .then(response => {
-        dispatch('receiveDastReports', {
-          head: response.data,
-        });
-      })
-      .catch(() => {
-        dispatch('receiveDastError');
-      });
-  }
 };
 
 /**
@@ -171,27 +139,17 @@ export const fetchDependencyScanningReports = ({ state, dispatch }) => {
 
   dispatch('requestDependencyScanningReports');
 
-  if (head && base) {
-    Promise.all([axios.get(head), axios.get(base)])
+  Promise.all([
+    head ? axios.get(head) : Promise.resolve(),
+    base ? axios.get(base) : Promise.resolve(),
+  ])
   .then(values => {
     dispatch('receiveDependencyScanningReports', {
-          head: values[0].data,
-          base: values[1].data,
-        });
-      })
-      .catch(() => {
-        dispatch('receiveDependencyScanningError');
-      });
-  } else {
-    axios
-      .get(head)
-      .then(response => {
-        dispatch('receiveDependencyScanningReports', {
-          head: response.data,
+      head: values[0] ? values[0].data : null,
+      base: values[1] ? values[1].data : null,
     });
   })
   .catch(() => {
     dispatch('receiveDependencyScanningError');
   });
-  }
 };

ee/app/assets/javascripts/vue_shared/security_reports/store/getters.js

 import { n__, s__ } from '~/locale';
 import { textBuilder, statusIcon } from './utils';
 
-export const groupedSastText = state => {
-  const { sast } = state;
-  return textBuilder(
+export const groupedSastText = ({ sast }) =>
+  textBuilder(
     'SAST',
     sast.paths,
     sast.newIssues.length,
     sast.resolvedIssues.length,
     sast.allIssues.length,
   );
-};
-
-export const groupedSastContainerText = state => {
-  const { sastContainer } = state;
 
-  return textBuilder(
+export const groupedSastContainerText = ({ sastContainer }) =>
+  textBuilder(
     'Container scanning',
     sastContainer.paths,
     sastContainer.newIssues.length,
     sastContainer.resolvedIssues.length,
   );
-};
 
-export const groupedDastText = state => {
-  const { dast } = state;
-  return textBuilder('DAST', dast.paths, dast.newIssues.length, dast.resolvedIssues.length);
-};
+export const groupedDastText = ({ dast }) =>
+  textBuilder('DAST', dast.paths, dast.newIssues.length, dast.resolvedIssues.length);
 
-export const groupedDependencyText = state => {
-  const { dependencyScanning } = state;
-  return textBuilder(
+export const groupedDependencyText = ({ dependencyScanning }) =>
+  textBuilder(
     'Dependency scanning',
     dependencyScanning.paths,
     dependencyScanning.newIssues.length,
     dependencyScanning.resolvedIssues.length,
   );
-};
-
-export const groupedSummaryText = (state, getters) => {
-  const { added, fixed } = state.summaryCounts;
 
+export const groupedSummaryText = ({ added, fixed }, getters) => {
   // All reports returned error
   if (getters.allReportsHaveError) {
     return s__('ciReport|Security scanning failed loading any results');
@@ -91,15 +80,15 @@ export const groupedSummaryText = (state, getters) => {
   return text.join(' ');
 };
 
-export const sastStatusIcon = state => statusIcon(state.sast.hasError, state.sast.newIssues.length);
+export const sastStatusIcon = ({ sast }) => statusIcon(sast.hasError, sast.newIssues.length);
 
-export const sastContainerStatusIcon = state =>
-  statusIcon(state.sastContainer.hasError, state.sastContainer.newIssues.length);
+export const sastContainerStatusIcon = ({ sastContainer }) =>
+  statusIcon(sastContainer.hasError, sastContainer.newIssues.length);
 
-export const dastStatusIcon = state => statusIcon(state.dast.hasError, state.dast.newIssues.length);
+export const dastStatusIcon = ({ dast }) => statusIcon(dast.hasError, dast.newIssues.length);
 
-export const dependencyScanningStatusIcon = state =>
-  statusIcon(state.dependencyScanning.hasError, state.dependencyScanning.newIssues.length);
+export const dependencyScanningStatusIcon = ({ dependencyScanning }) =>
+  statusIcon(dependencyScanning.hasError, dependencyScanning.newIssues.length);
 
 export const areReportsLoading = state =>
   state.sast.isLoading ||

ee/app/assets/javascripts/vue_shared/security_reports/store/mutations.js

@@ -55,16 +55,18 @@ export default {
 
       const allIssues = filterByKey(parsedHead, newIssues.concat(resolvedIssues), filterKey);
 
-      Object.assign(state.sast, {
+      Object.assign(state, {
+        sast: {
+          ...state.sast,
           newIssues,
           resolvedIssues,
           allIssues,
           isLoading: false,
-      });
-
-      Object.assign(state.summaryCounts, {
+        },
+        summaryCounts: {
           added: state.summaryCounts.added + newIssues.length,
           fixed: state.summaryCounts.fixed + resolvedIssues.length,
+        },
       });
     } else if (reports.head && !reports.base) {
       const newIssues = parseSastIssues(reports.head, state.blobPath.head);
@@ -114,15 +116,17 @@ export default {
       const newIssues = filterByKey(headIssues, baseIssues, filterKey);
       const resolvedIssues = filterByKey(baseIssues, headIssues, filterKey);
 
-      Object.assign(state.sastContainer, {
+      Object.assign(state, {
+        sastContainer: {
+          ...state.sastContainer,
           isLoading: false,
           newIssues,
           resolvedIssues,
-      });
-
-      Object.assign(state.summaryCounts, {
+        },
+        summaryCounts: {
           added: state.summaryCounts.added + newIssues.length,
           fixed: state.summaryCounts.fixed + resolvedIssues.length,
+        },
       });
     } else if (reports.head && !reports.base) {
       Object.assign(state.sastContainer, {
@@ -164,15 +168,17 @@ export default {
       const newIssues = filterByKey(headIssues, baseIssues, filterKey);
       const resolvedIssues = filterByKey(baseIssues, headIssues, filterKey);
 
-      Object.assign(state.dast, {
+      Object.assign(state, {
+        dast: {
+          ...state.dast,
           isLoading: false,
           newIssues,
           resolvedIssues,
-      });
-
-      Object.assign(state.summaryCounts, {
+        },
+        summaryCounts: {
           added: state.summaryCounts.added + newIssues.length,
           fixed: state.summaryCounts.fixed + resolvedIssues.length,
+        },
       });
     } else if (reports.head && !reports.base) {
       Object.assign(state.dast, {
@@ -228,16 +234,18 @@ export default {
       const resolvedIssues = filterByKey(parsedBase, parsedHead, filterKey);
       const allIssues = filterByKey(parsedHead, newIssues.concat(resolvedIssues), filterKey);
 
-      Object.assign(state.dependencyScanning, {
+      Object.assign(state, {
+        dependencyScanning: {
+          ...state.dependencyScanning,
           newIssues,
           resolvedIssues,
           allIssues,
           isLoading: false,
-      });
-
-      Object.assign(state.summaryCounts, {
+        },
+        summaryCounts: {
           added: state.summaryCounts.added + newIssues.length,
           fixed: state.summaryCounts.fixed + resolvedIssues.length,
+        },
       });
     } else {
       Object.assign(state.dependencyScanning, {

ee/app/assets/javascripts/vue_shared/security_reports/store/utils.js

@@ -11,13 +11,13 @@ import { n__, s__, sprintf } from '~/locale';
  * @param {String} path
  */
 export const parseSastIssues = (issues = [], path = '') =>
-  issues.map(issue =>
-    Object.assign({}, issue, {
+  issues.map(issue => ({
+    ...issue,
     name: issue.message,
     path: issue.file,
     urlPath: issue.line ? `${path}/${issue.file}#L${issue.line}` : `${path}/${issue.file}`,
   }),
-  );
+);
 
 /**
  * Parses Sast Container results into a common format to allow to use the same Vue component
@@ -27,13 +27,13 @@ export const parseSastIssues = (issues = [], path = '') =>
  * @returns {Array}
  */
 export const parseSastContainer = (data = []) =>
-  data.map(el => ({
-    name: el.vulnerability,
-    priority: el.severity,
-    path: el.namespace,
+  data.map(element => ({
+    ...element,
+    name: element.vulnerability,
+    priority: element.severity,
+    path: element.namespace,
     // external link to provide better description
-    nameLink: `https://cve.mitre.org/cgi-bin/cvename.cgi?name=${el.vulnerability}`,
-    ...el,
+    nameLink: `https://cve.mitre.org/cgi-bin/cvename.cgi?name=${element.vulnerability}`,
   }));
 
 export const parseDastIssues = (issues = []) =>

spec/javascripts/helpers/vuex_action_helper.js

-/* eslint-disable */
-
 /**
- * helper for testing action with expected mutations
+ * helper for testing action with expected mutations inspired in
  * https://vuex.vuejs.org/en/testing.html
+ *
+ * @example
+ * testAction(
+ *   actions.actionName, // action
+ *   { }, // mocked response
+ *   state, // state
+ *   [
+ *    { type: types.MUTATION}
+ *    { type: types.MUTATION_1, payload: {}}
+ *   ], // mutations
+ *   [
+ *    { type: 'actionName', payload: {}},
+ *    { type: 'actionName1', payload: {}}
+ *   ] //actions
+ *   done,
+ * );
  */
-export default (action, payload, state, expectedMutations, done) => {
-  let count = 0;
+export default (action, payload, state, expectedMutations, expectedActions, done) => {
+  let mutationsCount = 0;
+  let actionsCount = 0;
 
   // mock commit
-  const commit = (type, payload) => {
-    const mutation = expectedMutations[count];
+  const commit = (type, mutationPayload) => {
+    const mutation = expectedMutations[mutationsCount];
 
-    try {
-      expect(mutation.type).to.equal(type);
-      if (payload) {
-        expect(mutation.payload).to.deep.equal(payload);
-      }
-    } catch (error) {
-      done(error);
+    expect(mutation.type).toEqual(type);
+
+    if (mutation.payload) {
+      expect(mutation.payload).toEqual(mutationPayload);
     }
 
-    count++;
-    if (count >= expectedMutations.length) {
+    mutationsCount += 1;
+    if (mutationsCount >= expectedMutations.length) {
       done();
     }
   };
 
   // mock dispatch
-  const dispatch = (type, dispatchPayload) => {
-    const mutation = expectedMutations[count];
+  const dispatch = (type, actionPayload) => {
+    const actionExpected = expectedActions[actionsCount];
 
-    try {
-      expect(mutation.type).to.equal(type);
+    expect(actionExpected.type).toEqual(type);
 
-      if (dispatchPayload) {
-        expect(mutation.payload).to.deep.equal(dispatchPayload);
+    if (actionExpected.payload) {
+      expect(actionExpected.payload).toEqual(actionPayload);
     }
-    } catch (error) {
-      done(error);
-    }
-
-    count++;
 
-    if (count >= expectedMutations.length) {
+    actionsCount += 1;
+    if (actionsCount >= expectedActions.length) {
       done();
     }
   };
@@ -52,7 +59,13 @@ export default (action, payload, state, expectedMutations, done) => {
 
   // check if no mutations should have been dispatched
   if (expectedMutations.length === 0) {
-    expect(count).to.equal(0);
+    expect(mutationsCount).toEqual(0);
+    done();
+  }
+
+  // check if no mutations should have been dispatched
+  if (expectedActions.length === 0) {
+    expect(actionsCount).toEqual(0);
     done();
   }
 };

spec/javascripts/vue_shared/security_reports/store/getters_spec.js

@@ -305,7 +305,7 @@ describe('Security reports getters', () => {
     });
 
     it('returns added and fixed text', () => {
-      const newState = Object.assign({}, state());
+      const newState = state();
       newState.summaryCounts = {
         added: 2,
         fixed: 4,
@@ -321,7 +321,7 @@ describe('Security reports getters', () => {
     });
 
     it('returns added text', () => {
-      const newState = Object.assign({}, state());
+      const newState = state();
       newState.summaryCounts = {
         added: 2,
         fixed: 0,