Merge branch '271077-move-codequality-diff-out-of-frontend-step-2' into 'master'

Use backend code quality diff for MR widget when feature flag is enabled

See merge request gitlab-org/gitlab!52365

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

@@ -2,6 +2,7 @@
 import { mapState, mapActions, mapGetters } from 'vuex';
 import { componentNames } from '~/reports/components/issue_body';
 import { s__, sprintf } from '~/locale';
+import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 import ReportSection from '~/reports/components/report_section.vue';
 import createStore from './store';
 
@@ -11,6 +12,7 @@ export default {
   components: {
     ReportSection,
   },
+  mixins: [glFeatureFlagsMixin()],
   props: {
     headPath: {
       type: String,
@@ -30,6 +32,11 @@ export default {
       required: false,
       default: null,
     },
+    codequalityReportsPath: {
+      type: String,
+      required: false,
+      default: '',
+    },
     codequalityHelpPath: {
       type: String,
       required: true,
@@ -37,7 +44,7 @@ export default {
   },
   componentNames,
   computed: {
-    ...mapState(['newIssues', 'resolvedIssues']),
+    ...mapState(['newIssues', 'resolvedIssues', 'hasError', 'statusReason']),
     ...mapGetters([
       'hasCodequalityIssues',
       'codequalityStatus',
@@ -51,10 +58,11 @@ export default {
       headPath: this.headPath,
       baseBlobPath: this.baseBlobPath,
       headBlobPath: this.headBlobPath,
+      reportsPath: this.codequalityReportsPath,
       helpPath: this.codequalityHelpPath,
     });
 
-    this.fetchReports();
+    this.fetchReports(this.glFeatures.codequalityMrDiff);
   },
   methods: {
     ...mapActions(['fetchReports', 'setPaths']),
@@ -80,5 +88,7 @@ export default {
     :popover-options="codequalityPopover"
     :show-report-section-status-icon="false"
     class="js-codequality-widget mr-widget-border-top mr-report"
-  />
+  >
+    <template v-if="hasError" #sub-heading>{{ statusReason }}</template>
+  </report-section>
 </template>

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

@@ -4,9 +4,20 @@ import { parseCodeclimateMetrics, doCodeClimateComparison } from './utils/codequ
 
 export const setPaths = ({ commit }, paths) => commit(types.SET_PATHS, paths);
 
-export const fetchReports = ({ state, dispatch, commit }) => {
+export const fetchReports = ({ state, dispatch, commit }, diffFeatureFlagEnabled) => {
   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');
   }
@@ -18,13 +29,13 @@ export const fetchReports = ({ state, dispatch, commit }) => {
       ),
     )
     .then((data) => dispatch('receiveReportsSuccess', data))
-    .catch(() => dispatch('receiveReportsError'));
+    .catch((error) => dispatch('receiveReportsError', error));
 };
 
 export const receiveReportsSuccess = ({ commit }, data) => {
   commit(types.RECEIVE_REPORTS_SUCCESS, data);
 };
 
-export const receiveReportsError = ({ commit }) => {
-  commit(types.RECEIVE_REPORTS_ERROR);
+export const receiveReportsError = ({ commit }, error) => {
+  commit(types.RECEIVE_REPORTS_ERROR, error);
 };

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

@@ -6,6 +6,7 @@ export default {
     state.headPath = paths.headPath;
     state.baseBlobPath = paths.baseBlobPath;
     state.headBlobPath = paths.headBlobPath;
+    state.reportsPath = paths.reportsPath;
     state.helpPath = paths.helpPath;
   },
   [types.REQUEST_REPORTS](state) {
@@ -13,12 +14,14 @@ export default {
   },
   [types.RECEIVE_REPORTS_SUCCESS](state, data) {
     state.hasError = false;
+    state.statusReason = '';
     state.isLoading = false;
     state.newIssues = data.newIssues;
     state.resolvedIssues = data.resolvedIssues;
   },
-  [types.RECEIVE_REPORTS_ERROR](state) {
+  [types.RECEIVE_REPORTS_ERROR](state, error) {
     state.isLoading = false;
     state.hasError = true;
+    state.statusReason = error?.response?.data?.status_reason;
   },
 };

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

 export default () => ({
   basePath: null,
   headPath: null,
+  reportsPath: null,
 
   baseBlobPath: null,
   headBlobPath: null,
 
   isLoading: false,
   hasError: false,
+  statusReason: '',
 
   newIssues: [],
   resolvedIssues: [],

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

@@ -3,8 +3,10 @@ import CodeQualityComparisonWorker from '../../workers/codequality_comparison_wo
 export const parseCodeclimateMetrics = (issues = [], path = '') => {
   return issues.map((issue) => {
     const parsedIssue = {
-      ...issue,
       name: issue.description,
+      path: issue.file_path,
+      urlPath: `${path}/${issue.file_path}#L${issue.line}`,
+      ...issue,
     };
 
     if (issue?.location?.path) {

app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue

@@ -464,6 +464,7 @@ export default {
         :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"
       />
 

app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js

@@ -241,10 +241,11 @@ export default class MergeRequestStore {
     this.isDismissedSuggestPipeline = data.is_dismissed_suggest_pipeline;
     this.securityReportsDocsPath = data.security_reports_docs_path;
 
-    // codeclimate
+    // code quality
     const blobPath = data.blob_path || {};
     this.headBlobPath = blobPath.head_path || '';
     this.baseBlobPath = blobPath.base_path || '';
+    this.codequalityReportsPath = data.codequality_reports_path;
     this.codequalityHelpPath = data.codequality_help_path;
     this.codeclimate = data.codeclimate;
 

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

@@ -278,6 +278,7 @@ export default {
         :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"
       />
       <grouped-browser-performance-reports-app

spec/frontend/reports/codequality_report/mock_data.js

@@ -88,3 +88,53 @@ export const issueDiff = [
     urlPath: 'headPath/lib/six.rb#L6',
   },
 ];
+
+export const reportIssues = {
+  status: 'failed',
+  new_errors: [
+    {
+      description:
+        'Method `long_if` has a Cognitive Complexity of 10 (exceeds 5 allowed). Consider refactoring.',
+      severity: 'minor',
+      file_path: 'codequality.rb',
+      line: 5,
+    },
+  ],
+  resolved_errors: [
+    {
+      description: 'Insecure Dependency',
+      severity: 'major',
+      file_path: 'lib/six.rb',
+      line: 22,
+    },
+  ],
+  existing_errors: [],
+  summary: { total: 3, resolved: 0, errored: 3 },
+};
+
+export const parsedReportIssues = {
+  newIssues: [
+    {
+      description:
+        'Method `long_if` has a Cognitive Complexity of 10 (exceeds 5 allowed). Consider refactoring.',
+      file_path: 'codequality.rb',
+      line: 5,
+      name:
+        'Method `long_if` has a Cognitive Complexity of 10 (exceeds 5 allowed). Consider refactoring.',
+      path: 'codequality.rb',
+      severity: 'minor',
+      urlPath: 'null/codequality.rb#L5',
+    },
+  ],
+  resolvedIssues: [
+    {
+      description: 'Insecure Dependency',
+      file_path: 'lib/six.rb',
+      line: 22,
+      name: 'Insecure Dependency',
+      path: 'lib/six.rb',
+      severity: 'major',
+      urlPath: 'null/lib/six.rb#L22',
+    },
+  ],
+};

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

@@ -5,7 +5,14 @@ import axios from '~/lib/utils/axios_utils';
 import * as actions from '~/reports/codequality_report/store/actions';
 import * as types from '~/reports/codequality_report/store/mutation_types';
 import createStore from '~/reports/codequality_report/store';
-import { headIssues, baseIssues, mockParsedHeadIssues, mockParsedBaseIssues } from '../mock_data';
+import {
+  headIssues,
+  baseIssues,
+  mockParsedHeadIssues,
+  mockParsedBaseIssues,
+  reportIssues,
+  parsedReportIssues,
+} from '../mock_data';
 
 // mock codequality comparison worker
 jest.mock('~/reports/codequality_report/workers/codequality_comparison_worker', () =>
@@ -39,6 +46,7 @@ describe('Codequality Reports actions', () => {
         headPath: 'headPath',
         baseBlobPath: 'baseBlobPath',
         headBlobPath: 'headBlobPath',
+        reportsPath: 'reportsPath',
         helpPath: 'codequalityHelpPath',
       };
 
@@ -55,68 +63,119 @@ describe('Codequality Reports actions', () => {
 
   describe('fetchReports', () => {
     let mock;
+    let diffFeatureFlagEnabled;
 
-    beforeEach(() => {
-      localState.headPath = `${TEST_HOST}/head.json`;
-      localState.basePath = `${TEST_HOST}/base.json`;
-      mock = new MockAdapter(axios);
-    });
+    describe('with codequalityMrDiff feature flag enabled', () => {
+      beforeEach(() => {
+        diffFeatureFlagEnabled = true;
+        localState.reportsPath = `${TEST_HOST}/codequality_reports.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,
-          null,
-          localState,
-          [{ type: types.REQUEST_REPORTS }],
-          [
-            {
-              payload: {
-                newIssues: [mockParsedHeadIssues[0]],
-                resolvedIssues: [mockParsedBaseIssues[0]],
+      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',
               },
-              type: 'receiveReportsSuccess',
-            },
-          ],
-          done,
-        );
+            ],
+            done,
+          );
+        });
       });
-    });
 
-    describe('on error', () => {
-      it('commits REQUEST_REPORTS and dispatches receiveReportsError', (done) => {
-        mock.onGet(`${TEST_HOST}/head.json`).reply(500);
-
-        testAction(
-          actions.fetchReports,
-          null,
-          localState,
-          [{ type: types.REQUEST_REPORTS }],
-          [{ type: 'receiveReportsError' }],
-          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,
+          );
+        });
       });
     });
 
-    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,
-        );
+    describe('with codequalityMrDiff 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();
+      });
+
+      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 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('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,
+          );
+        });
       });
     });
   });
@@ -142,7 +201,7 @@ describe('Codequality Reports actions', () => {
         actions.receiveReportsError,
         null,
         localState,
-        [{ type: types.RECEIVE_REPORTS_ERROR }],
+        [{ type: types.RECEIVE_REPORTS_ERROR, payload: null }],
         [],
         done,
       );

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

@@ -55,6 +55,12 @@ describe('Codequality Reports mutations', () => {
       expect(localState.hasError).toEqual(false);
     });
 
+    it('clears statusReason', () => {
+      mutations.RECEIVE_REPORTS_SUCCESS(localState, {});
+
+      expect(localState.statusReason).toEqual('');
+    });
+
     it('sets newIssues and resolvedIssues from response data', () => {
       const data = { newIssues: [{ id: 1 }], resolvedIssues: [{ id: 2 }] };
       mutations.RECEIVE_REPORTS_SUCCESS(localState, data);
@@ -76,5 +82,13 @@ describe('Codequality Reports mutations', () => {
 
       expect(localState.hasError).toEqual(true);
     });
+
+    it('sets statusReason to string from error response data', () => {
+      const data = { status_reason: 'This merge request does not have codequality reports' };
+      const error = { response: { data } };
+      mutations.RECEIVE_REPORTS_ERROR(localState, error);
+
+      expect(localState.statusReason).toEqual(data.status_reason);
+    });
   });
 });

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

@@ -2,7 +2,13 @@ import {
   parseCodeclimateMetrics,
   doCodeClimateComparison,
 } from '~/reports/codequality_report/store/utils/codequality_comparison';
-import { baseIssues, mockParsedHeadIssues, mockParsedBaseIssues } from '../../mock_data';
+import {
+  baseIssues,
+  mockParsedHeadIssues,
+  mockParsedBaseIssues,
+  reportIssues,
+  parsedReportIssues,
+} from '../../mock_data';
 
 jest.mock('~/reports/codequality_report/workers/codequality_comparison_worker', () => {
   let mockPostMessageCallback;
@@ -34,7 +40,7 @@ describe('Codequality report store utils', () => {
   let result;
 
   describe('parseCodeclimateMetrics', () => {
-    it('should parse the received issues', () => {
+    it('should parse the issues from codeclimate artifacts', () => {
       [result] = parseCodeclimateMetrics(baseIssues, 'path');
 
       expect(result.name).toEqual(baseIssues[0].check_name);
@@ -42,6 +48,14 @@ describe('Codequality report store utils', () => {
       expect(result.line).toEqual(baseIssues[0].location.lines.begin);
     });
 
+    it('should parse the issues from backend codequality diff', () => {
+      [result] = parseCodeclimateMetrics(reportIssues.new_errors, 'path');
+
+      expect(result.name).toEqual(parsedReportIssues.newIssues[0].name);
+      expect(result.path).toEqual(parsedReportIssues.newIssues[0].path);
+      expect(result.line).toEqual(parsedReportIssues.newIssues[0].line);
+    });
+
     describe('when an issue has no location or path', () => {
       const issue = { description: 'Insecure Dependency' };