Skip to content

Commit

Permalink
[RAM] Fix bug where select all rules bypasses filters (elastic#176962)
Browse files Browse the repository at this point in the history
## Summary

Fixes elastic#176867 

A bug introduced in elastic#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
  • Loading branch information
Zacqary authored Feb 15, 2024
1 parent b4836ac commit e136a93
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 49 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@ describe('useBulkEditSelectTest', () => {
useBulkEditSelect({
items,
totalItemCount: 4,
tagsFilter: ['test: 123'],
searchText: 'rules*',
filters: { tags: ['test: 123'], searchText: 'rules*' },
})
);

Expand All @@ -58,8 +57,7 @@ describe('useBulkEditSelectTest', () => {
useBulkEditSelect({
items,
totalItemCount: 4,
tagsFilter: ['test: 123'],
searchText: 'rules*',
filters: { tags: ['test: 123'], searchText: 'rules*' },
})
);

Expand Down Expand Up @@ -107,8 +105,7 @@ describe('useBulkEditSelectTest', () => {
useBulkEditSelect({
items,
totalItemCount: 4,
tagsFilter: ['test: 123'],
searchText: 'rules*',
filters: { tags: ['test: 123'], searchText: 'rules*' },
})
);

Expand All @@ -124,8 +121,10 @@ describe('useBulkEditSelectTest', () => {
useBulkEditSelect({
items,
totalItemCount: 4,
tagsFilter: ['test: 123'],
searchText: 'rules*',
filters: {
tags: ['test: 123'],
searchText: 'rules*',
},
})
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<string>;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(
Expand All @@ -209,15 +198,7 @@ export function useBulkEditSelect(props: UseBulkEditSelectProps) {

return ruleFilterKueryNode;
},
[
typesFilter,
actionTypesFilter,
tagsFilter,
ruleExecutionStatusesFilter,
ruleLastRunOutcomesFilter,
ruleStatusesFilter,
searchText,
]
[filters]
);

const getFilter = useCallback(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -363,8 +363,7 @@ export const RulesList = ({
} = useBulkEditSelect({
totalItemCount: rulesState.totalItemCount,
items: tableItems,
...filters,
typesFilter: rulesTypesFilter,
filters: { ...filters, types: rulesTypesFilter },
});

const handleUpdateFiltersEffect = useCallback(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand All @@ -30,7 +35,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');
Expand Down Expand Up @@ -206,5 +211,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');
}
);
});
});
};

0 comments on commit e136a93

Please sign in to comment.