-
Notifications
You must be signed in to change notification settings - Fork 8.6k
[ResponseOps][Rules] Create Rules APIs package #214187
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
0be26ba
edcde44
67b20ac
08b7383
9cf0f85
e05ef91
e1e55f4
c2a425f
706a8be
9e77938
d9d4c47
1865c02
01bfbbd
d69bef0
752b87f
270c282
0561de5
590f8df
789a713
77c45c4
027bfef
f04eb76
859f879
e115542
c933c02
6a3b06d
74ed203
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,7 @@ import type { | |
| import type { Filter } from '@kbn/es-query'; | ||
| import type { RuleNotifyWhenType, RRuleParams } from '.'; | ||
|
|
||
| export type RuleTypeSolution = 'observability' | 'security' | 'stack'; | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Had to move this here to avoid circular dependencies between plugins and packages |
||
| export type RuleTypeParams = Record<string, unknown>; | ||
| export type RuleActionParams = SavedObjectAttributes; | ||
| export type RuleActionParam = SavedObjectAttribute; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,168 @@ | ||
| /* | ||
| * 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", the "GNU Affero General Public License v3.0 only", and the "Server Side | ||
| * Public License v 1"; you may not use this file except in compliance with, at | ||
| * your election, the "Elastic License 2.0", the "GNU Affero General Public | ||
| * License v3.0 only", or the "Server Side Public License, v 1". | ||
| */ | ||
|
|
||
| import React, { PropsWithChildren } from 'react'; | ||
| import { httpServiceMock } from '@kbn/core/public/mocks'; | ||
| import { notificationServiceMock } from '@kbn/core/public/mocks'; | ||
| import { renderHook, waitFor } from '@testing-library/react'; | ||
| import { useGetRuleTypesPermissions } from './use_get_rule_types_permissions'; | ||
| import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; | ||
| import { testQueryClientConfig } from '../test_utils/test_query_client_config'; | ||
|
|
||
| const http = httpServiceMock.createStartContract(); | ||
| const { toasts } = notificationServiceMock.createStartContract(); | ||
|
|
||
| jest.mock('@kbn/response-ops-rules-apis/apis/get_rule_types'); | ||
| const { getRuleTypes } = jest.requireMock('@kbn/response-ops-rules-apis/apis/get_rule_types'); | ||
| getRuleTypes.mockResolvedValue([ | ||
| { | ||
| id: 'rule-type-1', | ||
| authorizedConsumers: {}, | ||
| }, | ||
| { | ||
| id: 'rule-type-2', | ||
| authorizedConsumers: {}, | ||
| }, | ||
| ]); | ||
|
|
||
| const queryClient = new QueryClient(testQueryClientConfig); | ||
| const Wrapper = ({ children }: PropsWithChildren) => ( | ||
| <QueryClientProvider client={queryClient}>{children}</QueryClientProvider> | ||
| ); | ||
|
|
||
| describe('useGetRuleTypesPermissions', () => { | ||
| afterEach(() => { | ||
| queryClient.clear(); | ||
| }); | ||
|
|
||
| it('should not filter the rule types if `filteredRuleTypes` and `registeredRuleTypes` are not defined', async () => { | ||
| const { result } = renderHook( | ||
| () => | ||
| useGetRuleTypesPermissions({ | ||
| http, | ||
| toasts, | ||
| enabled: true, | ||
| }), | ||
| { | ||
| wrapper: Wrapper, | ||
| } | ||
| ); | ||
| await waitFor(() => result.current.isSuccess); | ||
| expect(result.current.ruleTypesState.data.size).toBe(2); | ||
| expect(result.current.authorizedRuleTypes.length).toBe(2); | ||
| }); | ||
|
|
||
| it('should filter the rule types according to `filteredRuleTypes`', async () => { | ||
| const { result } = renderHook( | ||
| () => | ||
| useGetRuleTypesPermissions({ | ||
| http, | ||
| toasts, | ||
| enabled: true, | ||
| filteredRuleTypes: ['rule-type-1'], | ||
| }), | ||
| { | ||
| wrapper: Wrapper, | ||
| } | ||
| ); | ||
| await waitFor(() => result.current.isSuccess); | ||
| expect(result.current.ruleTypesState.data.size).toBe(1); | ||
| expect(result.current.authorizedRuleTypes.length).toBe(1); | ||
| expect(result.current.ruleTypesState.data.keys().next().value).toBe('rule-type-1'); | ||
| }); | ||
|
|
||
| it('should filter out rule types not present in `registeredRuleTypes`', async () => { | ||
| const { result } = renderHook( | ||
| () => | ||
| useGetRuleTypesPermissions({ | ||
| http, | ||
| toasts, | ||
| enabled: true, | ||
| registeredRuleTypes: [{ id: 'rule-type-1', description: '' }], | ||
| }), | ||
| { | ||
| wrapper: Wrapper, | ||
| } | ||
| ); | ||
| await waitFor(() => result.current.isSuccess); | ||
| expect(result.current.ruleTypesState.data.size).toBe(1); | ||
| expect(result.current.authorizedRuleTypes.length).toBe(1); | ||
| expect(result.current.ruleTypesState.data.keys().next().value).toBe('rule-type-1'); | ||
| }); | ||
|
|
||
| it('should return the correct authz flags when no rule types are accessible', async () => { | ||
| getRuleTypes.mockResolvedValueOnce([]); | ||
| const { result } = renderHook( | ||
| () => | ||
| useGetRuleTypesPermissions({ | ||
| http, | ||
| toasts, | ||
| enabled: true, | ||
| }), | ||
| { | ||
| wrapper: Wrapper, | ||
| } | ||
| ); | ||
| await waitFor(() => result.current.isSuccess); | ||
| expect(result.current.ruleTypesState.data.size).toBe(0); | ||
| expect(result.current.hasAnyAuthorizedRuleType).toBe(false); | ||
| expect(result.current.authorizedToReadAnyRules).toBe(false); | ||
| expect(result.current.authorizedToCreateAnyRules).toBe(false); | ||
| }); | ||
|
|
||
| it('should return the correct authz flags for read-only rule types', async () => { | ||
| getRuleTypes.mockResolvedValueOnce([ | ||
| { | ||
| id: 'rule-type-1', | ||
| authorizedConsumers: { alerts: { read: true, all: false } }, | ||
| }, | ||
| ]); | ||
| const { result } = renderHook( | ||
| () => | ||
| useGetRuleTypesPermissions({ | ||
| http, | ||
| toasts, | ||
| enabled: true, | ||
| }), | ||
| { | ||
| wrapper: Wrapper, | ||
| } | ||
| ); | ||
| await waitFor(() => result.current.isSuccess); | ||
| expect(result.current.ruleTypesState.data.size).toBe(1); | ||
| expect(result.current.hasAnyAuthorizedRuleType).toBe(true); | ||
| expect(result.current.authorizedToReadAnyRules).toBe(true); | ||
| expect(result.current.authorizedToCreateAnyRules).toBe(false); | ||
| }); | ||
|
|
||
| it('should return the correct authz flags for read+write rule types', async () => { | ||
| getRuleTypes.mockResolvedValueOnce([ | ||
| { | ||
| id: 'rule-type-1', | ||
| authorizedConsumers: { alerts: { read: true, all: true } }, | ||
| }, | ||
| ]); | ||
| const { result } = renderHook( | ||
| () => | ||
| useGetRuleTypesPermissions({ | ||
| http, | ||
| toasts, | ||
| enabled: true, | ||
| }), | ||
| { | ||
| wrapper: Wrapper, | ||
| } | ||
| ); | ||
| await waitFor(() => result.current.isSuccess); | ||
| expect(result.current.ruleTypesState.data.size).toBe(1); | ||
| expect(result.current.hasAnyAuthorizedRuleType).toBe(true); | ||
| expect(result.current.authorizedToReadAnyRules).toBe(true); | ||
| expect(result.current.authorizedToCreateAnyRules).toBe(true); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,19 +9,19 @@ | |
|
|
||
| import { useMemo } from 'react'; | ||
| import { keyBy } from 'lodash'; | ||
| import { useQuery, UseQueryOptions } from '@tanstack/react-query'; | ||
| import { UseQueryOptions } from '@tanstack/react-query'; | ||
| import type { HttpStart } from '@kbn/core-http-browser'; | ||
| import type { ToastsStart } from '@kbn/core-notifications-browser'; | ||
| import { i18n } from '@kbn/i18n'; | ||
| import type { RuleType } from '@kbn/triggers-actions-ui-types'; | ||
| import { | ||
| RuleTypeIndexWithDescriptions, | ||
| RuleTypeWithDescription, | ||
| } from '@kbn/triggers-actions-ui-types'; | ||
| import { fetchRuleTypes } from '../apis/fetch_rule_types'; | ||
| import { useGetRuleTypesQuery } from '@kbn/response-ops-rules-apis/hooks/use_get_rule_types_query'; | ||
| import { i18n } from '@kbn/i18n'; | ||
| import { ALERTS_FEATURE_ID } from '../constants'; | ||
|
|
||
| export interface UseRuleTypesProps { | ||
| export interface UseGetRuleTypesPermissionsParams { | ||
| http: HttpStart; | ||
| toasts: ToastsStart; | ||
| filteredRuleTypes?: string[]; | ||
|
|
@@ -37,7 +37,7 @@ const getFilteredIndex = ({ | |
| }: { | ||
| data: Array<RuleType<string, string>>; | ||
| filteredRuleTypes?: string[]; | ||
| registeredRuleTypes: UseRuleTypesProps['registeredRuleTypes']; | ||
| registeredRuleTypes: UseGetRuleTypesPermissionsParams['registeredRuleTypes']; | ||
| }) => { | ||
| const index: RuleTypeIndexWithDescriptions = new Map(); | ||
| const registeredRuleTypesDictionary = registeredRuleTypes ? keyBy(registeredRuleTypes, 'id') : {}; | ||
|
|
@@ -63,19 +63,15 @@ const getFilteredIndex = ({ | |
| return filteredIndex; | ||
| }; | ||
|
|
||
| export const useLoadRuleTypesQuery = ({ | ||
| export const useGetRuleTypesPermissions = ({ | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This hook was acting as the main rule types fetcher but also as privilege checker, returning computed values and not the original query. I extracted the react-query part that only fetches rule types to the new package, keeping this hook as a more privilege-checking-focused one (hence the new name). I avoided further refactors such as removing the returned rule types registry thing since there is a bit of filtering logic that might be worth studying better, and moving the hook to the new package as it's not properly a data-fetching hook (I'm wondering where these higher-level hooks should belong in our new package structure 🤔). |
||
| http, | ||
| toasts, | ||
| filteredRuleTypes, | ||
| registeredRuleTypes, | ||
| context, | ||
| enabled = true, | ||
| }: UseRuleTypesProps) => { | ||
| const queryFn = () => { | ||
| return fetchRuleTypes({ http }); | ||
| }; | ||
|
|
||
| const onErrorFn = (error: Error) => { | ||
| }: UseGetRuleTypesPermissionsParams) => { | ||
| const onErrorFn = (error: unknown) => { | ||
| if (error) { | ||
| toasts.addDanger( | ||
| i18n.translate('alertsUIShared.hooks.useLoadRuleTypesQuery.unableToLoadRuleTypesMessage', { | ||
|
|
@@ -84,17 +80,15 @@ export const useLoadRuleTypesQuery = ({ | |
| ); | ||
| } | ||
| }; | ||
| const { data, isSuccess, isFetching, isInitialLoading, isLoading, error } = useQuery({ | ||
| queryKey: ['loadRuleTypes'], | ||
| queryFn, | ||
| onError: onErrorFn, | ||
| refetchOnWindowFocus: false, | ||
| // Leveraging TanStack Query's caching system to avoid duplicated requests as | ||
| // other state-sharing solutions turned out to be overly complex and less readable | ||
| staleTime: 60 * 1000, | ||
| enabled, | ||
| context, | ||
| }); | ||
|
|
||
| const { data, isSuccess, isFetching, isInitialLoading, isLoading, error } = useGetRuleTypesQuery( | ||
| { http }, | ||
| { | ||
| onError: onErrorFn, | ||
| enabled, | ||
| context, | ||
| } | ||
| ); | ||
|
|
||
| const filteredIndex = useMemo( | ||
| () => | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,15 +8,4 @@ | |
| */ | ||
|
|
||
| export const BASE_ALERTING_API_PATH = '/api/alerting'; | ||
|
|
||
| export const queryKeys = { | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I moved all queryKeys and queryMutations of the new packages to dedicated files to increase visibility and because they aren't properly constants in the JS reference sense of the term |
||
| root: 'alerts', | ||
| mutedAlerts: (ruleIds: string[]) => | ||
| [queryKeys.root, 'mutedInstanceIdsForRuleIds', ruleIds] as const, | ||
| }; | ||
|
|
||
| export const mutationKeys = { | ||
| root: 'alerts', | ||
| muteAlertInstance: () => [mutationKeys.root, 'muteAlertInstance'] as const, | ||
| unmuteAlertInstance: () => [mutationKeys.root, 'unmuteAlertInstance'] as const, | ||
| }; | ||
| export const INTERNAL_BASE_ALERTING_API_PATH = '/internal/alerting'; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using kebab-case for package folders as requested by the operations team