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.

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

[SECURITY] Bug fix for topN on draggables #70450

Merged
merged 14 commits into from
Jul 3, 2020
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ interface AlertsHistogramPanelProps {
showLinkToAlerts?: boolean;
showTotalAlertsCount?: boolean;
stackByOptions?: AlertsHistogramOption[];
timelineId?: string;
title?: string;
to: number;
updateDateRange: UpdateDateRange;
Expand Down Expand Up @@ -98,8 +99,9 @@ export const AlertsHistogramPanel = memo<AlertsHistogramPanelProps>(
showLinkToAlerts = false,
showTotalAlertsCount = false,
stackByOptions,
to,
timelineId,
title = i18n.HISTOGRAM_HEADER,
to,
updateDateRange,
}) => {
// create a unique, but stable (across re-renders) query id
Expand Down Expand Up @@ -163,11 +165,12 @@ export const AlertsHistogramPanel = memo<AlertsHistogramPanelProps>(
`draggable-legend-item-${uuid.v4()}-${selectedStackByOption.value}-${bucket.key}`
),
field: selectedStackByOption.value,
timelineId,
value: bucket.key,
}))
: NO_LEGEND_DATA,
// eslint-disable-next-line react-hooks/exhaustive-deps
[alertsData, selectedStackByOption.value]
[alertsData, selectedStackByOption.value, timelineId]
);

useEffect(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ interface BarChartComponentProps {
barChart: ChartSeriesData[] | null | undefined;
configs?: ChartSeriesConfigs | undefined;
stackByField?: string;
timelineId?: string;
}

const NO_LEGEND_DATA: LegendItem[] = [];
Expand All @@ -125,6 +126,7 @@ export const BarChartComponent: React.FC<BarChartComponentProps> = ({
barChart,
configs,
stackByField,
timelineId,
}) => {
const { ref: measureRef, width, height } = useThrottledResizeObserver();
const legendItems: LegendItem[] = useMemo(
Expand All @@ -135,11 +137,12 @@ export const BarChartComponent: React.FC<BarChartComponentProps> = ({
dataProviderId: escapeDataProviderId(
`draggable-legend-item-${uuid.v4()}-${stackByField}-${d.key}`
),
timelineId,
field: stackByField,
value: d.key,
}))
: NO_LEGEND_DATA,
[barChart, stackByField]
[barChart, stackByField, timelineId]
);

const customHeight = get('customHeight', configs);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,14 @@ export interface LegendItem {
color?: string;
dataProviderId: string;
field: string;
timelineId?: string;
value: string;
}

const DraggableLegendItemComponent: React.FC<{
legendItem: LegendItem;
}> = ({ legendItem }) => {
const { color, dataProviderId, field, value } = legendItem;
const { color, dataProviderId, field, timelineId, value } = legendItem;

return (
<EuiText size="xs">
Expand All @@ -44,6 +45,7 @@ const DraggableLegendItemComponent: React.FC<{
data-test-subj={`legend-item-${dataProviderId}`}
field={field}
id={dataProviderId}
timelineId={timelineId}
value={value}
/>
) : (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,10 @@ import { IdToDataProvider } from '../../store/drag_and_drop/model';
import { State } from '../../store/types';
import { DataProvider } from '../../../timelines/components/timeline/data_providers/data_provider';
import { reArrangeProviders } from '../../../timelines/components/timeline/data_providers/helpers';
import { ACTIVE_TIMELINE_REDUX_ID } from '../top_n';
import { ADDED_TO_TIMELINE_MESSAGE } from '../../hooks/translations';
import { useAddToTimelineSensor } from '../../hooks/use_add_to_timeline';
import { displaySuccessToast, useStateToaster } from '../toasters';

import { TimelineId } from '../../../../common/types/timeline';
import {
addFieldToTimelineColumns,
addProviderToTimeline,
Expand All @@ -35,7 +34,7 @@ import {
userIsReArrangingProviders,
} from './helpers';

// @ts-ignore
// @ts-expect-error
window['__react-beautiful-dnd-disable-dev-warnings'] = true;

interface Props {
Expand Down Expand Up @@ -67,7 +66,7 @@ const onDragEndHandler = ({
destination: result.destination,
dispatch,
source: result.source,
timelineId: ACTIVE_TIMELINE_REDUX_ID,
timelineId: TimelineId.active,
});
} else if (providerWasDroppedOnTimeline(result)) {
addProviderToTimeline({
Expand All @@ -76,7 +75,7 @@ const onDragEndHandler = ({
dispatch,
onAddedToTimeline,
result,
timelineId: ACTIVE_TIMELINE_REDUX_ID,
timelineId: TimelineId.active,
});
} else if (fieldWasDroppedOnTimelineColumns(result)) {
addFieldToTimelineColumns({
Expand Down Expand Up @@ -130,7 +129,6 @@ export const DragDropContextWrapperComponent = React.memo<Props & PropsFromRedux
[dataProviders, activeTimelineDataProviders, browserFields]
);
return (
// @ts-ignore
<DragDropContext onDragEnd={onDragEnd} onBeforeCapture={onBeforeCapture} sensors={sensors}>
{children}
</DragDropContext>
Expand All @@ -152,7 +150,7 @@ const emptyActiveTimelineDataProviders: DataProvider[] = []; // stable reference

const mapStateToProps = (state: State) => {
const activeTimelineDataProviders =
timelineSelectors.getTimelineByIdSelector()(state, ACTIVE_TIMELINE_REDUX_ID)?.dataProviders ??
timelineSelectors.getTimelineByIdSelector()(state, TimelineId.active)?.dataProviders ??
emptyActiveTimelineDataProviders;
const dataProviders = dragAndDropSelectors.dataProvidersSelector(state) ?? emptyDataProviders;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import React, { useCallback, useEffect, useMemo, useState } from 'react';
import React, { useCallback, useEffect, useMemo, useState, useRef } from 'react';
import {
Draggable,
DraggableProvided,
Expand All @@ -22,7 +22,7 @@ import { DataProvider } from '../../../timelines/components/timeline/data_provid
import { TruncatableText } from '../truncatable_text';
import { WithHoverActions } from '../with_hover_actions';

import { DraggableWrapperHoverContent } from './draggable_wrapper_hover_content';
import { DraggableWrapperHoverContent, useGetTimelineId } from './draggable_wrapper_hover_content';
import { getDraggableId, getDroppableId } from './helpers';
import { ProviderContainer } from './provider_container';

Expand Down Expand Up @@ -76,6 +76,7 @@ interface Props {
dataProvider: DataProvider;
inline?: boolean;
render: RenderFunctionProp;
timelineId?: string;
truncate?: boolean;
onFilterAdded?: () => void;
}
Expand All @@ -100,16 +101,31 @@ export const getStyle = (
};

export const DraggableWrapper = React.memo<Props>(
({ dataProvider, onFilterAdded, render, truncate }) => {
({ dataProvider, onFilterAdded, render, timelineId, truncate }) => {
const draggableRef = useRef<HTMLDivElement | null>(null);
const [closePopOverTrigger, setClosePopOverTrigger] = useState(false);
const [showTopN, setShowTopN] = useState<boolean>(false);
const toggleTopN = useCallback(() => {
setShowTopN(!showTopN);
}, [setShowTopN, showTopN]);

const [goGetTimelineId, setGoGetTimelineId] = useState(false);
const timelineIdFind = useGetTimelineId(draggableRef, goGetTimelineId);
const [providerRegistered, setProviderRegistered] = useState(false);

const dispatch = useDispatch();

const handleClosePopOverTrigger = useCallback(
() => setClosePopOverTrigger((prevClosePopOverTrigger) => !prevClosePopOverTrigger),
[]
);

const toggleTopN = useCallback(() => {
setShowTopN((prevShowTopN) => {
const newShowTopN = !prevShowTopN;
if (newShowTopN === false) {
handleClosePopOverTrigger();
}
return newShowTopN;
});
}, [handleClosePopOverTrigger]);

const registerProvider = useCallback(() => {
if (!providerRegistered) {
dispatch(dragAndDropActions.registerProvider({ provider: dataProvider }));
Expand All @@ -126,17 +142,19 @@ export const DraggableWrapper = React.memo<Props>(
() => () => {
unRegisterProvider();
},
// eslint-disable-next-line react-hooks/exhaustive-deps
[]
[unRegisterProvider]
);

const hoverContent = useMemo(
() => (
<DraggableWrapperHoverContent
closePopOver={handleClosePopOverTrigger}
draggableId={getDraggableId(dataProvider.id)}
field={dataProvider.queryMatch.field}
goGetTimelineId={setGoGetTimelineId}
onFilterAdded={onFilterAdded}
showTopN={showTopN}
timelineId={timelineId ?? timelineIdFind}
toggleTopN={toggleTopN}
value={
typeof dataProvider.queryMatch.value !== 'number'
Expand All @@ -145,7 +163,15 @@ export const DraggableWrapper = React.memo<Props>(
}
/>
),
[dataProvider, onFilterAdded, showTopN, toggleTopN]
[
dataProvider,
handleClosePopOverTrigger,
onFilterAdded,
showTopN,
timelineId,
timelineIdFind,
toggleTopN,
]
);

const renderContent = useCallback(
Expand Down Expand Up @@ -184,7 +210,10 @@ export const DraggableWrapper = React.memo<Props>(
<ProviderContainer
{...provided.draggableProps}
{...provided.dragHandleProps}
ref={provided.innerRef}
ref={(e: HTMLDivElement) => {
provided.innerRef(e);
draggableRef.current = e;
}}
data-test-subj="providerContainer"
isDragging={snapshot.isDragging}
registerProvider={registerProvider}
Expand Down Expand Up @@ -214,7 +243,12 @@ export const DraggableWrapper = React.memo<Props>(
);

return (
<WithHoverActions alwaysShow={showTopN} hoverContent={hoverContent} render={renderContent} />
<WithHoverActions
alwaysShow={showTopN}
closePopOverTrigger={closePopOverTrigger}
hoverContent={hoverContent}
render={renderContent}
/>
);
},
(prevProps, nextProps) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ jest.mock('../../../timelines/components/manage_timeline', () => {
return {
...original,
useManageTimeline: () => ({
getManageTimelineById: jest.fn().mockReturnValue({ indexToAdd: [] }),
getTimelineFilterManager: mockGetTimelineFilterManager,
isManagedTimeline: jest.fn().mockReturnValue(false),
}),
Expand All @@ -63,8 +64,10 @@ const timelineId = TimelineId.active;
const field = 'process.name';
const value = 'nice';
const toggleTopN = jest.fn();
const goGetTimelineId = jest.fn();
const defaultProps = {
field,
goGetTimelineId,
showTopN: false,
timelineId,
toggleTopN,
Expand Down Expand Up @@ -130,6 +133,18 @@ describe('DraggableWrapperHoverContent', () => {
wrapper.find(`[data-test-subj="filter-${hoverAction}-value"]`).first().exists()
).toBe(false);
});

test(`it should call goGetTimelineId when user is over the 'Filter ${hoverAction} value' button`, () => {
const wrapper = mount(
<TestProviders>
<DraggableWrapperHoverContent {...{ ...defaultProps, timelineId: undefined }} />
</TestProviders>
);
const button = wrapper.find(`[data-test-subj="filter-${hoverAction}-value"]`).first();
button.simulate('mouseenter');
expect(goGetTimelineId).toHaveBeenCalledWith(true);
});

describe('when run in the context of a timeline', () => {
let wrapper: ReactWrapper;
let onFilterAdded: () => void;
Expand All @@ -151,6 +166,7 @@ describe('DraggableWrapperHoverContent', () => {
</TestProviders>
);
});

test('when clicked, it adds a filter to the timeline when running in the context of a timeline', () => {
wrapper.find(`[data-test-subj="filter-${hoverAction}-value"]`).first().simulate('click');
wrapper.update();
Expand Down Expand Up @@ -459,6 +475,24 @@ describe('DraggableWrapperHoverContent', () => {
expect(wrapper.find('[data-test-subj="show-top-field"]').first().exists()).toBe(false);
});

test(`it should invokes goGetTimelineId when user is over the 'Show top field' button`, () => {
const whitelistedField = 'signal.rule.name';
const wrapper = mount(
<TestProviders>
<DraggableWrapperHoverContent
{...{
...defaultProps,
field: whitelistedField,
timelineId: undefined,
}}
/>
</TestProviders>
);
const button = wrapper.find(`[data-test-subj="show-top-field"]`).first();
button.simulate('mouseenter');
expect(goGetTimelineId).toHaveBeenCalledWith(true);
});

test(`invokes the toggleTopN function when the 'Show top field' button is clicked`, async () => {
const whitelistedField = 'signal.rule.name';
const wrapper = mount(
Expand Down
Loading