Skip to content

Commit

Permalink
[TableListView] Hint message for chars not allowed in search (elastic…
Browse files Browse the repository at this point in the history
  • Loading branch information
sebelga authored Oct 24, 2024
1 parent e0ef5b3 commit b45a8e0
Show file tree
Hide file tree
Showing 8 changed files with 131 additions and 53 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -72,8 +73,9 @@ export interface ShowConfirmDeleteItemsModalAction {
export interface OnSearchQueryChangeAction {
type: 'onSearchQueryChange';
data: {
query: Query;
text: string;
query?: Query;
text?: string;
error: SearchQueryError | null;
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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(),
Expand All @@ -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);

Expand Down
13 changes: 10 additions & 3 deletions packages/content-management/table_list_view_table/src/reducer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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': {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand All @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -146,6 +147,7 @@ export interface State<T extends UserContentCommonSchema = UserContentCommonSche
searchQuery: {
text: string;
query: Query;
error: SearchQueryError | null;
};
selectedIds: string[];
totalItems: number;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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);
}
});
}
Expand Down Expand Up @@ -1036,6 +1058,7 @@ function TableListViewTableComp<T extends UserContentCommonSchema>({
data: {
query: updatedQuery,
text,
error: null,
},
});
};
Expand Down Expand Up @@ -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]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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
Expand All @@ -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;
},
Expand Down Expand Up @@ -147,7 +147,7 @@ export function useTags({

const clearTagSelection = useCallback(() => {
const updatedQuery = query.removeOrFieldClauses('tag');
updateQuery(updatedQuery);
updateQuery(updatedQuery, null);
return updateQuery;
}, [query, updateQuery]);

Expand Down

0 comments on commit b45a8e0

Please sign in to comment.