diff --git a/src/chart_types/xy_chart/domains/x_domain.test.ts b/src/chart_types/xy_chart/domains/x_domain.test.ts index 6372a3589d..01f7dc5603 100644 --- a/src/chart_types/xy_chart/domains/x_domain.test.ts +++ b/src/chart_types/xy_chart/domains/x_domain.test.ts @@ -21,10 +21,17 @@ import { ChartTypes } from '../..'; import { MockSeriesSpecs } from '../../../mocks/specs'; import { ScaleType } from '../../../scales/constants'; import { SpecTypes, Direction, BinAgg } from '../../../specs/constants'; +import { Logger } from '../../../utils/logger'; import { getDataSeriesFromSpecs } from '../utils/series'; import { BasicSeriesSpec, SeriesTypes } from '../utils/specs'; import { convertXScaleTypes, findMinInterval, mergeXDomain } from './x_domain'; +jest.mock('../../../utils/logger', () => ({ + Logger: { + warn: jest.fn(), + }, +})); + describe('X Domain', () => { test('Should return null when missing specs or specs types', () => { const seriesSpecs: BasicSeriesSpec[] = []; @@ -738,17 +745,17 @@ describe('X Domain', () => { expect(basicMergedDomain.domain).toEqual([0, 3]); const arrayXDomain = [1, 2]; - const attemptToMergeArrayDomain = () => { - mergeXDomain(specs, xValues, arrayXDomain); - }; - const errorMessage = 'xDomain for continuous scale should be a DomainRange object, not an array'; - expect(attemptToMergeArrayDomain).toThrowError(errorMessage); + let { domain } = mergeXDomain(specs, xValues, arrayXDomain); + expect(domain).toEqual([1, 5]); + const warnMessage = 'xDomain for continuous scale should be a DomainRange object, not an array'; + expect(Logger.warn).toBeCalledWith(warnMessage); + + (Logger.warn as jest.Mock).mockClear(); const invalidXDomain = { min: 10, max: 0 }; - const attemptToMerge = () => { - mergeXDomain(specs, xValues, invalidXDomain); - }; - expect(attemptToMerge).toThrowError('custom xDomain is invalid, min is greater than max'); + domain = mergeXDomain(specs, xValues, invalidXDomain).domain; + expect(domain).toEqual([1, 5]); + expect(Logger.warn).toBeCalledWith('custom xDomain is invalid, min is greater than max. Custom domain is ignored.'); }); test('should account for custom domain when merging a linear domain: lower bounded domain', () => { @@ -762,10 +769,11 @@ describe('X Domain', () => { expect(mergedDomain.domain).toEqual([0, 5]); const invalidXDomain = { min: 10 }; - const attemptToMerge = () => { - mergeXDomain(specs, xValues, invalidXDomain); - }; - expect(attemptToMerge).toThrowError('custom xDomain is invalid, custom min is greater than computed max'); + const { domain } = mergeXDomain(specs, xValues, invalidXDomain); + expect(domain).toEqual([1, 5]); + expect(Logger.warn).toBeCalledWith( + 'custom xDomain is invalid, custom min is greater than computed max. Custom domain is ignored.', + ); }); test('should account for custom domain when merging a linear domain: upper bounded domain', () => { @@ -779,10 +787,11 @@ describe('X Domain', () => { expect(mergedDomain.domain).toEqual([1, 3]); const invalidXDomain = { max: -1 }; - const attemptToMerge = () => { - mergeXDomain(specs, xValues, invalidXDomain); - }; - expect(attemptToMerge).toThrowError('custom xDomain is invalid, computed min is greater than custom max'); + const { domain } = mergeXDomain(specs, xValues, invalidXDomain); + expect(domain).toEqual([1, 5]); + expect(Logger.warn).toBeCalledWith( + 'custom xDomain is invalid, computed min is greater than custom max. Custom domain is ignored.', + ); }); test('should account for custom domain when merging an ordinal domain', () => { @@ -795,11 +804,11 @@ describe('X Domain', () => { expect(basicMergedDomain.domain).toEqual(['a', 'b', 'c']); const objectXDomain = { max: 10, min: 0 }; - const attemptToMerge = () => { - mergeXDomain(specs, xValues, objectXDomain); - }; - const errorMessage = 'xDomain for ordinal scale should be an array of values, not a DomainRange object'; - expect(attemptToMerge).toThrowError(errorMessage); + const { domain } = mergeXDomain(specs, xValues, objectXDomain); + expect(domain).toEqual(['a', 'b', 'c', 'd']); + const warnMessage = + 'xDomain for ordinal scale should be an array of values, not a DomainRange object. xDomain is ignored.'; + expect(Logger.warn).toBeCalledWith(warnMessage); }); describe('should account for custom minInterval', () => { @@ -822,20 +831,20 @@ describe('X Domain', () => { test('with invalid minInterval greater than computed minInterval for multi data set', () => { const invalidXDomain = { minInterval: 10 }; - const attemptToMerge = () => { - mergeXDomain(specs, xValues, invalidXDomain); - }; - const expectedError = 'custom xDomain is invalid, custom minInterval is greater than computed minInterval'; - expect(attemptToMerge).toThrowError(expectedError); + const { minInterval } = mergeXDomain(specs, xValues, invalidXDomain); + expect(minInterval).toEqual(1); + const expectedWarning = + 'custom xDomain is invalid, custom minInterval is greater than computed minInterval. Using computed minInterval.'; + expect(Logger.warn).toBeCalledWith(expectedWarning); }); test('with invalid minInterval less than 0', () => { const invalidXDomain = { minInterval: -1 }; - const attemptToMerge = () => { - mergeXDomain(specs, xValues, invalidXDomain); - }; - const expectedError = 'custom xDomain is invalid, custom minInterval is less than 0'; - expect(attemptToMerge).toThrowError(expectedError); + const { minInterval } = mergeXDomain(specs, xValues, invalidXDomain); + expect(minInterval).toEqual(1); + const expectedWarning = + 'custom xDomain is invalid, custom minInterval is less than 0. Using computed minInterval.'; + expect(Logger.warn).toBeCalledWith(expectedWarning); }); }); diff --git a/src/chart_types/xy_chart/domains/x_domain.ts b/src/chart_types/xy_chart/domains/x_domain.ts index 1a3a054930..a16ec4e525 100644 --- a/src/chart_types/xy_chart/domains/x_domain.ts +++ b/src/chart_types/xy_chart/domains/x_domain.ts @@ -67,7 +67,9 @@ export function mergeXDomain( 1) Correct data to match ${mainXScaleType.scaleType} scale type (see previous warning) 2) Change xScaleType to ordinal and set xDomain to Domain array`); } else { - throw new TypeError('xDomain for ordinal scale should be an array of values, not a DomainRange object.'); + Logger.warn( + 'xDomain for ordinal scale should be an array of values, not a DomainRange object. xDomain is ignored.', + ); } } } @@ -77,44 +79,38 @@ export function mergeXDomain( if (customXDomain) { if (Array.isArray(customXDomain)) { - throw new TypeError('xDomain for continuous scale should be a DomainRange object, not an array'); - } - - customMinInterval = customXDomain.minInterval; - const [computedDomainMin, computedDomainMax] = seriesXComputedDomains; - - if (isCompleteBound(customXDomain)) { - if (customXDomain.min > customXDomain.max) { - throw new Error('custom xDomain is invalid, min is greater than max'); - } - - seriesXComputedDomains = [customXDomain.min, customXDomain.max]; - } else if (isLowerBound(customXDomain)) { - if (customXDomain.min > computedDomainMax) { - throw new Error('custom xDomain is invalid, custom min is greater than computed max'); - } - - seriesXComputedDomains = [customXDomain.min, computedDomainMax]; - } else if (isUpperBound(customXDomain)) { - if (computedDomainMin > customXDomain.max) { - throw new Error('custom xDomain is invalid, computed min is greater than custom max'); + Logger.warn('xDomain for continuous scale should be a DomainRange object, not an array'); + } else { + customMinInterval = customXDomain.minInterval; + const [computedDomainMin, computedDomainMax] = seriesXComputedDomains; + + if (isCompleteBound(customXDomain)) { + if (customXDomain.min > customXDomain.max) { + Logger.warn('custom xDomain is invalid, min is greater than max. Custom domain is ignored.'); + } else { + seriesXComputedDomains = [customXDomain.min, customXDomain.max]; + } + } else if (isLowerBound(customXDomain)) { + if (customXDomain.min > computedDomainMax) { + Logger.warn( + 'custom xDomain is invalid, custom min is greater than computed max. Custom domain is ignored.', + ); + } else { + seriesXComputedDomains = [customXDomain.min, computedDomainMax]; + } + } else if (isUpperBound(customXDomain)) { + if (computedDomainMin > customXDomain.max) { + Logger.warn( + 'custom xDomain is invalid, computed min is greater than custom max. Custom domain is ignored.', + ); + } else { + seriesXComputedDomains = [computedDomainMin, customXDomain.max]; + } } - - seriesXComputedDomains = [computedDomainMin, customXDomain.max]; } } const computedMinInterval = findMinInterval(values as number[]); - if (customMinInterval != null) { - // Allow greater custom min iff xValues has 1 member. - if (xValues.size > 1 && customMinInterval > computedMinInterval) { - throw new Error('custom xDomain is invalid, custom minInterval is greater than computed minInterval'); - } - if (customMinInterval < 0) { - throw new Error('custom xDomain is invalid, custom minInterval is less than 0'); - } - } - - minInterval = customMinInterval || computedMinInterval; + minInterval = getMinInterval(computedMinInterval, xValues.size, customMinInterval); } return { @@ -127,6 +123,25 @@ export function mergeXDomain( }; } +function getMinInterval(computedMinInterval: number, size: number, customMinInterval?: number): number { + if (customMinInterval == null) { + return computedMinInterval; + } + // Allow greater custom min if xValues has 1 member. + if (size > 1 && customMinInterval > computedMinInterval) { + Logger.warn( + 'custom xDomain is invalid, custom minInterval is greater than computed minInterval. Using computed minInterval.', + ); + return computedMinInterval; + } + if (customMinInterval < 0) { + Logger.warn('custom xDomain is invalid, custom minInterval is less than 0. Using computed minInterval.'); + return computedMinInterval; + } + + return customMinInterval; +} + /** * Find the minimum interval between xValues. * Default to 0 if an empty array, 1 if one item array diff --git a/src/chart_types/xy_chart/domains/y_domain.test.ts b/src/chart_types/xy_chart/domains/y_domain.test.ts index cea56ed189..6d44134e64 100644 --- a/src/chart_types/xy_chart/domains/y_domain.test.ts +++ b/src/chart_types/xy_chart/domains/y_domain.test.ts @@ -24,10 +24,17 @@ import { ScaleType } from '../../../scales/constants'; import { SpecTypes } from '../../../specs/constants'; import { Position } from '../../../utils/commons'; import { BARCHART_1Y0G } from '../../../utils/data_samples/test_dataset'; +import { Logger } from '../../../utils/logger'; import { computeSeriesDomainsSelector } from '../state/selectors/compute_series_domains'; import { BasicSeriesSpec, SeriesTypes, DEFAULT_GLOBAL_ID, StackMode } from '../utils/specs'; import { coerceYScaleTypes, groupSeriesByYGroup } from './y_domain'; +jest.mock('../../../utils/logger', () => ({ + Logger: { + warn: jest.fn(), + }, +})); + const DEMO_AREA_SPEC_1 = { id: 'a', groupId: 'a', @@ -448,12 +455,13 @@ describe('Y Domain', () => { store, ); - const attemptToMerge = () => { - computeSeriesDomainsSelector(store.getState()); - }; + const { + yDomain: [{ domain }], + } = computeSeriesDomainsSelector(store.getState()); + expect(domain).toEqual([20, 20]); - const errorMessage = 'custom yDomain for a is invalid, custom min is greater than computed max'; - expect(attemptToMerge).toThrowError(errorMessage); + const warnMessage = 'custom yDomain for a is invalid, custom min is greater than computed max.'; + expect(Logger.warn).toBeCalledWith(warnMessage); }); test('Should merge Y domain accounting for custom domain limits: partial upper bounded domain', () => { const store = MockStore.default(); @@ -496,12 +504,13 @@ describe('Y Domain', () => { store, ); - const attemptToMerge = () => { - computeSeriesDomainsSelector(store.getState()); - }; + const { + yDomain: [{ domain }], + } = computeSeriesDomainsSelector(store.getState()); + expect(domain).toEqual([-1, -1]); - const errorMessage = 'custom yDomain for a is invalid, computed min is greater than custom max'; - expect(attemptToMerge).toThrowError(errorMessage); + const warnMessage = 'custom yDomain for a is invalid, custom max is less than computed max.'; + expect(Logger.warn).toBeCalledWith(warnMessage); }); test('Should merge Y domain with stacked as percentage', () => { const store = MockStore.default(); diff --git a/src/chart_types/xy_chart/domains/y_domain.ts b/src/chart_types/xy_chart/domains/y_domain.ts index d11bba770d..24126c2188 100644 --- a/src/chart_types/xy_chart/domains/y_domain.ts +++ b/src/chart_types/xy_chart/domains/y_domain.ts @@ -17,7 +17,6 @@ * under the License. */ -import { GracefulError } from '../../../components/error_boundary/errors'; import { ScaleContinuousType } from '../../../scales'; import { ScaleType } from '../../../scales/constants'; import { identity } from '../../../utils/commons'; @@ -101,16 +100,18 @@ function mergeYDomainForGroup( domain = [newCustomDomain.min, newCustomDomain.max]; } else if (newCustomDomain && isLowerBound(newCustomDomain)) { if (newCustomDomain.min > computedDomainMax) { - throw new GracefulError(`custom yDomain for ${groupId} is invalid, custom min is greater than computed max`); + Logger.warn(`custom yDomain for ${groupId} is invalid, custom min is greater than computed max.`); + domain = [newCustomDomain.min, newCustomDomain.min]; + } else { + domain = [newCustomDomain.min, computedDomainMax]; } - - domain = [newCustomDomain.min, computedDomainMax]; } else if (newCustomDomain && isUpperBound(newCustomDomain)) { if (computedDomainMin > newCustomDomain.max) { - throw new Error(`custom yDomain for ${groupId} is invalid, computed min is greater than custom max`); + Logger.warn(`custom yDomain for ${groupId} is invalid, custom max is less than computed max.`); + domain = [newCustomDomain.max, newCustomDomain.max]; + } else { + domain = [computedDomainMin, newCustomDomain.max]; } - - domain = [computedDomainMin, newCustomDomain.max]; } } return {