Skip to content

Commit

Permalink
[ObsUX][Infra] Refactor duplicated hook (elastic#191739)
Browse files Browse the repository at this point in the history
#### Summary

After adding the locator to fix the bug from the issue
elastic#191294, we realized that
`use_node_details_redirect`, was kind of duplicated in infra plugin and
metrics data access plugin. This PR removed the hook in infra plugin and
set for reuse the one in metric access plugin.
  • Loading branch information
MiriamAparicio authored Aug 30, 2024
1 parent a57181a commit 9a2566b
Show file tree
Hide file tree
Showing 18 changed files with 88 additions and 185 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
import { FormattedMessage } from '@kbn/i18n-react';
import { useUiSetting } from '@kbn/kibana-react-plugin/public';
import { enableInfrastructureAssetCustomDashboards } from '@kbn/observability-plugin/common';
import type { RouteState } from '@kbn/metrics-data-access-plugin/public';
import { capitalize, isEmpty } from 'lodash';
import React, { useCallback, useMemo } from 'react';
import { useHistory, useLocation } from 'react-router-dom';
Expand All @@ -22,7 +23,7 @@ import { useKibanaContextForPlugin } from '../../../hooks/use_kibana';
import { useProfilingIntegrationSetting } from '../../../hooks/use_profiling_integration_setting';
import { CreateAlertRuleButton } from '../../shared/alerts/links/create_alert_rule_button';
import { LinkToNodeDetails } from '../links';
import { ContentTabIds, type LinkOptions, type RouteState, type Tab, type TabIds } from '../types';
import { ContentTabIds, type LinkOptions, type Tab, type TabIds } from '../types';
import { useAssetDetailsRenderPropsContext } from './use_asset_details_render_props';
import { useTabSwitcherContext } from './use_tab_switcher';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { FormattedMessage } from '@kbn/i18n-react';
import { EuiButtonEmpty } from '@elastic/eui';
import { parse } from '@kbn/datemath';
import type { InventoryItemType } from '@kbn/metrics-data-access-plugin/common';
import { useNodeDetailsRedirect } from '../../../pages/link_to';
import { useAssetDetailsRedirect } from '@kbn/metrics-data-access-plugin/public';

import { useAssetDetailsUrlState } from '../hooks/use_asset_details_url_state';

Expand All @@ -21,12 +21,12 @@ export interface LinkToNodeDetailsProps {

export const LinkToNodeDetails = ({ assetId, assetName, assetType }: LinkToNodeDetailsProps) => {
const [state] = useAssetDetailsUrlState();
const { getNodeDetailUrl } = useNodeDetailsRedirect();
const { getAssetDetailUrl } = useAssetDetailsRedirect();

// don't propagate the autoRefresh to the details page
const { dateRange, autoRefresh: _, ...assetDetails } = state ?? {};

const nodeDetailMenuItemLinkProps = getNodeDetailUrl({
const assetDetailMenuItemLinkProps = getAssetDetailUrl({
assetType,
assetId,
search: {
Expand All @@ -42,7 +42,7 @@ export const LinkToNodeDetails = ({ assetId, assetName, assetType }: LinkToNodeD
data-test-subj="infraAssetDetailsOpenAsPageButton"
size="xs"
flush="both"
{...nodeDetailMenuItemLinkProps}
{...assetDetailMenuItemLinkProps}
>
<FormattedMessage
id="xpack.infra.infra.nodeDetails.openAsPage"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,3 @@ export { LinkToLogsPage } from './link_to_logs';
export { LinkToMetricsPage } from './link_to_metrics';
export { RedirectToNodeLogs } from './redirect_to_node_logs';
export { RedirectToNodeDetail } from './redirect_to_node_detail';

export { useNodeDetailsRedirect } from './use_node_details_redirect';
export type {
AssetDetailsQueryParams,
MetricDetailsQueryParams,
} from './use_node_details_redirect';

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import React from 'react';
import { i18n } from '@kbn/i18n';
import { EuiFlexGroup, EuiFlexItem, EuiLink, EuiToolTip } from '@elastic/eui';
import { CloudProviderIcon } from '@kbn/custom-icons';
import { useNodeDetailsRedirect } from '../../../../link_to/use_node_details_redirect';
import { useAssetDetailsRedirect } from '@kbn/metrics-data-access-plugin/public';
import type { HostNodeRow } from '../../hooks/use_hosts_table';
import { useUnifiedSearchContext } from '../../hooks/use_unified_search';

Expand All @@ -20,9 +20,9 @@ interface EntryTitleProps {
export const EntryTitle = ({ onClick, title }: EntryTitleProps) => {
const { name, cloudProvider } = title;
const { parsedDateRange } = useUnifiedSearchContext();
const { getNodeDetailUrl } = useNodeDetailsRedirect();
const { getAssetDetailUrl } = useAssetDetailsRedirect();

const link = getNodeDetailUrl({
const link = getAssetDetailUrl({
assetId: name,
assetType: 'host',
search: {
Expand All @@ -33,7 +33,6 @@ export const EntryTitle = ({ onClick, title }: EntryTitleProps) => {
});

const providerName = cloudProvider ?? 'Unknown';

return (
<EuiToolTip
delay="long"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,17 @@ import {
ActionMenuDivider,
useLinkProps,
} from '@kbn/observability-shared-plugin/public';
import { findInventoryModel, findInventoryFields } from '@kbn/metrics-data-access-plugin/common';
import { InventoryItemType } from '@kbn/metrics-data-access-plugin/common';
import {
findInventoryModel,
findInventoryFields,
type InventoryItemType,
} from '@kbn/metrics-data-access-plugin/common';
import { useAssetDetailsRedirect } from '@kbn/metrics-data-access-plugin/public';
import { getLogsLocatorsFromUrlService } from '@kbn/logs-shared-plugin/common';
import { uptimeOverviewLocatorID } from '@kbn/observability-plugin/common';
import { useKibanaContextForPlugin } from '../../../../../hooks/use_kibana';
import { AlertFlyout } from '../../../../../alerting/inventory/components/alert_flyout';
import { InfraWaffleMapNode, InfraWaffleMapOptions } from '../../../../../common/inventory/types';
import { useNodeDetailsRedirect } from '../../../../link_to';
import { navigateToUptime } from '../../lib/navigate_to_uptime';

interface Props {
Expand All @@ -40,7 +43,7 @@ interface Props {

export const NodeContextMenu: React.FC<Props & { theme?: EuiTheme }> = withTheme(
({ options, currentTime, node, nodeType }) => {
const { getNodeDetailUrl } = useNodeDetailsRedirect();
const { getAssetDetailUrl } = useAssetDetailsRedirect();
const [flyoutVisible, setFlyoutVisible] = useState(false);
const inventoryModel = findInventoryModel(nodeType);
const nodeDetailFrom = currentTime - inventoryModel.metrics.defaultTimeRangeInSeconds * 1000;
Expand Down Expand Up @@ -90,7 +93,7 @@ export const NodeContextMenu: React.FC<Props & { theme?: EuiTheme }> = withTheme
return { label: '', value: '' };
}, [nodeType, node.ip, node.id]);

const nodeDetailMenuItemLinkProps = getNodeDetailUrl({
const nodeDetailMenuItemLinkProps = getAssetDetailUrl({
assetType: nodeType,
assetId: node.id,
search: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const mockDataView = {
toSpec: () => ({}),
} as jest.Mocked<DataView>;

const series = { id: 'exmaple-01', rows: [], columns: [] };
const series = { id: 'example-01', rows: [], columns: [] };
const uiCapabilities: Capabilities = {
navLinks: { show: false },
management: { fake: { show: false } },
Expand Down Expand Up @@ -71,14 +71,17 @@ const mountComponentWithProviders = (props: Props): ReactWrapper => {
);
};

jest.mock('../../../link_to', () => ({
useNodeDetailsRedirect: jest.fn(() => ({
getNodeDetailUrl: jest.fn(() => ({
onClick: jest.fn(),
href: '/ftw/app/r?l=ASSET_DETAILS_LOCATOR&v=8.15.0&lz=N4IghgzhCmAuAicwEsA2EQC5gF8A0IA%2BmFqLMgLbSkgBmATgPYVYgCMA7GwCwAMArADZB3NgCZBADhAFYjVpx69BvMaInSc%2BcFDgAVAJ4AHaphAALRhFgydMWAEkAJqwBUt62FinQjesgBzZAA7AEEjI2dWKlh%2FAGMMAj9AkIBlaDB6OPNWAH4Y%2BIgAUQAPI1Q%2FaHoAXgAKbMzYAHkjckZgiExazziAa0wAQlo8WGNoTFQQ6DwDUJLkCABZRidxhmYALSrGAEo8Rlbkds7asACA%2BmgAryPgzDAANwC8AuQEwdrT88vrtrvH55xRgVeiYIEg3h4WjIaCoJyYCAGazQCgAOjiRgArqi5LAwKhUcE%2FGijHFYHsvhcrjd2vcnnhwX4wcC%2FGwoTC4ZhepiAEZVYJwaAQVFGFborGozEQM7QQkrWWk8l4Sk%2FGn%2FemM0GasTs2HwpyMPpVcXY3H4kVknZ7CCMTFZcarWhgTGoJXkKj0MDBALjWrrCiYIkAdwAtGxzHgQt56A98ZgAKQAZiKSfgbF4EBGjEDjCDVoAZK8EqVypV6AA1GFB5zVADkvFrtmSQWCAAUvOZgmAqKwAPTQMogqogLRAA%3D',
jest.mock(
'@kbn/metrics-data-access-plugin/public/pages/link_to/use_asset_details_redirect',
() => ({
useAssetDetailsRedirect: jest.fn(() => ({
getAssetDetailUrl: jest.fn(() => ({
onClick: jest.fn(),
href: '/ftw/app/r?l=ASSET_DETAILS_LOCATOR&v=8.15.0&lz=N4IghgzhCmAuAicwEsA2EQC5gF8A0IA%2BmFqLMgLbSkgBmATgPYVYgCMA7GwCwAMArADZB3NgCZBADhAFYjVpx69BvMaInSc%2BcFDgAVAJ4AHaphAALRhFgydMWAEkAJqwBUt62FinQjesgBzZAA7AEEjI2dWKlh%2FAGMMAj9AkIBlaDB6OPNWAH4Y%2BIgAUQAPI1Q%2FaHoAXgAKbMzYAHkjckZgiExazziAa0wAQlo8WGNoTFQQ6DwDUJLkCABZRidxhmYALSrGAEo8Rlbkds7asACA%2BmgAryPgzDAANwC8AuQEwdrT88vrtrvH55xRgVeiYIEg3h4WjIaCoJyYCAGazQCgAOjiRgArqi5LAwKhUcE%2FGijHFYHsvhcrjd2vcnnhwX4wcC%2FGwoTC4ZhepiAEZVYJwaAQVFGFborGozEQM7QQkrWWk8l4Sk%2FGn%2FemM0GasTs2HwpyMPpVcXY3H4kVknZ7CCMTFZcarWhgTGoJXkKj0MDBALjWrrCiYIkAdwAtGxzHgQt56A98ZgAKQAZiKSfgbF4EBGjEDjCDVoAZK8EqVypV6AA1GFB5zVADkvFrtmSQWCAAUvOZgmAqKwAPTQMogqogLRAA%3D',
})),
})),
})),
}));
})
);

describe('MetricsExplorerChartContextMenu', () => {
describe('component', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import DateMath from '@kbn/datemath';
import { Capabilities } from '@kbn/core/public';
import { useLinkProps } from '@kbn/observability-shared-plugin/public';
import { InventoryItemType } from '@kbn/metrics-data-access-plugin/common';
import { useAssetDetailsRedirect } from '@kbn/metrics-data-access-plugin/public';
import { useMetricsDataViewContext } from '../../../../containers/metrics_source';
import { AlertFlyout } from '../../../../alerting/metric_threshold/components/alert_flyout';
import { MetricsExplorerSeries } from '../../../../../common/http_api/metrics_explorer';
Expand All @@ -27,7 +28,6 @@ import {
MetricsExplorerChartOptions,
} from '../hooks/use_metrics_explorer_options';
import { createTSVBLink, TSVB_WORKAROUND_INDEX_PATTERN } from './helpers/create_tsvb_link';
import { useNodeDetailsRedirect } from '../../../link_to';
import {
HOST_NAME_FIELD,
KUBERNETES_POD_UID_FIELD,
Expand Down Expand Up @@ -70,7 +70,7 @@ export const MetricsExplorerChartContextMenu: React.FC<Props> = ({
uiCapabilities,
chartOptions,
}: Props) => {
const { getNodeDetailUrl } = useNodeDetailsRedirect();
const { getAssetDetailUrl } = useAssetDetailsRedirect();
const [isPopoverOpen, setPopoverState] = useState(false);
const [flyoutVisible, setFlyoutVisible] = useState(false);
const { metricsView } = useMetricsDataViewContext();
Expand Down Expand Up @@ -107,7 +107,7 @@ export const MetricsExplorerChartContextMenu: React.FC<Props> = ({
const nodeType = options.groupBy && fieldToNodeType(options.groupBy);

const nodeDetailLinkProps = nodeType
? getNodeDetailUrl({
? getAssetDetailUrl({
assetType: nodeType,
assetId: series.id,
search: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ import { createLazyContainerMetricsTable } from './create_lazy_container_metrics
import IntegratedContainerMetricsTable from './integrated_container_metrics_table';
import { metricByField } from './use_container_metrics_table';

jest.mock('../../../pages/link_to/use_node_details_redirect', () => ({
useNodeDetailsRedirect: jest.fn(() => ({
getNodeDetailUrl: jest.fn(() => ({
jest.mock('../../../pages/link_to/use_asset_details_redirect', () => ({
useAssetDetailsRedirect: jest.fn(() => ({
getAssetDetailUrl: jest.fn(() => ({
app: 'metrics',
pathname: 'link-to/container-detail/example-01',
search: { from: '1546340400000', to: '1546344000000' },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ import { HostMetricsTable } from './host_metrics_table';
import IntegratedHostMetricsTable from './integrated_host_metrics_table';
import { metricByField } from './use_host_metrics_table';

jest.mock('../../../pages/link_to/use_node_details_redirect', () => ({
useNodeDetailsRedirect: jest.fn(() => ({
getNodeDetailUrl: jest.fn(() => ({
jest.mock('../../../pages/link_to/use_asset_details_redirect', () => ({
useAssetDetailsRedirect: jest.fn(() => ({
getAssetDetailUrl: jest.fn(() => ({
app: 'metrics',
pathname: 'link-to/host-detail/example-01',
search: { from: '1546340400000', to: '1546344000000' },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ import IntegratedPodMetricsTable from './integrated_pod_metrics_table';
import { PodMetricsTable } from './pod_metrics_table';
import { metricByField } from './use_pod_metrics_table';

jest.mock('../../../pages/link_to/use_node_details_redirect', () => ({
useNodeDetailsRedirect: jest.fn(() => ({
getNodeDetailUrl: jest.fn(() => ({
jest.mock('../../../pages/link_to/use_asset_details_redirect', () => ({
useAssetDetailsRedirect: jest.fn(() => ({
getAssetDetailUrl: jest.fn(() => ({
app: 'metrics',
pathname: 'link-to/pod-detail/example-01',
search: { from: '1546340400000', to: '1546344000000' },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { parse } from '@kbn/datemath';
import { EuiLink } from '@elastic/eui';
import React from 'react';
import type { InventoryItemType } from '../../../../../common/inventory_models/types';
import { useNodeDetailsRedirect } from '../../../../pages/link_to/use_node_details_redirect';
import { useAssetDetailsRedirect } from '../../../../pages/link_to/use_asset_details_redirect';

type ExtractStrict<T, U extends T> = Extract<T, U>;

Expand All @@ -26,11 +26,12 @@ export const MetricsNodeDetailsLink = ({
nodeType,
timerange,
}: MetricsNodeDetailsLinkProps) => {
const { getNodeDetailUrl } = useNodeDetailsRedirect();
const linkProps = getNodeDetailUrl({
nodeType,
nodeId: id,
const { getAssetDetailUrl } = useAssetDetailsRedirect();
const linkProps = getAssetDetailUrl({
assetType: nodeType,
assetId: id,
search: {
name: label,
from: parse(timerange.from)?.valueOf(),
to: parse(timerange.to)?.valueOf(),
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@

import { PluginInitializer, PluginInitializerContext } from '@kbn/core/public';
import { Plugin } from './plugin';
import { MetricsDataPluginSetup, MetricsDataPluginStart } from './types';
import { MetricsDataPluginSetup, MetricsDataPluginStart, RouteState } from './types';
import { useAssetDetailsRedirect } from './pages/link_to';

export const plugin: PluginInitializer<MetricsDataPluginSetup, MetricsDataPluginStart> = (
context: PluginInitializerContext
Expand All @@ -16,3 +17,7 @@ export const plugin: PluginInitializer<MetricsDataPluginSetup, MetricsDataPlugin
};

export type { MetricsDataPluginSetup, MetricsDataPluginStart };

export type { RouteState };

export { useAssetDetailsRedirect };
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@
* 2.0.
*/

export { useNodeDetailsRedirect } from './use_node_details_redirect';
export { useAssetDetailsRedirect } from './use_asset_details_redirect';
Loading

0 comments on commit 9a2566b

Please sign in to comment.