Skip to content

Commit

Permalink
[Enterprise Search] Fixes an index name inconsistency bug (#151720)
Browse files Browse the repository at this point in the history
This fixes a bug where sometimes (after creating a crawler index) we'd
see some inconsistent index name usage with unexpected redirects. This
was caused by the search index component not unloading properly.
Removing the `prop` usage makes sure we're no longer relying on
loading/unloading to set the index name.
  • Loading branch information
sphilipse authored Feb 22, 2023
1 parent cfe91b1 commit c2f639d
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,11 @@ describe('DocumentsLogic', () => {

beforeEach(() => {
jest.clearAllMocks();
// due to connect, need to pass props down to each logic
const indexNameProps = { indexName: 'indexName' };
mountIndexNameLogic(undefined, indexNameProps);
mountMappingsApiLogic(undefined, indexNameProps);
mountSearchDocumentsApiLogic(undefined, indexNameProps);
mount(undefined, indexNameProps);
const indexNameLogic = mountIndexNameLogic();
mountMappingsApiLogic();
mountSearchDocumentsApiLogic();
mount();
indexNameLogic.actions.setIndexName('indexName');
});

it('has expected default values', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,18 @@ export interface IndexNameActions {
setIndexName: (indexName: string) => { indexName: string };
}

export const IndexNameLogic = kea<MakeLogicType<IndexNameValues, IndexNameActions, IndexNameProps>>(
{
actions: {
setIndexName: (indexName) => ({ indexName }),
},
path: ['enterprise_search', 'content', 'index_name'],
reducers: ({ props }) => ({
indexName: [
// Short-circuiting this to empty string is necessary to enable testing logics relying on this
props.indexName ?? '',
{
setIndexName: (_, { indexName }) => indexName,
},
],
}),
}
);
export const IndexNameLogic = kea<MakeLogicType<IndexNameValues, IndexNameActions>>({
actions: {
setIndexName: (indexName) => ({ indexName }),
},
path: ['enterprise_search', 'content', 'index_name'],
reducers: () => ({
indexName: [
// Short-circuiting this to empty string is necessary to enable testing logics relying on this
'',
{
setIndexName: (_, { indexName }) => indexName,
},
],
}),
});
Original file line number Diff line number Diff line change
Expand Up @@ -31,21 +31,27 @@ import { IndexViewLogic } from './index_view_logic';
// And the timeoutId is non-deterministic. We use expect.object.containing throughout this test file
const DEFAULT_VALUES = {
connector: undefined,
connectorError: undefined,
connectorId: null,
error: null,
fetchIndexApiData: undefined,
fetchIndexApiStatus: Status.IDLE,
fetchIndexApiData: {},
fetchIndexApiStatus: Status.SUCCESS,
hasAdvancedFilteringFeature: false,
hasBasicFilteringFeature: false,
hasFilteringFeature: false,
index: undefined,
indexData: null,
htmlExtraction: undefined,
index: {
ingestionMethod: IngestionMethod.API,
ingestionStatus: IngestionStatus.CONNECTED,
lastUpdated: null,
},
indexData: {},
indexName: 'index-name',
ingestionMethod: IngestionMethod.API,
ingestionStatus: IngestionStatus.CONNECTED,
isCanceling: false,
isConnectorIndex: false,
isInitialLoading: true,
isInitialLoading: false,
isSyncing: false,
isWaitingForSync: false,
lastUpdated: null,
Expand All @@ -68,24 +74,23 @@ const CONNECTOR_VALUES = {
describe('IndexViewLogic', () => {
const { mount: apiLogicMount } = new LogicMounter(StartSyncApiLogic);
const { mount: fetchIndexMount } = new LogicMounter(CachedFetchIndexApiLogic);
const indexNameLogic = new LogicMounter(IndexNameLogic);
const { mount: indexNameMount } = new LogicMounter(IndexNameLogic);
const { mount } = new LogicMounter(IndexViewLogic);
const { flashSuccessToast } = mockFlashMessageHelpers;
const { http } = mockHttpValues;

beforeEach(() => {
jest.clearAllMocks();
jest.useRealTimers();
indexNameLogic.mount({ indexName: 'index-name' }, { indexName: 'index-name' });
http.get.mockReturnValueOnce(Promise.resolve({}));
const indexNameLogic = indexNameMount();
apiLogicMount();
fetchIndexMount({ indexName: 'index-name' }, { indexName: 'index-name' });
mount({ indexName: 'index-name' }, { indexName: 'index-name' });
fetchIndexMount();
mount();
indexNameLogic.actions.setIndexName('index-name');
});

it('has expected default values', () => {
http.get.mockReturnValueOnce(Promise.resolve(() => ({})));
mount({ indexName: 'index-name' }, { indexName: 'index-name' });

expect(IndexViewLogic.values).toEqual(DEFAULT_VALUES);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import { nextTick } from '@kbn/test-jest-helpers';

import { FetchCustomPipelineApiLogic } from '../../../api/index/fetch_custom_pipeline_api_logic';

import { IndexNameLogic } from '../index_name_logic';

import {
IndexPipelinesConfigurationsLogic,
IndexPipelinesConfigurationsValues,
Expand All @@ -28,13 +30,15 @@ const DEFAULT_VALUES: IndexPipelinesConfigurationsValues = {

describe('IndexPipelinesConfigurationsLogic', () => {
const { mount } = new LogicMounter(IndexPipelinesConfigurationsLogic);
const { mount: indexNameMount } = new LogicMounter(IndexNameLogic);
const { mount: mountFetchCustomPipelineApiLogic } = new LogicMounter(FetchCustomPipelineApiLogic);

beforeEach(async () => {
jest.clearAllMocks();
const indexNameProps = { indexName };
const indexNameLogic = indexNameMount();
mountFetchCustomPipelineApiLogic();
mount(undefined, indexNameProps);
mount();
indexNameLogic.actions.setIndexName(indexName);
});

it('has expected default values', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ export const SearchIndex: React.FC = () => {
useEffect(() => {
if (
isConnectorIndex(index) &&
index.name === indexName &&
index.connector.is_native &&
index.connector.service_type === null
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,10 @@ import { SearchIndex } from './search_index';

export const SearchIndexRouter: React.FC = () => {
const indexName = decodeURIComponent(useParams<{ indexName: string }>().indexName);

const indexNameLogic = IndexNameLogic({ indexName });
const { setIndexName } = useActions(indexNameLogic);
const { setIndexName } = useActions(IndexNameLogic);
const { stopFetchIndexPoll } = useActions(IndexViewLogic);
useEffect(() => {
const unmountName = indexNameLogic.mount();
const unmountName = IndexNameLogic.mount();
const unmountView = IndexViewLogic.mount();
return () => {
stopFetchIndexPoll();
Expand Down

0 comments on commit c2f639d

Please sign in to comment.