From 603fb0980b4b2ac4510fbf47e33b85ec8b54e132 Mon Sep 17 00:00:00 2001 From: Nathan L Smith Date: Mon, 8 Jun 2020 15:20:25 -0500 Subject: [PATCH] APM Storybook fixes * Resolve core legacy assets in @kbn/storybook webpack configuration * Ignore stories in Jest coverage * Combine effects in Cytoscape component so handlers are always added before events are triggered * Add mock context to ErrorRateAlertTrigger stories * Disable TransactionDurationAlertTrigger stories Changing the Cytoscape effect behavior is necessary because the layout was not being triggered when the final set of elements is provided as props to the component. When this is used in Kibana we're always starting with empty elements and fetching them, but in the Storybook we're starting out with the full elements. --- .../storybook_config/webpack.config.js | 23 ++++++++++++++++++- x-pack/plugins/apm/jest.config.js | 1 + .../app/ServiceMap/Cytoscape.stories.tsx | 2 +- .../components/app/ServiceMap/Cytoscape.tsx | 15 ++++-------- .../ErrorRateAlertTrigger/index.stories.tsx | 23 +++++++++++++------ .../index.stories.tsx | 16 +++++++++---- 6 files changed, 56 insertions(+), 24 deletions(-) diff --git a/packages/kbn-storybook/storybook_config/webpack.config.js b/packages/kbn-storybook/storybook_config/webpack.config.js index 2dd051882bb4bc..543bb47656df8d 100644 --- a/packages/kbn-storybook/storybook_config/webpack.config.js +++ b/packages/kbn-storybook/storybook_config/webpack.config.js @@ -17,7 +17,7 @@ * under the License. */ -const { resolve } = require('path'); +const { parse, resolve } = require('path'); const webpack = require('webpack'); const { stringifyRequest } = require('loader-utils'); const CopyWebpackPlugin = require('copy-webpack-plugin'); @@ -95,6 +95,27 @@ module.exports = async ({ config }) => { }, }, }, + { + loader: 'resolve-url-loader', + options: { + // If you don't have arguments (_, __) to the join function, the + // resolve-url-loader fails with a loader misconfiguration error. + // + // eslint-disable-next-line no-unused-vars + join: (_, __) => (uri, base) => { + if (!base || !parse(base).dir.includes('legacy')) { + return null; + } + + // URIs on mixins in src/legacy/public/styles need to be resolved. + if (uri.startsWith('ui/assets')) { + return resolve(REPO_ROOT, 'src/core/server/core_app/', uri.replace('ui/', '')); + } + + return null; + }, + }, + }, { loader: 'sass-loader', options: { diff --git a/x-pack/plugins/apm/jest.config.js b/x-pack/plugins/apm/jest.config.js index c3ae694fe8e142..43bdeb583c819e 100644 --- a/x-pack/plugins/apm/jest.config.js +++ b/x-pack/plugins/apm/jest.config.js @@ -31,6 +31,7 @@ module.exports = { collectCoverageFrom: [ '**/*.{js,jsx,ts,tsx}', '!**/{__test__,__snapshots__,__examples__,integration_tests,tests}/**', + '!**/*.stories.{js,ts,tsx}', '!**/*.test.{js,ts,tsx}', '!**/dev_docs/**', '!**/e2e/**', diff --git a/x-pack/plugins/apm/public/components/app/ServiceMap/Cytoscape.stories.tsx b/x-pack/plugins/apm/public/components/app/ServiceMap/Cytoscape.stories.tsx index 1c62d3cc03db0a..30031a05304bba 100644 --- a/x-pack/plugins/apm/public/components/app/ServiceMap/Cytoscape.stories.tsx +++ b/x-pack/plugins/apm/public/components/app/ServiceMap/Cytoscape.stories.tsx @@ -346,7 +346,7 @@ storiesOf('app/ServiceMap/Cytoscape', module).add( }, }, ]; - return ; + return ; }, { info: { propTables: false, source: false }, diff --git a/x-pack/plugins/apm/public/components/app/ServiceMap/Cytoscape.tsx b/x-pack/plugins/apm/public/components/app/ServiceMap/Cytoscape.tsx index cb908785d64d8e..c57d702b9a546a 100644 --- a/x-pack/plugins/apm/public/components/app/ServiceMap/Cytoscape.tsx +++ b/x-pack/plugins/apm/public/components/app/ServiceMap/Cytoscape.tsx @@ -121,15 +121,6 @@ export function Cytoscape({ const trackApmEvent = useUiTracker({ app: 'apm' }); - // Trigger a custom "data" event when data changes - useEffect(() => { - if (cy) { - cy.remove(cy.elements()); - cy.add(elements); - cy.trigger('data'); - } - }, [cy, elements]); - // Set up cytoscape event handlers useEffect(() => { const resetConnectedEdgeStyle = (node?: cytoscape.NodeSingular) => { @@ -223,6 +214,10 @@ export function Cytoscape({ cy.on('mouseout', 'edge, node', mouseoutHandler); cy.on('select', 'node', selectHandler); cy.on('unselect', 'node', unselectHandler); + + cy.remove(cy.elements()); + cy.add(elements); + cy.trigger('data'); } return () => { @@ -241,7 +236,7 @@ export function Cytoscape({ } clearTimeout(layoutstopDelayTimeout); }; - }, [cy, height, serviceName, trackApmEvent, width]); + }, [cy, elements, height, serviceName, trackApmEvent, width]); return ( diff --git a/x-pack/plugins/apm/public/components/shared/ErrorRateAlertTrigger/index.stories.tsx b/x-pack/plugins/apm/public/components/shared/ErrorRateAlertTrigger/index.stories.tsx index 8f7ed54f91bd09..ebcb1627984adb 100644 --- a/x-pack/plugins/apm/public/components/shared/ErrorRateAlertTrigger/index.stories.tsx +++ b/x-pack/plugins/apm/public/components/shared/ErrorRateAlertTrigger/index.stories.tsx @@ -7,6 +7,11 @@ import { storiesOf } from '@storybook/react'; import React from 'react'; import { ErrorRateAlertTrigger } from '.'; +import { ApmPluginContextValue } from '../../../context/ApmPluginContext'; +import { + mockApmPluginContextValue, + MockApmPluginContextWrapper, +} from '../../../context/ApmPluginContext/MockApmPluginContext'; storiesOf('app/ErrorRateAlertTrigger', module).add('example', () => { const params = { @@ -15,12 +20,16 @@ storiesOf('app/ErrorRateAlertTrigger', module).add('example', () => { }; return ( -
- undefined} - setAlertProperty={() => undefined} - /> -
+ +
+ undefined} + setAlertProperty={() => undefined} + /> +
+ ); }); diff --git a/x-pack/plugins/apm/public/components/shared/TransactionDurationAlertTrigger/index.stories.tsx b/x-pack/plugins/apm/public/components/shared/TransactionDurationAlertTrigger/index.stories.tsx index e2429d1225442b..da9adbb8dfeade 100644 --- a/x-pack/plugins/apm/public/components/shared/TransactionDurationAlertTrigger/index.stories.tsx +++ b/x-pack/plugins/apm/public/components/shared/TransactionDurationAlertTrigger/index.stories.tsx @@ -3,18 +3,24 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ +// import { storiesOf } from '@storybook/react'; import { cloneDeep, merge } from 'lodash'; -import { storiesOf } from '@storybook/react'; import React from 'react'; import { TransactionDurationAlertTrigger } from '.'; +import { ApmPluginContextValue } from '../../../context/ApmPluginContext'; import { - MockApmPluginContextWrapper, mockApmPluginContextValue, + MockApmPluginContextWrapper, } from '../../../context/ApmPluginContext/MockApmPluginContext'; import { MockUrlParamsContextProvider } from '../../../context/UrlParamsContext/MockUrlParamsContextProvider'; -import { ApmPluginContextValue } from '../../../context/ApmPluginContext'; -storiesOf('app/TransactionDurationAlertTrigger', module).add('example', () => { +// Disabling this because we currently don't have a way to mock `useEnvironments` +// which is used by this component. Using the fetch-mock module should work, but +// our current storybook setup has core-js-related problems when trying to import +// it. +// storiesOf('app/TransactionDurationAlertTrigger', module).add('example', +// eslint-disable-next-line no-unused-expressions +() => { const params = { threshold: 1500, aggregationType: 'avg' as const, @@ -44,4 +50,4 @@ storiesOf('app/TransactionDurationAlertTrigger', module).add('example', () => { ); -}); +};