[Discover] One Discover context awareness #183797
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -68,6 +68,7 @@ import { onResizeGridColumn } from '../../../../utils/on_resize_grid_column'; | |
| import { useContextualGridCustomisations } from '../../hooks/grid_customisations'; | ||
| import { useIsEsqlMode } from '../../hooks/use_is_esql_mode'; | ||
| import { useAdditionalFieldGroups } from '../../hooks/sidebar/use_additional_field_groups'; | ||
| import { useProfileAccessor } from '../../../../context_awareness'; | ||
|
|
||
| const containerStyles = css` | ||
| position: relative; | ||
|
|
@@ -263,6 +264,12 @@ function DiscoverDocumentsComponent({ | |
| useContextualGridCustomisations() || {}; | ||
| const additionalFieldGroups = useAdditionalFieldGroups(); | ||
|
|
||
| const getCellRenderersAccessor = useProfileAccessor('getCellRenderers'); | ||
| const cellRenderers = useMemo(() => { | ||
| const getCellRenderers = getCellRenderersAccessor(() => customCellRenderer ?? {}); | ||
| return getCellRenderers(); | ||
|
There was a problem hiding this comment. I might be misunderstanding, but isn't it necessary to get the cell renderer for each row with the corresponding There was a problem hiding this comment. Unified Data Table cell renders are column based currently, so they don't work on individual records and can't be supported by If we decide we need it, I'd recommend we add support for them and do the necessary Unified Data Table refactoring in a followup. There was a problem hiding this comment. Ah, sorry, I didn't know this was an intentional omission. Could this indicate that narrowing the composable profile interface by context type might be good? Because I would also have made that mistake when trying to implement a custom cell renderer. There was a problem hiding this comment. Yes, that would make sense and it's actually already supported, I just forgot to omit |
||
| }, [customCellRenderer, getCellRenderersAccessor]); | ||
|
|
||
| const documents = useObservable(stateContainer.dataState.data$.documents$); | ||
|
|
||
| const callouts = useMemo( | ||
|
|
@@ -373,66 +380,64 @@ function DiscoverDocumentsComponent({ | |
| </> | ||
| )} | ||
| {!isLegacy && ( | ||
| <div className="unifiedDataTable"> | ||
| <CellActionsProvider | ||
| getTriggerCompatibleActions={uiActions.getTriggerCompatibleActions} | ||
| > | ||
| <DiscoverGridMemoized | ||
| ariaLabelledBy="documentsAriaLabel" | ||
| columns={currentColumns} | ||
| columnsMeta={columnsMeta} | ||
| expandedDoc={expandedDoc} | ||
| dataView={dataView} | ||
| loadingState={ | ||
| isDataLoading | ||
| ? DataLoadingState.loading | ||
| : isMoreDataLoading | ||
| ? DataLoadingState.loadingMore | ||
| : DataLoadingState.loaded | ||
| } | ||
| rows={rows} | ||
| sort={(sort as SortOrder[]) || []} | ||
| searchDescription={savedSearch.description} | ||
| searchTitle={savedSearch.title} | ||
| setExpandedDoc={setExpandedDoc} | ||
| showTimeCol={showTimeCol} | ||
| settings={grid} | ||
| onFilter={onAddFilter as DocViewFilterFn} | ||
| onSetColumns={onSetColumns} | ||
| onSort={onSort} | ||
| onResize={onResizeDataGrid} | ||
| useNewFieldsApi={useNewFieldsApi} | ||
| configHeaderRowHeight={3} | ||
| headerRowHeightState={headerRowHeight} | ||
| onUpdateHeaderRowHeight={onUpdateHeaderRowHeight} | ||
| rowHeightState={rowHeight} | ||
| onUpdateRowHeight={onUpdateRowHeight} | ||
| isSortEnabled={true} | ||
| isPlainRecord={isEsqlMode} | ||
| rowsPerPageState={rowsPerPage ?? getDefaultRowsPerPage(services.uiSettings)} | ||
| onUpdateRowsPerPage={onUpdateRowsPerPage} | ||
| maxAllowedSampleSize={getMaxAllowedSampleSize(services.uiSettings)} | ||
| sampleSizeState={getAllowedSampleSize(sampleSizeState, services.uiSettings)} | ||
| onUpdateSampleSize={!isEsqlMode ? onUpdateSampleSize : undefined} | ||
| onFieldEdited={onFieldEdited} | ||
| configRowHeight={uiSettings.get(ROW_HEIGHT_OPTION)} | ||
| showMultiFields={uiSettings.get(SHOW_MULTIFIELDS)} | ||
| maxDocFieldsDisplayed={uiSettings.get(MAX_DOC_FIELDS_DISPLAYED)} | ||
| renderDocumentView={renderDocumentView} | ||
| renderCustomToolbar={renderCustomToolbarWithElements} | ||
| services={services} | ||
| totalHits={totalHits} | ||
| onFetchMoreRecords={onFetchMoreRecords} | ||
| componentsTourSteps={TOUR_STEPS} | ||
| externalCustomRenderers={cellRenderers} | ||
| customGridColumnsConfiguration={customGridColumnsConfiguration} | ||
| customControlColumnsConfiguration={customControlColumnsConfiguration} | ||
| additionalFieldGroups={additionalFieldGroups} | ||
| /> | ||
| </CellActionsProvider> | ||
| </div> | ||
| )} | ||
| </EuiFlexItem> | ||
| </> | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| /* | ||
| * 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 and the Server Side Public License, v 1; you may not use this file except | ||
| * in compliance with, at your election, the Elastic License 2.0 or the Server | ||
| * Side Public License, v 1. | ||
| */ | ||
|
|
||
| import type { EsHitRecord } from '@kbn/discover-utils'; | ||
| import type { ExecutionContract } from '@kbn/expressions-plugin/common'; | ||
| import { RequestAdapter } from '@kbn/inspector-plugin/common'; | ||
| import { of } from 'rxjs'; | ||
| import { dataViewWithTimefieldMock } from '../../../__mocks__/data_view_with_timefield'; | ||
| import { discoverServiceMock } from '../../../__mocks__/services'; | ||
| import { fetchEsql } from './fetch_esql'; | ||
|
|
||
| describe('fetchEsql', () => { | ||
| beforeEach(() => { | ||
| jest.clearAllMocks(); | ||
| }); | ||
|
|
||
| it('resolves with returned records', async () => { | ||
| const hits = [ | ||
| { _id: '1', foo: 'bar' }, | ||
| { _id: '2', foo: 'baz' }, | ||
| ] as unknown as EsHitRecord[]; | ||
| const records = hits.map((hit, i) => ({ | ||
| id: String(i), | ||
| raw: hit, | ||
| flattened: hit, | ||
| })); | ||
| const expressionsExecuteSpy = jest.spyOn(discoverServiceMock.expressions, 'execute'); | ||
| expressionsExecuteSpy.mockReturnValueOnce({ | ||
| cancel: jest.fn(), | ||
| getData: jest.fn(() => | ||
| of({ | ||
| result: { | ||
| columns: ['_id', 'foo'], | ||
| rows: hits, | ||
| }, | ||
| }) | ||
| ), | ||
| } as unknown as ExecutionContract); | ||
| const resolveDocumentProfileSpy = jest.spyOn( | ||
| discoverServiceMock.profilesManager, | ||
| 'resolveDocumentProfile' | ||
| ); | ||
| expect( | ||
| await fetchEsql({ | ||
| query: { esql: 'from *' }, | ||
| dataView: dataViewWithTimefieldMock, | ||
| inspectorAdapters: { requests: new RequestAdapter() }, | ||
| data: discoverServiceMock.data, | ||
| expressions: discoverServiceMock.expressions, | ||
| profilesManager: discoverServiceMock.profilesManager, | ||
| }) | ||
| ).toEqual({ | ||
| records, | ||
| esqlQueryColumns: ['_id', 'foo'], | ||
| esqlHeaderWarning: undefined, | ||
| }); | ||
| expect(resolveDocumentProfileSpy).toHaveBeenCalledTimes(2); | ||
| expect(resolveDocumentProfileSpy).toHaveBeenCalledWith({ record: records[0] }); | ||
| expect(resolveDocumentProfileSpy).toHaveBeenCalledWith({ record: records[1] }); | ||
| }); | ||
| }); |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: as
dataViewpreviously was the last optional argument, this function signature made sense. But if we add a parameter and invoke it without having a data view, we then need to force passing a second argument to pass then a third one, and it'll start looking something likeShall we refactor the function to get named parameters as we are adding the processRecord now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fully agree with this, but
buildDataTableRecordListis used outside of Discover and I'd like to avoid pinging other teams for review on this PR. I can make this change in a followup though to avoid blocking this one.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I opened a PR based on this one for the signature change here: #184976.