From a1b45d4ea27ae4592b2524c81c2798492974ffef Mon Sep 17 00:00:00 2001 From: Chris Roberson Date: Wed, 13 Oct 2021 19:17:34 -0400 Subject: [PATCH] Always call resolve (#114670) (#114913) --- .../components/alert_details_route.test.tsx | 118 +++--------------- .../components/alert_details_route.tsx | 24 +--- 2 files changed, 26 insertions(+), 116 deletions(-) diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_details_route.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_details_route.test.tsx index c07138990f88d..847c6c65464b2 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_details_route.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_details_route.test.tsx @@ -19,20 +19,6 @@ import { spacesPluginMock } from '../../../../../../spaces/public/mocks'; import { useKibana } from '../../../../common/lib/kibana'; jest.mock('../../../../common/lib/kibana'); -class NotFoundError extends Error { - public readonly body: { - statusCode: number; - name: string; - } = { - statusCode: 404, - name: 'Not found', - }; - - constructor(message: string | undefined) { - super(message); - } -} - describe('alert_details_route', () => { beforeEach(() => { jest.clearAllMocks(); @@ -58,11 +44,8 @@ describe('alert_details_route', () => { it('redirects to another page if fetched rule is an aliasMatch', async () => { await setup(); const rule = mockRule(); - const { loadAlert, resolveRule } = mockApis(); + const { resolveRule } = mockApis(); - loadAlert.mockImplementationOnce(async () => { - throw new NotFoundError('OMG'); - }); resolveRule.mockImplementationOnce(async () => ({ ...rule, id: 'new_id', @@ -70,17 +53,13 @@ describe('alert_details_route', () => { alias_target_id: rule.id, })); const wrapper = mountWithIntl( - + ); await act(async () => { await nextTick(); wrapper.update(); }); - expect(loadAlert).toHaveBeenCalledWith(rule.id); expect(resolveRule).toHaveBeenCalledWith(rule.id); expect((spacesMock as any).ui.redirectLegacyUrl).toHaveBeenCalledWith( `insightsAndAlerting/triggersActions/rule/new_id`, @@ -96,11 +75,8 @@ describe('alert_details_route', () => { name: 'type name', authorizedConsumers: ['consumer'], }; - const { loadAlert, loadAlertTypes, loadActionTypes, resolveRule } = mockApis(); + const { loadAlertTypes, loadActionTypes, resolveRule } = mockApis(); - loadAlert.mockImplementationOnce(async () => { - throw new NotFoundError('OMG'); - }); loadAlertTypes.mockImplementationOnce(async () => [ruleType]); loadActionTypes.mockImplementation(async () => []); resolveRule.mockImplementationOnce(async () => ({ @@ -112,7 +88,7 @@ describe('alert_details_route', () => { const wrapper = mountWithIntl( ); await act(async () => { @@ -120,7 +96,6 @@ describe('alert_details_route', () => { wrapper.update(); }); - expect(loadAlert).toHaveBeenCalledWith(rule.id); expect(resolveRule).toHaveBeenCalledWith(rule.id); expect((spacesMock as any).ui.components.getLegacyUrlConflict).toHaveBeenCalledWith({ currentObjectId: 'new_id', @@ -138,10 +113,10 @@ describe('getRuleData useEffect handler', () => { it('fetches rule', async () => { const rule = mockRule(); - const { loadAlert, loadAlertTypes, loadActionTypes, resolveRule } = mockApis(); + const { loadAlertTypes, loadActionTypes, resolveRule } = mockApis(); const { setAlert, setAlertType, setActionTypes } = mockStateSetter(); - loadAlert.mockImplementationOnce(async () => rule); + resolveRule.mockImplementationOnce(async () => rule); const toastNotifications = { addDanger: jest.fn(), @@ -149,7 +124,6 @@ describe('getRuleData useEffect handler', () => { await getRuleData( rule.id, - loadAlert, loadAlertTypes, resolveRule, loadActionTypes, @@ -159,8 +133,7 @@ describe('getRuleData useEffect handler', () => { toastNotifications ); - expect(loadAlert).toHaveBeenCalledWith(rule.id); - expect(resolveRule).not.toHaveBeenCalled(); + expect(resolveRule).toHaveBeenCalledWith(rule.id); expect(setAlert).toHaveBeenCalledWith(rule); }); @@ -184,10 +157,10 @@ describe('getRuleData useEffect handler', () => { id: rule.alertTypeId, name: 'type name', }; - const { loadAlert, loadAlertTypes, loadActionTypes, resolveRule } = mockApis(); + const { loadAlertTypes, loadActionTypes, resolveRule } = mockApis(); const { setAlert, setAlertType, setActionTypes } = mockStateSetter(); - loadAlert.mockImplementation(async () => rule); + resolveRule.mockImplementation(async () => rule); loadAlertTypes.mockImplementation(async () => [ruleType]); loadActionTypes.mockImplementation(async () => [connectorType]); @@ -197,7 +170,6 @@ describe('getRuleData useEffect handler', () => { await getRuleData( rule.id, - loadAlert, loadAlertTypes, resolveRule, loadActionTypes, @@ -209,58 +181,13 @@ describe('getRuleData useEffect handler', () => { expect(loadAlertTypes).toHaveBeenCalledTimes(1); expect(loadActionTypes).toHaveBeenCalledTimes(1); - expect(resolveRule).not.toHaveBeenCalled(); + expect(resolveRule).toHaveBeenCalled(); expect(setAlert).toHaveBeenCalledWith(rule); expect(setAlertType).toHaveBeenCalledWith(ruleType); expect(setActionTypes).toHaveBeenCalledWith([connectorType]); }); - it('fetches rule using resolve if initial GET results in a 404 error', async () => { - const connectorType = { - id: '.server-log', - name: 'Server log', - enabled: true, - }; - const rule = mockRule({ - actions: [ - { - group: '', - id: uuid.v4(), - actionTypeId: connectorType.id, - params: {}, - }, - ], - }); - - const { loadAlert, loadAlertTypes, loadActionTypes, resolveRule } = mockApis(); - const { setAlert, setAlertType, setActionTypes } = mockStateSetter(); - - loadAlert.mockImplementationOnce(async () => { - throw new NotFoundError('OMG'); - }); - resolveRule.mockImplementationOnce(async () => rule); - - const toastNotifications = { - addDanger: jest.fn(), - } as unknown as ToastsApi; - await getRuleData( - rule.id, - loadAlert, - loadAlertTypes, - resolveRule, - loadActionTypes, - setAlert, - setAlertType, - setActionTypes, - toastNotifications - ); - - expect(loadAlert).toHaveBeenCalledWith(rule.id); - expect(resolveRule).toHaveBeenCalledWith(rule.id); - expect(setAlert).toHaveBeenCalledWith(rule); - }); - it('displays an error if fetching the rule results in a non-404 error', async () => { const connectorType = { id: '.server-log', @@ -278,10 +205,10 @@ describe('getRuleData useEffect handler', () => { ], }); - const { loadAlert, loadAlertTypes, loadActionTypes, resolveRule } = mockApis(); + const { loadAlertTypes, loadActionTypes, resolveRule } = mockApis(); const { setAlert, setAlertType, setActionTypes } = mockStateSetter(); - loadAlert.mockImplementation(async () => { + resolveRule.mockImplementation(async () => { throw new Error('OMG'); }); @@ -290,7 +217,6 @@ describe('getRuleData useEffect handler', () => { } as unknown as ToastsApi; await getRuleData( rule.id, - loadAlert, loadAlertTypes, resolveRule, loadActionTypes, @@ -322,10 +248,10 @@ describe('getRuleData useEffect handler', () => { ], }); - const { loadAlert, loadAlertTypes, loadActionTypes, resolveRule } = mockApis(); + const { loadAlertTypes, loadActionTypes, resolveRule } = mockApis(); const { setAlert, setAlertType, setActionTypes } = mockStateSetter(); - loadAlert.mockImplementation(async () => rule); + resolveRule.mockImplementation(async () => rule); loadAlertTypes.mockImplementation(async () => { throw new Error('OMG no rule type'); @@ -337,7 +263,6 @@ describe('getRuleData useEffect handler', () => { } as unknown as ToastsApi; await getRuleData( rule.id, - loadAlert, loadAlertTypes, resolveRule, loadActionTypes, @@ -373,10 +298,10 @@ describe('getRuleData useEffect handler', () => { name: 'type name', }; - const { loadAlert, loadAlertTypes, loadActionTypes, resolveRule } = mockApis(); + const { loadAlertTypes, loadActionTypes, resolveRule } = mockApis(); const { setAlert, setAlertType, setActionTypes } = mockStateSetter(); - loadAlert.mockImplementation(async () => rule); + resolveRule.mockImplementation(async () => rule); loadAlertTypes.mockImplementation(async () => [ruleType]); loadActionTypes.mockImplementation(async () => { @@ -388,7 +313,6 @@ describe('getRuleData useEffect handler', () => { } as unknown as ToastsApi; await getRuleData( rule.id, - loadAlert, loadAlertTypes, resolveRule, loadActionTypes, @@ -425,10 +349,10 @@ describe('getRuleData useEffect handler', () => { name: 'type name', }; - const { loadAlert, loadAlertTypes, loadActionTypes, resolveRule } = mockApis(); + const { loadAlertTypes, loadActionTypes, resolveRule } = mockApis(); const { setAlert, setAlertType, setActionTypes } = mockStateSetter(); - loadAlert.mockImplementation(async () => rule); + resolveRule.mockImplementation(async () => rule); loadAlertTypes.mockImplementation(async () => [ruleType]); loadActionTypes.mockImplementation(async () => [connectorType]); @@ -437,7 +361,6 @@ describe('getRuleData useEffect handler', () => { } as unknown as ToastsApi; await getRuleData( rule.id, - loadAlert, loadAlertTypes, resolveRule, loadActionTypes, @@ -485,10 +408,10 @@ describe('getRuleData useEffect handler', () => { name: 'type name', }; - const { loadAlert, loadAlertTypes, loadActionTypes, resolveRule } = mockApis(); + const { loadAlertTypes, loadActionTypes, resolveRule } = mockApis(); const { setAlert, setAlertType, setActionTypes } = mockStateSetter(); - loadAlert.mockImplementation(async () => rule); + resolveRule.mockImplementation(async () => rule); loadAlertTypes.mockImplementation(async () => [ruleType]); loadActionTypes.mockImplementation(async () => [availableConnectorType]); @@ -497,7 +420,6 @@ describe('getRuleData useEffect handler', () => { } as unknown as ToastsApi; await getRuleData( rule.id, - loadAlert, loadAlertTypes, resolveRule, loadActionTypes, diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_details_route.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_details_route.tsx index 123d60bb9fea3..b530df986c277 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_details_route.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/alert_details/components/alert_details_route.tsx @@ -10,7 +10,7 @@ import React, { useState, useEffect } from 'react'; import { RouteComponentProps } from 'react-router-dom'; import { ToastsApi } from 'kibana/public'; import { EuiSpacer } from '@elastic/eui'; -import { Alert, AlertType, ActionType, ResolvedRule } from '../../../../types'; +import { AlertType, ActionType, ResolvedRule } from '../../../../types'; import { AlertDetailsWithApi as AlertDetails } from './alert_details'; import { throwIfAbsent, throwIfIsntContained } from '../../../lib/value_validators'; import { @@ -28,13 +28,12 @@ type AlertDetailsRouteProps = RouteComponentProps<{ ruleId: string; }> & Pick & - Pick; + Pick; export const AlertDetailsRoute: React.FunctionComponent = ({ match: { params: { ruleId }, }, - loadAlert, loadAlertTypes, loadActionTypes, resolveRule, @@ -47,14 +46,13 @@ export const AlertDetailsRoute: React.FunctionComponent const { basePath } = http; - const [alert, setAlert] = useState(null); + const [alert, setAlert] = useState(null); const [alertType, setAlertType] = useState(null); const [actionTypes, setActionTypes] = useState(null); const [refreshToken, requestRefresh] = React.useState(); useEffect(() => { getRuleData( ruleId, - loadAlert, loadAlertTypes, resolveRule, loadActionTypes, @@ -63,7 +61,7 @@ export const AlertDetailsRoute: React.FunctionComponent setActionTypes, toasts ); - }, [ruleId, http, loadActionTypes, loadAlert, loadAlertTypes, resolveRule, toasts, refreshToken]); + }, [ruleId, http, loadActionTypes, loadAlertTypes, resolveRule, toasts, refreshToken]); useEffect(() => { if (alert) { @@ -128,26 +126,16 @@ export const AlertDetailsRoute: React.FunctionComponent export async function getRuleData( ruleId: string, - loadAlert: AlertApis['loadAlert'], loadAlertTypes: AlertApis['loadAlertTypes'], resolveRule: AlertApis['resolveRule'], loadActionTypes: ActionApis['loadActionTypes'], - setAlert: React.Dispatch>, + setAlert: React.Dispatch>, setAlertType: React.Dispatch>, setActionTypes: React.Dispatch>, toasts: Pick ) { try { - let loadedRule: Alert | ResolvedRule; - try { - loadedRule = await loadAlert(ruleId); - } catch (err) { - // Try resolving this rule id if the error is a 404, otherwise re-throw - if (err?.body?.statusCode !== 404) { - throw err; - } - loadedRule = await resolveRule(ruleId); - } + const loadedRule: ResolvedRule = await resolveRule(ruleId); setAlert(loadedRule); const [loadedAlertType, loadedActionTypes] = await Promise.all([