From 02f4d743b7add6a25ff267f173dd65d1fafa558b Mon Sep 17 00:00:00 2001 From: mfluharty Date: Fri, 8 Oct 2021 11:35:01 -0600 Subject: [PATCH 1/4] Undo the old approach Remove graphql versions of actions and mutations Remove specs and mock data for them --- .../codequality_report/codequality_report.vue | 40 ++--- .../codequality_report/store/actions.js | 75 ++------ .../codequality_report/store/getters.js | 10 +- .../store/mutation_types.js | 1 - .../codequality_report/store/mutations.js | 19 +- .../codequality_report/store/state.js | 10 -- .../codequality_report/store/utils.js | 3 - .../codequality_report_spec.js | 40 +---- .../frontend/codequality_report/mock_data.js | 50 ------ .../codequality_report/store/actions_spec.js | 164 ++---------------- .../store/mutations_spec.js | 16 +- 11 files changed, 49 insertions(+), 379 deletions(-) delete mode 100644 ee/app/assets/javascripts/codequality_report/store/utils.js diff --git a/ee/app/assets/javascripts/codequality_report/codequality_report.vue b/ee/app/assets/javascripts/codequality_report/codequality_report.vue index 3dbee2320b91d1..0f67ec88f5dbc4 100644 --- a/ee/app/assets/javascripts/codequality_report/codequality_report.vue +++ b/ee/app/assets/javascripts/codequality_report/codequality_report.vue @@ -1,43 +1,37 @@ + + diff --git a/ee/app/assets/javascripts/codequality_report/graphql/queries/get_code_quality_violations.query.graphql b/ee/app/assets/javascripts/codequality_report/graphql/queries/get_code_quality_violations.query.graphql index 86c0b30612b58d..35cd8bc0f47832 100644 --- a/ee/app/assets/javascripts/codequality_report/graphql/queries/get_code_quality_violations.query.graphql +++ b/ee/app/assets/javascripts/codequality_report/graphql/queries/get_code_quality_violations.query.graphql @@ -1,3 +1,5 @@ +#import "~/graphql_shared/fragments/pageInfo.fragment.graphql" + query getCodeQualityViolations($projectPath: ID!, $iid: ID!, $first: Int, $after: String) { project(fullPath: $projectPath) { pipeline(iid: $iid) { @@ -13,9 +15,7 @@ query getCodeQualityViolations($projectPath: ID!, $iid: ID!, $first: Int, $after } } pageInfo { - startCursor - endCursor - hasNextPage + ...PageInfo } } } diff --git a/ee/app/assets/javascripts/pages/projects/pipelines/show/codequality_report.js b/ee/app/assets/javascripts/pages/projects/pipelines/show/codequality_report.js index 604bde065589f6..5d6ea62d0bf2ec 100644 --- a/ee/app/assets/javascripts/pages/projects/pipelines/show/codequality_report.js +++ b/ee/app/assets/javascripts/pages/projects/pipelines/show/codequality_report.js @@ -1,13 +1,22 @@ import Vue from 'vue'; +import VueApollo from 'vue-apollo'; +import createDefaultClient from '~/lib/graphql'; import CodequalityReportApp from 'ee/codequality_report/codequality_report.vue'; +import CodequalityReportAppGraphQL from 'ee/codequality_report/codequality_report_graphql.vue'; import createStore from 'ee/codequality_report/store'; import Translate from '~/vue_shared/translate'; Vue.use(Translate); +Vue.use(VueApollo); + +const apolloProvider = new VueApollo({ + defaultClient: createDefaultClient(), +}); export default () => { const tabsElement = document.querySelector('.pipelines-tabs'); const codequalityTab = document.getElementById('js-pipeline-codequality-report'); + const isGraphqlFeatureFlagEnabled = gon.features?.graphqlCodeQualityFullReport; if (tabsElement && codequalityTab) { const fetchReportAction = 'fetchReport'; @@ -17,38 +26,68 @@ export default () => { projectPath, pipelineIid, } = codequalityTab.dataset; - const store = createStore({ - endpoint: codequalityReportDownloadPath, - blobPath, - projectPath, - pipelineIid, - }); - const isCodequalityTabActive = Boolean( document.querySelector('.pipelines-tabs > li > a.codequality-tab.active'), ); - if (isCodequalityTabActive) { - store.dispatch(fetchReportAction); - } else { - const tabClickHandler = (e) => { - if (e.target.className === 'codequality-tab') { - store.dispatch(fetchReportAction); - tabsElement.removeEventListener('click', tabClickHandler); - } + if (isGraphqlFeatureFlagEnabled) { + const vueOptions = { + el: codequalityTab, + apolloProvider, + components: { + CodequalityReportApp: CodequalityReportAppGraphQL, + }, + provide: { + projectPath, + pipelineIid, + blobPath, + }, + render: (createElement) => createElement('codequality-report-app'), }; - tabsElement.addEventListener('click', tabClickHandler); - } + if (isCodequalityTabActive) { + // eslint-disable-next-line no-new + new Vue(vueOptions); + } else { + const tabClickHandler = (e) => { + if (e.target.className === 'codequality-tab') { + // eslint-disable-next-line no-new + new Vue(vueOptions); + tabsElement.removeEventListener('click', tabClickHandler); + } + }; + tabsElement.addEventListener('click', tabClickHandler); + } + } else { + const store = createStore({ + endpoint: codequalityReportDownloadPath, + blobPath, + projectPath, + pipelineIid, + }); + + if (isCodequalityTabActive) { + store.dispatch(fetchReportAction); + } else { + const tabClickHandler = (e) => { + if (e.target.className === 'codequality-tab') { + store.dispatch(fetchReportAction); + tabsElement.removeEventListener('click', tabClickHandler); + } + }; - // eslint-disable-next-line no-new - new Vue({ - el: codequalityTab, - components: { - CodequalityReportApp, - }, - store, - render: (createElement) => createElement('codequality-report-app'), - }); + tabsElement.addEventListener('click', tabClickHandler); + } + + // eslint-disable-next-line no-new + new Vue({ + el: codequalityTab, + components: { + CodequalityReportApp, + }, + store, + render: (createElement) => createElement('codequality-report-app'), + }); + } } }; diff --git a/ee/spec/frontend/codequality_report/codequality_report_graphql_spec.js b/ee/spec/frontend/codequality_report/codequality_report_graphql_spec.js new file mode 100644 index 00000000000000..ef933b3648e908 --- /dev/null +++ b/ee/spec/frontend/codequality_report/codequality_report_graphql_spec.js @@ -0,0 +1,134 @@ +import { GlInfiniteScroll } from '@gitlab/ui'; +import { mount, createLocalVue } from '@vue/test-utils'; +import VueApollo from 'vue-apollo'; +import codeQualityViolationsQueryResponse from 'test_fixtures/graphql/codequality_report/graphql/queries/get_code_quality_violations.query.graphql.json'; +import waitForPromises from 'helpers/wait_for_promises'; +import createMockApollo from 'helpers/mock_apollo_helper'; +import CodequalityReportApp from 'ee/codequality_report/codequality_report_graphql.vue'; +import getCodeQualityViolations from 'ee/codequality_report/graphql/queries/get_code_quality_violations.query.graphql'; + +const codeQualityViolations = + codeQualityViolationsQueryResponse.data.project.pipeline.codeQualityReports; + +const localVue = createLocalVue(); +localVue.use(VueApollo); + +describe('Codequality report app', () => { + let wrapper; + + const createComponent = ( + mockReturnValue = jest.fn().mockResolvedValue(codeQualityViolationsQueryResponse), + mountFn = mount, + ) => { + const apolloProvider = createMockApollo([[getCodeQualityViolations, mockReturnValue]]); + + wrapper = mountFn(CodequalityReportApp, { + localVue, + apolloProvider, + provide: { + projectPath: 'project-path', + pipelineIid: 'pipeline-iid', + blobPath: '/blob/path', + }, + }); + }; + + const findStatus = () => wrapper.find('.js-code-text'); + const findSuccessIcon = () => wrapper.find('.js-ci-status-icon-success'); + const findWarningIcon = () => wrapper.find('.js-ci-status-icon-warning'); + const findInfiniteScroll = () => wrapper.find(GlInfiniteScroll); + + afterEach(() => { + wrapper.destroy(); + }); + + describe('when loading', () => { + beforeEach(() => { + createComponent(jest.fn().mockReturnValueOnce(new Promise(() => {}))); + }); + + it('shows a loading state', () => { + expect(findStatus().text()).toBe('Loading Code Quality report'); + }); + }); + + describe('on error', () => { + beforeEach(() => { + createComponent(jest.fn().mockRejectedValueOnce(new Error('Error!'))); + }); + + it('shows a warning icon and error message', () => { + expect(findWarningIcon().exists()).toBe(true); + expect(findStatus().text()).toBe('Failed to load Code Quality report'); + }); + }); + + describe('when there are codequality issues', () => { + beforeEach(() => { + createComponent(jest.fn().mockResolvedValue(codeQualityViolationsQueryResponse)); + }); + + it('renders the codequality issues', () => { + const expectedIssueTotal = codeQualityViolations.count; + + expect(findWarningIcon().exists()).toBe(true); + expect(findInfiniteScroll().exists()).toBe(true); + expect(findStatus().text()).toContain(`Found ${expectedIssueTotal} code quality issues`); + expect(findStatus().text()).toContain( + `This report contains all Code Quality issues in the source branch.`, + ); + expect(wrapper.findAll('.report-block-list-issue')).toHaveLength(expectedIssueTotal); + }); + + it('renders a link to the line where the issue was found', () => { + const issueLink = wrapper.find('.report-block-list-issue a'); + + expect(issueLink.text()).toBe('foo.rb:10'); + expect(issueLink.attributes('href')).toBe('/blob/path/foo.rb#L10'); + }); + + it('loads the next page when the end of the list is reached', async () => { + jest + .spyOn(wrapper.vm.$apollo.queries.codequalityViolations, 'fetchMore') + .mockResolvedValue({}); + + findInfiniteScroll().vm.$emit('bottomReached'); + + await waitForPromises(); + + expect(wrapper.vm.$apollo.queries.codequalityViolations.fetchMore).toHaveBeenCalledWith( + expect.objectContaining({ + variables: expect.objectContaining({ + after: codeQualityViolations.pageInfo.endCursor, + }), + }), + ); + }); + }); + + describe('when there are no codequality issues', () => { + beforeEach(() => { + const emptyResponse = { + data: { + project: { + pipeline: { + codeQualityReports: { + ...codeQualityViolations, + edges: [], + count: 0, + }, + }, + }, + }, + }; + + createComponent(jest.fn().mockResolvedValue(emptyResponse)); + }); + + it('shows a message that no codequality issues were found', () => { + expect(findSuccessIcon().exists()).toBe(true); + expect(findStatus().text()).toBe('No code quality issues found'); + expect(wrapper.findAll('.report-block-list-issue')).toHaveLength(0); + }); + }); +}); diff --git a/ee/spec/frontend/fixtures/codequality_report.rb b/ee/spec/frontend/fixtures/codequality_report.rb new file mode 100644 index 00000000000000..847354459b2c4c --- /dev/null +++ b/ee/spec/frontend/fixtures/codequality_report.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Code Quality Report (GraphQL fixtures)' do + describe GraphQL::Query, type: :request do + include ApiHelpers + include GraphqlHelpers + include JavaScriptFixturesHelpers + + let_it_be(:project) { create(:project, :repository) } + let_it_be(:current_user) { create(:user) } + let_it_be(:pipeline) { create(:ci_pipeline, :success, :with_codequality_reports, project: project) } + + codequality_report_query_path = 'codequality_report/graphql/queries/get_code_quality_violations.query.graphql' + + it "graphql/#{codequality_report_query_path}.json" do + project.add_developer(current_user) + + query = get_graphql_query_as_string(codequality_report_query_path, ee: true) + + post_graphql(query, current_user: current_user, variables: { projectPath: project.full_path, iid: pipeline.iid }) + + expect_graphql_errors_to_be_empty + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 3841c7bcbeca79..2ff256c5d92b94 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -40214,6 +40214,9 @@ msgstr "" msgid "ciReport|Failed to load %{reportName} report" msgstr "" +msgid "ciReport|Failed to load Code Quality report" +msgstr "" + msgid "ciReport|Fixed" msgstr "" @@ -40246,6 +40249,9 @@ msgstr "" msgid "ciReport|Loading %{reportName} report" msgstr "" +msgid "ciReport|Loading Code Quality report" +msgstr "" + msgid "ciReport|Manage licenses" msgstr "" @@ -40282,6 +40288,9 @@ msgstr "" msgid "ciReport|Security scanning failed loading any results" msgstr "" +msgid "ciReport|Showing %{fetchedItems} of %{totalItems} items" +msgstr "" + msgid "ciReport|Solution" msgstr "" -- GitLab From 61dc8aaf686e3d47ad65d5988076ee11cff3bd7d Mon Sep 17 00:00:00 2001 From: mfluharty Date: Fri, 29 Oct 2021 12:03:59 -0600 Subject: [PATCH 3/4] Adjust query to get nodes without edges We're not using the edges for anything anyway --- .../codequality_report_graphql.vue | 17 +++++++---------- .../get_code_quality_violations.query.graphql | 14 ++++++-------- .../codequality_report_graphql_spec.js | 2 +- 3 files changed, 14 insertions(+), 19 deletions(-) diff --git a/ee/app/assets/javascripts/codequality_report/codequality_report_graphql.vue b/ee/app/assets/javascripts/codequality_report/codequality_report_graphql.vue index c2fb9f901d2bf8..a91eb96893aceb 100644 --- a/ee/app/assets/javascripts/codequality_report/codequality_report_graphql.vue +++ b/ee/app/assets/javascripts/codequality_report/codequality_report_graphql.vue @@ -36,15 +36,12 @@ export default { }, update({ project: { - pipeline: { codeQualityReports: { edges = [], pageInfo = {}, count = 0 } = {} } = {}, + pipeline: { codeQualityReports: { nodes = [], pageInfo = {}, count = 0 } = {} } = {}, } = {}, }) { return { - edges, - parsedList: parseCodeclimateMetrics( - edges.map((edge) => edge.node), - this.blobPath, - ), + nodes, + parsedList: parseCodeclimateMetrics(nodes, this.blobPath), count, pageInfo, }; @@ -62,7 +59,7 @@ export default { data() { return { codequalityViolations: { - edges: [], + nodes: [], parsedList: [], count: 0, pageInfo: {}, @@ -116,9 +113,9 @@ export default { }, updateQuery: (previousResult, { fetchMoreResult }) => { return produce(fetchMoreResult, (draftData) => { - draftData.project.pipeline.codeQualityReports.edges = [ - ...previousResult.project.pipeline.codeQualityReports.edges, - ...draftData.project.pipeline.codeQualityReports.edges, + draftData.project.pipeline.codeQualityReports.nodes = [ + ...previousResult.project.pipeline.codeQualityReports.nodes, + ...draftData.project.pipeline.codeQualityReports.nodes, ]; }); }, diff --git a/ee/app/assets/javascripts/codequality_report/graphql/queries/get_code_quality_violations.query.graphql b/ee/app/assets/javascripts/codequality_report/graphql/queries/get_code_quality_violations.query.graphql index 35cd8bc0f47832..febfcbd77b76de 100644 --- a/ee/app/assets/javascripts/codequality_report/graphql/queries/get_code_quality_violations.query.graphql +++ b/ee/app/assets/javascripts/codequality_report/graphql/queries/get_code_quality_violations.query.graphql @@ -5,14 +5,12 @@ query getCodeQualityViolations($projectPath: ID!, $iid: ID!, $first: Int, $after pipeline(iid: $iid) { codeQualityReports(first: $first, after: $after) { count - edges { - node { - line - description - path - fingerprint - severity - } + nodes { + line + description + path + fingerprint + severity } pageInfo { ...PageInfo diff --git a/ee/spec/frontend/codequality_report/codequality_report_graphql_spec.js b/ee/spec/frontend/codequality_report/codequality_report_graphql_spec.js index ef933b3648e908..79095bbf981738 100644 --- a/ee/spec/frontend/codequality_report/codequality_report_graphql_spec.js +++ b/ee/spec/frontend/codequality_report/codequality_report_graphql_spec.js @@ -114,7 +114,7 @@ describe('Codequality report app', () => { pipeline: { codeQualityReports: { ...codeQualityViolations, - edges: [], + nodes: [], count: 0, }, }, -- GitLab From f5b268eeae0b77ceaf1241815fef4df9f50507b8 Mon Sep 17 00:00:00 2001 From: mfluharty Date: Tue, 2 Nov 2021 16:40:09 -0600 Subject: [PATCH 4/4] Suggested spec updates Add feature spec cases for disabled feature flag and tab navigation Import query response fixture into mock data, then into component spec --- .../projects/pipelines/pipeline_spec.rb | 52 ++++++++++++++++--- .../codequality_report_graphql_spec.js | 9 ++-- .../frontend/codequality_report/mock_data.js | 7 +++ 3 files changed, 54 insertions(+), 14 deletions(-) diff --git a/ee/spec/features/projects/pipelines/pipeline_spec.rb b/ee/spec/features/projects/pipelines/pipeline_spec.rb index fe8771a1987bbf..240863424a7981 100644 --- a/ee/spec/features/projects/pipelines/pipeline_spec.rb +++ b/ee/spec/features/projects/pipelines/pipeline_spec.rb @@ -214,18 +214,33 @@ context 'with code quality artifact' do before do create(:ee_ci_build, :codequality, pipeline: pipeline) - visit codequality_report_project_pipeline_path(project, pipeline) end - it 'shows code quality tab pane as active, quality issue with link to file, and events for data tracking' do - expect(page).to have_content('Code Quality') - expect(page).to have_css('#js-tab-codequality') + context 'when navigating directly to the code quality tab' do + before do + visit codequality_report_project_pipeline_path(project, pipeline) + end - expect(page).to have_content('Method `new_array` has 12 arguments (exceeds 4 allowed). Consider refactoring.') - expect(find_link('foo.rb:10')[:href]).to end_with(project_blob_path(project, File.join(pipeline.commit.id, 'foo.rb')) + '#L10') + it_behaves_like 'an active code quality tab' + end + + context 'when starting from the pipeline tab' do + before do + visit project_pipeline_path(project, pipeline) + end + + it 'shows the code quality tab as inactive' do + expect(page).to have_content('Code Quality') + expect(page).not_to have_css('#js-tab-codequality') + end + + context 'when the code quality tab is clicked' do + before do + click_link 'Code Quality' + end - expect(page).to have_selector('[data-track-action="click_button"]') - expect(page).to have_selector('[data-track-label="get_codequality_report"]') + it_behaves_like 'an active code quality tab' + end end end @@ -257,6 +272,19 @@ end end + shared_examples_for 'an active code quality tab' do + it 'shows code quality tab pane as active, quality issue with link to file, and events for data tracking' do + expect(page).to have_content('Code Quality') + expect(page).to have_css('#js-tab-codequality') + + expect(page).to have_content('Method `new_array` has 12 arguments (exceeds 4 allowed). Consider refactoring.') + expect(find_link('foo.rb:10')[:href]).to end_with(project_blob_path(project, File.join(pipeline.commit.id, 'foo.rb')) + '#L10') + + expect(page).to have_selector('[data-track-action="click_button"]') + expect(page).to have_selector('[data-track-label="get_codequality_report"]') + end + end + context 'for a branch pipeline' do let(:pipeline) { create(:ci_pipeline, project: project, ref: 'master', sha: project.commit.id) } @@ -278,6 +306,14 @@ it_behaves_like 'full codequality report' end + + context 'with graphql feature flag disabled' do + let(:pipeline) { create(:ci_pipeline, project: project, ref: 'master', sha: project.commit.id) } + + stub_feature_flags(graphql_code_quality_full_report: false) + + it_behaves_like 'full codequality report' + end end private diff --git a/ee/spec/frontend/codequality_report/codequality_report_graphql_spec.js b/ee/spec/frontend/codequality_report/codequality_report_graphql_spec.js index 79095bbf981738..8113c2f8fec90d 100644 --- a/ee/spec/frontend/codequality_report/codequality_report_graphql_spec.js +++ b/ee/spec/frontend/codequality_report/codequality_report_graphql_spec.js @@ -1,14 +1,11 @@ import { GlInfiniteScroll } from '@gitlab/ui'; import { mount, createLocalVue } from '@vue/test-utils'; import VueApollo from 'vue-apollo'; -import codeQualityViolationsQueryResponse from 'test_fixtures/graphql/codequality_report/graphql/queries/get_code_quality_violations.query.graphql.json'; import waitForPromises from 'helpers/wait_for_promises'; import createMockApollo from 'helpers/mock_apollo_helper'; import CodequalityReportApp from 'ee/codequality_report/codequality_report_graphql.vue'; import getCodeQualityViolations from 'ee/codequality_report/graphql/queries/get_code_quality_violations.query.graphql'; - -const codeQualityViolations = - codeQualityViolationsQueryResponse.data.project.pipeline.codeQualityReports; +import { mockGetCodeQualityViolationsResponse, codeQualityViolations } from './mock_data'; const localVue = createLocalVue(); localVue.use(VueApollo); @@ -17,7 +14,7 @@ describe('Codequality report app', () => { let wrapper; const createComponent = ( - mockReturnValue = jest.fn().mockResolvedValue(codeQualityViolationsQueryResponse), + mockReturnValue = jest.fn().mockResolvedValue(mockGetCodeQualityViolationsResponse), mountFn = mount, ) => { const apolloProvider = createMockApollo([[getCodeQualityViolations, mockReturnValue]]); @@ -65,7 +62,7 @@ describe('Codequality report app', () => { describe('when there are codequality issues', () => { beforeEach(() => { - createComponent(jest.fn().mockResolvedValue(codeQualityViolationsQueryResponse)); + createComponent(jest.fn().mockResolvedValue(mockGetCodeQualityViolationsResponse)); }); it('renders the codequality issues', () => { diff --git a/ee/spec/frontend/codequality_report/mock_data.js b/ee/spec/frontend/codequality_report/mock_data.js index 18f6b1ad3be455..87da5a847635f6 100644 --- a/ee/spec/frontend/codequality_report/mock_data.js +++ b/ee/spec/frontend/codequality_report/mock_data.js @@ -1,3 +1,10 @@ +import mockGetCodeQualityViolationsResponse from 'test_fixtures/graphql/codequality_report/graphql/queries/get_code_quality_violations.query.graphql.json'; + +export { mockGetCodeQualityViolationsResponse }; + +export const codeQualityViolations = + mockGetCodeQualityViolationsResponse.data.project.pipeline.codeQualityReports; + export const unparsedIssues = [ { type: 'issue', -- GitLab