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

[Discover] Rely on dataSource for ES|QL mode #183108

Merged
merged 14 commits into from
May 18, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@ import { DOC_TABLE_LEGACY } from '../constants';

export function isLegacyTableEnabled({
uiSettings,
isTextBasedQueryMode,
isEsqlMode,
}: {
uiSettings: IUiSettingsClient;
isTextBasedQueryMode: boolean;
isEsqlMode: boolean;
}): boolean {
if (isTextBasedQueryMode) {
if (isEsqlMode) {
return false; // only show the new data grid
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,21 @@ import { isSortable } from './get_sort';

/**
* use in case the user didn't manually sort.
* the default sort is returned depending on the data view or non for text based queries
* the default sort is returned depending on the data view or non for ES|QL queries
*/
export function getDefaultSort(
dataView: DataView | undefined,
defaultSortOrder: string = 'desc',
hidingTimeColumn: boolean = false,
isTextBasedQueryMode: boolean
isEsqlMode: boolean
): SortOrder[] {
if (isTextBasedQueryMode) {
if (isEsqlMode) {
return [];
}

if (
dataView?.timeFieldName &&
isSortable(dataView.timeFieldName, dataView, isTextBasedQueryMode) &&
isSortable(dataView.timeFieldName, dataView, isEsqlMode) &&
!hidingTimeColumn
) {
return [[dataView.timeFieldName, defaultSortOrder]];
Expand Down
28 changes: 12 additions & 16 deletions src/plugins/discover/common/utils/sorting/get_sort.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,10 @@ export type SortPairObj = Record<string, string>;
export type SortPair = SortOrder | SortPairObj;
export type SortInput = SortPair | SortPair[];

export function isSortable(
fieldName: string,
dataView: DataView,
isTextBasedQueryMode: boolean
): boolean {
if (isTextBasedQueryMode) {
// in-memory sorting is used for text-based queries
// would be great to have a way to determine if a text-based column is sortable
export function isSortable(fieldName: string, dataView: DataView, isEsqlMode: boolean): boolean {
if (isEsqlMode) {
// in-memory sorting is used for ES|QL queries
// would be great to have a way to determine if a ES|QL column is sortable
return fieldName !== '_source';
}
const field = dataView.getFieldByName(fieldName);
Expand All @@ -31,18 +27,18 @@ export function isSortable(
function createSortObject(
sortPair: SortInput,
dataView: DataView,
isTextBasedQueryMode: boolean
isEsqlMode: boolean
): SortPairObj | undefined {
if (
Array.isArray(sortPair) &&
sortPair.length === 2 &&
isSortable(String(sortPair[0]), dataView, isTextBasedQueryMode)
isSortable(String(sortPair[0]), dataView, isEsqlMode)
) {
const [field, direction] = sortPair as SortOrder;
return { [field]: direction };
} else if (
isPlainObject(sortPair) &&
isSortable(Object.keys(sortPair)[0], dataView, isTextBasedQueryMode)
isSortable(Object.keys(sortPair)[0], dataView, isEsqlMode)
) {
return sortPair as SortPairObj;
}
Expand All @@ -59,21 +55,21 @@ export function isLegacySort(sort: SortPair[] | SortPair): sort is SortPair {
* @param {array} sort two dimensional array [[fieldToSort, directionToSort]]
* or an array of objects [{fieldToSort: directionToSort}]
* @param {object} dataView used for determining default sort
* @param {boolean} isTextBasedQueryMode
* @param {boolean} isEsqlMode
* @returns Array<{object}> an array of sort objects
*/
export function getSort(
sort: SortPair[] | SortPair,
dataView: DataView,
isTextBasedQueryMode: boolean
isEsqlMode: boolean
): SortPairObj[] {
if (Array.isArray(sort)) {
if (isLegacySort(sort)) {
// To stay compatible with legacy sort, which just supported a single sort field
return [{ [sort[0]]: sort[1] }];
}
return sort
.map((sortPair: SortPair) => createSortObject(sortPair, dataView, isTextBasedQueryMode))
.map((sortPair: SortPair) => createSortObject(sortPair, dataView, isEsqlMode))
.filter((sortPairObj) => typeof sortPairObj === 'object') as SortPairObj[];
}
return [];
Expand All @@ -86,9 +82,9 @@ export function getSort(
export function getSortArray(
sort: SortInput,
dataView: DataView,
isTextBasedQueryMode: boolean
isEsqlMode: boolean
): SortOrder[] {
return getSort(sort, dataView, isTextBasedQueryMode).reduce((acc: SortOrder[], sortPair) => {
return getSort(sort, dataView, isEsqlMode).reduce((acc: SortOrder[], sortPair) => {
const entries = Object.entries(sortPair);
if (entries && entries[0]) {
acc.push(entries[0]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export function getSortForSearchSource({
}

const { timeFieldName } = dataView;
const sortPairs = getSort(sort, dataView, false); // text based request is not using search source
const sortPairs = getSort(sort, dataView, false); // ES|QL request is not using search source

const sortForSearchSource = sortPairs.map((sortPair: Record<string, string>) => {
if (timeFieldName && sortPair[timeFieldName]) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ import { LocalStorageMock } from '../../../../__mocks__/local_storage_mock';
import { DiscoverServices } from '../../../../build_services';
import { discoverServiceMock } from '../../../../__mocks__/services';
import { DiscoverTourProvider } from '../../../../components/discover_tour';
import { getDiscoverStateMock } from '../../../../__mocks__/discover_state.mock';
import { DiscoverMainProvider } from '../../state_management/discover_state_provider';

const defaultServices = {
...discoverServiceMock,
Expand Down Expand Up @@ -62,9 +64,11 @@ describe('Document Explorer Update callout', () => {
it('should start a tour when the button is clicked', () => {
const result = mountWithIntl(
<KibanaContextProvider services={defaultServices}>
<DiscoverTourProvider isPlainRecord={false}>
<DocumentExplorerUpdateCallout />
</DiscoverTourProvider>
<DiscoverMainProvider value={getDiscoverStateMock({})}>
<DiscoverTourProvider>
<DocumentExplorerUpdateCallout />
</DiscoverTourProvider>
</DiscoverMainProvider>
</KibanaContextProvider>
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ import { useInternalStateSelector } from '../../state_management/discover_intern
import { useAppStateSelector } from '../../state_management/discover_app_state_container';
import { useDiscoverServices } from '../../../../hooks/use_discover_services';
import { FetchStatus } from '../../../types';
import { RecordRawType } from '../../state_management/discover_data_state_container';
import { DiscoverStateContainer } from '../../state_management/discover_state';
import { useDataState } from '../../hooks/use_data_state';
import { DocTableInfinite } from '../../../../components/doc_table/doc_table_infinite';
Expand All @@ -56,7 +55,6 @@ import {
DISCOVER_TOUR_STEP_ANCHOR_IDS,
DiscoverTourProvider,
} from '../../../../components/discover_tour';
import { getRawRecordType } from '../../utils/get_raw_record_type';
import {
getMaxAllowedSampleSize,
getAllowedSampleSize,
Expand All @@ -68,6 +66,7 @@ import { SelectedVSAvailableCallout } from './selected_vs_available_callout';
import { useDiscoverCustomization } from '../../../../customizations';
import { onResizeGridColumn } from '../../../../utils/on_resize_grid_column';
import { useContextualGridCustomisations } from '../../hooks/grid_customisations';
import { useIsEsqlMode } from '../../hooks/use_is_esql_mode';

const containerStyles = css`
position: relative;
Expand Down Expand Up @@ -123,20 +122,20 @@ function DiscoverDocumentsComponent({
];
});
const expandedDoc = useInternalStateSelector((state) => state.expandedDoc);
const isTextBasedQuery = useMemo(() => getRawRecordType(query) === RecordRawType.PLAIN, [query]);
const isEsqlMode = useIsEsqlMode();
const useNewFieldsApi = useMemo(() => !uiSettings.get(SEARCH_FIELDS_FROM_SOURCE), [uiSettings]);
const hideAnnouncements = useMemo(() => uiSettings.get(HIDE_ANNOUNCEMENTS), [uiSettings]);
const isLegacy = useMemo(
() => isLegacyTableEnabled({ uiSettings, isTextBasedQueryMode: isTextBasedQuery }),
[uiSettings, isTextBasedQuery]
() => isLegacyTableEnabled({ uiSettings, isEsqlMode }),
[uiSettings, isEsqlMode]
);
const documentState = useDataState(documents$);
const isDataLoading =
documentState.fetchStatus === FetchStatus.LOADING ||
documentState.fetchStatus === FetchStatus.PARTIAL;

// This is needed to prevent EuiDataGrid pushing onSort because the data view has been switched.
// It's just necessary for non-text-based query lang requests since they don't have a partial result state, that's
// It's just necessary for non ES|QL requests since they don't have a partial result state, that's
// considered as loading state in the Component.
// 1. When switching the data view, the sorting in the URL is reset to the default sorting of the selected data view.
// 2. The new sort param is already available in this component and propagated to the EuiDataGrid.
Expand All @@ -145,13 +144,12 @@ function DiscoverDocumentsComponent({
// 5. this is propagated to Discover's URL and causes an unwanted change of state to an unsorted state
// This solution switches to the loading state in this component when the URL index doesn't match the dataView.id
const isDataViewLoading =
useInternalStateSelector((state) => state.isDataViewLoading) && !isTextBasedQuery;
useInternalStateSelector((state) => state.isDataViewLoading) && !isEsqlMode;
const isEmptyDataResult =
isTextBasedQuery || !documentState.result || documentState.result.length === 0;
isEsqlMode || !documentState.result || documentState.result.length === 0;
const rows = useMemo(() => documentState.result || [], [documentState.result]);

const { isMoreDataLoading, totalHits, onFetchMoreRecords } = useFetchMoreRecords({
isTextBasedQuery,
stateContainer,
});

Expand Down Expand Up @@ -227,10 +225,10 @@ function DiscoverDocumentsComponent({

const columnsMeta: DataTableColumnsMeta | undefined = useMemo(
() =>
documentState.textBasedQueryColumns
? getTextBasedColumnsMeta(documentState.textBasedQueryColumns)
documentState.esqlQueryColumns
? getTextBasedColumnsMeta(documentState.esqlQueryColumns)
: undefined,
[documentState.textBasedQueryColumns]
[documentState.esqlQueryColumns]
);

const renderDocumentView = useCallback(
Expand Down Expand Up @@ -269,32 +267,26 @@ function DiscoverDocumentsComponent({
() => (
<>
<SelectedVSAvailableCallout
isPlainRecord={isTextBasedQuery}
textBasedQueryColumns={documents?.textBasedQueryColumns}
esqlQueryColumns={documents?.esqlQueryColumns}
selectedColumns={currentColumns}
/>
<SearchResponseWarningsCallout warnings={documentState.interceptedWarnings ?? []} />
</>
),
[
isTextBasedQuery,
currentColumns,
documents?.textBasedQueryColumns,
documentState.interceptedWarnings,
]
[currentColumns, documents?.esqlQueryColumns, documentState.interceptedWarnings]
);

const gridAnnouncementCallout = useMemo(() => {
if (hideAnnouncements || isLegacy) {
return null;
}

return !isTextBasedQuery ? (
<DiscoverTourProvider isPlainRecord={isTextBasedQuery}>
return !isEsqlMode ? (
<DiscoverTourProvider>
<DocumentExplorerUpdateCallout />
</DiscoverTourProvider>
) : null;
}, [hideAnnouncements, isLegacy, isTextBasedQuery]);
}, [hideAnnouncements, isLegacy, isEsqlMode]);

const loadingIndicator = useMemo(
() =>
Expand Down Expand Up @@ -364,12 +356,12 @@ function DiscoverDocumentsComponent({
isLoading={isDataLoading}
searchDescription={savedSearch.description}
sharedItemTitle={savedSearch.title}
isPlainRecord={isTextBasedQuery}
isEsqlMode={isEsqlMode}
onAddColumn={onAddColumn}
onFilter={onAddFilter as DocViewFilterFn}
onMoveColumn={onMoveColumn}
onRemoveColumn={onRemoveColumn}
onSort={!isTextBasedQuery ? onSort : undefined}
onSort={!isEsqlMode ? onSort : undefined}
useNewFieldsApi={useNewFieldsApi}
dataTestSubj="discoverDocTable"
/>
Expand Down Expand Up @@ -415,12 +407,12 @@ function DiscoverDocumentsComponent({
rowHeightState={rowHeight}
onUpdateRowHeight={onUpdateRowHeight}
isSortEnabled={true}
isPlainRecord={isTextBasedQuery}
isPlainRecord={isEsqlMode}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why here we are keeping the isPlainRecord and we dont change the prop to isEsqlMode ? I see that we are doing this in other places

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because this component is a trick 😄 It's actually inheriting the props from Unified Data Table, and I wanted to keep this PR limited to the Discover codebase. But it would be a good idea to make similar changes within our shared components too separately.

rowsPerPageState={rowsPerPage ?? getDefaultRowsPerPage(services.uiSettings)}
onUpdateRowsPerPage={onUpdateRowsPerPage}
maxAllowedSampleSize={getMaxAllowedSampleSize(services.uiSettings)}
sampleSizeState={getAllowedSampleSize(sampleSizeState, services.uiSettings)}
onUpdateSampleSize={!isTextBasedQuery ? onUpdateSampleSize : undefined}
onUpdateSampleSize={!isEsqlMode ? onUpdateSampleSize : undefined}
onFieldEdited={onFieldEdited}
configRowHeight={uiSettings.get(ROW_HEIGHT_OPTION)}
showMultiFields={uiSettings.get(SHOW_MULTIFIELDS)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import {
DataDocuments$,
DataMain$,
DataTotalHits$,
RecordRawType,
} from '../../state_management/discover_data_state_container';
import { discoverServiceMock } from '../../../../__mocks__/services';
import { FetchStatus, SidebarToggleState } from '../../../types';
Expand Down Expand Up @@ -52,12 +51,12 @@ function getStateContainer(savedSearch?: SavedSearch) {
}

const mountComponent = async ({
isPlainRecord = false,
isEsqlMode = false,
storage,
savedSearch = savedSearchMockWithTimeField,
searchSessionId = '123',
}: {
isPlainRecord?: boolean;
isEsqlMode?: boolean;
isTimeBased?: boolean;
storage?: Storage;
savedSearch?: SavedSearch;
Expand Down Expand Up @@ -86,7 +85,6 @@ const mountComponent = async ({

const main$ = new BehaviorSubject({
fetchStatus: FetchStatus.COMPLETE,
recordRawType: isPlainRecord ? RecordRawType.PLAIN : RecordRawType.DOCUMENT,
foundDocuments: true,
}) as DataMain$;

Expand Down Expand Up @@ -121,7 +119,6 @@ const mountComponent = async ({
stateContainer.actions.undoSavedSearchChanges = jest.fn();

const props: DiscoverHistogramLayoutProps = {
isPlainRecord,
dataView,
stateContainer,
onFieldEdited: jest.fn(),
Expand Down Expand Up @@ -176,8 +173,8 @@ describe('Discover histogram layout component', () => {
expect(component.isEmptyRender()).toBe(false);
}, 10000);

it('should not render null if there is no search session, but isPlainRecord is true', async () => {
const { component } = await mountComponent({ isPlainRecord: true });
it('should not render null if there is no search session, but isEsqlMode is true', async () => {
const { component } = await mountComponent({ isEsqlMode: true });
expect(component.isEmptyRender()).toBe(false);
});

Expand Down
Loading