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

[TableListView] Hint message for chars not allowed in search #197307

Merged
merged 5 commits into from
Oct 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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