From 291c6a5a194508b3bb6abcaf9562f25efbe13a53 Mon Sep 17 00:00:00 2001 From: Zacqary Adam Xeper Date: Thu, 15 Feb 2024 12:30:08 -0600 Subject: [PATCH 1/4] [RAM] Fix bug where select all rules bypasses filters (#176962) ## Summary Fixes #176867 A bug introduced in https://github.com/elastic/kibana/pull/174954 bypassed most filters when using Select All on the Rules List. This was because the names of the filter properties changed, and no longer matched what the `useBulkEditSelect` hook was expecting. Because all of these types were optional, it didn't trigger any type errors. This syncs up the type definitions with the new filter store hook, and adds a functional test to make sure filters are working on bulk actions when clicking the select all button. ### Checklist - [x] [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 (cherry picked from commit e136a9318215d5913a5e957aec5d9ad0b8132506) --- .../hooks/use_bulk_edit_select.test.tsx | 15 ++-- .../hooks/use_bulk_edit_select.tsx | 55 ++++-------- .../rules_list/components/rules_list.tsx | 3 +- .../rules_list/bulk_actions.ts | 87 ++++++++++++++++++- 4 files changed, 111 insertions(+), 49 deletions(-) diff --git a/x-pack/plugins/triggers_actions_ui/public/application/hooks/use_bulk_edit_select.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/hooks/use_bulk_edit_select.test.tsx index 07bf3516fbdef..1a5e3f278eb5e 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/hooks/use_bulk_edit_select.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/hooks/use_bulk_edit_select.test.tsx @@ -45,8 +45,7 @@ describe('useBulkEditSelectTest', () => { useBulkEditSelect({ items, totalItemCount: 4, - tagsFilter: ['test: 123'], - searchText: 'rules*', + filters: { tags: ['test: 123'], searchText: 'rules*' }, }) ); @@ -58,8 +57,7 @@ describe('useBulkEditSelectTest', () => { useBulkEditSelect({ items, totalItemCount: 4, - tagsFilter: ['test: 123'], - searchText: 'rules*', + filters: { tags: ['test: 123'], searchText: 'rules*' }, }) ); @@ -107,8 +105,7 @@ describe('useBulkEditSelectTest', () => { useBulkEditSelect({ items, totalItemCount: 4, - tagsFilter: ['test: 123'], - searchText: 'rules*', + filters: { tags: ['test: 123'], searchText: 'rules*' }, }) ); @@ -124,8 +121,10 @@ describe('useBulkEditSelectTest', () => { useBulkEditSelect({ items, totalItemCount: 4, - tagsFilter: ['test: 123'], - searchText: 'rules*', + filters: { + tags: ['test: 123'], + searchText: 'rules*', + }, }) ); diff --git a/x-pack/plugins/triggers_actions_ui/public/application/hooks/use_bulk_edit_select.tsx b/x-pack/plugins/triggers_actions_ui/public/application/hooks/use_bulk_edit_select.tsx index 84e762cbe93f8..fe70b4fa0e3bc 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/hooks/use_bulk_edit_select.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/hooks/use_bulk_edit_select.tsx @@ -7,7 +7,7 @@ import { useReducer, useMemo, useCallback } from 'react'; import { fromKueryExpression, nodeBuilder } from '@kbn/es-query'; import { mapFiltersToKueryNode } from '../lib/rule_api/map_filters_to_kuery_node'; -import { RuleTableItem, RuleStatus } from '../../types'; +import { RuleTableItem, RulesListFilters } from '../../types'; interface BulkEditSelectionState { selectedIds: Set; @@ -73,27 +73,11 @@ const reducer = (state: BulkEditSelectionState, action: Action) => { interface UseBulkEditSelectProps { totalItemCount: number; items: RuleTableItem[]; - typesFilter?: string[]; - actionTypesFilter?: string[]; - tagsFilter?: string[]; - ruleExecutionStatusesFilter?: string[]; - ruleLastRunOutcomesFilter?: string[]; - ruleStatusesFilter?: RuleStatus[]; - searchText?: string; + filters?: RulesListFilters; } export function useBulkEditSelect(props: UseBulkEditSelectProps) { - const { - totalItemCount = 0, - items = [], - typesFilter, - actionTypesFilter, - tagsFilter, - ruleExecutionStatusesFilter, - ruleLastRunOutcomesFilter, - ruleStatusesFilter, - searchText, - } = props; + const { totalItemCount = 0, items = [], filters } = props; const [state, dispatch] = useReducer(reducer, { ...initialState, @@ -187,15 +171,20 @@ export function useBulkEditSelect(props: UseBulkEditSelectProps) { const getFilterKueryNode = useCallback( (idsToExclude?: string[]) => { - const ruleFilterKueryNode = mapFiltersToKueryNode({ - typesFilter, - actionTypesFilter, - tagsFilter, - ruleExecutionStatusesFilter, - ruleLastRunOutcomesFilter, - ruleStatusesFilter, - searchText, - }); + const ruleFilterKueryNode = mapFiltersToKueryNode( + filters + ? { + typesFilter: filters.types, + actionTypesFilter: filters.actionTypes, + tagsFilter: filters.tags, + ruleExecutionStatusesFilter: filters.ruleExecutionStatuses, + ruleLastRunOutcomesFilter: filters.ruleLastRunOutcomes, + ruleParamsFilter: filters.ruleParams, + ruleStatusesFilter: filters.ruleStatuses, + searchText: filters.searchText, + } + : {} + ); if (idsToExclude && idsToExclude.length) { const excludeFilter = fromKueryExpression( @@ -209,15 +198,7 @@ export function useBulkEditSelect(props: UseBulkEditSelectProps) { return ruleFilterKueryNode; }, - [ - typesFilter, - actionTypesFilter, - tagsFilter, - ruleExecutionStatusesFilter, - ruleLastRunOutcomesFilter, - ruleStatusesFilter, - searchText, - ] + [filters] ); const getFilter = useCallback(() => { diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rules_list/components/rules_list.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rules_list/components/rules_list.tsx index 67d475ae2689e..eb82c6c14583f 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rules_list/components/rules_list.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rules_list/components/rules_list.tsx @@ -363,8 +363,7 @@ export const RulesList = ({ } = useBulkEditSelect({ totalItemCount: rulesState.totalItemCount, items: tableItems, - ...filters, - typesFilter: rulesTypesFilter, + filters: { ...filters, types: rulesTypesFilter }, }); const handleUpdateFiltersEffect = useCallback( diff --git a/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/rules_list/bulk_actions.ts b/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/rules_list/bulk_actions.ts index 617429f3a23c9..50c8fc10f4a35 100644 --- a/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/rules_list/bulk_actions.ts +++ b/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/rules_list/bulk_actions.ts @@ -14,7 +14,12 @@ import expect from '@kbn/expect'; import { FtrProviderContext } from '../../../ftr_provider_context'; -import { createAlert, scheduleRule, snoozeAlert } from '../../../lib/alert_api_actions'; +import { + createAlert, + createAlertManualCleanup, + scheduleRule, + snoozeAlert, +} from '../../../lib/alert_api_actions'; import { ObjectRemover } from '../../../lib/object_remover'; export default ({ getPageObjects, getService }: FtrProviderContext) => { @@ -29,7 +34,7 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => { await testSubjects.click('rulesTab'); } - describe('rules list', () => { + describe('rules list bulk actions', () => { before(async () => { await pageObjects.common.navigateToApp('triggersActions'); await testSubjects.click('rulesTab'); @@ -205,5 +210,83 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => { expect(toastTitle).to.eql('Updated API key for 1 rule.'); }); }); + + it('should apply filters to bulk actions when using the select all button', async () => { + const rule1 = await createAlert({ + supertest, + objectRemover, + overwrites: { name: 'a' }, + }); + const rule2 = await createAlertManualCleanup({ + supertest, + overwrites: { name: 'b', rule_type_id: 'test.always-firing' }, + }); + const rule3 = await createAlert({ + supertest, + objectRemover, + overwrites: { name: 'c' }, + }); + + await refreshAlertsList(); + expect(await testSubjects.getVisibleText('totalRulesCount')).to.be('3 rules'); + + await testSubjects.click('ruleTypeFilterButton'); + await testSubjects.existOrFail('ruleTypetest.noopFilterOption'); + await testSubjects.click('ruleTypetest.noopFilterOption'); + await testSubjects.click(`checkboxSelectRow-${rule1.id}`); + await testSubjects.click('selectAllRulesButton'); + + await testSubjects.click('showBulkActionButton'); + await testSubjects.click('bulkDisable'); + await testSubjects.existOrFail('untrackAlertsModal'); + await testSubjects.click('confirmModalConfirmButton'); + + await retry.try(async () => { + const toastTitle = await toasts.getTitleAndDismiss(); + expect(toastTitle).to.eql('Disabled 2 rules'); + }); + + await testSubjects.click('rules-list-clear-filter'); + await refreshAlertsList(); + + await pageObjects.triggersActionsUI.searchAlerts(rule1.name); + expect(await testSubjects.getVisibleText('statusDropdown')).to.be('Disabled'); + await pageObjects.triggersActionsUI.searchAlerts(rule2.name); + expect(await testSubjects.getVisibleText('statusDropdown')).to.be('Enabled'); + await pageObjects.triggersActionsUI.searchAlerts(rule3.name); + expect(await testSubjects.getVisibleText('statusDropdown')).to.be('Disabled'); + + await testSubjects.click('rules-list-clear-filter'); + await refreshAlertsList(); + + await testSubjects.click('ruleStatusFilterButton'); + await testSubjects.existOrFail('ruleStatusFilterOption-enabled'); + await testSubjects.click('ruleStatusFilterOption-enabled'); + await testSubjects.click(`checkboxSelectRow-${rule2.id}`); + await testSubjects.click('selectAllRulesButton'); + + await testSubjects.click('showBulkActionButton'); + await testSubjects.click('bulkDelete'); + await testSubjects.existOrFail('rulesDeleteConfirmation'); + await testSubjects.click('confirmModalConfirmButton'); + + await retry.try(async () => { + const toastTitle = await toasts.getTitleAndDismiss(); + expect(toastTitle).to.eql('Deleted 1 rule'); + }); + + await testSubjects.click('rules-list-clear-filter'); + await refreshAlertsList(); + + await retry.try( + async () => { + expect(await testSubjects.getVisibleText('totalRulesCount')).to.be('2 rules'); + }, + async () => { + // If the delete fails, make sure rule2 gets cleaned up + objectRemover.add(rule2.id, 'alert', 'alerts'); + } + ); + }); }); }; From 83f7aff37350ac96c140afa1d3a6edc6b7aa5329 Mon Sep 17 00:00:00 2001 From: Zacqary Xeper Date: Thu, 15 Feb 2024 16:36:04 -0600 Subject: [PATCH 2/4] Remove untrack alerts modal check --- .../apps/triggers_actions_ui/rules_list/bulk_actions.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/rules_list/bulk_actions.ts b/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/rules_list/bulk_actions.ts index 50c8fc10f4a35..4c4f879f209a9 100644 --- a/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/rules_list/bulk_actions.ts +++ b/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/rules_list/bulk_actions.ts @@ -238,8 +238,6 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => { await testSubjects.click('showBulkActionButton'); await testSubjects.click('bulkDisable'); - await testSubjects.existOrFail('untrackAlertsModal'); - await testSubjects.click('confirmModalConfirmButton'); await retry.try(async () => { const toastTitle = await toasts.getTitleAndDismiss(); From db8ed66f80430c7d1321ec2c57a99633c4ccdf23 Mon Sep 17 00:00:00 2001 From: Zacqary Xeper Date: Mon, 4 Mar 2024 15:51:48 -0600 Subject: [PATCH 3/4] Fix typechecks --- .../plugins/triggers_actions_ui/public/types.ts | 16 ++++++++-------- .../rules_list/bulk_actions.ts | 1 + 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/x-pack/plugins/triggers_actions_ui/public/types.ts b/x-pack/plugins/triggers_actions_ui/public/types.ts index 655a3e6de8662..dece3f172c55a 100644 --- a/x-pack/plugins/triggers_actions_ui/public/types.ts +++ b/x-pack/plugins/triggers_actions_ui/public/types.ts @@ -782,14 +782,14 @@ export interface ConnectorServices { } export interface RulesListFilters { - actionTypes: string[]; - ruleExecutionStatuses: string[]; - ruleLastRunOutcomes: string[]; - ruleParams: Record; - ruleStatuses: RuleStatus[]; - searchText: string; - tags: string[]; - types: string[]; + actionTypes?: string[]; + ruleExecutionStatuses?: string[]; + ruleLastRunOutcomes?: string[]; + ruleParams?: Record; + ruleStatuses?: RuleStatus[]; + searchText?: string; + tags?: string[]; + types?: string[]; kueryNode?: KueryNode; } diff --git a/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/rules_list/bulk_actions.ts b/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/rules_list/bulk_actions.ts index 4c4f879f209a9..8c1876e92b913 100644 --- a/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/rules_list/bulk_actions.ts +++ b/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/rules_list/bulk_actions.ts @@ -28,6 +28,7 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => { const supertest = getService('supertest'); const retry = getService('retry'); const objectRemover = new ObjectRemover(supertest); + const toasts = getService('toasts'); async function refreshAlertsList() { await testSubjects.click('logsTab'); From 50e0ce89fef13748bd07489a737aaace0d173a9f Mon Sep 17 00:00:00 2001 From: Zacqary Xeper Date: Fri, 8 Mar 2024 15:10:21 -0600 Subject: [PATCH 4/4] Fix typecheck and functional test --- .../rules_list/components/rules_list_filters_bar.tsx | 12 ++++++------ .../triggers_actions_ui/rules_list/bulk_actions.ts | 5 ++--- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/rules_list/components/rules_list_filters_bar.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/rules_list/components/rules_list_filters_bar.tsx index 038b79bded049..8181815478926 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/sections/rules_list/components/rules_list_filters_bar.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/sections/rules_list/components/rules_list_filters_bar.tsx @@ -81,7 +81,7 @@ export const RulesListFiltersBar = React.memo((props: RulesListFiltersBarProps) isGrouped refresh={refresh} canLoadRules={canLoadRules} - selectedTags={filters.tags} + selectedTags={filters.tags!} onChange={(value) => updateFilters({ filter: 'tags', value })} />, ]; @@ -93,7 +93,7 @@ export const RulesListFiltersBar = React.memo((props: RulesListFiltersBarProps) if (isRuleStatusFilterEnabled) { return ( updateFilters({ filter: 'ruleStatuses', value })} /> ); @@ -106,7 +106,7 @@ export const RulesListFiltersBar = React.memo((props: RulesListFiltersBarProps) return [ updateFilters({ filter: 'ruleExecutionStatuses', value })} />, ]; @@ -114,7 +114,7 @@ export const RulesListFiltersBar = React.memo((props: RulesListFiltersBarProps) return [ updateFilters({ filter: 'ruleLastRunOutcomes', value })} />, ]; @@ -124,14 +124,14 @@ export const RulesListFiltersBar = React.memo((props: RulesListFiltersBarProps) updateFilters({ filter: 'types', value })} />, showActionFilter && ( updateFilters({ filter: 'actionTypes', value })} /> ), diff --git a/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/rules_list/bulk_actions.ts b/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/rules_list/bulk_actions.ts index 8c1876e92b913..df754be0506b2 100644 --- a/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/rules_list/bulk_actions.ts +++ b/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/rules_list/bulk_actions.ts @@ -28,7 +28,6 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => { const supertest = getService('supertest'); const retry = getService('retry'); const objectRemover = new ObjectRemover(supertest); - const toasts = getService('toasts'); async function refreshAlertsList() { await testSubjects.click('logsTab'); @@ -241,7 +240,7 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => { await testSubjects.click('bulkDisable'); await retry.try(async () => { - const toastTitle = await toasts.getTitleAndDismiss(); + const toastTitle = await pageObjects.common.closeToast(); expect(toastTitle).to.eql('Disabled 2 rules'); }); @@ -270,7 +269,7 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => { await testSubjects.click('confirmModalConfirmButton'); await retry.try(async () => { - const toastTitle = await toasts.getTitleAndDismiss(); + const toastTitle = await pageObjects.common.closeToast(); expect(toastTitle).to.eql('Deleted 1 rule'); });