[RAC] [Security Solution] Hides the Show top <field> action in char…

…t legends ## Summary Fixes <#108910> , which allowed users to launch a new `Top <field>` popover from an existing popover, to infinity (and beyond) The `Show top <field>` action is now hidden in the `Top <field>` popover's chart legend, and also: - In the legend items of charts throughout the Security Solution (e.g. on the `Overview` page, and in the `Trend` chart on the `Alerts` page) - For items in the `Count` aggregation table on the `Alerts` page ## Screenshots ### Before (Top `signal.rule.name` popover) ![before-top-signal-rule-name](https://user-images.githubusercontent.com/4459398/130302784-00a6c24d-17c8-4361-979e-01b8467f100e.png) _Above: It was possible to launch another `Top <field>` popover from the legend of an existing popover_ ### After (Top `signal.rule.name` popover) ![after-top-signal-rule-name](https://user-images.githubusercontent.com/4459398/130302925-d5aaa1ff-9565-4374-aa87-bde5880cb64f.png) _Above: It is no longer possible to launch another `Top <field>` popover from the legend of an existing popover_ ### Before (Chart legends) ![before-overview](https://user-images.githubusercontent.com/4459398/130303169-dc6c6de3-a2ba-40fe-a1f0-fe0d78b9638c.png) _Above: It was possible to launch a `Top <field>` popover from chart legends_ ### After (Chart legends) ![after-overview](https://user-images.githubusercontent.com/4459398/130303519-2eb0a60e-c6cd-4659-b6b2-d5ba234f668f.png) _Above: It is no longer possible to launch a `Top <field>` popover from chart legends_ ### Before (`Count` items) ![before-count](https://user-images.githubusercontent.com/4459398/130304111-b37373cf-1afb-41b8-9f38-b5d9b37cdb2d.png) _Above: It was possible to launch a `Top <field>` popover from `Count` items_ ### After (`Count` items) ![after-count](https://user-images.githubusercontent.com/4459398/130304166-fb641fa2-b52e-44ff-8210-0e228a43330c.png) _Above: It is no longer possible to launch a `Top <field>` popover from `Count` items_ cc @mdefazio
elastic · Aug 21, 2021 · b634e15 · b634e15
1 parent 92ec225
commit b634e15
Show file tree

Hide file tree

Showing 15 changed files with 78 additions and 99 deletions.
diff --git a/.../plugins/security_solution/public/common/components/charts/draggable_legend_item.test.tsx b/.../plugins/security_solution/public/common/components/charts/draggable_legend_item.test.tsx
@@ -54,4 +54,10 @@ describe('DraggableLegendItem', () => {
       wrapper.find(`[data-test-subj="legend-item-${legendItem.dataProviderId}"]`).first().text()
     ).toEqual(legendItem.value);
   });
+
+  it('always hides the Top N action for legend items', () => {
+    expect(
+      wrapper.find(`[data-test-subj="legend-item-${legendItem.dataProviderId}"]`).prop('hideTopN')
+    ).toEqual(true);
+  });
 });
diff --git a/x-pack/plugins/security_solution/public/common/components/charts/draggable_legend_item.tsx b/x-pack/plugins/security_solution/public/common/components/charts/draggable_legend_item.tsx
@@ -36,6 +36,7 @@ const DraggableLegendItemComponent: React.FC<{
           <DefaultDraggable
             data-test-subj={`legend-item-${dataProviderId}`}
             field={field}
+            hideTopN={true}
             id={dataProviderId}
             isDraggable={false}
             timelineId={timelineId}

diff --git a/...ck/plugins/security_solution/public/common/components/drag_and_drop/draggable_wrapper.tsx b/...ck/plugins/security_solution/public/common/components/drag_and_drop/draggable_wrapper.tsx
@@ -96,6 +96,7 @@ type RenderFunctionProp = (
 interface Props {
   dataProvider: DataProvider;
   disabled?: boolean;
+  hideTopN?: boolean;
   isDraggable?: boolean;
   inline?: boolean;
   render: RenderFunctionProp;
@@ -125,6 +126,7 @@ export const getStyle = (
 
 const DraggableOnWrapperComponent: React.FC<Props> = ({
   dataProvider,
+  hideTopN = false,
   onFilterAdded,
   render,
   timelineId,
@@ -147,6 +149,7 @@ const DraggableOnWrapperComponent: React.FC<Props> = ({
     showTopN,
   } = useHoverActions({
     dataProvider,
+    hideTopN,
     onFilterAdded,
     render,
     timelineId,
@@ -304,6 +307,7 @@ const DraggableOnWrapperComponent: React.FC<Props> = ({
 
 const DraggableWrapperComponent: React.FC<Props> = ({
   dataProvider,
+  hideTopN = false,
   isDraggable = false,
   onFilterAdded,
   render,
@@ -319,6 +323,7 @@ const DraggableWrapperComponent: React.FC<Props> = ({
     showTopN,
   } = useHoverActions({
     dataProvider,
+    hideTopN,
     isDraggable,
     onFilterAdded,
     render,
@@ -363,6 +368,7 @@ const DraggableWrapperComponent: React.FC<Props> = ({
   return (
     <DraggableOnWrapperComponent
       dataProvider={dataProvider}
+      hideTopN={hideTopN}
       onFilterAdded={onFilterAdded}
       render={render}
       timelineId={timelineId}

diff --git a/x-pack/plugins/security_solution/public/common/components/drag_and_drop/helpers.test.ts b/x-pack/plugins/security_solution/public/common/components/drag_and_drop/helpers.test.ts
@@ -655,6 +655,7 @@ describe('helpers', () => {
         allowTopN({
           browserField: aggregatableAllowedType,
           fieldName: aggregatableAllowedType.name,
+          hideTopN: false,
         })
       ).toBe(true);
     });
@@ -664,6 +665,7 @@ describe('helpers', () => {
         allowTopN({
           browserField: undefined,
           fieldName: 'signal.rule.name',
+          hideTopN: false,
         })
       ).toBe(true);
     });
@@ -678,6 +680,7 @@ describe('helpers', () => {
         allowTopN({
           browserField: nonAggregatableAllowedType,
           fieldName: nonAggregatableAllowedType.name,
+          hideTopN: false,
         })
       ).toBe(false);
     });
@@ -692,6 +695,7 @@ describe('helpers', () => {
         allowTopN({
           browserField: aggregatableNotAllowedType,
           fieldName: aggregatableNotAllowedType.name,
+          hideTopN: false,
         })
       ).toBe(false);
     });
@@ -703,6 +707,7 @@ describe('helpers', () => {
         allowTopN({
           browserField: missingAggregatable,
           fieldName: missingAggregatable.name,
+          hideTopN: false,
         })
       ).toBe(false);
     });
@@ -714,6 +719,7 @@ describe('helpers', () => {
         allowTopN({
           browserField: missingType,
           fieldName: missingType.name,
+          hideTopN: false,
         })
       ).toBe(false);
     });
@@ -723,6 +729,17 @@ describe('helpers', () => {
         allowTopN({
           browserField: undefined,
           fieldName: 'non-allowlisted',
+          hideTopN: false,
+        })
+      ).toBe(false);
+    });
+
+    test('it returns false when hideTopN is true', () => {
+      expect(
+        allowTopN({
+          browserField: aggregatableAllowedType,
+          fieldName: aggregatableAllowedType.name,
+          hideTopN: true, // <-- the Top N action shall not be shown for this (otherwise valid) field
         })
       ).toBe(false);
     });

diff --git a/x-pack/plugins/security_solution/public/common/components/drag_and_drop/helpers.ts b/x-pack/plugins/security_solution/public/common/components/drag_and_drop/helpers.ts
@@ -92,9 +92,11 @@ export const addProviderToTimeline = ({
 export const allowTopN = ({
   browserField,
   fieldName,
+  hideTopN,
 }: {
   browserField: Partial<BrowserField> | undefined;
   fieldName: string;
+  hideTopN: boolean;
 }): boolean => {
   const isAggregatable = browserField?.aggregatable ?? false;
   const fieldType = browserField?.type ?? '';
@@ -181,5 +183,9 @@ export const allowTopN = ({
     'signal.status',
   ].includes(fieldName);
 
+  if (hideTopN) {
+    return false;
+  }
+
   return isAllowlistedNonBrowserField || (isAggregatable && isAllowedType);
 };
diff --git a/...s/security_solution/public/common/components/draggables/__snapshots__/index.test.tsx.snap b/...s/security_solution/public/common/components/draggables/__snapshots__/index.test.tsx.snap
diff --git a/x-pack/plugins/security_solution/public/common/components/draggables/index.tsx b/x-pack/plugins/security_solution/public/common/components/draggables/index.tsx
@@ -19,6 +19,7 @@ import {
 import { Provider } from '../../../timelines/components/timeline/data_providers/provider';
 
 export interface DefaultDraggableType {
+  hideTopN?: boolean;
   id: string;
   isDraggable?: boolean;
   field: string;
@@ -88,9 +89,11 @@ Content.displayName = 'Content';
  * @param tooltipContent - defaults to displaying `field`, pass `null` to
  * prevent a tooltip from being displayed, or pass arbitrary content
  * @param queryValue - defaults to `value`, this query overrides the `queryMatch.value` used by the `DataProvider` that represents the data
+ * @param hideTopN - defaults to `false`, when true, the option to aggregate this field will be hidden
  */
 export const DefaultDraggable = React.memo<DefaultDraggableType>(
   ({
+    hideTopN = false,
     id,
     isDraggable = true,
     field,
@@ -137,6 +140,7 @@ export const DefaultDraggable = React.memo<DefaultDraggableType>(
     return (
       <DraggableWrapper
         dataProvider={dataProviderProp}
+        hideTopN={hideTopN}
         isDraggable={isDraggable}
         render={renderCallback}
         timelineId={timelineId}

diff --git a/x-pack/plugins/security_solution/public/common/components/hover_actions/index.tsx b/x-pack/plugins/security_solution/public/common/components/hover_actions/index.tsx
@@ -34,9 +34,10 @@ AdditionalContent.displayName = 'AdditionalContent';
 const StyledHoverActionsContainer = styled.div<{
   $showTopN: boolean;
   $showOwnFocus: boolean;
+  $hideTopN: boolean;
   $isActive: boolean;
 }>`
-  min-width: 138px;
+  min-width: ${({ $hideTopN }) => `${$hideTopN ? '112px' : '138px'}`};
   padding: ${(props) => `0 ${props.theme.eui.paddingSizes.s}`};
   display: flex;
 
@@ -91,6 +92,7 @@ interface Props {
   enableOverflowButton?: boolean;
   field: string;
   goGetTimelineId?: (args: boolean) => void;
+  hideTopN?: boolean;
   isObjectArray: boolean;
   onFilterAdded?: () => void;
   ownFocus: boolean;
@@ -129,6 +131,7 @@ export const HoverActions: React.FC<Props> = React.memo(
     field,
     goGetTimelineId,
     isObjectArray,
+    hideTopN = false,
     onFilterAdded,
     ownFocus,
     showOwnFocus = true,
@@ -207,6 +210,7 @@ export const HoverActions: React.FC<Props> = React.memo(
       enableOverflowButton,
       field,
       handleHoverActionClicked,
+      hideTopN,
       isObjectArray,
       isOverflowPopoverOpen,
       onFilterAdded,
@@ -231,6 +235,7 @@ export const HoverActions: React.FC<Props> = React.memo(
           onKeyDown={onKeyDown}
           $showTopN={showTopN}
           $showOwnFocus={showOwnFocus}
+          $hideTopN={hideTopN}
           $isActive={isActive}
           className={isActive ? 'hoverActions-active' : ''}
         >

diff --git a/.../security_solution/public/common/components/hover_actions/use_hover_action_items.test.tsx b/.../security_solution/public/common/components/hover_actions/use_hover_action_items.test.tsx
@@ -22,6 +22,7 @@ describe('useHoverActionItems', () => {
     defaultFocusedButtonRef: null,
     field: 'signal.rule.name',
     handleHoverActionClicked: jest.fn(),
+    hideTopN: false,
     isObjectArray: true,
     ownFocus: false,
     showTopN: false,
@@ -112,4 +113,21 @@ describe('useHoverActionItems', () => {
       );
     });
   });
+
+  test('it should hide the Top N action when hideTopN is true', async () => {
+    await act(async () => {
+      const { result, waitForNextUpdate } = renderHook(() => {
+        const testProps = {
+          ...defaultProps,
+          hideTopN: true, // <-- hide the Top N action
+        };
+        return useHoverActionItems(testProps);
+      });
+      await waitForNextUpdate();
+
+      result.current.allActionItems.forEach((item) => {
+        expect(item.props['data-test-subj']).not.toEqual('hover-actions-show-top-n');
+      });
+    });
+  });
 });
diff --git a/...ugins/security_solution/public/common/components/hover_actions/use_hover_action_items.tsx b/...ugins/security_solution/public/common/components/hover_actions/use_hover_action_items.tsx
@@ -13,7 +13,7 @@ import { isEmpty } from 'lodash';
 
 import { useKibana } from '../../lib/kibana';
 import { getAllFieldsByName } from '../../containers/source';
-import { allowTopN } from './utils';
+import { allowTopN } from '../drag_and_drop/helpers';
 import { useDeepEqualSelector } from '../../hooks/use_selector';
 import { ColumnHeaderOptions, DataProvider, TimelineId } from '../../../../common/types/timeline';
 import { SourcererScopeName } from '../../store/sourcerer/model';
@@ -29,6 +29,7 @@ export interface UseHoverActionItemsProps {
   enableOverflowButton?: boolean;
   field: string;
   handleHoverActionClicked: () => void;
+  hideTopN: boolean;
   isObjectArray: boolean;
   isOverflowPopoverOpen?: boolean;
   itemsToShow?: number;
@@ -56,6 +57,7 @@ export const useHoverActionItems = ({
   enableOverflowButton,
   field,
   handleHoverActionClicked,
+  hideTopN,
   isObjectArray,
   isOverflowPopoverOpen,
   itemsToShow = 2,
@@ -182,6 +184,7 @@ export const useHoverActionItems = ({
         allowTopN({
           browserField: getAllFieldsByName(browserFields)[field],
           fieldName: field,
+          hideTopN,
         }) ? (
           <ShowTopNButton
             Component={enableOverflowButton ? EuiContextMenuItem : undefined}
@@ -229,6 +232,7 @@ export const useHoverActionItems = ({
       getFilterForValueButton,
       getFilterOutValueButton,
       handleHoverActionClicked,
+      hideTopN,
       isObjectArray,
       onFilterAdded,
       ownFocus,

diff --git a/...ck/plugins/security_solution/public/common/components/hover_actions/use_hover_actions.tsx b/...ck/plugins/security_solution/public/common/components/hover_actions/use_hover_actions.tsx
@@ -28,6 +28,7 @@ type RenderFunctionProp = (
 interface Props {
   dataProvider: DataProvider;
   disabled?: boolean;
+  hideTopN: boolean;
   isDraggable?: boolean;
   inline?: boolean;
   render: RenderFunctionProp;
@@ -38,6 +39,7 @@ interface Props {
 
 export const useHoverActions = ({
   dataProvider,
+  hideTopN,
   isDraggable,
   onFilterAdded,
   render,
@@ -101,6 +103,7 @@ export const useHoverActions = ({
         dataProvider={dataProvider}
         draggableId={isDraggable ? getDraggableId(dataProvider.id) : undefined}
         field={dataProvider.queryMatch.field}
+        hideTopN={hideTopN}
         isObjectArray={false}
         goGetTimelineId={setGoGetTimelineId}
         onFilterAdded={onFilterAdded}
@@ -120,6 +123,7 @@ export const useHoverActions = ({
     closeTopN,
     dataProvider,
     handleClosePopOverTrigger,
+    hideTopN,
     hoverActionsOwnFocus,
     isDraggable,
     onFilterAdded,