addresses comments and updates tests

elastic · Jun 20, 2023 · 9d8ae0c · 9d8ae0c
1 parent 0119e69
commit 9d8ae0c
Show file tree

Hide file tree

Showing 6 changed files with 34 additions and 36 deletions.
diff --git a/x-pack/plugins/security_solution/cypress/e2e/detection_alerts/alert_tags.cy.ts b/x-pack/plugins/security_solution/cypress/e2e/detection_alerts/alert_tags.cy.ts
@@ -9,7 +9,6 @@ import { getNewRule } from '../../objects/rule';
 import {
   clickAlertTag,
   openAlertTaggingBulkActionMenu,
-  openAlertTaggingContextMenu,
   selectNumberOfAlerts,
   updateAlertTags,
 } from '../../tasks/alerts';
@@ -18,7 +17,12 @@ import { cleanKibana, deleteAlertsAndRules } from '../../tasks/common';
 import { login, visit } from '../../tasks/login';
 import { ALERTS_URL } from '../../urls/navigation';
 import { waitForAlertsToPopulate } from '../../tasks/create_new_rule';
-import { MIXED_ALERT_TAG, SELECTED_ALERT_TAG, UNSELECTED_ALERT_TAG } from '../../screens/alerts';
+import {
+  ALERTS_TABLE_ROW_LOADER,
+  MIXED_ALERT_TAG,
+  SELECTED_ALERT_TAG,
+  UNSELECTED_ALERT_TAG,
+} from '../../screens/alerts';
 import { esArchiverLoad, esArchiverResetKibana, esArchiverUnload } from '../../tasks/es_archiver';
 
 describe('Alert tagging', () => {
@@ -40,36 +44,23 @@ describe('Alert tagging', () => {
     esArchiverUnload('endpoint');
   });
 
-  it('Add and remove tag using the alert context menu', () => {
-    // Add a tag to one alert
-    openAlertTaggingContextMenu();
-    clickAlertTag('Duplicate');
-    updateAlertTags();
-    waitForAlertsToPopulate();
-    openAlertTaggingContextMenu();
-    cy.get(SELECTED_ALERT_TAG).contains('Duplicate');
-    // Remove tag from that alert
-    clickAlertTag('Duplicate');
-    updateAlertTags();
-    waitForAlertsToPopulate();
-    openAlertTaggingContextMenu();
-    cy.get(UNSELECTED_ALERT_TAG).first().contains('Duplicate');
-  });
-
   it('Add and remove a tag using the alert bulk action menu', () => {
     // Add a tag to one alert
     selectNumberOfAlerts(1);
     openAlertTaggingBulkActionMenu();
     clickAlertTag('Duplicate');
     updateAlertTags();
-    waitForAlertsToPopulate();
+    // waitForAlertsToPopulate();
+    cy.get(ALERTS_TABLE_ROW_LOADER).should('not.exist');
+    // waitForAlerts();
     selectNumberOfAlerts(1);
     openAlertTaggingBulkActionMenu();
     cy.get(SELECTED_ALERT_TAG).contains('Duplicate');
     // Remove tag from that alert
     clickAlertTag('Duplicate');
     updateAlertTags();
-    waitForAlertsToPopulate();
+    // waitForAlertsToPopulate();
+    cy.get(ALERTS_TABLE_ROW_LOADER).should('not.exist');
     selectNumberOfAlerts(1);
     openAlertTaggingBulkActionMenu();
     cy.get(UNSELECTED_ALERT_TAG).first().contains('Duplicate');
@@ -81,14 +72,16 @@ describe('Alert tagging', () => {
     openAlertTaggingBulkActionMenu();
     clickAlertTag('Duplicate');
     updateAlertTags();
-    waitForAlertsToPopulate();
+    // waitForAlertsToPopulate();
+    cy.get(ALERTS_TABLE_ROW_LOADER).should('not.exist');
     // Then add tags to both alerts
     selectNumberOfAlerts(2);
     openAlertTaggingBulkActionMenu();
     cy.get(MIXED_ALERT_TAG).contains('Duplicate');
     clickAlertTag('Duplicate');
     updateAlertTags();
-    waitForAlertsToPopulate();
+    // waitForAlertsToPopulate();
+    cy.get(ALERTS_TABLE_ROW_LOADER).should('not.exist');
     selectNumberOfAlerts(2);
     openAlertTaggingBulkActionMenu();
     cy.get(SELECTED_ALERT_TAG).contains('Duplicate');
@@ -100,15 +93,17 @@ describe('Alert tagging', () => {
     openAlertTaggingBulkActionMenu();
     clickAlertTag('Duplicate');
     updateAlertTags();
-    waitForAlertsToPopulate();
+    // waitForAlertsToPopulate();
+    cy.get(ALERTS_TABLE_ROW_LOADER).should('not.exist');
     // Then remove tags from both alerts
     selectNumberOfAlerts(2);
     openAlertTaggingBulkActionMenu();
     cy.get(MIXED_ALERT_TAG).contains('Duplicate');
     clickAlertTag('Duplicate');
     clickAlertTag('Duplicate'); // Clicking twice will return to unselected state
     updateAlertTags();
-    waitForAlertsToPopulate();
+    // waitForAlertsToPopulate();
+    cy.get(ALERTS_TABLE_ROW_LOADER).should('not.exist');
     selectNumberOfAlerts(2);
     openAlertTaggingBulkActionMenu();
     cy.get(UNSELECTED_ALERT_TAG).first().contains('Duplicate');

diff --git a/x-pack/plugins/security_solution/cypress/screens/alerts.ts b/x-pack/plugins/security_solution/cypress/screens/alerts.ts
@@ -199,3 +199,5 @@ export const SELECTED_ALERT_TAG = '[data-test-subj="selected-alert-tag"]';
 export const MIXED_ALERT_TAG = '[data-test-subj="mixed-alert-tag"]';
 
 export const UNSELECTED_ALERT_TAG = '[data-test-subj="unselected-alert-tag"]';
+
+export const ALERTS_TABLE_ROW_LOADER = '[data-test-subj="row-loader"]';
diff --git a/.../security_solution/public/common/components/toolbar/bulk_actions/alert_bulk_tags.test.tsx b/.../security_solution/public/common/components/toolbar/bulk_actions/alert_bulk_tags.test.tsx
@@ -35,9 +35,7 @@ const mockTagItems = [
   addSuccess: jest.fn(),
   addWarning: jest.fn(),
 });
-(useSetAlertTags as jest.Mock).mockReturnValue({
-  setAlertTags: jest.fn(),
-});
+(useSetAlertTags as jest.Mock).mockReturnValue([false, jest.fn()]);
 (getUpdateAlertsQuery as jest.Mock).mockReturnValue({ query: {} });
 
 describe('BulkAlertTagsPanel', () => {

diff --git a/x-pack/plugins/security_solution/public/common/components/toolbar/bulk_actions/index.tsx b/x-pack/plugins/security_solution/public/common/components/toolbar/bulk_actions/index.tsx
@@ -6,7 +6,7 @@
  */
 
 import { EuiPopover, EuiButtonEmpty, EuiContextMenu } from '@elastic/eui';
-import React, { useState, useCallback } from 'react';
+import React, { useState, useCallback, useMemo } from 'react';
 import styled from 'styled-components';
 import type { AlertTableContextMenuItem } from '../../../../detections/components/alerts_table/types';
 
@@ -61,12 +61,15 @@ const BulkActionsComponent: React.FC<OwnProps> = ({
     }
   }, [onClearSelection, onSelectAll, showClearSelection]);
 
-  const panels = [
-    {
-      id: 0,
-      items: bulkActionItems,
-    },
-  ];
+  const panels = useMemo(
+    () => [
+      {
+        id: 0,
+        items: bulkActionItems,
+      },
+    ],
+    [bulkActionItems]
+  );
 
   return (
     <BulkActionsContainer

diff --git a/...ns/security_solution/public/common/components/toolbar/bulk_actions/use_set_alert_tags.tsx b/...ns/security_solution/public/common/components/toolbar/bulk_actions/use_set_alert_tags.tsx
@@ -80,12 +80,12 @@ export const useSetAlertTags = (): ReturnSetAlertTags => {
           }
         );
         if (!ignore) {
+          setTableLoading(false);
           onSuccess();
           if (response.version_conflicts && ids.length === 1) {
             throw new Error(i18n.BULK_ACTION_FAILED_SINGLE_ALERT);
           }
           setIsLoading(false);
-          setTableLoading(false);
           onUpdateSuccess(response.updated ?? 0, response.version_conflicts ?? 0);
         }
       } catch (error) {

diff --git a/...gins/security_solution/server/lib/detection_engine/routes/signals/set_alert_tags_route.ts b/...gins/security_solution/server/lib/detection_engine/routes/signals/set_alert_tags_route.ts
@@ -62,7 +62,7 @@ export const setAlertTagsRoute = (router: SecuritySolutionPluginRouter) => {
       try {
         const body = await esClient.updateByQuery({
           index: `${DEFAULT_ALERTS_INDEX}-${spaceId}`,
-          refresh: true,
+          refresh: false,
           body: {
             script: {
               params: { tagsToAdd, tagsToRemove },
Original file line number	Diff line number	Diff line change
Expand Up		@@ -199,3 +199,5 @@ export const SELECTED_ALERT_TAG = '[data-test-subj="selected-alert-tag"]';
		export const MIXED_ALERT_TAG = '[data-test-subj="mixed-alert-tag"]';

		export const UNSELECTED_ALERT_TAG = '[data-test-subj="unselected-alert-tag"]';

		export const ALERTS_TABLE_ROW_LOADER = '[data-test-subj="row-loader"]';