diff --git a/ee/app/assets/javascripts/analytics/productivity_analytics/components/app.vue b/ee/app/assets/javascripts/analytics/productivity_analytics/components/app.vue index f7a9dedff724df4307509f4d9a8533a958dedaa9..f10d7550cf28c8936c919db635dc4064c64be2f0 100644 --- a/ee/app/assets/javascripts/analytics/productivity_analytics/components/app.vue +++ b/ee/app/assets/javascripts/analytics/productivity_analytics/components/app.vue @@ -9,6 +9,7 @@ import { GlTooltipDirective, } from '@gitlab/ui'; import { GlColumnChart } from '@gitlab/ui/dist/charts'; +import featureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import Icon from '~/vue_shared/components/icon.vue'; import MetricChart from './metric_chart.vue'; import Scatterplot from '../../shared/components/scatterplot.vue'; @@ -31,6 +32,7 @@ export default { directives: { GlTooltip: GlTooltipDirective, }, + mixins: [featureFlagsMixin()], props: { endpoint: { type: String, @@ -65,6 +67,7 @@ export default { 'getSelectedMetric', 'scatterplotYaxisLabel', 'hasNoAccessError', + 'isChartEnabled', ]), ...mapGetters('table', [ 'sortFieldDropdownLabel', @@ -89,10 +92,19 @@ export default { }, mounted() { this.setEndpoint(this.endpoint); + this.setChartEnabled({ + chartKey: chartKeys.scatterplot, + isEnabled: this.isScatterplotFeatureEnabled(), + }); }, methods: { ...mapActions(['setEndpoint']), - ...mapActions('charts', ['fetchChartData', 'setMetricType', 'chartItemClicked']), + ...mapActions('charts', [ + 'fetchChartData', + 'setMetricType', + 'chartItemClicked', + 'setChartEnabled', + ]), ...mapActions('table', ['setSortField', 'setPage', 'toggleSortOrder', 'setColumnMetric']), onMainChartItemClicked({ params }) { const itemValue = params.data.value[0]; @@ -108,6 +120,9 @@ export default { ...this.getColumnChartDatazoomOption(chartKey), }; }, + isScatterplotFeatureEnabled() { + return this.glFeatures.productivityAnalyticsScatterplotEnabled; + }, }, }; </script> @@ -217,6 +232,7 @@ export default { </div> <metric-chart + v-if="isChartEnabled(chartKeys.scatterplot)" ref="scatterplot" class="mb-4" :title="s__('ProductivityAnalytics|Trendline')" diff --git a/ee/app/assets/javascripts/analytics/productivity_analytics/store/modules/charts/actions.js b/ee/app/assets/javascripts/analytics/productivity_analytics/store/modules/charts/actions.js index 9bc4fa88846f943471d12efa330cdee70936e39d..0a7cf309cc83688126b61eb04c2d59d3e9392419 100644 --- a/ee/app/assets/javascripts/analytics/productivity_analytics/store/modules/charts/actions.js +++ b/ee/app/assets/javascripts/analytics/productivity_analytics/store/modules/charts/actions.js @@ -16,18 +16,21 @@ export const fetchSecondaryChartData = ({ state, dispatch }) => { export const requestChartData = ({ commit }, chartKey) => commit(types.REQUEST_CHART_DATA, chartKey); -export const fetchChartData = ({ dispatch, getters, rootState }, chartKey) => { - dispatch('requestChartData', chartKey); - - const params = getters.getFilterParams(chartKey); - - return axios - .get(rootState.endpoint, { params }) - .then(response => { - const { data } = response; - dispatch('receiveChartDataSuccess', { chartKey, data }); - }) - .catch(error => dispatch('receiveChartDataError', { chartKey, error })); +export const fetchChartData = ({ dispatch, getters, state, rootState }, chartKey) => { + // let's fetch data for enabled charts only + if (state.charts[chartKey].enabled) { + dispatch('requestChartData', chartKey); + + const params = getters.getFilterParams(chartKey); + + axios + .get(rootState.endpoint, { params }) + .then(response => { + const { data } = response; + dispatch('receiveChartDataSuccess', { chartKey, data }); + }) + .catch(error => dispatch('receiveChartDataError', { chartKey, error })); + } }; export const receiveChartDataSuccess = ({ commit }, { chartKey, data = {} }) => { @@ -57,5 +60,8 @@ export const chartItemClicked = ({ commit, dispatch }, { chartKey, item }) => { dispatch('table/setPage', 0, { root: true }); }; +export const setChartEnabled = ({ commit }, { chartKey, isEnabled }) => + commit(types.SET_CHART_ENABLED, { chartKey, isEnabled }); + // prevent babel-plugin-rewire from generating an invalid default during karma tests export default () => {}; diff --git a/ee/app/assets/javascripts/analytics/productivity_analytics/store/modules/charts/getters.js b/ee/app/assets/javascripts/analytics/productivity_analytics/store/modules/charts/getters.js index 6f8398106fff0b59aa9cf8ac1f731c1ee2bac628..e466b32e8895534bf56de7afa2544d3846d14dab 100644 --- a/ee/app/assets/javascripts/analytics/productivity_analytics/store/modules/charts/getters.js +++ b/ee/app/assets/javascripts/analytics/productivity_analytics/store/modules/charts/getters.js @@ -169,5 +169,7 @@ export const scatterplotYaxisLabel = (_state, getters, rootState) => { export const hasNoAccessError = state => state.charts[chartKeys.main].errorCode === httpStatus.FORBIDDEN; +export const isChartEnabled = state => chartKey => state.charts[chartKey].enabled; + // prevent babel-plugin-rewire from generating an invalid default during karma tests export default () => {}; diff --git a/ee/app/assets/javascripts/analytics/productivity_analytics/store/modules/charts/mutation_types.js b/ee/app/assets/javascripts/analytics/productivity_analytics/store/modules/charts/mutation_types.js index a341470bbfc7da5a42ccd9c674265f6ac32c9c47..089d0bebac886b1b0520474fd426819fcee4ced0 100644 --- a/ee/app/assets/javascripts/analytics/productivity_analytics/store/modules/charts/mutation_types.js +++ b/ee/app/assets/javascripts/analytics/productivity_analytics/store/modules/charts/mutation_types.js @@ -5,3 +5,4 @@ export const RECEIVE_CHART_DATA_ERROR = 'RECEIVE_CHART_DATA_ERROR'; export const SET_METRIC_TYPE = 'SET_METRIC_TYPE'; export const UPDATE_SELECTED_CHART_ITEMS = 'UPDATE_SELECTED_CHART_ITEMS'; +export const SET_CHART_ENABLED = 'SET_CHART_ENABLED'; diff --git a/ee/app/assets/javascripts/analytics/productivity_analytics/store/modules/charts/mutations.js b/ee/app/assets/javascripts/analytics/productivity_analytics/store/modules/charts/mutations.js index 92d9f7fdc64a5748cddfafa9287a1aefac690dfb..bd2e97e42095f2cdcc59cc57d6b752efdea62011 100644 --- a/ee/app/assets/javascripts/analytics/productivity_analytics/store/modules/charts/mutations.js +++ b/ee/app/assets/javascripts/analytics/productivity_analytics/store/modules/charts/mutations.js @@ -29,4 +29,7 @@ export default { state.charts[chartKey].selected.splice(idx, 1); } }, + [types.SET_CHART_ENABLED](state, { chartKey, isEnabled }) { + state.charts[chartKey].enabled = isEnabled; + }, }; diff --git a/ee/app/assets/javascripts/analytics/productivity_analytics/store/modules/charts/state.js b/ee/app/assets/javascripts/analytics/productivity_analytics/store/modules/charts/state.js index 66d78bd64a53e75dc011cbea25259acf1244f077..c6d02ea08aee76f374da7cd2b7d4683776eb657d 100644 --- a/ee/app/assets/javascripts/analytics/productivity_analytics/store/modules/charts/state.js +++ b/ee/app/assets/javascripts/analytics/productivity_analytics/store/modules/charts/state.js @@ -5,6 +5,7 @@ export default () => ({ [chartKeys.main]: { isLoading: false, errorCode: null, + enabled: true, data: {}, selected: [], params: { @@ -14,6 +15,7 @@ export default () => ({ [chartKeys.timeBasedHistogram]: { isLoading: false, errorCode: null, + enabled: true, data: {}, selected: [], params: { @@ -24,6 +26,7 @@ export default () => ({ [chartKeys.commitBasedHistogram]: { isLoading: false, errorCode: null, + enabled: true, data: {}, selected: [], params: { @@ -34,6 +37,7 @@ export default () => ({ [chartKeys.scatterplot]: { isLoading: false, errorCode: null, + enabled: true, data: {}, selected: [], params: { diff --git a/ee/app/controllers/analytics/productivity_analytics_controller.rb b/ee/app/controllers/analytics/productivity_analytics_controller.rb index 6b10a3980786433b1ca67bc2c148f63d4cc2bb1c..2ecd1d1739836f0fd03e50c3062d8b4b6ca3e000 100644 --- a/ee/app/controllers/analytics/productivity_analytics_controller.rb +++ b/ee/app/controllers/analytics/productivity_analytics_controller.rb @@ -13,6 +13,9 @@ class Analytics::ProductivityAnalyticsController < Analytics::ApplicationControl before_action -> { authorize_view_productivity_analytics!(:view_productivity_analytics) } + before_action -> { + push_frontend_feature_flag(:productivity_analytics_scatterplot_enabled, default_enabled: true) + } include IssuableCollections diff --git a/ee/spec/frontend/analytics/productivity_analytics/components/app_spec.js b/ee/spec/frontend/analytics/productivity_analytics/components/app_spec.js index b7db4a486502218d9e83373a80f2221af7e7fff9..9281ccd251ef3ca78b16b5dd8a86a55c974db0dc 100644 --- a/ee/spec/frontend/analytics/productivity_analytics/components/app_spec.js +++ b/ee/spec/frontend/analytics/productivity_analytics/components/app_spec.js @@ -34,8 +34,7 @@ describe('ProductivityApp component', () => { const mainChartData = { 1: 2, 2: 3 }; - beforeEach(() => { - mock = new MockAdapter(axios); + const createComponent = (scatterplotEnabled = true) => { wrapper = shallowMount(localVue.extend(ProductivityApp), { localVue, store, @@ -44,7 +43,14 @@ describe('ProductivityApp component', () => { methods: { ...actionSpies, }, + provide: { + glFeatures: { productivityAnalyticsScatterplotEnabled: scatterplotEnabled }, + }, }); + }; + + beforeEach(() => { + mock = new MockAdapter(axios); }); afterEach(() => { @@ -66,6 +72,7 @@ describe('ProductivityApp component', () => { describe('template', () => { describe('without a group being selected', () => { it('renders the empty state illustration', () => { + createComponent(); const emptyState = wrapper.find(GlEmptyState); expect(emptyState.exists()).toBe(true); @@ -81,6 +88,7 @@ describe('ProductivityApp component', () => { describe('user has no access to the group', () => { beforeEach(() => { + createComponent(); const error = { response: { status: 403 } }; wrapper.vm.$store.dispatch('charts/receiveChartDataError', { chartKey: chartKeys.main, @@ -106,6 +114,7 @@ describe('ProductivityApp component', () => { describe('when the main chart is loading', () => { beforeEach(() => { + createComponent(); wrapper.vm.$store.dispatch('charts/requestChartData', chartKeys.main); }); @@ -130,6 +139,7 @@ describe('ProductivityApp component', () => { describe('when the main chart finished loading', () => { describe('and has data', () => { beforeEach(() => { + createComponent(); wrapper.vm.$store.dispatch('charts/receiveChartDataSuccess', { chartKey: chartKeys.main, data: mainChartData, @@ -252,50 +262,70 @@ describe('ProductivityApp component', () => { }); describe('Scatterplot', () => { - it('renders a metric chart component', () => { - expect(findScatterplotMetricChart().exists()).toBe(true); - }); + describe('when the feature flag is enabled', () => { + it('isScatterplotFeatureEnabled returns true', () => { + expect(wrapper.vm.isScatterplotFeatureEnabled()).toBe(true); + }); - describe('when chart finished loading', () => { - describe('and the chart has data', () => { - beforeEach(() => { - wrapper.vm.$store.dispatch('charts/receiveChartDataSuccess', { - chartKey: chartKeys.scatterplot, - data: { - 1: { metric: 2, merged_at: '2019-07-01T07:06:23.193Z' }, - 2: { metric: 3, merged_at: '2019-07-05T08:27:42.411Z' }, - }, - }); - }); + it('renders a metric chart component', () => { + expect(findScatterplotMetricChart().exists()).toBe(true); + }); - it('sets isLoading=false on the metric chart', () => { - expect(findScatterplotMetricChart().props('isLoading')).toBe(false); - }); + describe('when chart finished loading', () => { + describe('and the chart has data', () => { + beforeEach(() => { + wrapper.vm.$store.dispatch('charts/receiveChartDataSuccess', { + chartKey: chartKeys.scatterplot, + data: { + 1: { metric: 2, merged_at: '2019-07-01T07:06:23.193Z' }, + 2: { metric: 3, merged_at: '2019-07-05T08:27:42.411Z' }, + }, + }); + }); - it('passes non-empty chartData to the metric chart', () => { - expect(findScatterplotMetricChart().props('chartData')).not.toEqual([]); - }); + it('sets isLoading=false on the metric chart', () => { + expect(findScatterplotMetricChart().props('isLoading')).toBe(false); + }); - describe('when the user changes the metric', () => { - beforeEach(() => { - jest.spyOn(store, 'dispatch'); - findScatterplotMetricChart().vm.$emit('metricTypeChange', 'loc_per_commit'); + it('passes non-empty chartData to the metric chart', () => { + expect(findScatterplotMetricChart().props('chartData')).not.toEqual([]); }); - it('should call setMetricType when `metricTypeChange` is emitted on the metric chart', () => { - expect(store.dispatch).toHaveBeenCalledWith('charts/setMetricType', { - metricType: 'loc_per_commit', - chartKey: chartKeys.scatterplot, + describe('when the user changes the metric', () => { + beforeEach(() => { + jest.spyOn(store, 'dispatch'); + findScatterplotMetricChart().vm.$emit('metricTypeChange', 'loc_per_commit'); }); - }); - it("should update the chart's y axis label", () => { - const scatterplot = findScatterplotMetricChart().find(Scatterplot); - expect(scatterplot.props('yAxisTitle')).toBe('Number of LOCs per commit'); + it('should call setMetricType when `metricTypeChange` is emitted on the metric chart', () => { + expect(store.dispatch).toHaveBeenCalledWith('charts/setMetricType', { + metricType: 'loc_per_commit', + chartKey: chartKeys.scatterplot, + }); + }); + + it("should update the chart's y axis label", () => { + const scatterplot = findScatterplotMetricChart().find(Scatterplot); + expect(scatterplot.props('yAxisTitle')).toBe('Number of LOCs per commit'); + }); }); }); }); }); + + describe('when the feature flag is disabled', () => { + beforeEach(() => { + createComponent(false); + }); + + it('isScatterplotFeatureEnabled returns false', () => { + expect(wrapper.vm.isScatterplotFeatureEnabled()).toBe(false); + }); + + it("doesn't render a metric chart component", () => { + expect(findScatterplotMetricChart().exists()).toBe(false); + }); + }); }); describe('MR table', () => { @@ -400,6 +430,7 @@ describe('ProductivityApp component', () => { describe('and has no data', () => { beforeEach(() => { + createComponent(); wrapper.vm.$store.dispatch('charts/receiveChartDataSuccess', { chartKey: chartKeys.main, data: {}, diff --git a/ee/spec/frontend/analytics/productivity_analytics/store/modules/charts/actions_spec.js b/ee/spec/frontend/analytics/productivity_analytics/store/modules/charts/actions_spec.js index dd43f2e09feb48f2a1897e6fd20ab717699026b0..0c7f3efab7ed2cf88166f719deab8ffda08f8f54 100644 --- a/ee/spec/frontend/analytics/productivity_analytics/store/modules/charts/actions_spec.js +++ b/ee/spec/frontend/analytics/productivity_analytics/store/modules/charts/actions_spec.js @@ -47,62 +47,64 @@ describe('Productivity analytics chart actions', () => { }); describe('fetchChartData', () => { - describe('success', () => { - beforeEach(() => { - mock.onGet(mockedState.endpoint).replyOnce(200, mockHistogramData); - }); - - it('calls API with params', () => { - jest.spyOn(axios, 'get'); - - actions.fetchChartData(mockedContext, chartKey); - - expect(axios.get).toHaveBeenCalledWith(mockedState.endpoint, { params: globalParams }); - }); - - it('dispatches success with received data', done => - testAction( - actions.fetchChartData, - chartKey, - mockedState, - [], - [ - { type: 'requestChartData', payload: chartKey }, - { - type: 'receiveChartDataSuccess', - payload: expect.objectContaining({ chartKey, data: mockHistogramData }), - }, - ], - done, - )); - }); - - describe('error', () => { - beforeEach(() => { - mock.onGet(mockedState.endpoint).replyOnce(500); + describe('when chart is enabled', () => { + describe('success', () => { + beforeEach(() => { + mock.onGet(mockedState.endpoint).replyOnce(200, mockHistogramData); + }); + + it('calls API with params', () => { + jest.spyOn(axios, 'get'); + + actions.fetchChartData(mockedContext, chartKey); + + expect(axios.get).toHaveBeenCalledWith(mockedState.endpoint, { params: globalParams }); + }); + + it('dispatches success with received data', done => + testAction( + actions.fetchChartData, + chartKey, + mockedState, + [], + [ + { type: 'requestChartData', payload: chartKey }, + { + type: 'receiveChartDataSuccess', + payload: expect.objectContaining({ chartKey, data: mockHistogramData }), + }, + ], + done, + )); }); - it('dispatches error', done => { - testAction( - actions.fetchChartData, - chartKey, - mockedState, - [], - [ - { - type: 'requestChartData', - payload: chartKey, - }, - { - type: 'receiveChartDataError', - payload: { - chartKey, - error: new Error('Request failed with status code 500'), + describe('error', () => { + beforeEach(() => { + mock.onGet(mockedState.endpoint).replyOnce(500); + }); + + it('dispatches error', done => { + testAction( + actions.fetchChartData, + chartKey, + mockedState, + [], + [ + { + type: 'requestChartData', + payload: chartKey, }, - }, - ], - done, - ); + { + type: 'receiveChartDataError', + payload: { + chartKey, + error: new Error('Request failed with status code 500'), + }, + }, + ], + done, + ); + }); }); }); }); @@ -118,6 +120,24 @@ describe('Productivity analytics chart actions', () => { done, ); }); + + describe('when chart is disabled', () => { + const disabledChartKey = chartKeys.scatterplot; + beforeEach(() => { + mock.onGet(mockedState.endpoint).replyOnce(200); + mockedState.charts[disabledChartKey].enabled = false; + }); + + it('does not dispatch the requestChartData action', done => { + testAction(actions.fetchChartData, disabledChartKey, mockedState, [], [], done); + }); + + it('does not call the API', () => { + actions.fetchChartData(mockedContext, disabledChartKey); + jest.spyOn(axios, 'get'); + expect(axios.get).not.toHaveBeenCalled(); + }); + }); }); describe('receiveChartDataSuccess', () => { @@ -205,4 +225,22 @@ describe('Productivity analytics chart actions', () => { ); }); }); + + describe('setChartEnabled', () => { + it('should commit enabled state', done => { + testAction( + actions.setChartEnabled, + { chartKey: chartKeys.scatterplot, isEnabled: false }, + mockedContext.state, + [ + { + type: types.SET_CHART_ENABLED, + payload: { chartKey: chartKeys.scatterplot, isEnabled: false }, + }, + ], + [], + done, + ); + }); + }); }); diff --git a/ee/spec/frontend/analytics/productivity_analytics/store/modules/charts/getters_spec.js b/ee/spec/frontend/analytics/productivity_analytics/store/modules/charts/getters_spec.js index 0bb087adafb0dd95e21f85c5c61f7d4d1de99eb2..74c1b776e24e72612542a4f24d6526064b64cfbd 100644 --- a/ee/spec/frontend/analytics/productivity_analytics/store/modules/charts/getters_spec.js +++ b/ee/spec/frontend/analytics/productivity_analytics/store/modules/charts/getters_spec.js @@ -283,4 +283,17 @@ describe('Productivity analytics chart getters', () => { expect(getters.hasNoAccessError(state)).toEqual(false); }); }); + + describe('isChartEnabled', () => { + const chartKey = chartKeys.scatterplot; + it('returns true if the chart is enabled', () => { + state.charts[chartKey].enabled = true; + expect(getters.isChartEnabled(state)(chartKey)).toBe(true); + }); + + it('returns false if the chart is disabled', () => { + state.charts[chartKey].enabled = false; + expect(getters.isChartEnabled(state)(chartKey)).toBe(false); + }); + }); }); diff --git a/ee/spec/frontend/analytics/productivity_analytics/store/modules/charts/mutations_spec.js b/ee/spec/frontend/analytics/productivity_analytics/store/modules/charts/mutations_spec.js index d10238b0196bdd9ff3c197bfeac7750ca990ff91..8de0cffbf848db5b04b2d42c26e1be43cd1cdfe6 100644 --- a/ee/spec/frontend/analytics/productivity_analytics/store/modules/charts/mutations_spec.js +++ b/ee/spec/frontend/analytics/productivity_analytics/store/modules/charts/mutations_spec.js @@ -85,4 +85,20 @@ describe('Productivity analytics chart mutations', () => { expect(state.charts[chartKey].selected).toEqual([]); }); }); + + describe(types.SET_CHART_ENABLED, () => { + chartKey = chartKeys.scatterplot; + + it('sets the enabled flag to true on the scatterplot chart', () => { + mutations[types.SET_CHART_ENABLED](state, { chartKey, isEnabled: true }); + + expect(state.charts[chartKey].enabled).toBe(true); + }); + + it('sets the enabled flag to false on the scatterplot chart', () => { + mutations[types.SET_CHART_ENABLED](state, { chartKey, isEnabled: false }); + + expect(state.charts[chartKey].enabled).toBe(false); + }); + }); });