From d9da438b3e6743c557ca8c214505baffc8188a8d Mon Sep 17 00:00:00 2001 From: Georgii Gorbachev Date: Thu, 18 Feb 2021 01:37:52 +0100 Subject: [PATCH] [Security Solution][Detections] Pre-refactoring for the rule management table (#91302) **Related to:** https://github.com/elastic/kibana/pull/89877 ## Summary This is based on https://github.com/elastic/kibana/pull/89877 and the kind of pre-refactoring that has been done there. Mainly this: - consolidates application logic in a single place (moves the reducer and the side effects close to each other, etc) - removes some of the redundant state, leverages the reducer as the source of truth for state - makes it easier to dispatch events, removes some of the noise While this refactoring is a totally unfinished work, and might look not good enough (or at all), still I'd like to merge it because of the logic consolidation. I'm going to finalize the refactoring later when I start implementing new filters and other table UX improvements. So the code is going to become better and maybe even quite different from what it is right now. (Btw because of that, I'm not adding or removing any tests here because this is an intermediate kind of state of the code). ### Checklist - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --- .../components/rules/rule_switch/index.tsx | 5 +- .../detection_engine/rules/index.ts | 2 +- .../rules/rules_table/index.ts | 11 ++ .../rules/rules_table/rules_table_facade.ts | 84 +++++++++ .../rules_table/rules_table_reducer.test.ts} | 31 ++-- .../rules/rules_table/rules_table_reducer.ts | 165 ++++++++++++++++++ .../{ => rules_table}/use_rules.test.tsx | 4 +- .../rules/{ => rules_table}/use_rules.tsx | 8 +- .../rules/rules_table/use_rules_table.ts | 131 ++++++++++++++ .../detection_engine/rules/all/actions.tsx | 13 +- .../rules/all/batch_actions.tsx | 4 +- .../detection_engine/rules/all/columns.tsx | 6 +- .../detection_engine/rules/all/index.test.tsx | 52 ++++-- .../detection_engine/rules/all/reducer.ts | 156 ----------------- .../rules/all/rules_tables.tsx | 151 ++++++---------- 15 files changed, 522 insertions(+), 301 deletions(-) create mode 100644 x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/rules_table/index.ts create mode 100644 x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/rules_table/rules_table_facade.ts rename x-pack/plugins/security_solution/public/detections/{pages/detection_engine/rules/all/reducer.test.ts => containers/detection_engine/rules/rules_table/rules_table_reducer.test.ts} (94%) create mode 100644 x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/rules_table/rules_table_reducer.ts rename x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/{ => rules_table}/use_rules.test.tsx (99%) rename x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/{ => rules_table}/use_rules.tsx (93%) create mode 100644 x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/rules_table/use_rules_table.ts delete mode 100644 x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/reducer.ts diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/rule_switch/index.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/rule_switch/index.tsx index 4f87f9332475b..268ffe620ad4e 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/rule_switch/index.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/rules/rule_switch/index.tsx @@ -17,9 +17,8 @@ import styled from 'styled-components'; import React, { useMemo, useCallback, useState, useEffect } from 'react'; import * as i18n from '../../../pages/detection_engine/rules/translations'; -import { enableRules } from '../../../containers/detection_engine/rules'; +import { enableRules, RulesTableAction } from '../../../containers/detection_engine/rules'; import { enableRulesAction } from '../../../pages/detection_engine/rules/all/actions'; -import { Action } from '../../../pages/detection_engine/rules/all/reducer'; import { useStateToaster, displayErrorToast } from '../../../../common/components/toasters'; import { bucketRulesResponse } from '../../../pages/detection_engine/rules/all/helpers'; @@ -33,7 +32,7 @@ const StaticSwitch = styled(EuiSwitch)` StaticSwitch.displayName = 'StaticSwitch'; export interface RuleSwitchProps { - dispatch?: React.Dispatch; + dispatch?: React.Dispatch; id: string; enabled: boolean; isDisabled?: boolean; diff --git a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/index.ts b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/index.ts index 751cde64bb87d..8128eb045f759 100644 --- a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/index.ts +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/index.ts @@ -10,6 +10,6 @@ export * from './use_update_rule'; export * from './use_create_rule'; export * from './types'; export * from './use_rule'; -export * from './use_rules'; +export * from './rules_table'; export * from './use_pre_packaged_rules'; export * from './use_rule_status'; diff --git a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/rules_table/index.ts b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/rules_table/index.ts new file mode 100644 index 0000000000000..a05349fa4fa3a --- /dev/null +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/rules_table/index.ts @@ -0,0 +1,11 @@ +/* + * 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. + */ + +export * from './rules_table_facade'; +export * from './rules_table_reducer'; +export * from './use_rules'; +export * from './use_rules_table'; diff --git a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/rules_table/rules_table_facade.ts b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/rules_table/rules_table_facade.ts new file mode 100644 index 0000000000000..77c327c9f7939 --- /dev/null +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/rules_table/rules_table_facade.ts @@ -0,0 +1,84 @@ +/* + * 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 type { Dispatch } from 'react'; +import { Rule, FilterOptions, PaginationOptions } from '../types'; +import { RulesTableAction, LoadingRuleAction } from './rules_table_reducer'; + +export interface RulesTableFacade { + setRules(newRules: Rule[], newPagination: Partial): void; + updateRules(rules: Rule[]): void; + updateOptions(filter: Partial, pagination: Partial): void; + actionStarted(actionType: LoadingRuleAction, ruleIds: string[]): void; + actionStopped(): void; + setShowIdleModal(show: boolean): void; + setLastRefreshDate(): void; + setAutoRefreshOn(on: boolean): void; +} + +export const createRulesTableFacade = (dispatch: Dispatch): RulesTableFacade => { + return { + setRules: (newRules: Rule[], newPagination: Partial) => { + dispatch({ + type: 'setRules', + rules: newRules, + pagination: newPagination, + }); + }, + + updateRules: (rules: Rule[]) => { + dispatch({ + type: 'updateRules', + rules, + }); + }, + + updateOptions: (filter: Partial, pagination: Partial) => { + dispatch({ + type: 'updateFilterOptions', + filterOptions: filter, + pagination, + }); + }, + + actionStarted: (actionType: LoadingRuleAction, ruleIds: string[]) => { + dispatch({ + type: 'loadingRuleIds', + actionType, + ids: ruleIds, + }); + }, + + actionStopped: () => { + dispatch({ + type: 'loadingRuleIds', + actionType: null, + ids: [], + }); + }, + + setShowIdleModal: (show: boolean) => { + dispatch({ + type: 'setShowIdleModal', + show, + }); + }, + + setLastRefreshDate: () => { + dispatch({ + type: 'setLastRefreshDate', + }); + }, + + setAutoRefreshOn: (on: boolean) => { + dispatch({ + type: 'setAutoRefreshOn', + on, + }); + }, + }; +}; diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/reducer.test.ts b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/rules_table/rules_table_reducer.test.ts similarity index 94% rename from x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/reducer.test.ts rename to x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/rules_table/rules_table_reducer.test.ts index 20e4e5f747349..1a45c60dba58a 100644 --- a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/reducer.test.ts +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/rules_table/rules_table_reducer.test.ts @@ -5,13 +5,17 @@ * 2.0. */ -import { FilterOptions, PaginationOptions } from '../../../../containers/detection_engine/rules'; +import { mockRule } from '../../../../pages/detection_engine/rules/all/__mocks__/mock'; +import { FilterOptions, PaginationOptions } from '../types'; +import { RulesTableAction, RulesTableState, createRulesTableReducer } from './rules_table_reducer'; -import { Action, State, allRulesReducer } from './reducer'; -import { mockRule } from './__mocks__/mock'; - -const initialState: State = { - exportRuleIds: [], +const initialState: RulesTableState = { + rules: [], + pagination: { + page: 1, + perPage: 20, + total: 0, + }, filterOptions: { filter: '', sortField: 'enabled', @@ -20,29 +24,24 @@ const initialState: State = { showCustomRules: false, showElasticRules: false, }, - loadingRuleIds: [], loadingRulesAction: null, - pagination: { - page: 1, - perPage: 20, - total: 0, - }, - rules: [], + loadingRuleIds: [], selectedRuleIds: [], + exportRuleIds: [], lastUpdated: 0, - showIdleModal: false, isRefreshOn: false, + showIdleModal: false, }; describe('allRulesReducer', () => { - let reducer: (state: State, action: Action) => State; + let reducer: (state: RulesTableState, action: RulesTableAction) => RulesTableState; beforeEach(() => { jest.useFakeTimers(); jest .spyOn(global.Date, 'now') .mockImplementationOnce(() => new Date('2020-10-31T11:01:58.135Z').valueOf()); - reducer = allRulesReducer({ current: undefined }); + reducer = createRulesTableReducer({ current: undefined }); }); afterEach(() => { diff --git a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/rules_table/rules_table_reducer.ts b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/rules_table/rules_table_reducer.ts new file mode 100644 index 0000000000000..edcf4f6395d89 --- /dev/null +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/rules_table/rules_table_reducer.ts @@ -0,0 +1,165 @@ +/* + * 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 type React from 'react'; +import { EuiBasicTable } from '@elastic/eui'; +import { FilterOptions, PaginationOptions, Rule } from '../types'; + +export type LoadingRuleAction = + | 'load' + | 'duplicate' + | 'enable' + | 'disable' + | 'export' + | 'delete' + | null; + +export interface RulesTableState { + rules: Rule[]; + pagination: PaginationOptions; + filterOptions: FilterOptions; + loadingRulesAction: LoadingRuleAction; + loadingRuleIds: string[]; + selectedRuleIds: string[]; + exportRuleIds: string[]; + lastUpdated: number; + isRefreshOn: boolean; + showIdleModal: boolean; +} + +export type RulesTableAction = + | { type: 'setRules'; rules: Rule[]; pagination: Partial } + | { type: 'updateRules'; rules: Rule[] } + | { + type: 'updateFilterOptions'; + filterOptions: Partial; + pagination: Partial; + } + | { type: 'loadingRuleIds'; ids: string[]; actionType: LoadingRuleAction } + | { type: 'selectedRuleIds'; ids: string[] } + | { type: 'exportRuleIds'; ids: string[] } + | { type: 'setLastRefreshDate' } + | { type: 'setAutoRefreshOn'; on: boolean } + | { type: 'setShowIdleModal'; show: boolean } + | { type: 'failure' }; + +export const createRulesTableReducer = ( + tableRef: React.MutableRefObject | undefined> +) => { + const rulesTableReducer = (state: RulesTableState, action: RulesTableAction): RulesTableState => { + switch (action.type) { + case 'setRules': { + if ( + tableRef != null && + tableRef.current != null && + tableRef.current.changeSelection != null + ) { + // for future devs: eui basic table is not giving us a prop to set the value, so + // we are using the ref in setTimeout to reset on the next loop so that we + // do not get a warning telling us we are trying to update during a render + window.setTimeout(() => tableRef?.current?.changeSelection([]), 0); + } + + return { + ...state, + rules: action.rules, + selectedRuleIds: [], + loadingRuleIds: [], + loadingRulesAction: null, + pagination: { + ...state.pagination, + ...action.pagination, + }, + }; + } + case 'updateRules': { + const ruleIds = state.rules.map((r) => r.id); + const updatedRules = action.rules.reduce((rules, updatedRule) => { + let newRules = rules; + if (ruleIds.includes(updatedRule.id)) { + newRules = newRules.map((r) => (updatedRule.id === r.id ? updatedRule : r)); + } else { + newRules = [...newRules, updatedRule]; + } + return newRules; + }, state.rules); + const updatedRuleIds = action.rules.map((r) => r.id); + const newLoadingRuleIds = state.loadingRuleIds.filter((id) => !updatedRuleIds.includes(id)); + return { + ...state, + rules: updatedRules, + loadingRuleIds: newLoadingRuleIds, + loadingRulesAction: newLoadingRuleIds.length === 0 ? null : state.loadingRulesAction, + }; + } + case 'updateFilterOptions': { + return { + ...state, + filterOptions: { + ...state.filterOptions, + ...action.filterOptions, + }, + pagination: { + ...state.pagination, + ...action.pagination, + }, + }; + } + case 'loadingRuleIds': { + return { + ...state, + loadingRuleIds: action.actionType == null ? [] : [...state.loadingRuleIds, ...action.ids], + loadingRulesAction: action.actionType, + }; + } + case 'selectedRuleIds': { + return { + ...state, + selectedRuleIds: action.ids, + }; + } + case 'exportRuleIds': { + return { + ...state, + loadingRuleIds: action.ids, + loadingRulesAction: 'export', + exportRuleIds: action.ids, + }; + } + case 'setLastRefreshDate': { + return { + ...state, + lastUpdated: Date.now(), + }; + } + case 'setAutoRefreshOn': { + return { + ...state, + isRefreshOn: action.on, + }; + } + case 'setShowIdleModal': { + return { + ...state, + showIdleModal: action.show, + isRefreshOn: !action.show, + }; + } + case 'failure': { + return { + ...state, + rules: [], + }; + } + default: { + return state; + } + } + }; + + return rulesTableReducer; +}; diff --git a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_rules.test.tsx b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/rules_table/use_rules.test.tsx similarity index 99% rename from x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_rules.test.tsx rename to x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/rules_table/use_rules.test.tsx index 5d226e8152596..4532d3427375b 100644 --- a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_rules.test.tsx +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/rules_table/use_rules.test.tsx @@ -7,9 +7,9 @@ import { renderHook, act } from '@testing-library/react-hooks'; import { useRules, UseRules, ReturnRules } from './use_rules'; -import * as api from './api'; +import * as api from '../api'; -jest.mock('./api'); +jest.mock('../api'); describe('useRules', () => { beforeEach(() => { diff --git a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_rules.tsx b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/rules_table/use_rules.tsx similarity index 93% rename from x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_rules.tsx rename to x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/rules_table/use_rules.tsx index 02784333992be..f3c90ae12ae33 100644 --- a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_rules.tsx +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/rules_table/use_rules.tsx @@ -7,10 +7,10 @@ import { useEffect, useState, useRef } from 'react'; -import { FetchRulesResponse, FilterOptions, PaginationOptions, Rule } from './types'; -import { errorToToaster, useStateToaster } from '../../../../common/components/toasters'; -import { fetchRules } from './api'; -import * as i18n from './translations'; +import { FetchRulesResponse, FilterOptions, PaginationOptions, Rule } from '../types'; +import { errorToToaster, useStateToaster } from '../../../../../common/components/toasters'; +import { fetchRules } from '../api'; +import * as i18n from '../translations'; export type ReturnRules = [boolean, FetchRulesResponse | null, () => Promise]; diff --git a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/rules_table/use_rules_table.ts b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/rules_table/use_rules_table.ts new file mode 100644 index 0000000000000..f31b2894301ba --- /dev/null +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/rules_table/use_rules_table.ts @@ -0,0 +1,131 @@ +/* + * 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 { Dispatch, useMemo, useReducer, useEffect, useRef } from 'react'; +import { EuiBasicTable } from '@elastic/eui'; + +import { errorToToaster, useStateToaster } from '../../../../../common/components/toasters'; +import * as i18n from '../translations'; + +import { fetchRules } from '../api'; +import { createRulesTableReducer, RulesTableState, RulesTableAction } from './rules_table_reducer'; +import { createRulesTableFacade, RulesTableFacade } from './rules_table_facade'; + +const INITIAL_SORT_FIELD = 'enabled'; + +const initialStateDefaults: RulesTableState = { + rules: [], + pagination: { + page: 1, + perPage: 20, + total: 0, + }, + filterOptions: { + filter: '', + sortField: INITIAL_SORT_FIELD, + sortOrder: 'desc', + tags: [], + showCustomRules: false, + showElasticRules: false, + }, + loadingRulesAction: null, + loadingRuleIds: [], + selectedRuleIds: [], + exportRuleIds: [], + lastUpdated: 0, + isRefreshOn: true, + showIdleModal: false, +}; + +export interface UseRulesTableParams { + tableRef: React.MutableRefObject | undefined>; + initialStateOverride?: Partial; +} + +export interface UseRulesTableReturn extends RulesTableFacade { + state: RulesTableState; + dispatch: Dispatch; + reFetchRules: () => Promise; +} + +export const useRulesTable = (params: UseRulesTableParams): UseRulesTableReturn => { + const { tableRef, initialStateOverride } = params; + + const initialState: RulesTableState = { + ...initialStateDefaults, + lastUpdated: Date.now(), + ...initialStateOverride, + }; + + const reducer = useMemo(() => createRulesTableReducer(tableRef), [tableRef]); + const [state, dispatch] = useReducer(reducer, initialState); + const facade = useRef(createRulesTableFacade(dispatch)); + + const reFetchRules = useRef<() => Promise>(() => Promise.resolve()); + const [, dispatchToaster] = useStateToaster(); + + const { pagination, filterOptions } = state; + const filterTags = filterOptions.tags.sort().join(); + + useEffect(() => { + let isSubscribed = true; + const abortCtrl = new AbortController(); + + const fetchData = async () => { + try { + facade.current.actionStarted('load', []); + + const fetchRulesResult = await fetchRules({ + filterOptions, + pagination, + signal: abortCtrl.signal, + }); + + if (isSubscribed) { + facade.current.setRules(fetchRulesResult.data, { + page: fetchRulesResult.page, + perPage: fetchRulesResult.perPage, + total: fetchRulesResult.total, + }); + } + } catch (error) { + if (isSubscribed) { + errorToToaster({ title: i18n.RULE_AND_TIMELINE_FETCH_FAILURE, error, dispatchToaster }); + facade.current.setRules([], {}); + } + } + if (isSubscribed) { + facade.current.actionStopped(); + } + }; + + fetchData(); + reFetchRules.current = () => fetchData(); + + return () => { + isSubscribed = false; + abortCtrl.abort(); + }; + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [ + pagination.page, + pagination.perPage, + filterOptions.filter, + filterOptions.sortField, + filterOptions.sortOrder, + filterTags, + filterOptions.showCustomRules, + filterOptions.showElasticRules, + ]); + + return { + state, + dispatch, + ...facade.current, + reFetchRules: reFetchRules.current, + }; +}; diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/actions.tsx b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/actions.tsx index 44b0476501a5b..3b1f9e620127d 100644 --- a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/actions.tsx +++ b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/actions.tsx @@ -13,6 +13,7 @@ import { duplicateRules, enableRules, Rule, + RulesTableAction, } from '../../../../containers/detection_engine/rules'; import { getEditRuleUrl } from '../../../../../common/components/link_to/redirect_to_detection_engine'; @@ -27,7 +28,6 @@ import { track, METRIC_TYPE, TELEMETRY_EVENT } from '../../../../../common/lib/t import * as i18n from '../translations'; import { bucketRulesResponse } from './helpers'; -import { Action } from './reducer'; export const editRuleAction = (rule: Rule, history: H.History) => { history.push(getEditRuleUrl(rule.id)); @@ -36,7 +36,7 @@ export const editRuleAction = (rule: Rule, history: H.History) => { export const duplicateRulesAction = async ( rules: Rule[], ruleIds: string[], - dispatch: React.Dispatch, + dispatch: React.Dispatch, dispatchToaster: Dispatch ) => { try { @@ -59,13 +59,16 @@ export const duplicateRulesAction = async ( } }; -export const exportRulesAction = (exportRuleId: string[], dispatch: React.Dispatch) => { +export const exportRulesAction = ( + exportRuleId: string[], + dispatch: React.Dispatch +) => { dispatch({ type: 'exportRuleIds', ids: exportRuleId }); }; export const deleteRulesAction = async ( ruleIds: string[], - dispatch: React.Dispatch, + dispatch: React.Dispatch, dispatchToaster: Dispatch, onRuleDeleted?: () => void ) => { @@ -96,7 +99,7 @@ export const deleteRulesAction = async ( export const enableRulesAction = async ( ids: string[], enabled: boolean, - dispatch: React.Dispatch, + dispatch: React.Dispatch, dispatchToaster: Dispatch ) => { const errorTitle = enabled diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/batch_actions.tsx b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/batch_actions.tsx index ee19a0b535075..d3e055a695d61 100644 --- a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/batch_actions.tsx +++ b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/batch_actions.tsx @@ -8,7 +8,7 @@ import { EuiContextMenuItem, EuiToolTip } from '@elastic/eui'; import React, { Dispatch } from 'react'; import * as i18n from '../translations'; -import { Action } from './reducer'; +import { RulesTableAction } from '../../../../containers/detection_engine/rules/rules_table'; import { deleteRulesAction, duplicateRulesAction, @@ -23,7 +23,7 @@ import { canEditRuleWithActions } from '../../../../../common/utils/privileges'; interface GetBatchItems { closePopover: () => void; - dispatch: Dispatch; + dispatch: Dispatch; dispatchToaster: Dispatch; hasMlPermissions: boolean; hasActionsPrivileges: boolean; diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/columns.tsx b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/columns.tsx index d110f2d52b3c5..d2488bd3d043c 100644 --- a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/columns.tsx +++ b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/columns.tsx @@ -34,14 +34,14 @@ import { editRuleAction, exportRulesAction, } from './actions'; -import { Action } from './reducer'; +import { RulesTableAction } from '../../../../containers/detection_engine/rules/rules_table'; import { LocalizedDateTooltip } from '../../../../../common/components/localized_date_tooltip'; import { LinkAnchor } from '../../../../../common/components/links'; import { getToolTipContent, canEditRuleWithActions } from '../../../../../common/utils/privileges'; import { TagsDisplay } from './tag_display'; export const getActions = ( - dispatch: React.Dispatch, + dispatch: React.Dispatch, dispatchToaster: Dispatch, history: H.History, reFetchRules: () => Promise, @@ -112,7 +112,7 @@ export type RulesColumns = EuiBasicTableColumn | EuiTableActionsColumnType export type RulesStatusesColumns = EuiBasicTableColumn; type FormatUrl = (path: string) => string; interface GetColumns { - dispatch: React.Dispatch; + dispatch: React.Dispatch; dispatchToaster: Dispatch; formatUrl: FormatUrl; history: H.History; diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/index.test.tsx b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/index.test.tsx index a784d23ab0015..7f9061b6abc83 100644 --- a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/index.test.tsx +++ b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/index.test.tsx @@ -13,7 +13,7 @@ import '../../../../../common/mock/match_media'; import '../../../../../common/mock/formatted_relative'; import { AllRules } from './index'; import { useKibana, useUiSetting$ } from '../../../../../common/lib/kibana'; -import { useRules, useRulesStatuses } from '../../../../containers/detection_engine/rules'; +import { useRulesTable, useRulesStatuses } from '../../../../containers/detection_engine/rules'; import { TestProviders } from '../../../../../common/mock'; import { createUseUiSetting$Mock } from '../../../../../common/lib/kibana/kibana_react.mock'; import { @@ -40,6 +40,7 @@ jest.mock('../../../../containers/detection_engine/rules'); const useKibanaMock = useKibana as jest.Mocked; const mockUseUiSetting$ = useUiSetting$ as jest.Mock; +const mockUseRulesTable = useRulesTable as jest.Mock; describe('AllRules', () => { const mockRefetchRulesData = jest.fn(); @@ -62,13 +63,9 @@ describe('AllRules', () => { : useUiSetting$Mock(key, defaultValue); }); - (useRules as jest.Mock).mockReturnValue([ - false, - { - page: 1, - perPage: 20, - total: 1, - data: [ + mockUseRulesTable.mockImplementation(({ initialStateOverride }) => { + const initialState = { + rules: [ { actions: [], created_at: '2020-02-14T19:49:28.178Z', @@ -101,9 +98,42 @@ describe('AllRules', () => { version: 1, }, ], - }, - mockRefetchRulesData, - ]); + pagination: { + page: 1, + perPage: 20, + total: 1, + }, + filterOptions: { + filter: '', + sortField: 'enabled', + sortOrder: 'desc', + tags: [], + showCustomRules: false, + showElasticRules: false, + }, + loadingRulesAction: null, + loadingRuleIds: [], + selectedRuleIds: [], + exportRuleIds: [], + lastUpdated: 0, + isRefreshOn: true, + showIdleModal: false, + }; + + return { + state: { ...initialState, ...initialStateOverride }, + dispatch: jest.fn(), + reFetchRules: mockRefetchRulesData, + setRules: jest.fn(), + updateRules: jest.fn(), + updateOptions: jest.fn(), + actionStarted: jest.fn(), + actionStopped: jest.fn(), + setShowIdleModal: jest.fn(), + setLastRefreshDate: jest.fn(), + setAutoRefreshOn: jest.fn(), + }; + }); (useRulesStatuses as jest.Mock).mockReturnValue({ loading: false, diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/reducer.ts b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/reducer.ts deleted file mode 100644 index 60798f10a4c58..0000000000000 --- a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/reducer.ts +++ /dev/null @@ -1,156 +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 type React from 'react'; -import { EuiBasicTable } from '@elastic/eui'; -import { - FilterOptions, - PaginationOptions, - Rule, -} from '../../../../containers/detection_engine/rules'; - -type LoadingRuleAction = 'duplicate' | 'enable' | 'disable' | 'export' | 'delete' | null; -export interface State { - exportRuleIds: string[]; - filterOptions: FilterOptions; - loadingRuleIds: string[]; - loadingRulesAction: LoadingRuleAction; - pagination: PaginationOptions; - rules: Rule[]; - selectedRuleIds: string[]; - lastUpdated: number; - showIdleModal: boolean; - isRefreshOn: boolean; -} - -export type Action = - | { type: 'exportRuleIds'; ids: string[] } - | { type: 'loadingRuleIds'; ids: string[]; actionType: LoadingRuleAction } - | { type: 'selectedRuleIds'; ids: string[] } - | { type: 'setRules'; rules: Rule[]; pagination: Partial } - | { type: 'updateRules'; rules: Rule[] } - | { - type: 'updateFilterOptions'; - filterOptions: Partial; - pagination: Partial; - } - | { type: 'failure' } - | { type: 'setLastRefreshDate' } - | { type: 'setShowIdleModal'; show: boolean } - | { type: 'setAutoRefreshOn'; on: boolean }; - -export const allRulesReducer = ( - tableRef: React.MutableRefObject | undefined> -) => (state: State, action: Action): State => { - switch (action.type) { - case 'exportRuleIds': { - return { - ...state, - loadingRuleIds: action.ids, - loadingRulesAction: 'export', - exportRuleIds: action.ids, - }; - } - case 'loadingRuleIds': { - return { - ...state, - loadingRuleIds: action.actionType == null ? [] : [...state.loadingRuleIds, ...action.ids], - loadingRulesAction: action.actionType, - }; - } - case 'selectedRuleIds': { - return { - ...state, - selectedRuleIds: action.ids, - }; - } - case 'setRules': { - if ( - tableRef != null && - tableRef.current != null && - tableRef.current.changeSelection != null - ) { - // for future devs: eui basic table is not giving us a prop to set the value, so - // we are using the ref in setTimeout to reset on the next loop so that we - // do not get a warning telling us we are trying to update during a render - window.setTimeout(() => tableRef?.current?.changeSelection([]), 0); - } - - return { - ...state, - rules: action.rules, - selectedRuleIds: [], - loadingRuleIds: [], - loadingRulesAction: null, - pagination: { - ...state.pagination, - ...action.pagination, - }, - }; - } - case 'updateRules': { - const ruleIds = state.rules.map((r) => r.id); - const updatedRules = action.rules.reduce((rules, updatedRule) => { - let newRules = rules; - if (ruleIds.includes(updatedRule.id)) { - newRules = newRules.map((r) => (updatedRule.id === r.id ? updatedRule : r)); - } else { - newRules = [...newRules, updatedRule]; - } - return newRules; - }, state.rules); - const updatedRuleIds = action.rules.map((r) => r.id); - const newLoadingRuleIds = state.loadingRuleIds.filter((id) => !updatedRuleIds.includes(id)); - return { - ...state, - rules: updatedRules, - loadingRuleIds: newLoadingRuleIds, - loadingRulesAction: newLoadingRuleIds.length === 0 ? null : state.loadingRulesAction, - }; - } - case 'updateFilterOptions': { - return { - ...state, - filterOptions: { - ...state.filterOptions, - ...action.filterOptions, - }, - pagination: { - ...state.pagination, - ...action.pagination, - }, - }; - } - case 'failure': { - return { - ...state, - rules: [], - }; - } - case 'setLastRefreshDate': { - return { - ...state, - lastUpdated: Date.now(), - }; - } - case 'setShowIdleModal': { - return { - ...state, - showIdleModal: action.show, - isRefreshOn: !action.show, - }; - } - case 'setAutoRefreshOn': { - return { - ...state, - isRefreshOn: action.on, - }; - } - default: - return state; - } -}; diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/rules_tables.tsx b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/rules_tables.tsx index 04bf3c544030a..a40833d8d14ac 100644 --- a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/rules_tables.tsx +++ b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/rules_tables.tsx @@ -12,21 +12,21 @@ import { EuiConfirmModal, EuiWindowEvent, } from '@elastic/eui'; -import React, { useCallback, useEffect, useMemo, useReducer, useRef, useState } from 'react'; +import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import uuid from 'uuid'; import { debounce } from 'lodash/fp'; import { History } from 'history'; import { - useRules, + useRulesTable, useRulesStatuses, CreatePreBuiltRules, FilterOptions, Rule, - PaginationOptions, exportRules, RulesSortingFields, } from '../../../../containers/detection_engine/rules'; + import { FormatUrl } from '../../../../../common/components/link_to'; import { HeaderSection } from '../../../../../common/components/header_section'; import { useKibana, useUiSetting$ } from '../../../../../common/lib/kibana'; @@ -42,7 +42,6 @@ import { EuiBasicTableOnChange } from '../types'; import { getBatchItems } from './batch_actions'; import { getColumns, getMonitoringColumns } from './columns'; import { showRulesTable } from './helpers'; -import { allRulesReducer, State } from './reducer'; import { RulesTableFilters } from './rules_table_filters/rules_table_filters'; import { useMlCapabilities } from '../../../../../common/components/ml/hooks/use_ml_capabilities'; import { hasMlAdminPermissions } from '../../../../../../common/machine_learning/has_ml_admin_permissions'; @@ -54,29 +53,6 @@ import { DEFAULT_RULES_TABLE_REFRESH_SETTING } from '../../../../../../common/co import { AllRulesTabs } from '.'; const INITIAL_SORT_FIELD = 'enabled'; -const initialState: State = { - exportRuleIds: [], - filterOptions: { - filter: '', - sortField: INITIAL_SORT_FIELD, - sortOrder: 'desc', - tags: [], - showCustomRules: false, - showElasticRules: false, - }, - loadingRuleIds: [], - loadingRulesAction: null, - pagination: { - page: 1, - perPage: 20, - total: 0, - }, - rules: [], - selectedRuleIds: [], - lastUpdated: 0, - showIdleModal: false, - isRefreshOn: true, -}; interface RulesTableProps { history: History; @@ -119,7 +95,7 @@ export const RulesTables = React.memo( selectedTab, }) => { const [initLoading, setInitLoading] = useState(true); - const tableRef = useRef(); + const { services: { application: { @@ -127,30 +103,47 @@ export const RulesTables = React.memo( }, }, } = useKibana(); + + const tableRef = useRef(); + const [defaultAutoRefreshSetting] = useUiSetting$<{ on: boolean; value: number; idleTimeout: number; }>(DEFAULT_RULES_TABLE_REFRESH_SETTING); - const [ - { - exportRuleIds, - filterOptions, - loadingRuleIds, - loadingRulesAction, - pagination, - rules, - selectedRuleIds, - lastUpdated, - showIdleModal, - isRefreshOn, + + const rulesTable = useRulesTable({ + tableRef, + initialStateOverride: { + isRefreshOn: defaultAutoRefreshSetting.on, }, - dispatch, - ] = useReducer(allRulesReducer(tableRef), { - ...initialState, - lastUpdated: Date.now(), - isRefreshOn: defaultAutoRefreshSetting.on, }); + + const { + exportRuleIds, + filterOptions, + loadingRuleIds, + loadingRulesAction, + pagination, + rules, + selectedRuleIds, + lastUpdated, + showIdleModal, + isRefreshOn, + } = rulesTable.state; + + const { + dispatch, + updateOptions, + actionStopped, + setShowIdleModal, + setLastRefreshDate, + setAutoRefreshOn, + reFetchRules, + } = rulesTable; + + const isLoadingRules = loadingRulesAction === 'load'; + const { loading: isLoadingRulesStatuses, rulesStatuses } = useRulesStatuses(rules); const [, dispatchToaster] = useStateToaster(); const mlCapabilities = useMlCapabilities(); @@ -158,40 +151,6 @@ export const RulesTables = React.memo( // TODO: Refactor license check + hasMlAdminPermissions to common check const hasMlPermissions = hasMlLicense(mlCapabilities) && hasMlAdminPermissions(mlCapabilities); - const setRules = useCallback((newRules: Rule[], newPagination: Partial) => { - dispatch({ - type: 'setRules', - rules: newRules, - pagination: newPagination, - }); - }, []); - - const setShowIdleModal = useCallback((show: boolean) => { - dispatch({ - type: 'setShowIdleModal', - show, - }); - }, []); - - const setLastRefreshDate = useCallback(() => { - dispatch({ - type: 'setLastRefreshDate', - }); - }, []); - - const setAutoRefreshOn = useCallback((on: boolean) => { - dispatch({ - type: 'setAutoRefreshOn', - on, - }); - }, []); - - const [isLoadingRules, , reFetchRules] = useRules({ - pagination, - filterOptions, - dispatchRulesInReducer: setRules, - }); - const sorting = useMemo( (): SortingType => ({ sort: { @@ -250,18 +209,24 @@ export const RulesTables = React.memo( [pagination] ); + const onFilterChangedCallback = useCallback( + (newFilter: Partial) => { + updateOptions(newFilter, { page: 1 }); + }, + [updateOptions] + ); + const tableOnChangeCallback = useCallback( ({ page, sort }: EuiBasicTableOnChange) => { - dispatch({ - type: 'updateFilterOptions', - filterOptions: { + updateOptions( + { sortField: (sort?.field as RulesSortingFields) ?? INITIAL_SORT_FIELD, // Narrowing EuiBasicTable sorting types sortOrder: sort?.direction ?? 'desc', }, - pagination: { page: page.index + 1, perPage: page.size }, - }); + { page: page.index + 1, perPage: page.size } + ); }, - [dispatch] + [updateOptions] ); const rulesColumns = useMemo(() => { @@ -324,19 +289,9 @@ export const RulesTables = React.memo( onSelectionChange: (selected: Rule[]) => dispatch({ type: 'selectedRuleIds', ids: selected.map((r) => r.id) }), }), - [loadingRuleIds] + [loadingRuleIds, dispatch] ); - const onFilterChangedCallback = useCallback((newFilterOptions: Partial) => { - dispatch({ - type: 'updateFilterOptions', - filterOptions: { - ...newFilterOptions, - }, - pagination: { page: 1 }, - }); - }, []); - const isLoadingAnActionOnRule = useMemo(() => { if ( loadingRuleIds.length > 0 && @@ -412,7 +367,7 @@ export const RulesTables = React.memo( const handleGenericDownloaderSuccess = useCallback( (exportCount) => { - dispatch({ type: 'loadingRuleIds', ids: [], actionType: null }); + actionStopped(); dispatchToaster({ type: 'addToaster', toast: { @@ -423,7 +378,7 @@ export const RulesTables = React.memo( }, }); }, - [dispatchToaster] + [actionStopped, dispatchToaster] ); return (