From ecffd1679b2e41be1785f6b0e88b75762bb434cc Mon Sep 17 00:00:00 2001 From: Claire Lin Date: Mon, 11 Mar 2024 11:08:39 -0700 Subject: [PATCH] handle asset search fork in hook --- .../packages/ui-core/src/app/AppTopNav.tsx | 2 +- .../ui-core/src/search/SearchDialog.tsx | 30 ++--- .../ui-core/src/search/SearchResults.tsx | 4 +- .../packages/ui-core/src/search/types.ts | 11 -- .../ui-core/src/search/useGlobalSearch.tsx | 126 +++++++++--------- 5 files changed, 78 insertions(+), 95 deletions(-) diff --git a/js_modules/dagster-ui/packages/ui-core/src/app/AppTopNav.tsx b/js_modules/dagster-ui/packages/ui-core/src/app/AppTopNav.tsx index 462ed93ae8caa..84265df6a20a7 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/app/AppTopNav.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/app/AppTopNav.tsx @@ -180,7 +180,7 @@ export const AppTopNav = ({
) : null} - + {children} diff --git a/js_modules/dagster-ui/packages/ui-core/src/search/SearchDialog.tsx b/js_modules/dagster-ui/packages/ui-core/src/search/SearchDialog.tsx index 3ea67d200779e..8d72f75e363e1 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/search/SearchDialog.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/search/SearchDialog.tsx @@ -8,7 +8,7 @@ import {useHistory} from 'react-router-dom'; import styled from 'styled-components'; import {SearchResults} from './SearchResults'; -import {SearchResult, SearchResultType} from './types'; +import {SearchResult} from './types'; import {useGlobalSearch} from './useGlobalSearch'; import {__updateSearchVisibility} from './useSearchVisibility'; import {ShortcutHandler} from '../app/ShortcutHandler'; @@ -72,15 +72,11 @@ const initialState: State = { const DEBOUNCE_MSEC = 100; -export const SearchDialog = ({ - searchPlaceholder, - isAssetSearch, -}: { - searchPlaceholder: string; - isAssetSearch: boolean; -}) => { +export const SearchDialog = ({searchPlaceholder}: {searchPlaceholder: string}) => { const history = useHistory(); - const {initialize, loading, searchPrimary, searchSecondary} = useGlobalSearch(); + const {initialize, loading, searchPrimary, searchSecondary} = useGlobalSearch({ + includeAssetFilters: false, + }); const trackEvent = useTrackEvent(); const [state, dispatch] = React.useReducer(reducer, initialState); @@ -124,17 +120,9 @@ export const SearchDialog = ({ ); const searchAndHandleSecondary = React.useCallback( - async (queryString: string, returnAssetFilters: boolean) => { + async (queryString: string) => { const {queryString: queryStringForResults, results} = await searchSecondary(queryString); - if (!returnAssetFilters) { - dispatch({ - type: 'complete-secondary', - queryString: queryStringForResults, - results: results.filter((result) => result.item.type === SearchResultType.Asset), // Only return asset results - }); - } else { - dispatch({type: 'complete-secondary', queryString: queryStringForResults, results}); - } + dispatch({type: 'complete-secondary', queryString: queryStringForResults, results}); }, [searchSecondary], ); @@ -142,7 +130,7 @@ export const SearchDialog = ({ const debouncedSearch = React.useMemo(() => { const debouncedSearch = debounce(async (queryString: string) => { searchAndHandlePrimary(queryString); - searchAndHandleSecondary(queryString, isAssetSearch); + searchAndHandleSecondary(queryString); }, DEBOUNCE_MSEC); return (queryString: string) => { if (!firstSearchTrace.current && isFirstSearch.current) { @@ -153,7 +141,7 @@ export const SearchDialog = ({ } return debouncedSearch(queryString); }; - }, [searchAndHandlePrimary, searchAndHandleSecondary, isAssetSearch]); + }, [searchAndHandlePrimary, searchAndHandleSecondary]); const onChange = async (e: React.ChangeEvent) => { const newValue = e.target.value; diff --git a/js_modules/dagster-ui/packages/ui-core/src/search/SearchResults.tsx b/js_modules/dagster-ui/packages/ui-core/src/search/SearchResults.tsx index 0684f90835839..5d78c7a743b7e 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/search/SearchResults.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/search/SearchResults.tsx @@ -4,9 +4,9 @@ import * as React from 'react'; import {Link} from 'react-router-dom'; import styled from 'styled-components'; -import {SearchResult, SearchResultType} from './types'; +import {AssetFilterSearchResultType, SearchResult, SearchResultType} from './types'; -const iconForType = (type: SearchResultType): IconName => { +const iconForType = (type: SearchResultType | AssetFilterSearchResultType): IconName => { switch (type) { case SearchResultType.Asset: return 'asset'; diff --git a/js_modules/dagster-ui/packages/ui-core/src/search/types.ts b/js_modules/dagster-ui/packages/ui-core/src/search/types.ts index 7016114c04d5c..0d466b84570e3 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/search/types.ts +++ b/js_modules/dagster-ui/packages/ui-core/src/search/types.ts @@ -23,17 +23,6 @@ export enum AssetFilterSearchResultType { AssetGroup = 'AssetFilterSearchResultType.AssetGroup', } -export function isAssetFilterSearchResultType( - type: SearchResultType | AssetFilterSearchResultType, -): type is AssetFilterSearchResultType { - return ( - type === AssetFilterSearchResultType.AssetGroup || - type === AssetFilterSearchResultType.CodeLocation || - type === AssetFilterSearchResultType.ComputeKind || - type === AssetFilterSearchResultType.Owner - ); -} - export type SearchResult = { label: string; description: string; diff --git a/js_modules/dagster-ui/packages/ui-core/src/search/useGlobalSearch.tsx b/js_modules/dagster-ui/packages/ui-core/src/search/useGlobalSearch.tsx index 8d5884d2ca58d..0f25e9a51ca8f 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/search/useGlobalSearch.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/search/useGlobalSearch.tsx @@ -141,7 +141,10 @@ const primaryDataToSearchResults = (input: {data?: SearchPrimaryQuery}) => { return allEntries; }; -const secondaryDataToSearchResults = (input: {data?: SearchSecondaryQuery}) => { +const secondaryDataToSearchResults = ( + input: {data?: SearchSecondaryQuery}, + includeAssetFilters: boolean, +) => { const {data} = input; if (!data?.assetsOrError || data.assetsOrError.__typename === 'PythonError') { return []; @@ -149,55 +152,6 @@ const secondaryDataToSearchResults = (input: {data?: SearchSecondaryQuery}) => { const {nodes} = data.assetsOrError; - const countsBySection = buildAssetCountBySection(nodes); - - const computeKindResults: SearchResult[] = Object.entries( - countsBySection.countsByComputeKind, - ).map(([computeKind, count]) => ({ - label: computeKind, - description: '', - type: AssetFilterSearchResultType.ComputeKind, - // TODO display correct link once https://github.com/dagster-io/dagster/pull/20342 lands - href: '/assets', - numResults: count, - })); - - const codeLocationResults: SearchResult[] = countsBySection.countPerCodeLocation.map( - (codeLocationAssetCount) => ({ - label: buildRepoPathForHuman( - codeLocationAssetCount.repoAddress.name, - codeLocationAssetCount.repoAddress.location, - ), - description: '', - type: AssetFilterSearchResultType.CodeLocation, - // TODO display correct link once https://github.com/dagster-io/dagster/pull/20342 lands - href: '/assets', - numResults: codeLocationAssetCount.assetCount, - }), - ); - - const groupResults: SearchResult[] = countsBySection.countPerAssetGroup.map( - (groupAssetCount) => ({ - label: groupAssetCount.groupMetadata.groupName, - description: '', - type: AssetFilterSearchResultType.AssetGroup, - // TODO display correct link once https://github.com/dagster-io/dagster/pull/20342 lands - href: '/assets', - numResults: groupAssetCount.assetCount, - }), - ); - - const ownerResults: SearchResult[] = Object.entries(countsBySection.countsByOwner).map( - ([owner, count]) => ({ - label: owner, - description: '', - type: AssetFilterSearchResultType.Owner, - // TODO display correct link once https://github.com/dagster-io/dagster/pull/20342 lands - href: '/assets', - numResults: count, - }), - ); - const assets = nodes .filter(({definition}) => definition !== null) .map(({key, definition}) => { @@ -213,13 +167,65 @@ const secondaryDataToSearchResults = (input: {data?: SearchSecondaryQuery}) => { }; }); - return [ - ...assets, - ...computeKindResults, - ...codeLocationResults, - ...ownerResults, - ...groupResults, - ]; + if (!includeAssetFilters) { + return [...assets]; + } else { + const countsBySection = buildAssetCountBySection(nodes); + + const computeKindResults: SearchResult[] = Object.entries( + countsBySection.countsByComputeKind, + ).map(([computeKind, count]) => ({ + label: computeKind, + description: '', + type: AssetFilterSearchResultType.ComputeKind, + // TODO display correct link once https://github.com/dagster-io/dagster/pull/20342 lands + href: '/assets', + numResults: count, + })); + + const codeLocationResults: SearchResult[] = countsBySection.countPerCodeLocation.map( + (codeLocationAssetCount) => ({ + label: buildRepoPathForHuman( + codeLocationAssetCount.repoAddress.name, + codeLocationAssetCount.repoAddress.location, + ), + description: '', + type: AssetFilterSearchResultType.CodeLocation, + // TODO display correct link once https://github.com/dagster-io/dagster/pull/20342 lands + href: '/assets', + numResults: codeLocationAssetCount.assetCount, + }), + ); + + const groupResults: SearchResult[] = countsBySection.countPerAssetGroup.map( + (groupAssetCount) => ({ + label: groupAssetCount.groupMetadata.groupName, + description: '', + type: AssetFilterSearchResultType.AssetGroup, + // TODO display correct link once https://github.com/dagster-io/dagster/pull/20342 lands + href: '/assets', + numResults: groupAssetCount.assetCount, + }), + ); + + const ownerResults: SearchResult[] = Object.entries(countsBySection.countsByOwner).map( + ([owner, count]) => ({ + label: owner, + description: '', + type: AssetFilterSearchResultType.Owner, + // TODO display correct link once https://github.com/dagster-io/dagster/pull/20342 lands + href: '/assets', + numResults: count, + }), + ); + return [ + ...assets, + ...computeKindResults, + ...codeLocationResults, + ...ownerResults, + ...groupResults, + ]; + } }; const fuseOptions = { @@ -250,7 +256,7 @@ type IndexBuffer = { * * A `terminate` function is provided, but it's probably not necessary to use it. */ -export const useGlobalSearch = () => { +export const useGlobalSearch = ({includeAssetFilters}: {includeAssetFilters: boolean}) => { const primarySearch = useRef(null); const secondarySearch = useRef(null); @@ -300,13 +306,13 @@ export const useGlobalSearch = () => { if (!secondaryData) { return; } - const results = secondaryDataToSearchResults({data: secondaryData}); + const results = secondaryDataToSearchResults({data: secondaryData}, includeAssetFilters); if (!secondarySearch.current) { secondarySearch.current = createSearchWorker('secondary', fuseOptions); } secondarySearch.current.update(results); consumeBufferEffect(secondarySearchBuffer, secondarySearch.current); - }, [consumeBufferEffect, secondaryData]); + }, [consumeBufferEffect, secondaryData, includeAssetFilters]); const primarySearchBuffer = useRef(null); const secondarySearchBuffer = useRef(null);