From 0c309355887e7feab9f616763a46ffaf22fb464e Mon Sep 17 00:00:00 2001 From: Martin Wortschack <mwortschack@gitlab.com> Date: Thu, 19 Sep 2019 10:26:55 +0000 Subject: [PATCH] Rename hasError to errorCode on state - This avoid confusion about the value since we now set the error code instead of a Boolean --- .../store/modules/charts/getters.js | 2 +- .../store/modules/charts/mutations.js | 4 ++-- .../store/modules/charts/state.js | 8 ++++---- .../store/modules/table/mutations.js | 4 ++-- .../store/modules/table/state.js | 2 +- .../productivity_analytics/components/app_spec.js | 4 ++-- .../store/modules/charts/getters_spec.js | 8 ++++---- .../store/modules/charts/mutations_spec.js | 14 ++++++++++---- .../store/modules/table/mutations_spec.js | 14 ++++++++++---- 9 files changed, 36 insertions(+), 24 deletions(-) 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 3de59851e08ec..b3a4ea0143ff9 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 @@ -111,7 +111,7 @@ export const isSelectedMetric = state => ({ metric, chartKey }) => state.charts[chartKey].params.metricType === metric; export const hasNoAccessError = state => - state.charts[chartKeys.main].hasError === httpStatus.FORBIDDEN; + state.charts[chartKeys.main].errorCode === httpStatus.FORBIDDEN; // 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/mutations.js b/ee/app/assets/javascripts/analytics/productivity_analytics/store/modules/charts/mutations.js index 49fddb0b4ae56..92d9f7fdc64a5 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 @@ -10,12 +10,12 @@ export default { }, [types.RECEIVE_CHART_DATA_SUCCESS](state, { chartKey, data }) { state.charts[chartKey].isLoading = false; - state.charts[chartKey].hasError = false; + state.charts[chartKey].errorCode = null; state.charts[chartKey].data = data; }, [types.RECEIVE_CHART_DATA_ERROR](state, { chartKey, status }) { state.charts[chartKey].isLoading = false; - state.charts[chartKey].hasError = status; + state.charts[chartKey].errorCode = status; state.charts[chartKey].data = {}; }, [types.SET_METRIC_TYPE](state, { chartKey, metricType }) { 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 13a9d244da1aa..434520bb71031 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 @@ -4,7 +4,7 @@ export default () => ({ charts: { [chartKeys.main]: { isLoading: false, - hasError: false, + errorCode: null, data: {}, selected: [], params: { @@ -13,7 +13,7 @@ export default () => ({ }, [chartKeys.timeBasedHistogram]: { isLoading: false, - hasError: false, + errorCode: null, data: {}, selected: [], params: { @@ -23,7 +23,7 @@ export default () => ({ }, [chartKeys.commitBasedHistogram]: { isLoading: false, - hasError: false, + errorCode: null, data: {}, selected: [], params: { @@ -33,7 +33,7 @@ export default () => ({ }, [chartKeys.scatterplot]: { isLoading: false, - hasError: false, + errorCode: null, data: {}, selected: [], params: { diff --git a/ee/app/assets/javascripts/analytics/productivity_analytics/store/modules/table/mutations.js b/ee/app/assets/javascripts/analytics/productivity_analytics/store/modules/table/mutations.js index 6571c909a2da0..9f9e2fda4df1b 100644 --- a/ee/app/assets/javascripts/analytics/productivity_analytics/store/modules/table/mutations.js +++ b/ee/app/assets/javascripts/analytics/productivity_analytics/store/modules/table/mutations.js @@ -7,13 +7,13 @@ export default { }, [types.RECEIVE_MERGE_REQUESTS_SUCCESS](state, { pageInfo, mergeRequests }) { state.isLoadingTable = false; - state.hasError = false; + state.errorCode = null; state.pageInfo = pageInfo; state.mergeRequests = mergeRequests; }, [types.RECEIVE_MERGE_REQUESTS_ERROR](state, errCode) { state.isLoadingTable = false; - state.hasError = errCode; + state.errorCode = errCode; state.pageInfo = {}; state.mergeRequests = []; }, diff --git a/ee/app/assets/javascripts/analytics/productivity_analytics/store/modules/table/state.js b/ee/app/assets/javascripts/analytics/productivity_analytics/store/modules/table/state.js index dc827831409d9..e495c494534ea 100644 --- a/ee/app/assets/javascripts/analytics/productivity_analytics/store/modules/table/state.js +++ b/ee/app/assets/javascripts/analytics/productivity_analytics/store/modules/table/state.js @@ -2,7 +2,7 @@ import { tableSortOrder } from '../../../constants'; export default () => ({ isLoadingTable: false, - hasError: false, + errorCode: null, mergeRequests: [], pageInfo: {}, sortOrder: tableSortOrder.asc.value, 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 113f9016d4f27..e2b6e2c36ee98 100644 --- a/ee/spec/frontend/analytics/productivity_analytics/components/app_spec.js +++ b/ee/spec/frontend/analytics/productivity_analytics/components/app_spec.js @@ -75,7 +75,7 @@ describe('ProductivityApp component', () => { describe('and user has no access to the group', () => { beforeEach(() => { - store.state.charts.charts[chartKeys.main].hasError = 403; + store.state.charts.charts[chartKeys.main].errorCode = 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.charts.charts[chartKeys.main].hasError = false; + store.state.charts.charts[chartKeys.main].errorCode = null; }); describe('Time to merge chart', () => { 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 91b1486b0a62b..95d41b5e2118d 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 @@ -179,13 +179,13 @@ describe('Productivity analytics chart getters', () => { }); describe('hasNoAccessError', () => { - it('returns true if "hasError" is set to 403', () => { - state.charts[chartKeys.main].hasError = 403; + it('returns true if errorCode is set to 403', () => { + state.charts[chartKeys.main].errorCode = 403; expect(getters.hasNoAccessError(state)).toEqual(true); }); - it('returns false if "hasError" is not set to 403', () => { - state.charts[chartKeys.main].hasError = false; + it('returns false if errorCode is not set to 403', () => { + state.charts[chartKeys.main].errorCode = null; expect(getters.hasNoAccessError(state)).toEqual(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 0c053fc22592f..d10238b0196bd 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 @@ -34,18 +34,24 @@ describe('Productivity analytics chart mutations', () => { mutations[types.RECEIVE_CHART_DATA_SUCCESS](state, { chartKey, data: mockHistogramData }); expect(state.charts[chartKey].isLoading).toBe(false); - expect(state.charts[chartKey].hasError).toBe(false); + expect(state.charts[chartKey].errorCode).toBe(null); expect(state.charts[chartKey].data).toEqual(mockHistogramData); }); }); describe(types.RECEIVE_CHART_DATA_ERROR, () => { - it('sets isError to error code and clears data', () => { - const status = 500; + const status = 500; + beforeEach(() => { mutations[types.RECEIVE_CHART_DATA_ERROR](state, { chartKey, status }); + }); + + it('sets errorCode to 500', () => { + expect(state.charts[chartKey].isLoading).toBe(false); + expect(state.charts[chartKey].errorCode).toBe(status); + }); + it('clears data', () => { expect(state.charts[chartKey].isLoading).toBe(false); - expect(state.charts[chartKey].hasError).toBe(status); expect(state.charts[chartKey].data).toEqual({}); }); }); diff --git a/ee/spec/frontend/analytics/productivity_analytics/store/modules/table/mutations_spec.js b/ee/spec/frontend/analytics/productivity_analytics/store/modules/table/mutations_spec.js index aaa5c199bbdc5..94e1cb5c96e43 100644 --- a/ee/spec/frontend/analytics/productivity_analytics/store/modules/table/mutations_spec.js +++ b/ee/spec/frontend/analytics/productivity_analytics/store/modules/table/mutations_spec.js @@ -33,19 +33,25 @@ describe('Productivity analytics table mutations', () => { }); expect(state.isLoadingTable).toBe(false); - expect(state.hasError).toBe(false); + expect(state.errorCode).toBe(null); expect(state.mergeRequests).toEqual(mockMergeRequests); expect(state.pageInfo).toEqual(pageInfo); }); }); describe(types.RECEIVE_MERGE_REQUESTS_ERROR, () => { - it('sets hasError to error code and clears data', () => { - const errorCode = 500; + const errorCode = 500; + beforeEach(() => { mutations[types.RECEIVE_MERGE_REQUESTS_ERROR](state, errorCode); + }); + + it('sets errorCode to 500', () => { + expect(state.isLoadingTable).toBe(false); + expect(state.errorCode).toBe(errorCode); + }); + it('clears data', () => { expect(state.isLoadingTable).toBe(false); - expect(state.hasError).toBe(errorCode); expect(state.mergeRequests).toEqual([]); expect(state.pageInfo).toEqual({}); }); -- GitLab