Skip to content

Commit

Permalink
refactored code to address comments
Browse files Browse the repository at this point in the history
Signed-off-by: Amardeepsingh Siglani <[email protected]>
  • Loading branch information
amsiglan committed Nov 13, 2024
1 parent 99d9633 commit 502cdab
Show file tree
Hide file tree
Showing 27 changed files with 144 additions and 494 deletions.
2 changes: 1 addition & 1 deletion src/plugins/data/common/datasets/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ export interface Dataset extends BaseDataset {
/** Optional reference to the source dataset. Example usage is for indexed views to store the
* reference to the table dataset
*/
ref?: {
sourceDatasetRef?: {
id: string;
type: string;
};
Expand Down
5 changes: 0 additions & 5 deletions src/plugins/data/public/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -494,11 +494,6 @@ export {
PersistedLog,
LanguageReference,
DatasetIndexedViewsService,
QueryResultEnhancements,
QueryResultService,
QueryResultExtensionConfig,
QueryResultExtensionDependencies,
QueryResultServiceContract,
} from './query';

export { AggsStart } from './search/aggs';
Expand Down
2 changes: 0 additions & 2 deletions src/plugins/data/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,6 @@ export class DataPublicPlugin
if (enhancements.search) searchService.__enhance(enhancements.search);
if (enhancements.editor)
queryService.queryString.getLanguageService().__enhance(enhancements.editor);
if (enhancements.queryResults)
queryService.queryString.getQueryResultService().__enhance(enhancements.queryResults);
},
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,8 @@ export interface DatasetIndexedViewsService {
getIndexedViews: (dataset: Dataset) => Promise<DatasetIndexedView[]>;
/**
* Returns the data source saved object connected with the data connection object
* @param dataConnectionId Id of the data-connection saved object for which we want the data-source
* @returns Data source saved object for the Collection/Cluster attached with the data-connection
*/
getConnectedDataSource: (dataConnectionId: string) => Promise<SavedObject>;
getConnectedDataSource: (dataset: Dataset) => Promise<SavedObject>;
}

/**
Expand Down
7 changes: 0 additions & 7 deletions src/plugins/data/public/query/query_string/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,3 @@ export {
QueryStatus,
LanguageReference,
} from './language_service';
export {
QueryResultService,
QueryResultEnhancements,
QueryResultExtensionConfig,
QueryResultExtensionDependencies,
QueryResultServiceContract,
} from './query_results_service';

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,6 @@ import { coreMock } from '../../../../../core/public/mocks';
import { Query } from '../../../common/query';
import { ISearchInterceptor } from '../../search';
import { DataStorage, DEFAULT_DATA } from 'src/plugins/data/common';
import { QueryResultService } from './query_results_service';

const mockGetQueryResultExtensionMap = jest.fn();
jest.mock('./query_results_service', () => {
return {
QueryResultService: jest.fn().mockImplementation(() => {
return { getQueryResultExtensionMap: mockGetQueryResultExtensionMap };
}),
};
});

describe('QueryStringManager', () => {
let service: QueryStringManager;
Expand All @@ -63,16 +53,6 @@ describe('QueryStringManager', () => {
);
});

test('Created instance of QueryResultService', () => {
expect(QueryResultService).toHaveBeenCalledTimes(1);
});

test('getQueryResultService returns the instantiated query result service', () => {
expect(service.getQueryResultService().getQueryResultExtensionMap).toBe(
mockGetQueryResultExtensionMap
);
});

test('getUpdates$ is a cold emits only after query changes', () => {
const obs$ = service.getUpdates$();
const emittedValues: Query[] = [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,12 @@ import { DatasetService, DatasetServiceContract } from './dataset_service';
import { LanguageService, LanguageServiceContract } from './language_service';
import { ISearchInterceptor } from '../../search';
import { getApplication } from '../../services';
import { QueryResultService, QueryResultServiceContract } from './query_results_service';

export class QueryStringManager {
private query$: BehaviorSubject<Query>;
private queryHistory: QueryHistory;
private datasetService!: DatasetServiceContract;
private languageService!: LanguageServiceContract;
private queryResultService!: QueryResultServiceContract;

constructor(
private readonly storage: DataStorage,
Expand All @@ -59,7 +57,6 @@ export class QueryStringManager {
this.queryHistory = createHistory({ storage: this.sessionStorage });
this.datasetService = new DatasetService(uiSettings, this.sessionStorage);
this.languageService = new LanguageService(this.defaultSearchInterceptor, this.storage);
this.queryResultService = new QueryResultService();
}

private getDefaultQueryString() {
Expand Down Expand Up @@ -231,10 +228,6 @@ export class QueryStringManager {
return this.languageService;
};

public getQueryResultService = () => {
return this.queryResultService;
};

/**
* Gets the initial query based on the provided partial query object.
* If both language and dataset are provided, generates a new query without using current state
Expand Down
2 changes: 0 additions & 2 deletions src/plugins/data/public/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,10 @@ import { UsageCollectionSetup } from '../../usage_collection/public';
import { DataSourceStart } from './data_sources/datasource_services/types';
import { IUiStart } from './ui';
import { DataStorage } from '../common';
import { QueryResultEnhancements } from './query/query_string/query_results_service/types';

export interface DataPublicPluginEnhancements {
search?: SearchEnhancements;
editor?: EditorEnhancements;
queryResults?: QueryResultEnhancements;
}

export interface DataSetupDependencies {
Expand Down
62 changes: 26 additions & 36 deletions src/plugins/data/public/ui/dataset_selector/configurator.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import React from 'react';
import { setQueryService, setIndexPatterns } from '../../services';
import { IntlProvider } from 'react-intl';
import { Query } from '../../../../data/public';
import { Dataset } from 'src/plugins/data/common';

const getQueryMock = jest.fn().mockReturnValue({
query: '',
Expand All @@ -35,7 +36,7 @@ const languageService = {

const datasetService = {
getType: jest.fn().mockReturnValue({
fetchFields: jest.fn(),
fetchFields: jest.fn().mockResolvedValue([{ name: 'timestamp', type: 'date' }]),
supportedLanguages: jest.fn().mockReturnValue(['kuery', 'lucene']),
indexedViewsService: {
getIndexedViews: jest.fn().mockResolvedValue([
Expand All @@ -53,7 +54,7 @@ const datasetService = {
addRecentDataset: jest.fn(),
};

const fetchFieldsMock = jest.fn().mockResolvedValue([]);
const fetchFieldsMock = jest.fn().mockResolvedValue([{ name: 'timestamp', type: 'date' }]);

const mockServices = {
getQueryService: () => ({
Expand Down Expand Up @@ -83,11 +84,17 @@ const mockServices = {
]),
};

const mockBaseDataset = {
const mockBaseDataset: Dataset = {
id: 'mock-dataset',
title: 'Sample Dataset',
type: 'index-pattern',
timeFieldName: 'timestamp',
dataSource: { id: 'test-connection-id', meta: { supportsTimeFilter: true } },
dataSource: {
id: 'test-connection-id',
meta: { supportsTimeFilter: true },
title: 'mock-datasource',
type: 'DATA_SOURCE',
},
};

const messages = {
Expand Down Expand Up @@ -161,27 +168,6 @@ describe('Configurator Component', () => {
expect(mockOnPrevious).toHaveBeenCalledTimes(1);
});

it('should disable the submit button when conditions are not met', async () => {
render(
<IntlProvider locale="en" messages={messages}>
<Configurator
services={mockServices}
baseDataset={mockBaseDataset}
onConfirm={mockOnConfirm}
onCancel={mockOnCancel}
onPrevious={mockOnPrevious}
/>
</IntlProvider>
);

const submitButton = screen.getByTestId('advancedSelectorConfirmButton');
expect(submitButton).toBeDisabled();
fireEvent.change(screen.getByLabelText('Time field'), {
target: { value: 'valid-time-field' },
});
await waitFor(() => expect(submitButton).not.toBeDisabled());
});

it('should update state correctly when language is selected', async () => {
render(
<IntlProvider locale="en" messages={messages}>
Expand Down Expand Up @@ -339,30 +325,34 @@ describe('Configurator Component', () => {
});

it('should disable the confirm button when submit is disabled', async () => {
render(
const mockDataset = {
...mockBaseDataset,
timeFieldName: undefined,
type: 'index',
};
const { container } = render(
<IntlProvider locale="en" messages={messages}>
<Configurator
services={mockServices}
baseDataset={mockBaseDataset}
baseDataset={mockDataset}
onConfirm={mockOnConfirm}
onCancel={mockOnCancel}
onPrevious={mockOnPrevious}
/>
</IntlProvider>
);

const submitButton = screen.getByRole('button', { name: /select data/i });

expect(submitButton).toBeDisabled();

const languageSelect = screen.getByLabelText('Language');
fireEvent.change(languageSelect, { target: { value: 'kuery' } });
const submitButton = container.querySelector(
`button[data-test-subj="advancedSelectorConfirmButton"]`
); // screen.getAllByTestId() // screen.getByRole('button', { name: /select data/i });
await waitFor(() => {
expect(submitButton).toBeEnabled();
expect(submitButton).toBeDisabled();
});

const timeFieldSelect = screen.getByLabelText('Time field');
fireEvent.change(timeFieldSelect, { target: { value: 'timestamp' } });
const timeFieldSelect = container.querySelector(
`[data-test-subj="advancedSelectorTimeFieldSelect"]`
);
fireEvent.change(timeFieldSelect!, { target: { value: 'timestamp' } });

await waitFor(() => {
expect(submitButton).toBeEnabled();
Expand Down
Loading

0 comments on commit 502cdab

Please sign in to comment.