[Actionable Observability] - Hide the alert details page behind a fea…

…ture flag (#139806) * WIP - feature flag alert details page * Add tests * Add comment for upcoming tests * Fix tests * fix test and add mocks for usePluginContext * Fix failing test * Fix wording * Fix test * Update readme and kibana-docker * Fix tests permissions * Restore tests * Fix flaky test * Fix flaky * Fix flaky * wait to display the button * Fix flaky * Fix flaky * Add 404 page and skip the flaky test * re-enable tests * Put message for the flaky test * Update comment
elastic · Sep 6, 2022 · d11ee88 · d11ee88
1 parent 02c17cc
commit d11ee88
Show file tree

Hide file tree

Showing 21 changed files with 235 additions and 35 deletions.
diff --git a/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker b/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker
@@ -295,6 +295,7 @@ kibana_vars=(
     xpack.ingestManager.registryUrl
     xpack.observability.annotations.index
     xpack.observability.unsafe.slo.enabled
+    xpack.observability.unsafe.alertDetails.enabled
     xpack.reporting.capture.browser.autoDownload
     xpack.reporting.capture.browser.chromium.disableSandbox
     xpack.reporting.capture.browser.chromium.inspect

diff --git a/test/plugin_functional/test_suites/core_plugins/rendering.ts b/test/plugin_functional/test_suites/core_plugins/rendering.ts
@@ -221,6 +221,8 @@ export default function ({ getService }: PluginFunctionalProviderContext) {
         'xpack.trigger_actions_ui.enableGeoTrackingThresholdAlert (boolean)',
         'xpack.upgrade_assistant.readonly (boolean)',
         'xpack.upgrade_assistant.ui.enabled (boolean)',
+        'xpack.observability.unsafe.alertDetails.enabled (boolean)',
+        'xpack.observability.unsafe.slo.enabled (boolean)',
       ];
       // We don't assert that actualExposedConfigKeys and expectedExposedConfigKeys are equal, because test failure messages with large
       // arrays are hard to grok. Instead, we take the difference between the two arrays and assert them separately, that way it's

diff --git a/x-pack/README.md b/x-pack/README.md
@@ -4,6 +4,17 @@ This directory tree contains files subject to the Elastic License 2.0. The files
 to the Elastic License 2.0 are grouped in this directory to clearly separate them
 from files dual-licensed under the Server Side Public License and the Elastic License 2.0.
 
+## Alert Details page (feature flag)
+
+If you have:
+
+```yaml
+xpack.observability.unsafe.alertDetails.enabled: true
+```
+In Kibana configuration, will allow the user to navigate to the new Alert Details page, instead of the Alert Flyout when clicking on `View alert details` in the Alert table
+
+
+
 # Development
 
 By default, Kibana will run with X-Pack installed as mentioned in the [contributing guide](../CONTRIBUTING.md).

diff --git a/x-pack/plugins/observability/public/application/application.test.tsx b/x-pack/plugins/observability/public/application/application.test.tsx
@@ -12,7 +12,7 @@ import { Observable } from 'rxjs';
 import { AppMountParameters, CoreStart } from '@kbn/core/public';
 import { themeServiceMock } from '@kbn/core/public/mocks';
 import { KibanaPageTemplate } from '@kbn/shared-ux-page-kibana-template';
-import { ObservabilityPublicPluginsStart } from '../plugin';
+import { ConfigSchema, ObservabilityPublicPluginsStart } from '../plugin';
 import { createObservabilityRuleTypeRegistryMock } from '../rules/observability_rule_type_registry_mock';
 import { renderApp } from '.';
 
@@ -66,9 +66,16 @@ describe('renderApp', () => {
       theme$: themeServiceMock.createTheme$(),
     } as unknown as AppMountParameters;
 
+    const config = {
+      unsafe: {
+        alertDetails: { enabled: false },
+      },
+    } as ConfigSchema;
+
     expect(() => {
       const unmount = renderApp({
         core,
+        config,
         plugins,
         appMountParameters: params,
         observabilityRuleTypeRegistry: createObservabilityRuleTypeRegistryMock(),

diff --git a/x-pack/plugins/observability/public/application/index.tsx b/x-pack/plugins/observability/public/application/index.tsx
@@ -23,7 +23,7 @@ import { DatePickerContextProvider } from '../context/date_picker_context';
 import { HasDataContextProvider } from '../context/has_data_context';
 import { PluginContext } from '../context/plugin_context';
 import { useRouteParams } from '../hooks/use_route_params';
-import { ObservabilityPublicPluginsStart } from '../plugin';
+import { ConfigSchema, ObservabilityPublicPluginsStart } from '../plugin';
 import { routes } from '../routes';
 import { ObservabilityRuleTypeRegistry } from '../rules/create_observability_rule_type_registry';
 
@@ -47,6 +47,7 @@ function App() {
 
 export const renderApp = ({
   core,
+  config,
   plugins,
   appMountParameters,
   observabilityRuleTypeRegistry,
@@ -55,6 +56,7 @@ export const renderApp = ({
   isDev,
 }: {
   core: CoreStart;
+  config: ConfigSchema;
   plugins: ObservabilityPublicPluginsStart;
   observabilityRuleTypeRegistry: ObservabilityRuleTypeRegistry;
   appMountParameters: AppMountParameters;
@@ -86,6 +88,7 @@ export const renderApp = ({
         >
           <PluginContext.Provider
             value={{
+              config,
               appMountParameters,
               observabilityRuleTypeRegistry,
               ObservabilityPageTemplate,

diff --git a/x-pack/plugins/observability/public/components/app/section/apm/index.test.tsx b/x-pack/plugins/observability/public/components/app/section/apm/index.test.tsx
@@ -9,7 +9,7 @@ import React from 'react';
 import * as fetcherHook from '../../../../hooks/use_fetcher';
 import { render, data as dataMock } from '../../../../utils/test_helper';
 import { CoreStart } from '@kbn/core/public';
-import { ObservabilityPublicPluginsStart } from '../../../../plugin';
+import { ConfigSchema, ObservabilityPublicPluginsStart } from '../../../../plugin';
 import { APMSection } from '.';
 import { response } from './mock_data/apm.mock';
 import * as hasDataHook from '../../../../hooks/use_has_data';
@@ -43,10 +43,16 @@ describe('APMSection', () => {
       from: '2020-10-08T06:00:00.000Z',
       to: '2020-10-08T07:00:00.000Z',
     });
+    const config = {
+      unsafe: {
+        alertDetails: { enabled: false },
+      },
+    } as ConfigSchema;
 
     jest.spyOn(pluginContext, 'usePluginContext').mockImplementation(() => ({
       appMountParameters: {} as AppMountParameters,
       core: {} as CoreStart,
+      config,
       plugins: {} as ObservabilityPublicPluginsStart,
       observabilityRuleTypeRegistry: createObservabilityRuleTypeRegistryMock(),
       ObservabilityPageTemplate: KibanaPageTemplate,

diff --git a/x-pack/plugins/observability/public/context/plugin_context.tsx b/x-pack/plugins/observability/public/context/plugin_context.tsx
@@ -9,8 +9,10 @@ import { AppMountParameters } from '@kbn/core/public';
 import { createContext } from 'react';
 import { ObservabilityRuleTypeRegistry } from '../rules/create_observability_rule_type_registry';
 import type { LazyObservabilityPageTemplateProps } from '../components/shared/page_template/lazy_page_template';
+import { ConfigSchema } from '../plugin';
 
 export interface PluginContextValue {
+  config: ConfigSchema;
   appMountParameters: AppMountParameters;
   observabilityRuleTypeRegistry: ObservabilityRuleTypeRegistry;
   ObservabilityPageTemplate: React.ComponentType<LazyObservabilityPageTemplateProps>;

diff --git a/x-pack/plugins/observability/public/pages/404.tsx b/x-pack/plugins/observability/public/pages/404.tsx
@@ -0,0 +1,41 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+import React from 'react';
+import { FormattedMessage } from '@kbn/i18n-react';
+import { EuiCallOut } from '@elastic/eui';
+import { usePluginContext } from '../hooks/use_plugin_context';
+
+function PageNotFound() {
+  const { ObservabilityPageTemplate } = usePluginContext();
+
+  return (
+    <ObservabilityPageTemplate data-test-subj="pageNotFound">
+      <EuiCallOut
+        color="warning"
+        iconType="iInCircle"
+        title={
+          <FormattedMessage
+            id="xpack.observability.notFoundPage.title"
+            defaultMessage="Page Not Found"
+          />
+        }
+        data-test-subj={'observabilityPageNotFoundBanner'}
+      >
+        <p data-test-subj={'observabilityPageNotFoundBannerText'}>
+          <FormattedMessage
+            id="xpack.observability.notFoundPage.bannerText"
+            defaultMessage="The Observability application doesn't recognize this route"
+          />
+        </p>
+      </EuiCallOut>
+    </ObservabilityPageTemplate>
+  );
+}
+
+// eslint-disable-next-line import/no-default-export
+export default PageNotFound;
diff --git a/x-pack/plugins/observability/public/pages/alert_details/index.tsx b/x-pack/plugins/observability/public/pages/alert_details/index.tsx
@@ -13,14 +13,15 @@ import { ObservabilityAppServices } from '../../application/types';
 import { usePluginContext } from '../../hooks/use_plugin_context';
 import { useBreadcrumbs } from '../../hooks/use_breadcrumbs';
 import { paths } from '../../config/paths';
+import PageNotFound from '../404';
 
 // import { useParams } from 'react-router';
 // import { AlertDetailsPathParams } from './types';
 
 export function AlertDetailsPage() {
   const { http } = useKibana<ObservabilityAppServices>().services;
 
-  const { ObservabilityPageTemplate } = usePluginContext();
+  const { ObservabilityPageTemplate, config } = usePluginContext();
   // const { alertId } = useParams<AlertDetailsPathParams>();
   const alert = {};
 
@@ -33,6 +34,11 @@ export function AlertDetailsPage() {
     },
   ]);
 
+  // Redirect to the the 404 page when the user hit the page url directly in the browser while the feature flag is off.
+  if (!config.unsafe.alertDetails.enabled) {
+    return <PageNotFound />;
+  }
+
   return (
     <ObservabilityPageTemplate data-test-subj="alertDetails">
       <AlertSummary alert={alert} />

diff --git a/x-pack/plugins/observability/public/pages/alerts/components/observability_actions.test.tsx b/x-pack/plugins/observability/public/pages/alerts/components/observability_actions.test.tsx
@@ -13,6 +13,10 @@ import { inventoryThresholdAlert } from '../../../rules/fixtures/example_alerts'
 import { RULE_DETAILS_PAGE_ID } from '../../rule_details/types';
 import { createObservabilityRuleTypeRegistryMock } from '../../../rules/observability_rule_type_registry_mock';
 import { TimelineNonEcsData } from '@kbn/timelines-plugin/common';
+import * as pluginContext from '../../../hooks/use_plugin_context';
+import { ConfigSchema, ObservabilityPublicPluginsStart } from '../../../plugin';
+import { AppMountParameters, CoreStart } from '@kbn/core/public';
+import { KibanaPageTemplate } from '@kbn/shared-ux-page-kibana-template';
 
 const mockUseKibanaReturnValue = kibanaStartMock.startContract();
 
@@ -25,6 +29,21 @@ jest.mock('../../../hooks/use_get_user_cases_permissions', () => ({
   useGetUserCasesPermissions: jest.fn(() => ({})),
 }));
 
+const config = {
+  unsafe: {
+    alertDetails: { enabled: false },
+  },
+} as ConfigSchema;
+
+jest.spyOn(pluginContext, 'usePluginContext').mockImplementation(() => ({
+  appMountParameters: {} as AppMountParameters,
+  core: {} as CoreStart,
+  config,
+  plugins: {} as ObservabilityPublicPluginsStart,
+  observabilityRuleTypeRegistry: createObservabilityRuleTypeRegistryMock(),
+  ObservabilityPageTemplate: KibanaPageTemplate,
+}));
+
 describe('ObservabilityActions component', () => {
   const setup = async (pageId: string) => {
     const props: ObservabilityActionsProps = {
@@ -53,14 +72,14 @@ describe('ObservabilityActions component', () => {
     const wrapper = await setup(RULE_DETAILS_PAGE_ID);
     wrapper.find('[data-test-subj="alertsTableRowActionMore"]').hostNodes().simulate('click');
     expect(wrapper.find('[data-test-subj~="viewRuleDetails"]').hostNodes().length).toBe(0);
-    expect(wrapper.find('[data-test-subj~="viewAlertDetails"]').hostNodes().length).toBe(1);
+    expect(wrapper.find('[data-test-subj~="viewAlertDetailsFlyout"]').hostNodes().length).toBe(1);
   });
 
   it('should show "View rule details" menu item', async () => {
     const wrapper = await setup('nothing');
     wrapper.find('[data-test-subj="alertsTableRowActionMore"]').hostNodes().simulate('click');
     expect(wrapper.find('[data-test-subj~="viewRuleDetails"]').hostNodes().length).toBe(1);
-    expect(wrapper.find('[data-test-subj~="viewAlertDetails"]').hostNodes().length).toBe(1);
+    expect(wrapper.find('[data-test-subj~="viewAlertDetailsFlyout"]').hostNodes().length).toBe(1);
   });
 
   it('should create a valid link for rule details page', async () => {

diff --git a/x-pack/plugins/observability/public/pages/alerts/components/observability_actions.tsx b/x-pack/plugins/observability/public/pages/alerts/components/observability_actions.tsx
@@ -19,6 +19,7 @@ import React, { useMemo, useState, useCallback } from 'react';
 import { CaseAttachmentsWithoutOwner } from '@kbn/cases-plugin/public';
 import { CommentType } from '@kbn/cases-plugin/common';
 import type { ActionProps } from '@kbn/timelines-plugin/common';
+import { usePluginContext } from '../../../hooks/use_plugin_context';
 import { useKibana } from '../../../utils/kibana_react';
 import { useGetUserCasesPermissions } from '../../../hooks/use_get_user_cases_permissions';
 import { parseAlert } from './parse_alert';
@@ -53,6 +54,7 @@ export function ObservabilityActions({
   const dataFieldEs = data.reduce((acc, d) => ({ ...acc, [d.field]: d.value }), {});
   const [openActionsPopoverId, setActionsPopover] = useState(null);
   const { cases, http } = useKibana<ObservabilityAppServices>().services;
+  const { config } = usePluginContext();
 
   const parseObservabilityAlert = useMemo(
     () => parseAlert(observabilityRuleTypeRegistry),
@@ -141,40 +143,39 @@ export function ObservabilityActions({
         : []),
 
       ...[
-        <EuiContextMenuItem
-          key="viewAlertDetails"
-          data-test-subj="viewAlertDetails"
-          onClick={() => {
-            closeActionsPopover();
-            setFlyoutAlert(alert);
-          }}
-        >
-          {translations.alertsTable.viewAlertDetailsButtonText}
-        </EuiContextMenuItem>,
+        config.unsafe.alertDetails.enabled && linkToAlert ? (
+          <EuiContextMenuItem
+            key="viewAlertDetailsPage"
+            data-test-subj="viewAlertDetailsPage"
+            href={linkToAlert}
+          >
+            {translations.alertsTable.viewAlertDetailsButtonText}
+          </EuiContextMenuItem>
+        ) : (
+          <EuiContextMenuItem
+            key="viewAlertDetailsFlyout"
+            data-test-subj="viewAlertDetailsFlyout"
+            onClick={() => {
+              closeActionsPopover();
+              setFlyoutAlert(alert);
+            }}
+          >
+            {translations.alertsTable.viewAlertDetailsButtonText}
+          </EuiContextMenuItem>
+        ),
       ],
-
-      ...(linkToAlert
-        ? [
-            <EuiContextMenuItem
-              key="viewAlertDetailsPage"
-              data-test-subj="viewAlertDetailsPage"
-              href={linkToAlert}
-            >
-              {translations.alertsTable.viewAlertDetailsPageButtonText}
-            </EuiContextMenuItem>,
-          ]
-        : []),
     ];
   }, [
     userCasesPermissions.create,
     userCasesPermissions.read,
     handleAddToExistingCaseClick,
     handleAddToNewCaseClick,
     linkToRule,
-    alert,
+    config.unsafe.alertDetails.enabled,
     linkToAlert,
-    setFlyoutAlert,
     closeActionsPopover,
+    setFlyoutAlert,
+    alert,
   ]);
 
   const actionsToolTip =

diff --git a/x-pack/plugins/observability/public/pages/alerts/containers/alerts_page/alerts_page.tsx b/x-pack/plugins/observability/public/pages/alerts/containers/alerts_page/alerts_page.tsx
@@ -220,7 +220,7 @@ function AlertsPage() {
     <ObservabilityPageTemplate
       noDataConfig={noDataConfig}
       isPageDataLoaded={isAllRequestsComplete}
-      data-test-subj={noDataConfig ? 'noDataPage' : undefined}
+      data-test-subj={noDataConfig ? 'noDataPage' : 'alertsPageWithData'}
       pageHeader={{
         pageTitle: (
           <>{i18n.translate('xpack.observability.alertsTitle', { defaultMessage: 'Alerts' })} </>

diff --git a/x-pack/plugins/observability/public/pages/overview/overview.stories.tsx b/x-pack/plugins/observability/public/pages/overview/overview.stories.tsx
@@ -25,6 +25,7 @@ import { newsFeedFetchData } from './mock/news_feed.mock';
 import { emptyResponse as emptyUptimeResponse, fetchUptimeData } from './mock/uptime.mock';
 import { createObservabilityRuleTypeRegistryMock } from '../../rules/observability_rule_type_registry_mock';
 import { ApmIndicesConfig } from '../../../common/typings';
+import { ConfigSchema } from '../../plugin';
 
 function unregisterAll() {
   unregisterDataHandler({ appName: 'apm' });
@@ -74,6 +75,12 @@ const withCore = makeDecorator({
       },
     } as unknown as Partial<CoreStart>);
 
+    const config = {
+      unsafe: {
+        alertDetails: { enabled: false },
+      },
+    } as ConfigSchema;
+
     return (
       <MemoryRouter>
         <KibanaReactContext.Provider>
@@ -82,6 +89,7 @@ const withCore = makeDecorator({
               appMountParameters: {
                 setHeaderActionMenu: () => {},
               } as unknown as AppMountParameters,
+              config,
               observabilityRuleTypeRegistry: createObservabilityRuleTypeRegistryMock(),
               ObservabilityPageTemplate: KibanaPageTemplate,
             }}

diff --git a/x-pack/plugins/observability/public/pages/rules/index.test.tsx b/x-pack/plugins/observability/public/pages/rules/index.test.tsx
@@ -10,7 +10,7 @@ import { mountWithIntl, nextTick } from '@kbn/test-jest-helpers';
 import { act } from 'react-dom/test-utils';
 import { ReactWrapper } from 'enzyme';
 import { CoreStart } from '@kbn/core/public';
-import { ObservabilityPublicPluginsStart } from '../../plugin';
+import { ConfigSchema, ObservabilityPublicPluginsStart } from '../../plugin';
 import { RulesPage } from '.';
 import { kibanaStartMock } from '../../utils/kibana_react.mock';
 import * as pluginContext from '../../hooks/use_plugin_context';
@@ -34,8 +34,15 @@ jest.mock('@kbn/triggers-actions-ui-plugin/public', () => ({
   useLoadRuleTypes: jest.fn(),
 }));
 
+const config = {
+  unsafe: {
+    alertDetails: { enabled: false },
+  },
+} as ConfigSchema;
+
 jest.spyOn(pluginContext, 'usePluginContext').mockImplementation(() => ({
   appMountParameters: {} as AppMountParameters,
+  config,
   observabilityRuleTypeRegistry: createObservabilityRuleTypeRegistryMock(),
   ObservabilityPageTemplate: KibanaPageTemplate,
   kibanaFeatures: [],