Skip to content

Commit

Permalink
[Search] [Playground] [Bug] Add playground index validation (#201032)
Browse files Browse the repository at this point in the history
Fix bug when localStorage has invalid index, then playground chat page
opens with empty source.
  • Loading branch information
yansavitski authored Nov 25, 2024
1 parent 2e004f8 commit d99431e
Show file tree
Hide file tree
Showing 9 changed files with 104 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ describe('SelectIndicesFlyout', () => {
mockedUseQueryIndices.mockReturnValue({
indices: ['index1', 'index2', 'index3'],
isLoading: false,
isFetched: true,
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ interface SelectIndicesFlyout {

export const SelectIndicesFlyout: React.FC<SelectIndicesFlyout> = ({ onClose }) => {
const [query, setQuery] = useState<string>('');
const { indices, isLoading: isIndicesLoading } = useQueryIndices(query);
const { indices, isLoading: isIndicesLoading } = useQueryIndices({ query });
const { indices: selectedIndices, setIndices: setSelectedIndices } = useSourceIndicesFields();
const [selectedTempIndices, setSelectedTempIndices] = useState<string[]>(selectedIndices);
const handleSelectOptions = (options: EuiSelectableOption[]) => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { useEffect, useState } from 'react';
import { useQueryIndices } from './use_query_indices';

export const useIndicesValidation = (unvalidatedIndices: string[]) => {
const [isValidated, setIsValidated] = useState<boolean>(false);
const [validIndices, setValidIndices] = useState<string[]>([]);
const { indices, isFetched: isIndicesLoaded } = useQueryIndices({
query: unvalidatedIndices.join(','),
exact: true,
});

useEffect(() => {
if (isIndicesLoaded) {
setValidIndices(indices.filter((index) => unvalidatedIndices.includes(index)));
setIsValidated(true);
}
}, [unvalidatedIndices, indices, isIndicesLoaded]);

return { isValidated, validIndices };
};
41 changes: 28 additions & 13 deletions x-pack/plugins/search_playground/public/hooks/use_query_indices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,26 +11,41 @@ import { useKibana } from './use_kibana';
import { APIRoutes } from '../types';

export const useQueryIndices = (
query: string = ''
): { indices: IndexName[]; isLoading: boolean } => {
{
query,
exact,
}: {
query?: string;
exact?: boolean;
} = { query: '', exact: false }
): { indices: IndexName[]; isLoading: boolean; isFetched: boolean } => {
const { services } = useKibana();

const { data, isLoading } = useQuery({
const { data, isLoading, isFetched } = useQuery({
queryKey: ['indices', query],
queryFn: async () => {
const response = await services.http.get<{
indices: string[];
}>(APIRoutes.GET_INDICES, {
query: {
search_query: query,
size: 10,
},
});
try {
const response = await services.http.get<{
indices: string[];
}>(APIRoutes.GET_INDICES, {
query: {
search_query: query,
exact,
size: 50,
},
});

return response.indices;
return response.indices;
} catch (err) {
if (err?.response?.status === 404) {
return [];
}

throw err;
}
},
initialData: [],
});

return { indices: data, isLoading };
return { indices: data, isLoading, isFetched };
};
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ jest.mock('../hooks/use_llms_models');
jest.mock('react-router-dom-v5-compat', () => ({
useSearchParams: jest.fn(() => [{ get: jest.fn() }]),
}));
jest.mock('../hooks/use_indices_validation', () => ({
useIndicesValidation: jest.fn((indices) => ({ isValidated: true, validIndices: indices })),
}));

let formHookSpy: jest.SpyInstance;

Expand Down Expand Up @@ -216,6 +219,7 @@ describe('FormProvider', () => {
});

it('updates indices from search params', async () => {
expect.assertions(1);
const mockSearchParams = new URLSearchParams();
mockSearchParams.get = jest.fn().mockReturnValue('new-index');
mockUseSearchParams.mockReturnValue([mockSearchParams]);
Expand All @@ -237,10 +241,12 @@ describe('FormProvider', () => {
</FormProvider>
);

const { getValues } = formHookSpy.mock.results[0].value;
await act(async () => {
const { getValues } = formHookSpy.mock.results[0].value;

await waitFor(() => {
expect(getValues(ChatFormFields.indices)).toEqual(['new-index']);
await waitFor(() => {
expect(getValues(ChatFormFields.indices)).toEqual(['new-index']);
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import { FormProvider as ReactHookFormProvider, useForm } from 'react-hook-form';
import React, { useEffect, useMemo } from 'react';
import { useSearchParams } from 'react-router-dom-v5-compat';
import { useIndicesValidation } from '../hooks/use_indices_validation';
import { useLoadFieldsByIndices } from '../hooks/use_load_fields_by_indices';
import { ChatForm, ChatFormFields } from '../types';
import { useLLMsModels } from '../hooks/use_llms_models';
Expand Down Expand Up @@ -53,16 +54,27 @@ export const FormProvider: React.FC<React.PropsWithChildren<FormProviderProps>>
}) => {
const models = useLLMsModels();
const [searchParams] = useSearchParams();
const index = useMemo(() => searchParams.get('default-index'), [searchParams]);
const defaultIndex = useMemo(() => {
const index = searchParams.get('default-index');

return index ? [index] : null;
}, [searchParams]);
const sessionState = useMemo(() => getLocalSession(storage), [storage]);
const form = useForm<ChatForm>({
defaultValues: {
...sessionState,
indices: index ? [index] : sessionState.indices,
indices: [],
search_query: '',
},
});
useLoadFieldsByIndices({ watch: form.watch, setValue: form.setValue, getValues: form.getValues });
const { isValidated: isValidatedIndices, validIndices } = useIndicesValidation(
defaultIndex || sessionState.indices || []
);
useLoadFieldsByIndices({
watch: form.watch,
setValue: form.setValue,
getValues: form.getValues,
});

useEffect(() => {
const subscription = form.watch((values) =>
Expand All @@ -80,5 +92,11 @@ export const FormProvider: React.FC<React.PropsWithChildren<FormProviderProps>>
}
}, [form, models]);

useEffect(() => {
if (isValidatedIndices) {
form.setValue(ChatFormFields.indices, validIndices);
}
}, [form, isValidatedIndices, validIndices]);

return <ReactHookFormProvider {...form}>{children}</ReactHookFormProvider>;
};
5 changes: 3 additions & 2 deletions x-pack/plugins/search_playground/server/lib/fetch_indices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,12 @@ function isClosed(index: IndicesIndexState): boolean {

export const fetchIndices = async (
client: ElasticsearchClient,
searchQuery: string | undefined
searchQuery: string | undefined,
{ exact }: { exact?: boolean } = { exact: false }
): Promise<{
indexNames: string[];
}> => {
const indexPattern = searchQuery ? `*${searchQuery}*` : '*';
const indexPattern = exact && searchQuery ? searchQuery : searchQuery ? `*${searchQuery}*` : '*';
const allIndexMatches = await client.indices.get({
expand_wildcards: ['open'],
// for better performance only compute aliases and settings of indices but not mappings
Expand Down
6 changes: 3 additions & 3 deletions x-pack/plugins/search_playground/server/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,17 +198,17 @@ export function defineRoutes({
query: schema.object({
search_query: schema.maybe(schema.string()),
size: schema.number({ defaultValue: 10, min: 0 }),
exact: schema.maybe(schema.boolean({ defaultValue: false })),
}),
},
},
errorHandler(logger)(async (context, request, response) => {
const { search_query: searchQuery, size } = request.query;
const { search_query: searchQuery, exact, size } = request.query;
const {
client: { asCurrentUser },
} = (await context.core).elasticsearch;

const { indexNames } = await fetchIndices(asCurrentUser, searchQuery);

const { indexNames } = await fetchIndices(asCurrentUser, searchQuery, { exact });
const indexNameSlice = indexNames.slice(0, size).filter(isNotNullish);

return response.ok({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
});

describe('without any indices', () => {
it('hide no create index button when index added', async () => {
it('hide create index button when index added', async () => {
await createIndex();
await pageObjects.searchPlayground.PlaygroundStartChatPage.expectOpenFlyoutAndSelectIndex();
});
Expand All @@ -121,6 +121,9 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
before(async () => {
await createConnector();
await createIndex();
});

beforeEach(async () => {
await pageObjects.searchPlayground.session.setSession();
await browser.refresh();
});
Expand All @@ -129,6 +132,13 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
await pageObjects.searchPlayground.PlaygroundStartChatPage.expectToSelectIndicesAndLoadChat();
});

it('load start page after removing selected index', async () => {
await pageObjects.searchPlayground.PlaygroundStartChatPage.expectToSelectIndicesAndLoadChat();
await esArchiver.unload(esArchiveIndex);
await browser.refresh();
await pageObjects.searchPlayground.PlaygroundStartChatPage.expectCreateIndexButtonToExists();
});

after(async () => {
await removeOpenAIConnector?.();
await esArchiver.unload(esArchiveIndex);
Expand Down

0 comments on commit d99431e

Please sign in to comment.