From 666bbc500e8328e4ddb200970d622db110d59869 Mon Sep 17 00:00:00 2001 From: Jose Vargas <jvargas@gitlab.com> Date: Mon, 7 Dec 2020 11:41:22 -0600 Subject: [PATCH 1/2] Refactor CI/CD analytics page to GraphQL This moves the way the chart data is being sent, instead of sending the data via HAML data attributes, this uses GraphQL to request the data asynchronously --- .../pipelines/charts/components/app.vue | 172 +++++++++++--- .../charts/components/app_legacy.vue | 151 ++++++++++++ .../charts/components/statistics_list.vue | 7 +- ...get_pipeline_count_by_status.query.graphql | 14 ++ ..._project_pipeline_statistics.query.graphql | 17 ++ .../projects/pipelines/charts/index.js | 29 ++- .../projects/pipelines_controller.rb | 1 + .../graphql_pipeline_analytics.yml | 8 + locale/gitlab.pot | 6 + .../charts/components/app_legacy_spec.js | 72 ++++++ .../pipelines/charts/components/app_spec.js | 52 +++-- .../charts/components/statistics_list_spec.js | 2 +- .../projects/pipelines/charts/mock_data.js | 215 ++++++++++++++++++ 13 files changed, 688 insertions(+), 58 deletions(-) create mode 100644 app/assets/javascripts/projects/pipelines/charts/components/app_legacy.vue create mode 100644 app/assets/javascripts/projects/pipelines/charts/graphql/queries/get_pipeline_count_by_status.query.graphql create mode 100644 app/assets/javascripts/projects/pipelines/charts/graphql/queries/get_project_pipeline_statistics.query.graphql create mode 100644 config/feature_flags/development/graphql_pipeline_analytics.yml create mode 100644 spec/frontend/projects/pipelines/charts/components/app_legacy_spec.js diff --git a/app/assets/javascripts/projects/pipelines/charts/components/app.vue b/app/assets/javascripts/projects/pipelines/charts/components/app.vue index c6e2b2e1140ec0..e89b5c76e38594 100644 --- a/app/assets/javascripts/projects/pipelines/charts/components/app.vue +++ b/app/assets/javascripts/projects/pipelines/charts/components/app.vue @@ -1,59 +1,129 @@ <script> import dateFormat from 'dateformat'; import { GlColumnChart } from '@gitlab/ui/dist/charts'; -import { __, sprintf } from '~/locale'; +import { __, s__, sprintf } from '~/locale'; +import createFlash, { FLASH_TYPES } from '~/flash'; import { getDateInPast } from '~/lib/utils/datetime_utility'; +import getPipelineCountByStatus from '../graphql/queries/get_pipeline_count_by_status.query.graphql'; +import getProjectPipelineStatistics from '../graphql/queries/get_project_pipeline_statistics.query.graphql'; import StatisticsList from './statistics_list.vue'; import PipelinesAreaChart from './pipelines_area_chart.vue'; import { CHART_CONTAINER_HEIGHT, - INNER_CHART_HEIGHT, - X_AXIS_LABEL_ROTATION, - X_AXIS_TITLE_OFFSET, CHART_DATE_FORMAT, + INNER_CHART_HEIGHT, ONE_WEEK_AGO_DAYS, ONE_MONTH_AGO_DAYS, + X_AXIS_LABEL_ROTATION, + X_AXIS_TITLE_OFFSET, } from '../constants'; +const defaultCountValues = { + totalPipelines: { + count: 0, + }, + successfulPipelines: { + count: 0, + }, +}; + +const defaultAnalyticsValues = { + weekPipelinesTotals: [], + weekPipelinesLabels: [], + weekPipelinesSuccessful: [], + monthPipelinesLabels: [], + monthPipelinesTotals: [], + monthPipelinesSuccessful: [], + yearPipelinesLabels: [], + yearPipelinesTotals: [], + yearPipelinesSuccessful: [], + pipelineTimesLabels: [], + pipelineTimesValues: [], +}; + export default { components: { - StatisticsList, GlColumnChart, + StatisticsList, PipelinesAreaChart, }, - props: { - counts: { - type: Object, - required: true, - }, - timesChartData: { - type: Object, - required: true, - }, - lastWeekChartData: { - type: Object, - required: true, - }, - lastMonthChartData: { - type: Object, - required: true, - }, - lastYearChartData: { - type: Object, - required: true, + inject: { + projectPath: { + type: String, + default: '', }, }, data() { return { - timesChartTransformedData: [ - { - name: 'full', - data: this.mergeLabelsAndValues(this.timesChartData.labels, this.timesChartData.values), - }, - ], + counts: { + ...defaultCountValues, + }, + analytics: { + ...defaultAnalyticsValues, + }, }; }, + apollo: { + counts: { + query: getPipelineCountByStatus, + variables() { + return { + projectPath: this.projectPath, + }; + }, + update(res) { + return res.project; + }, + error() { + createFlash({ + message: s__('PipelineCharts|An error has ocurred when retrieving the pipeline data'), + type: FLASH_TYPES.ALERT, + }); + }, + }, + analytics: { + query: getProjectPipelineStatistics, + variables() { + return { + projectPath: this.projectPath, + }; + }, + update(res) { + return res.project.pipelineAnalytics; + }, + error() { + createFlash({ + message: s__('PipelineCharts|An error has ocurred when retrieving the analytics data'), + type: FLASH_TYPES.ALERT, + }); + }, + }, + }, computed: { + successRatio() { + const { successfulPipelines, failedPipelines } = this.counts; + const successfulCount = successfulPipelines?.count; + const failedCount = failedPipelines?.count; + const ratio = (successfulCount / (successfulCount + failedCount)) * 100; + + return failedCount === 0 ? 100 : ratio; + }, + formattedCounts() { + const { + totalPipelines, + successfulPipelines, + failedPipelines, + totalPipelineDuration, + } = this.counts; + + return { + total: totalPipelines?.count, + success: successfulPipelines?.count, + failed: failedPipelines?.count, + successRatio: this.successRatio, + totalDuration: totalPipelineDuration, + }; + }, areaCharts() { const { lastWeek, lastMonth, lastYear } = this.$options.chartTitles; @@ -63,6 +133,38 @@ export default { this.buildAreaChartData(lastYear, this.lastYearChartData), ]; }, + lastWeekChartData() { + return { + labels: this.analytics.weekPipelinesLabels, + totals: this.analytics.weekPipelinesTotals, + success: this.analytics.weekPipelinesSuccessful, + }; + }, + lastMonthChartData() { + return { + labels: this.analytics.monthPipelinesLabels, + totals: this.analytics.monthPipelinesTotals, + success: this.analytics.monthPipelinesSuccessful, + }; + }, + lastYearChartData() { + return { + labels: this.analytics.yearPipelinesLabels, + totals: this.analytics.yearPipelinesTotals, + success: this.analytics.yearPipelinesSuccessful, + }; + }, + timesChartTransformedData() { + return [ + { + name: 'full', + data: this.mergeLabelsAndValues( + this.analytics.pipelineTimesLabels, + this.analytics.pipelineTimesValues, + ), + }, + ]; + }, }, methods: { mergeLabelsAndValues(labels, values) { @@ -116,13 +218,13 @@ export default { </script> <template> <div> - <div class="mb-3"> + <div class="gl-mb-3"> <h3>{{ s__('PipelineCharts|CI / CD Analytics') }}</h3> </div> - <h4 class="my-4">{{ s__('PipelineCharts|Overall statistics') }}</h4> + <h4 class="gl-my-4">{{ s__('PipelineCharts|Overall statistics') }}</h4> <div class="row"> <div class="col-md-6"> - <statistics-list :counts="counts" /> + <statistics-list :counts="formattedCounts" /> </div> <div class="col-md-6"> <strong> @@ -139,7 +241,7 @@ export default { </div> </div> <hr /> - <h4 class="my-4">{{ __('Pipelines charts') }}</h4> + <h4 class="gl-my-4">{{ __('Pipelines charts') }}</h4> <pipelines-area-chart v-for="(chart, index) in areaCharts" :key="index" diff --git a/app/assets/javascripts/projects/pipelines/charts/components/app_legacy.vue b/app/assets/javascripts/projects/pipelines/charts/components/app_legacy.vue new file mode 100644 index 00000000000000..c6e2b2e1140ec0 --- /dev/null +++ b/app/assets/javascripts/projects/pipelines/charts/components/app_legacy.vue @@ -0,0 +1,151 @@ +<script> +import dateFormat from 'dateformat'; +import { GlColumnChart } from '@gitlab/ui/dist/charts'; +import { __, sprintf } from '~/locale'; +import { getDateInPast } from '~/lib/utils/datetime_utility'; +import StatisticsList from './statistics_list.vue'; +import PipelinesAreaChart from './pipelines_area_chart.vue'; +import { + CHART_CONTAINER_HEIGHT, + INNER_CHART_HEIGHT, + X_AXIS_LABEL_ROTATION, + X_AXIS_TITLE_OFFSET, + CHART_DATE_FORMAT, + ONE_WEEK_AGO_DAYS, + ONE_MONTH_AGO_DAYS, +} from '../constants'; + +export default { + components: { + StatisticsList, + GlColumnChart, + PipelinesAreaChart, + }, + props: { + counts: { + type: Object, + required: true, + }, + timesChartData: { + type: Object, + required: true, + }, + lastWeekChartData: { + type: Object, + required: true, + }, + lastMonthChartData: { + type: Object, + required: true, + }, + lastYearChartData: { + type: Object, + required: true, + }, + }, + data() { + return { + timesChartTransformedData: [ + { + name: 'full', + data: this.mergeLabelsAndValues(this.timesChartData.labels, this.timesChartData.values), + }, + ], + }; + }, + computed: { + areaCharts() { + const { lastWeek, lastMonth, lastYear } = this.$options.chartTitles; + + return [ + this.buildAreaChartData(lastWeek, this.lastWeekChartData), + this.buildAreaChartData(lastMonth, this.lastMonthChartData), + this.buildAreaChartData(lastYear, this.lastYearChartData), + ]; + }, + }, + methods: { + mergeLabelsAndValues(labels, values) { + return labels.map((label, index) => [label, values[index]]); + }, + buildAreaChartData(title, data) { + const { labels, totals, success } = data; + + return { + title, + data: [ + { + name: 'all', + data: this.mergeLabelsAndValues(labels, totals), + }, + { + name: 'success', + data: this.mergeLabelsAndValues(labels, success), + }, + ], + }; + }, + }, + chartContainerHeight: CHART_CONTAINER_HEIGHT, + timesChartOptions: { + height: INNER_CHART_HEIGHT, + xAxis: { + axisLabel: { + rotate: X_AXIS_LABEL_ROTATION, + }, + nameGap: X_AXIS_TITLE_OFFSET, + }, + }, + get chartTitles() { + const today = dateFormat(new Date(), CHART_DATE_FORMAT); + const pastDate = timeScale => + dateFormat(getDateInPast(new Date(), timeScale), CHART_DATE_FORMAT); + return { + lastWeek: sprintf(__('Pipelines for last week (%{oneWeekAgo} - %{today})'), { + oneWeekAgo: pastDate(ONE_WEEK_AGO_DAYS), + today, + }), + lastMonth: sprintf(__('Pipelines for last month (%{oneMonthAgo} - %{today})'), { + oneMonthAgo: pastDate(ONE_MONTH_AGO_DAYS), + today, + }), + lastYear: __('Pipelines for last year'), + }; + }, +}; +</script> +<template> + <div> + <div class="mb-3"> + <h3>{{ s__('PipelineCharts|CI / CD Analytics') }}</h3> + </div> + <h4 class="my-4">{{ s__('PipelineCharts|Overall statistics') }}</h4> + <div class="row"> + <div class="col-md-6"> + <statistics-list :counts="counts" /> + </div> + <div class="col-md-6"> + <strong> + {{ __('Duration for the last 30 commits') }} + </strong> + <gl-column-chart + :height="$options.chartContainerHeight" + :option="$options.timesChartOptions" + :bars="timesChartTransformedData" + :y-axis-title="__('Minutes')" + :x-axis-title="__('Commit')" + x-axis-type="category" + /> + </div> + </div> + <hr /> + <h4 class="my-4">{{ __('Pipelines charts') }}</h4> + <pipelines-area-chart + v-for="(chart, index) in areaCharts" + :key="index" + :chart-data="chart.data" + > + {{ chart.title }} + </pipelines-area-chart> + </div> +</template> diff --git a/app/assets/javascripts/projects/pipelines/charts/components/statistics_list.vue b/app/assets/javascripts/projects/pipelines/charts/components/statistics_list.vue index aa59717ddcdc8f..94cecd2e479818 100644 --- a/app/assets/javascripts/projects/pipelines/charts/components/statistics_list.vue +++ b/app/assets/javascripts/projects/pipelines/charts/components/statistics_list.vue @@ -1,7 +1,10 @@ <script> import { formatTime } from '~/lib/utils/datetime_utility'; +import { SUPPORTED_FORMATS, getFormatter } from '~/lib/utils/unit_format'; import { s__, n__ } from '~/locale'; +const defaultPrecision = 2; + export default { props: { counts: { @@ -14,6 +17,8 @@ export default { return formatTime(this.counts.totalDuration); }, statistics() { + const formatter = getFormatter(SUPPORTED_FORMATS.percentHundred); + return [ { title: s__('PipelineCharts|Total:'), @@ -29,7 +34,7 @@ export default { }, { title: s__('PipelineCharts|Success ratio:'), - value: `${this.counts.successRatio}%`, + value: formatter(this.counts.successRatio, defaultPrecision), }, { title: s__('PipelineCharts|Total duration:'), diff --git a/app/assets/javascripts/projects/pipelines/charts/graphql/queries/get_pipeline_count_by_status.query.graphql b/app/assets/javascripts/projects/pipelines/charts/graphql/queries/get_pipeline_count_by_status.query.graphql new file mode 100644 index 00000000000000..eb0dbf8dd1602f --- /dev/null +++ b/app/assets/javascripts/projects/pipelines/charts/graphql/queries/get_pipeline_count_by_status.query.graphql @@ -0,0 +1,14 @@ +query getPipelineCountByStatus($projectPath: ID!) { + project(fullPath: $projectPath) { + totalPipelines: pipelines { + count + } + successfulPipelines: pipelines(status: SUCCESS) { + count + } + failedPipelines: pipelines(status: FAILED) { + count + } + totalPipelineDuration + } +} diff --git a/app/assets/javascripts/projects/pipelines/charts/graphql/queries/get_project_pipeline_statistics.query.graphql b/app/assets/javascripts/projects/pipelines/charts/graphql/queries/get_project_pipeline_statistics.query.graphql new file mode 100644 index 00000000000000..18b645f88312c5 --- /dev/null +++ b/app/assets/javascripts/projects/pipelines/charts/graphql/queries/get_project_pipeline_statistics.query.graphql @@ -0,0 +1,17 @@ +query getProjectPipelineStatistics($projectPath: ID!) { + project(fullPath: $projectPath) { + pipelineAnalytics { + weekPipelinesTotals + weekPipelinesLabels + weekPipelinesSuccessful + monthPipelinesLabels + monthPipelinesTotals + monthPipelinesSuccessful + yearPipelinesLabels + yearPipelinesTotals + yearPipelinesSuccessful + pipelineTimesLabels + pipelineTimesValues + } + } +} diff --git a/app/assets/javascripts/projects/pipelines/charts/index.js b/app/assets/javascripts/projects/pipelines/charts/index.js index eef1bc2d28bcc3..9229bb27f3d848 100644 --- a/app/assets/javascripts/projects/pipelines/charts/index.js +++ b/app/assets/javascripts/projects/pipelines/charts/index.js @@ -1,6 +1,15 @@ import Vue from 'vue'; +import VueApollo from 'vue-apollo'; +import createDefaultClient from '~/lib/graphql'; +import ProjectPipelinesChartsLegacy from './components/app_legacy.vue'; import ProjectPipelinesCharts from './components/app.vue'; +Vue.use(VueApollo); + +const apolloProvider = new VueApollo({ + defaultClient: createDefaultClient(), +}); + export default () => { const el = document.querySelector('#js-project-pipelines-charts-app'); const { @@ -20,6 +29,7 @@ export default () => { lastYearChartLabels, lastYearChartTotals, lastYearChartSuccess, + projectPath, } = el.dataset; const parseAreaChartData = (labels, totals, success) => ({ @@ -28,14 +38,29 @@ export default () => { success: JSON.parse(success), }); + if (gon.features.graphqlPipelineAnalytics) { + return new Vue({ + el, + name: 'ProjectPipelinesChartsApp', + components: { + ProjectPipelinesCharts, + }, + apolloProvider, + provide: { + projectPath, + }, + render: createElement => createElement(ProjectPipelinesCharts, {}), + }); + } + return new Vue({ el, name: 'ProjectPipelinesChartsApp', components: { - ProjectPipelinesCharts, + ProjectPipelinesChartsLegacy, }, render: createElement => - createElement(ProjectPipelinesCharts, { + createElement(ProjectPipelinesChartsLegacy, { props: { counts: { failed: countsFailed, diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb index 270fa056014e8e..0918325d214f59 100644 --- a/app/controllers/projects/pipelines_controller.rb +++ b/app/controllers/projects/pipelines_controller.rb @@ -17,6 +17,7 @@ class Projects::PipelinesController < Projects::ApplicationController push_frontend_feature_flag(:new_pipeline_form, project, default_enabled: true) push_frontend_feature_flag(:graphql_pipeline_header, project, type: :development, default_enabled: false) push_frontend_feature_flag(:graphql_pipeline_details, project, type: :development, default_enabled: false) + push_frontend_feature_flag(:graphql_pipeline_analytics, project, type: :development) push_frontend_feature_flag(:new_pipeline_form_prefilled_vars, project, type: :development) end before_action :ensure_pipeline, only: [:show] diff --git a/config/feature_flags/development/graphql_pipeline_analytics.yml b/config/feature_flags/development/graphql_pipeline_analytics.yml new file mode 100644 index 00000000000000..f91475fcbd7cb8 --- /dev/null +++ b/config/feature_flags/development/graphql_pipeline_analytics.yml @@ -0,0 +1,8 @@ +--- +name: graphql_pipeline_analytics +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/48267 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/290153 +milestone: '13.7' +type: development +group: group::continuos integration +default_enabled: false diff --git a/locale/gitlab.pot b/locale/gitlab.pot index cd34fa3beb3f1d..04bd99c22cb8b9 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -20113,6 +20113,12 @@ msgstr "" msgid "Pipeline: %{status}" msgstr "" +msgid "PipelineCharts|An error has ocurred when retrieving the analytics data" +msgstr "" + +msgid "PipelineCharts|An error has ocurred when retrieving the pipeline data" +msgstr "" + msgid "PipelineCharts|CI / CD Analytics" msgstr "" diff --git a/spec/frontend/projects/pipelines/charts/components/app_legacy_spec.js b/spec/frontend/projects/pipelines/charts/components/app_legacy_spec.js new file mode 100644 index 00000000000000..c03b571eb26116 --- /dev/null +++ b/spec/frontend/projects/pipelines/charts/components/app_legacy_spec.js @@ -0,0 +1,72 @@ +import { shallowMount } from '@vue/test-utils'; +import { GlColumnChart } from '@gitlab/ui/dist/charts'; +import Component from '~/projects/pipelines/charts/components/app_legacy.vue'; +import StatisticsList from '~/projects/pipelines/charts/components/statistics_list.vue'; +import PipelinesAreaChart from '~/projects/pipelines/charts/components/pipelines_area_chart.vue'; +import { + counts, + timesChartData, + areaChartData as lastWeekChartData, + areaChartData as lastMonthChartData, + lastYearChartData, +} from '../mock_data'; + +describe('ProjectsPipelinesChartsApp', () => { + let wrapper; + + beforeEach(() => { + wrapper = shallowMount(Component, { + propsData: { + counts, + timesChartData, + lastWeekChartData, + lastMonthChartData, + lastYearChartData, + }, + }); + }); + + afterEach(() => { + wrapper.destroy(); + wrapper = null; + }); + + describe('overall statistics', () => { + it('displays the statistics list', () => { + const list = wrapper.find(StatisticsList); + + expect(list.exists()).toBeTruthy(); + expect(list.props('counts')).toBe(counts); + }); + + it('displays the commit duration chart', () => { + const chart = wrapper.find(GlColumnChart); + + expect(chart.exists()).toBeTruthy(); + expect(chart.props('yAxisTitle')).toBe('Minutes'); + expect(chart.props('xAxisTitle')).toBe('Commit'); + expect(chart.props('bars')).toBe(wrapper.vm.timesChartTransformedData); + expect(chart.props('option')).toBe(wrapper.vm.$options.timesChartOptions); + }); + }); + + describe('pipelines charts', () => { + it('displays 3 area charts', () => { + expect(wrapper.findAll(PipelinesAreaChart).length).toBe(3); + }); + + describe('displays individual correctly', () => { + it('renders with the correct data', () => { + const charts = wrapper.findAll(PipelinesAreaChart); + + for (let i = 0; i < charts.length; i += 1) { + const chart = charts.at(i); + + expect(chart.exists()).toBeTruthy(); + expect(chart.props('chartData')).toBe(wrapper.vm.areaCharts[i].data); + expect(chart.text()).toBe(wrapper.vm.areaCharts[i].title); + } + }); + }); + }); +}); diff --git a/spec/frontend/projects/pipelines/charts/components/app_spec.js b/spec/frontend/projects/pipelines/charts/components/app_spec.js index 0dd3407dbbc6c0..568220d624294e 100644 --- a/spec/frontend/projects/pipelines/charts/components/app_spec.js +++ b/spec/frontend/projects/pipelines/charts/components/app_spec.js @@ -1,28 +1,36 @@ -import { shallowMount } from '@vue/test-utils'; +import { createLocalVue, shallowMount } from '@vue/test-utils'; +import VueApollo from 'vue-apollo'; +import createMockApollo from 'jest/helpers/mock_apollo_helper'; import { GlColumnChart } from '@gitlab/ui/dist/charts'; import Component from '~/projects/pipelines/charts/components/app.vue'; import StatisticsList from '~/projects/pipelines/charts/components/statistics_list.vue'; import PipelinesAreaChart from '~/projects/pipelines/charts/components/pipelines_area_chart.vue'; -import { - counts, - timesChartData, - areaChartData as lastWeekChartData, - areaChartData as lastMonthChartData, - lastYearChartData, -} from '../mock_data'; +import getPipelineCountByStatus from '~/projects/pipelines/charts/graphql/queries/get_pipeline_count_by_status.query.graphql'; +import getProjectPipelineStatistics from '~/projects/pipelines/charts/graphql/queries/get_project_pipeline_statistics.query.graphql'; +import { mockPipelineCount, mockPipelineStatistics } from '../mock_data'; + +const projectPath = 'gitlab-org/gitlab'; +const localVue = createLocalVue(); +localVue.use(VueApollo); describe('ProjectsPipelinesChartsApp', () => { let wrapper; + let fakeApollo; beforeEach(() => { + const requestHandlers = [ + [getPipelineCountByStatus, jest.fn().mockResolvedValue(mockPipelineCount)], + [getProjectPipelineStatistics, jest.fn().mockResolvedValue(mockPipelineStatistics)], + ]; + + fakeApollo = createMockApollo(requestHandlers); + wrapper = shallowMount(Component, { - propsData: { - counts, - timesChartData, - lastWeekChartData, - lastMonthChartData, - lastYearChartData, + provide: { + projectPath, }, + localVue, + apolloProvider: fakeApollo, }); }); @@ -35,14 +43,20 @@ describe('ProjectsPipelinesChartsApp', () => { it('displays the statistics list', () => { const list = wrapper.find(StatisticsList); - expect(list.exists()).toBeTruthy(); - expect(list.props('counts')).toBe(counts); + expect(list.exists()).toBe(true); + expect(list.props('counts')).toMatchObject({ + failed: 1, + success: 23, + total: 34, + successRatio: 95.83333333333334, + totalDuration: 2471, + }); }); it('displays the commit duration chart', () => { const chart = wrapper.find(GlColumnChart); - expect(chart.exists()).toBeTruthy(); + expect(chart.exists()).toBe(true); expect(chart.props('yAxisTitle')).toBe('Minutes'); expect(chart.props('xAxisTitle')).toBe('Commit'); expect(chart.props('bars')).toBe(wrapper.vm.timesChartTransformedData); @@ -52,7 +66,7 @@ describe('ProjectsPipelinesChartsApp', () => { describe('pipelines charts', () => { it('displays 3 area charts', () => { - expect(wrapper.findAll(PipelinesAreaChart).length).toBe(3); + expect(wrapper.findAll(PipelinesAreaChart)).toHaveLength(3); }); describe('displays individual correctly', () => { @@ -62,7 +76,7 @@ describe('ProjectsPipelinesChartsApp', () => { for (let i = 0; i < charts.length; i += 1) { const chart = charts.at(i); - expect(chart.exists()).toBeTruthy(); + expect(chart.exists()).toBe(true); expect(chart.props('chartData')).toBe(wrapper.vm.areaCharts[i].data); expect(chart.text()).toBe(wrapper.vm.areaCharts[i].title); } diff --git a/spec/frontend/projects/pipelines/charts/components/statistics_list_spec.js b/spec/frontend/projects/pipelines/charts/components/statistics_list_spec.js index f78608e9cb2556..4e79f62ce81406 100644 --- a/spec/frontend/projects/pipelines/charts/components/statistics_list_spec.js +++ b/spec/frontend/projects/pipelines/charts/components/statistics_list_spec.js @@ -18,7 +18,7 @@ describe('StatisticsList', () => { wrapper = null; }); - it('matches the snapshot', () => { + it('displays the counts data with labels', () => { expect(wrapper.element).toMatchSnapshot(); }); }); diff --git a/spec/frontend/projects/pipelines/charts/mock_data.js b/spec/frontend/projects/pipelines/charts/mock_data.js index 84e0ccb828ada9..da055536fcc27f 100644 --- a/spec/frontend/projects/pipelines/charts/mock_data.js +++ b/spec/frontend/projects/pipelines/charts/mock_data.js @@ -32,3 +32,218 @@ export const transformedAreaChartData = [ data: [['01 Jan', 3], ['02 Jan', 3], ['03 Jan', 3], ['04 Jan', 3], ['05 Jan', 5]], }, ]; + +export const mockPipelineCount = { + data: { + project: { + totalPipelines: { count: 34, __typename: 'PipelineConnection' }, + successfulPipelines: { count: 23, __typename: 'PipelineConnection' }, + failedPipelines: { count: 1, __typename: 'PipelineConnection' }, + totalPipelineDuration: 2471, + __typename: 'Project', + }, + }, +}; + +export const mockPipelineStatistics = { + data: { + project: { + pipelineAnalytics: { + weekPipelinesTotals: [0, 0, 0, 0, 0, 0, 0, 0], + weekPipelinesLabels: [ + '24 November', + '25 November', + '26 November', + '27 November', + '28 November', + '29 November', + '30 November', + '01 December', + ], + weekPipelinesSuccessful: [0, 0, 0, 0, 0, 0, 0, 0], + monthPipelinesLabels: [ + '01 November', + '02 November', + '03 November', + '04 November', + '05 November', + '06 November', + '07 November', + '08 November', + '09 November', + '10 November', + '11 November', + '12 November', + '13 November', + '14 November', + '15 November', + '16 November', + '17 November', + '18 November', + '19 November', + '20 November', + '21 November', + '22 November', + '23 November', + '24 November', + '25 November', + '26 November', + '27 November', + '28 November', + '29 November', + '30 November', + '01 December', + ], + monthPipelinesTotals: [ + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 2, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + ], + monthPipelinesSuccessful: [ + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + ], + yearPipelinesLabels: [ + 'December 2019', + 'January 2020', + 'February 2020', + 'March 2020', + 'April 2020', + 'May 2020', + 'June 2020', + 'July 2020', + 'August 2020', + 'September 2020', + 'October 2020', + 'November 2020', + 'December 2020', + ], + yearPipelinesTotals: [0, 0, 0, 0, 0, 0, 0, 0, 23, 7, 2, 2, 0], + yearPipelinesSuccessful: [0, 0, 0, 0, 0, 0, 0, 0, 17, 5, 1, 0, 0], + pipelineTimesLabels: [ + 'b3781247', + 'b3781247', + 'a50ba059', + '8e414f3b', + 'b2964d50', + '7caa525b', + '761b164e', + 'd3eccd18', + 'e2750f63', + 'e2750f63', + '1dfb4b96', + 'b49d6f94', + '66fa2f80', + 'e2750f63', + 'fc82cf15', + '19fb20b2', + '25f03a24', + 'e054110f', + '0278b7b2', + '38478c16', + '38478c16', + '38478c16', + '1fb2103e', + '97b99fb5', + '8abc6e87', + 'c94e80e3', + '5d349a50', + '5d349a50', + '9c581037', + '02d95fb2', + ], + pipelineTimesValues: [ + 1, + 0, + 0, + 0, + 0, + 1, + 1, + 2, + 1, + 0, + 1, + 2, + 2, + 0, + 4, + 2, + 1, + 2, + 1, + 1, + 0, + 1, + 1, + 0, + 1, + 5, + 2, + 0, + 0, + 0, + ], + __typename: 'Analytics', + }, + __typename: 'Project', + }, + }, +}; -- GitLab From 8235198133e143d83a3db5697cab6c3fc7f4c59a Mon Sep 17 00:00:00 2001 From: Jose Vargas <jvargas@gitlab.com> Date: Mon, 7 Dec 2020 13:57:34 -0600 Subject: [PATCH 2/2] Refactor tests and improve error handling This improves on the existing error handling to use the GlAlert component. This also adds comments to remove/refactor the parts that will be affected once the feature flag rolls out --- .../pipelines/charts/components/app.vue | 90 +++++++++++++++---- .../projects/pipelines/charts/constants.js | 6 ++ .../projects/pipelines/charts/index.js | 36 ++++++-- app/views/projects/pipelines/charts.html.haml | 13 +-- locale/gitlab.pot | 8 +- .../statistics_list_spec.js.snap | 4 +- .../pipelines/charts/components/app_spec.js | 18 +++- 7 files changed, 136 insertions(+), 39 deletions(-) diff --git a/app/assets/javascripts/projects/pipelines/charts/components/app.vue b/app/assets/javascripts/projects/pipelines/charts/components/app.vue index e89b5c76e38594..8b5bed08931ef0 100644 --- a/app/assets/javascripts/projects/pipelines/charts/components/app.vue +++ b/app/assets/javascripts/projects/pipelines/charts/components/app.vue @@ -1,8 +1,8 @@ <script> import dateFormat from 'dateformat'; import { GlColumnChart } from '@gitlab/ui/dist/charts'; +import { GlAlert } from '@gitlab/ui'; import { __, s__, sprintf } from '~/locale'; -import createFlash, { FLASH_TYPES } from '~/flash'; import { getDateInPast } from '~/lib/utils/datetime_utility'; import getPipelineCountByStatus from '../graphql/queries/get_pipeline_count_by_status.query.graphql'; import getProjectPipelineStatistics from '../graphql/queries/get_project_pipeline_statistics.query.graphql'; @@ -11,9 +11,14 @@ import PipelinesAreaChart from './pipelines_area_chart.vue'; import { CHART_CONTAINER_HEIGHT, CHART_DATE_FORMAT, + DEFAULT, INNER_CHART_HEIGHT, + LOAD_ANALYTICS_FAILURE, + LOAD_PIPELINES_FAILURE, ONE_WEEK_AGO_DAYS, ONE_MONTH_AGO_DAYS, + PARSE_FAILURE, + UNSUPPORTED_DATA, X_AXIS_LABEL_ROTATION, X_AXIS_TITLE_OFFSET, } from '../constants'; @@ -43,6 +48,7 @@ const defaultAnalyticsValues = { export default { components: { + GlAlert, GlColumnChart, StatisticsList, PipelinesAreaChart, @@ -61,6 +67,8 @@ export default { analytics: { ...defaultAnalyticsValues, }, + showFailureAlert: false, + failureType: null, }; }, apollo: { @@ -71,14 +79,11 @@ export default { projectPath: this.projectPath, }; }, - update(res) { - return res.project; + update(data) { + return data?.project; }, error() { - createFlash({ - message: s__('PipelineCharts|An error has ocurred when retrieving the pipeline data'), - type: FLASH_TYPES.ALERT, - }); + this.reportFailure(LOAD_PIPELINES_FAILURE); }, }, analytics: { @@ -88,18 +93,39 @@ export default { projectPath: this.projectPath, }; }, - update(res) { - return res.project.pipelineAnalytics; + update(data) { + return data?.project?.pipelineAnalytics; }, error() { - createFlash({ - message: s__('PipelineCharts|An error has ocurred when retrieving the analytics data'), - type: FLASH_TYPES.ALERT, - }); + this.reportFailure(LOAD_ANALYTICS_FAILURE); }, }, }, computed: { + failure() { + switch (this.failureType) { + case LOAD_ANALYTICS_FAILURE: + return { + text: this.$options.errorTexts[LOAD_ANALYTICS_FAILURE], + variant: 'danger', + }; + case PARSE_FAILURE: + return { + text: this.$options.errorTexts[PARSE_FAILURE], + variant: 'danger', + }; + case UNSUPPORTED_DATA: + return { + text: this.$options.errorTexts[UNSUPPORTED_DATA], + variant: 'info', + }; + default: + return { + text: this.$options.errorTexts[DEFAULT], + variant: 'danger', + }; + } + }, successRatio() { const { successfulPipelines, failedPipelines } = this.counts; const successfulCount = successfulPipelines?.count; @@ -126,12 +152,20 @@ export default { }, areaCharts() { const { lastWeek, lastMonth, lastYear } = this.$options.chartTitles; + let areaChartsData = []; - return [ - this.buildAreaChartData(lastWeek, this.lastWeekChartData), - this.buildAreaChartData(lastMonth, this.lastMonthChartData), - this.buildAreaChartData(lastYear, this.lastYearChartData), - ]; + try { + areaChartsData = [ + this.buildAreaChartData(lastWeek, this.lastWeekChartData), + this.buildAreaChartData(lastMonth, this.lastMonthChartData), + this.buildAreaChartData(lastYear, this.lastYearChartData), + ]; + } catch { + areaChartsData = []; + this.reportFailure(PARSE_FAILURE); + } + + return areaChartsData; }, lastWeekChartData() { return { @@ -187,6 +221,13 @@ export default { ], }; }, + hideAlert() { + this.showFailureAlert = false; + }, + reportFailure(type) { + this.showFailureAlert = true; + this.failureType = type; + }, }, chartContainerHeight: CHART_CONTAINER_HEIGHT, timesChartOptions: { @@ -198,6 +239,16 @@ export default { nameGap: X_AXIS_TITLE_OFFSET, }, }, + errorTexts: { + [LOAD_ANALYTICS_FAILURE]: s__( + 'PipelineCharts|An error has ocurred when retrieving the analytics data', + ), + [LOAD_PIPELINES_FAILURE]: s__( + 'PipelineCharts|An error has ocurred when retrieving the pipelines data', + ), + [PARSE_FAILURE]: s__('PipelineCharts|There was an error parsing the data for the charts.'), + [DEFAULT]: s__('PipelineCharts|An unknown error occurred while processing CI/CD analytics.'), + }, get chartTitles() { const today = dateFormat(new Date(), CHART_DATE_FORMAT); const pastDate = timeScale => @@ -218,6 +269,9 @@ export default { </script> <template> <div> + <gl-alert v-if="showFailureAlert" :variant="failure.variant" @dismiss="hideAlert"> + {{ failure.text }} + </gl-alert> <div class="gl-mb-3"> <h3>{{ s__('PipelineCharts|CI / CD Analytics') }}</h3> </div> diff --git a/app/assets/javascripts/projects/pipelines/charts/constants.js b/app/assets/javascripts/projects/pipelines/charts/constants.js index 5dbe3c01100c8f..079e23943c135d 100644 --- a/app/assets/javascripts/projects/pipelines/charts/constants.js +++ b/app/assets/javascripts/projects/pipelines/charts/constants.js @@ -11,3 +11,9 @@ export const ONE_WEEK_AGO_DAYS = 7; export const ONE_MONTH_AGO_DAYS = 31; export const CHART_DATE_FORMAT = 'dd mmm'; + +export const DEFAULT = 'default'; +export const PARSE_FAILURE = 'parse_failure'; +export const LOAD_ANALYTICS_FAILURE = 'load_analytics_failure'; +export const LOAD_PIPELINES_FAILURE = 'load_analytics_failure'; +export const UNSUPPORTED_DATA = 'unsupported_data'; diff --git a/app/assets/javascripts/projects/pipelines/charts/index.js b/app/assets/javascripts/projects/pipelines/charts/index.js index 9229bb27f3d848..f6e79f0ab51489 100644 --- a/app/assets/javascripts/projects/pipelines/charts/index.js +++ b/app/assets/javascripts/projects/pipelines/charts/index.js @@ -10,8 +10,11 @@ const apolloProvider = new VueApollo({ defaultClient: createDefaultClient(), }); -export default () => { - const el = document.querySelector('#js-project-pipelines-charts-app'); +const mountPipelineChartsApp = el => { + // Not all of the values will be defined since some them will be + // empty depending on the value of the graphql_pipeline_analytics + // feature flag, once the rollout of the feature flag is completed + // the undefined values will be deleted const { countsFailed, countsSuccess, @@ -32,13 +35,23 @@ export default () => { projectPath, } = el.dataset; - const parseAreaChartData = (labels, totals, success) => ({ - labels: JSON.parse(labels), - totals: JSON.parse(totals), - success: JSON.parse(success), - }); + const parseAreaChartData = (labels, totals, success) => { + let parsedData = {}; + + try { + parsedData = { + labels: JSON.parse(labels), + totals: JSON.parse(totals), + success: JSON.parse(success), + }; + } catch { + parsedData = {}; + } + + return parsedData; + }; - if (gon.features.graphqlPipelineAnalytics) { + if (gon?.features?.graphqlPipelineAnalytics) { return new Vue({ el, name: 'ProjectPipelinesChartsApp', @@ -55,7 +68,7 @@ export default () => { return new Vue({ el, - name: 'ProjectPipelinesChartsApp', + name: 'ProjectPipelinesChartsAppLegacy', components: { ProjectPipelinesChartsLegacy, }, @@ -92,3 +105,8 @@ export default () => { }), }); }; + +export default () => { + const el = document.querySelector('#js-project-pipelines-charts-app'); + return !el ? {} : mountPipelineChartsApp(el); +}; diff --git a/app/views/projects/pipelines/charts.html.haml b/app/views/projects/pipelines/charts.html.haml index 55f1b9098c3365..f3360e150ad124 100644 --- a/app/views/projects/pipelines/charts.html.haml +++ b/app/views/projects/pipelines/charts.html.haml @@ -1,7 +1,10 @@ - page_title _('CI / CD Analytics') -#js-project-pipelines-charts-app{ data: { counts: @counts, success_ratio: success_ratio(@counts), - times_chart: { labels: @charts[:pipeline_times].labels, values: @charts[:pipeline_times].pipeline_times }, - last_week_chart: { labels: @charts[:week].labels, totals: @charts[:week].total, success: @charts[:week].success }, - last_month_chart: { labels: @charts[:month].labels, totals: @charts[:month].total, success: @charts[:month].success }, - last_year_chart: { labels: @charts[:year].labels, totals: @charts[:year].total, success: @charts[:year].success } } } +- if Feature.enabled?(:graphql_pipeline_analytics) + #js-project-pipelines-charts-app{ data: { project_path: @project.full_path } } +- else + #js-project-pipelines-charts-app{ data: { counts: @counts, success_ratio: success_ratio(@counts), + times_chart: { labels: @charts[:pipeline_times].labels, values: @charts[:pipeline_times].pipeline_times }, + last_week_chart: { labels: @charts[:week].labels, totals: @charts[:week].total, success: @charts[:week].success }, + last_month_chart: { labels: @charts[:month].labels, totals: @charts[:month].total, success: @charts[:month].success }, + last_year_chart: { labels: @charts[:year].labels, totals: @charts[:year].total, success: @charts[:year].success } } } diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 04bd99c22cb8b9..ff177e79a2fc15 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -20116,7 +20116,10 @@ msgstr "" msgid "PipelineCharts|An error has ocurred when retrieving the analytics data" msgstr "" -msgid "PipelineCharts|An error has ocurred when retrieving the pipeline data" +msgid "PipelineCharts|An error has ocurred when retrieving the pipelines data" +msgstr "" + +msgid "PipelineCharts|An unknown error occurred while processing CI/CD analytics." msgstr "" msgid "PipelineCharts|CI / CD Analytics" @@ -20134,6 +20137,9 @@ msgstr "" msgid "PipelineCharts|Successful:" msgstr "" +msgid "PipelineCharts|There was an error parsing the data for the charts." +msgstr "" + msgid "PipelineCharts|Total duration:" msgstr "" diff --git a/spec/frontend/projects/pipelines/charts/components/__snapshots__/statistics_list_spec.js.snap b/spec/frontend/projects/pipelines/charts/components/__snapshots__/statistics_list_spec.js.snap index ac87fe893b98ee..c7e760486c0b05 100644 --- a/spec/frontend/projects/pipelines/charts/components/__snapshots__/statistics_list_spec.js.snap +++ b/spec/frontend/projects/pipelines/charts/components/__snapshots__/statistics_list_spec.js.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`StatisticsList matches the snapshot 1`] = ` +exports[`StatisticsList displays the counts data with labels 1`] = ` <ul> <li> <span> @@ -35,7 +35,7 @@ exports[`StatisticsList matches the snapshot 1`] = ` </span> <strong> - 50% + 50.00% </strong> </li> <li> diff --git a/spec/frontend/projects/pipelines/charts/components/app_spec.js b/spec/frontend/projects/pipelines/charts/components/app_spec.js index 568220d624294e..f8737dda5f6391 100644 --- a/spec/frontend/projects/pipelines/charts/components/app_spec.js +++ b/spec/frontend/projects/pipelines/charts/components/app_spec.js @@ -15,23 +15,31 @@ localVue.use(VueApollo); describe('ProjectsPipelinesChartsApp', () => { let wrapper; - let fakeApollo; - beforeEach(() => { + function createMockApolloProvider() { const requestHandlers = [ [getPipelineCountByStatus, jest.fn().mockResolvedValue(mockPipelineCount)], [getProjectPipelineStatistics, jest.fn().mockResolvedValue(mockPipelineStatistics)], ]; - fakeApollo = createMockApollo(requestHandlers); + return createMockApollo(requestHandlers); + } + + function createComponent(options = {}) { + const { fakeApollo } = options; - wrapper = shallowMount(Component, { + return shallowMount(Component, { provide: { projectPath, }, localVue, apolloProvider: fakeApollo, }); + } + + beforeEach(() => { + const fakeApollo = createMockApolloProvider(); + wrapper = createComponent({ fakeApollo }); }); afterEach(() => { @@ -77,6 +85,8 @@ describe('ProjectsPipelinesChartsApp', () => { const chart = charts.at(i); expect(chart.exists()).toBe(true); + // TODO: Refactor this to use the mocked data instead of the vm data + // https://gitlab.com/gitlab-org/gitlab/-/issues/292085 expect(chart.props('chartData')).toBe(wrapper.vm.areaCharts[i].data); expect(chart.text()).toBe(wrapper.vm.areaCharts[i].title); } -- GitLab