Merge branch 'frontend-code-quality-cleanup' into 'master'

Remove frontend code quality comparison

See merge request gitlab-org/gitlab!55708

app/assets/javascripts/reports/codequality_report/grouped_codequality_reports_app.vue

@@ -3,7 +3,6 @@ import { mapState, mapActions, mapGetters } from 'vuex';
 import { s__, sprintf } from '~/locale';
 import { componentNames } from '~/reports/components/issue_body';
 import ReportSection from '~/reports/components/report_section.vue';
-import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 import createStore from './store';
 
 export default {
@@ -12,26 +11,12 @@ export default {
   components: {
     ReportSection,
   },
-  mixins: [glFeatureFlagsMixin()],
   props: {
-    headPath: {
-      type: String,
-      required: true,
-    },
-    headBlobPath: {
-      type: String,
-      required: true,
-    },
     basePath: {
       type: String,
       required: false,
       default: null,
     },
-    baseBlobPath: {
-      type: String,
-      required: false,
-      default: null,
-    },
     codequalityReportsPath: {
       type: String,
       required: false,
@@ -55,9 +40,6 @@ export default {
   created() {
     this.setPaths({
       basePath: this.basePath,
-      headPath: this.headPath,
-      baseBlobPath: this.baseBlobPath,
-      headBlobPath: this.headBlobPath,
       reportsPath: this.codequalityReportsPath,
       helpPath: this.codequalityHelpPath,
     });

app/assets/javascripts/reports/codequality_report/store/actions.js

 import axios from '~/lib/utils/axios_utils';
 import * as types from './mutation_types';
-import { parseCodeclimateMetrics, doCodeClimateComparison } from './utils/codequality_comparison';
+import { parseCodeclimateMetrics } from './utils/codequality_parser';
 
 export const setPaths = ({ commit }, paths) => commit(types.SET_PATHS, paths);
 
-export const fetchReports = ({ state, dispatch, commit }, diffFeatureFlagEnabled) => {
+export const fetchReports = ({ state, dispatch, commit }) => {
   commit(types.REQUEST_REPORTS);
 
-  if (diffFeatureFlagEnabled) {
-    return axios
-      .get(state.reportsPath)
-      .then(({ data }) => {
-        return dispatch('receiveReportsSuccess', {
-          newIssues: parseCodeclimateMetrics(data.new_errors, state.headBlobPath),
-          resolvedIssues: parseCodeclimateMetrics(data.resolved_errors, state.baseBlobPath),
-        });
-      })
-      .catch((error) => dispatch('receiveReportsError', error));
-  }
   if (!state.basePath) {
     return dispatch('receiveReportsError');
   }
-  return Promise.all([axios.get(state.headPath), axios.get(state.basePath)])
-    .then((results) =>
-      doCodeClimateComparison(
-        parseCodeclimateMetrics(results[0].data, state.headBlobPath),
-        parseCodeclimateMetrics(results[1].data, state.baseBlobPath),
-      ),
-    )
-    .then((data) => dispatch('receiveReportsSuccess', data))
+  return axios
+    .get(state.reportsPath)
+    .then(({ data }) => {
+      return dispatch('receiveReportsSuccess', {
+        newIssues: parseCodeclimateMetrics(data.new_errors, state.headBlobPath),
+        resolvedIssues: parseCodeclimateMetrics(data.resolved_errors, state.baseBlobPath),
+      });
+    })
     .catch((error) => dispatch('receiveReportsError', error));
 };
 

app/assets/javascripts/reports/codequality_report/store/mutations.js

@@ -3,9 +3,6 @@ import * as types from './mutation_types';
 export default {
   [types.SET_PATHS](state, paths) {
     state.basePath = paths.basePath;
-    state.headPath = paths.headPath;
-    state.baseBlobPath = paths.baseBlobPath;
-    state.headBlobPath = paths.headBlobPath;
     state.reportsPath = paths.reportsPath;
     state.helpPath = paths.helpPath;
   },

app/assets/javascripts/reports/codequality_report/store/utils/codequality_comparison.js → app/assets/javascripts/reports/codequality_report/store/utils/codequality_parser.js

-import CodeQualityComparisonWorker from '../../workers/codequality_comparison_worker';
-
 export const parseCodeclimateMetrics = (issues = [], path = '') => {
   return issues.map((issue) => {
     const parsedIssue = {
@@ -27,17 +25,3 @@ export const parseCodeclimateMetrics = (issues = [], path = '') => {
     return parsedIssue;
   });
 };
-
-export const doCodeClimateComparison = (headIssues, baseIssues) => {
-  // Do these comparisons in worker threads to avoid blocking the main thread
-  return new Promise((resolve, reject) => {
-    const worker = new CodeQualityComparisonWorker();
-    worker.addEventListener('message', ({ data }) =>
-      data.newIssues && data.resolvedIssues ? resolve(data) : reject(data),
-    );
-    worker.postMessage({
-      headIssues,
-      baseIssues,
-    });
-  });
-};

app/assets/javascripts/reports/codequality_report/workers/codequality_comparison_worker.js

-import { differenceBy } from 'lodash';
-
-const KEY_TO_FILTER_BY = 'fingerprint';
-
-// eslint-disable-next-line no-restricted-globals
-self.addEventListener('message', (e) => {
-  const { data } = e;
-
-  if (data === undefined) {
-    return null;
-  }
-
-  const { headIssues, baseIssues } = data;
-
-  if (!headIssues || !baseIssues) {
-    // eslint-disable-next-line no-restricted-globals
-    return self.postMessage({});
-  }
-
-  // eslint-disable-next-line no-restricted-globals
-  self.postMessage({
-    newIssues: differenceBy(headIssues, baseIssues, KEY_TO_FILTER_BY),
-    resolvedIssues: differenceBy(baseIssues, headIssues, KEY_TO_FILTER_BY),
-  });
-
-  // eslint-disable-next-line no-restricted-globals
-  return self.close();
-});

app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue

@@ -460,9 +460,6 @@ export default {
       <grouped-codequality-reports-app
         v-if="shouldRenderCodeQuality"
         :base-path="mr.codeclimate.base_path"
-        :head-path="mr.codeclimate.head_path"
-        :head-blob-path="mr.headBlobPath"
-        :base-blob-path="mr.baseBlobPath"
         :codequality-reports-path="mr.codequalityReportsPath"
         :codequality-help-path="mr.codequalityHelpPath"
       />

ee/app/assets/javascripts/codequality_report/store/actions.js

@@ -3,7 +3,7 @@ import createFlash from '~/flash';
 import axios from '~/lib/utils/axios_utils';
 import { s__ } from '~/locale';
 
-import { parseCodeclimateMetrics } from '~/reports/codequality_report/store/utils/codequality_comparison';
+import { parseCodeclimateMetrics } from '~/reports/codequality_report/store/utils/codequality_parser';
 import { VIEW_EVENT_FEATURE_FLAG, VIEW_EVENT_NAME } from './constants';
 import * as types from './mutation_types';
 

ee/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue

@@ -279,9 +279,6 @@ export default {
       <grouped-codequality-reports-app
         v-if="shouldRenderCodeQuality"
         :base-path="mr.codeclimate.base_path"
-        :head-path="mr.codeclimate.head_path"
-        :head-blob-path="mr.headBlobPath"
-        :base-blob-path="mr.baseBlobPath"
         :codequality-reports-path="mr.codequalityReportsPath"
         :codequality-help-path="mr.codequalityHelpPath"
       />

spec/frontend/reports/codequality_report/grouped_codequality_reports_app_spec.js

@@ -3,7 +3,7 @@ import Vuex from 'vuex';
 import CodequalityIssueBody from '~/reports/codequality_report/components/codequality_issue_body.vue';
 import GroupedCodequalityReportsApp from '~/reports/codequality_report/grouped_codequality_reports_app.vue';
 import { getStoreConfig } from '~/reports/codequality_report/store';
-import { mockParsedHeadIssues, mockParsedBaseIssues } from './mock_data';
+import { parsedReportIssues } from './mock_data';
 
 const localVue = createLocalVue();
 localVue.use(Vuex);
@@ -80,7 +80,7 @@ describe('Grouped code quality reports app', () => {
     describe('with issues', () => {
       describe('with new issues', () => {
         beforeEach(() => {
-          mockStore.state.newIssues = [mockParsedHeadIssues[0]];
+          mockStore.state.newIssues = parsedReportIssues.newIssues;
           mockStore.state.resolvedIssues = [];
         });
 
@@ -89,14 +89,14 @@ describe('Grouped code quality reports app', () => {
         });
 
         it('renders custom codequality issue body', () => {
-          expect(findIssueBody().props('issue')).toEqual(mockParsedHeadIssues[0]);
+          expect(findIssueBody().props('issue')).toEqual(parsedReportIssues.newIssues[0]);
         });
       });
 
       describe('with resolved issues', () => {
         beforeEach(() => {
           mockStore.state.newIssues = [];
-          mockStore.state.resolvedIssues = [mockParsedBaseIssues[0]];
+          mockStore.state.resolvedIssues = parsedReportIssues.resolvedIssues;
         });
 
         it('renders summary text', () => {
@@ -104,14 +104,14 @@ describe('Grouped code quality reports app', () => {
         });
 
         it('renders custom codequality issue body', () => {
-          expect(findIssueBody().props('issue')).toEqual(mockParsedBaseIssues[0]);
+          expect(findIssueBody().props('issue')).toEqual(parsedReportIssues.resolvedIssues[0]);
         });
       });
 
       describe('with new and resolved issues', () => {
         beforeEach(() => {
-          mockStore.state.newIssues = [mockParsedHeadIssues[0]];
-          mockStore.state.resolvedIssues = [mockParsedBaseIssues[0]];
+          mockStore.state.newIssues = parsedReportIssues.newIssues;
+          mockStore.state.resolvedIssues = parsedReportIssues.resolvedIssues;
         });
 
         it('renders summary text', () => {
@@ -121,7 +121,7 @@ describe('Grouped code quality reports app', () => {
         });
 
         it('renders custom codequality issue body', () => {
-          expect(findIssueBody().props('issue')).toEqual(mockParsedHeadIssues[0]);
+          expect(findIssueBody().props('issue')).toEqual(parsedReportIssues.newIssues[0]);
         });
       });
     });

spec/frontend/reports/codequality_report/mock_data.js

-export const headIssues = [
-  {
-    check_name: 'Rubocop/Lint/UselessAssignment',
-    description: 'Insecure Dependency',
-    location: {
-      path: 'lib/six.rb',
-      lines: {
-        begin: 6,
-        end: 7,
-      },
-    },
-    fingerprint: 'e879dd9bbc0953cad5037cde7ff0f627',
-  },
-  {
-    categories: ['Security'],
-    check_name: 'Insecure Dependency',
-    description: 'Insecure Dependency',
-    location: {
-      path: 'Gemfile.lock',
-      lines: {
-        begin: 22,
-        end: 22,
-      },
-    },
-    fingerprint: 'ca2e59451e98ae60ba2f54e3857c50e5',
-  },
-];
-
-export const mockParsedHeadIssues = [
-  {
-    ...headIssues[1],
-    name: 'Insecure Dependency',
-    path: 'lib/six.rb',
-    urlPath: 'headPath/lib/six.rb#L6',
-    line: 6,
-  },
-];
-
-export const baseIssues = [
-  {
-    categories: ['Security'],
-    check_name: 'Insecure Dependency',
-    description: 'Insecure Dependency',
-    location: {
-      path: 'Gemfile.lock',
-      lines: {
-        begin: 22,
-        end: 22,
-      },
-    },
-    fingerprint: 'ca2e59451e98ae60ba2f54e3857c50e5',
-  },
-  {
-    categories: ['Security'],
-    check_name: 'Insecure Dependency',
-    description: 'Insecure Dependency',
-    location: {
-      path: 'Gemfile.lock',
-      lines: {
-        begin: 21,
-        end: 21,
-      },
-    },
-    fingerprint: 'ca2354534dee94ae60ba2f54e3857c50e5',
-  },
-];
-
-export const mockParsedBaseIssues = [
-  {
-    ...baseIssues[1],
-    name: 'Insecure Dependency',
-    path: 'Gemfile.lock',
-    line: 21,
-    urlPath: 'basePath/Gemfile.lock#L21',
-  },
-];
-
-export const issueDiff = [
-  {
-    categories: ['Security'],
-    check_name: 'Insecure Dependency',
-    description: 'Insecure Dependency',
-    fingerprint: 'ca2e59451e98ae60ba2f54e3857c50e5',
-    line: 6,
-    location: { lines: { begin: 22, end: 22 }, path: 'Gemfile.lock' },
-    name: 'Insecure Dependency',
-    path: 'lib/six.rb',
-    urlPath: 'headPath/lib/six.rb#L6',
-  },
-];
-
 export const reportIssues = {
   status: 'failed',
   new_errors: [

spec/frontend/reports/codequality_report/store/actions_spec.js

@@ -5,30 +5,7 @@ import axios from '~/lib/utils/axios_utils';
 import createStore from '~/reports/codequality_report/store';
 import * as actions from '~/reports/codequality_report/store/actions';
 import * as types from '~/reports/codequality_report/store/mutation_types';
-import {
-  headIssues,
-  baseIssues,
-  mockParsedHeadIssues,
-  mockParsedBaseIssues,
-  reportIssues,
-  parsedReportIssues,
-} from '../mock_data';
-
-// mock codequality comparison worker
-jest.mock('~/reports/codequality_report/workers/codequality_comparison_worker', () =>
-  jest.fn().mockImplementation(() => {
-    return {
-      addEventListener: (eventName, callback) => {
-        callback({
-          data: {
-            newIssues: [mockParsedHeadIssues[0]],
-            resolvedIssues: [mockParsedBaseIssues[0]],
-          },
-        });
-      },
-    };
-  }),
-);
+import { reportIssues, parsedReportIssues } from '../mock_data';
 
 describe('Codequality Reports actions', () => {
   let localState;
@@ -43,9 +20,6 @@ describe('Codequality Reports actions', () => {
     it('should commit SET_PATHS mutation', (done) => {
       const paths = {
         basePath: 'basePath',
-        headPath: 'headPath',
-        baseBlobPath: 'baseBlobPath',
-        headBlobPath: 'headBlobPath',
         reportsPath: 'reportsPath',
         helpPath: 'codequalityHelpPath',
       };
@@ -63,119 +37,64 @@ describe('Codequality Reports actions', () => {
 
   describe('fetchReports', () => {
     let mock;
-    let diffFeatureFlagEnabled;
 
-    describe('with codequalityBackendComparison feature flag enabled', () => {
-      beforeEach(() => {
-        diffFeatureFlagEnabled = true;
-        localState.reportsPath = `${TEST_HOST}/codequality_reports.json`;
-        mock = new MockAdapter(axios);
-      });
-
-      afterEach(() => {
-        mock.restore();
-      });
-
-      describe('on success', () => {
-        it('commits REQUEST_REPORTS and dispatches receiveReportsSuccess', (done) => {
-          mock.onGet(`${TEST_HOST}/codequality_reports.json`).reply(200, reportIssues);
-
-          testAction(
-            actions.fetchReports,
-            diffFeatureFlagEnabled,
-            localState,
-            [{ type: types.REQUEST_REPORTS }],
-            [
-              {
-                payload: parsedReportIssues,
-                type: 'receiveReportsSuccess',
-              },
-            ],
-            done,
-          );
-        });
-      });
-
-      describe('on error', () => {
-        it('commits REQUEST_REPORTS and dispatches receiveReportsError', (done) => {
-          mock.onGet(`${TEST_HOST}/codequality_reports.json`).reply(500);
-
-          testAction(
-            actions.fetchReports,
-            diffFeatureFlagEnabled,
-            localState,
-            [{ type: types.REQUEST_REPORTS }],
-            [{ type: 'receiveReportsError', payload: expect.any(Error) }],
-            done,
-          );
-        });
-      });
+    beforeEach(() => {
+      localState.reportsPath = `${TEST_HOST}/codequality_reports.json`;
+      localState.basePath = '/base/path';
+      mock = new MockAdapter(axios);
     });
 
-    describe('with codequalityBackendComparison feature flag disabled', () => {
-      beforeEach(() => {
-        diffFeatureFlagEnabled = false;
-        localState.headPath = `${TEST_HOST}/head.json`;
-        localState.basePath = `${TEST_HOST}/base.json`;
-        mock = new MockAdapter(axios);
-      });
-
-      afterEach(() => {
-        mock.restore();
-      });
+    afterEach(() => {
+      mock.restore();
+    });
 
-      describe('on success', () => {
-        it('commits REQUEST_REPORTS and dispatches receiveReportsSuccess', (done) => {
-          mock.onGet(`${TEST_HOST}/head.json`).reply(200, headIssues);
-          mock.onGet(`${TEST_HOST}/base.json`).reply(200, baseIssues);
-
-          testAction(
-            actions.fetchReports,
-            diffFeatureFlagEnabled,
-            localState,
-            [{ type: types.REQUEST_REPORTS }],
-            [
-              {
-                payload: {
-                  newIssues: [mockParsedHeadIssues[0]],
-                  resolvedIssues: [mockParsedBaseIssues[0]],
-                },
-                type: 'receiveReportsSuccess',
-              },
-            ],
-            done,
-          );
-        });
+    describe('on success', () => {
+      it('commits REQUEST_REPORTS and dispatches receiveReportsSuccess', (done) => {
+        mock.onGet(`${TEST_HOST}/codequality_reports.json`).reply(200, reportIssues);
+
+        testAction(
+          actions.fetchReports,
+          null,
+          localState,
+          [{ type: types.REQUEST_REPORTS }],
+          [
+            {
+              payload: parsedReportIssues,
+              type: 'receiveReportsSuccess',
+            },
+          ],
+          done,
+        );
       });
+    });
 
-      describe('on error', () => {
-        it('commits REQUEST_REPORTS and dispatches receiveReportsError', (done) => {
-          mock.onGet(`${TEST_HOST}/head.json`).reply(500);
-
-          testAction(
-            actions.fetchReports,
-            diffFeatureFlagEnabled,
-            localState,
-            [{ type: types.REQUEST_REPORTS }],
-            [{ type: 'receiveReportsError', payload: expect.any(Error) }],
-            done,
-          );
-        });
+    describe('on error', () => {
+      it('commits REQUEST_REPORTS and dispatches receiveReportsError', (done) => {
+        mock.onGet(`${TEST_HOST}/codequality_reports.json`).reply(500);
+
+        testAction(
+          actions.fetchReports,
+          null,
+          localState,
+          [{ type: types.REQUEST_REPORTS }],
+          [{ type: 'receiveReportsError', payload: expect.any(Error) }],
+          done,
+        );
       });
+    });
 
-      describe('with no base path', () => {
-        it('commits REQUEST_REPORTS and dispatches receiveReportsError', (done) => {
-          localState.basePath = null;
-
-          testAction(
-            actions.fetchReports,
-            diffFeatureFlagEnabled,
-            localState,
-            [{ type: types.REQUEST_REPORTS }],
-            [{ type: 'receiveReportsError' }],
-            done,
-          );
-        });
+    describe('with no base path', () => {
+      it('commits REQUEST_REPORTS and dispatches receiveReportsError', (done) => {
+        localState.basePath = null;
+
+        testAction(
+          actions.fetchReports,
+          null,
+          localState,
+          [{ type: types.REQUEST_REPORTS }],
+          [{ type: 'receiveReportsError' }],
+          done,
+        );
       });
     });
   });

spec/frontend/reports/codequality_report/store/mutations_spec.js

@@ -13,23 +13,17 @@ describe('Codequality Reports mutations', () => {
   describe('SET_PATHS', () => {
     it('sets paths to given values', () => {
       const basePath = 'base.json';
-      const headPath = 'head.json';
-      const baseBlobPath = 'base/blob/path/';
-      const headBlobPath = 'head/blob/path/';
+      const reportsPath = 'reports.json';
       const helpPath = 'help.html';
 
       mutations.SET_PATHS(localState, {
         basePath,
-        headPath,
-        baseBlobPath,
-        headBlobPath,
+        reportsPath,
         helpPath,
       });
 
       expect(localState.basePath).toEqual(basePath);
-      expect(localState.headPath).toEqual(headPath);
-      expect(localState.baseBlobPath).toEqual(baseBlobPath);
-      expect(localState.headBlobPath).toEqual(headBlobPath);
+      expect(localState.reportsPath).toEqual(reportsPath);
       expect(localState.helpPath).toEqual(helpPath);
     });
   });

spec/frontend/reports/codequality_report/store/utils/codequality_comparison_spec.js → spec/frontend/reports/codequality_report/store/utils/codequality_parser_spec.js

-import {
-  parseCodeclimateMetrics,
-  doCodeClimateComparison,
-} from '~/reports/codequality_report/store/utils/codequality_comparison';
-import {
-  baseIssues,
-  mockParsedHeadIssues,
-  mockParsedBaseIssues,
-  reportIssues,
-  parsedReportIssues,
-} from '../../mock_data';
-
-jest.mock('~/reports/codequality_report/workers/codequality_comparison_worker', () => {
-  let mockPostMessageCallback;
-  return jest.fn().mockImplementation(() => {
-    return {
-      addEventListener: (_, callback) => {
-        mockPostMessageCallback = callback;
-      },
-      postMessage: (data) => {
-        if (!data.headIssues) return mockPostMessageCallback({ data: {} });
-        if (!data.baseIssues) throw new Error();
-        const key = 'fingerprint';
-        return mockPostMessageCallback({
-          data: {
-            newIssues: data.headIssues.filter(
-              (item) => !data.baseIssues.find((el) => el[key] === item[key]),
-            ),
-            resolvedIssues: data.baseIssues.filter(
-              (item) => !data.headIssues.find((el) => el[key] === item[key]),
-            ),
-          },
-        });
-      },
-    };
-  });
-});
+import { reportIssues, parsedReportIssues } from 'jest/reports/codequality_report/mock_data';
+import { parseCodeclimateMetrics } from '~/reports/codequality_report/store/utils/codequality_parser';
 
 describe('Codequality report store utils', () => {
   let result;
 
   describe('parseCodeclimateMetrics', () => {
-    it('should parse the issues from codeclimate artifacts', () => {
-      [result] = parseCodeclimateMetrics(baseIssues, 'path');
-
-      expect(result.name).toEqual(baseIssues[0].check_name);
-      expect(result.path).toEqual(baseIssues[0].location.path);
-      expect(result.line).toEqual(baseIssues[0].location.lines.begin);
-    });
-
     it('should parse the issues from backend codequality diff', () => {
       [result] = parseCodeclimateMetrics(reportIssues.new_errors, 'path');
 
@@ -114,40 +71,4 @@ describe('Codequality report store utils', () => {
       });
     });
   });
-
-  describe('doCodeClimateComparison', () => {
-    describe('when the comparison worker finds changed issues', () => {
-      beforeEach(async () => {
-        result = await doCodeClimateComparison(mockParsedHeadIssues, mockParsedBaseIssues);
-      });
-
-      it('returns the new and resolved issues', () => {
-        expect(result.resolvedIssues[0]).toEqual(mockParsedBaseIssues[0]);
-        expect(result.newIssues[0]).toEqual(mockParsedHeadIssues[0]);
-      });
-    });
-
-    describe('when the comparison worker finds no changed issues', () => {
-      beforeEach(async () => {
-        result = await doCodeClimateComparison([], []);
-      });
-
-      it('returns the empty issue arrays', () => {
-        expect(result.newIssues).toEqual([]);
-        expect(result.resolvedIssues).toEqual([]);
-      });
-    });
-
-    describe('when the comparison worker is given malformed data', () => {
-      it('rejects the promise', () => {
-        return expect(doCodeClimateComparison(null)).rejects.toEqual({});
-      });
-    });
-
-    describe('when the comparison worker encounters an error', () => {
-      it('rejects the promise and throws an error', () => {
-        return expect(doCodeClimateComparison([], null)).rejects.toThrow();
-      });
-    });
-  });
 });