From d01ed5df50913b9d70cf23108b0ee932f09d29a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Caplette?= Date: Tue, 18 Oct 2022 11:05:36 -0400 Subject: [PATCH 1/2] Add VueRouter to pipeline tabs To faciliate navigation in the new pipeline tabs in Vue, we are adding VueRouter to preserves old routing and faciliate navigation, sharing urls and reloading on the right tab. --- .../pipelines/components/dag/dag.vue | 4 +- .../pipelines/components/pipeline_tabs.vue | 77 +++++-- app/assets/javascripts/pipelines/constants.js | 3 +- .../pipelines/pipeline_details_bundle.js | 10 +- .../javascripts/pipelines/pipeline_tabs.js | 15 +- app/assets/javascripts/pipelines/routes.js | 20 ++ app/assets/javascripts/pipelines/utils.js | 17 +- .../projects/pipelines_controller.rb | 22 +- app/helpers/projects/pipeline_helper.rb | 1 + app/views/projects/pipelines/show.html.haml | 2 +- .../codequality_report_app_wrapper.vue | 35 ++++ .../pipelines/components/pipeline_tabs.vue | 68 +++--- .../javascripts/pipelines/pipeline_tabs.js | 5 +- ee/app/assets/javascripts/pipelines/routes.js | 12 ++ .../ee/projects/pipelines_controller.rb | 18 +- .../projects/pipelines_controller_spec.rb | 27 +-- .../codequality_report_app_wrapper_spec.js | 52 +++++ .../components/pipeline_tabs_spec.js | 197 +++++++----------- .../projects/pipelines_controller_spec.rb | 52 ++--- .../components/pipeline_tabs_spec.js | 26 +-- .../pipelines/pipeline_graph/utils_spec.js | 17 +- spec/frontend/pipelines/pipeline_tabs_spec.js | 32 +-- spec/helpers/projects/pipeline_helper_spec.rb | 1 + 23 files changed, 370 insertions(+), 343 deletions(-) create mode 100644 app/assets/javascripts/pipelines/routes.js create mode 100644 ee/app/assets/javascripts/pipelines/components/codequality_report_app_wrapper.vue create mode 100644 ee/app/assets/javascripts/pipelines/routes.js create mode 100644 ee/spec/frontend/pipelines/components/codequality_report_app_wrapper_spec.js diff --git a/app/assets/javascripts/pipelines/components/dag/dag.vue b/app/assets/javascripts/pipelines/components/dag/dag.vue index 475dd3bf36ee5d..be12df68f762ce 100644 --- a/app/assets/javascripts/pipelines/components/dag/dag.vue +++ b/app/assets/javascripts/pipelines/components/dag/dag.vue @@ -29,7 +29,7 @@ export default { dagDocPath: { default: null, }, - emptySvgPath: { + emptyDagSvgPath: { default: '', }, pipelineIid: { @@ -213,7 +213,7 @@ export default { /> diff --git a/ee/app/assets/javascripts/pipelines/pipeline_tabs.js b/ee/app/assets/javascripts/pipelines/pipeline_tabs.js index e0a886834cab53..fca1459413ae34 100644 --- a/ee/app/assets/javascripts/pipelines/pipeline_tabs.js +++ b/ee/app/assets/javascripts/pipelines/pipeline_tabs.js @@ -9,12 +9,12 @@ import { __ } from '~/locale'; Vue.use(VueRouter); -export const createAppOptions = (selector, apolloProvider) => { +export const createAppOptions = (selector, apolloProvider, router) => { const el = document.querySelector(selector); if (!el) return null; - const appOptionsCE = createAppOptionsCE(selector, apolloProvider); + const appOptionsCE = createAppOptionsCE(selector, apolloProvider, router); const { dataset } = el; @@ -33,7 +33,6 @@ export const createAppOptions = (selector, apolloProvider) => { } return merge({}, appOptionsCE, { - router: new VueRouter(), provide: vulnerabilityReportProvides, }); }; diff --git a/ee/app/assets/javascripts/pipelines/routes.js b/ee/app/assets/javascripts/pipelines/routes.js new file mode 100644 index 00000000000000..e0641a73200f2d --- /dev/null +++ b/ee/app/assets/javascripts/pipelines/routes.js @@ -0,0 +1,12 @@ +import LicenseReportApp from 'ee/vue_shared/license_compliance/mr_widget_license_report.vue'; +import CodequalityReportAppWrapper from 'ee/pipelines/components/codequality_report_app_wrapper.vue'; +import { codeQualityTabName, licensesTabName, securityTabName } from '~/pipelines/constants'; +import { routes as ceRoutes } from '~/pipelines/routes'; +import PipelineSecurityDashboard from 'ee/security_dashboard/components/pipeline/pipeline_security_dashboard.vue'; + +export const routes = [ + ...ceRoutes, + { name: securityTabName, path: '/security', component: PipelineSecurityDashboard }, + { name: licensesTabName, path: '/licenses', component: LicenseReportApp }, + { name: codeQualityTabName, path: '/codequality_report', component: CodequalityReportAppWrapper }, +]; diff --git a/ee/app/controllers/ee/projects/pipelines_controller.rb b/ee/app/controllers/ee/projects/pipelines_controller.rb index 1a0ec1c74ce1ff..19d2d713accf80 100644 --- a/ee/app/controllers/ee/projects/pipelines_controller.rb +++ b/ee/app/controllers/ee/projects/pipelines_controller.rb @@ -23,11 +23,7 @@ module PipelinesController def security if pipeline.expose_security_dashboard? - if ::Feature.enabled?(:pipeline_tabs_vue, project) - redirect_to pipeline_path(pipeline, tab: 'security') - else - render_show - end + render_show else redirect_to pipeline_path(pipeline) end @@ -39,11 +35,7 @@ def licenses respond_to do |format| format.html do - if ::Feature.enabled?(:pipeline_tabs_vue, project) - redirect_to pipeline_path(pipeline, tab: 'licenses') - else - render_show - end + render_show end format.json do render status: :ok, json: LicenseScanningReportsSerializer.new.represent( @@ -54,11 +46,7 @@ def licenses end def codequality_report - if ::Feature.enabled?(:pipeline_tabs_vue, project) - redirect_to pipeline_path(pipeline, tab: 'codequality_report') - else - render_show - end + render_show end private diff --git a/ee/spec/controllers/projects/pipelines_controller_spec.rb b/ee/spec/controllers/projects/pipelines_controller_spec.rb index 9d5089ef24059d..12db99cf75655b 100644 --- a/ee/spec/controllers/projects/pipelines_controller_spec.rb +++ b/ee/spec/controllers/projects/pipelines_controller_spec.rb @@ -26,8 +26,9 @@ get :security, params: { namespace_id: project.namespace, project_id: project, id: pipeline } end - it 'redirects to the pipeline page with the security tab query param' do - expect(response).to redirect_to(pipeline_path(pipeline, tab: 'security')) + it 'responds with a 200 and show the template' do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template :show end end @@ -36,7 +37,7 @@ get :security, params: { namespace_id: project.namespace, project_id: project, id: pipeline } end - it do + it 'redirects to the pipeline page' do expect(response).to redirect_to(pipeline_path(pipeline)) end end @@ -50,7 +51,7 @@ get :security, params: { namespace_id: project.namespace, project_id: project, id: pipeline } end - it do + it 'redirects to the pipeline page' do expect(response).to redirect_to(pipeline_path(pipeline)) end end @@ -60,7 +61,7 @@ get :security, params: { namespace_id: project.namespace, project_id: project, id: pipeline } end - it do + it 'redirects to the pipeline page' do expect(response).to redirect_to(pipeline_path(pipeline)) end end @@ -70,10 +71,11 @@ describe 'GET codequality_report' do let(:pipeline) { create(:ci_pipeline, project: project) } - it 'redirects to pipeline path with dag tab query param' do + it 'renders the show template' do get :codequality_report, params: { namespace_id: project.namespace, project_id: project, id: pipeline } - expect(response).to redirect_to(pipeline_path(pipeline, tab: 'codequality_report')) + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template :show end end @@ -97,8 +99,9 @@ licenses_with_html end - it 'redirects to the pipeline page with the licenses tab query param' do - expect(response).to redirect_to(pipeline_path(pipeline, tab: 'licenses')) + it 'responds with a 200 and show the template' do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template :show end end @@ -164,7 +167,7 @@ licenses_with_html end - it do + it 'redirects to the pipeline page' do expect(response).to redirect_to(pipeline_path(pipeline)) end end @@ -187,7 +190,7 @@ licenses_with_html end - it do + it 'redirects to the pipeline page' do expect(response).to redirect_to(pipeline_path(pipeline)) end end @@ -208,7 +211,7 @@ licenses_with_html end - it do + it 'redirects to the pipeline page' do expect(response).to redirect_to(pipeline_path(pipeline)) end end diff --git a/ee/spec/frontend/pipelines/components/codequality_report_app_wrapper_spec.js b/ee/spec/frontend/pipelines/components/codequality_report_app_wrapper_spec.js new file mode 100644 index 00000000000000..a2c4a315bf8f4b --- /dev/null +++ b/ee/spec/frontend/pipelines/components/codequality_report_app_wrapper_spec.js @@ -0,0 +1,52 @@ +import { shallowMount } from '@vue/test-utils'; +import { extendedWrapper } from 'helpers/vue_test_utils_helper'; +import CodequalityReportApp from 'ee/codequality_report/codequality_report.vue'; +import CodequalityReportAppGraphql from 'ee/codequality_report/codequality_report_graphql.vue'; +import CodequalityReportAppWrapper from 'ee/pipelines/components/codequality_report_app_wrapper.vue'; + +describe('Codequality report app wrapper', () => { + let wrapper; + + const findCodeQualityApp = () => wrapper.findComponent(CodequalityReportApp); + const findCodeQualityAppGraphql = () => wrapper.findComponent(CodequalityReportAppGraphql); + + const defaultProvide = { + codequalityProjectPath: '', + codequalityBlobPath: '', + codequalityReportDownloadPath: '', + pipelineIid: '0', + }; + + const createComponent = ({ provide = {} } = {}) => { + wrapper = extendedWrapper( + shallowMount(CodequalityReportAppWrapper, { + provide: { + ...defaultProvide, + ...provide, + }, + }), + ); + }; + + afterEach(() => { + wrapper.destroy(); + }); + + it.each` + ffEnabled + ${true} + ${false} + `('Show the correct code quality app When graphQL ff is $ffEnabled', ({ ffEnabled }) => { + createComponent({ + provide: { + canGenerateCodequalityReports: true, + glFeatures: { + graphqlCodeQualityFullReport: ffEnabled, + }, + }, + }); + + expect(findCodeQualityAppGraphql().exists()).toBe(ffEnabled); + expect(findCodeQualityApp().exists()).toBe(!ffEnabled); + }); +}); diff --git a/ee/spec/frontend/pipelines/components/pipeline_tabs_spec.js b/ee/spec/frontend/pipelines/components/pipeline_tabs_spec.js index 04cfdb72587cf4..f1de6fdcc97169 100644 --- a/ee/spec/frontend/pipelines/components/pipeline_tabs_spec.js +++ b/ee/spec/frontend/pipelines/components/pipeline_tabs_spec.js @@ -4,10 +4,7 @@ import { shallowMount } from '@vue/test-utils'; import { extendedWrapper } from 'helpers/vue_test_utils_helper'; import BasePipelineTabs from '~/pipelines/components/pipeline_tabs.vue'; import PipelineTabs from 'ee/pipelines/components/pipeline_tabs.vue'; -import CodequalityReportApp from 'ee/codequality_report/codequality_report.vue'; -import CodequalityReportAppGraphql from 'ee/codequality_report/codequality_report_graphql.vue'; -import LicenseReportApp from 'ee/vue_shared/license_compliance/mr_widget_license_report.vue'; -import PipelineSecurityDashboard from 'ee/security_dashboard/components/pipeline/pipeline_security_dashboard.vue'; +import CodequalityReportAppWrapper from 'ee/pipelines/components/codequality_report_app_wrapper.vue'; describe('The Pipeline Tabs', () => { let wrapper; @@ -20,31 +17,25 @@ describe('The Pipeline Tabs', () => { const findPipelineTab = () => wrapper.findByTestId('pipeline-tab'); const findSecurityTab = () => wrapper.findByTestId('security-tab'); const findTestsTab = () => wrapper.findByTestId('tests-tab'); - - const findCodeQualityApp = () => wrapper.findComponent(CodequalityReportApp); - const findCodeQualityAppGraphql = () => wrapper.findComponent(CodequalityReportAppGraphql); - const findLicenseApp = () => wrapper.findComponent(LicenseReportApp); - const findSecurityApp = () => wrapper.findComponent(PipelineSecurityDashboard); - const getLicenseCount = () => wrapper.findByTestId('license-counter').text(); const getCodequalityCount = () => wrapper.findByTestId('codequality-counter').text(); + const findCodeQualityRouterView = () => wrapper.findComponent({ ref: 'router-view-codequality' }); + const findLicensesRouterView = () => wrapper.findComponent({ ref: 'router-view-licenses' }); const defaultProvide = { canGenerateCodequalityReports: false, + canManageLicenses: true, codequalityReportDownloadPath: '', - codequalityProjectPath: '', - codequalityBlobPath: '', - pipelineIid: '0', defaultTabValue: '', exposeSecurityDashboard: false, exposeLicenseScanningData: false, + failedJobsCount: 1, + failedJobsSummary: [], isFullCodequalityReportAvailable: true, licenseManagementApiUrl: '/path/to/license_management_api_url', licensesApiPath: '/path/to/licenses_api', licenseManagementSettingsPath: '/path/to/license_management_settings', - canManageLicenses: true, - failedJobsCount: 1, - failedJobsSummary: [], + pipelineIid: '100', totalJobCount: 10, testsCount: 123, }; @@ -59,7 +50,7 @@ describe('The Pipeline Tabs', () => { }, stubs: { BasePipelineTabs, - TestReports: { template: '
' }, + RouterView: true, ...stubs, }, }), @@ -100,136 +91,94 @@ describe('The Pipeline Tabs', () => { describe('EE Tabs', () => { describe('visibility', () => { it.each` - tabName | tabComponent | appComponent | provideKey | isVisible | text - ${'Security'} | ${findSecurityTab} | ${findSecurityApp} | ${'exposeSecurityDashboard'} | ${true} | ${'shows'} - ${'Security'} | ${findSecurityTab} | ${findSecurityApp} | ${'exposeSecurityDashboard'} | ${false} | ${'hides'} - ${'License'} | ${findLicenseTab} | ${findLicenseApp} | ${'exposeLicenseScanningData'} | ${true} | ${'shows'} - ${'License'} | ${findLicenseTab} | ${findLicenseApp} | ${'exposeLicenseScanningData'} | ${false} | ${'hides'} + tabName | tabComponent | provideKey | isVisible | text + ${'Security'} | ${findSecurityTab} | ${'exposeSecurityDashboard'} | ${true} | ${'shows'} + ${'Security'} | ${findSecurityTab} | ${'exposeSecurityDashboard'} | ${false} | ${'hides'} + ${'License'} | ${findLicenseTab} | ${'exposeLicenseScanningData'} | ${true} | ${'shows'} + ${'License'} | ${findLicenseTab} | ${'exposeLicenseScanningData'} | ${false} | ${'hides'} `( - '$text $tabName and its associated component when $provideKey is $provideKey', - ({ tabComponent, appComponent, provideKey, isVisible }) => { + '$text $tabName tab when $provideKey is $provideKey', + ({ tabComponent, provideKey, isVisible }) => { createComponent({ provide: { [provideKey]: isVisible }, }); expect(tabComponent().exists()).toBe(isVisible); - expect(appComponent().exists()).toBe(isVisible); }, ); }); - describe('code quality visibility', () => { - describe('feature flags', () => { - describe('with `graphqlCodeQualityFullReport` enabled', () => { - beforeEach(() => { - createComponent({ - provide: { - canGenerateCodequalityReports: true, - glFeatures: { - graphqlCodeQualityFullReport: true, - }, - }, - stubs: { GlTab }, - }); - }); - - it('shows the graphql code quality report app', () => { - expect(findCodeQualityAppGraphql().exists()).toBe(true); - expect(findCodeQualityApp().exists()).toBe(false); - }); - - it('updates the codequality badge after a new count has been emitted', async () => { - const newLicenseCount = 100; - expect(getCodequalityCount()).toBe('0'); - - findCodeQualityAppGraphql().vm.$emit('updateBadgeCount', newLicenseCount); - await nextTick(); - - expect(getCodequalityCount()).toBe(`${newLicenseCount}`); - }); + it.each` + canGenerate | isVisible | codequalityReportDownloadPath | isReportAvailable | text + ${true} | ${true} | ${''} | ${true} | ${'shows'} + ${false} | ${false} | ${''} | ${true} | ${'hides'} + ${false} | ${true} | ${'/path'} | ${true} | ${'shows'} + ${true} | ${true} | ${'/path'} | ${true} | ${'shows'} + ${true} | ${false} | ${'/path'} | ${false} | ${'hides'} + `( + '$text Code Quality tab when canGenerateCodequalityReports is $canGenerate and codequalityReportDownloadPath is $codequalityReportDownloadPath', + ({ canGenerate, isReportAvailable, isVisible, codequalityReportDownloadPath }) => { + createComponent({ + provide: { + isFullCodequalityReportAvailable: isReportAvailable, + canGenerateCodequalityReports: canGenerate, + codequalityReportDownloadPath, + }, }); + expect(findCodeQualityTab().exists()).toBe(isVisible); + }, + ); + }); - describe('with `graphqlCodeQualityFullReport` disabled', () => { - beforeEach(() => { - createComponent({ - provide: { - canGenerateCodequalityReports: true, - glFeatures: { - graphqlCodeQualityFullReport: false, - }, - }, - stubs: { GlTab }, - }); - }); - - it('shows the default code quality report app', () => { - expect(findCodeQualityAppGraphql().exists()).toBe(false); - expect(findCodeQualityApp().exists()).toBe(true); - }); - - it('updates the codequality badge after a new count has been emitted', async () => { - const newLicenseCount = 100; - - expect(getCodequalityCount()).toBe('0'); + describe('codequality badge count', () => { + beforeEach(() => { + createComponent({ + provide: { + isFullCodequalityReportAvailable: true, + canGenerateCodequalityReports: true, + codequalityReportDownloadPath: '/dsda', + }, + stubs: { GlTab, CodequalityReportAppWrapper }, + }); + }); - findCodeQualityApp().vm.$emit('updateBadgeCount', newLicenseCount); - await nextTick(); + it('updates the codequality badge after a new count has been emitted', async () => { + const newLicenseCount = 100; + expect(getCodequalityCount()).toBe('0'); - expect(getCodequalityCount()).toBe(`${newLicenseCount}`); - }); - }); - }); + findCodeQualityRouterView().vm.$emit('updateBadgeCount', newLicenseCount); + await nextTick(); - it.each` - provideValue | isVisible | codequalityReportDownloadPath | isReportAvailable | text - ${true} | ${true} | ${''} | ${true} | ${'shows'} - ${false} | ${false} | ${''} | ${true} | ${'hides'} - ${false} | ${true} | ${'/path'} | ${true} | ${'shows'} - ${true} | ${true} | ${'/path'} | ${true} | ${'shows'} - ${true} | ${false} | ${'/path'} | ${false} | ${'hides'} - `( - '$text Code Quality and its associated component when canGenerateCodequalityReports is $provideValue and codequalityReportDownloadPath is $codequalityReportDownloadPath', - ({ provideValue, isReportAvailable, isVisible, codequalityReportDownloadPath }) => { - createComponent({ - provide: { - isFullCodequalityReportAvailable: isReportAvailable, - canGenerateCodequalityReports: provideValue, - codequalityReportDownloadPath, - }, - }); - expect(findCodeQualityTab().exists()).toBe(isVisible); - expect(findCodeQualityApp().exists()).toBe(isVisible); - }, - ); + expect(getCodequalityCount()).toBe(`${newLicenseCount}`); }); + }); - describe('license compliance', () => { - beforeEach(() => { - createComponent({ - provide: { exposeLicenseScanningData: true }, - stubs: { GlTab }, - }); + describe('license compliance', () => { + beforeEach(() => { + createComponent({ + provide: { exposeLicenseScanningData: true }, + stubs: { GlTab }, }); + }); - it('passes the correct props to the license report app', () => { - expect(findLicenseApp().props()).toMatchObject({ - apiUrl: defaultProvide.licenseManagementApiUrl, - licensesApiPath: defaultProvide.licensesApiPath, - licenseManagementSettingsPath: defaultProvide.licenseManagementSettingsPath, - canManageLicenses: defaultProvide.canManageLicenses, - alwaysOpen: true, - }); + it('passes down all props to the license app', () => { + expect(findLicensesRouterView().attributes()).toMatchObject({ + 'api-url': defaultProvide.licenseManagementApiUrl, + 'licenses-api-path': defaultProvide.licensesApiPath, + 'license-management-settings-path': defaultProvide.licenseManagementSettingsPath, + 'can-manage-licenses': defaultProvide.canManageLicenses.toString(), + 'always-open': 'true', }); + }); - it('updates the license count badge after a new count has been emitted', async () => { - const newLicenseCount = 100; + it('updates the license count badge after a new count has been emitted', async () => { + const newLicenseCount = 100; - expect(getLicenseCount()).toBe('0'); + expect(getLicenseCount()).toBe('0'); - findLicenseApp().vm.$emit('updateBadgeCount', newLicenseCount); - await nextTick(); + findLicensesRouterView().vm.$emit('updateBadgeCount', newLicenseCount); + await nextTick(); - expect(getLicenseCount()).toBe(`${newLicenseCount}`); - }); + expect(getLicenseCount()).toBe(`${newLicenseCount}`); }); }); }); diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb index b132c0b5a692c1..527cba9e618939 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -20,23 +20,11 @@ end shared_examples 'the show page' do |param| - it 'redirects to pipeline path with param' do + it 'renders the show template' do get param, params: { namespace_id: project.namespace, project_id: project, id: pipeline } - expect(response).to redirect_to(pipeline_path(pipeline, tab: param)) - end - - context 'when the FF pipeline_tabs_vue is disabled' do - before do - stub_feature_flags(pipeline_tabs_vue: false) - end - - it 'renders the show template' do - get param, params: { namespace_id: project.namespace, project_id: project, id: pipeline } - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template :show - end + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template :show end end @@ -710,37 +698,25 @@ def create_build(stage, stage_idx, name, params = {}) describe 'GET failures' do let(:pipeline) { create(:ci_pipeline, project: project) } - context 'with ff `pipeline_tabs_vue` disabled' do + context 'with failed jobs' do before do - stub_feature_flags(pipeline_tabs_vue: false) - end - - context 'with failed jobs' do - before do - create(:ci_build, :failed, pipeline: pipeline, name: 'hello') - end - - it 'shows the page' do - get :failures, params: { namespace_id: project.namespace, project_id: project, id: pipeline } - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template :show - end + create(:ci_build, :failed, pipeline: pipeline, name: 'hello') end - context 'without failed jobs' do - it 'redirects to the main pipeline page' do - get :failures, params: { namespace_id: project.namespace, project_id: project, id: pipeline } + it 'shows the page' do + get :failures, params: { namespace_id: project.namespace, project_id: project, id: pipeline } - expect(response).to redirect_to(pipeline_path(pipeline)) - end + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template :show end end - it 'redirects to the pipeline page with `failures` query param' do - get :failures, params: { namespace_id: project.namespace, project_id: project, id: pipeline } + context 'without failed jobs' do + it 'redirects to the main pipeline page' do + get :failures, params: { namespace_id: project.namespace, project_id: project, id: pipeline } - expect(response).to redirect_to(pipeline_path(pipeline, tab: 'failures')) + expect(response).to redirect_to(pipeline_path(pipeline)) + end end end diff --git a/spec/frontend/pipelines/components/pipeline_tabs_spec.js b/spec/frontend/pipelines/components/pipeline_tabs_spec.js index 3680d9d62c703f..c2cb95d4320ecf 100644 --- a/spec/frontend/pipelines/components/pipeline_tabs_spec.js +++ b/spec/frontend/pipelines/components/pipeline_tabs_spec.js @@ -2,10 +2,6 @@ import { shallowMount } from '@vue/test-utils'; import { GlTab } from '@gitlab/ui'; import { extendedWrapper } from 'helpers/vue_test_utils_helper'; import PipelineTabs from '~/pipelines/components/pipeline_tabs.vue'; -import PipelineGraphWrapper from '~/pipelines/components/graph/graph_component_wrapper.vue'; -import Dag from '~/pipelines/components/dag/dag.vue'; -import JobsApp from '~/pipelines/components/jobs/jobs_app.vue'; -import TestReports from '~/pipelines/components/test_reports/test_reports.vue'; describe('The Pipeline Tabs', () => { let wrapper; @@ -16,12 +12,6 @@ describe('The Pipeline Tabs', () => { const findPipelineTab = () => wrapper.findByTestId('pipeline-tab'); const findTestsTab = () => wrapper.findByTestId('tests-tab'); - const findDagApp = () => wrapper.findComponent(Dag); - const findFailedJobsApp = () => wrapper.findComponent(JobsApp); - const findJobsApp = () => wrapper.findComponent(JobsApp); - const findPipelineApp = () => wrapper.findComponent(PipelineGraphWrapper); - const findTestsApp = () => wrapper.findComponent(TestReports); - const findFailedJobsBadge = () => wrapper.findByTestId('failed-builds-counter'); const findJobsBadge = () => wrapper.findByTestId('builds-counter'); const findTestsBadge = () => wrapper.findByTestId('tests-counter'); @@ -43,6 +33,7 @@ describe('The Pipeline Tabs', () => { }, stubs: { GlTab, + RouterView: true, }, }), ); @@ -54,17 +45,16 @@ describe('The Pipeline Tabs', () => { describe('Tabs', () => { it.each` - tabName | tabComponent | appComponent - ${'Pipeline'} | ${findPipelineTab} | ${findPipelineApp} - ${'Dag'} | ${findDagTab} | ${findDagApp} - ${'Jobs'} | ${findJobsTab} | ${findJobsApp} - ${'Failed Jobs'} | ${findFailedJobsTab} | ${findFailedJobsApp} - ${'Tests'} | ${findTestsTab} | ${findTestsApp} - `('shows $tabName tab with its associated component', ({ appComponent, tabComponent }) => { + tabName | tabComponent + ${'Pipeline'} | ${findPipelineTab} + ${'Dag'} | ${findDagTab} + ${'Jobs'} | ${findJobsTab} + ${'Failed Jobs'} | ${findFailedJobsTab} + ${'Tests'} | ${findTestsTab} + `('shows $tabName tab', ({ tabComponent }) => { createComponent(); expect(tabComponent().exists()).toBe(true); - expect(appComponent().exists()).toBe(true); }); describe('with no failed jobs', () => { diff --git a/spec/frontend/pipelines/pipeline_graph/utils_spec.js b/spec/frontend/pipelines/pipeline_graph/utils_spec.js index d6b13da3c3a5b8..41b020189d07de 100644 --- a/spec/frontend/pipelines/pipeline_graph/utils_spec.js +++ b/spec/frontend/pipelines/pipeline_graph/utils_spec.js @@ -1,5 +1,5 @@ import { createJobsHash, generateJobNeedsDict, getPipelineDefaultTab } from '~/pipelines/utils'; -import { TAB_QUERY_PARAM, validPipelineTabNames } from '~/pipelines/constants'; +import { validPipelineTabNames } from '~/pipelines/constants'; describe('utils functions', () => { const jobName1 = 'build_1'; @@ -173,18 +173,25 @@ describe('utils functions', () => { describe('getPipelineDefaultTab', () => { const baseUrl = 'http://gitlab.com/user/multi-projects-small/-/pipelines/332/'; - it('returns null if there was no `tab` params', () => { + it('returns null if there is only the base url', () => { expect(getPipelineDefaultTab(baseUrl)).toBe(null); }); - it('returns null if there was no valid tab param', () => { - expect(getPipelineDefaultTab(`${baseUrl}?${TAB_QUERY_PARAM}=invalid`)).toBe(null); + it('returns null if there was no valid last url part', () => { + expect(getPipelineDefaultTab(`${baseUrl}something`)).toBe(null); }); it('returns the correct tab name if present', () => { validPipelineTabNames.forEach((tabName) => { - expect(getPipelineDefaultTab(`${baseUrl}?${TAB_QUERY_PARAM}=${tabName}`)).toBe(tabName); + expect(getPipelineDefaultTab(`${baseUrl}${tabName}`)).toBe(tabName); }); }); + + it('returns the right value even with query params', () => { + const [tabName] = validPipelineTabNames; + expect(getPipelineDefaultTab(`${baseUrl}${tabName}?query="something"&query2="else"`)).toBe( + tabName, + ); + }); }); }); diff --git a/spec/frontend/pipelines/pipeline_tabs_spec.js b/spec/frontend/pipelines/pipeline_tabs_spec.js index b184ce31d20dd7..099748a5ccacd6 100644 --- a/spec/frontend/pipelines/pipeline_tabs_spec.js +++ b/spec/frontend/pipelines/pipeline_tabs_spec.js @@ -1,9 +1,7 @@ -import { createAppOptions, createPipelineTabs } from '~/pipelines/pipeline_tabs'; -import { updateHistory } from '~/lib/utils/url_utility'; +import { createAppOptions } from '~/pipelines/pipeline_tabs'; jest.mock('~/lib/utils/url_utility', () => ({ removeParams: () => 'gitlab.com', - updateHistory: jest.fn(), joinPaths: () => {}, setUrlFragment: () => {}, })); @@ -64,32 +62,4 @@ describe('~/pipelines/pipeline_tabs.js', () => { expect(createAppOptions('foo', null)).toBe(null); }); }); - - describe('createPipelineTabs', () => { - const title = 'Pipeline Tabs'; - - beforeAll(() => { - document.title = title; - }); - - afterAll(() => { - document.title = ''; - }); - - it('calls `updateHistory` with correct params', () => { - createPipelineTabs({}); - - expect(updateHistory).toHaveBeenCalledWith({ - title, - url: 'gitlab.com', - replace: true, - }); - }); - - it("returns early if options aren't provided", () => { - createPipelineTabs(); - - expect(updateHistory).not.toHaveBeenCalled(); - }); - }); }); diff --git a/spec/helpers/projects/pipeline_helper_spec.rb b/spec/helpers/projects/pipeline_helper_spec.rb index a70544ace1a01b..0d3466d6ed2757 100644 --- a/spec/helpers/projects/pipeline_helper_spec.rb +++ b/spec/helpers/projects/pipeline_helper_spec.rb @@ -31,6 +31,7 @@ suite_endpoint: project_pipeline_test_path(project, pipeline, suite_name: 'suite', format: :json), blob_path: project_blob_path(project, pipeline.sha), has_test_report: pipeline.complete_and_has_reports?(Ci::JobArtifact.of_report_type(:test)), + empty_dag_svg_path: match_asset_path('illustrations/empty-state/empty-dag-md.svg'), empty_state_image_path: match_asset_path('illustrations/empty-state/empty-test-cases-lg.svg'), artifacts_expired_image_path: match_asset_path('illustrations/pipeline.svg'), tests_count: pipeline.test_report_summary.total[:count] -- GitLab From 367ae6f22ec605fbd09fa4d5b9f1247257389678 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Caplette?= Date: Thu, 3 Nov 2022 11:31:46 -0400 Subject: [PATCH 2/2] Remove overly verbose methods --- .../pipelines/components/pipeline_tabs.vue | 25 ++++--------------- .../pipelines/components/pipeline_tabs.vue | 15 +++-------- 2 files changed, 8 insertions(+), 32 deletions(-) diff --git a/app/assets/javascripts/pipelines/components/pipeline_tabs.vue b/app/assets/javascripts/pipelines/components/pipeline_tabs.vue index 6878780c1aab93..3798863ae60557 100644 --- a/app/assets/javascripts/pipelines/components/pipeline_tabs.vue +++ b/app/assets/javascripts/pipelines/components/pipeline_tabs.vue @@ -60,21 +60,6 @@ export default { navigateTo(tabName) { this.$router.push({ name: tabName }); }, - selectDagTab() { - this.navigateTo(needsTabName); - }, - selectJobsTab() { - this.navigateTo(jobsTabName); - }, - selectFailuresTab() { - this.navigateTo(failedJobsTabName); - }, - selectPipelineTab() { - this.navigateTo(pipelineTabName); - }, - selectTestReport() { - this.navigateTo(testReportTabName); - }, }, }; @@ -87,7 +72,7 @@ export default { :active="isActive($options.tabNames.pipeline)" data-testid="pipeline-tab" lazy - @click="selectPipelineTab" + @click="navigateTo($options.tabNames.pipeline)" > @@ -97,7 +82,7 @@ export default { :active="isActive($options.tabNames.needs)" data-testid="dag-tab" lazy - @click="selectDagTab" + @click="navigateTo($options.tabNames.needs)" > @@ -105,7 +90,7 @@ export default { :active="isActive($options.tabNames.jobs)" data-testid="jobs-tab" lazy - @click="selectJobsTab" + @click="navigateTo($options.tabNames.jobs)" >