From c758bead0d3c76234c03e28b176d57adf1e061fc Mon Sep 17 00:00:00 2001 From: Nathan L Smith Date: Mon, 14 Sep 2020 23:00:57 -0500 Subject: [PATCH] Move remaining uses of serviceName away from urlParams (#77248) (#77428) * Move remaining uses of serviceName away from urlParams There were a few of these that were either missed or lost in merge conflict resolution. I went through everything that's used as a path parameter and made sure it wasn't being used anywhere with `urlParams`. Previously none of the charts were working, now they all are. Looks like #67791 introduced a find/replace change that broke APM's e2e tests. This reverts that change. --- .../e2e/cypress/support/step_definitions/apm.ts | 2 +- .../support/step_definitions/rum/rum_dashboard.ts | 2 +- .../app/RumDashboard/CoreVitals/index.tsx | 5 +++-- .../components/app/ServiceMap/Popover/Buttons.tsx | 6 +----- .../public/components/shared/DatePicker/index.tsx | 3 +-- .../ErroneousTransactionsRateChart/index.tsx | 14 ++++---------- ...er.test.ts => useAvgDurationByBrowser.test.tsx} | 10 +++++++++- .../apm/public/hooks/useAvgDurationByBrowser.ts | 10 ++++++---- .../apm/public/hooks/useAvgDurationByCountry.ts | 5 +++-- .../apm/public/hooks/useTransactionBreakdown.ts | 4 +++- .../apm/public/hooks/useTransactionCharts.ts | 4 +++- .../apm/public/hooks/useTransactionDistribution.ts | 9 +++++---- .../services/rest/apm_overview_fetchers.test.ts | 6 +++--- .../public/services/rest/apm_overview_fetchers.ts | 2 +- 14 files changed, 44 insertions(+), 38 deletions(-) rename x-pack/plugins/apm/public/hooks/{useAvgDurationByBrowser.test.ts => useAvgDurationByBrowser.test.tsx} (82%) diff --git a/x-pack/plugins/apm/e2e/cypress/support/step_definitions/apm.ts b/x-pack/plugins/apm/e2e/cypress/support/step_definitions/apm.ts index 66d604a663fbfe..ab2bf20b36ed4b 100644 --- a/x-pack/plugins/apm/e2e/cypress/support/step_definitions/apm.ts +++ b/x-pack/plugins/apm/e2e/cypress/support/step_definitions/apm.ts @@ -12,7 +12,7 @@ export const DEFAULT_TIMEOUT = 60 * 1000; Given(`a user browses the APM UI application`, () => { // open service overview page - loginAndWaitForPage(`/app/apm#/services`, { + loginAndWaitForPage(`/app/apm/services`, { from: '2020-06-01T14:59:32.686Z', to: '2020-06-16T16:59:36.219Z', }); diff --git a/x-pack/plugins/apm/e2e/cypress/support/step_definitions/rum/rum_dashboard.ts b/x-pack/plugins/apm/e2e/cypress/support/step_definitions/rum/rum_dashboard.ts index 804974d8d437d7..31aef30c4e23f5 100644 --- a/x-pack/plugins/apm/e2e/cypress/support/step_definitions/rum/rum_dashboard.ts +++ b/x-pack/plugins/apm/e2e/cypress/support/step_definitions/rum/rum_dashboard.ts @@ -15,7 +15,7 @@ Given(`a user browses the APM UI application for RUM Data`, () => { // open service overview page const RANGE_FROM = 'now-24h'; const RANGE_TO = 'now'; - loginAndWaitForPage(`/app/apm#/rum-preview`, { + loginAndWaitForPage(`/app/apm/rum-preview`, { from: RANGE_FROM, to: RANGE_TO, }); diff --git a/x-pack/plugins/apm/public/components/app/RumDashboard/CoreVitals/index.tsx b/x-pack/plugins/apm/public/components/app/RumDashboard/CoreVitals/index.tsx index e8305a6aef0d4f..d0bd674ef5c797 100644 --- a/x-pack/plugins/apm/public/components/app/RumDashboard/CoreVitals/index.tsx +++ b/x-pack/plugins/apm/public/components/app/RumDashboard/CoreVitals/index.tsx @@ -20,10 +20,11 @@ const CoreVitalsThresholds = { export function CoreVitals() { const { urlParams, uiFilters } = useUrlParams(); - const { start, end, serviceName } = urlParams; + const { start, end } = urlParams; const { data, status } = useFetcher( (callApmApi) => { + const { serviceName } = uiFilters; if (start && end && serviceName) { return callApmApi({ pathname: '/api/apm/rum-client/web-core-vitals', @@ -34,7 +35,7 @@ export function CoreVitals() { } return Promise.resolve(null); }, - [start, end, serviceName, uiFilters] + [start, end, uiFilters] ); const { lcp, lcpRanks, fid, fidRanks, cls, clsRanks } = data || {}; diff --git a/x-pack/plugins/apm/public/components/app/ServiceMap/Popover/Buttons.tsx b/x-pack/plugins/apm/public/components/app/ServiceMap/Popover/Buttons.tsx index 55d068cba5935d..8670cf623c2532 100644 --- a/x-pack/plugins/apm/public/components/app/ServiceMap/Popover/Buttons.tsx +++ b/x-pack/plugins/apm/public/components/app/ServiceMap/Popover/Buttons.tsx @@ -25,11 +25,7 @@ export function Buttons({ }: ButtonsProps) { const { core } = useApmPluginContext(); const { basePath } = core.http; - // The params may contain the service name. We want to use the selected node's - // service name in the button URLs, so make a copy and set the - // `serviceName` property. - const urlParams = { ...useUrlParams().urlParams } as APMQueryParams; - urlParams.serviceName = selectedNodeServiceName; + const urlParams = useUrlParams().urlParams as APMQueryParams; const detailsUrl = getAPMHref({ basePath, diff --git a/x-pack/plugins/apm/public/components/shared/DatePicker/index.tsx b/x-pack/plugins/apm/public/components/shared/DatePicker/index.tsx index 549ee228b7afeb..b4d716f89169e1 100644 --- a/x-pack/plugins/apm/public/components/shared/DatePicker/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/DatePicker/index.tsx @@ -7,10 +7,9 @@ import { EuiSuperDatePicker } from '@elastic/eui'; import { isEmpty, isEqual, pickBy } from 'lodash'; import React from 'react'; -import { useHistory } from 'react-router-dom'; +import { useHistory, useLocation } from 'react-router-dom'; import { UI_SETTINGS } from '../../../../../../../src/plugins/data/common'; import { useApmPluginContext } from '../../../hooks/useApmPluginContext'; -import { useLocation } from '../../../hooks/useLocation'; import { useUrlParams } from '../../../hooks/useUrlParams'; import { clearCache } from '../../../services/rest/callApi'; import { fromQuery, toQuery } from '../Links/url_helpers'; diff --git a/x-pack/plugins/apm/public/components/shared/charts/ErroneousTransactionsRateChart/index.tsx b/x-pack/plugins/apm/public/components/shared/charts/ErroneousTransactionsRateChart/index.tsx index 1a91e398cec76c..85d975870d9bc9 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/ErroneousTransactionsRateChart/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/charts/ErroneousTransactionsRateChart/index.tsx @@ -3,12 +3,11 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ -import { EuiTitle } from '@elastic/eui'; +import { EuiPanel, EuiSpacer, EuiTitle } from '@elastic/eui'; import theme from '@elastic/eui/dist/eui_theme_light.json'; import { i18n } from '@kbn/i18n'; import React, { useCallback } from 'react'; -import { EuiPanel } from '@elastic/eui'; -import { EuiSpacer } from '@elastic/eui'; +import { useParams } from 'react-router-dom'; import { asPercent } from '../../../../../common/utils/formatters'; import { useChartsSync } from '../../../../hooks/useChartsSync'; import { useFetcher } from '../../../../hooks/useFetcher'; @@ -22,16 +21,11 @@ const tickFormatY = (y?: number) => { }; export function ErroneousTransactionsRateChart() { + const { serviceName } = useParams<{ serviceName?: string }>(); const { urlParams, uiFilters } = useUrlParams(); const syncedChartsProps = useChartsSync(); - const { - serviceName, - start, - end, - transactionType, - transactionName, - } = urlParams; + const { start, end, transactionType, transactionName } = urlParams; const { data } = useFetcher(() => { if (serviceName && start && end) { diff --git a/x-pack/plugins/apm/public/hooks/useAvgDurationByBrowser.test.ts b/x-pack/plugins/apm/public/hooks/useAvgDurationByBrowser.test.tsx similarity index 82% rename from x-pack/plugins/apm/public/hooks/useAvgDurationByBrowser.test.ts rename to x-pack/plugins/apm/public/hooks/useAvgDurationByBrowser.test.tsx index b34cc95a26399e..bb947e307437eb 100644 --- a/x-pack/plugins/apm/public/hooks/useAvgDurationByBrowser.test.ts +++ b/x-pack/plugins/apm/public/hooks/useAvgDurationByBrowser.test.tsx @@ -8,6 +8,12 @@ import { renderHook } from '@testing-library/react-hooks'; import theme from '@elastic/eui/dist/eui_theme_light.json'; import * as useFetcherModule from './useFetcher'; import { useAvgDurationByBrowser } from './useAvgDurationByBrowser'; +import React, { ReactNode } from 'react'; +import { MemoryRouter } from 'react-router-dom'; + +function Wrapper({ children }: { children?: ReactNode }) { + return {children}; +} describe('useAvgDurationByBrowser', () => { it('returns data', () => { @@ -19,7 +25,9 @@ describe('useAvgDurationByBrowser', () => { refetch: () => {}, status: 'success' as useFetcherModule.FETCH_STATUS, }); - const { result } = renderHook(() => useAvgDurationByBrowser()); + const { result } = renderHook(() => useAvgDurationByBrowser(), { + wrapper: Wrapper, + }); expect(result.current.data).toEqual([ { diff --git a/x-pack/plugins/apm/public/hooks/useAvgDurationByBrowser.ts b/x-pack/plugins/apm/public/hooks/useAvgDurationByBrowser.ts index a8312dd0448e69..78dc4210711ef6 100644 --- a/x-pack/plugins/apm/public/hooks/useAvgDurationByBrowser.ts +++ b/x-pack/plugins/apm/public/hooks/useAvgDurationByBrowser.ts @@ -5,12 +5,13 @@ */ import theme from '@elastic/eui/dist/eui_theme_light.json'; -import { useFetcher } from './useFetcher'; -import { useUrlParams } from './useUrlParams'; +import { useParams } from 'react-router-dom'; +import { getVizColorForIndex } from '../../common/viz_colors'; // eslint-disable-next-line @kbn/eslint/no-restricted-paths import { AvgDurationByBrowserAPIResponse } from '../../server/lib/transactions/avg_duration_by_browser'; import { TimeSeries } from '../../typings/timeseries'; -import { getVizColorForIndex } from '../../common/viz_colors'; +import { useFetcher } from './useFetcher'; +import { useUrlParams } from './useUrlParams'; function toTimeSeries(data?: AvgDurationByBrowserAPIResponse): TimeSeries[] { if (!data) { @@ -27,8 +28,9 @@ function toTimeSeries(data?: AvgDurationByBrowserAPIResponse): TimeSeries[] { } export function useAvgDurationByBrowser() { + const { serviceName } = useParams<{ serviceName?: string }>(); const { - urlParams: { serviceName, start, end, transactionName }, + urlParams: { start, end, transactionName }, uiFilters, } = useUrlParams(); diff --git a/x-pack/plugins/apm/public/hooks/useAvgDurationByCountry.ts b/x-pack/plugins/apm/public/hooks/useAvgDurationByCountry.ts index 818657226cb14d..983f949b72961b 100644 --- a/x-pack/plugins/apm/public/hooks/useAvgDurationByCountry.ts +++ b/x-pack/plugins/apm/public/hooks/useAvgDurationByCountry.ts @@ -3,13 +3,14 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ - +import { useParams } from 'react-router-dom'; import { useFetcher } from './useFetcher'; import { useUrlParams } from './useUrlParams'; export function useAvgDurationByCountry() { + const { serviceName } = useParams<{ serviceName?: string }>(); const { - urlParams: { serviceName, start, end, transactionName }, + urlParams: { start, end, transactionName }, uiFilters, } = useUrlParams(); diff --git a/x-pack/plugins/apm/public/hooks/useTransactionBreakdown.ts b/x-pack/plugins/apm/public/hooks/useTransactionBreakdown.ts index 9db78fde2d8c86..08d2300c3254a6 100644 --- a/x-pack/plugins/apm/public/hooks/useTransactionBreakdown.ts +++ b/x-pack/plugins/apm/public/hooks/useTransactionBreakdown.ts @@ -4,12 +4,14 @@ * you may not use this file except in compliance with the Elastic License. */ +import { useParams } from 'react-router-dom'; import { useFetcher } from './useFetcher'; import { useUrlParams } from './useUrlParams'; export function useTransactionBreakdown() { + const { serviceName } = useParams<{ serviceName?: string }>(); const { - urlParams: { serviceName, start, end, transactionName, transactionType }, + urlParams: { start, end, transactionName, transactionType }, uiFilters, } = useUrlParams(); diff --git a/x-pack/plugins/apm/public/hooks/useTransactionCharts.ts b/x-pack/plugins/apm/public/hooks/useTransactionCharts.ts index 2ecd9003864965..e66d70a53afa65 100644 --- a/x-pack/plugins/apm/public/hooks/useTransactionCharts.ts +++ b/x-pack/plugins/apm/public/hooks/useTransactionCharts.ts @@ -5,13 +5,15 @@ */ import { useMemo } from 'react'; +import { useParams } from 'react-router-dom'; import { getTransactionCharts } from '../selectors/chartSelectors'; import { useFetcher } from './useFetcher'; import { useUrlParams } from './useUrlParams'; export function useTransactionCharts() { + const { serviceName } = useParams<{ serviceName?: string }>(); const { - urlParams: { serviceName, transactionType, start, end, transactionName }, + urlParams: { transactionType, start, end, transactionName }, uiFilters, } = useUrlParams(); diff --git a/x-pack/plugins/apm/public/hooks/useTransactionDistribution.ts b/x-pack/plugins/apm/public/hooks/useTransactionDistribution.ts index 539918c4327837..d93a27df1c8616 100644 --- a/x-pack/plugins/apm/public/hooks/useTransactionDistribution.ts +++ b/x-pack/plugins/apm/public/hooks/useTransactionDistribution.ts @@ -4,11 +4,12 @@ * you may not use this file except in compliance with the Elastic License. */ -import { IUrlParams } from '../context/UrlParamsContext/types'; -import { useFetcher } from './useFetcher'; -import { useUiFilters } from '../context/UrlParamsContext'; +import { useParams } from 'react-router-dom'; // eslint-disable-next-line @kbn/eslint/no-restricted-paths import { TransactionDistributionAPIResponse } from '../../server/lib/transactions/distribution'; +import { useUiFilters } from '../context/UrlParamsContext'; +import { IUrlParams } from '../context/UrlParamsContext/types'; +import { useFetcher } from './useFetcher'; const INITIAL_DATA = { buckets: [] as TransactionDistributionAPIResponse['buckets'], @@ -17,8 +18,8 @@ const INITIAL_DATA = { }; export function useTransactionDistribution(urlParams: IUrlParams) { + const { serviceName } = useParams<{ serviceName?: string }>(); const { - serviceName, start, end, transactionType, diff --git a/x-pack/plugins/apm/public/services/rest/apm_overview_fetchers.test.ts b/x-pack/plugins/apm/public/services/rest/apm_overview_fetchers.test.ts index 8b3ed38e25319f..4e306c93805d05 100644 --- a/x-pack/plugins/apm/public/services/rest/apm_overview_fetchers.test.ts +++ b/x-pack/plugins/apm/public/services/rest/apm_overview_fetchers.test.ts @@ -51,7 +51,7 @@ describe('Observability dashboard data', () => { ); const response = await fetchOverviewPageData(params); expect(response).toEqual({ - appLink: '/app/apm#/services?rangeFrom=now-15m&rangeTo=now', + appLink: '/app/apm/services?rangeFrom=now-15m&rangeTo=now', stats: { services: { type: 'number', @@ -82,7 +82,7 @@ describe('Observability dashboard data', () => { ); const response = await fetchOverviewPageData(params); expect(response).toEqual({ - appLink: '/app/apm#/services?rangeFrom=now-15m&rangeTo=now', + appLink: '/app/apm/services?rangeFrom=now-15m&rangeTo=now', stats: { services: { type: 'number', @@ -109,7 +109,7 @@ describe('Observability dashboard data', () => { ); const response = await fetchOverviewPageData(params); expect(response).toEqual({ - appLink: '/app/apm#/services?rangeFrom=now-15m&rangeTo=now', + appLink: '/app/apm/services?rangeFrom=now-15m&rangeTo=now', stats: { services: { type: 'number', diff --git a/x-pack/plugins/apm/public/services/rest/apm_overview_fetchers.ts b/x-pack/plugins/apm/public/services/rest/apm_overview_fetchers.ts index a20f89fac2d601..422c7b882e5dc4 100644 --- a/x-pack/plugins/apm/public/services/rest/apm_overview_fetchers.ts +++ b/x-pack/plugins/apm/public/services/rest/apm_overview_fetchers.ts @@ -32,7 +32,7 @@ export const fetchOverviewPageData = async ({ const { serviceCount, transactionCoordinates } = data; return { - appLink: `/app/apm#/services?rangeFrom=${relativeTime.start}&rangeTo=${relativeTime.end}`, + appLink: `/app/apm/services?rangeFrom=${relativeTime.start}&rangeTo=${relativeTime.end}`, stats: { services: { type: 'number',