Commit 71a949f0 authored by Martin Wortschack's avatar Martin Wortschack Committed by Filipa Lacerda

Add "no data" message to charts

Refactor loading behavior

- Let's fetch the main chart first (instead of the MR table)
and use the responde to determine
whether the user has access to a given group
parent 8897a392
......@@ -26,15 +26,18 @@ export const fetchChartData = ({ dispatch, getters, rootState }, chartKey) => {
const { data } = response;
dispatch('receiveChartDataSuccess', { chartKey, data });
})
.catch(() => dispatch('receiveChartDataError', chartKey));
.catch(error => dispatch('receiveChartDataError', { chartKey, error }));
};
export const receiveChartDataSuccess = ({ commit }, { chartKey, data = {} }) => {
commit(types.RECEIVE_CHART_DATA_SUCCESS, { chartKey, data });
};
export const receiveChartDataError = ({ commit }, chartKey) => {
commit(types.RECEIVE_CHART_DATA_ERROR, chartKey);
export const receiveChartDataError = ({ commit }, { chartKey, error }) => {
const {
response: { status },
} = error;
commit(types.RECEIVE_CHART_DATA_ERROR, { chartKey, status });
};
export const setMetricType = ({ commit, dispatch }, { chartKey, metricType }) => {
......
import _ from 'underscore';
import httpStatus from '~/lib/utils/http_status';
import {
chartKeys,
metricTypes,
......@@ -43,11 +45,11 @@ export const getChartData = state => chartKey => {
};
});
return {
full: dataWithSelected,
};
return dataWithSelected;
};
export const chartHasData = state => chartKey => !_.isEmpty(state.charts[chartKey].data);
export const getMetricDropdownLabel = state => chartKey =>
metricTypes.find(m => m.key === state.charts[chartKey].params.metricType).label;
......@@ -108,5 +110,8 @@ export const getColumnChartDatazoomOption = state => chartKey => {
export const isSelectedMetric = state => ({ metric, chartKey }) =>
state.charts[chartKey].params.metricType === metric;
export const hasNoAccessError = state =>
state.charts[chartKeys.main].hasError === httpStatus.FORBIDDEN;
// prevent babel-plugin-rewire from generating an invalid default during karma tests
export default () => {};
......@@ -13,9 +13,9 @@ export default {
state.charts[chartKey].hasError = false;
state.charts[chartKey].data = data;
},
[types.RECEIVE_CHART_DATA_ERROR](state, chartKey) {
[types.RECEIVE_CHART_DATA_ERROR](state, { chartKey, status }) {
state.charts[chartKey].isLoading = false;
state.charts[chartKey].hasError = true;
state.charts[chartKey].hasError = status;
state.charts[chartKey].data = {};
},
[types.SET_METRIC_TYPE](state, { chartKey, metricType }) {
......
import * as types from './mutation_types';
import { chartKeys } from '../../../constants';
export const setGroupNamespace = ({ commit, dispatch }, groupNamespace) => {
commit(types.SET_GROUP_NAMESPACE, groupNamespace);
// let's fetch the merge requests first to see if the user has access to the selected group
// if there's no 403, then we fetch all chart data
return dispatch('table/fetchMergeRequests', null, { root: true }).then(() =>
dispatch('charts/fetchAllChartData', null, { root: true }),
);
// let's fetch the main chart data first to see if the user has access to the selected group
// if there's no 403, then we fetch all remaining chart data and table data
return dispatch('charts/fetchChartData', chartKeys.main, { root: true }).then(() => {
dispatch('charts/fetchChartData', chartKeys.timeBasedHistogram, { root: true });
dispatch('charts/fetchChartData', chartKeys.commitBasedHistogram, { root: true });
dispatch('table/fetchMergeRequests', null, { root: true });
});
};
export const setProjectPath = ({ commit, dispatch }, projectPath) => {
commit(types.SET_PROJECT_PATH, projectPath);
dispatch('charts/fetchAllChartData', null, { root: true });
dispatch('table/fetchMergeRequests', null, { root: true });
return dispatch('charts/fetchChartData', chartKeys.main, { root: true }).then(() => {
dispatch('charts/fetchChartData', chartKeys.timeBasedHistogram, { root: true });
dispatch('charts/fetchChartData', chartKeys.commitBasedHistogram, { root: true });
dispatch('table/fetchMergeRequests', null, { root: true });
});
};
export const setPath = ({ commit, dispatch }, path) => {
commit(types.SET_PATH, path);
dispatch('charts/fetchAllChartData', null, { root: true });
dispatch('table/fetchMergeRequests', null, { root: true });
return dispatch('charts/fetchChartData', chartKeys.main, { root: true }).then(() => {
dispatch('charts/fetchChartData', chartKeys.timeBasedHistogram, { root: true });
dispatch('charts/fetchChartData', chartKeys.commitBasedHistogram, { root: true });
dispatch('table/fetchMergeRequests', null, { root: true });
});
};
export const setDaysInPast = ({ commit, dispatch }, days) => {
commit(types.SET_DAYS_IN_PAST, days);
dispatch('charts/fetchAllChartData', null, { root: true });
dispatch('table/fetchMergeRequests', null, { root: true });
return dispatch('charts/fetchChartData', chartKeys.main, { root: true }).then(() => {
dispatch('charts/fetchChartData', chartKeys.timeBasedHistogram, { root: true });
dispatch('charts/fetchChartData', chartKeys.commitBasedHistogram, { root: true });
dispatch('table/fetchMergeRequests', null, { root: true });
});
};
// prevent babel-plugin-rewire from generating an invalid default during karma tests
......
import httpStatus from '~/lib/utils/http_status';
import { chartKeys, tableSortOrder, daysToMergeMetric } from '../../../constants';
export const sortIcon = state => tableSortOrder[state.sortOrder].icon;
......@@ -20,7 +19,5 @@ export const columnMetricLabel = (state, _getters, _rootState, rootGetters) =>
export const isSelectedSortField = state => sortField => state.sortField === sortField;
export const hasNoAccessError = state => state.hasError === httpStatus.FORBIDDEN;
// prevent babel-plugin-rewire from generating an invalid default during karma tests
export default () => {};
......@@ -75,7 +75,7 @@ describe('ProductivityApp component', () => {
describe('and user has no access to the group', () => {
beforeEach(() => {
store.state.table.hasError = 403;
store.state.charts.charts[chartKeys.main].hasError = 403;
});
it('renders the no access illustration', () => {
......@@ -88,7 +88,7 @@ describe('ProductivityApp component', () => {
describe('and user has access to the group', () => {
beforeEach(() => {
store.state.table.hasError = false;
store.state.charts.charts[chartKeys.main].hasError = false;
});
describe('Time to merge chart', () => {
......@@ -115,34 +115,57 @@ describe('ProductivityApp component', () => {
store.state.charts.charts[chartKeys.main].isLoading = false;
});
it('renders a column chart', () => {
expect(
describe('and the chart has data', () => {
beforeEach(() => {
store.state.charts.charts[chartKeys.main].data = { 1: 2, 2: 3 };
});
it('renders a column chart', () => {
expect(
findTimeToMergeSection()
.find(GlColumnChart)
.exists(),
).toBe(true);
});
it('calls onMainChartItemClicked when chartItemClicked is emitted on the column chart ', () => {
const data = {
chart: null,
params: {
data: {
value: [0, 1],
},
},
};
findTimeToMergeSection()
.find(GlColumnChart)
.exists(),
).toBe(true);
});
.vm.$emit('chartItemClicked', data);
it('calls onMainChartItemClicked when chartItemClicked is emitted on the column chart ', () => {
const data = {
chart: null,
params: {
data: {
value: [0, 1],
},
},
};
expect(onMainChartItemClickedMock).toHaveBeenCalledWith(data);
});
});
findTimeToMergeSection()
.find(GlColumnChart)
.vm.$emit('chartItemClicked', data);
describe("and the chart doesn't have any data", () => {
beforeEach(() => {
store.state.charts.charts[chartKeys.main].data = null;
});
expect(onMainChartItemClickedMock).toHaveBeenCalledWith(data);
it('renders a "no data" message', () => {
expect(findTimeToMergeSection().text()).toContain(
'There is no data available. Please change your selection.',
);
});
});
});
});
describe('Time based histogram', () => {
beforeEach(() => {
store.state.charts.charts[chartKeys.main].isLoading = false;
store.state.charts.charts[chartKeys.main].data = { 1: 2, 2: 3 };
});
describe('when chart is loading', () => {
beforeEach(() => {
store.state.charts.charts[chartKeys.timeBasedHistogram].isLoading = true;
......@@ -162,37 +185,60 @@ describe('ProductivityApp component', () => {
store.state.charts.charts[chartKeys.timeBasedHistogram].isLoading = false;
});
it('renders a metric type dropdown', () => {
expect(
describe('and the chart has data', () => {
beforeEach(() => {
store.state.charts.charts[chartKeys.timeBasedHistogram].data = { 1: 2, 2: 3 };
});
it('renders a metric type dropdown', () => {
expect(
findTimeBasedSection()
.find(GlDropdown)
.exists(),
).toBe(true);
});
it('should change the metric type', () => {
findTimeBasedSection()
.find(GlDropdown)
.exists(),
).toBe(true);
});
.findAll(GlDropdownItem)
.at(0)
.vm.$emit('click');
it('should change the metric type', () => {
findTimeBasedSection()
.findAll(GlDropdownItem)
.at(0)
.vm.$emit('click');
expect(actionSpies.setMetricType).toHaveBeenCalledWith({
metricType: 'time_to_first_comment',
chartKey: chartKeys.timeBasedHistogram,
});
});
expect(actionSpies.setMetricType).toHaveBeenCalledWith({
metricType: 'time_to_first_comment',
chartKey: chartKeys.timeBasedHistogram,
it('renders a column chart', () => {
expect(
findTimeBasedSection()
.find(GlColumnChart)
.exists(),
).toBe(true);
});
});
it('renders a column chart', () => {
expect(
findTimeBasedSection()
.find(GlColumnChart)
.exists(),
).toBe(true);
describe("and the chart doesn't have any data", () => {
beforeEach(() => {
store.state.charts.charts[chartKeys.timeBasedHistogram].data = null;
});
it('renders a "no data" message', () => {
expect(findTimeBasedSection().text()).toContain(
'There is no data for the selected metric. Please change your selection.',
);
});
});
});
});
describe('Commit based histogram', () => {
beforeEach(() => {
store.state.charts.charts[chartKeys.main].isLoading = false;
store.state.charts.charts[chartKeys.main].data = { 1: 2, 2: 3 };
});
describe('when chart is loading', () => {
beforeEach(() => {
store.state.charts.charts[chartKeys.commitBasedHistogram].isLoading = true;
......@@ -212,37 +258,60 @@ describe('ProductivityApp component', () => {
store.state.charts.charts[chartKeys.commitBasedHistogram].isLoading = false;
});
it('renders a metric type dropdown', () => {
expect(
describe('and the chart has data', () => {
beforeEach(() => {
store.state.charts.charts[chartKeys.commitBasedHistogram].data = { 1: 2, 2: 3 };
});
it('renders a metric type dropdown', () => {
expect(
findCommitBasedSection()
.find(GlDropdown)
.exists(),
).toBe(true);
});
it('should change the metric type', () => {
findCommitBasedSection()
.find(GlDropdown)
.exists(),
).toBe(true);
});
.findAll(GlDropdownItem)
.at(0)
.vm.$emit('click');
it('should change the metric type', () => {
findCommitBasedSection()
.findAll(GlDropdownItem)
.at(0)
.vm.$emit('click');
expect(actionSpies.setMetricType).toHaveBeenCalledWith({
metricType: 'commits_count',
chartKey: chartKeys.commitBasedHistogram,
});
});
expect(actionSpies.setMetricType).toHaveBeenCalledWith({
metricType: 'commits_count',
chartKey: chartKeys.commitBasedHistogram,
it('renders a column chart', () => {
expect(
findCommitBasedSection()
.find(GlColumnChart)
.exists(),
).toBe(true);
});
});
it('renders a column chart', () => {
expect(
findCommitBasedSection()
.find(GlColumnChart)
.exists(),
).toBe(true);
describe("and the chart doesn't have any data", () => {
beforeEach(() => {
store.state.charts.charts[chartKeys.commitBasedHistogram].data = null;
});
it('renders a "no data" message', () => {
expect(findTimeBasedSection().text()).toContain(
'There is no data for the selected metric. Please change your selection.',
);
});
});
});
});
describe('MR table', () => {
beforeEach(() => {
store.state.charts.charts[chartKeys.main].isLoading = false;
store.state.charts.charts[chartKeys.main].data = { 1: 2, 2: 3 };
});
describe('when isLoadingTable is true', () => {
beforeEach(() => {
store.state.table.isLoadingTable = true;
......@@ -260,6 +329,7 @@ describe('ProductivityApp component', () => {
describe('when isLoadingTable is false', () => {
beforeEach(() => {
store.state.table.isLoadingTable = false;
store.state.table.mergeRequests = [{ id: 1, title: 'This is a test MR' }];
});
it('renders the MR table', () => {
......
......@@ -79,7 +79,7 @@ describe('Productivity analytics chart actions', () => {
describe('error', () => {
beforeEach(() => {
mock.onGet(mockedState.endpoint).replyOnce(500, chartKey);
mock.onGet(mockedState.endpoint).replyOnce(500);
});
it('dispatches error', done => {
......@@ -95,7 +95,10 @@ describe('Productivity analytics chart actions', () => {
},
{
type: 'receiveChartDataError',
payload: chartKey,
payload: {
chartKey,
error: new Error('Request failed with status code 500'),
},
},
],
done,
......@@ -137,14 +140,18 @@ describe('Productivity analytics chart actions', () => {
describe('receiveChartDataError', () => {
it('should commit error', done => {
const error = { response: { status: 500 } };
testAction(
actions.receiveChartDataError,
chartKey,
{ chartKey, error },
mockedContext.state,
[
{
type: types.RECEIVE_CHART_DATA_ERROR,
payload: chartKey,
payload: {
chartKey,
status: 500,
},
},
],
[],
......
......@@ -38,12 +38,10 @@ describe('Productivity analytics chart getters', () => {
selected: ['5'],
};
const chartData = {
full: [
{ value: ['1', 32], itemStyle: {} },
{ value: ['5', 17], itemStyle: columnHighlightStyle },
],
};
const chartData = [
{ value: ['1', 32], itemStyle: {} },
{ value: ['5', 17], itemStyle: columnHighlightStyle },
];
expect(getters.getChartData(state)(chartKey)).toEqual(chartData);
});
......@@ -179,4 +177,16 @@ describe('Productivity analytics chart getters', () => {
});
});
});
describe('hasNoAccessError', () => {
it('returns true if "hasError" is set to 403', () => {
state.charts[chartKeys.main].hasError = 403;
expect(getters.hasNoAccessError(state)).toEqual(true);
});
it('returns false if "hasError" is not set to 403', () => {
state.charts[chartKeys.main].hasError = false;
expect(getters.hasNoAccessError(state)).toEqual(false);
});
});
});
......@@ -40,11 +40,12 @@ describe('Productivity analytics chart mutations', () => {
});
describe(types.RECEIVE_CHART_DATA_ERROR, () => {
it('sets isError and clears data', () => {
mutations[types.RECEIVE_CHART_DATA_ERROR](state, chartKey);
it('sets isError to error code and clears data', () => {
const status = 500;
mutations[types.RECEIVE_CHART_DATA_ERROR](state, { chartKey, status });
expect(state.charts[chartKey].isLoading).toBe(false);
expect(state.charts[chartKey].hasError).toBe(true);
expect(state.charts[chartKey].hasError).toBe(status);
expect(state.charts[chartKey].data).toEqual({});
});
});
......
import testAction from 'helpers/vuex_action_helper';
import * as actions from 'ee/analytics/productivity_analytics/store/modules/filters/actions';
import * as types from 'ee/analytics/productivity_analytics/store/modules/filters/mutation_types';
import getInitialState from 'ee/analytics/productivity_analytics/store/modules/filters/state';
import { chartKeys } from 'ee/analytics/productivity_analytics/constants';
describe('Productivity analytics filter actions', () => {
const groupNamespace = 'gitlab-org';
const projectPath = 'gitlab-org/gitlab-test';
const path = 'author_username=root';
const daysInPast = 90;
describe('setGroupNamespace', () => {
it('commits the SET_GROUP_NAMESPACE mutation', done => {
......@@ -20,13 +21,25 @@ describe('Productivity analytics filter actions', () => {
expect(store.commit).toHaveBeenCalledWith(types.SET_GROUP_NAMESPACE, groupNamespace);
expect(store.dispatch.mock.calls[0]).toEqual([
'table/fetchMergeRequests',
jasmine.any(Object),
'charts/fetchChartData',
chartKeys.main,
{ root: true },
]);
expect(store.dispatch.mock.calls[1]).toEqual([
'charts/fetchAllChartData',
'charts/fetchChartData',
chartKeys.timeBasedHistogram,
{ root: true },
]);
expect(store.dispatch.mock.calls[2]).toEqual([
'charts/fetchChartData',
chartKeys.commitBasedHistogram,
{ root: true },
]);
expect(store.dispatch.mock.calls[3]).toEqual([
'table/fetchMergeRequests',
jasmine.any(Object),
{ root: true },
]);
......@@ -37,80 +50,125 @@ describe('Productivity analytics filter actions', () => {
});
describe('setProjectPath', () => {
it('commits the SET_PROJECT_PATH mutation', done =>
testAction(
actions.setProjectPath,
projectPath,
getInitialState(),
[
{
type: types.SET_PROJECT_PATH,
payload: projectPath,
},
],
[
{
type: 'charts/fetchAllChartData',
payload: null,
},
{
type: 'table/fetchMergeRequests',
payload: null,
},
],
done,
));
it('commits the SET_PROJECT_PATH mutation', done => {
const store = {
commit: jest.fn(),
dispatch: jest.fn(() => Promise.resolve()),
};
actions
.setProjectPath(store, projectPath)
.then(() => {
expect(store.commit).toHaveBeenCalledWith(types.SET_PROJECT_PATH, projectPath);
expect(store.dispatch.mock.calls[0]).toEqual([
'charts/fetchChartData',
chartKeys.main,
{ root: true },
]);
expect(store.dispatch.mock.calls[1]).toEqual([
'charts/fetchChartData',
chartKeys.timeBasedHistogram,
{ root: true },
]);
expect(store.dispatch.mock.calls[2]).toEqual([
'charts/fetchChartData',
chartKeys.commitBasedHistogram,
{ root: true },
]);
expect(store.dispatch.mock.calls[3]).toEqual([
'table/fetchMergeRequests',
jasmine.any(Object),
{ root: true },
]);
})
.then(done)
.catch(done.fail);
});
});
describe('setPath', () => {
it('commits the SET_PATH mutation', done =>
testAction(
actions.setPath,
'author_username=root',
getInitialState(),
[
{
type: types.SET_PATH,
payload: 'author_username=root',
},
],
[
{
type: 'charts/fetchAllChartData',
payload: null,
},
{
type: 'table/fetchMergeRequests',
payload: null,
},
],
done,
));
it('commits the SET_PATH mutation', done => {
const store = {
commit: jest.fn(),
dispatch: jest.fn(() => Promise.resolve()),
};
actions
.setPath(store, path)
.then(() => {
expect(store.commit).toHaveBeenCalledWith(types.SET_PATH, path);
expect(store.dispatch.mock.calls[0]).toEqual([
'charts/fetchChartData',
chartKeys.main,
{ root: true },
]);
expect(store.dispatch.mock.calls[1]).toEqual([
'charts/fetchChartData',
chartKeys.timeBasedHistogram,
{ root: true },
]);
expect(store.dispatch.mock.calls[2]).toEqual([
'charts/fetchChartData',
chartKeys.commitBasedHistogram,
{ root: true },
]);
expect(store.dispatch.mock.calls[3]).toEqual([
'table/fetchMergeRequests',
jasmine.any(Object),
{ root: true },
]);
})
.then(done)
.catch(done.fail);
});
});
describe('setDaysInPast', () => {
it('commits the SET_DAYS_IN_PAST mutation', done =>
testAction(
actions.setDaysInPast,
90,
getInitialState(),
[
{
type: types.SET_DAYS_IN_PAST,
payload: 90,
},
],
[
{
type: 'charts/fetchAllChartData',
payload: null,
},
{
type: 'table/fetchMergeRequests',
payload: null,
},
],
done,
));
it('commits the SET_DAYS_IN_PAST mutation', done => {
const store = {
commit: jest.fn(),
dispatch: jest.fn(() => Promise.resolve()),
};
actions
.setDaysInPast(store, daysInPast)
.then(() => {
expect(store.commit).toHaveBeenCalledWith(types.SET_DAYS_IN_PAST, daysInPast);
expect(store.dispatch.mock.calls[0]).toEqual([
'charts/fetchChartData',
chartKeys.main,
{ root: true },
]);
expect(store.dispatch.mock.calls[1]).toEqual([
'charts/fetchChartData',
chartKeys.timeBasedHistogram,
{ root: true },
]);
expect(store.dispatch.mock.calls[2]).toEqual([
'charts/fetchChartData',
chartKeys.commitBasedHistogram,
{ root: true },
]);
expect(store.dispatch.mock.calls[3]).toEqual([
'table/fetchMergeRequests',
jasmine.any(Object),
{ root: true },
]);
})
.then(done)
.catch(done.fail);
});
});
});
......@@ -76,16 +76,4 @@ describe('Productivity analytics table getters', () => {
expect(getters.tableSortOptions(null, null, null, rootGetters)).toEqual(expected);
});
});
describe('hasNoAccessError', () => {
it('returns true if "hasError" is set to 403', () => {
state.hasError = 403;
expect(getters.hasNoAccessError(state)).toEqual(true);
});
it('returns false if "hasError" is not set to 403', () => {
state.hasError = false;
expect(getters.hasNoAccessError(state)).toEqual(false);
});
});
});
......@@ -15547,6 +15547,12 @@ msgstr ""
msgid "There is already a repository with that name on disk"
msgstr ""
msgid "There is no data available. Please change your selection."
msgstr ""
msgid "There is no data for the selected metric. Please change your selection."
msgstr ""
msgid "There was a problem communicating with your device."
msgstr ""
......
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