[8.x] [TableListView] Hint message for chars not allowed in search (e…

…lastic#197307) (elastic#197570) # Backport This will backport the following commits from `main` to `8.x`: - [[TableListView] Hint message for chars not allowed in search (elastic#197307)](elastic#197307)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Sébastien Loix <[email protected]>
gsoldevila · Oct 24, 2024 · 47374df · 47374df
1 parent 289bcec
commit 47374df
Show file tree

Hide file tree

Showing 8 changed files with 131 additions and 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
@@ -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
@@ -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
@@ -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<T extends UserContentCommonSchema> extends State<T>, TagManagementProps {
   dispatch: Dispatch<Action<T>>;
   entityName: string;
@@ -219,6 +223,7 @@ export function Table<T extends UserContentCommonSchema>({
   }, [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<T extends UserContentCommonSchema>({
         'data-test-subj': 'tableListSearchBox',
       },
       filters: searchFilters,
+      hint: {
+        content: (
+          <EuiText color="red" size="s" data-test-subj="forbiddenCharErrorMessage">
+            <FormattedMessage
+              id="contentManagement.tableList.listing.charsNotAllowedHint"
+              defaultMessage="Characters not allowed: {chars}"
+              values={{
+                chars: <EuiCode>{FORBIDDEN_SEARCH_CHARS}</EuiCode>,
+              }}
+            />
+          </EuiText>
+        ),
+        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
@@ -84,14 +84,21 @@ export function getReducer<T extends UserContentCommonSchema>() {
         };
       }
       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
@@ -1078,25 +1078,29 @@ describe('TableListView', () => {
 
     const findItems = jest.fn();
 
-    const setupSearch = (...args: Parameters<ReturnType<typeof registerTestBed>>) => {
-      const testBed = registerTestBed<string, TableListViewTableProps>(
-        WithServices<TableListViewTableProps>(TableListViewTable),
-        {
-          defaultProps: {
-            ...requiredProps,
-            findItems,
-            urlStateEnabled: false,
-            entityName: 'Foo',
-            entityNamePlural: 'Foos',
-          },
-          memoryRouter: { wrapComponent: true },
-        }
-      )(...args);
+    const setupSearch = async (...args: Parameters<ReturnType<typeof registerTestBed>>) => {
+      let testBed: TestBed;
 
-      const { updateSearchText, getSearchBoxValue } = getActions(testBed);
+      await act(async () => {
+        testBed = registerTestBed<string, TableListViewTableProps>(
+          WithServices<TableListViewTableProps>(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<void>;
-      let getLastCallArgsFromFindItems: () => Parameters<typeof findItems>;
-      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<void>;
-
-      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
@@ -50,14 +50,15 @@ import {
   ListingLimitWarning,
   ItemDetails,
   UpdatedAtField,
+  FORBIDDEN_SEARCH_CHARS,
 } from './components';
 import { useServices } from './services';
 import type { SavedObjectsFindOptionsReference } from './services';
 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<T extends UserContentCommonSchema = UserContentCommonSche
   searchQuery: {
     text: string;
     query: Query;
+    error: SearchQueryError | null;
   };
   selectedIds: string[];
   totalItems: number;
@@ -189,6 +191,8 @@ interface URLQueryParams {
   [key: string]: unknown;
 }
 
+const FORBIDDEN_SEARCH_CHARS_ARRAY = FORBIDDEN_SEARCH_CHARS.split('');
+
 /**
  * Deserializer to convert the URL query params to a sanitized object
  * we can rely on in this component.
@@ -407,7 +411,7 @@ function TableListViewTableComp<T extends UserContentCommonSchema>({
       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<T extends UserContentCommonSchema>({
   }, [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<T extends UserContentCommonSchema>({
   );
 
   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<T extends UserContentCommonSchema>({
         data: {
           query: updatedQuery,
           text,
+          error: null,
         },
       });
     };
@@ -1089,7 +1112,7 @@ function TableListViewTableComp<T extends UserContentCommonSchema>({
   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
@@ -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
@@ -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]);