From 14d0645e83e80103f0bdc6f418b0536104b7c937 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cau=C3=AA=20Marcondes?= <55978943+cauemarcondes@users.noreply.github.com> Date: Thu, 16 Jul 2020 17:40:42 +0100 Subject: [PATCH] [Observability] not showing empty state when apps return hasData false (#71844) (#72065) * show empty state when has data returns false * adding return type * addressing pr comments Co-authored-by: Elastic Machine Co-authored-by: Elastic Machine --- .../observability/public/data_handler.test.ts | 182 +++++++++++++++++- .../observability/public/data_handler.ts | 26 ++- .../pages/overview/overview.stories.tsx | 16 +- .../public/services/get_news_feed.test.ts | 9 + .../services/get_observability_alerts.test.ts | 9 + 5 files changed, 229 insertions(+), 13 deletions(-) diff --git a/x-pack/plugins/observability/public/data_handler.test.ts b/x-pack/plugins/observability/public/data_handler.test.ts index 7170ffe1486dc..935fc0682c414 100644 --- a/x-pack/plugins/observability/public/data_handler.test.ts +++ b/x-pack/plugins/observability/public/data_handler.test.ts @@ -3,8 +3,19 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ -import { registerDataHandler, getDataHandler } from './data_handler'; +import { + registerDataHandler, + getDataHandler, + unregisterDataHandler, + fetchHasData, +} from './data_handler'; import moment from 'moment'; +import { + ApmFetchDataResponse, + LogsFetchDataResponse, + MetricsFetchDataResponse, + UptimeFetchDataResponse, +} from './typings'; const params = { absoluteTime: { @@ -19,6 +30,16 @@ const params = { }; describe('registerDataHandler', () => { + const originalConsole = global.console; + beforeAll(() => { + // mocks console to avoid poluting the test output + global.console = ({ error: jest.fn() } as unknown) as typeof console; + }); + + afterAll(() => { + global.console = originalConsole; + }); + describe('APM', () => { registerDataHandler({ appName: 'apm', @@ -369,4 +390,163 @@ describe('registerDataHandler', () => { expect(hasData).toBeTruthy(); }); }); + describe('fetchHasData', () => { + it('returns false when an exception happens', async () => { + unregisterDataHandler({ appName: 'apm' }); + unregisterDataHandler({ appName: 'infra_logs' }); + unregisterDataHandler({ appName: 'infra_metrics' }); + unregisterDataHandler({ appName: 'uptime' }); + + registerDataHandler({ + appName: 'apm', + fetchData: async () => (({} as unknown) as ApmFetchDataResponse), + hasData: async () => { + throw new Error('BOOM'); + }, + }); + registerDataHandler({ + appName: 'infra_logs', + fetchData: async () => (({} as unknown) as LogsFetchDataResponse), + hasData: async () => { + throw new Error('BOOM'); + }, + }); + registerDataHandler({ + appName: 'infra_metrics', + fetchData: async () => (({} as unknown) as MetricsFetchDataResponse), + hasData: async () => { + throw new Error('BOOM'); + }, + }); + registerDataHandler({ + appName: 'uptime', + fetchData: async () => (({} as unknown) as UptimeFetchDataResponse), + hasData: async () => { + throw new Error('BOOM'); + }, + }); + expect(await fetchHasData()).toEqual({ + apm: false, + uptime: false, + infra_logs: false, + infra_metrics: false, + }); + }); + it('returns true when has data and false when an exception happens', async () => { + unregisterDataHandler({ appName: 'apm' }); + unregisterDataHandler({ appName: 'infra_logs' }); + unregisterDataHandler({ appName: 'infra_metrics' }); + unregisterDataHandler({ appName: 'uptime' }); + + registerDataHandler({ + appName: 'apm', + fetchData: async () => (({} as unknown) as ApmFetchDataResponse), + hasData: async () => true, + }); + registerDataHandler({ + appName: 'infra_logs', + fetchData: async () => (({} as unknown) as LogsFetchDataResponse), + hasData: async () => true, + }); + registerDataHandler({ + appName: 'infra_metrics', + fetchData: async () => (({} as unknown) as MetricsFetchDataResponse), + hasData: async () => { + throw new Error('BOOM'); + }, + }); + registerDataHandler({ + appName: 'uptime', + fetchData: async () => (({} as unknown) as UptimeFetchDataResponse), + hasData: async () => { + throw new Error('BOOM'); + }, + }); + expect(await fetchHasData()).toEqual({ + apm: true, + uptime: false, + infra_logs: true, + infra_metrics: false, + }); + }); + it('returns true when has data', async () => { + unregisterDataHandler({ appName: 'apm' }); + unregisterDataHandler({ appName: 'infra_logs' }); + unregisterDataHandler({ appName: 'infra_metrics' }); + unregisterDataHandler({ appName: 'uptime' }); + + registerDataHandler({ + appName: 'apm', + fetchData: async () => (({} as unknown) as ApmFetchDataResponse), + hasData: async () => true, + }); + registerDataHandler({ + appName: 'infra_logs', + fetchData: async () => (({} as unknown) as LogsFetchDataResponse), + hasData: async () => true, + }); + registerDataHandler({ + appName: 'infra_metrics', + fetchData: async () => (({} as unknown) as MetricsFetchDataResponse), + hasData: async () => true, + }); + registerDataHandler({ + appName: 'uptime', + fetchData: async () => (({} as unknown) as UptimeFetchDataResponse), + hasData: async () => true, + }); + expect(await fetchHasData()).toEqual({ + apm: true, + uptime: true, + infra_logs: true, + infra_metrics: true, + }); + }); + it('returns false when has no data', async () => { + unregisterDataHandler({ appName: 'apm' }); + unregisterDataHandler({ appName: 'infra_logs' }); + unregisterDataHandler({ appName: 'infra_metrics' }); + unregisterDataHandler({ appName: 'uptime' }); + + registerDataHandler({ + appName: 'apm', + fetchData: async () => (({} as unknown) as ApmFetchDataResponse), + hasData: async () => false, + }); + registerDataHandler({ + appName: 'infra_logs', + fetchData: async () => (({} as unknown) as LogsFetchDataResponse), + hasData: async () => false, + }); + registerDataHandler({ + appName: 'infra_metrics', + fetchData: async () => (({} as unknown) as MetricsFetchDataResponse), + hasData: async () => false, + }); + registerDataHandler({ + appName: 'uptime', + fetchData: async () => (({} as unknown) as UptimeFetchDataResponse), + hasData: async () => false, + }); + expect(await fetchHasData()).toEqual({ + apm: false, + uptime: false, + infra_logs: false, + infra_metrics: false, + }); + }); + it('returns false when has data was not registered', async () => { + unregisterDataHandler({ appName: 'apm' }); + unregisterDataHandler({ appName: 'infra_logs' }); + unregisterDataHandler({ appName: 'infra_metrics' }); + unregisterDataHandler({ appName: 'uptime' }); + + expect(await fetchHasData()).toEqual({ + apm: false, + uptime: false, + infra_logs: false, + infra_metrics: false, + }); + }); + }); }); diff --git a/x-pack/plugins/observability/public/data_handler.ts b/x-pack/plugins/observability/public/data_handler.ts index 73e34f214da28..834d7a52d767f 100644 --- a/x-pack/plugins/observability/public/data_handler.ts +++ b/x-pack/plugins/observability/public/data_handler.ts @@ -28,9 +28,27 @@ export function getDataHandler(appName: T) { } } -export async function fetchHasData() { +export async function fetchHasData(): Promise> { const apps: ObservabilityApp[] = ['apm', 'uptime', 'infra_logs', 'infra_metrics']; - const promises = apps.map((app) => getDataHandler(app)?.hasData()); - const [apm, uptime, logs, metrics] = await Promise.allSettled(promises); - return { apm, uptime, infra_logs: logs, infra_metrics: metrics }; + + const promises = apps.map(async (app) => getDataHandler(app)?.hasData() || false); + + const results = await Promise.allSettled(promises); + + const [apm, uptime, logs, metrics] = results.map((result) => { + if (result.status === 'fulfilled') { + return result.value; + } + + // eslint-disable-next-line no-console + console.error('Error while fetching has data', result.reason); + return false; + }); + + return { + apm, + uptime, + infra_logs: logs, + infra_metrics: metrics, + }; } diff --git a/x-pack/plugins/observability/public/pages/overview/overview.stories.tsx b/x-pack/plugins/observability/public/pages/overview/overview.stories.tsx index 896cad7b72ecd..79f3c63a98051 100644 --- a/x-pack/plugins/observability/public/pages/overview/overview.stories.tsx +++ b/x-pack/plugins/observability/public/pages/overview/overview.stories.tsx @@ -495,7 +495,7 @@ storiesOf('app/Overview', module) fetchData: fetchApmData, // @ts-ignore thows an error instead hasData: async () => { - new Error('Error has data'); + throw new Error('Error has data'); }, }); registerDataHandler({ @@ -503,7 +503,7 @@ storiesOf('app/Overview', module) fetchData: fetchLogsData, // @ts-ignore thows an error instead hasData: async () => { - new Error('Error has data'); + throw new Error('Error has data'); }, }); registerDataHandler({ @@ -511,7 +511,7 @@ storiesOf('app/Overview', module) fetchData: fetchMetricsData, // @ts-ignore thows an error instead hasData: async () => { - new Error('Error has data'); + throw new Error('Error has data'); }, }); registerDataHandler({ @@ -519,7 +519,7 @@ storiesOf('app/Overview', module) fetchData: fetchUptimeData, // @ts-ignore thows an error instead hasData: async () => { - new Error('Error has data'); + throw new Error('Error has data'); }, }); return ( @@ -545,7 +545,7 @@ storiesOf('app/Overview', module) fetchData: fetchApmData, // @ts-ignore thows an error instead hasData: async () => { - new Error('Error has data'); + throw new Error('Error has data'); }, }); registerDataHandler({ @@ -553,7 +553,7 @@ storiesOf('app/Overview', module) fetchData: fetchLogsData, // @ts-ignore thows an error instead hasData: async () => { - new Error('Error has data'); + throw new Error('Error has data'); }, }); registerDataHandler({ @@ -561,7 +561,7 @@ storiesOf('app/Overview', module) fetchData: fetchMetricsData, // @ts-ignore thows an error instead hasData: async () => { - new Error('Error has data'); + throw new Error('Error has data'); }, }); registerDataHandler({ @@ -569,7 +569,7 @@ storiesOf('app/Overview', module) fetchData: fetchUptimeData, // @ts-ignore thows an error instead hasData: async () => { - new Error('Error has data'); + throw new Error('Error has data'); }, }); return ( diff --git a/x-pack/plugins/observability/public/services/get_news_feed.test.ts b/x-pack/plugins/observability/public/services/get_news_feed.test.ts index 49eb2da803ab6..eca0108095b24 100644 --- a/x-pack/plugins/observability/public/services/get_news_feed.test.ts +++ b/x-pack/plugins/observability/public/services/get_news_feed.test.ts @@ -7,6 +7,15 @@ import { getNewsFeed } from './get_news_feed'; import { AppMountContext } from 'kibana/public'; describe('getNewsFeed', () => { + const originalConsole = global.console; + beforeAll(() => { + // mocks console to avoid poluting the test output + global.console = ({ error: jest.fn() } as unknown) as typeof console; + }); + + afterAll(() => { + global.console = originalConsole; + }); it('Returns empty array when api throws exception', async () => { const core = ({ http: { diff --git a/x-pack/plugins/observability/public/services/get_observability_alerts.test.ts b/x-pack/plugins/observability/public/services/get_observability_alerts.test.ts index 2027c9fa4a25d..64f5f4aab1c2b 100644 --- a/x-pack/plugins/observability/public/services/get_observability_alerts.test.ts +++ b/x-pack/plugins/observability/public/services/get_observability_alerts.test.ts @@ -10,6 +10,15 @@ import { getObservabilityAlerts } from './get_observability_alerts'; const basePath = { prepend: (path: string) => path }; describe('getObservabilityAlerts', () => { + const originalConsole = global.console; + beforeAll(() => { + // mocks console to avoid poluting the test output + global.console = ({ error: jest.fn() } as unknown) as typeof console; + }); + + afterAll(() => { + global.console = originalConsole; + }); it('Returns empty array when api throws exception', async () => { const core = ({ http: {