[Reporting] Remove PNG V1 (#162517)

## Summary Closes #162293 and partially addresses [154601](#154601) Removes the screenshot diagnostic tool but keeps the browser check Removes PngV1ExportType from core and the export types registry ### Before There were two steps so the EuiSteps component definitely made more sense. ![Screenshot 2023-07-31 at 8 38 50 AM](https://github.com/elastic/kibana/assets/20343860/f054f024-9148-4343-be45-9ddf175b8c71) ![Screenshot 2023-07-31 at 8 41 53 AM](https://github.com/elastic/kibana/assets/20343860/71c6de8a-723c-462a-a7ad-51a4ca45f58f) ### After I removed the use of the EuiSteps component since there's only the browser check. Since the EuiSteps also showed some validation, I added a callout to let users know the status of the diagnostic. ![Screenshot 2023-07-31 at 8 35 05 AM](https://github.com/elastic/kibana/assets/20343860/fce09be1-ec2d-43bf-ab7d-f90df82c0579) ![Screenshot 2023-07-31 at 1 51 00 PM](https://github.com/elastic/kibana/assets/20343860/3fdeb41e-5d3a-4e99-b9aa-63d2d739715f) ### Checklist Delete any items that are not applicable to this PR. - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: kibanamachine <[email protected]>
elastic · Aug 2, 2023 · d8078b6 · d8078b6
1 parent 787454a
commit d8078b6
Show file tree

Hide file tree

Showing 32 changed files with 83 additions and 1,237 deletions.
diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS
@@ -559,7 +559,6 @@ packages/kbn-repo-source-classifier @elastic/kibana-operations
 packages/kbn-repo-source-classifier-cli @elastic/kibana-operations
 packages/kbn-reporting/common @elastic/appex-sharedux
 x-pack/examples/reporting_example @elastic/appex-sharedux
-x-pack/plugins/reporting_export_types @elastic/appex-sharedux
 x-pack/plugins/reporting @elastic/appex-sharedux
 x-pack/test/plugin_functional/plugins/resolver_test @elastic/security-solution
 examples/response_stream @elastic/ml-ui

diff --git a/docs/developer/plugin-list.asciidoc b/docs/developer/plugin-list.asciidoc
@@ -685,10 +685,6 @@ Elastic.
 |An awesome Kibana reporting plugin
 
 
-|{kib-repo}blob/{branch}/x-pack/plugins/reporting_export_types/README.md[reportingExportTypes]
-|This plugin is intended to be a central point for the export types of reporting to convene before hitting the reporting plugin.
-
-
 |{kib-repo}blob/{branch}/x-pack/plugins/rollup/README.md[rollup]
 |Welcome to the Kibana rollup plugin! This plugin provides Kibana support for Elasticsearch's rollup feature. Please refer to the Elasticsearch documentation to understand rollup indices and how to create rollup jobs.
 

diff --git a/package.json b/package.json
@@ -562,7 +562,6 @@
     "@kbn/repo-packages": "link:packages/kbn-repo-packages",
     "@kbn/reporting-common": "link:packages/kbn-reporting/common",
     "@kbn/reporting-example-plugin": "link:x-pack/examples/reporting_example",
-    "@kbn/reporting-export-types-plugin": "link:x-pack/plugins/reporting_export_types",
     "@kbn/reporting-plugin": "link:x-pack/plugins/reporting",
     "@kbn/resolver-test-plugin": "link:x-pack/test/plugin_functional/plugins/resolver_test",
     "@kbn/response-stream-plugin": "link:examples/response_stream",

diff --git a/tsconfig.base.json b/tsconfig.base.json
@@ -1112,8 +1112,6 @@
       "@kbn/reporting-common/*": ["packages/kbn-reporting/common/*"],
       "@kbn/reporting-example-plugin": ["x-pack/examples/reporting_example"],
       "@kbn/reporting-example-plugin/*": ["x-pack/examples/reporting_example/*"],
-      "@kbn/reporting-export-types-plugin": ["x-pack/plugins/reporting_export_types"],
-      "@kbn/reporting-export-types-plugin/*": ["x-pack/plugins/reporting_export_types/*"],
       "@kbn/reporting-plugin": ["x-pack/plugins/reporting"],
       "@kbn/reporting-plugin/*": ["x-pack/plugins/reporting/*"],
       "@kbn/resolver-test-plugin": ["x-pack/test/plugin_functional/plugins/resolver_test"],

diff --git a/x-pack/plugins/reporting/public/management/components/report_diagnostic.tsx b/x-pack/plugins/reporting/public/management/components/report_diagnostic.tsx
@@ -6,19 +6,16 @@
  */
 
 import { i18n } from '@kbn/i18n';
-import React, { useState, Fragment } from 'react';
+import React, { useState } from 'react';
 import { FormattedMessage } from '@kbn/i18n-react';
 import {
   EuiButton,
   EuiButtonEmpty,
   EuiCallOut,
-  EuiCodeBlock,
   EuiFlyout,
   EuiFlyoutBody,
   EuiFlyoutHeader,
-  EuiMarkdownFormat,
   EuiSpacer,
-  EuiSteps,
   EuiText,
   EuiTitle,
 } from '@elastic/eui';
@@ -38,7 +35,6 @@ enum statuses {
 interface State {
   isFlyoutVisible: boolean;
   chromeStatus: ResultStatus;
-  screenshotStatus: ResultStatus;
   help: string[];
   logs: string;
   isBusy: boolean;
@@ -47,7 +43,6 @@ interface State {
 
 const initialState: State = {
   [statuses.chromeStatus]: 'incomplete',
-  [statuses.screenshotStatus]: 'incomplete',
   isFlyoutVisible: false,
   help: [],
   logs: '',
@@ -62,7 +57,7 @@ export const ReportDiagnostic = ({ apiClient }: Props) => {
       ...state,
       ...s,
     });
-  const { isBusy, screenshotStatus, chromeStatus, isFlyoutVisible, help, logs, success } = state;
+  const { isBusy, chromeStatus, isFlyoutVisible } = state;
 
   const closeFlyout = () => setState({ ...initialState, isFlyoutVisible: false });
   const showFlyout = () => setState({ isFlyoutVisible: true });
@@ -94,124 +89,41 @@ export const ReportDiagnostic = ({ apiClient }: Props) => {
       });
   };
 
-  const steps = [
-    {
-      title: i18n.translate('xpack.reporting.listing.diagnosticBrowserTitle', {
-        defaultMessage: 'Check browser',
-      }),
-      children: (
-        <Fragment>
-          <FormattedMessage
-            id="xpack.reporting.listing.diagnosticBrowserMessage"
-            defaultMessage="Reporting uses a headless browser to generate PDF and PNGs. Validate that the browser can launch successfully."
-          />
-          <EuiSpacer />
-          <EuiButton
-            disabled={isBusy || chromeStatus === 'complete'}
-            onClick={apiWrapper(() => apiClient.verifyBrowser(), statuses.chromeStatus)}
-            isLoading={isBusy && chromeStatus === 'incomplete'}
-            iconType={chromeStatus === 'complete' ? 'check' : undefined}
-          >
-            <FormattedMessage
-              id="xpack.reporting.listing.diagnosticBrowserButton"
-              defaultMessage="Check browser"
-            />
-          </EuiButton>
-        </Fragment>
-      ),
-      status: !success && chromeStatus !== 'complete' ? 'danger' : chromeStatus,
-    },
-  ];
-
-  if (chromeStatus === 'complete') {
-    steps.push({
-      title: i18n.translate('xpack.reporting.listing.diagnosticScreenshotTitle', {
-        defaultMessage: 'Check screen capture',
-      }),
-      children: (
-        <Fragment>
-          <FormattedMessage
-            id="xpack.reporting.listing.diagnosticScreenshotMessage"
-            defaultMessage="Ensure that the headless browser can capture a screenshot of a page."
-          />
-          <EuiSpacer />
-          <EuiButton
-            disabled={isBusy || screenshotStatus === 'complete'}
-            onClick={apiWrapper(() => apiClient.verifyScreenCapture(), statuses.screenshotStatus)}
-            isLoading={isBusy && screenshotStatus === 'incomplete'}
-            iconType={screenshotStatus === 'complete' ? 'check' : undefined}
-          >
-            <FormattedMessage
-              id="xpack.reporting.listing.diagnosticScreenshotButton"
-              defaultMessage="Capture screenshot"
-            />
-          </EuiButton>
-        </Fragment>
-      ),
-      status: !success && screenshotStatus !== 'complete' ? 'danger' : screenshotStatus,
-    });
-  }
-
-  if (screenshotStatus === 'complete') {
-    steps.push({
-      title: i18n.translate('xpack.reporting.listing.diagnosticSuccessTitle', {
-        defaultMessage: 'All set!',
-      }),
-      children: (
-        <Fragment>
-          <FormattedMessage
-            id="xpack.reporting.listing.diagnosticSuccessMessage"
-            defaultMessage="Everything looks good for reporting to function."
-          />
-        </Fragment>
-      ),
-      status: !success ? 'danger' : screenshotStatus,
-    });
-  }
-
-  if (!success) {
-    steps.push({
-      title: i18n.translate('xpack.reporting.listing.diagnosticFailureTitle', {
-        defaultMessage: "Something isn't working properly.",
-      }),
-      children: (
-        <Fragment>
-          {help.length ? (
-            <Fragment>
-              <EuiCallOut color="danger" iconType="warning">
-                <p>
-                  <EuiMarkdownFormat>{help.join('\n')}</EuiMarkdownFormat>
-                </p>
-              </EuiCallOut>
-            </Fragment>
-          ) : null}
-          {logs.length ? (
-            <Fragment>
-              <EuiSpacer />
-              <FormattedMessage
-                id="xpack.reporting.listing.diagnosticFailureDescription"
-                defaultMessage="Here are some details about the issue:"
-              />
-              <EuiSpacer />
-              <EuiCodeBlock>{logs}</EuiCodeBlock>
-            </Fragment>
-          ) : null}
-        </Fragment>
-      ),
-      status: 'danger',
-    });
-  }
-
   let flyout;
   if (isFlyoutVisible) {
+    let outcomeCallout;
+
+    if (state.success && chromeStatus === 'complete') {
+      outcomeCallout = (
+        <EuiCallOut
+          id="xpack.reporting.listing.diagnosticSuccessMessage"
+          color="success"
+          title={i18n.translate('xpack.reporting.listing.diagnosticSuccessMessage', {
+            defaultMessage: 'Everything looks good for reporting to function.',
+          })}
+        />
+      );
+    } else if (!state.success && chromeStatus === 'complete') {
+      outcomeCallout = (
+        <EuiCallOut
+          id="xpack.reporting.listing.diagnosticFailureTitle"
+          iconType="warning"
+          color="danger"
+          title={i18n.translate('xpack.reporting.listing.diagnosticFailureTitle', {
+            defaultMessage: "Something isn't working properly.",
+          })}
+        />
+      );
+    }
+
     flyout = (
       <EuiFlyout onClose={closeFlyout} aria-labelledby="reportingHelperTitle" size="m">
         <EuiFlyoutHeader hasBorder>
           <EuiTitle size="m">
             <h2>
               <FormattedMessage
                 id="xpack.reporting.listing.diagnosticTitle"
-                defaultMessage="Reporting Diagnostics"
+                defaultMessage="Screenshotting Diagnostics"
               />
             </h2>
           </EuiTitle>
@@ -223,8 +135,34 @@ export const ReportDiagnostic = ({ apiClient }: Props) => {
             />
           </EuiText>
         </EuiFlyoutHeader>
-        <EuiFlyoutBody>
-          <EuiSteps steps={steps} />
+        <EuiFlyoutBody banner={outcomeCallout}>
+          <EuiTitle size="s">
+            <h2>
+              <FormattedMessage
+                id="xpack.reporting.listing.diagnosticBrowserTitle"
+                defaultMessage="Check browser"
+              />
+            </h2>
+          </EuiTitle>
+          <EuiSpacer size="s" />
+          <EuiText color="subdued">
+            <FormattedMessage
+              id="xpack.reporting.listing.diagnosticBrowserMessage"
+              defaultMessage="Reporting uses a headless browser to generate PDF and PNGs. Validate that the browser can launch successfully."
+            />
+          </EuiText>
+          <EuiSpacer />
+          <EuiButton
+            disabled={isBusy || chromeStatus === 'complete'}
+            onClick={apiWrapper(() => apiClient.verifyBrowser(), statuses.chromeStatus)}
+            isLoading={isBusy && chromeStatus === 'incomplete'}
+            iconType={chromeStatus === 'complete' ? 'check' : undefined}
+          >
+            <FormattedMessage
+              id="xpack.reporting.listing.diagnosticBrowserButton"
+              defaultMessage="Check browser"
+            />
+          </EuiButton>
         </EuiFlyoutBody>
       </EuiFlyout>
     );
@@ -235,7 +173,7 @@ export const ReportDiagnostic = ({ apiClient }: Props) => {
       <EuiButtonEmpty size="xs" flush="left" onClick={showFlyout}>
         <FormattedMessage
           id="xpack.reporting.listing.diagnosticButton"
-          defaultMessage="Run reporting diagnostics"
+          defaultMessage="Run screenshot diagnostics"
         />
       </EuiButtonEmpty>
     </div>

diff --git a/x-pack/plugins/reporting/server/core.ts b/x-pack/plugins/reporting/server/core.ts
@@ -46,7 +46,6 @@ import { CsvSearchSourceExportType } from './export_types/csv_searchsource';
 import { CsvV2ExportType } from './export_types/csv_v2';
 import { PdfV1ExportType } from './export_types/printable_pdf';
 import { PdfExportType } from './export_types/printable_pdf_v2';
-import { PngV1ExportType } from './export_types/png';
 import { PngExportType } from './export_types/png_v2';
 import { checkLicense, ExportTypesRegistry } from './lib';
 import { reportingEventLoggerFactory } from './lib/event_logger/logger';
@@ -135,7 +134,6 @@ export class ReportingCore {
     this.exportTypes.push(new PngExportType(this.core, this.config, this.logger, this.context));
     // deprecated export types for tests
     this.exportTypes.push(new PdfV1ExportType(this.core, this.config, this.logger, this.context));
-    this.exportTypes.push(new PngV1ExportType(this.core, this.config, this.logger, this.context));
 
     this.exportTypes.forEach((et) => {
       this.exportTypesRegistry.register(et);

diff --git a/x-pack/plugins/reporting/server/export_types/common/get_full_urls.test.ts b/x-pack/plugins/reporting/server/export_types/common/get_full_urls.test.ts
@@ -6,12 +6,11 @@
  */
 
 import { ReportingServerInfo } from '../../core';
-import { TaskPayloadPNG } from '../png/types';
 import { TaskPayloadPDF } from '../printable_pdf/types';
 import { getFullUrls } from './get_full_urls';
 import { ReportingConfigType } from '../../config';
 
-const getMockJob = (base: object) => base as TaskPayloadPNG & TaskPayloadPDF;
+const getMockJob = (base: object) => base as TaskPayloadPDF;
 const mockConfig = { kibanaServer: {} } as unknown as ReportingConfigType;
 const mockServerInfo: ReportingServerInfo = {
   hostname: 'localhost',
@@ -29,25 +28,6 @@ test(`fails if no URL is passed`, async () => {
   );
 });
 
-test(`fails if URLs are file-protocols for PNGs`, async () => {
-  const forceNow = '2000-01-01T00:00:00.000Z';
-  const relativeUrl = 'file://etc/passwd/#/something';
-  const fn = () => getFullUrls(mockServerInfo, mockConfig, getMockJob({ relativeUrl, forceNow }));
-  expect(fn).toThrowErrorMatchingInlineSnapshot(
-    `"Found invalid URL(s), all URLs must be relative: file://etc/passwd/#/something"`
-  );
-});
-
-test(`fails if URLs are absolute for PNGs`, async () => {
-  const forceNow = '2000-01-01T00:00:00.000Z';
-  const relativeUrl =
-    'http://169.254.169.254/latest/meta-data/iam/security-credentials/profileName/#/something';
-  const fn = () => getFullUrls(mockServerInfo, mockConfig, getMockJob({ relativeUrl, forceNow }));
-  expect(fn).toThrowErrorMatchingInlineSnapshot(
-    `"Found invalid URL(s), all URLs must be relative: http://169.254.169.254/latest/meta-data/iam/security-credentials/profileName/#/something"`
-  );
-});
-
 test(`fails if URLs are file-protocols for PDF`, async () => {
   const forceNow = '2000-01-01T00:00:00.000Z';
   const relativeUrl = 'file://etc/passwd/#/something';
@@ -95,7 +75,11 @@ test(`fails if any URLs are absolute or file's for PDF`, async () => {
 
 test(`fails if URL does not route to a visualization`, async () => {
   const fn = () =>
-    getFullUrls(mockServerInfo, mockConfig, getMockJob({ relativeUrl: '/app/phoney' }));
+    getFullUrls(
+      mockServerInfo,
+      mockConfig,
+      getMockJob({ objects: [{ relativeUrl: '/app/phoney' }] })
+    );
   expect(fn).toThrowErrorMatchingInlineSnapshot(
     `"No valid hash in the URL! A hash is expected for the application to route to the intended visualization."`
   );
@@ -106,7 +90,7 @@ test(`adds forceNow to hash's query, if it exists`, async () => {
   const urls = getFullUrls(
     mockServerInfo,
     mockConfig,
-    getMockJob({ relativeUrl: '/app/kibana#/something', forceNow })
+    getMockJob({ objects: [{ relativeUrl: '/app/kibana#/something' }], forceNow })
   );
 
   expect(urls[0]).toEqual(
@@ -120,7 +104,7 @@ test(`appends forceNow to hash's query, if it exists`, async () => {
   const urls = getFullUrls(
     mockServerInfo,
     mockConfig,
-    getMockJob({ relativeUrl: '/app/kibana#/something?_g=something', forceNow })
+    getMockJob({ objects: [{ relativeUrl: '/app/kibana#/something?_g=something' }], forceNow })
   );
 
   expect(urls[0]).toEqual(
@@ -132,7 +116,7 @@ test(`doesn't append forceNow query to url, if it doesn't exists`, async () => {
   const urls = getFullUrls(
     mockServerInfo,
     mockConfig,
-    getMockJob({ relativeUrl: '/app/kibana#/something' })
+    getMockJob({ objects: [{ relativeUrl: '/app/kibana#/something' }] })
   );
 
   expect(urls[0]).toEqual('http://localhost:5601/sbp/app/kibana#/something');