From b5bc707c995d8e58381d34591e28929d840a653a Mon Sep 17 00:00:00 2001 From: Andrew Tate Date: Thu, 11 Nov 2021 14:03:00 -0600 Subject: [PATCH] Datatable Embeddable Supports Ephemeral Sort (#117742) --- .../__snapshots__/table_basic.test.tsx.snap | 26 +++-- .../components/columns.tsx | 24 ++--- .../components/table_basic.test.tsx | 2 +- .../components/table_basic.tsx | 6 +- .../public/editor_frame_service/service.tsx | 13 ++- .../public/embeddable/embeddable.test.tsx | 98 +++++++++++++++++++ .../lens/public/embeddable/embeddable.tsx | 97 ++++++++++++++---- .../public/embeddable/embeddable_factory.ts | 4 + x-pack/plugins/lens/public/plugin.ts | 3 + x-pack/test/functional/apps/lens/index.ts | 1 + .../functional/apps/lens/table_dashboard.ts | 55 +++++++++++ 11 files changed, 279 insertions(+), 50 deletions(-) create mode 100644 x-pack/test/functional/apps/lens/table_dashboard.ts diff --git a/x-pack/plugins/lens/public/datatable_visualization/components/__snapshots__/table_basic.test.tsx.snap b/x-pack/plugins/lens/public/datatable_visualization/components/__snapshots__/table_basic.test.tsx.snap index 7e3c8c3342e4c..bf8497e686e96 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/components/__snapshots__/table_basic.test.tsx.snap +++ b/x-pack/plugins/lens/public/datatable_visualization/components/__snapshots__/table_basic.test.tsx.snap @@ -487,7 +487,7 @@ exports[`DatatableComponent it renders the title and value 1`] = ` `; -exports[`DatatableComponent it should render hide and reset actions on header even when it is in read only mode 1`] = ` +exports[`DatatableComponent it should render hide, reset, and sort actions on header even when it is in read only mode 1`] = ` { ).toMatchSnapshot(); }); - test('it should render hide and reset actions on header even when it is in read only mode', () => { + test('it should render hide, reset, and sort actions on header even when it is in read only mode', () => { const { data, args } = sampleArgs(); expect( diff --git a/x-pack/plugins/lens/public/datatable_visualization/components/table_basic.tsx b/x-pack/plugins/lens/public/datatable_visualization/components/table_basic.tsx index 6be69e5d4d236..f627a3ef8ff91 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/components/table_basic.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/components/table_basic.tsx @@ -103,11 +103,9 @@ export const DatatableComponent = (props: DatatableRenderProps) => { const onEditAction = useCallback( (data: LensSortAction['data'] | LensResizeAction['data'] | LensToggleAction['data']) => { - if (renderMode === 'edit') { - dispatchEvent({ name: 'edit', data }); - } + dispatchEvent({ name: 'edit', data }); }, - [dispatchEvent, renderMode] + [dispatchEvent] ); const onRowContextMenuClick = useCallback( (data: LensTableRowContextMenuEvent['data']) => { diff --git a/x-pack/plugins/lens/public/editor_frame_service/service.tsx b/x-pack/plugins/lens/public/editor_frame_service/service.tsx index d97cfd3cbca23..b585d03e12f8f 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/service.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/service.tsx @@ -61,16 +61,19 @@ export class EditorFrameService { private readonly datasources: Array Promise)> = []; private readonly visualizations: Array Promise)> = []; + private loadDatasources = () => collectAsyncDefinitions(this.datasources); + public loadVisualizations = () => collectAsyncDefinitions(this.visualizations); + /** * This method takes a Lens saved object as returned from the persistence helper, * initializes datsources and visualization and creates the current expression. - * This is an asynchronous process and should only be triggered once for a saved object. + * This is an asynchronous process. * @param doc parsed Lens saved object */ public documentToExpression = async (doc: Document) => { const [resolvedDatasources, resolvedVisualizations] = await Promise.all([ - collectAsyncDefinitions(this.datasources), - collectAsyncDefinitions(this.visualizations), + this.loadDatasources(), + this.loadVisualizations(), ]); const { persistedStateToExpression } = await import('../async_services'); @@ -92,8 +95,8 @@ export class EditorFrameService { public start(core: CoreStart, plugins: EditorFrameStartPlugins): EditorFrameStart { const createInstance = async (): Promise => { const [resolvedDatasources, resolvedVisualizations] = await Promise.all([ - collectAsyncDefinitions(this.datasources), - collectAsyncDefinitions(this.visualizations), + this.loadDatasources(), + this.loadVisualizations(), ]); const { EditorFrame } = await import('../async_services'); diff --git a/x-pack/plugins/lens/public/embeddable/embeddable.test.tsx b/x-pack/plugins/lens/public/embeddable/embeddable.test.tsx index 4c247c031eac0..59d6325e1c0ce 100644 --- a/x-pack/plugins/lens/public/embeddable/embeddable.test.tsx +++ b/x-pack/plugins/lens/public/embeddable/embeddable.test.tsx @@ -25,6 +25,7 @@ import { LensAttributeService } from '../lens_attribute_service'; import { OnSaveProps } from '../../../../../src/plugins/saved_objects/public/save_modal'; import { act } from 'react-dom/test-utils'; import { inspectorPluginMock } from '../../../../../src/plugins/inspector/public/mocks'; +import { Visualization } from '../types'; jest.mock('../../../../../src/plugins/inspector/public/', () => ({ isAvailable: false, @@ -125,6 +126,7 @@ describe('embeddable', () => { }, inspector: inspectorPluginMock.createStartContract(), getTrigger, + visualizationMap: {}, documentToExpression: () => Promise.resolve({ ast: { @@ -165,6 +167,7 @@ describe('embeddable', () => { inspector: inspectorPluginMock.createStartContract(), capabilities: { canSaveDashboards: true, canSaveVisualizations: true }, getTrigger, + visualizationMap: {}, documentToExpression: () => Promise.resolve({ ast: { @@ -209,6 +212,7 @@ describe('embeddable', () => { canSaveVisualizations: true, }, getTrigger, + visualizationMap: {}, documentToExpression: () => Promise.resolve({ ast: { @@ -255,6 +259,7 @@ describe('embeddable', () => { canSaveVisualizations: true, }, getTrigger, + visualizationMap: {}, documentToExpression: () => Promise.resolve({ ast: { @@ -297,6 +302,7 @@ describe('embeddable', () => { canSaveVisualizations: true, }, getTrigger, + visualizationMap: {}, documentToExpression: () => Promise.resolve({ ast: { @@ -336,6 +342,7 @@ describe('embeddable', () => { canSaveVisualizations: true, }, getTrigger, + visualizationMap: {}, documentToExpression: () => Promise.resolve({ ast: { @@ -378,6 +385,7 @@ describe('embeddable', () => { indexPatternService: {} as IndexPatternsContract, capabilities: { canSaveDashboards: true, canSaveVisualizations: true }, getTrigger, + visualizationMap: {}, documentToExpression: () => Promise.resolve({ ast: { @@ -427,6 +435,7 @@ describe('embeddable', () => { canSaveVisualizations: true, }, getTrigger, + visualizationMap: {}, documentToExpression: () => Promise.resolve({ ast: { @@ -474,6 +483,7 @@ describe('embeddable', () => { canSaveVisualizations: true, }, getTrigger, + visualizationMap: {}, documentToExpression: () => Promise.resolve({ ast: { @@ -528,6 +538,7 @@ describe('embeddable', () => { canSaveVisualizations: true, }, getTrigger, + visualizationMap: {}, documentToExpression: () => Promise.resolve({ ast: { @@ -583,6 +594,7 @@ describe('embeddable', () => { canSaveVisualizations: true, }, getTrigger, + visualizationMap: {}, documentToExpression: () => Promise.resolve({ ast: { @@ -641,6 +653,7 @@ describe('embeddable', () => { canSaveVisualizations: true, }, getTrigger, + visualizationMap: {}, documentToExpression: () => Promise.resolve({ ast: { @@ -683,6 +696,7 @@ describe('embeddable', () => { canSaveVisualizations: true, }, getTrigger, + visualizationMap: {}, documentToExpression: () => Promise.resolve({ ast: { @@ -725,6 +739,7 @@ describe('embeddable', () => { canSaveVisualizations: true, }, getTrigger, + visualizationMap: {}, documentToExpression: () => Promise.resolve({ ast: { @@ -767,6 +782,7 @@ describe('embeddable', () => { canSaveVisualizations: true, }, getTrigger, + visualizationMap: {}, documentToExpression: () => Promise.resolve({ ast: { @@ -824,6 +840,7 @@ describe('embeddable', () => { canSaveVisualizations: true, }, getTrigger, + visualizationMap: {}, documentToExpression: () => Promise.resolve({ ast: { @@ -897,6 +914,7 @@ describe('embeddable', () => { canSaveVisualizations: true, }, getTrigger, + visualizationMap: {}, documentToExpression: () => Promise.resolve({ ast: { @@ -945,6 +963,7 @@ describe('embeddable', () => { canSaveVisualizations: true, }, getTrigger, + visualizationMap: {}, documentToExpression: () => Promise.resolve({ ast: { @@ -993,6 +1012,7 @@ describe('embeddable', () => { canSaveVisualizations: true, }, getTrigger, + visualizationMap: {}, documentToExpression: () => Promise.resolve({ ast: { @@ -1016,4 +1036,82 @@ describe('embeddable', () => { expect(onTableRowClick).toHaveBeenCalledWith({ name: 'test' }); expect(onTableRowClick).toHaveBeenCalledTimes(1); }); + + it('handles edit actions ', async () => { + const editedVisualizationState = { value: 'edited' }; + const onEditActionMock = jest.fn().mockReturnValue(editedVisualizationState); + const documentToExpressionMock = jest.fn().mockImplementation(async (document) => { + const isStateEdited = document.state.visualization.value === 'edited'; + return { + ast: { + type: 'expression', + chain: [ + { + type: 'function', + function: isStateEdited ? 'edited' : 'not_edited', + arguments: {}, + }, + ], + }, + errors: undefined, + }; + }); + + const visDocument: Document = { + state: { + visualization: {}, + datasourceStates: {}, + query: { query: '', language: 'lucene' }, + filters: [], + }, + references: [], + title: 'My title', + visualizationType: 'lensDatatable', + }; + + const embeddable = new Embeddable( + { + timefilter: dataPluginMock.createSetupContract().query.timefilter.timefilter, + attributeService: attributeServiceMockFromSavedVis(visDocument), + expressionRenderer, + basePath, + inspector: inspectorPluginMock.createStartContract(), + indexPatternService: {} as IndexPatternsContract, + capabilities: { + canSaveDashboards: true, + canSaveVisualizations: true, + }, + getTrigger, + visualizationMap: { + [visDocument.visualizationType as string]: { + onEditAction: onEditActionMock, + } as unknown as Visualization, + }, + documentToExpression: documentToExpressionMock, + }, + { id: '123' } as unknown as LensEmbeddableInput + ); + + // SETUP FRESH STATE + await embeddable.initializeSavedVis({ id: '123' } as LensEmbeddableInput); + embeddable.render(mountpoint); + + await new Promise((resolve) => setTimeout(resolve, 0)); + + expect(expressionRenderer).toHaveBeenCalledTimes(1); + expect(expressionRenderer.mock.calls[0][0]!.expression).toBe(`not_edited`); + + // TEST EDIT EVENT + await embeddable.handleEvent({ name: 'edit' }); + + expect(onEditActionMock).toHaveBeenCalledTimes(1); + expect(documentToExpressionMock).toHaveBeenCalled(); + + const docToExpCalls = documentToExpressionMock.mock.calls; + const editedVisDocument = docToExpCalls[docToExpCalls.length - 1][0]; + expect(editedVisDocument.state.visualization).toEqual(editedVisualizationState); + + expect(expressionRenderer).toHaveBeenCalledTimes(2); + expect(expressionRenderer.mock.calls[1][0]!.expression).toBe(`edited`); + }); }); diff --git a/x-pack/plugins/lens/public/embeddable/embeddable.tsx b/x-pack/plugins/lens/public/embeddable/embeddable.tsx index 7faf873cf0b0a..563e10bb03abd 100644 --- a/x-pack/plugins/lens/public/embeddable/embeddable.tsx +++ b/x-pack/plugins/lens/public/embeddable/embeddable.tsx @@ -47,10 +47,13 @@ import { UiActionsStart } from '../../../../../src/plugins/ui_actions/public'; import { isLensBrushEvent, isLensFilterEvent, + isLensEditEvent, isLensTableRowContextMenuClickEvent, LensBrushEvent, LensFilterEvent, LensTableRowContextMenuEvent, + VisualizationMap, + Visualization, } from '../types'; import { IndexPatternsContract } from '../../../../../src/plugins/data/public'; @@ -97,6 +100,7 @@ export interface LensEmbeddableDeps { documentToExpression: ( doc: Document ) => Promise<{ ast: Ast | null; errors: ErrorMessage[] | undefined }>; + visualizationMap: VisualizationMap; indexPatternService: IndexPatternsContract; expressionRenderer: ReactExpressionRendererType; timefilter: TimefilterContract; @@ -109,6 +113,17 @@ export interface LensEmbeddableDeps { spaces?: SpacesPluginStart; } +const getExpressionFromDocument = async ( + document: Document, + documentToExpression: LensEmbeddableDeps['documentToExpression'] +) => { + const { ast, errors } = await documentToExpression(document); + return { + expression: ast ? toExpression(ast) : null, + errors, + }; +}; + export class Embeddable extends AbstractEmbeddable implements ReferenceOrValueEmbeddable @@ -260,6 +275,29 @@ export class Embeddable return this.lensInspector.adapters; } + private maybeAddConflictError( + errors: ErrorMessage[], + sharingSavedObjectProps?: SharingSavedObjectProps + ) { + const ret = [...errors]; + + if (sharingSavedObjectProps?.outcome === 'conflict' && !!this.deps.spaces) { + ret.push({ + shortMessage: i18n.translate('xpack.lens.embeddable.legacyURLConflict.shortMessage', { + defaultMessage: `You've encountered a URL conflict`, + }), + longMessage: ( + + ), + }); + } + + return ret; + } + async initializeSavedVis(input: LensEmbeddableInput) { const attrs: ResolvedLensSavedObjectAttributes | false = await this.deps.attributeService .unwrapAttributes(input) @@ -278,23 +316,14 @@ export class Embeddable type: this.type, savedObjectId: (input as LensByReferenceInput)?.savedObjectId, }; - const { ast, errors } = await this.deps.documentToExpression(this.savedVis); - this.errors = errors; - if (sharingSavedObjectProps?.outcome === 'conflict' && this.deps.spaces) { - const conflictError = { - shortMessage: i18n.translate('xpack.lens.embeddable.legacyURLConflict.shortMessage', { - defaultMessage: `You've encountered a URL conflict`, - }), - longMessage: ( - - ), - }; - this.errors = this.errors ? [...this.errors, conflictError] : [conflictError]; - } - this.expression = ast ? toExpression(ast) : null; + + const { expression, errors } = await getExpressionFromDocument( + this.savedVis, + this.deps.documentToExpression + ); + this.expression = expression; + this.errors = errors && this.maybeAddConflictError(errors, sharingSavedObjectProps); + if (this.errors) { this.logError('validation'); } @@ -432,7 +461,17 @@ export class Embeddable return output; } - handleEvent = (event: ExpressionRendererEvent) => { + private get onEditAction(): Visualization['onEditAction'] { + const visType = this.savedVis?.visualizationType; + + if (!visType) { + return; + } + + return this.deps.visualizationMap[visType].onEditAction; + } + + handleEvent = async (event: ExpressionRendererEvent) => { if (!this.deps.getTrigger || this.input.disableTriggers) { return; } @@ -468,9 +507,29 @@ export class Embeddable this.input.onTableRowClick(event.data as unknown as LensTableRowContextMenuEvent['data']); } } + + // We allow for edit actions in the Embeddable for display purposes only (e.g. changing the datatable sort order). + // No state changes made here with an edit action are persisted. + if (isLensEditEvent(event) && this.onEditAction) { + if (!this.savedVis) return; + + // have to dance since this.savedVis.state is readonly + const newVis = JSON.parse(JSON.stringify(this.savedVis)) as Document; + newVis.state.visualization = this.onEditAction(newVis.state.visualization, event); + this.savedVis = newVis; + + const { expression, errors } = await getExpressionFromDocument( + this.savedVis, + this.deps.documentToExpression + ); + this.expression = expression; + this.errors = errors; + + this.reload(); + } }; - async reload() { + reload() { if (!this.savedVis || !this.isInitialized || this.isDestroyed) { return; } diff --git a/x-pack/plugins/lens/public/embeddable/embeddable_factory.ts b/x-pack/plugins/lens/public/embeddable/embeddable_factory.ts index e51ec4c3e5588..811f391e32f9a 100644 --- a/x-pack/plugins/lens/public/embeddable/embeddable_factory.ts +++ b/x-pack/plugins/lens/public/embeddable/embeddable_factory.ts @@ -25,6 +25,7 @@ import { DOC_TYPE } from '../../common/constants'; import { ErrorMessage } from '../editor_frame_service/types'; import { extract, inject } from '../../common/embeddable_factory'; import type { SpacesPluginStart } from '../../../spaces/public'; +import { VisualizationMap } from '../types'; export interface LensEmbeddableStartServices { timefilter: TimefilterContract; @@ -39,6 +40,7 @@ export interface LensEmbeddableStartServices { documentToExpression: ( doc: Document ) => Promise<{ ast: Ast | null; errors: ErrorMessage[] | undefined }>; + visualizationMap: VisualizationMap; spaces?: SpacesPluginStart; } @@ -85,6 +87,7 @@ export class EmbeddableFactory implements EmbeddableFactoryDefinition { timefilter, expressionRenderer, documentToExpression, + visualizationMap, uiActions, coreHttp, attributeService, @@ -108,6 +111,7 @@ export class EmbeddableFactory implements EmbeddableFactoryDefinition { getTrigger: uiActions?.getTrigger, getTriggerCompatibleActions: uiActions?.getTriggerCompatibleActions, documentToExpression, + visualizationMap, capabilities: { canSaveDashboards: Boolean(capabilities.dashboard?.showWriteControls), canSaveVisualizations: Boolean(capabilities.visualize.save), diff --git a/x-pack/plugins/lens/public/plugin.ts b/x-pack/plugins/lens/public/plugin.ts index 1532b2b099104..fb0a922c7e9a2 100644 --- a/x-pack/plugins/lens/public/plugin.ts +++ b/x-pack/plugins/lens/public/plugin.ts @@ -201,6 +201,8 @@ export class LensPlugin { plugins.fieldFormats.deserialize ); + const visualizationMap = await this.editorFrameService!.loadVisualizations(); + return { attributeService: getLensAttributeService(coreStart, plugins), capabilities: coreStart.application.capabilities, @@ -208,6 +210,7 @@ export class LensPlugin { timefilter: plugins.data.query.timefilter.timefilter, expressionRenderer: plugins.expressions.ReactExpressionRenderer, documentToExpression: this.editorFrameService!.documentToExpression, + visualizationMap, indexPatternService: plugins.data.indexPatterns, uiActions: plugins.uiActions, usageCollection, diff --git a/x-pack/test/functional/apps/lens/index.ts b/x-pack/test/functional/apps/lens/index.ts index 79f9b8f645c1a..26fb100adf133 100644 --- a/x-pack/test/functional/apps/lens/index.ts +++ b/x-pack/test/functional/apps/lens/index.ts @@ -70,6 +70,7 @@ export default function ({ getService, loadTestFile, getPageObjects }: FtrProvid this.tags(['ciGroup16', 'skipFirefox']); loadTestFile(require.resolve('./add_to_dashboard')); + loadTestFile(require.resolve('./table_dashboard')); loadTestFile(require.resolve('./table')); loadTestFile(require.resolve('./runtime_fields')); loadTestFile(require.resolve('./dashboard')); diff --git a/x-pack/test/functional/apps/lens/table_dashboard.ts b/x-pack/test/functional/apps/lens/table_dashboard.ts new file mode 100644 index 0000000000000..6e76d816fa6a6 --- /dev/null +++ b/x-pack/test/functional/apps/lens/table_dashboard.ts @@ -0,0 +1,55 @@ +/* + * 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; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import expect from '@kbn/expect'; +import { FtrProviderContext } from '../../ftr_provider_context'; + +export default function ({ getService, getPageObjects }: FtrProviderContext) { + const PageObjects = getPageObjects(['lens', 'visualize', 'dashboard']); + const listingTable = getService('listingTable'); + const retry = getService('retry'); + + const checkTableSorting = async () => { + // Sort by number + await PageObjects.lens.changeTableSortingBy(2, 'ascending'); + await PageObjects.lens.waitForVisualization(); + expect(await PageObjects.lens.getDatatableCellText(0, 2)).to.eql('17,246'); + // Now sort by IP + await PageObjects.lens.changeTableSortingBy(0, 'ascending'); + await PageObjects.lens.waitForVisualization(); + expect(await PageObjects.lens.getDatatableCellText(0, 0)).to.eql('78.83.247.30'); + // Change the sorting + await PageObjects.lens.changeTableSortingBy(0, 'descending'); + await PageObjects.lens.waitForVisualization(); + expect(await PageObjects.lens.getDatatableCellText(0, 0)).to.eql('169.228.188.120'); + // Remove the sorting + await retry.try(async () => { + await PageObjects.lens.changeTableSortingBy(0, 'none'); + await PageObjects.lens.waitForVisualization(); + expect(await PageObjects.lens.isDatatableHeaderSorted(0)).to.eql(false); + }); + }; + + describe('lens table on dashboard', () => { + it('should sort a table by column in dashboard edit mode', async () => { + await PageObjects.visualize.gotoVisualizationLandingPage(); + await listingTable.searchForItemWithName('lnsXYvis'); + await PageObjects.lens.clickVisualizeListItemTitle('lnsXYvis'); + await PageObjects.lens.goToTimeRange(); + await PageObjects.lens.switchToVisualization('lnsDatatable'); + await PageObjects.lens.save('New Table', true, false, false, 'new'); + + await checkTableSorting(); + }); + + it('should sort a table by column in dashboard view mode', async () => { + await PageObjects.dashboard.saveDashboard('Dashboard with a Lens Table'); + + await checkTableSorting(); + }); + }); +}