From 23ca49555fa6553bc5582ed87446877c28fe711f Mon Sep 17 00:00:00 2001 From: Maryam Saeidi Date: Thu, 15 Dec 2022 16:11:52 +0100 Subject: [PATCH] [AO] Refactor observability alert search bar (#147461) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## ๐Ÿ“ Summary In the previous [PR](https://github.com/elastic/kibana/pull/146401), I've added a provider to pass Kibana dependencies to the observability alert search bar component. Since then, I had a lot of discussions about whether it makes sense to have a context for the dependencies of this component or not. Since we haven't decided yet whether this pattern should be adopted or not and there are suggestions about having one provider for the alert-related components, I refactored it to pass the dependency via props instead. ## ๐Ÿงช How to test Add this component to [APM](https://github.com/elastic/kibana/blob/main/x-pack/plugins/apm/public/components/app/alerts_overview/index.tsx): ``` // Add import { ObservabilityAlertSearchBar } from '@kbn/observability-plugin/public'; export const useToasts = () => useKibana().services.notifications!.toasts; // Update const { triggersActionsUi: { getAlertsStateTable: AlertsStateTable, getAlertsSearchBar: AlertsSearchBar, alertsTableConfigurationRegistry, }, data: { query: { timefilter: { timefilter: timeFilterService }, }, }, } = services; // Replace // console.log(input)} onRangeToChange={(input) => console.log(input)} onKueryChange={(input) => console.log(input)} onStatusChange={(input) => console.log(input)} onEsQueryChange={(input) => console.log(input)} rangeTo={'now'} rangeFrom={'now-15m'} status={'all'} services={{ timeFilterService, AlertsSearchBar, useToasts }} /> ``` --- .../alert_search_bar.test.tsx | 25 +++-------- .../alert_search_bar/alert_search_bar.tsx | 3 +- .../alert_search_bar_with_url_sync.tsx | 22 +++++---- .../shared/alert_search_bar/services.tsx | 45 ------------------- .../shared/alert_search_bar/types.ts | 11 ++--- x-pack/plugins/observability/public/index.ts | 1 - 6 files changed, 27 insertions(+), 80 deletions(-) delete mode 100644 x-pack/plugins/observability/public/components/shared/alert_search_bar/services.tsx diff --git a/x-pack/plugins/observability/public/components/shared/alert_search_bar/alert_search_bar.test.tsx b/x-pack/plugins/observability/public/components/shared/alert_search_bar/alert_search_bar.test.tsx index d12bee2f87e47..3e6d32d6a569c 100644 --- a/x-pack/plugins/observability/public/components/shared/alert_search_bar/alert_search_bar.test.tsx +++ b/x-pack/plugins/observability/public/components/shared/alert_search_bar/alert_search_bar.test.tsx @@ -8,28 +8,14 @@ import React from 'react'; import { waitFor } from '@testing-library/react'; import { timefilterServiceMock } from '@kbn/data-plugin/public/query/timefilter/timefilter_service.mock'; -import { useServices } from './services'; import { ObservabilityAlertSearchBarProps } from './types'; import { ObservabilityAlertSearchBar } from './alert_search_bar'; import { observabilityAlertFeatureIds } from '../../../config'; import { render } from '../../../utils/test_helper'; -const useServicesMock = useServices as jest.Mock; const getAlertsSearchBarMock = jest.fn(); const ALERT_SEARCH_BAR_DATA_TEST_SUBJ = 'alerts-search-bar'; -jest.mock('./services'); - -const mockServices = () => { - useServicesMock.mockReturnValue({ - timeFilterService: timefilterServiceMock, - AlertsSearchBar: getAlertsSearchBarMock.mockReturnValue( -
- ), - useToasts: jest.fn(), - }); -}; - describe('ObservabilityAlertSearchBar', () => { const renderComponent = (props: Partial = {}) => { const observabilityAlertSearchBarProps: ObservabilityAlertSearchBarProps = { @@ -43,15 +29,18 @@ describe('ObservabilityAlertSearchBar', () => { rangeTo: 'now', rangeFrom: 'now-15m', status: 'all', + services: { + timeFilterService: timefilterServiceMock.createStartContract().timefilter, + AlertsSearchBar: getAlertsSearchBarMock.mockReturnValue( +
+ ), + useToasts: jest.fn(), + }, ...props, }; return render(); }; - beforeAll(() => { - mockServices(); - }); - beforeEach(() => { jest.clearAllMocks(); }); diff --git a/x-pack/plugins/observability/public/components/shared/alert_search_bar/alert_search_bar.tsx b/x-pack/plugins/observability/public/components/shared/alert_search_bar/alert_search_bar.tsx index 3d8de61482694..4d154504ce6ea 100644 --- a/x-pack/plugins/observability/public/components/shared/alert_search_bar/alert_search_bar.tsx +++ b/x-pack/plugins/observability/public/components/shared/alert_search_bar/alert_search_bar.tsx @@ -10,7 +10,6 @@ import React, { useCallback, useEffect } from 'react'; import { i18n } from '@kbn/i18n'; import { Query } from '@kbn/es-query'; -import { useServices } from './services'; import { AlertsStatusFilter } from './components'; import { observabilityAlertFeatureIds } from '../../../config'; import { ALERT_STATUS_QUERY, DEFAULT_QUERIES, DEFAULT_QUERY_STRING } from './constants'; @@ -35,9 +34,9 @@ export function ObservabilityAlertSearchBar({ kuery, rangeFrom, rangeTo, + services: { AlertsSearchBar, timeFilterService, useToasts }, status, }: ObservabilityAlertSearchBarProps) { - const { AlertsSearchBar, timeFilterService, useToasts } = useServices(); const toasts = useToasts(); const onAlertStatusChange = useCallback( diff --git a/x-pack/plugins/observability/public/components/shared/alert_search_bar/alert_search_bar_with_url_sync.tsx b/x-pack/plugins/observability/public/components/shared/alert_search_bar/alert_search_bar_with_url_sync.tsx index 405382f49b47f..24f8dcd99c83a 100644 --- a/x-pack/plugins/observability/public/components/shared/alert_search_bar/alert_search_bar_with_url_sync.tsx +++ b/x-pack/plugins/observability/public/components/shared/alert_search_bar/alert_search_bar_with_url_sync.tsx @@ -12,7 +12,6 @@ import { useAlertSearchBarStateContainer, } from './containers'; import { ObservabilityAlertSearchBar } from './alert_search_bar'; -import { ObservabilityAlertSearchBarProvider } from './services'; import { AlertSearchBarWithUrlSyncProps } from './types'; import { useKibana } from '../../../utils/kibana_react'; import { ObservabilityAppServices } from '../../../application/types'; @@ -21,16 +20,21 @@ import { useToasts } from '../../../hooks/use_toast'; function AlertSearchbarWithUrlSync(props: AlertSearchBarWithUrlSyncProps) { const { urlStorageKey, ...searchBarProps } = props; const stateProps = useAlertSearchBarStateContainer(urlStorageKey); - const { data, triggersActionsUi } = useKibana().services; + const { + data: { + query: { + timefilter: { timefilter: timeFilterService }, + }, + }, + triggersActionsUi: { getAlertsSearchBar: AlertsSearchBar }, + } = useKibana().services; return ( - - - + ); } diff --git a/x-pack/plugins/observability/public/components/shared/alert_search_bar/services.tsx b/x-pack/plugins/observability/public/components/shared/alert_search_bar/services.tsx deleted file mode 100644 index 186178d6747d2..0000000000000 --- a/x-pack/plugins/observability/public/components/shared/alert_search_bar/services.tsx +++ /dev/null @@ -1,45 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -import React, { FC, useContext } from 'react'; -import { ObservabilityAlertSearchBarDependencies, Services } from './types'; - -const ObservabilityAlertSearchBarContext = React.createContext(null); - -export const ObservabilityAlertSearchBarProvider: FC = ({ - children, - data: { - query: { - timefilter: { timefilter: timeFilterService }, - }, - }, - useToasts, - triggersActionsUi: { getAlertsSearchBar: AlertsSearchBar }, -}) => { - const services = { - timeFilterService, - useToasts, - AlertsSearchBar, - }; - return ( - - {children} - - ); -}; - -export function useServices() { - const context = useContext(ObservabilityAlertSearchBarContext); - - if (!context) { - throw new Error( - 'ObservabilityAlertSearchBarContext is missing. Ensure your component or React root is wrapped with ObservabilityAlertSearchBarProvider.' - ); - } - - return context; -} diff --git a/x-pack/plugins/observability/public/components/shared/alert_search_bar/types.ts b/x-pack/plugins/observability/public/components/shared/alert_search_bar/types.ts index be3ccce505505..51e3e5fa48bf9 100644 --- a/x-pack/plugins/observability/public/components/shared/alert_search_bar/types.ts +++ b/x-pack/plugins/observability/public/components/shared/alert_search_bar/types.ts @@ -33,17 +33,18 @@ export interface Dependencies { useToasts: () => ToastsStart; } -export type ObservabilityAlertSearchBarDependencies = Dependencies; - export interface Services { timeFilterService: TimefilterContract; AlertsSearchBar: (props: AlertsSearchBarProps) => ReactElement; useToasts: () => ToastsStart; } -export type ObservabilityAlertSearchBarProps = AlertSearchBarContainerState & - AlertSearchBarStateTransitions & - CommonAlertSearchBarProps; +export interface ObservabilityAlertSearchBarProps + extends AlertSearchBarContainerState, + AlertSearchBarStateTransitions, + CommonAlertSearchBarProps { + services: Services; +} interface AlertSearchBarContainerState { rangeFrom: string; diff --git a/x-pack/plugins/observability/public/index.ts b/x-pack/plugins/observability/public/index.ts index 2f2b273433875..62f980bfc9d32 100644 --- a/x-pack/plugins/observability/public/index.ts +++ b/x-pack/plugins/observability/public/index.ts @@ -47,7 +47,6 @@ export * from './components/shared/action_menu'; export type { UXMetrics } from './components/shared/core_web_vitals'; export { DatePickerContextProvider } from './context/date_picker_context'; -export { ObservabilityAlertSearchBarProvider } from './components/shared/alert_search_bar/services'; export { getCoreVitalsComponent,