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.
elastic · Sep 15, 2020 · c758bea · c758bea
1 parent e88a4cd
commit c758bea
Show file tree

Hide file tree

Showing 14 changed files with 44 additions and 38 deletions.
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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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/...lic/hooks/useAvgDurationByBrowser.test.ts → ...ic/hooks/useAvgDurationByBrowser.test.tsx b/...lic/hooks/useAvgDurationByBrowser.test.ts → ...ic/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 <MemoryRouter>{children}</MemoryRouter>;
+}
 
 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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',