From d4f4afa28a400afbed42271be2db50d82347d902 Mon Sep 17 00:00:00 2001 From: Umberto Pepato Date: Thu, 16 Nov 2023 16:19:41 +0100 Subject: [PATCH 01/12] Assign useLoadRuleTypesQuery a staleTime to prevent duplicate requests Remove old useLoadRuleTypes hook usages Closes #155394 --- .../public/pages/rules/rules.tsx | 6 +- .../public/application/hooks/index.ts | 2 +- .../application/hooks/use_load_rule_types.ts | 71 ------------------- .../hooks/use_load_rule_types_query.ts | 8 ++- .../components/rule_definition.tsx | 10 ++- .../sections/rule_form/rule_form.tsx | 16 +++-- .../triggers_actions_ui/public/index.ts | 2 +- 7 files changed, 28 insertions(+), 87 deletions(-) delete mode 100644 x-pack/plugins/triggers_actions_ui/public/application/hooks/use_load_rule_types.ts diff --git a/x-pack/plugins/observability/public/pages/rules/rules.tsx b/x-pack/plugins/observability/public/pages/rules/rules.tsx index fa774d96b1c4f..5d186d06d103a 100644 --- a/x-pack/plugins/observability/public/pages/rules/rules.tsx +++ b/x-pack/plugins/observability/public/pages/rules/rules.tsx @@ -10,10 +10,10 @@ import { useHistory } from 'react-router-dom'; import { EuiButton, EuiFlexGroup, EuiFlexItem, EuiButtonEmpty } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n-react'; -import { useLoadRuleTypes } from '@kbn/triggers-actions-ui-plugin/public'; import { ALERTS_FEATURE_ID } from '@kbn/alerting-plugin/common'; import { useBreadcrumbs } from '@kbn/observability-shared-plugin/public'; +import { useLoadRuleTypesQuery } from '@kbn/triggers-actions-ui-plugin/public/application/hooks/use_load_rule_types_query'; import { RULES_LOGS_PATH, RULES_PATH } from '../../../common/locators/paths'; import { useKibana } from '../../utils/kibana_react'; import { usePluginContext } from '../../hooks/use_plugin_context'; @@ -56,7 +56,9 @@ export function RulesPage({ activeTab = RULES_TAB_NAME }: RulesPageProps) { ]); const filteredRuleTypes = useGetFilteredRuleTypes(); - const { ruleTypes } = useLoadRuleTypes({ + const { + ruleTypesState: { data: ruleTypes }, + } = useLoadRuleTypesQuery({ filteredRuleTypes, }); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/hooks/index.ts b/x-pack/plugins/triggers_actions_ui/public/application/hooks/index.ts index bc3d1c35c09ed..28a01ce8ed7b8 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/hooks/index.ts +++ b/x-pack/plugins/triggers_actions_ui/public/application/hooks/index.ts @@ -6,4 +6,4 @@ */ export { useSubAction } from './use_sub_action'; -export { useLoadRuleTypes } from './use_load_rule_types'; +export { useLoadRuleTypesQuery } from './use_load_rule_types_query'; diff --git a/x-pack/plugins/triggers_actions_ui/public/application/hooks/use_load_rule_types.ts b/x-pack/plugins/triggers_actions_ui/public/application/hooks/use_load_rule_types.ts deleted file mode 100644 index bce0b9ec4ff66..0000000000000 --- a/x-pack/plugins/triggers_actions_ui/public/application/hooks/use_load_rule_types.ts +++ /dev/null @@ -1,71 +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 { useEffect, useState, useRef } from 'react'; -import { loadRuleTypes } from '../lib/rule_api/rule_types'; -import { RuleType, RuleTypeIndex } from '../../types'; -import { useKibana } from '../../common/lib/kibana'; - -interface RuleTypesState { - isLoading: boolean; - data: Array>; - error: string | null; -} - -interface RuleTypesProps { - filteredRuleTypes?: string[]; -} - -export function useLoadRuleTypes({ filteredRuleTypes }: RuleTypesProps) { - const { http } = useKibana().services; - const isMounted = useRef(false); - const [ruleTypesState, setRuleTypesState] = useState({ - isLoading: true, - data: [], - error: null, - }); - const [ruleTypeIndex, setRuleTypeIndex] = useState(new Map()); - - async function fetchRuleTypes() { - try { - const response = await loadRuleTypes({ http }); - const index: RuleTypeIndex = new Map(); - for (const ruleTypeItem of response) { - index.set(ruleTypeItem.id, ruleTypeItem); - } - if (isMounted.current) { - setRuleTypeIndex(index); - - let filteredResponse = response; - - if (filteredRuleTypes && filteredRuleTypes.length > 0) { - filteredResponse = response.filter((item) => filteredRuleTypes.includes(item.id)); - } - setRuleTypesState({ ...ruleTypesState, isLoading: false, data: filteredResponse }); - } - } catch (e) { - if (isMounted.current) { - setRuleTypesState({ ...ruleTypesState, isLoading: false, error: e }); - } - } - } - - useEffect(() => { - isMounted.current = true; - fetchRuleTypes(); - return () => { - isMounted.current = false; - }; - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); - - return { - ruleTypes: ruleTypesState.data, - error: ruleTypesState.error, - ruleTypeIndex, - ruleTypesIsLoading: ruleTypesState.isLoading, - }; -} diff --git a/x-pack/plugins/triggers_actions_ui/public/application/hooks/use_load_rule_types_query.ts b/x-pack/plugins/triggers_actions_ui/public/application/hooks/use_load_rule_types_query.ts index 4892341e57385..b5b99f9b0723e 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/hooks/use_load_rule_types_query.ts +++ b/x-pack/plugins/triggers_actions_ui/public/application/hooks/use_load_rule_types_query.ts @@ -50,14 +50,17 @@ export const useLoadRuleTypesQuery = (props: UseLoadRuleTypesQueryProps) => { ); }; - const { data, isSuccess, isFetching, isInitialLoading, isLoading } = useQuery({ + const { data, isSuccess, isFetching, isInitialLoading, isLoading, error } = useQuery({ queryKey: ['loadRuleTypes'], queryFn, onError: onErrorFn, refetchOnWindowFocus: false, + staleTime: 60 * 1000, }); - const filteredIndex = data ? getFilteredIndex(data, filteredRuleTypes) : new Map(); + const filteredIndex = data + ? getFilteredIndex(data, filteredRuleTypes) + : new Map(); const hasAnyAuthorizedRuleType = filteredIndex.size > 0; const authorizedRuleTypes = [...filteredIndex.values()]; @@ -73,6 +76,7 @@ export const useLoadRuleTypesQuery = (props: UseLoadRuleTypesQueryProps) => { initialLoad: isLoading || isInitialLoading, isLoading: isLoading || isFetching, data: filteredIndex, + error, }, hasAnyAuthorizedRuleType, authorizedRuleTypes, diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/rule_definition.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/rule_definition.tsx index 721118b1d6f0e..0b38f6942ebf1 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/rule_definition.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/rule_definition.tsx @@ -18,8 +18,9 @@ import { import { AlertConsumers } from '@kbn/rule-data-utils'; import { i18n } from '@kbn/i18n'; import { formatDuration } from '@kbn/alerting-plugin/common'; +import { useLoadRuleTypesQuery } from '../../../hooks/use_load_rule_types_query'; import { RuleDefinitionProps } from '../../../../types'; -import { RuleType, useLoadRuleTypes } from '../../../..'; +import { RuleType } from '../../../..'; import { useKibana } from '../../../../common/lib/kibana'; import { hasAllPrivilege, @@ -35,7 +36,7 @@ export const RuleDefinition: React.FunctionComponent = ({ ruleTypeRegistry, onEditRule, hideEditButton = false, - filteredRuleTypes, + filteredRuleTypes = [], }) => { const { application: { capabilities }, @@ -43,9 +44,12 @@ export const RuleDefinition: React.FunctionComponent = ({ const [editFlyoutVisible, setEditFlyoutVisible] = useState(false); const [ruleType, setRuleType] = useState(); - const { ruleTypes, ruleTypeIndex, ruleTypesIsLoading } = useLoadRuleTypes({ + const { + ruleTypesState: { data: ruleTypeIndex, isLoading: ruleTypesIsLoading }, + } = useLoadRuleTypesQuery({ filteredRuleTypes, }); + const ruleTypes = useMemo(() => [...ruleTypeIndex.values()], [ruleTypeIndex]); const getRuleType = useMemo(() => { if (ruleTypes.length && rule) { diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form.tsx index c321ba5a01d7a..378212af8899d 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form.tsx @@ -90,8 +90,8 @@ import { VIEW_LICENSE_OPTIONS_LINK } from '../../../common/constants'; import { MULTI_CONSUMER_RULE_TYPE_IDS } from '../../constants'; import { SectionLoading } from '../../components/section_loading'; import { RuleFormConsumerSelection, VALID_CONSUMERS } from './rule_form_consumer_selection'; -import { useLoadRuleTypes } from '../../hooks/use_load_rule_types'; import { getInitialInterval } from './get_initial_interval'; +import { useLoadRuleTypesQuery } from '../../hooks/use_load_rule_types_query'; const ENTER_KEY = 13; @@ -172,7 +172,7 @@ export const RuleForm = ({ ruleTypeRegistry, actionTypeRegistry, metadata, - filteredRuleTypes: ruleTypeToFilter, + filteredRuleTypes: ruleTypeToFilter = [], hideGrouping = false, hideInterval, connectorFeatureId = AlertingConnectorFeatureId, @@ -219,11 +219,13 @@ export const RuleForm = ({ const [solutionsFilter, setSolutionFilter] = useState([]); let hasDisabledByLicenseRuleTypes: boolean = false; const { - ruleTypes, - error: loadRuleTypesError, - ruleTypeIndex, - ruleTypesIsLoading, - } = useLoadRuleTypes({ filteredRuleTypes: ruleTypeToFilter }); + ruleTypesState: { + data: ruleTypeIndex, + error: loadRuleTypesError, + isLoading: ruleTypesIsLoading, + }, + } = useLoadRuleTypesQuery({ filteredRuleTypes: ruleTypeToFilter }); + const ruleTypes = useMemo(() => [...ruleTypeIndex.values()], [ruleTypeIndex]); // load rule types useEffect(() => { diff --git a/x-pack/plugins/triggers_actions_ui/public/index.ts b/x-pack/plugins/triggers_actions_ui/public/index.ts index 7dd3d36ce6c06..42fa6366878c7 100644 --- a/x-pack/plugins/triggers_actions_ui/public/index.ts +++ b/x-pack/plugins/triggers_actions_ui/public/index.ts @@ -124,7 +124,7 @@ export { deprecatedMessage, } from './common'; -export { useLoadRuleTypes, useSubAction } from './application/hooks'; +export { useLoadRuleTypesQuery, useSubAction } from './application/hooks'; export type { TriggersAndActionsUIPublicPluginSetup, From acd6c2224f718ab04d21abddc76b3481684c9d48 Mon Sep 17 00:00:00 2001 From: Umberto Pepato Date: Thu, 16 Nov 2023 16:21:32 +0100 Subject: [PATCH 02/12] Hide Logs tab in Stack Mgmt Rules page when missing the necessary privileges Closes #158256 --- .../public/application/home.tsx | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/triggers_actions_ui/public/application/home.tsx b/x-pack/plugins/triggers_actions_ui/public/application/home.tsx index d55e66644a5ae..585327ff340c9 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/home.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/home.tsx @@ -21,6 +21,7 @@ import { HealthCheck } from './components/health_check'; import { HealthContextProvider } from './context/health_context'; import { useKibana } from '../common/lib/kibana'; import { suspendedComponentWithProps } from './lib/suspended_component_with_props'; +import { useLoadRuleTypesQuery } from './hooks/use_load_rule_types_query'; const RulesList = lazy(() => import('./sections/rules_list/components/rules_list')); const LogsList = lazy( @@ -41,6 +42,7 @@ export const TriggersActionsUIHome: React.FunctionComponent(); const { chrome, setBreadcrumbs } = useKibana().services; const isInternalAlertsTableEnabled = getIsExperimentalFeatureEnabled('internalAlertsTable'); + const { authorizedToReadAnyRules } = useLoadRuleTypesQuery({ filteredRuleTypes: [] }); const tabs: Array<{ id: Section; @@ -54,10 +56,14 @@ export const TriggersActionsUIHome: React.FunctionComponent, - }); + if (authorizedToReadAnyRules) { + tabs.push({ + id: 'logs', + name: ( + + ), + }); + } if (isInternalAlertsTableEnabled) { tabs.push({ From 3dae8bb38c82d46b0d64e1d640fb88833f761a41 Mon Sep 17 00:00:00 2001 From: Umberto Pepato Date: Thu, 16 Nov 2023 16:44:37 +0100 Subject: [PATCH 03/12] Fix wrong useLoadRuleTypesQuery import --- x-pack/plugins/observability/public/pages/rules/rules.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/observability/public/pages/rules/rules.tsx b/x-pack/plugins/observability/public/pages/rules/rules.tsx index 5d186d06d103a..5ee2a8f9732ef 100644 --- a/x-pack/plugins/observability/public/pages/rules/rules.tsx +++ b/x-pack/plugins/observability/public/pages/rules/rules.tsx @@ -13,7 +13,7 @@ import { FormattedMessage } from '@kbn/i18n-react'; import { ALERTS_FEATURE_ID } from '@kbn/alerting-plugin/common'; import { useBreadcrumbs } from '@kbn/observability-shared-plugin/public'; -import { useLoadRuleTypesQuery } from '@kbn/triggers-actions-ui-plugin/public/application/hooks/use_load_rule_types_query'; +import { useLoadRuleTypesQuery } from '@kbn/triggers-actions-ui-plugin/public'; import { RULES_LOGS_PATH, RULES_PATH } from '../../../common/locators/paths'; import { useKibana } from '../../utils/kibana_react'; import { usePluginContext } from '../../hooks/use_plugin_context'; From 25e617e070a612f8ac86cba04112c3dbcfe32659 Mon Sep 17 00:00:00 2001 From: Umberto Pepato Date: Tue, 21 Nov 2023 10:25:30 +0100 Subject: [PATCH 04/12] Fix failing tests related to useLoadRuleTypesQuery --- .../public/alerts/alert_form.test.tsx | 47 +++++--- .../public/pages/rules/rules.test.tsx | 108 +++++++++++------- .../hooks/use_load_rule_types_query.ts | 13 ++- .../components/rule_definition.test.tsx | 74 +++++++----- .../sections/rule_form/rule_form.test.tsx | 108 ++++++++++-------- .../sections/rule_form/rule_form.tsx | 4 +- 6 files changed, 216 insertions(+), 138 deletions(-) diff --git a/x-pack/plugins/monitoring/public/alerts/alert_form.test.tsx b/x-pack/plugins/monitoring/public/alerts/alert_form.test.tsx index 0113391558854..5093a70eef302 100644 --- a/x-pack/plugins/monitoring/public/alerts/alert_form.test.tsx +++ b/x-pack/plugins/monitoring/public/alerts/alert_form.test.tsx @@ -27,6 +27,7 @@ import ActionForm from '@kbn/triggers-actions-ui-plugin/public/application/secti import { Legacy } from '../legacy_shims'; import { I18nProvider } from '@kbn/i18n-react'; import { createKibanaReactContext } from '@kbn/kibana-react-plugin/public'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; interface AlertAction { group: string; @@ -43,9 +44,12 @@ const { loadActionTypes } = jest.requireMock( '@kbn/triggers-actions-ui-plugin/public/application/lib/action_connector_api' ); -jest.mock('@kbn/triggers-actions-ui-plugin/public/application/lib/rule_api', () => ({ - loadAlertTypes: jest.fn(), +jest.mock('@kbn/triggers-actions-ui-plugin/public/application/lib/rule_api/rule_types', () => ({ + loadRuleTypes: jest.fn(), })); +const { loadRuleTypes } = jest.requireMock( + '@kbn/triggers-actions-ui-plugin/public/application/lib/rule_api/rule_types' +); jest.mock('@kbn/kibana-react-plugin/public/ui_settings/use_ui_setting', () => ({ useUiSetting: jest.fn().mockImplementation((_, defaultValue) => defaultValue), @@ -109,6 +113,7 @@ describe('alert_form', () => { let wrapper: ReactWrapper; beforeEach(async () => { + loadRuleTypes.mockResolvedValue([]); ruleTypeRegistry.list.mockReturnValue([ruleType]); ruleTypeRegistry.get.mockReturnValue(ruleType); ruleTypeRegistry.has.mockReturnValue(true); @@ -136,19 +141,31 @@ describe('alert_form', () => { wrapper = mountWithIntl( - {}} - errors={{ name: [], 'schedule.interval': [] }} - operation="create" - actionTypeRegistry={actionTypeRegistry} - ruleTypeRegistry={ruleTypeRegistry} - onChangeMetaData={() => {}} - /> + + {}} + errors={{ name: [], 'schedule.interval': [] }} + operation="create" + actionTypeRegistry={actionTypeRegistry} + ruleTypeRegistry={ruleTypeRegistry} + onChangeMetaData={() => {}} + /> + ); diff --git a/x-pack/plugins/observability/public/pages/rules/rules.test.tsx b/x-pack/plugins/observability/public/pages/rules/rules.test.tsx index 4239cdeee811e..8be89be03d57b 100644 --- a/x-pack/plugins/observability/public/pages/rules/rules.test.tsx +++ b/x-pack/plugins/observability/public/pages/rules/rules.test.tsx @@ -28,7 +28,7 @@ jest.mock('../../utils/kibana_react', () => ({ jest.mock('@kbn/observability-shared-plugin/public'); jest.mock('@kbn/triggers-actions-ui-plugin/public', () => ({ - useLoadRuleTypes: jest.fn(), + useLoadRuleTypesQuery: jest.fn(), })); jest.spyOn(pluginContext, 'usePluginContext').mockImplementation(() => ({ @@ -55,7 +55,7 @@ jest.spyOn(pluginContext, 'usePluginContext').mockImplementation(() => ({ plugins: {} as ObservabilityPublicPluginsStart, })); -const { useLoadRuleTypes } = jest.requireMock('@kbn/triggers-actions-ui-plugin/public'); +const { useLoadRuleTypesQuery } = jest.requireMock('@kbn/triggers-actions-ui-plugin/public'); describe('RulesPage with all capabilities', () => { async function setup() { @@ -65,41 +65,54 @@ describe('RulesPage with all capabilities', () => { enabledInLicense: true, id: '1', name: 'test rule', + actionGroups: [{ id: 'default', name: 'Default' }], + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, + actionVariables: { context: [], state: [] }, + defaultActionGroupId: 'default', + producer: ALERTS_FEATURE_ID, + minimumLicenseRequired: 'basic', + authorizedConsumers: { + [ALERTS_FEATURE_ID]: { all: true }, + }, + ruleTaskTimeout: '1m', }, '2': { enabledInLicense: true, id: '2', name: 'test rule ok', + actionGroups: [{ id: 'default', name: 'Default' }], + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, + actionVariables: { context: [], state: [] }, + defaultActionGroupId: 'default', + producer: ALERTS_FEATURE_ID, + minimumLicenseRequired: 'basic', + authorizedConsumers: { + [ALERTS_FEATURE_ID]: { all: true }, + }, + ruleTaskTimeout: '1m', }, '3': { enabledInLicense: true, id: '3', name: 'test rule pending', + actionGroups: [{ id: 'default', name: 'Default' }], + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, + actionVariables: { context: [], state: [] }, + defaultActionGroupId: 'default', + producer: ALERTS_FEATURE_ID, + minimumLicenseRequired: 'basic', + authorizedConsumers: { + [ALERTS_FEATURE_ID]: { all: true }, + }, + ruleTaskTimeout: '1m', }, }) ); - const ruleTypes = [ - { - id: 'test_rule_type', - name: 'some rule type', - actionGroups: [{ id: 'default', name: 'Default' }], - recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, - actionVariables: { context: [], state: [] }, - defaultActionGroupId: 'default', - producer: ALERTS_FEATURE_ID, - minimumLicenseRequired: 'basic', - enabledInLicense: true, - authorizedConsumers: { - [ALERTS_FEATURE_ID]: { all: true }, - }, - ruleTaskTimeout: '1m', + useLoadRuleTypesQuery.mockReturnValue({ + ruleTypesState: { + data: ruleTypeIndex, }, - ]; - - useLoadRuleTypes.mockReturnValue({ - ruleTypes, - ruleTypeIndex, }); return render( @@ -133,9 +146,29 @@ describe('RulesPage with show only capability', () => { enabledInLicense: true, id: '1', name: 'test rule', + actionGroups: [{ id: 'default', name: 'Default' }], + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, + actionVariables: { context: [], state: [] }, + defaultActionGroupId: 'default', + producer: ALERTS_FEATURE_ID, + minimumLicenseRequired: 'basic', + authorizedConsumers: { + [ALERTS_FEATURE_ID]: { read: true, all: false }, + }, + ruleTaskTimeout: '1m', }, '2': { enabledInLicense: true, + actionGroups: [{ id: 'default', name: 'Default' }], + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, + actionVariables: { context: [], state: [] }, + defaultActionGroupId: 'default', + producer: ALERTS_FEATURE_ID, + minimumLicenseRequired: 'basic', + authorizedConsumers: { + [ALERTS_FEATURE_ID]: { read: true, all: false }, + }, + ruleTaskTimeout: '1m', id: '2', name: 'test rule ok', }, @@ -143,28 +176,25 @@ describe('RulesPage with show only capability', () => { enabledInLicense: true, id: '3', name: 'test rule pending', + actionGroups: [{ id: 'default', name: 'Default' }], + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, + actionVariables: { context: [], state: [] }, + defaultActionGroupId: 'default', + producer: ALERTS_FEATURE_ID, + minimumLicenseRequired: 'basic', + authorizedConsumers: { + [ALERTS_FEATURE_ID]: { read: true, all: false }, + }, + ruleTaskTimeout: '1m', }, }) ); - const ruleTypes = [ - { - id: 'test_rule_type', - name: 'some rule type', - actionGroups: [{ id: 'default', name: 'Default' }], - recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, - actionVariables: { context: [], state: [] }, - defaultActionGroupId: 'default', - producer: ALERTS_FEATURE_ID, - minimumLicenseRequired: 'basic', - enabledInLicense: true, - authorizedConsumers: { - [ALERTS_FEATURE_ID]: { read: true, all: false }, - }, - ruleTaskTimeout: '1m', + useLoadRuleTypesQuery.mockReturnValue({ + ruleTypesState: { + data: ruleTypeIndex, }, - ]; - useLoadRuleTypes.mockReturnValue({ ruleTypes, ruleTypeIndex }); + }); return render(); } diff --git a/x-pack/plugins/triggers_actions_ui/public/application/hooks/use_load_rule_types_query.ts b/x-pack/plugins/triggers_actions_ui/public/application/hooks/use_load_rule_types_query.ts index b5b99f9b0723e..8e7e551db771c 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/hooks/use_load_rule_types_query.ts +++ b/x-pack/plugins/triggers_actions_ui/public/application/hooks/use_load_rule_types_query.ts @@ -7,6 +7,7 @@ import { i18n } from '@kbn/i18n'; import { useQuery } from '@tanstack/react-query'; import { ALERTS_FEATURE_ID } from '@kbn/alerting-plugin/common'; +import { useMemo } from 'react'; import { loadRuleTypes } from '../lib/rule_api/rule_types'; import { useKibana } from '../../common/lib/kibana'; import { RuleType, RuleTypeIndex } from '../../types'; @@ -31,8 +32,7 @@ const getFilteredIndex = (data: Array>, filteredRuleTyp return filteredIndex; }; -export const useLoadRuleTypesQuery = (props: UseLoadRuleTypesQueryProps) => { - const { filteredRuleTypes } = props; +export const useLoadRuleTypesQuery = ({ filteredRuleTypes }: UseLoadRuleTypesQueryProps) => { const { http, notifications: { toasts }, @@ -58,12 +58,13 @@ export const useLoadRuleTypesQuery = (props: UseLoadRuleTypesQueryProps) => { staleTime: 60 * 1000, }); - const filteredIndex = data - ? getFilteredIndex(data, filteredRuleTypes) - : new Map(); + const filteredIndex = useMemo( + () => (data ? getFilteredIndex(data, filteredRuleTypes) : new Map()), + [data, filteredRuleTypes] + ); const hasAnyAuthorizedRuleType = filteredIndex.size > 0; - const authorizedRuleTypes = [...filteredIndex.values()]; + const authorizedRuleTypes = useMemo(() => [...filteredIndex.values()], [filteredIndex]); const authorizedToCreateAnyRules = authorizedRuleTypes.some( (ruleType) => ruleType.authorizedConsumers[ALERTS_FEATURE_ID]?.all ); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/rule_definition.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/rule_definition.test.tsx index 842822f98c43f..49c07a698c6b3 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/rule_definition.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_details/components/rule_definition.test.tsx @@ -13,6 +13,7 @@ import { RuleDefinition } from './rule_definition'; import { actionTypeRegistryMock } from '../../../action_type_registry.mock'; import { ActionTypeModel, Rule, RuleTypeModel } from '../../../../types'; import { ruleTypeRegistryMock } from '../../../rule_type_registry.mock'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; jest.mock('./rule_actions', () => ({ RuleActions: () => { @@ -28,27 +29,10 @@ jest.mock('../../../lib/capabilities', () => ({ hasManageApiKeysCapability: jest.fn(() => true), })); jest.mock('../../../../common/lib/kibana'); -jest.mock('../../../..', () => ({ - useLoadRuleTypes: jest.fn(), +jest.mock('../../../hooks/use_load_rule_types_query', () => ({ + useLoadRuleTypesQuery: jest.fn(), })); -const { useLoadRuleTypes } = jest.requireMock('../../../..'); -const ruleTypes = [ - { - id: 'test_rule_type', - name: 'some rule type', - actionGroups: [{ id: 'default', name: 'Default' }], - recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, - actionVariables: { context: [], state: [] }, - defaultActionGroupId: 'default', - producer: ALERTS_FEATURE_ID, - minimumLicenseRequired: 'basic', - enabledInLicense: true, - authorizedConsumers: { - [ALERTS_FEATURE_ID]: { read: true, all: false }, - }, - ruleTaskTimeout: '1m', - }, -]; +const { useLoadRuleTypesQuery } = jest.requireMock('../../../hooks/use_load_rule_types_query'); const mockedRuleTypeIndex = new Map( Object.entries({ @@ -56,16 +40,46 @@ const mockedRuleTypeIndex = new Map( enabledInLicense: true, id: 'test_rule_type', name: 'test rule', + actionGroups: [{ id: 'default', name: 'Default' }], + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, + actionVariables: { context: [], state: [] }, + defaultActionGroupId: 'default', + producer: ALERTS_FEATURE_ID, + minimumLicenseRequired: 'basic', + authorizedConsumers: { + [ALERTS_FEATURE_ID]: { read: true, all: false }, + }, + ruleTaskTimeout: '1m', }, '2': { enabledInLicense: true, id: '2', name: 'test rule ok', + actionGroups: [{ id: 'default', name: 'Default' }], + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, + actionVariables: { context: [], state: [] }, + defaultActionGroupId: 'default', + producer: ALERTS_FEATURE_ID, + minimumLicenseRequired: 'basic', + authorizedConsumers: { + [ALERTS_FEATURE_ID]: { read: true, all: false }, + }, + ruleTaskTimeout: '1m', }, '3': { enabledInLicense: true, id: '3', name: 'test rule pending', + actionGroups: [{ id: 'default', name: 'Default' }], + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, + actionVariables: { context: [], state: [] }, + defaultActionGroupId: 'default', + producer: ALERTS_FEATURE_ID, + minimumLicenseRequired: 'basic', + authorizedConsumers: { + [ALERTS_FEATURE_ID]: { read: true, all: false }, + }, + ruleTaskTimeout: '1m', }, }) ); @@ -116,15 +130,17 @@ describe('Rule Definition', () => { { id: '.index', iconClass: 'indexOpen' }, ] as ActionTypeModel[]); - useLoadRuleTypes.mockReturnValue({ ruleTypes, ruleTypeIndex: mockedRuleTypeIndex }); + useLoadRuleTypesQuery.mockReturnValue({ ruleTypesState: { data: mockedRuleTypeIndex } }); wrapper = mount( - + + + ); await act(async () => { await nextTick(); @@ -141,8 +157,8 @@ describe('Rule Definition', () => { expect(wrapper.find('[data-test-subj="ruleSummaryRuleDefinition"]')).toBeTruthy(); }); - it('show rule type name from "useLoadRuleTypes"', async () => { - expect(useLoadRuleTypes).toHaveBeenCalledTimes(2); + it('show rule type name from "useLoadRuleTypesQuery"', async () => { + expect(useLoadRuleTypesQuery).toHaveBeenCalledTimes(2); const ruleType = wrapper.find('[data-test-subj="ruleSummaryRuleType"]'); expect(ruleType).toBeTruthy(); expect(ruleType.find('div.euiText').text()).toEqual( diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form.test.tsx index 59d3c4f1f8c3b..e3ae04ec987e2 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form.test.tsx @@ -29,6 +29,11 @@ import { coreMock } from '@kbn/core/public/mocks'; import { ALERTS_FEATURE_ID, RecoveredActionGroup } from '@kbn/alerting-plugin/common'; import { useKibana } from '../../../common/lib/kibana'; +const toMapById = [ + (acc: Map, val: { id: unknown }) => acc.set(val.id, val), + new Map(), +] as const; + const actionTypeRegistry = actionTypeRegistryMock.create(); const ruleTypeRegistry = ruleTypeRegistryMock.create(); @@ -46,8 +51,8 @@ export const TestExpression: FunctionComponent = () => { ); }; -jest.mock('../../hooks/use_load_rule_types', () => ({ - useLoadRuleTypes: jest.fn(), +jest.mock('../../hooks/use_load_rule_types_query', () => ({ + useLoadRuleTypesQuery: jest.fn(), })); jest.mock('../../../common/lib/kibana'); jest.mock('../../lib/capabilities', () => ({ @@ -113,7 +118,7 @@ describe('rule_form', () => { async function setup(enforceMinimum = false, schedule = '1m') { const mocks = coreMock.createSetup(); - const { useLoadRuleTypes } = jest.requireMock('../../hooks/use_load_rule_types'); + const { useLoadRuleTypesQuery } = jest.requireMock('../../hooks/use_load_rule_types_query'); const myRuleModel = { id: 'my-rule-type', description: 'Sample rule type model', @@ -172,12 +177,13 @@ describe('rule_form', () => { }, enabledInLicense: false, }; - useLoadRuleTypes.mockReturnValue({ - ruleTypes: [myRule, disabledByLicenseRule], - ruleTypeIndex: new Map([ - [myRule.id, myRule], - [disabledByLicenseRule.id, disabledByLicenseRule], - ]), + useLoadRuleTypesQuery.mockReturnValue({ + ruleTypesState: { + data: new Map([ + [myRule.id, myRule], + [disabledByLicenseRule.id, disabledByLicenseRule], + ]), + }, }); const [ { @@ -276,7 +282,7 @@ describe('rule_form', () => { } = options || {}; const mocks = coreMock.createSetup(); - const { useLoadRuleTypes } = jest.requireMock('../../hooks/use_load_rule_types'); + const { useLoadRuleTypesQuery } = jest.requireMock('../../hooks/use_load_rule_types_query'); const ruleTypes: RuleType[] = ruleTypesOverwrite || [ { id: 'my-rule-type', @@ -325,7 +331,11 @@ describe('rule_form', () => { enabledInLicense: false, }, ]; - useLoadRuleTypes.mockReturnValue({ ruleTypes }); + useLoadRuleTypesQuery.mockReturnValue({ + ruleTypesState: { + data: ruleTypes.reduce(...toMapById), + }, + }); const [ { application: { capabilities }, @@ -672,46 +682,48 @@ describe('rule_form', () => { let wrapper: ReactWrapper; async function setup() { - const { useLoadRuleTypes } = jest.requireMock('../../hooks/use_load_rule_types'); - useLoadRuleTypes.mockReturnValue({ - ruleTypes: [ - { - id: 'other-consumer-producer-rule-type', - name: 'Test', - actionGroups: [ - { - id: 'testActionGroup', - name: 'Test Action Group', + const { useLoadRuleTypesQuery } = jest.requireMock('../../hooks/use_load_rule_types_query'); + useLoadRuleTypesQuery.mockReturnValue({ + ruleTypesState: { + data: [ + { + id: 'other-consumer-producer-rule-type', + name: 'Test', + actionGroups: [ + { + id: 'testActionGroup', + name: 'Test Action Group', + }, + ], + defaultActionGroupId: 'testActionGroup', + minimumLicenseRequired: 'basic', + recoveryActionGroup: RecoveredActionGroup, + producer: ALERTS_FEATURE_ID, + authorizedConsumers: { + [ALERTS_FEATURE_ID]: { read: true, all: true }, + test: { read: true, all: true }, }, - ], - defaultActionGroupId: 'testActionGroup', - minimumLicenseRequired: 'basic', - recoveryActionGroup: RecoveredActionGroup, - producer: ALERTS_FEATURE_ID, - authorizedConsumers: { - [ALERTS_FEATURE_ID]: { read: true, all: true }, - test: { read: true, all: true }, }, - }, - { - id: 'same-consumer-producer-rule-type', - name: 'Test', - actionGroups: [ - { - id: 'testActionGroup', - name: 'Test Action Group', + { + id: 'same-consumer-producer-rule-type', + name: 'Test', + actionGroups: [ + { + id: 'testActionGroup', + name: 'Test Action Group', + }, + ], + defaultActionGroupId: 'testActionGroup', + minimumLicenseRequired: 'basic', + recoveryActionGroup: RecoveredActionGroup, + producer: 'test', + authorizedConsumers: { + [ALERTS_FEATURE_ID]: { read: true, all: true }, + test: { read: true, all: true }, }, - ], - defaultActionGroupId: 'testActionGroup', - minimumLicenseRequired: 'basic', - recoveryActionGroup: RecoveredActionGroup, - producer: 'test', - authorizedConsumers: { - [ALERTS_FEATURE_ID]: { read: true, all: true }, - test: { read: true, all: true }, }, - }, - ], + ].reduce(...toMapById), + }, }); const mocks = coreMock.createSetup(); const [ @@ -790,7 +802,7 @@ describe('rule_form', () => { wrapper.update(); }); - expect(useLoadRuleTypes).toHaveBeenCalled(); + expect(useLoadRuleTypesQuery).toHaveBeenCalled(); } it('renders rule type options which producer correspond to the rule consumer', async () => { diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form.tsx index 378212af8899d..403f34f133e9d 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form.tsx @@ -158,6 +158,8 @@ interface RuleFormProps> { useRuleProducer?: boolean; } +const EMPTY_ARRAY: string[] = []; + export const RuleForm = ({ rule, config, @@ -172,7 +174,7 @@ export const RuleForm = ({ ruleTypeRegistry, actionTypeRegistry, metadata, - filteredRuleTypes: ruleTypeToFilter = [], + filteredRuleTypes: ruleTypeToFilter = EMPTY_ARRAY, hideGrouping = false, hideInterval, connectorFeatureId = AlertingConnectorFeatureId, From 5a33ef170674ef21fb49ab87c291c11138b9706f Mon Sep 17 00:00:00 2001 From: Umberto Pepato Date: Tue, 21 Nov 2023 18:15:48 +0100 Subject: [PATCH 05/12] Fix failing tests related to useLoadRuleTypesQuery --- .../sections/rule_form/rule_add.test.tsx | 39 ++++++++++++------- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_add.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_add.test.tsx index 4409be029092c..1bb0d09fa7d8f 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_add.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_add.test.tsx @@ -34,6 +34,7 @@ import { useKibana } from '../../../common/lib/kibana'; import { triggersActionsUiConfig } from '../../../common/lib/config_api'; import { triggersActionsUiHealth } from '../../../common/lib/health_api'; import { loadActionTypes, loadAllActions } from '../../lib/action_connector_api'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; jest.mock('../../../common/lib/kibana'); @@ -192,19 +193,31 @@ describe('rule_add', () => { actionTypeRegistry.has.mockReturnValue(true); wrapper = mountWithIntl( - { - return new Promise(() => {}); - }} - actionTypeRegistry={actionTypeRegistry} - ruleTypeRegistry={ruleTypeRegistry} - metadata={{ test: 'some value', fields: ['test'] }} - ruleTypeId={ruleTypeId} - validConsumers={validConsumers} - /> + + { + return new Promise(() => {}); + }} + actionTypeRegistry={actionTypeRegistry} + ruleTypeRegistry={ruleTypeRegistry} + metadata={{ test: 'some value', fields: ['test'] }} + ruleTypeId={ruleTypeId} + validConsumers={validConsumers} + /> + ); // Wait for active space to resolve before requesting the component to update From 427607fdfbc114e4c2a626578ff1409f02838e92 Mon Sep 17 00:00:00 2001 From: Xavier Mouligneau Date: Wed, 22 Nov 2023 20:58:13 -0500 Subject: [PATCH 06/12] rmv flaky tests on rule-add --- .../sections/rule_form/rule_add.test.tsx | 133 +++++++++----- .../sections/rule_form/rule_form.tsx | 168 +++++++++--------- 2 files changed, 174 insertions(+), 127 deletions(-) diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_add.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_add.test.tsx index 1bb0d09fa7d8f..3b74c7ec96389 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_add.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_add.test.tsx @@ -35,7 +35,7 @@ import { triggersActionsUiConfig } from '../../../common/lib/config_api'; import { triggersActionsUiHealth } from '../../../common/lib/health_api'; import { loadActionTypes, loadAllActions } from '../../lib/action_connector_api'; import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; - +import { waitFor } from '@testing-library/react'; jest.mock('../../../common/lib/kibana'); jest.mock('../../lib/rule_api/rule_types', () => ({ @@ -193,17 +193,7 @@ describe('rule_add', () => { actionTypeRegistry.has.mockReturnValue(true); wrapper = mountWithIntl( - + { }); }); + it('renders selection of rule types to pick in the modal', async () => { + (triggersActionsUiConfig as jest.Mock).mockResolvedValue({ + minimumScheduleInterval: { value: '1m', enforce: false }, + }); + const onClose = jest.fn(); + await setup({ + initialValues: {}, + onClose, + }); + + await act(async () => { + await nextTick(); + wrapper.update(); + }); + + await waitFor(() => { + const ruleTypesContainer = wrapper.find('[data-test-subj="ruleGroupTypeSelectContainer"]'); + const ruleTypeButton = ruleTypesContainer + .render() + .find('[data-test-subj="my-rule-type-SelectOption"]'); + + expect(ruleTypeButton.length).toEqual(1); + expect(ruleTypeButton.text()).toMatchInlineSnapshot(`"Testtest"`); + }); + }); + it('renders a confirm close modal if the flyout is closed after inputs have changed', async () => { (triggersActionsUiConfig as jest.Mock).mockResolvedValue({ minimumScheduleInterval: { value: '1m', enforce: false }, @@ -260,6 +276,7 @@ describe('rule_add', () => { await setup({ initialValues: {}, onClose, + ruleTypeId: 'my-rule-type', }); await act(async () => { @@ -267,14 +284,20 @@ describe('rule_add', () => { wrapper.update(); }); - wrapper.find('[data-test-subj="my-rule-type-SelectOption"]').last().simulate('click'); - expect(wrapper.find('input#ruleName').props().value).toBe(''); - expect(wrapper.find('[data-test-subj="tagsComboBox"]').first().text()).toBe(''); - expect(wrapper.find('.euiSelect').first().props().value).toBe('m'); + wrapper + .find('input#ruleName') + .at(0) + .simulate('change', { target: { value: 'my rule type' } }); - wrapper.find('[data-test-subj="cancelSaveRuleButton"]').last().simulate('click'); - expect(onClose).not.toHaveBeenCalled(); - expect(wrapper.find('[data-test-subj="confirmRuleCloseModal"]').exists()).toBe(true); + await waitFor(() => { + expect(wrapper.find('input#ruleName').props().value).toBe('my rule type'); + expect(wrapper.find('[data-test-subj="tagsComboBox"]').first().text()).toBe(''); + expect(wrapper.find('.euiSelect').first().props().value).toBe('m'); + + wrapper.find('[data-test-subj="cancelSaveRuleButton"]').last().simulate('click'); + expect(onClose).not.toHaveBeenCalled(); + expect(wrapper.find('[data-test-subj="confirmRuleCloseModal"]').exists()).toBe(true); + }); }); it('renders rule add flyout with initial values', async () => { @@ -504,13 +527,15 @@ describe('rule_add', () => { wrapper.update(); }); - expect(createRule).toHaveBeenLastCalledWith( - expect.objectContaining({ - rule: expect.objectContaining({ - consumer: 'logs', - }), - }) - ); + await waitFor(() => { + expect(createRule).toHaveBeenLastCalledWith( + expect.objectContaining({ + rule: expect.objectContaining({ + consumer: 'logs', + }), + }) + ); + }); }); it('should enforce any default interval', async () => { @@ -531,14 +556,16 @@ describe('rule_add', () => { wrapper.update(); }); - const intervalInputUnit = wrapper - .find('[data-test-subj="intervalInputUnit"]') - .first() - .getElement().props.value; - const intervalInput = wrapper.find('[data-test-subj="intervalInput"]').first().getElement() - .props.value; - expect(intervalInputUnit).toBe('h'); - expect(intervalInput).toBe(3); + await waitFor(() => { + const intervalInputUnit = wrapper + .find('[data-test-subj="intervalInputUnit"]') + .first() + .getElement().props.value; + const intervalInput = wrapper.find('[data-test-subj="intervalInput"]').first().getElement() + .props.value; + expect(intervalInputUnit).toBe('h'); + expect(intervalInput).toBe(3); + }); }); it('should load connectors and connector types when there is a pre-selected rule type', async () => { @@ -553,10 +580,18 @@ describe('rule_add', () => { actionsShow: true, }); - expect(triggersActionsUiHealth).toHaveBeenCalledTimes(1); - expect(alertingFrameworkHealth).toHaveBeenCalledTimes(1); - expect(loadActionTypes).toHaveBeenCalledTimes(1); - expect(loadAllActions).toHaveBeenCalledTimes(1); + // Wait for handlers to fire + await act(async () => { + await nextTick(); + wrapper.update(); + }); + + await waitFor(() => { + expect(triggersActionsUiHealth).toHaveBeenCalledTimes(1); + expect(alertingFrameworkHealth).toHaveBeenCalledTimes(1); + expect(loadActionTypes).toHaveBeenCalledTimes(1); + expect(loadAllActions).toHaveBeenCalledTimes(1); + }); }); it('should not load connectors and connector types when there is not an encryptionKey', async () => { @@ -575,13 +610,21 @@ describe('rule_add', () => { actionsShow: true, }); - expect(triggersActionsUiHealth).toHaveBeenCalledTimes(1); - expect(alertingFrameworkHealth).toHaveBeenCalledTimes(1); - expect(loadActionTypes).not.toHaveBeenCalled(); - expect(loadAllActions).not.toHaveBeenCalled(); - expect(wrapper.find('[data-test-subj="actionNeededEmptyPrompt"]').first().text()).toContain( - 'You must configure an encryption key to use Alerting' - ); + // Wait for handlers to fire + await act(async () => { + await nextTick(); + wrapper.update(); + }); + + await waitFor(() => { + expect(triggersActionsUiHealth).toHaveBeenCalledTimes(1); + expect(alertingFrameworkHealth).toHaveBeenCalledTimes(1); + expect(loadActionTypes).not.toHaveBeenCalled(); + expect(loadAllActions).not.toHaveBeenCalled(); + expect(wrapper.find('[data-test-subj="actionNeededEmptyPrompt"]').first().text()).toContain( + 'You must configure an encryption key to use Alerting' + ); + }); }); }); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form.tsx index 403f34f133e9d..4d5e13b253d9a 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form.tsx @@ -932,94 +932,98 @@ export const RuleForm = ({ - {ruleTypeModel ? ( - <>{ruleTypeDetails} - ) : availableRuleTypes.length ? ( - <> - - + {ruleTypeModel ? ( + <>{ruleTypeDetails} + ) : availableRuleTypes.length ? ( + <> + + + + + + + ) + } + label={ - +
- +
- ) - } - label={ - -
- -
- - } - > - - - { - setInputText(e.target.value); - if (e.target.value === '') { - setSearchText(''); - } - }} - onKeyUp={(e) => { - if (e.keyCode === ENTER_KEY) { - setSearchText(inputText); - } - }} - placeholder={i18n.translate( - 'xpack.triggersActionsUI.sections.ruleForm.searchPlaceholderTitle', - { defaultMessage: 'Search' } - )} - /> - - {solutions ? ( - - setSolutionFilter(selectedSolutions)} + } + > + + + { + setInputText(e.target.value); + if (e.target.value === '') { + setSearchText(''); + } + }} + onKeyUp={(e) => { + if (e.keyCode === ENTER_KEY) { + setSearchText(inputText); + } + }} + placeholder={i18n.translate( + 'xpack.triggersActionsUI.sections.ruleForm.searchPlaceholderTitle', + { defaultMessage: 'Search' } + )} /> - ) : null} - - - - {errors.ruleTypeId.length >= 1 && rule.ruleTypeId !== undefined ? ( - <> - - - - - ) : null} - {ruleTypeNodes} - - ) : ruleTypeIndex && !ruleTypesIsLoading ? ( - - ) : ( - - - - )} + {solutions ? ( + + + setSolutionFilter(selectedSolutions) + } + /> + + ) : null} + + + + {errors.ruleTypeId.length >= 1 && rule.ruleTypeId !== undefined ? ( + <> + + + + + ) : null} + {ruleTypeNodes} + + ) : ruleTypeIndex && !ruleTypesIsLoading ? ( + + ) : ( + + + + )} + ); }; From 5f559c6909f1e0c4a33dc4cb8dde577b10d34278 Mon Sep 17 00:00:00 2001 From: Umberto Pepato Date: Tue, 28 Nov 2023 15:38:05 +0100 Subject: [PATCH 07/12] Fix actionConnectors missing in test --- .../public/application/sections/rule_form/rule_form.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form.test.tsx index 9239a90ef729f..690002491c7cc 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_form.test.tsx @@ -1206,7 +1206,7 @@ describe('rule_form', () => { minimumScheduleInterval: { value: '1m', enforce: false }, }} dispatch={() => {}} - errors={{ name: [], 'schedule.interval': [], ruleTypeId: [] }} + errors={{ name: [], 'schedule.interval': [], ruleTypeId: [], actionConnectors: [] }} operation="create" actionTypeRegistry={actionTypeRegistry} ruleTypeRegistry={ruleTypeRegistry} From 2e7538eeb65210ec92a991b4a53e04a5bbb57f9c Mon Sep 17 00:00:00 2001 From: Umberto Pepato Date: Wed, 29 Nov 2023 11:46:42 +0100 Subject: [PATCH 08/12] Fix failing triggersActionsUi tests --- .../public/application/home.test.tsx | 68 ++++++++++++++++--- .../sections/rule_form/rule_edit.test.tsx | 21 +++--- 2 files changed, 69 insertions(+), 20 deletions(-) diff --git a/x-pack/plugins/triggers_actions_ui/public/application/home.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/home.test.tsx index 5289a2305a28d..c08715ba2b5ca 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/home.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/home.test.tsx @@ -16,6 +16,7 @@ import { mountWithIntl } from '@kbn/test-jest-helpers'; import TriggersActionsUIHome, { MatchParams } from './home'; import { hasShowActionsCapability } from './lib/capabilities'; import { getIsExperimentalFeatureEnabled } from '../common/get_experimental_features'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; jest.mock('../common/lib/kibana'); jest.mock('../common/get_experimental_features'); @@ -32,10 +33,21 @@ jest.mock('./context/health_context', () => ({ HealthContextProvider: ({ children }: { children: React.ReactNode }) => <>{children}, })); +jest.mock('./hooks/use_load_rule_types_query', () => ({ + useLoadRuleTypesQuery: jest.fn().mockReturnValue({ + authorizedToReadAnyRules: true, + }), +})); + +const { useLoadRuleTypesQuery } = jest.requireMock('./hooks/use_load_rule_types_query'); + +const queryClient = new QueryClient(); + describe('home', () => { beforeEach(() => { (hasShowActionsCapability as jest.Mock).mockClear(); - (getIsExperimentalFeatureEnabled as jest.Mock).mockClear(); + (getIsExperimentalFeatureEnabled as jest.Mock).mockImplementation(() => false); + useLoadRuleTypesQuery.mockClear(); }); it('renders rule list components', async () => { @@ -57,7 +69,9 @@ describe('home', () => { render( - + + + ); @@ -67,7 +81,7 @@ describe('home', () => { }); }); - it('hides the internal alerts table route if the config is not set', async () => { + it('hides the internal alerts table tab if the config is not set', async () => { (hasShowActionsCapability as jest.Mock).mockImplementation(() => { return true; }); @@ -76,7 +90,7 @@ describe('home', () => { location: createLocation('/'), match: { isExact: true, - path: `/connectorss`, + path: `/connectors`, url: '', params: { section: 'connectors', @@ -86,26 +100,58 @@ describe('home', () => { let home = mountWithIntl( - + + + ); - // Just rules/logs + // Just rules and logs expect(home.find('span.euiTab__content').length).toBe(2); (getIsExperimentalFeatureEnabled as jest.Mock).mockImplementation((feature: string) => { - if (feature === 'internalAlertsTable') { - return true; - } - return false; + return feature === 'internalAlertsTable'; }); home = mountWithIntl( - + + + ); // alerts now too! expect(home.find('span.euiTab__content').length).toBe(3); }); + + it('hides the logs tab if the read rules privilege is missing', async () => { + useLoadRuleTypesQuery.mockReturnValue({ + authorizedToReadAnyRules: false, + }); + const props: RouteComponentProps = { + history: createMemoryHistory({ + initialEntries: ['/rules'], + }), + location: createLocation('/rules'), + match: { + isExact: true, + path: `/rules`, + url: '', + params: { + section: 'rules', + }, + }, + }; + + const home = mountWithIntl( + + + + + + ); + + // Just rules + expect(home.find('span.euiTab__content').length).toBe(1); + }); }); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_edit.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_edit.test.tsx index 7e937d17f8684..ee64a408dcb37 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_edit.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rule_form/rule_edit.test.tsx @@ -16,6 +16,7 @@ import { ReactWrapper } from 'enzyme'; import RuleEdit from './rule_edit'; import { useKibana } from '../../../common/lib/kibana'; import { ALERTS_FEATURE_ID } from '@kbn/alerting-plugin/common'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; jest.mock('../../../common/lib/kibana'); const actionTypeRegistry = actionTypeRegistryMock.create(); const ruleTypeRegistry = ruleTypeRegistryMock.create(); @@ -186,15 +187,17 @@ describe('rule_edit', () => { actionTypeRegistry.has.mockReturnValue(true); wrapper = mountWithIntl( - {}} - initialRule={rule} - onSave={() => { - return new Promise(() => {}); - }} - actionTypeRegistry={actionTypeRegistry} - ruleTypeRegistry={ruleTypeRegistry} - /> + + {}} + initialRule={rule} + onSave={() => { + return new Promise(() => {}); + }} + actionTypeRegistry={actionTypeRegistry} + ruleTypeRegistry={ruleTypeRegistry} + /> + ); // Wait for active space to resolve before requesting the component to update await act(async () => { From e1766f2e57a9318fab5192d7e9a17bc6d365b96e Mon Sep 17 00:00:00 2001 From: Umberto Pepato Date: Thu, 30 Nov 2023 13:51:12 +0100 Subject: [PATCH 09/12] Fix missing queryClient in standalone rule form --- .../triggers_actions_ui/public/application/app.tsx | 4 ++-- .../public/application/query_client.ts | 13 +++++++++++++ .../public/common/get_add_rule_flyout.tsx | 6 +++++- .../apps/discover_ml_uptime/config.ts | 4 ++-- 4 files changed, 22 insertions(+), 5 deletions(-) create mode 100644 x-pack/plugins/triggers_actions_ui/public/application/query_client.ts diff --git a/x-pack/plugins/triggers_actions_ui/public/application/app.tsx b/x-pack/plugins/triggers_actions_ui/public/application/app.tsx index ac0fd6a6d92a3..e5b7cc2b7bbe6 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/app.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/app.tsx @@ -28,7 +28,7 @@ import { Storage } from '@kbn/kibana-utils-plugin/public'; import { EuiThemeProvider } from '@kbn/kibana-react-plugin/common'; import { ActionsPublicPluginSetup } from '@kbn/actions-plugin/public'; import { ruleDetailsRoute } from '@kbn/rule-data-utils'; -import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { QueryClientProvider } from '@tanstack/react-query'; import { DashboardStart } from '@kbn/dashboard-plugin/public'; import { ExpressionsStart } from '@kbn/expressions-plugin/public'; import { suspendedComponentWithProps } from './lib/suspended_component_with_props'; @@ -43,12 +43,12 @@ import { setDataViewsService } from '../common/lib/data_apis'; import { KibanaContextProvider, useKibana } from '../common/lib/kibana'; import { ConnectorProvider } from './context/connector_context'; import { CONNECTORS_PLUGIN_ID } from '../common/constants'; +import { queryClient } from './query_client'; const TriggersActionsUIHome = lazy(() => import('./home')); const RuleDetailsRoute = lazy( () => import('./sections/rule_details/components/rule_details_route') ); -const queryClient = new QueryClient(); export interface TriggersAndActionsUiServices extends CoreStart { actions: ActionsPublicPluginSetup; diff --git a/x-pack/plugins/triggers_actions_ui/public/application/query_client.ts b/x-pack/plugins/triggers_actions_ui/public/application/query_client.ts new file mode 100644 index 0000000000000..1dc1a60f757dc --- /dev/null +++ b/x-pack/plugins/triggers_actions_ui/public/application/query_client.ts @@ -0,0 +1,13 @@ +/* + * 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 { QueryClient } from '@tanstack/react-query'; + +/** + * The main query client for the triggers_actions_ui app + */ +export const queryClient = new QueryClient(); diff --git a/x-pack/plugins/triggers_actions_ui/public/common/get_add_rule_flyout.tsx b/x-pack/plugins/triggers_actions_ui/public/common/get_add_rule_flyout.tsx index c91fbf2c7ccdf..23f751201d1be 100644 --- a/x-pack/plugins/triggers_actions_ui/public/common/get_add_rule_flyout.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/common/get_add_rule_flyout.tsx @@ -6,16 +6,20 @@ */ import React from 'react'; +import { QueryClientProvider } from '@tanstack/react-query'; import { ConnectorProvider } from '../application/context/connector_context'; import { RuleAdd } from '../application/sections/rule_form'; import type { ConnectorServices, RuleAddProps } from '../types'; +import { queryClient } from '../application/query_client'; export const getAddRuleFlyoutLazy = ( props: RuleAddProps & { connectorServices: ConnectorServices } ) => { return ( - + + + ); }; diff --git a/x-pack/test/functional_with_es_ssl/apps/discover_ml_uptime/config.ts b/x-pack/test/functional_with_es_ssl/apps/discover_ml_uptime/config.ts index fc51ae51a2be7..4cf933e36df0c 100644 --- a/x-pack/test/functional_with_es_ssl/apps/discover_ml_uptime/config.ts +++ b/x-pack/test/functional_with_es_ssl/apps/discover_ml_uptime/config.ts @@ -22,8 +22,8 @@ export default async function ({ readConfigFile }: FtrConfigProviderContext) { }, testFiles: [ resolve(__dirname, './discover'), - resolve(__dirname, './uptime'), - resolve(__dirname, './ml'), + // resolve(__dirname, './uptime'), + // resolve(__dirname, './ml'), ], junit: { reportName: 'Chrome X-Pack UI Functional Tests with ES SSL - Discover, Uptime, ML', From c8a5cd2ae8a1e62c3c6c3c8a2e90f0f92c5d28c4 Mon Sep 17 00:00:00 2001 From: Umberto Pepato Date: Mon, 4 Dec 2023 08:44:23 +0100 Subject: [PATCH 10/12] Uncomment test imports --- .../functional_with_es_ssl/apps/discover_ml_uptime/config.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/test/functional_with_es_ssl/apps/discover_ml_uptime/config.ts b/x-pack/test/functional_with_es_ssl/apps/discover_ml_uptime/config.ts index 4cf933e36df0c..fc51ae51a2be7 100644 --- a/x-pack/test/functional_with_es_ssl/apps/discover_ml_uptime/config.ts +++ b/x-pack/test/functional_with_es_ssl/apps/discover_ml_uptime/config.ts @@ -22,8 +22,8 @@ export default async function ({ readConfigFile }: FtrConfigProviderContext) { }, testFiles: [ resolve(__dirname, './discover'), - // resolve(__dirname, './uptime'), - // resolve(__dirname, './ml'), + resolve(__dirname, './uptime'), + resolve(__dirname, './ml'), ], junit: { reportName: 'Chrome X-Pack UI Functional Tests with ES SSL - Discover, Uptime, ML', From 15d71c0d1f6f1c1a3ca8fa0e6d440f5f06712b8a Mon Sep 17 00:00:00 2001 From: Umberto Pepato Date: Mon, 4 Dec 2023 09:41:40 +0100 Subject: [PATCH 11/12] Comment usage of staleTime in useLoadRuleTypesQuery --- .../public/application/hooks/use_load_rule_types_query.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/x-pack/plugins/triggers_actions_ui/public/application/hooks/use_load_rule_types_query.ts b/x-pack/plugins/triggers_actions_ui/public/application/hooks/use_load_rule_types_query.ts index e98a5b9b6b86b..bfa00d818da55 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/hooks/use_load_rule_types_query.ts +++ b/x-pack/plugins/triggers_actions_ui/public/application/hooks/use_load_rule_types_query.ts @@ -59,6 +59,8 @@ export const useLoadRuleTypesQuery = ({ queryFn, onError: onErrorFn, refetchOnWindowFocus: false, + // Leveraging TanStack Query's caching system to avoid duplicated requests as + // other state-sharing solutions turned out to be overly complex and less readable staleTime: 60 * 1000, enabled, }); From 0385d6ada03aba66cb3e337696bff737c1595e10 Mon Sep 17 00:00:00 2001 From: Umberto Pepato Date: Mon, 4 Dec 2023 10:06:26 +0100 Subject: [PATCH 12/12] Fix flaky rule form tests --- .../public/alerts/alert_form.test.tsx | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/monitoring/public/alerts/alert_form.test.tsx b/x-pack/plugins/monitoring/public/alerts/alert_form.test.tsx index 5093a70eef302..7ac7d435f89d9 100644 --- a/x-pack/plugins/monitoring/public/alerts/alert_form.test.tsx +++ b/x-pack/plugins/monitoring/public/alerts/alert_form.test.tsx @@ -44,11 +44,11 @@ const { loadActionTypes } = jest.requireMock( '@kbn/triggers-actions-ui-plugin/public/application/lib/action_connector_api' ); -jest.mock('@kbn/triggers-actions-ui-plugin/public/application/lib/rule_api/rule_types', () => ({ - loadRuleTypes: jest.fn(), -})); -const { loadRuleTypes } = jest.requireMock( - '@kbn/triggers-actions-ui-plugin/public/application/lib/rule_api/rule_types' +jest.mock( + '@kbn/triggers-actions-ui-plugin/public/application/hooks/use_load_rule_types_query', + () => ({ + useLoadRuleTypesQuery: jest.fn(), + }) ); jest.mock('@kbn/kibana-react-plugin/public/ui_settings/use_ui_setting', () => ({ @@ -113,7 +113,14 @@ describe('alert_form', () => { let wrapper: ReactWrapper; beforeEach(async () => { - loadRuleTypes.mockResolvedValue([]); + const { useLoadRuleTypesQuery } = jest.requireMock( + '@kbn/triggers-actions-ui-plugin/public/application/hooks/use_load_rule_types_query' + ); + useLoadRuleTypesQuery.mockReturnValue({ + ruleTypesState: { + data: new Map(), + }, + }); ruleTypeRegistry.list.mockReturnValue([ruleType]); ruleTypeRegistry.get.mockReturnValue(ruleType); ruleTypeRegistry.has.mockReturnValue(true);