Skip to content

Commit

Permalink
[SIEM] [Detections] Refactor the all rules page (#58428) (#58570)
Browse files Browse the repository at this point in the history
* Refactor the all rules page to be easier to test

* review with Garrett

* bring back utility bar under condition

* fix bugs tags and allow switch to show loading when enable/disable rule

* fix rules selection when trigerring new rules

* fix imports/exports can only use rule_id as learned today

* review I
  • Loading branch information
XavierM authored Feb 26, 2020
1 parent 42b2f90 commit 050fd1a
Show file tree
Hide file tree
Showing 21 changed files with 461 additions and 575 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@
*/

import { renderHook, act } from '@testing-library/react-hooks';
import { useRules, ReturnRules } from './use_rules';
import { useRules, UseRules, ReturnRules } from './use_rules';
import * as api from './api';
import { PaginationOptions, FilterOptions } from '.';

jest.mock('./api');

Expand All @@ -17,55 +16,40 @@ describe('useRules', () => {
});
test('init', async () => {
await act(async () => {
const { result, waitForNextUpdate } = renderHook<
[PaginationOptions, FilterOptions],
ReturnRules
>(props =>
useRules(
{
const { result, waitForNextUpdate } = renderHook<UseRules, ReturnRules>(props =>
useRules({
pagination: {
page: 1,
perPage: 10,
total: 100,
},
{
filterOptions: {
filter: '',
sortField: 'created_at',
sortOrder: 'desc',
}
)
},
})
);
await waitForNextUpdate();
expect(result.current).toEqual([
true,
{
data: [],
page: 1,
perPage: 20,
total: 0,
},
null,
]);
expect(result.current).toEqual([true, null, result.current[2]]);
});
});

test('fetch rules', async () => {
await act(async () => {
const { result, waitForNextUpdate } = renderHook<
[PaginationOptions, FilterOptions],
ReturnRules
>(() =>
useRules(
{
const { result, waitForNextUpdate } = renderHook<UseRules, ReturnRules>(() =>
useRules({
pagination: {
page: 1,
perPage: 10,
total: 100,
},
{
filterOptions: {
filter: '',
sortField: 'created_at',
sortOrder: 'desc',
}
)
},
})
);
await waitForNextUpdate();
await waitForNextUpdate();
Expand Down Expand Up @@ -148,22 +132,19 @@ describe('useRules', () => {
test('re-fetch rules', async () => {
const spyOnfetchRules = jest.spyOn(api, 'fetchRules');
await act(async () => {
const { result, waitForNextUpdate } = renderHook<
[PaginationOptions, FilterOptions],
ReturnRules
>(id =>
useRules(
{
const { result, waitForNextUpdate } = renderHook<UseRules, ReturnRules>(id =>
useRules({
pagination: {
page: 1,
perPage: 10,
total: 100,
},
{
filterOptions: {
filter: '',
sortField: 'created_at',
sortOrder: 'desc',
}
)
},
})
);
await waitForNextUpdate();
await waitForNextUpdate();
Expand All @@ -178,37 +159,37 @@ describe('useRules', () => {
test('fetch rules if props changes', async () => {
const spyOnfetchRules = jest.spyOn(api, 'fetchRules');
await act(async () => {
const { rerender, waitForNextUpdate } = renderHook<
[PaginationOptions, FilterOptions],
ReturnRules
>(args => useRules(args[0], args[1]), {
initialProps: [
{
page: 1,
perPage: 10,
total: 100,
},
{
filter: '',
sortField: 'created_at',
sortOrder: 'desc',
const { rerender, waitForNextUpdate } = renderHook<UseRules, ReturnRules>(
args => useRules(args),
{
initialProps: {
pagination: {
page: 1,
perPage: 10,
total: 100,
},
filterOptions: {
filter: '',
sortField: 'created_at',
sortOrder: 'desc',
},
},
],
});
}
);
await waitForNextUpdate();
await waitForNextUpdate();
rerender([
{
rerender({
pagination: {
page: 1,
perPage: 10,
total: 100,
},
{
filterOptions: {
filter: 'hello world',
sortField: 'created_at',
sortOrder: 'desc',
},
]);
});
await waitForNextUpdate();
expect(spyOnfetchRules).toHaveBeenCalledTimes(2);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,34 +4,42 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { noop } from 'lodash/fp';
import { useEffect, useState, useRef } from 'react';

import { FetchRulesResponse, FilterOptions, PaginationOptions } from './types';
import { FetchRulesResponse, FilterOptions, PaginationOptions, Rule } from './types';
import { useStateToaster } from '../../../components/toasters';
import { fetchRules } from './api';
import { errorToToaster } from '../../../components/ml/api/error_to_toaster';
import * as i18n from './translations';

type Func = () => void;
export type ReturnRules = [boolean, FetchRulesResponse, Func | null];
export type ReturnRules = [
boolean,
FetchRulesResponse | null,
(refreshPrePackagedRule?: boolean) => void
];

export interface UseRules {
pagination: PaginationOptions;
filterOptions: FilterOptions;
refetchPrePackagedRulesStatus?: () => void;
dispatchRulesInReducer?: (rules: Rule[]) => void;
}

/**
* Hook for using the list of Rules from the Detection Engine API
*
* @param pagination desired pagination options (e.g. page/perPage)
* @param filterOptions desired filters (e.g. filter/sortField/sortOrder)
*/
export const useRules = (
pagination: PaginationOptions,
filterOptions: FilterOptions
): ReturnRules => {
const [rules, setRules] = useState<FetchRulesResponse>({
page: 1,
perPage: 20,
total: 0,
data: [],
});
const reFetchRules = useRef<Func | null>(null);
export const useRules = ({
pagination,
filterOptions,
refetchPrePackagedRulesStatus,
dispatchRulesInReducer,
}: UseRules): ReturnRules => {
const [rules, setRules] = useState<FetchRulesResponse | null>(null);
const reFetchRules = useRef<(refreshPrePackagedRule?: boolean) => void>(noop);
const [loading, setLoading] = useState(true);
const [, dispatchToaster] = useStateToaster();

Expand All @@ -50,10 +58,16 @@ export const useRules = (

if (isSubscribed) {
setRules(fetchRulesResult);
if (dispatchRulesInReducer != null) {
dispatchRulesInReducer(fetchRulesResult.data);
}
}
} catch (error) {
if (isSubscribed) {
errorToToaster({ title: i18n.RULE_FETCH_FAILURE, error, dispatchToaster });
if (dispatchRulesInReducer != null) {
dispatchRulesInReducer([]);
}
}
}
if (isSubscribed) {
Expand All @@ -62,7 +76,12 @@ export const useRules = (
}

fetchData();
reFetchRules.current = fetchData.bind(null, true);
reFetchRules.current = (refreshPrePackagedRule: boolean = false) => {
fetchData(true);
if (refreshPrePackagedRule && refetchPrePackagedRulesStatus != null) {
refetchPrePackagedRulesStatus();
}
};
return () => {
isSubscribed = false;
abortCtrl.abort();
Expand All @@ -76,6 +95,7 @@ export const useRules = (
filterOptions.tags?.sort().join(),
filterOptions.showCustomRules,
filterOptions.showElasticRules,
refetchPrePackagedRulesStatus,
]);

return [loading, rules, reFetchRules.current];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ describe('useTags', () => {
await act(async () => {
const { result, waitForNextUpdate } = renderHook<unknown, ReturnTags>(() => useTags());
await waitForNextUpdate();
expect(result.current).toEqual([true, []]);
expect(result.current).toEqual([true, [], result.current[2]]);
});
});

Expand All @@ -23,7 +23,11 @@ describe('useTags', () => {
const { result, waitForNextUpdate } = renderHook<unknown, ReturnTags>(() => useTags());
await waitForNextUpdate();
await waitForNextUpdate();
expect(result.current).toEqual([false, ['elastic', 'love', 'quality', 'code']]);
expect(result.current).toEqual([
false,
['elastic', 'love', 'quality', 'code'],
result.current[2],
]);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { useEffect, useState } from 'react';
import { noop } from 'lodash/fp';
import { useEffect, useState, useRef } from 'react';
import { useStateToaster } from '../../../components/toasters';
import { fetchTags } from './api';
import { errorToToaster } from '../../../components/ml/api/error_to_toaster';
import * as i18n from './translations';

export type ReturnTags = [boolean, string[]];
export type ReturnTags = [boolean, string[], () => void];

/**
* Hook for using the list of Tags from the Detection Engine API
Expand All @@ -20,6 +21,7 @@ export const useTags = (): ReturnTags => {
const [tags, setTags] = useState<string[]>([]);
const [loading, setLoading] = useState(true);
const [, dispatchToaster] = useStateToaster();
const reFetchTags = useRef<() => void>(noop);

useEffect(() => {
let isSubscribed = true;
Expand All @@ -46,12 +48,13 @@ export const useTags = (): ReturnTags => {
}

fetchData();
reFetchTags.current = fetchData;

return () => {
isSubscribed = false;
abortCtrl.abort();
};
}, []);

return [loading, tags];
return [loading, tags, reFetchTags.current];
};
Loading

0 comments on commit 050fd1a

Please sign in to comment.