Revert "Merge branch '267612-codequality-badge-improvements' into 'master'"

This reverts merge request !58833

app/assets/javascripts/diffs/components/app.vue

@@ -184,7 +184,12 @@ export default {
       'viewDiffsFileByFile',
       'mrReviews',
     ]),
-    ...mapGetters('diffs', ['whichCollapsedTypes', 'isParallelView', 'currentDiffIndex']),
+    ...mapGetters('diffs', [
+      'whichCollapsedTypes',
+      'isParallelView',
+      'currentDiffIndex',
+      'fileCodequalityDiff',
+    ]),
     ...mapGetters(['isNotesFetched', 'getNoteableData']),
     diffs() {
       if (!this.viewDiffsFileByFile) {
@@ -282,6 +287,7 @@ export default {
       endpointMetadata: this.endpointMetadata,
       endpointBatch: this.endpointBatch,
       endpointCoverage: this.endpointCoverage,
+      endpointCodequality: this.endpointCodequality,
       endpointUpdateUser: this.endpointUpdateUser,
       projectPath: this.projectPath,
       dismissEndpoint: this.dismissEndpoint,
@@ -291,10 +297,6 @@ export default {
       mrReviews: this.rehydratedMrReviews,
     });
 
-    if (this.endpointCodequality) {
-      this.setCodequalityEndpoint(this.endpointCodequality);
-    }
-
     if (this.shouldShow) {
       this.fetchData();
     }
@@ -339,7 +341,6 @@ export default {
     ...mapActions('diffs', [
       'moveToNeighboringCommit',
       'setBaseConfig',
-      'setCodequalityEndpoint',
       'fetchDiffFilesMeta',
       'fetchDiffFilesBatch',
       'fetchCoverageFiles',
@@ -531,6 +532,7 @@ export default {
               :help-page-path="helpPagePath"
               :can-current-user-fork="canCurrentUserFork"
               :view-diffs-file-by-file="viewDiffsFileByFile"
+              :codequality-diff="fileCodequalityDiff(file.file_path)"
             />
             <div
               v-if="showFileByFileNavigation"

app/assets/javascripts/diffs/components/diff_file.vue

@@ -67,6 +67,11 @@ export default {
       type: Boolean,
       required: true,
     },
+    codequalityDiff: {
+      type: Array,
+      required: false,
+      default: () => [],
+    },
   },
   data() {
     return {
@@ -80,7 +85,7 @@ export default {
     genericError: GENERIC_ERROR,
   },
   computed: {
-    ...mapState('diffs', ['currentDiffFileId', 'codequalityDiff']),
+    ...mapState('diffs', ['currentDiffFileId']),
     ...mapGetters(['isNotesFetched']),
     ...mapGetters('diffs', ['getDiffFileDiscussions']),
     viewBlobHref() {
@@ -149,9 +154,7 @@ export default {
       return loggedIn && featureOn;
     },
     hasCodequalityChanges() {
-      return (
-        this.codequalityDiff?.files && this.codequalityDiff?.files[this.file.file_path]?.length > 0
-      );
+      return this.codequalityDiff.length > 0;
     },
   },
   watch: {

app/assets/javascripts/diffs/store/actions.js

 import Cookies from 'js-cookie';
+import Visibility from 'visibilityjs';
 import Vue from 'vue';
 import { deprecatedCreateFlash as createFlash } from '~/flash';
 import { diffViewerModes } from '~/ide/constants';
@@ -52,12 +53,15 @@ import {
   prepareLineForRenamedFile,
 } from './utils';
 
+let eTagPoll;
+
 export const setBaseConfig = ({ commit }, options) => {
   const {
     endpoint,
     endpointMetadata,
     endpointBatch,
     endpointCoverage,
+    endpointCodequality,
     endpointUpdateUser,
     projectPath,
     dismissEndpoint,
@@ -71,6 +75,7 @@ export const setBaseConfig = ({ commit }, options) => {
     endpointMetadata,
     endpointBatch,
     endpointCoverage,
+    endpointCodequality,
     endpointUpdateUser,
     projectPath,
     dismissEndpoint,
@@ -233,6 +238,48 @@ export const fetchCoverageFiles = ({ commit, state }) => {
   coveragePoll.makeRequest();
 };
 
+export const clearEtagPoll = () => {
+  eTagPoll = null;
+};
+
+export const stopCodequalityPolling = () => {
+  if (eTagPoll) eTagPoll.stop();
+};
+
+export const restartCodequalityPolling = () => {
+  if (eTagPoll) eTagPoll.restart();
+};
+
+export const fetchCodequality = ({ commit, state, dispatch }) => {
+  eTagPoll = new Poll({
+    resource: {
+      getCodequalityDiffReports: (endpoint) => axios.get(endpoint),
+    },
+    data: state.endpointCodequality,
+    method: 'getCodequalityDiffReports',
+    successCallback: ({ status, data }) => {
+      if (status === httpStatusCodes.OK) {
+        commit(types.SET_CODEQUALITY_DATA, data);
+
+        eTagPoll.stop();
+      }
+    },
+    errorCallback: () => createFlash(__('Something went wrong on our end. Please try again!')),
+  });
+
+  if (!Visibility.hidden()) {
+    eTagPoll.makeRequest();
+  }
+
+  Visibility.change(() => {
+    if (!Visibility.hidden()) {
+      dispatch('restartCodequalityPolling');
+    } else {
+      dispatch('stopCodequalityPolling');
+    }
+  });
+};
+
 export const setHighlightedRow = ({ commit }, lineCode) => {
   const fileHash = lineCode.split('_')[0];
   commit(types.SET_HIGHLIGHTED_ROW, lineCode);

app/assets/javascripts/diffs/store/getters.js

@@ -135,6 +135,16 @@ export const fileLineCoverage = (state) => (file, line) => {
   return {};
 };
 
+/**
+ * Returns the codequality diff data for a given file
+ * @param {string} filePath
+ * @returns {Array}
+ */
+export const fileCodequalityDiff = (state) => (filePath) => {
+  if (!state.codequalityDiff.files || !state.codequalityDiff.files[filePath]) return [];
+  return state.codequalityDiff.files[filePath];
+};
+
 /**
  * Returns index of a currently selected diff in diffFiles
  * @returns {number}

app/assets/javascripts/diffs/store/modules/diff_state.js

@@ -29,6 +29,7 @@ export default () => ({
   startVersion: null, // Null unless a target diff is selected for comparison that is not the "base" diff
   diffFiles: [],
   coverageFiles: {},
+  codequalityDiff: {},
   mergeRequestDiffs: [],
   mergeRequestDiff: null,
   diffViewType: viewTypeFromQueryString || viewTypeFromCookie || defaultViewType,

app/assets/javascripts/diffs/store/modules/index.js

-import * as actions from 'ee_else_ce/diffs/store/actions';
-import createState from 'ee_else_ce/diffs/store/modules/diff_state';
-import mutations from 'ee_else_ce/diffs/store/mutations';
+import * as actions from '../actions';
 import * as getters from '../getters';
+import mutations from '../mutations';
+import createState from './diff_state';
 
 export default () => ({
   namespaced: true,

app/assets/javascripts/diffs/store/mutation_types.js

@@ -11,6 +11,7 @@ export const SET_MR_FILE_REVIEWS = 'SET_MR_FILE_REVIEWS';
 
 export const SET_DIFF_VIEW_TYPE = 'SET_DIFF_VIEW_TYPE';
 export const SET_COVERAGE_DATA = 'SET_COVERAGE_DATA';
+export const SET_CODEQUALITY_DATA = 'SET_CODEQUALITY_DATA';
 export const SET_MERGE_REQUEST_DIFFS = 'SET_MERGE_REQUEST_DIFFS';
 export const TOGGLE_LINE_HAS_FORM = 'TOGGLE_LINE_HAS_FORM';
 export const ADD_CONTEXT_LINES = 'ADD_CONTEXT_LINES';

app/assets/javascripts/diffs/store/mutations.js

@@ -33,6 +33,7 @@ export default {
       endpointMetadata,
       endpointBatch,
       endpointCoverage,
+      endpointCodequality,
       endpointUpdateUser,
       projectPath,
       dismissEndpoint,
@@ -46,6 +47,7 @@ export default {
       endpointMetadata,
       endpointBatch,
       endpointCoverage,
+      endpointCodequality,
       endpointUpdateUser,
       projectPath,
       dismissEndpoint,
@@ -89,6 +91,10 @@ export default {
     Object.assign(state, { coverageFiles });
   },
 
+  [types.SET_CODEQUALITY_DATA](state, codequalityDiffData) {
+    Object.assign(state, { codequalityDiff: codequalityDiffData });
+  },
+
   [types.RENDER_FILE](state, file) {
     renderFile(file);
   },

ee/app/assets/javascripts/diffs/components/code_quality_badge.vue

@@ -12,7 +12,7 @@ export default {
   i18n: {
     badgeTitle: __('Code Quality'),
     badgeTooltip: __(
-      'The merge request has made changes to this file that affect the number of code quality violations in it.',
+      'The merge request has been updated, and the number of code quality violations in this file has changed.',
     ),
   },
 };

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

-import Visibility from 'visibilityjs';
-import createFlash from '~/flash';
-import axios from '~/lib/utils/axios_utils';
-import httpStatusCodes from '~/lib/utils/http_status';
-import Poll from '~/lib/utils/poll';
-import { __ } from '~/locale';
-
-import * as types from './mutation_types';
-
-export * from '~/diffs/store/actions';
-
-let codequalityPoll;
-
-export const setCodequalityEndpoint = ({ commit }, endpoint) => {
-  commit(types.SET_CODEQUALITY_ENDPOINT, endpoint);
-};
-
-export const clearCodequalityPoll = () => {
-  codequalityPoll = null;
-};
-
-export const stopCodequalityPolling = () => {
-  if (codequalityPoll) codequalityPoll.stop();
-};
-
-export const restartCodequalityPolling = () => {
-  if (codequalityPoll) codequalityPoll.restart();
-};
-
-export const fetchCodequality = ({ commit, state, dispatch }) => {
-  codequalityPoll = new Poll({
-    resource: {
-      getCodequalityDiffReports: (endpoint) => axios.get(endpoint),
-    },
-    data: state.endpointCodequality,
-    method: 'getCodequalityDiffReports',
-    successCallback: ({ status, data }) => {
-      if (status === httpStatusCodes.OK) {
-        commit(types.SET_CODEQUALITY_DATA, data);
-
-        dispatch('stopCodequalityPolling');
-      }
-    },
-    errorCallback: () =>
-      createFlash(__('Something went wrong on our end while loading the code quality diff.')),
-  });
-
-  if (!Visibility.hidden()) {
-    codequalityPoll.makeRequest();
-  }
-
-  Visibility.change(() => {
-    if (!Visibility.hidden()) {
-      dispatch('restartCodequalityPolling');
-    } else {
-      dispatch('stopCodequalityPolling');
-    }
-  });
-};

ee/app/assets/javascripts/diffs/store/modules/diff_state.js

-import createStateCE from '~/diffs/store/modules/diff_state';
-
-export default () => ({
-  ...createStateCE(),
-
-  endpointCodequality: '',
-  codequalityDiff: {},
-});

ee/app/assets/javascripts/diffs/store/mutation_types.js

-export const SET_CODEQUALITY_ENDPOINT = 'SET_CODEQUALITY_ENDPOINT';
-export const SET_CODEQUALITY_DATA = 'SET_CODEQUALITY_DATA';

ee/app/assets/javascripts/diffs/store/mutations.js

-import CEMutations from '~/diffs/store/mutations';
-
-import * as types from './mutation_types';
-
-export default {
-  ...CEMutations,
-
-  [types.SET_CODEQUALITY_ENDPOINT](state, endpoint) {
-    Object.assign(state, { endpointCodequality: endpoint });
-  },
-
-  [types.SET_CODEQUALITY_DATA](state, codequalityDiffData) {
-    Object.assign(state, { codequalityDiff: codequalityDiffData });
-  },
-};

ee/spec/frontend/diffs/components/diff_file_spec.js

-import { mount, createLocalVue } from '@vue/test-utils';
+import { shallowMount, createLocalVue } from '@vue/test-utils';
 import Vuex from 'vuex';
 
 import CodeQualityBadge from 'ee/diffs/components/code_quality_badge.vue';
@@ -9,7 +9,7 @@ import createDiffsStore from '~/diffs/store/modules';
 
 const getReadableFile = () => JSON.parse(JSON.stringify(diffFileMockDataReadable));
 
-function createComponent({ withCodequality = true }) {
+function createComponent({ first = false, last = false, options = {}, props = {} }) {
   const file = getReadableFile();
   const localVue = createLocalVue();
 
@@ -23,31 +23,18 @@ function createComponent({ withCodequality = true }) {
 
   store.state.diffs.diffFiles = [file];
 
-  if (withCodequality) {
-    store.state.diffs.codequalityDiff = {
-      files: {
-        [file.file_path]: [
-          { line: 1, description: 'Unexpected alert.', severity: 'minor' },
-          {
-            line: 3,
-            description: 'Arrow function has too many statements (52). Maximum allowed is 30.',
-            severity: 'minor',
-          },
-        ],
-      },
-    };
-  }
-
-  const wrapper = mount(DiffFileComponent, {
+  const wrapper = shallowMount(DiffFileComponent, {
     store,
     localVue,
     propsData: {
       file,
       canCurrentUserFork: false,
       viewDiffsFileByFile: false,
-      isFirstFile: false,
-      isLastFile: false,
+      isFirstFile: first,
+      isLastFile: last,
+      ...props,
     },
+    ...options,
   });
 
   return {
@@ -65,24 +52,27 @@ describe('EE DiffFile', () => {
   });
 
   describe('code quality badge', () => {
-    describe('when there is diff data for the file', () => {
-      beforeEach(() => {
-        ({ wrapper } = createComponent({ withCodequality: true }));
-      });
+    it('is shown when there is diff data for the file', () => {
+      ({ wrapper } = createComponent({
+        props: {
+          codequalityDiff: [
+            { line: 1, description: 'Unexpected alert.', severity: 'minor' },
+            {
+              line: 3,
+              description: 'Arrow function has too many statements (52). Maximum allowed is 30.',
+              severity: 'minor',
+            },
+          ],
+        },
+      }));
 
-      it('shows the code quality badge', () => {
-        expect(wrapper.find(CodeQualityBadge).exists()).toBe(true);
-      });
+      expect(wrapper.find(CodeQualityBadge)).toExist();
     });
 
-    describe('when there is no diff data for the file', () => {
-      beforeEach(() => {
-        ({ wrapper } = createComponent({ withCodequality: false }));
-      });
+    it('is not shown when there is no diff data for the file', () => {
+      ({ wrapper } = createComponent({}));
 
-      it('does not show the code quality badge', () => {
-        expect(wrapper.find(CodeQualityBadge).exists()).toBe(false);
-      });
+      expect(wrapper.find(CodeQualityBadge)).toExist();
     });
   });
 });

ee/spec/frontend/diffs/store/actions_spec.js

-import MockAdapter from 'axios-mock-adapter';
-import {
-  setCodequalityEndpoint,
-  clearCodequalityPoll,
-  stopCodequalityPolling,
-  fetchCodequality,
-} from 'ee/diffs/store/actions';
-import * as types from 'ee/diffs/store/mutation_types';
-import testAction from 'helpers/vuex_action_helper';
-import createFlash from '~/flash';
-import axios from '~/lib/utils/axios_utils';
-
-jest.mock('~/flash');
-
-describe('EE DiffsStoreActions', () => {
-  describe('setCodequalityEndpoint', () => {
-    it('should set given endpoint', (done) => {
-      const endpoint = '/codequality_mr_diff.json';
-
-      testAction(
-        setCodequalityEndpoint,
-        { endpoint },
-        {},
-        [{ type: types.SET_CODEQUALITY_ENDPOINT, payload: { endpoint } }],
-        [],
-        done,
-      );
-    });
-  });
-
-  describe('fetchCodequality', () => {
-    let mock;
-    const endpoint = '/codequality_mr_diff.json';
-
-    beforeEach(() => {
-      mock = new MockAdapter(axios);
-    });
-
-    afterEach(() => {
-      stopCodequalityPolling();
-      clearCodequalityPoll();
-    });
-
-    it('should commit SET_CODEQUALITY_DATA with received response and stop polling', (done) => {
-      const data = {
-        files: { 'app.js': [{ line: 1, description: 'Unexpected alert.', severity: 'minor' }] },
-      };
-
-      mock.onGet(endpoint).reply(200, { data });
-
-      testAction(
-        fetchCodequality,
-        {},
-        { endpointCodequality: endpoint },
-        [{ type: types.SET_CODEQUALITY_DATA, payload: { data } }],
-        [{ type: 'stopCodequalityPolling' }],
-        done,
-      );
-    });
-
-    it('should show flash on API error', (done) => {
-      mock.onGet(endpoint).reply(400);
-
-      testAction(fetchCodequality, {}, { endpoint }, [], [], () => {
-        expect(createFlash).toHaveBeenCalledTimes(1);
-        expect(createFlash).toHaveBeenCalledWith(expect.stringMatching('Something went wrong'));
-        done();
-      });
-    });
-  });
-});

ee/spec/frontend/diffs/store/mutations_spec.js

-import * as types from 'ee/diffs/store/mutation_types';
-import mutations from 'ee/diffs/store/mutations';
-
-describe('EE DiffsStoreMutations', () => {
-  describe('SET_CODEQUALITY_ENDPOINT', () => {
-    it('sets the endpoint into state', () => {
-      const state = {};
-      const endpoint = '/codequality_mr_diff.json';
-
-      mutations[types.SET_CODEQUALITY_ENDPOINT](state, endpoint);
-
-      expect(state.endpointCodequality).toEqual(endpoint);
-    });
-  });
-
-  describe('SET_CODEQUALITY_DATA', () => {
-    it('should set codequality data', () => {
-      const state = { codequalityDiff: {} };
-      const codequality = {
-        files: { 'app.js': [{ line: 1, description: 'Unexpected alert.', severity: 'minor' }] },
-      };
-
-      mutations[types.SET_CODEQUALITY_DATA](state, codequality);
-
-      expect(state.codequalityDiff).toEqual(codequality);
-    });
-  });
-});

locale/gitlab.pot

@@ -29044,9 +29044,6 @@ msgstr ""
 msgid "Something went wrong on our end"
 msgstr ""
 
-msgid "Something went wrong on our end while loading the code quality diff."
-msgstr ""
-
 msgid "Something went wrong on our end."
 msgstr ""
 
@@ -31018,7 +31015,7 @@ msgstr ""
 msgid "The merge request can now be merged."
 msgstr ""
 
-msgid "The merge request has made changes to this file that affect the number of code quality violations in it."
+msgid "The merge request has been updated, and the number of code quality violations in this file has changed."
 msgstr ""
 
 msgid "The metric must be one of %{metrics}."

spec/frontend/diffs/store/actions_spec.js

@@ -17,6 +17,9 @@ import {
   fetchDiffFilesBatch,
   fetchDiffFilesMeta,
   fetchCoverageFiles,
+  clearEtagPoll,
+  stopCodequalityPolling,
+  fetchCodequality,
   assignDiscussionsToDiff,
   removeDiscussionsFromDiff,
   startRenderDiffsQueue,
@@ -98,6 +101,7 @@ describe('DiffsStoreActions', () => {
       const endpointMetadata = '/diffs/set/endpoint/metadata';
       const endpointBatch = '/diffs/set/endpoint/batch';
       const endpointCoverage = '/diffs/set/coverage_reports';
+      const endpointCodequality = '/diffs/set/codequality_diff';
       const projectPath = '/root/project';
       const dismissEndpoint = '/-/user_callouts';
       const showSuggestPopover = false;
@@ -109,6 +113,7 @@ describe('DiffsStoreActions', () => {
           endpointBatch,
           endpointMetadata,
           endpointCoverage,
+          endpointCodequality,
           projectPath,
           dismissEndpoint,
           showSuggestPopover,
@@ -118,6 +123,7 @@ describe('DiffsStoreActions', () => {
           endpointBatch: '',
           endpointMetadata: '',
           endpointCoverage: '',
+          endpointCodequality: '',
           projectPath: '',
           dismissEndpoint: '',
           showSuggestPopover: true,
@@ -130,6 +136,7 @@ describe('DiffsStoreActions', () => {
               endpointMetadata,
               endpointBatch,
               endpointCoverage,
+              endpointCodequality,
               projectPath,
               dismissEndpoint,
               showSuggestPopover,
@@ -299,6 +306,47 @@ describe('DiffsStoreActions', () => {
     });
   });
 
+  describe('fetchCodequality', () => {
+    let mock;
+    const endpointCodequality = '/fetch';
+
+    beforeEach(() => {
+      mock = new MockAdapter(axios);
+    });
+
+    afterEach(() => {
+      stopCodequalityPolling();
+      clearEtagPoll();
+    });
+
+    it('should commit SET_CODEQUALITY_DATA with received response', (done) => {
+      const data = {
+        files: { 'app.js': [{ line: 1, description: 'Unexpected alert.', severity: 'minor' }] },
+      };
+
+      mock.onGet(endpointCodequality).reply(200, { data });
+
+      testAction(
+        fetchCodequality,
+        {},
+        { endpointCodequality },
+        [{ type: types.SET_CODEQUALITY_DATA, payload: { data } }],
+        [],
+        done,
+      );
+    });
+
+    it('should show flash on API error', (done) => {
+      mock.onGet(endpointCodequality).reply(400);
+
+      testAction(fetchCodequality, {}, { endpointCodequality }, [], [], () => {
+        expect(createFlash).toHaveBeenCalledTimes(1);
+        expect(createFlash).toHaveBeenCalledWith(expect.stringMatching('Something went wrong'));
+        done();
+      });
+    });
+  });
+
   describe('setHighlightedRow', () => {
     it('should mark currently selected diff and set lineHash and fileHash of highlightedRow', () => {
       testAction(setHighlightedRow, 'ABC_123', {}, [

spec/frontend/diffs/store/getters_spec.js

@@ -376,6 +376,26 @@ describe('Diffs Module Getters', () => {
     });
   });
 
+  describe('fileCodequalityDiff', () => {
+    beforeEach(() => {
+      Object.assign(localState.codequalityDiff, {
+        files: { 'app.js': [{ line: 1, description: 'Unexpected alert.', severity: 'minor' }] },
+      });
+    });
+
+    it('returns empty array when no codequality data is available', () => {
+      Object.assign(localState.codequalityDiff, {});
+
+      expect(getters.fileCodequalityDiff(localState)('test.js')).toEqual([]);
+    });
+
+    it('returns array when codequality data is available for given file', () => {
+      expect(getters.fileCodequalityDiff(localState)('app.js')).toEqual([
+        { line: 1, description: 'Unexpected alert.', severity: 'minor' },
+      ]);
+    });
+  });
+
   describe('suggestionCommitMessage', () => {
     let rootState;
 

spec/frontend/diffs/store/mutations_spec.js

@@ -115,6 +115,19 @@ describe('DiffsStoreMutations', () => {
     });
   });
 
+  describe('SET_CODEQUALITY_DATA', () => {
+    it('should set codequality data', () => {
+      const state = { codequalityDiff: {} };
+      const codequality = {
+        files: { 'app.js': [{ line: 1, description: 'Unexpected alert.', severity: 'minor' }] },
+      };
+
+      mutations[types.SET_CODEQUALITY_DATA](state, codequality);
+
+      expect(state.codequalityDiff).toEqual(codequality);
+    });
+  });
+
   describe('SET_DIFF_VIEW_TYPE', () => {
     it('should set diff view type properly', () => {
       const state = {};