From b45a8e0866f334c1468cfdf9d3a7b0076032ca92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loix?= Date: Thu, 24 Oct 2024 09:10:49 +0100 Subject: [PATCH] [TableListView] Hint message for chars not allowed in search (#197307) --- .../table_list_view_table/src/actions.ts | 6 +- .../src/components/index.ts | 2 +- .../src/components/table.tsx | 30 +++++++- .../table_list_view_table/src/reducer.tsx | 13 +++- .../src/table_list_view.test.tsx | 73 +++++++++++-------- .../src/table_list_view_table.tsx | 45 +++++++++--- .../table_list_view_table/src/types.ts | 7 ++ .../table_list_view_table/src/use_tags.ts | 8 +- 8 files changed, 131 insertions(+), 53 deletions(-) diff --git a/packages/content-management/table_list_view_table/src/actions.ts b/packages/content-management/table_list_view_table/src/actions.ts index ee2e96aadeb07..17f932f0e7b6d 100644 --- a/packages/content-management/table_list_view_table/src/actions.ts +++ b/packages/content-management/table_list_view_table/src/actions.ts @@ -11,6 +11,7 @@ import type { IHttpFetchError } from '@kbn/core-http-browser'; import type { Query } from '@elastic/eui'; import type { UserContentCommonSchema } from '@kbn/content-management-table-list-view-common'; import type { State } from './table_list_view_table'; +import type { SearchQueryError } from './types'; /** Action to trigger a fetch of the table items */ export interface OnFetchItemsAction { @@ -72,8 +73,9 @@ export interface ShowConfirmDeleteItemsModalAction { export interface OnSearchQueryChangeAction { type: 'onSearchQueryChange'; data: { - query: Query; - text: string; + query?: Query; + text?: string; + error: SearchQueryError | null; }; } diff --git a/packages/content-management/table_list_view_table/src/components/index.ts b/packages/content-management/table_list_view_table/src/components/index.ts index f3024081cd58e..beca60ccd1668 100644 --- a/packages/content-management/table_list_view_table/src/components/index.ts +++ b/packages/content-management/table_list_view_table/src/components/index.ts @@ -7,7 +7,7 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -export { Table } from './table'; +export { Table, FORBIDDEN_SEARCH_CHARS } from './table'; export { UpdatedAtField } from './updated_at_field'; export { ConfirmDeleteModal } from './confirm_delete_modal'; export { ListingLimitWarning } from './listing_limit_warning'; diff --git a/packages/content-management/table_list_view_table/src/components/table.tsx b/packages/content-management/table_list_view_table/src/components/table.tsx index 4a23f22ebf352..66c0eaec4d1f8 100644 --- a/packages/content-management/table_list_view_table/src/components/table.tsx +++ b/packages/content-management/table_list_view_table/src/components/table.tsx @@ -20,6 +20,8 @@ import { Search, type EuiTableSelectionType, useEuiTheme, + EuiCode, + EuiText, } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import type { UserContentCommonSchema } from '@kbn/content-management-table-list-view-common'; @@ -58,6 +60,8 @@ type TagManagementProps = Pick< 'addOrRemoveIncludeTagFilter' | 'addOrRemoveExcludeTagFilter' | 'tagsToTableItemMap' >; +export const FORBIDDEN_SEARCH_CHARS = '()[]{}<>+=\\"$#!¿?,;`\'/|&'; + interface Props extends State, TagManagementProps { dispatch: Dispatch>; entityName: string; @@ -219,6 +223,7 @@ export function Table({ }, [tableSortSelectFilter, tagFilterPanel, userFilterPanel]); const search = useMemo((): Search => { + const showHint = !!searchQuery.error && searchQuery.error.containsForbiddenChars; return { onChange: onTableSearchChange, toolsLeft: renderToolsLeft(), @@ -229,8 +234,31 @@ export function Table({ 'data-test-subj': 'tableListSearchBox', }, filters: searchFilters, + hint: { + content: ( + + {FORBIDDEN_SEARCH_CHARS}, + }} + /> + + ), + popoverProps: { + isOpen: showHint, + }, + }, }; - }, [onTableSearchChange, renderCreateButton, renderToolsLeft, searchFilters, searchQuery.query]); + }, [ + onTableSearchChange, + renderCreateButton, + renderToolsLeft, + searchFilters, + searchQuery.query, + searchQuery.error, + ]); const hasQueryOrFilters = Boolean(searchQuery.text || tableFilter.createdBy.length > 0); diff --git a/packages/content-management/table_list_view_table/src/reducer.tsx b/packages/content-management/table_list_view_table/src/reducer.tsx index 931b5225e71db..0ca05b97163a4 100644 --- a/packages/content-management/table_list_view_table/src/reducer.tsx +++ b/packages/content-management/table_list_view_table/src/reducer.tsx @@ -84,14 +84,21 @@ export function getReducer() { }; } case 'onSearchQueryChange': { - if (action.data.text === state.searchQuery.text) { + if ( + action.data.text === state.searchQuery.text && + action.data.error === state.searchQuery.error + ) { return state; } return { ...state, - searchQuery: action.data, - isFetchingItems: true, + searchQuery: { + ...state.searchQuery, + ...action.data, + }, + isFetchingItems: + action.data.error === null && action.data.text !== state.searchQuery.text, }; } case 'onTableChange': { diff --git a/packages/content-management/table_list_view_table/src/table_list_view.test.tsx b/packages/content-management/table_list_view_table/src/table_list_view.test.tsx index a4e06fbb1e4a4..38e05299184e4 100644 --- a/packages/content-management/table_list_view_table/src/table_list_view.test.tsx +++ b/packages/content-management/table_list_view_table/src/table_list_view.test.tsx @@ -1078,25 +1078,29 @@ describe('TableListView', () => { const findItems = jest.fn(); - const setupSearch = (...args: Parameters>) => { - const testBed = registerTestBed( - WithServices(TableListViewTable), - { - defaultProps: { - ...requiredProps, - findItems, - urlStateEnabled: false, - entityName: 'Foo', - entityNamePlural: 'Foos', - }, - memoryRouter: { wrapComponent: true }, - } - )(...args); + const setupSearch = async (...args: Parameters>) => { + let testBed: TestBed; - const { updateSearchText, getSearchBoxValue } = getActions(testBed); + await act(async () => { + testBed = registerTestBed( + WithServices(TableListViewTable), + { + defaultProps: { + ...requiredProps, + findItems, + urlStateEnabled: false, + entityName: 'Foo', + entityNamePlural: 'Foos', + }, + memoryRouter: { wrapComponent: true }, + } + )(...args); + }); + + const { updateSearchText, getSearchBoxValue } = getActions(testBed!); return { - testBed, + testBed: testBed!, updateSearchText, getSearchBoxValue, getLastCallArgsFromFindItems: () => findItems.mock.calls[findItems.mock.calls.length - 1], @@ -1108,15 +1112,8 @@ describe('TableListView', () => { }); test('should search the table items', async () => { - let testBed: TestBed; - let updateSearchText: (value: string) => Promise; - let getLastCallArgsFromFindItems: () => Parameters; - let getSearchBoxValue: () => string; - - await act(async () => { - ({ testBed, getLastCallArgsFromFindItems, getSearchBoxValue, updateSearchText } = - await setupSearch()); - }); + const { testBed, getLastCallArgsFromFindItems, getSearchBoxValue, updateSearchText } = + await setupSearch(); const { component, table } = testBed!; component.update(); @@ -1173,12 +1170,7 @@ describe('TableListView', () => { }); test('should search and render empty list if no result', async () => { - let testBed: TestBed; - let updateSearchText: (value: string) => Promise; - - await act(async () => { - ({ testBed, updateSearchText } = await setupSearch()); - }); + const { testBed, updateSearchText } = await setupSearch(); const { component, table, find } = testBed!; component.update(); @@ -1217,6 +1209,25 @@ describe('TableListView', () => { ] `); }); + + test('should show error hint when inserting invalid chars', async () => { + const { testBed, getLastCallArgsFromFindItems, getSearchBoxValue, updateSearchText } = + await setupSearch(); + + const { component, exists } = testBed; + component.update(); + + expect(exists('forbiddenCharErrorMessage')).toBe(false); + + const expected = '[foo'; + await updateSearchText!(expected); + expect(getSearchBoxValue!()).toBe(expected); + + expect(exists('forbiddenCharErrorMessage')).toBe(true); // hint is shown + + const [searchTerm] = getLastCallArgsFromFindItems!(); + expect(searchTerm).toBe(''); // no search has been made + }); }); describe('url state', () => { diff --git a/packages/content-management/table_list_view_table/src/table_list_view_table.tsx b/packages/content-management/table_list_view_table/src/table_list_view_table.tsx index 0f6df7c81533b..1fe5123d54151 100644 --- a/packages/content-management/table_list_view_table/src/table_list_view_table.tsx +++ b/packages/content-management/table_list_view_table/src/table_list_view_table.tsx @@ -50,6 +50,7 @@ import { ListingLimitWarning, ItemDetails, UpdatedAtField, + FORBIDDEN_SEARCH_CHARS, } from './components'; import { useServices } from './services'; import type { SavedObjectsFindOptionsReference } from './services'; @@ -57,7 +58,7 @@ import { getReducer } from './reducer'; import { type SortColumnField, getInitialSorting, saveSorting } from './components'; import { useTags } from './use_tags'; import { useInRouterContext, useUrlState } from './use_url_state'; -import { RowActions, TableItemsRowActions } from './types'; +import type { RowActions, SearchQueryError, TableItemsRowActions } from './types'; import { sortByRecentlyAccessed } from './components/table_sort_select'; import { ContentEditorActivityRow } from './components/content_editor_activity_row'; @@ -146,6 +147,7 @@ export interface State({ hasCreatedByMetadata: false, hasRecentlyAccessedMetadata: recentlyAccessed ? recentlyAccessed.get().length > 0 : false, selectedIds: [], - searchQuery: { text: '', query: new Query(Ast.create([]), undefined, '') }, + searchQuery: { text: '', query: new Query(Ast.create([]), undefined, ''), error: null }, pagination: { pageIndex: 0, totalItemCount: 0, @@ -492,14 +496,14 @@ function TableListViewTableComp({ }, [searchQueryParser, searchQuery.text, findItems, onFetchSuccess, recentlyAccessed]); const updateQuery = useCallback( - (query: Query) => { - if (urlStateEnabled) { + (query: Query | null, error: SearchQueryError | null) => { + if (urlStateEnabled && query) { setUrlState({ s: query.text }); } dispatch({ type: 'onSearchQueryChange', - data: { query, text: query.text }, + data: query ? { query, text: query.text, error } : { error }, }); }, [urlStateEnabled, setUrlState] @@ -809,14 +813,32 @@ function TableListViewTableComp({ ); const onTableSearchChange = useCallback( - (arg: { query: Query | null; queryText: string }) => { - if (arg.query) { - updateQuery(arg.query); + (arg: { + query: Query | null; + queryText: string; + error?: { message: string; name: string }; + }) => { + const { query, queryText, error: _error } = arg; + + let error: SearchQueryError | null = null; + if (_error) { + const containsForbiddenChars = FORBIDDEN_SEARCH_CHARS_ARRAY.some((char) => + queryText.includes(char) + ); + error = { + ..._error, + queryText, + containsForbiddenChars, + }; + } + + if (query || error) { + updateQuery(query, error); } else { const idx = tableSearchChangeIdx.current + 1; - buildQueryFromText(arg.queryText).then((query) => { + buildQueryFromText(queryText).then((q) => { if (idx === tableSearchChangeIdx.current) { - updateQuery(query); + updateQuery(q, null); } }); } @@ -1036,6 +1058,7 @@ function TableListViewTableComp({ data: { query: updatedQuery, text, + error: null, }, }); }; @@ -1089,7 +1112,7 @@ function TableListViewTableComp({ useEffect(() => { if (initialQuery && !initialQueryInitialized.current) { initialQueryInitialized.current = true; - buildQueryFromText(initialQuery).then(updateQuery); + buildQueryFromText(initialQuery).then((q) => updateQuery(q, null)); } }, [initialQuery, buildQueryFromText, updateQuery]); diff --git a/packages/content-management/table_list_view_table/src/types.ts b/packages/content-management/table_list_view_table/src/types.ts index 0815aea627d38..a2c260454dfea 100644 --- a/packages/content-management/table_list_view_table/src/types.ts +++ b/packages/content-management/table_list_view_table/src/types.ts @@ -27,3 +27,10 @@ export type RowActions = { export interface TableItemsRowActions { [id: string]: RowActions | undefined; } + +export interface SearchQueryError { + message: string; + name: string; + queryText: string; + containsForbiddenChars: boolean; +} diff --git a/packages/content-management/table_list_view_table/src/use_tags.ts b/packages/content-management/table_list_view_table/src/use_tags.ts index 36f6a7ce54421..7e460ba5405fd 100644 --- a/packages/content-management/table_list_view_table/src/use_tags.ts +++ b/packages/content-management/table_list_view_table/src/use_tags.ts @@ -10,7 +10,7 @@ import { useCallback, useMemo } from 'react'; import { Query } from '@elastic/eui'; import type { UserContentCommonSchema } from '@kbn/content-management-table-list-view-common'; -import type { Tag } from './types'; +import type { SearchQueryError, Tag } from './types'; type QueryUpdater = (query: Query, tag: Tag) => Query; @@ -20,7 +20,7 @@ export function useTags({ items, }: { query: Query; - updateQuery: (query: Query) => void; + updateQuery: (query: Query, error: SearchQueryError | null) => void; items: UserContentCommonSchema[]; }) { // Return a map of tag.id to an array of saved object ids having that tag @@ -47,7 +47,7 @@ export function useTags({ (tag: Tag, q: Query = query, doUpdate: boolean = true) => { const updatedQuery = queryUpdater(q, tag); if (doUpdate) { - updateQuery(updatedQuery); + updateQuery(updatedQuery, null); } return updatedQuery; }, @@ -147,7 +147,7 @@ export function useTags({ const clearTagSelection = useCallback(() => { const updatedQuery = query.removeOrFieldClauses('tag'); - updateQuery(updatedQuery); + updateQuery(updatedQuery, null); return updateQuery; }, [query, updateQuery]);