From 0625fb417b00fc8e14137bc528d5ff617f3dbe2a 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 # Conflicts: # x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/rules_tables.tsx --- .../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 | 175 +++++++----------- 15 files changed, 535 insertions(+), 312 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 (94%) 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 e3617e34d4dcc..2920ad940b524 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,41 +5,40 @@ * 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', sortOrder: 'desc', }, - 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 d719443ff07f0..c3e9980311515 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 94% 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 f6002991ed581..3a914a9be0abb 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 @@ -8,10 +8,10 @@ import { noop } from 'lodash/fp'; 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, 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 fa7c18587fc14..b1eabe368fbfd 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 0f84e0dd7cd16..1278f837b8c5d 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: (refreshPrePackagedRule?: boolean) => void, @@ -109,7 +109,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 4483dc4c6abee..f7f042c922efd 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 @@ -13,21 +13,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'; @@ -43,7 +43,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'; @@ -55,26 +54,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', - }, - loadingRuleIds: [], - loadingRulesAction: null, - pagination: { - page: 1, - perPage: 20, - total: 0, - }, - rules: [], - selectedRuleIds: [], - lastUpdated: 0, - showIdleModal: false, - isRefreshOn: true, -}; interface RulesTableProps { history: History; @@ -117,7 +96,7 @@ export const RulesTables = React.memo( selectedTab, }) => { const [initLoading, setInitLoading] = useState(true); - const tableRef = useRef(); + const { services: { application: { @@ -125,30 +104,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(); @@ -156,41 +152,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, , reFetchRulesData] = useRules({ - pagination, - filterOptions, - refetchPrePackagedRulesStatus, - dispatchRulesInReducer: setRules, - }); - const sorting = useMemo( (): SortingType => ({ sort: { @@ -221,7 +182,7 @@ export const RulesTables = React.memo( hasActionsPrivileges, loadingRuleIds, selectedRuleIds, - reFetchRules: reFetchRulesData, + reFetchRules, rules, }); }, @@ -230,7 +191,7 @@ export const RulesTables = React.memo( dispatchToaster, hasMlPermissions, loadingRuleIds, - reFetchRulesData, + reFetchRules, rules, selectedRuleIds, hasActionsPrivileges, @@ -247,18 +208,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(() => { @@ -274,7 +241,7 @@ export const RulesTables = React.memo( (loadingRulesAction === 'enable' || loadingRulesAction === 'disable') ? loadingRuleIds : [], - reFetchRules: reFetchRulesData, + reFetchRules: reFetchRules, hasReadActionsPrivileges: hasActionsPrivileges, }); // eslint-disable-next-line react-hooks/exhaustive-deps @@ -286,7 +253,7 @@ export const RulesTables = React.memo( history, loadingRuleIds, loadingRulesAction, - reFetchRulesData, + reFetchRules, ]); const monitoringColumns = useMemo(() => getMonitoringColumns(history, formatUrl), [ @@ -295,10 +262,10 @@ export const RulesTables = React.memo( ]); useEffect(() => { - if (reFetchRulesData != null) { - setRefreshRulesData(reFetchRulesData); + if (reFetchRules != null) { + setRefreshRulesData(reFetchRules); } - }, [reFetchRulesData, setRefreshRulesData]); + }, [reFetchRules, setRefreshRulesData]); useEffect(() => { if (initLoading && !loading && !isLoadingRules && !isLoadingRulesStatuses) { @@ -307,11 +274,11 @@ export const RulesTables = React.memo( }, [initLoading, loading, isLoadingRules, isLoadingRulesStatuses]); const handleCreatePrePackagedRules = useCallback(async () => { - if (createPrePackagedRules != null && reFetchRulesData != null) { + if (createPrePackagedRules != null && reFetchRules != null) { await createPrePackagedRules(); - reFetchRulesData(true); + reFetchRules(true); } - }, [createPrePackagedRules, reFetchRulesData]); + }, [createPrePackagedRules, reFetchRules]); const euiBasicTableSelectionProps = useMemo( () => ({ @@ -319,19 +286,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 && @@ -345,11 +302,11 @@ export const RulesTables = React.memo( }, [loadingRuleIds, loadingRulesAction]); const handleRefreshData = useCallback((): void => { - if (reFetchRulesData != null && !isLoadingAnActionOnRule) { - reFetchRulesData(true); + if (reFetchRules != null && !isLoadingAnActionOnRule) { + reFetchRules(true); setLastRefreshDate(); } - }, [reFetchRulesData, isLoadingAnActionOnRule, setLastRefreshDate]); + }, [reFetchRules, isLoadingAnActionOnRule, setLastRefreshDate]); const handleResetIdleTimer = useCallback((): void => { if (isRefreshOn) { @@ -406,7 +363,7 @@ export const RulesTables = React.memo( const handleGenericDownloaderSuccess = useCallback( (exportCount) => { - dispatch({ type: 'loadingRuleIds', ids: [], actionType: null }); + actionStopped(); dispatchToaster({ type: 'addToaster', toast: { @@ -417,7 +374,7 @@ export const RulesTables = React.memo( }, }); }, - [dispatchToaster] + [actionStopped, dispatchToaster] ); return (