[Security solution] Guided onboarding unhappy path fixes

x-pack/plugins/security_solution/public/common/components/guided_onboarding_tour/README.md

@@ -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
@@ -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);
   }
   ```

x-pack/plugins/security_solution/public/common/components/guided_onboarding_tour/tour.tsx

@@ -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;
 }
 

x-pack/plugins/security_solution/public/common/components/guided_onboarding_tour/tour_config.ts

@@ -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'
@@ -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',
     }),
@@ -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',
     }),
@@ -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',
       {
@@ -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`,
     }),
@@ -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,
   },
 ];

x-pack/plugins/security_solution/public/common/components/guided_onboarding_tour/tour_step.test.tsx

@@ -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
@@ -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');
@@ -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');
@@ -83,7 +91,8 @@ describe('SecurityTourStep', () => {
     render(
       <SecurityTourStep {...securityTourStepDefaultProps} step={99}>
         {mockChildren}
-      </SecurityTourStep>
+      </SecurityTourStep>,
+      { wrapper: TestProviders }
     );
     expect(mockTourStep).not.toHaveBeenCalled();
   });
@@ -92,7 +101,8 @@ describe('SecurityTourStep', () => {
     render(
       <SecurityTourStep {...securityTourStepDefaultProps} step={4}>
         {mockChildren}
-      </SecurityTourStep>
+      </SecurityTourStep>,
+      { wrapper: TestProviders }
     );
     expect(mockTourStep).not.toHaveBeenCalled();
   });
@@ -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);
@@ -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);
@@ -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(`
@@ -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"]`
@@ -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"]`
@@ -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();
   });
@@ -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();
+  });
 });

x-pack/plugins/security_solution/public/common/components/guided_onboarding_tour/tour_step.tsx

@@ -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;
   }
 
@@ -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;
 };