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: Improve 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 { useGetDataViewsTitleIdMap } from './use_get_data_views_title_id_map';
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,33 @@
/*
* 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 { TRANSFORM_REACT_QUERY_KEYS } from '../../../common/constants';

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

type DataViewListTitleIdMap = Record<string, string>;

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

return useQuery<DataViewListTitleIdMap, IHttpFetchError>(
[TRANSFORM_REACT_QUERY_KEYS.GET_DATA_VIEW_IDS_WITH_TITLE],
async () => {
return (await data.dataViews.getIdsWithTitle(true)).reduce<Record<string, string>>(
(acc, { id, title }) => {
acc[title] = id;
return acc;
},
{}
);
}
);
};
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,30 +11,30 @@ 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 { useGetDataViewsTitleIdMap, 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: dataViewsTitleIdMap } = useGetDataViewsTitleIdMap();
Copy link
Member

Choose a reason for hiding this comment

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

Total nit pick. but rather than having to rename data -> dataViewsTitleIdMap on every use, could it be renamed when being returned from useGetDataViewsTitleIdMap ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This hook wraps useQuery so that whole returned object comes from it and data is part of its standard API (like isLoading, error etc.).

const { canCreateTransform } = useTransformCapabilities();

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

const dataViewTitle = Array.isArray(item.config.source.index)
? item.config.source.index.join(',')
: item.config.source.index;
const dataViewId = getDataViewIdByTitle(dataViewTitle);
const dataViewId = dataViewsTitleIdMap[dataViewTitle];

if (dataViewId === undefined) {
toastNotifications.addDanger(
Expand All @@ -55,20 +55,24 @@ export const useCloneAction = (forceDisable: boolean, transformNodes: number) =>
});
}
},
[history, dataViewsContract, toastNotifications, loadDataViews, getDataViewIdByTitle]
[dataViewsTitleIdMap, history, toastNotifications]
);

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

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 || !dataViewsTitleIdMap) return;
const dataViewId = dataViewsTitleIdMap[item.config.dest.index];
if (dataViewId) {
locator.navigateSync({
indexPatternId: dataViewId,
});
}
},
[getDataViewIdByTitle, share]
[dataViewsTitleIdMap, share]
);

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

const dataViewId = dataViewsTitleIdMap[item.config.dest.index];
return dataViewId !== undefined;
},
[getDataViewIdByTitle]
[dataViewsTitleIdMap]
);

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 { useGetDataViewsTitleIdMap, 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: dataViewsTitleIdMap } = useGetDataViewsTitleIdMap();
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 (!dataViewsTitleIdMap) {
return;
}

const dataViewTitle = Array.isArray(item.config.source.index)
? item.config.source.index.join(',')
: item.config.source.index;
const currentDataViewId = getDataViewIdByTitle(dataViewTitle);
const newDataViewId = dataViewsTitleIdMap[dataViewTitle];

if (currentDataViewId === undefined) {
if (newDataViewId === 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(newDataViewId);
}
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]
[dataViewsTitleIdMap, 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