Commit 9bed4094 authored by Martin Wortschack's avatar Martin Wortschack Committed by Filipa Lacerda

Show scatterplot behind feature flag

- Introduce productivity_analytics_scatterplot_enabled feature flag and
prevent scatterplot from being loaded and show
parent be5aa0e0
...@@ -9,6 +9,7 @@ import { ...@@ -9,6 +9,7 @@ import {
GlTooltipDirective, GlTooltipDirective,
} from '@gitlab/ui'; } from '@gitlab/ui';
import { GlColumnChart } from '@gitlab/ui/dist/charts'; 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 Icon from '~/vue_shared/components/icon.vue';
import MetricChart from './metric_chart.vue'; import MetricChart from './metric_chart.vue';
import Scatterplot from '../../shared/components/scatterplot.vue'; import Scatterplot from '../../shared/components/scatterplot.vue';
...@@ -31,6 +32,7 @@ export default { ...@@ -31,6 +32,7 @@ export default {
directives: { directives: {
GlTooltip: GlTooltipDirective, GlTooltip: GlTooltipDirective,
}, },
mixins: [featureFlagsMixin()],
props: { props: {
endpoint: { endpoint: {
type: String, type: String,
...@@ -65,6 +67,7 @@ export default { ...@@ -65,6 +67,7 @@ export default {
'getSelectedMetric', 'getSelectedMetric',
'scatterplotYaxisLabel', 'scatterplotYaxisLabel',
'hasNoAccessError', 'hasNoAccessError',
'isChartEnabled',
]), ]),
...mapGetters('table', [ ...mapGetters('table', [
'sortFieldDropdownLabel', 'sortFieldDropdownLabel',
...@@ -89,10 +92,19 @@ export default { ...@@ -89,10 +92,19 @@ export default {
}, },
mounted() { mounted() {
this.setEndpoint(this.endpoint); this.setEndpoint(this.endpoint);
this.setChartEnabled({
chartKey: chartKeys.scatterplot,
isEnabled: this.isScatterplotFeatureEnabled(),
});
}, },
methods: { methods: {
...mapActions(['setEndpoint']), ...mapActions(['setEndpoint']),
...mapActions('charts', ['fetchChartData', 'setMetricType', 'chartItemClicked']), ...mapActions('charts', [
'fetchChartData',
'setMetricType',
'chartItemClicked',
'setChartEnabled',
]),
...mapActions('table', ['setSortField', 'setPage', 'toggleSortOrder', 'setColumnMetric']), ...mapActions('table', ['setSortField', 'setPage', 'toggleSortOrder', 'setColumnMetric']),
onMainChartItemClicked({ params }) { onMainChartItemClicked({ params }) {
const itemValue = params.data.value[0]; const itemValue = params.data.value[0];
...@@ -108,6 +120,9 @@ export default { ...@@ -108,6 +120,9 @@ export default {
...this.getColumnChartDatazoomOption(chartKey), ...this.getColumnChartDatazoomOption(chartKey),
}; };
}, },
isScatterplotFeatureEnabled() {
return this.glFeatures.productivityAnalyticsScatterplotEnabled;
},
}, },
}; };
</script> </script>
...@@ -217,6 +232,7 @@ export default { ...@@ -217,6 +232,7 @@ export default {
</div> </div>
<metric-chart <metric-chart
v-if="isChartEnabled(chartKeys.scatterplot)"
ref="scatterplot" ref="scatterplot"
class="mb-4" class="mb-4"
:title="s__('ProductivityAnalytics|Trendline')" :title="s__('ProductivityAnalytics|Trendline')"
......
...@@ -16,18 +16,21 @@ export const fetchSecondaryChartData = ({ state, dispatch }) => { ...@@ -16,18 +16,21 @@ export const fetchSecondaryChartData = ({ state, dispatch }) => {
export const requestChartData = ({ commit }, chartKey) => export const requestChartData = ({ commit }, chartKey) =>
commit(types.REQUEST_CHART_DATA, chartKey); commit(types.REQUEST_CHART_DATA, chartKey);
export const fetchChartData = ({ dispatch, getters, rootState }, chartKey) => { export const fetchChartData = ({ dispatch, getters, state, rootState }, chartKey) => {
// let's fetch data for enabled charts only
if (state.charts[chartKey].enabled) {
dispatch('requestChartData', chartKey); dispatch('requestChartData', chartKey);
const params = getters.getFilterParams(chartKey); const params = getters.getFilterParams(chartKey);
return axios axios
.get(rootState.endpoint, { params }) .get(rootState.endpoint, { params })
.then(response => { .then(response => {
const { data } = response; const { data } = response;
dispatch('receiveChartDataSuccess', { chartKey, data }); dispatch('receiveChartDataSuccess', { chartKey, data });
}) })
.catch(error => dispatch('receiveChartDataError', { chartKey, error })); .catch(error => dispatch('receiveChartDataError', { chartKey, error }));
}
}; };
export const receiveChartDataSuccess = ({ commit }, { chartKey, data = {} }) => { export const receiveChartDataSuccess = ({ commit }, { chartKey, data = {} }) => {
...@@ -57,5 +60,8 @@ export const chartItemClicked = ({ commit, dispatch }, { chartKey, item }) => { ...@@ -57,5 +60,8 @@ export const chartItemClicked = ({ commit, dispatch }, { chartKey, item }) => {
dispatch('table/setPage', 0, { root: true }); 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 // prevent babel-plugin-rewire from generating an invalid default during karma tests
export default () => {}; export default () => {};
...@@ -169,5 +169,7 @@ export const scatterplotYaxisLabel = (_state, getters, rootState) => { ...@@ -169,5 +169,7 @@ export const scatterplotYaxisLabel = (_state, getters, rootState) => {
export const hasNoAccessError = state => export const hasNoAccessError = state =>
state.charts[chartKeys.main].errorCode === httpStatus.FORBIDDEN; 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 // prevent babel-plugin-rewire from generating an invalid default during karma tests
export default () => {}; export default () => {};
...@@ -5,3 +5,4 @@ export const RECEIVE_CHART_DATA_ERROR = 'RECEIVE_CHART_DATA_ERROR'; ...@@ -5,3 +5,4 @@ export const RECEIVE_CHART_DATA_ERROR = 'RECEIVE_CHART_DATA_ERROR';
export const SET_METRIC_TYPE = 'SET_METRIC_TYPE'; export const SET_METRIC_TYPE = 'SET_METRIC_TYPE';
export const UPDATE_SELECTED_CHART_ITEMS = 'UPDATE_SELECTED_CHART_ITEMS'; export const UPDATE_SELECTED_CHART_ITEMS = 'UPDATE_SELECTED_CHART_ITEMS';
export const SET_CHART_ENABLED = 'SET_CHART_ENABLED';
...@@ -29,4 +29,7 @@ export default { ...@@ -29,4 +29,7 @@ export default {
state.charts[chartKey].selected.splice(idx, 1); state.charts[chartKey].selected.splice(idx, 1);
} }
}, },
[types.SET_CHART_ENABLED](state, { chartKey, isEnabled }) {
state.charts[chartKey].enabled = isEnabled;
},
}; };
...@@ -5,6 +5,7 @@ export default () => ({ ...@@ -5,6 +5,7 @@ export default () => ({
[chartKeys.main]: { [chartKeys.main]: {
isLoading: false, isLoading: false,
errorCode: null, errorCode: null,
enabled: true,
data: {}, data: {},
selected: [], selected: [],
params: { params: {
...@@ -14,6 +15,7 @@ export default () => ({ ...@@ -14,6 +15,7 @@ export default () => ({
[chartKeys.timeBasedHistogram]: { [chartKeys.timeBasedHistogram]: {
isLoading: false, isLoading: false,
errorCode: null, errorCode: null,
enabled: true,
data: {}, data: {},
selected: [], selected: [],
params: { params: {
...@@ -24,6 +26,7 @@ export default () => ({ ...@@ -24,6 +26,7 @@ export default () => ({
[chartKeys.commitBasedHistogram]: { [chartKeys.commitBasedHistogram]: {
isLoading: false, isLoading: false,
errorCode: null, errorCode: null,
enabled: true,
data: {}, data: {},
selected: [], selected: [],
params: { params: {
...@@ -34,6 +37,7 @@ export default () => ({ ...@@ -34,6 +37,7 @@ export default () => ({
[chartKeys.scatterplot]: { [chartKeys.scatterplot]: {
isLoading: false, isLoading: false,
errorCode: null, errorCode: null,
enabled: true,
data: {}, data: {},
selected: [], selected: [],
params: { params: {
......
...@@ -13,6 +13,9 @@ class Analytics::ProductivityAnalyticsController < Analytics::ApplicationControl ...@@ -13,6 +13,9 @@ class Analytics::ProductivityAnalyticsController < Analytics::ApplicationControl
before_action -> { before_action -> {
authorize_view_productivity_analytics!(:view_productivity_analytics) authorize_view_productivity_analytics!(:view_productivity_analytics)
} }
before_action -> {
push_frontend_feature_flag(:productivity_analytics_scatterplot_enabled, default_enabled: true)
}
include IssuableCollections include IssuableCollections
......
...@@ -34,8 +34,7 @@ describe('ProductivityApp component', () => { ...@@ -34,8 +34,7 @@ describe('ProductivityApp component', () => {
const mainChartData = { 1: 2, 2: 3 }; const mainChartData = { 1: 2, 2: 3 };
beforeEach(() => { const createComponent = (scatterplotEnabled = true) => {
mock = new MockAdapter(axios);
wrapper = shallowMount(localVue.extend(ProductivityApp), { wrapper = shallowMount(localVue.extend(ProductivityApp), {
localVue, localVue,
store, store,
...@@ -44,7 +43,14 @@ describe('ProductivityApp component', () => { ...@@ -44,7 +43,14 @@ describe('ProductivityApp component', () => {
methods: { methods: {
...actionSpies, ...actionSpies,
}, },
provide: {
glFeatures: { productivityAnalyticsScatterplotEnabled: scatterplotEnabled },
},
}); });
};
beforeEach(() => {
mock = new MockAdapter(axios);
}); });
afterEach(() => { afterEach(() => {
...@@ -66,6 +72,7 @@ describe('ProductivityApp component', () => { ...@@ -66,6 +72,7 @@ describe('ProductivityApp component', () => {
describe('template', () => { describe('template', () => {
describe('without a group being selected', () => { describe('without a group being selected', () => {
it('renders the empty state illustration', () => { it('renders the empty state illustration', () => {
createComponent();
const emptyState = wrapper.find(GlEmptyState); const emptyState = wrapper.find(GlEmptyState);
expect(emptyState.exists()).toBe(true); expect(emptyState.exists()).toBe(true);
...@@ -81,6 +88,7 @@ describe('ProductivityApp component', () => { ...@@ -81,6 +88,7 @@ describe('ProductivityApp component', () => {
describe('user has no access to the group', () => { describe('user has no access to the group', () => {
beforeEach(() => { beforeEach(() => {
createComponent();
const error = { response: { status: 403 } }; const error = { response: { status: 403 } };
wrapper.vm.$store.dispatch('charts/receiveChartDataError', { wrapper.vm.$store.dispatch('charts/receiveChartDataError', {
chartKey: chartKeys.main, chartKey: chartKeys.main,
...@@ -106,6 +114,7 @@ describe('ProductivityApp component', () => { ...@@ -106,6 +114,7 @@ describe('ProductivityApp component', () => {
describe('when the main chart is loading', () => { describe('when the main chart is loading', () => {
beforeEach(() => { beforeEach(() => {
createComponent();
wrapper.vm.$store.dispatch('charts/requestChartData', chartKeys.main); wrapper.vm.$store.dispatch('charts/requestChartData', chartKeys.main);
}); });
...@@ -130,6 +139,7 @@ describe('ProductivityApp component', () => { ...@@ -130,6 +139,7 @@ describe('ProductivityApp component', () => {
describe('when the main chart finished loading', () => { describe('when the main chart finished loading', () => {
describe('and has data', () => { describe('and has data', () => {
beforeEach(() => { beforeEach(() => {
createComponent();
wrapper.vm.$store.dispatch('charts/receiveChartDataSuccess', { wrapper.vm.$store.dispatch('charts/receiveChartDataSuccess', {
chartKey: chartKeys.main, chartKey: chartKeys.main,
data: mainChartData, data: mainChartData,
...@@ -252,6 +262,11 @@ describe('ProductivityApp component', () => { ...@@ -252,6 +262,11 @@ describe('ProductivityApp component', () => {
}); });
describe('Scatterplot', () => { describe('Scatterplot', () => {
describe('when the feature flag is enabled', () => {
it('isScatterplotFeatureEnabled returns true', () => {
expect(wrapper.vm.isScatterplotFeatureEnabled()).toBe(true);
});
it('renders a metric chart component', () => { it('renders a metric chart component', () => {
expect(findScatterplotMetricChart().exists()).toBe(true); expect(findScatterplotMetricChart().exists()).toBe(true);
}); });
...@@ -298,6 +313,21 @@ describe('ProductivityApp component', () => { ...@@ -298,6 +313,21 @@ describe('ProductivityApp component', () => {
}); });
}); });
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', () => { describe('MR table', () => {
describe('when table is loading', () => { describe('when table is loading', () => {
beforeEach(() => { beforeEach(() => {
...@@ -400,6 +430,7 @@ describe('ProductivityApp component', () => { ...@@ -400,6 +430,7 @@ describe('ProductivityApp component', () => {
describe('and has no data', () => { describe('and has no data', () => {
beforeEach(() => { beforeEach(() => {
createComponent();
wrapper.vm.$store.dispatch('charts/receiveChartDataSuccess', { wrapper.vm.$store.dispatch('charts/receiveChartDataSuccess', {
chartKey: chartKeys.main, chartKey: chartKeys.main,
data: {}, data: {},
......
...@@ -47,6 +47,7 @@ describe('Productivity analytics chart actions', () => { ...@@ -47,6 +47,7 @@ describe('Productivity analytics chart actions', () => {
}); });
describe('fetchChartData', () => { describe('fetchChartData', () => {
describe('when chart is enabled', () => {
describe('success', () => { describe('success', () => {
beforeEach(() => { beforeEach(() => {
mock.onGet(mockedState.endpoint).replyOnce(200, mockHistogramData); mock.onGet(mockedState.endpoint).replyOnce(200, mockHistogramData);
...@@ -106,6 +107,7 @@ describe('Productivity analytics chart actions', () => { ...@@ -106,6 +107,7 @@ describe('Productivity analytics chart actions', () => {
}); });
}); });
}); });
});
describe('requestChartData', () => { describe('requestChartData', () => {
it('should commit the request mutation', done => { it('should commit the request mutation', done => {
...@@ -118,6 +120,24 @@ describe('Productivity analytics chart actions', () => { ...@@ -118,6 +120,24 @@ describe('Productivity analytics chart actions', () => {
done, 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', () => { describe('receiveChartDataSuccess', () => {
...@@ -205,4 +225,22 @@ describe('Productivity analytics chart actions', () => { ...@@ -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,
);
});
});
}); });
...@@ -283,4 +283,17 @@ describe('Productivity analytics chart getters', () => { ...@@ -283,4 +283,17 @@ describe('Productivity analytics chart getters', () => {
expect(getters.hasNoAccessError(state)).toEqual(false); 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);
});
});
}); });
...@@ -85,4 +85,20 @@ describe('Productivity analytics chart mutations', () => { ...@@ -85,4 +85,20 @@ describe('Productivity analytics chart mutations', () => {
expect(state.charts[chartKey].selected).toEqual([]); 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);
});
});
}); });
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment