cleaning up tabify to return Datatable #89857
Conversation
ppisljar
added
release_note:skip
|
Pinging @elastic/kibana-app-services (Team:AppServices) |
ppisljar
force-pushed
the
tabify/cleanup
branch
from
0e21a0e to
9036e47
Compare
| @@ -28,5 +28,3 @@ export const tabify = ( | |||
|
|
|||
| export { tabifyAggResponse } from './tabify'; | |||
| export { tabifyGetColumns } from './get_columns'; | |||
|
|
|||
| export { TabbedTable, TabbedAggRow, TabbedAggColumn } from './types'; | |||
There was a problem hiding this comment.
Not 100%, but looks like TabbedTable interface in types.ts can be removed?
package.json
Outdated
| @@ -681,7 +681,7 @@ | |||
| "http-proxy": "^1.18.1", | |||
| "i18n-iso-countries": "^4.3.1", | |||
| "icalendar": "0.7.1", | |||
| "iedriver": "^3.14.2", | |||
|
|
|||
There was a problem hiding this comment.
Why is iedriver removed? How is Selenium driver related to this PR?
There was a problem hiding this comment.
ups, my mistake .... i had problems yarn kbn bootstraping ... need to revert this
| const [{ elasticsearch, savedObjects, uiSettings }, , self] = await getStartServices(); | ||
| const { fieldFormats, indexPatterns, search } = self; |
| @@ -65,9 +65,37 @@ export class TabbedAggResponseWriter { | |||
| } | |||
| } | |||
|
|
|||
| response(): TabbedTable { | |||
| response(): Datatable { | |||
There was a problem hiding this comment.
Maybe worth adding a couple of unit tests for the new implementation of this response() method.
# Conflicts: # src/plugins/data/server/server.api.md
| to: this.params.timeRange?.to?.toISOString(), | ||
| } | ||
| : undefined, | ||
| ...column.aggConfig.serialize(), |
There was a problem hiding this comment.
Is it possible for different columns to have different appliedTimeRange? It seems that way from the code
There was a problem hiding this comment.
not different (as this.params.timeRange doesn't change) but some columns will have it and some won't. also on the datatable level there could actually be dfferent timeranges on different columns, but that's not in the scope of tabify.
| @@ -28,19 +29,18 @@ export class TabbedAggResponseWriter { | |||
| metricBuffer: BufferColumn[] = []; | |||
|
|
|||
| private readonly partialRows: boolean; | |||
| private readonly params: Partial<TabbedResponseWriterOptions>; | |||
There was a problem hiding this comment.
Can params be called something else that is more meaningful?
There was a problem hiding this comment.
open for suggestions, but as this it not our public API i wouldn't spend too much time on it.
💚 Build SucceededMetrics [docs]Module Count
Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
Summary
resolves #84050
Checklist
Delete any items that are not applicable to this PR.
For maintainers