Skip to content

Commit

Permalink
[Security solution] Guided onboarding unhappy path fixes (#144178)
Browse files Browse the repository at this point in the history
  • Loading branch information
@stephmilovic
stephmilovic authored Nov 1, 2022
1 parent 6f5b716 commit 8969009
Show file tree
Hide file tree
Showing 8 changed files with 276 additions and 40 deletions.
4 changes: 2 additions & 2 deletions ...ins/security_solution/public/common/components/guided_onboarding_tour/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ It was important that the `EuiTourStep` **anchor** is in the DOM when the tour s
```
createCaseFlyout.open({
attachments: caseAttachments,
...(isTourShown(SecurityStepId.alertsCases) && activeStep === 4
...(isTourShown(SecurityStepId.alertsCases) && activeStep === AlertsCasesTourSteps.addAlertToCase
? {
headerContent: (
// isTourAnchor=true no matter what in order to
Expand All @@ -132,7 +132,7 @@ It was important that the `EuiTourStep` **anchor** is in the DOM when the tour s
So we utilize the `useTourContext` to do the following check and increment the step in `handleAddToNewCaseClick`:
```
if (isTourShown(SecurityStepId.alertsCases) && activeStep === 4) {
if (isTourShown(SecurityStepId.alertsCases) && activeStep === AlertsCasesTourSteps.addAlertToCase) {
incrementStep(SecurityStepId.alertsCases);
}
```
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/security_solution/public/common/components/guided_onboarding_tour/tour.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { securityTourConfig, SecurityStepId } from './tour_config';
export interface TourContextValue {
activeStep: number;
endTourStep: (stepId: SecurityStepId) => void;
incrementStep: (stepId: SecurityStepId, step?: number) => void;
incrementStep: (stepId: SecurityStepId) => void;
isTourShown: (stepId: SecurityStepId) => boolean;
}

Expand Down
37 changes: 26 additions & 11 deletions .../plugins/security_solution/public/common/components/guided_onboarding_tour/tour_config.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,15 @@ export const enum SecurityStepId {
alertsCases = 'alertsCases',
}

export const enum AlertsCasesTourSteps {
none = 0,
pointToAlertName = 1,
expandEvent = 2,
reviewAlertDetailsFlyout = 3,
addAlertToCase = 4,
createCase = 5,
}

export type StepConfig = Pick<
EuiTourStepProps,
'step' | 'content' | 'anchorPosition' | 'title' | 'initialFocus' | 'anchor'
Expand All @@ -40,7 +49,7 @@ export const getTourAnchor = (step: number, stepId: SecurityStepId) =>
const alertsCasesConfig: StepConfig[] = [
{
...defaultConfig,
step: 1,
step: AlertsCasesTourSteps.pointToAlertName,
title: i18n.translate('xpack.securitySolution.guided_onboarding.tour.ruleNameStep.tourTitle', {
defaultMessage: 'Test alert for practice',
}),
Expand All @@ -52,12 +61,12 @@ const alertsCasesConfig: StepConfig[] = [
}
),
anchorPosition: 'downCenter',
dataTestSubj: getTourAnchor(1, SecurityStepId.alertsCases),
dataTestSubj: getTourAnchor(AlertsCasesTourSteps.pointToAlertName, SecurityStepId.alertsCases),
initialFocus: `button[tour-step="nextButton"]`,
},
{
...defaultConfig,
step: 2,
step: AlertsCasesTourSteps.expandEvent,
title: i18n.translate('xpack.securitySolution.guided_onboarding.tour.openFlyout.tourTitle', {
defaultMessage: 'Review the alert details',
}),
Expand All @@ -69,12 +78,12 @@ const alertsCasesConfig: StepConfig[] = [
}
),
anchorPosition: 'rightUp',
dataTestSubj: getTourAnchor(2, SecurityStepId.alertsCases),
dataTestSubj: getTourAnchor(AlertsCasesTourSteps.expandEvent, SecurityStepId.alertsCases),
hideNextButton: true,
},
{
...defaultConfig,
step: 3,
step: AlertsCasesTourSteps.reviewAlertDetailsFlyout,
title: i18n.translate(
'xpack.securitySolution.guided_onboarding.tour.flyoutOverview.tourTitle',
{
Expand All @@ -89,26 +98,32 @@ const alertsCasesConfig: StepConfig[] = [
}
),
// needs to use anchor to properly place tour step
anchor: `[tour-step="${getTourAnchor(3, SecurityStepId.alertsCases)}"] .euiTabs`,
anchor: `[tour-step="${getTourAnchor(
AlertsCasesTourSteps.reviewAlertDetailsFlyout,
SecurityStepId.alertsCases
)}"] .euiTabs`,
anchorPosition: 'leftUp',
dataTestSubj: getTourAnchor(3, SecurityStepId.alertsCases),
dataTestSubj: getTourAnchor(
AlertsCasesTourSteps.reviewAlertDetailsFlyout,
SecurityStepId.alertsCases
),
},
{
...defaultConfig,
step: 4,
step: AlertsCasesTourSteps.addAlertToCase,
title: i18n.translate('xpack.securitySolution.guided_onboarding.tour.addToCase.tourTitle', {
defaultMessage: 'Create a case',
}),
content: i18n.translate('xpack.securitySolution.guided_onboarding.tour.addToCase.tourContent', {
defaultMessage: 'From the Take action menu, add the alert to a new case.',
}),
anchorPosition: 'upRight',
dataTestSubj: getTourAnchor(4, SecurityStepId.alertsCases),
dataTestSubj: getTourAnchor(AlertsCasesTourSteps.addAlertToCase, SecurityStepId.alertsCases),
hideNextButton: true,
},
{
...defaultConfig,
step: 5,
step: AlertsCasesTourSteps.createCase,
title: i18n.translate('xpack.securitySolution.guided_onboarding.tour.createCase.tourTitle', {
defaultMessage: `Add details`,
}),
Expand All @@ -120,7 +135,7 @@ const alertsCasesConfig: StepConfig[] = [
),
anchor: `[data-test-subj="create-case-flyout"]`,
anchorPosition: 'leftUp',
dataTestSubj: getTourAnchor(5, SecurityStepId.alertsCases),
dataTestSubj: getTourAnchor(AlertsCasesTourSteps.createCase, SecurityStepId.alertsCases),
hideNextButton: true,
},
];
Expand Down
79 changes: 66 additions & 13 deletions ...gins/security_solution/public/common/components/guided_onboarding_tour/tour_step.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ import { render } from '@testing-library/react';
import { GuidedOnboardingTourStep, SecurityTourStep } from './tour_step';
import { SecurityStepId } from './tour_config';
import { useTourContext } from './tour';
import { mockGlobalState, SUB_PLUGINS_REDUCER, TestProviders } from '../../mock';
import { TimelineId } from '../../../../common/types';
import { createStore } from '../../store';
import { tGridReducer } from '@kbn/timelines-plugin/public';
import { kibanaObservable } from '@kbn/timelines-plugin/public/mock';
import { createSecuritySolutionStorageMock } from '@kbn/timelines-plugin/public/mock/mock_local_storage';

jest.mock('./tour');
const mockTourStep = jest
Expand Down Expand Up @@ -43,7 +49,8 @@ describe('GuidedOnboardingTourStep', () => {
});
it('renders as a tour step', () => {
const { getByTestId } = render(
<GuidedOnboardingTourStep {...defaultProps}>{mockChildren}</GuidedOnboardingTourStep>
<GuidedOnboardingTourStep {...defaultProps}>{mockChildren}</GuidedOnboardingTourStep>,
{ wrapper: TestProviders }
);
const tourStep = getByTestId('tourStepMock');
const header = getByTestId('h1');
Expand All @@ -54,7 +61,8 @@ describe('GuidedOnboardingTourStep', () => {
const { getByTestId, queryByTestId } = render(
<GuidedOnboardingTourStep {...defaultProps} isTourAnchor={false}>
{mockChildren}
</GuidedOnboardingTourStep>
</GuidedOnboardingTourStep>,
{ wrapper: TestProviders }
);
const tourStep = queryByTestId('tourStepMock');
const header = getByTestId('h1');
Expand Down Expand Up @@ -83,7 +91,8 @@ describe('SecurityTourStep', () => {
render(
<SecurityTourStep {...securityTourStepDefaultProps} step={99}>
{mockChildren}
</SecurityTourStep>
</SecurityTourStep>,
{ wrapper: TestProviders }
);
expect(mockTourStep).not.toHaveBeenCalled();
});
Expand All @@ -92,7 +101,8 @@ describe('SecurityTourStep', () => {
render(
<SecurityTourStep {...securityTourStepDefaultProps} step={4}>
{mockChildren}
</SecurityTourStep>
</SecurityTourStep>,
{ wrapper: TestProviders }
);
expect(mockTourStep).not.toHaveBeenCalled();
});
Expand All @@ -103,12 +113,16 @@ describe('SecurityTourStep', () => {
incrementStep: jest.fn(),
isTourShown: () => false,
});
render(<SecurityTourStep {...securityTourStepDefaultProps}>{mockChildren}</SecurityTourStep>);
render(<SecurityTourStep {...securityTourStepDefaultProps}>{mockChildren}</SecurityTourStep>, {
wrapper: TestProviders,
});
expect(mockTourStep).not.toHaveBeenCalled();
});

it('renders tour step with correct number of steppers', () => {
render(<SecurityTourStep {...securityTourStepDefaultProps}>{mockChildren}</SecurityTourStep>);
render(<SecurityTourStep {...securityTourStepDefaultProps}>{mockChildren}</SecurityTourStep>, {
wrapper: TestProviders,
});
const mockCall = { ...mockTourStep.mock.calls[0][0] };
expect(mockCall.step).toEqual(1);
expect(mockCall.stepsTotal).toEqual(5);
Expand All @@ -118,7 +132,8 @@ describe('SecurityTourStep', () => {
render(
<SecurityTourStep {...securityTourStepDefaultProps} step={5}>
{mockChildren}
</SecurityTourStep>
</SecurityTourStep>,
{ wrapper: TestProviders }
);
const mockCall = { ...mockTourStep.mock.calls[0][0] };
expect(mockCall.step).toEqual(5);
Expand All @@ -134,7 +149,8 @@ describe('SecurityTourStep', () => {
render(
<SecurityTourStep {...securityTourStepDefaultProps} step={3}>
{mockChildren}
</SecurityTourStep>
</SecurityTourStep>,
{ wrapper: TestProviders }
);
const mockCall = { ...mockTourStep.mock.calls[0][0] };
expect(mockCall.footerAction).toMatchInlineSnapshot(`
Expand Down Expand Up @@ -163,7 +179,8 @@ describe('SecurityTourStep', () => {
const { container } = render(
<SecurityTourStep {...securityTourStepDefaultProps} step={3}>
{mockChildren}
</SecurityTourStep>
</SecurityTourStep>,
{ wrapper: TestProviders }
);
const selectParent = container.querySelector(
`[data-test-subj="tourStepMock"] [data-test-subj="h1"]`
Expand All @@ -184,7 +201,8 @@ describe('SecurityTourStep', () => {
const { container } = render(
<SecurityTourStep {...securityTourStepDefaultProps} step={2}>
{mockChildren}
</SecurityTourStep>
</SecurityTourStep>,
{ wrapper: TestProviders }
);
const selectParent = container.querySelector(
`[data-test-subj="tourStepMock"] [data-test-subj="h1"]`
Expand All @@ -197,13 +215,17 @@ describe('SecurityTourStep', () => {
});

it('if a tour step does not have children and has anchor, only render tour step', () => {
const { getByTestId } = render(<SecurityTourStep {...securityTourStepDefaultProps} step={5} />);
const { getByTestId } = render(
<SecurityTourStep {...securityTourStepDefaultProps} step={5} />,
{ wrapper: TestProviders }
);
expect(getByTestId('tourStepMock')).toBeInTheDocument();
});

it('if a tour step does not have children and does not have anchor, render nothing', () => {
const { queryByTestId } = render(
<SecurityTourStep {...securityTourStepDefaultProps} step={1} />
<SecurityTourStep {...securityTourStepDefaultProps} step={1} />,
{ wrapper: TestProviders }
);
expect(queryByTestId('tourStepMock')).not.toBeInTheDocument();
});
Expand All @@ -217,9 +239,40 @@ describe('SecurityTourStep', () => {
render(
<SecurityTourStep {...securityTourStepDefaultProps} step={4}>
{mockChildren}
</SecurityTourStep>
</SecurityTourStep>,
{ wrapper: TestProviders }
);
const mockCall = { ...mockTourStep.mock.calls[0][0] };
expect(mockCall.footerAction).toMatchInlineSnapshot(`<React.Fragment />`);
});

it('does not render step if timeline is open', () => {
const mockstate = {
...mockGlobalState,
timeline: {
...mockGlobalState.timeline,
timelineById: {
[TimelineId.active]: {
...mockGlobalState.timeline.timelineById.test,
show: true,
},
},
},
};
const { storage } = createSecuritySolutionStorageMock();
const mockStore = createStore(
mockstate,
SUB_PLUGINS_REDUCER,
{ dataTable: tGridReducer },
kibanaObservable,
storage
);

render(
<TestProviders store={mockStore}>
<SecurityTourStep {...securityTourStepDefaultProps}>{mockChildren}</SecurityTourStep>
</TestProviders>
);
expect(mockTourStep).not.toHaveBeenCalled();
});
});
42 changes: 36 additions & 6 deletions ...k/plugins/security_solution/public/common/components/guided_onboarding_tour/tour_step.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,26 +11,54 @@ import type { EuiTourStepProps } from '@elastic/eui';
import { EuiButton, EuiImage, EuiSpacer, EuiText, EuiTourStep } from '@elastic/eui';
import { FormattedMessage } from '@kbn/i18n-react';

import styled from 'styled-components';
import { useShallowEqualSelector } from '../../hooks/use_selector';
import { TimelineId } from '../../../../common/types';
import { timelineDefaults } from '../../../timelines/store/timeline/defaults';
import { timelineSelectors } from '../../../timelines/store/timeline';
import { useTourContext } from './tour';
import { securityTourConfig, SecurityStepId } from './tour_config';
import { AlertsCasesTourSteps, SecurityStepId, securityTourConfig } from './tour_config';

interface SecurityTourStep {
children?: React.ReactElement;
step: number;
stepId: SecurityStepId;
}

const isStepExternallyMounted = (stepId: SecurityStepId, step: number) =>
step === AlertsCasesTourSteps.createCase && stepId === SecurityStepId.alertsCases;

const StyledTourStep = styled(EuiTourStep)<EuiTourStepProps & { stepId: SecurityStepId }>`
&.euiPopover__panel[data-popover-open] {
z-index: ${({ step, stepId }) =>
isStepExternallyMounted(stepId, step) ? '9000 !important' : '1000 !important'};
}
`;

export const SecurityTourStep = ({ children, step, stepId }: SecurityTourStep) => {
const { activeStep, incrementStep, isTourShown } = useTourContext();
const tourStep = useMemo(
() => securityTourConfig[stepId].find((config) => config.step === step),
[step, stepId]
);

const getTimeline = useMemo(() => timelineSelectors.getTimelineByIdSelector(), []);
const showTimeline = useShallowEqualSelector(
(state) => (getTimeline(state, TimelineId.active) ?? timelineDefaults).show
);

const onClick = useCallback(() => incrementStep(stepId), [incrementStep, stepId]);
// step === 5 && stepId === SecurityStepId.alertsCases is in Cases app and out of context.

// step === AlertsCasesTourSteps.createCase && stepId === SecurityStepId.alertsCases is in Cases app and out of context.
// If we mount this step, we know we need to render it
// we are also managing the context on the siem end in the background
const overrideContext = step === 5 && stepId === SecurityStepId.alertsCases;
if (tourStep == null || ((step !== activeStep || !isTourShown(stepId)) && !overrideContext)) {
const overrideContext = isStepExternallyMounted(stepId, step);

if (
tourStep == null ||
((step !== activeStep || !isTourShown(stepId)) && !overrideContext) ||
showTimeline
) {
return children ? children : null;
}

Expand Down Expand Up @@ -89,11 +117,13 @@ export const SecurityTourStep = ({ children, step, stepId }: SecurityTourStep) =
// see type EuiTourStepAnchorProps
return anchor != null ? (
<>
<EuiTourStep {...commonProps} anchor={anchor} />
<StyledTourStep stepId={stepId} {...commonProps} anchor={anchor} />
<>{children}</>
</>
) : children != null ? (
<EuiTourStep {...commonProps}>{children}</EuiTourStep>
<StyledTourStep stepId={stepId} {...commonProps}>
{children}
</StyledTourStep>
) : null;
};

Expand Down
Loading

0 comments on commit 8969009

Please sign in to comment.