Skip to content
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.

Sign up for GitHub

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

[Alerting] Reload the Alerts List when alerts are deleted #73715

Merged
merged 8 commits into from
Aug 5, 2020
Merged

[Alerting] Reload the Alerts List when alerts are deleted #73715

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
12ff182
reload alerts when an alert is deleted in the list
gmmorris Jul 29, 2020
c4ceec1
added e2e test
gmmorris Jul 29, 2020
d63f1f7
Merge branch 'master' into alerting/refresh-on-deletion
gmmorris Jul 30, 2020
61e75ec
improved e2e test
gmmorris Jul 30, 2020
2024d25
Merge branch 'master' into alerting/refresh-on-deletion
gmmorris Aug 5, 2020
d794d96
Merge branch 'master' into alerting/refresh-on-deletion
gmmorris Aug 5, 2020
e1efef9
reset page index in alerts list if the loaded page is empty and were …
gmmorris Aug 5, 2020
c8f8001
Merge branch 'master' into alerting/refresh-on-deletion
gmmorris Aug 5, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 2 additions & 11 deletions ...ns/triggers_actions_ui/public/application/sections/alerts_list/components/alerts_list.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,17 +400,8 @@ export const AlertsList: React.FunctionComponent = () => {
<section data-test-subj="alertsList">
<DeleteModalConfirmation
onDeleted={(deleted: string[]) => {
if (selectedIds.length === 0 || selectedIds.length === deleted.length) {
const updatedAlerts = alertsState.data.filter(
(alert) => alert.id && !alertsToDelete.includes(alert.id)
);
setAlertsState({
isLoading: false,
data: updatedAlerts,
totalItemCount: alertsState.totalItemCount - deleted.length,
});
setSelectedIds([]);
}
loadAlertsData();
setSelectedIds([]);
setAlertsToDelete([]);
}}
onErrors={async () => {
Expand Down
24 changes: 19 additions & 5 deletions x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/alerts.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/

import uuid from 'uuid';
import { times } from 'lodash';
import expect from '@kbn/expect';
import { FtrProviderContext } from '../../ftr_provider_context';

Expand Down Expand Up @@ -361,11 +362,22 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => {
});

it('should delete all selection', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Seems like testing for this could be covered in a unit test instead of end to end.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can test the implementation of the fix in a unit test, but you can't test the actual behaviour as it relies on hooks and calls being made inside of other components (including EUI).
You could never be sure it's working correctly with just a unit test...

I have a real fear that we're compromising on test quality instead of addressing the scaling issues in our infrastructure. :/

const createdAlert = await createAlert();
const namePrefix = generateUniqueKey();
let count = 0;
const createdAlertsFirstPage = await Promise.all(
times(10, () => createAlert({ name: `${namePrefix}-0${count++}` }))
);

const createdAlertsSecondPage = await Promise.all(
times(2, () => createAlert({ name: `${namePrefix}-1${count++}` }))
);

await pageObjects.common.navigateToApp('triggersActions');
await pageObjects.triggersActionsUI.searchAlerts(createdAlert.name);
await pageObjects.triggersActionsUI.searchAlerts(namePrefix);

await testSubjects.click(`checkboxSelectRow-${createdAlert.id}`);
for (const createdAlert of createdAlertsFirstPage) {
await testSubjects.click(`checkboxSelectRow-${createdAlert.id}`);
}

await testSubjects.click('bulkAction');

Expand All @@ -377,9 +389,11 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => {
await pageObjects.common.closeToast();

await pageObjects.common.navigateToApp('triggersActions');
await pageObjects.triggersActionsUI.searchAlerts(createdAlert.name);
await pageObjects.triggersActionsUI.searchAlerts(namePrefix);
const searchResultsAfterDelete = await pageObjects.triggersActionsUI.getAlertsList();
expect(searchResultsAfterDelete.length).to.eql(0);
expect(searchResultsAfterDelete).to.have.length(2);
expect(searchResultsAfterDelete[0].name).to.eql(createdAlertsSecondPage[0].name);
expect(searchResultsAfterDelete[1].name).to.eql(createdAlertsSecondPage[1].name);
});
});
};