Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ML] Transforms: Improves data view checks #181892

Merged
merged 12 commits into from
May 13, 2024
1 change: 1 addition & 0 deletions x-pack/plugins/transform/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export const addExternalBasePath = (uri: string): string => `${EXTERNAL_API_BASE
export const TRANSFORM_REACT_QUERY_KEYS = {
DATA_SEARCH: 'transform.data_search',
DATA_VIEW_EXISTS: 'transform.data_view_exists',
GET_DATA_VIEW_IDS_WITH_TITLE: 'transform.get_data_view_ids_with_title',
GET_DATA_VIEW_TITLES: 'transform.get_data_view_titles',
GET_ES_INDICES: 'transform.get_es_indices',
GET_ES_INGEST_PIPELINES: 'transform.get_es_ingest_pipelines',
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/transform/public/app/hooks/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

export { useCreateTransform } from './use_create_transform';
export { useDocumentationLinks } from './use_documentation_links';
export { useGetDataViewIdsWithTitle } from './use_get_data_view_ids_with_title';
export { useGetDataViewTitles } from './use_get_data_view_titles';
export { useGetEsIndices } from './use_get_es_indices';
export { useGetEsIngestPipelines } from './use_get_es_ingest_pipelines';
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { useQuery } from '@tanstack/react-query';

import type { IHttpFetchError } from '@kbn/core-http-browser';
import type { DataViewListItem } from '@kbn/data-views-plugin/public';

import { TRANSFORM_REACT_QUERY_KEYS } from '../../../common/constants';

import { useAppDependencies } from '../app_dependencies';

export const useGetDataViewIdsWithTitle = () => {
const { data } = useAppDependencies();

return useQuery<DataViewListItem[], IHttpFetchError>(
[TRANSFORM_REACT_QUERY_KEYS.GET_DATA_VIEW_IDS_WITH_TITLE],
async () => {
// Since we let useQuery take care of caching,
// clear the cache to ensure we get the latest data view list.
await data.dataViews.clearCache();
return await data.dataViews.getIdsWithTitle();
}
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,11 @@
import { buildEsQuery } from '@kbn/es-query';
import type { IUiSettingsClient } from '@kbn/core/public';
import { getEsQueryConfig } from '@kbn/data-plugin/public';
import type { DataView, DataViewsContract } from '@kbn/data-views-plugin/public';
import type { DataView } from '@kbn/data-views-plugin/public';
import { matchAllQuery } from '@kbn/ml-query-utils';

import { isDataView } from '../../../../common/types/data_view';

let dataViewCache: DataView[] = [];

export let refreshDataViews: () => Promise<unknown>;

export async function loadDataViews(dataViewsContract: DataViewsContract) {
dataViewCache = await dataViewsContract.find('*', 10000);
return dataViewCache;
}

export function getDataViewIdByTitle(dataViewTitle: string): string | undefined {
return dataViewCache.find(({ title }) => title === dataViewTitle)?.id;
}

type CombinedQuery = Record<'bool', any> | object;

export interface SearchItems {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { i18n } from '@kbn/i18n';
import { isDataView } from '../../../../common/types/data_view';
import { useAppDependencies } from '../../app_dependencies';
import type { SearchItems } from './common';
import { createSearchItems, getDataViewIdByTitle, loadDataViews } from './common';
import { createSearchItems } from './common';

export const useSearchItems = (defaultSavedObjectId: string | undefined) => {
const [savedObjectId, setSavedObjectId] = useState(defaultSavedObjectId);
Expand Down Expand Up @@ -73,8 +73,6 @@ export const useSearchItems = (defaultSavedObjectId: string | undefined) => {

return {
error,
getDataViewIdByTitle,
loadDataViews,
searchItems,
setSavedObjectId,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,32 +11,32 @@ import { i18n } from '@kbn/i18n';

import type { TransformListAction, TransformListRow } from '../../../../common';
import { SECTION_SLUG } from '../../../../common/constants';
import { useTransformCapabilities, useSearchItems } from '../../../../hooks';
import { useAppDependencies, useToastNotifications } from '../../../../app_dependencies';
import { useGetDataViewIdsWithTitle, useTransformCapabilities } from '../../../../hooks';
import { useToastNotifications } from '../../../../app_dependencies';

import { cloneActionNameText, CloneActionName } from './clone_action_name';

export type CloneAction = ReturnType<typeof useCloneAction>;
export const useCloneAction = (forceDisable: boolean, transformNodes: number) => {
const history = useHistory();
const appDeps = useAppDependencies();
const dataViewsContract = appDeps.data.dataViews;
const toastNotifications = useToastNotifications();

const { getDataViewIdByTitle, loadDataViews } = useSearchItems(undefined);

const { data: dataViewListItems } = useGetDataViewIdsWithTitle();
const { canCreateTransform } = useTransformCapabilities();

const clickHandler = useCallback(
async (item: TransformListRow) => {
try {
await loadDataViews(dataViewsContract);
if (!dataViewListItems) {
return;
}

const dataViewTitle = Array.isArray(item.config.source.index)
? item.config.source.index.join(',')
: item.config.source.index;
const dataViewId = getDataViewIdByTitle(dataViewTitle);
const dataViewListItem = dataViewListItems.find((d) => d.title === dataViewTitle);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Each of the uses of dataViewListItems runs a find, these won't be too expensive, but it might be slightly more efficient if useGetDataViewIdsWithTitle returned a map of data views, keyed on title

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Updated the hook to return a map, also renamed everything to be in line with the new returned structure in 9f23cb8.


if (dataViewId === undefined) {
if (dataViewListItem === undefined) {
toastNotifications.addDanger(
i18n.translate('xpack.transform.clone.noDataViewErrorPromptText', {
defaultMessage:
Expand All @@ -45,7 +45,9 @@ export const useCloneAction = (forceDisable: boolean, transformNodes: number) =>
})
);
} else {
history.push(`/${SECTION_SLUG.CLONE_TRANSFORM}/${item.id}?dataViewId=${dataViewId}`);
history.push(
`/${SECTION_SLUG.CLONE_TRANSFORM}/${item.id}?dataViewId=${dataViewListItem.id}`
);
}
} catch (e) {
toastNotifications.addError(e, {
Expand All @@ -55,20 +57,24 @@ export const useCloneAction = (forceDisable: boolean, transformNodes: number) =>
});
}
},
[history, dataViewsContract, toastNotifications, loadDataViews, getDataViewIdByTitle]
[dataViewListItems, history, toastNotifications]
);

const action: TransformListAction = useMemo(
() => ({
name: (item: TransformListRow) => <CloneActionName disabled={!canCreateTransform} />,
enabled: () => canCreateTransform && !forceDisable && transformNodes > 0,
enabled: () =>
dataViewListItems !== undefined &&
canCreateTransform &&
!forceDisable &&
transformNodes > 0,
description: cloneActionNameText,
icon: 'copy',
type: 'icon',
onClick: clickHandler,
'data-test-subj': 'transformActionClone',
}),
[canCreateTransform, forceDisable, clickHandler, transformNodes]
[canCreateTransform, dataViewListItems, forceDisable, clickHandler, transformNodes]
);

return { action };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@
* 2.0.
*/

import React, { useCallback, useEffect, useMemo, useState } from 'react';
import React, { useCallback, useMemo } from 'react';

import { DISCOVER_APP_LOCATOR } from '@kbn/discover-plugin/common';
import type { TransformListAction, TransformListRow } from '../../../../common';

import { useSearchItems } from '../../../../hooks/use_search_items';
import { useAppDependencies } from '../../../../app_dependencies';
import { useGetDataViewIdsWithTitle } from '../../../../hooks';

import {
isDiscoverActionDisabled,
Expand All @@ -23,42 +23,36 @@ export type DiscoverAction = ReturnType<typeof useDiscoverAction>;
export const useDiscoverAction = (forceDisable: boolean) => {
const {
share,
data: { dataViews: dataViewsContract },
application: { capabilities },
} = useAppDependencies();
const isDiscoverAvailable = !!capabilities.discover?.show;

const { getDataViewIdByTitle, loadDataViews } = useSearchItems(undefined);

const [dataViewsLoaded, setDataViewsLoaded] = useState(false);

useEffect(() => {
async function checkDataViewAvailability() {
await loadDataViews(dataViewsContract);
setDataViewsLoaded(true);
}

checkDataViewAvailability();
}, [loadDataViews, dataViewsContract]);
const { data: dataViewListItems } = useGetDataViewIdsWithTitle();

const clickHandler = useCallback(
(item: TransformListRow) => {
const locator = share.url.locators.get(DISCOVER_APP_LOCATOR);
if (!locator) return;
const dataViewId = getDataViewIdByTitle(item.config.dest.index);
locator.navigateSync({
indexPatternId: dataViewId,
});
if (!locator || !dataViewListItems) return;
const dataView = dataViewListItems.find((d) => d.title === item.config.dest.index);
if (dataView) {
locator.navigateSync({
indexPatternId: dataView.id,
});
}
},
[getDataViewIdByTitle, share]
[dataViewListItems, share]
);

const dataViewExists = useCallback(
(item: TransformListRow) => {
const dataViewId = getDataViewIdByTitle(item.config.dest.index);
(item: TransformListRow): boolean => {
if (!dataViewListItems) {
return false;
}

const dataViewId = dataViewListItems.find((d) => d.title === item.config.dest.index);
return dataViewId !== undefined;
},
[getDataViewIdByTitle]
[dataViewListItems]
);

const action: TransformListAction = useMemo(
Expand All @@ -68,14 +62,14 @@ export const useDiscoverAction = (forceDisable: boolean) => {
},
available: () => isDiscoverAvailable,
enabled: (item: TransformListRow) =>
dataViewsLoaded && !isDiscoverActionDisabled([item], forceDisable, dataViewExists(item)),
!isDiscoverActionDisabled([item], forceDisable, dataViewExists(item)),
description: discoverActionNameText,
icon: 'visTable',
type: 'icon',
onClick: clickHandler,
'data-test-subj': 'transformActionDiscover',
}),
[forceDisable, dataViewExists, dataViewsLoaded, isDiscoverAvailable, clickHandler]
[forceDisable, dataViewExists, isDiscoverAvailable, clickHandler]
);

return { action };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@ import React, { useCallback, useMemo, useState } from 'react';
import { i18n } from '@kbn/i18n';

import type { TransformListAction, TransformListRow } from '../../../../common';
import { useTransformCapabilities } from '../../../../hooks';
import { useGetDataViewIdsWithTitle, useTransformCapabilities } from '../../../../hooks';

import { editActionNameText, EditActionName } from './edit_action_name';
import { useSearchItems } from '../../../../hooks/use_search_items';
import { useAppDependencies, useToastNotifications } from '../../../../app_dependencies';
import { useToastNotifications } from '../../../../app_dependencies';
import type { TransformConfigUnion } from '../../../../../../common/types/transform';

export type EditAction = ReturnType<typeof useEditAction>;
export const useEditAction = (forceDisable: boolean, transformNodes: number) => {
const { data: dataViewListItems } = useGetDataViewIdsWithTitle();
const { canCreateTransform } = useTransformCapabilities();

const [config, setConfig] = useState<TransformConfigUnion>();
Expand All @@ -27,29 +27,31 @@ export const useEditAction = (forceDisable: boolean, transformNodes: number) =>

const closeFlyout = () => setIsFlyoutVisible(false);

const { getDataViewIdByTitle } = useSearchItems(undefined);
const toastNotifications = useToastNotifications();
const appDeps = useAppDependencies();
const dataViews = appDeps.data.dataViews;

const clickHandler = useCallback(
async (item: TransformListRow) => {
try {
if (!dataViewListItems) {
return;
}

const dataViewTitle = Array.isArray(item.config.source.index)
? item.config.source.index.join(',')
: item.config.source.index;
const currentDataViewId = getDataViewIdByTitle(dataViewTitle);
const dataViewListItem = dataViewListItems.find((d) => d.title === dataViewTitle);

if (currentDataViewId === undefined) {
if (dataViewListItem === undefined) {
toastNotifications.addWarning(
i18n.translate('xpack.transform.edit.noDataViewErrorPromptText', {
defaultMessage:
'Unable to get the data view for the transform {transformId}. No data view exists for {dataViewTitle}.',
values: { dataViewTitle, transformId: item.id },
})
);
} else {
setDataViewId(dataViewListItem.id);
}
setDataViewId(currentDataViewId);
setConfig(item.config);
setIsFlyoutVisible(true);
} catch (e) {
Expand All @@ -60,8 +62,7 @@ export const useEditAction = (forceDisable: boolean, transformNodes: number) =>
});
}
},
// eslint-disable-next-line react-hooks/exhaustive-deps
[dataViews, toastNotifications, getDataViewIdByTitle]
[dataViewListItems, toastNotifications]
);

const action: TransformListAction = useMemo(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ describe('Transform: Transform List <TransformList />', () => {
);

await waitFor(() => {
expect(useQueryMock).toHaveBeenCalledTimes(4);
expect(useQueryMock).toHaveBeenCalledTimes(5);
expect(container.textContent).toContain('Create your first transform');
});
});
Expand Down