[Lens] prevent double load in editor (#93930)

This commit is contained in:
Joe Reuter 2021-03-10 09:00:01 +01:00 committed by GitHub
parent 7d7a16883c
commit f9e28831c5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 78 additions and 91 deletions

View file

@ -83,6 +83,7 @@ function createMockSearchService() {
session: {
start: jest.fn(() => `sessionId-${sessionIdCounter++}`),
clear: jest.fn(),
getSessionId: jest.fn(() => `sessionId-${sessionIdCounter}`),
},
};
}

View file

@ -120,7 +120,8 @@ export function App({
const { resolvedDateRange, from: fromDate, to: toDate } = useTimeRange(
data,
state.lastKnownDoc,
setState
setState,
state.searchSessionId
);
const onError = useCallback(

View file

@ -26,15 +26,16 @@ const TIME_LAG_PERCENTAGE_LIMIT = 0.02;
* @param data data plugin contract to manage current now value, time range and session
* @param lastKnownDoc Current state of the editor
* @param setState state setter for Lens app state
* @param searchSessionId current session id
*/
export function useTimeRange(
data: DataPublicPluginStart,
lastKnownDoc: Document | undefined,
setState: React.Dispatch<React.SetStateAction<LensAppState>>
setState: React.Dispatch<React.SetStateAction<LensAppState>>,
searchSessionId: string
) {
const timefilter = data.query.timefilter.timefilter;
const { from, to } = data.query.timefilter.timefilter.getTime();
const currentNow = data.nowProvider.get();
// Need a stable reference for the frame component of the dateRange
const resolvedDateRange = useMemo(() => {
@ -43,10 +44,10 @@ export function useTimeRange(
to,
});
return { fromDate: min?.toISOString() || from, toDate: max?.toISOString() || to };
// recalculate current date range if current "now" value changes because calculateBounds
// depends on it internally
// recalculate current date range if the session gets updated because it
// might change "now" and calculateBounds depends on it internally
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [timefilter, currentNow, from, to]);
}, [timefilter, searchSessionId, from, to]);
useEffect(() => {
const unresolvedTimeRange = timefilter.getTime();

View file

@ -26,6 +26,7 @@ import { EditorFrame } from './editor_frame';
import { DatasourcePublicAPI, DatasourceSuggestion, Visualization } from '../../types';
import { act } from 'react-dom/test-utils';
import { coreMock } from 'src/core/public/mocks';
import { fromExpression } from '@kbn/interpreter/common';
import {
createMockVisualization,
createMockDatasource,
@ -442,42 +443,9 @@ describe('editor_frame', () => {
instance.update();
expect(instance.find(expressionRendererMock).prop('expression')).toMatchInlineSnapshot(`
Object {
"chain": Array [
Object {
"arguments": Object {},
"function": "kibana",
"type": "function",
},
Object {
"arguments": Object {
"layerIds": Array [
"first",
],
"tables": Array [
Object {
"chain": Array [
Object {
"arguments": Object {},
"function": "datasource",
"type": "function",
},
],
"type": "expression",
},
],
},
"function": "lens_merge_tables",
"type": "function",
},
Object {
"arguments": Object {},
"function": "vis",
"type": "function",
},
],
"type": "expression",
}
"kibana
| lens_merge_tables layerIds=\\"first\\" tables={datasource}
| vis"
`);
});
@ -525,7 +493,9 @@ describe('editor_frame', () => {
instance.update();
expect(instance.find(expressionRendererMock).prop('expression')).toEqual({
expect(
fromExpression(instance.find(expressionRendererMock).prop('expression') as string)
).toEqual({
type: 'expression',
chain: expect.arrayContaining([
expect.objectContaining({
@ -533,7 +503,8 @@ describe('editor_frame', () => {
}),
]),
});
expect(instance.find(expressionRendererMock).prop('expression')).toMatchInlineSnapshot(`
expect(fromExpression(instance.find(expressionRendererMock).prop('expression') as string))
.toMatchInlineSnapshot(`
Object {
"chain": Array [
Object {

View file

@ -27,7 +27,7 @@ import { WorkspacePanel, WorkspacePanelProps } from './workspace_panel';
import { mountWithIntl as mount } from '@kbn/test/jest';
import { ReactWrapper } from 'enzyme';
import { DragDrop, ChildDragDropProvider } from '../../../drag_drop';
import { Ast } from '@kbn/interpreter/common';
import { fromExpression } from '@kbn/interpreter/common';
import { coreMock } from 'src/core/public/mocks';
import {
DataPublicPluginStart,
@ -177,42 +177,9 @@ describe('workspace_panel', () => {
);
expect(instance.find(expressionRendererMock).prop('expression')).toMatchInlineSnapshot(`
Object {
"chain": Array [
Object {
"arguments": Object {},
"function": "kibana",
"type": "function",
},
Object {
"arguments": Object {
"layerIds": Array [
"first",
],
"tables": Array [
Object {
"chain": Array [
Object {
"arguments": Object {},
"function": "datasource",
"type": "function",
},
],
"type": "expression",
},
],
},
"function": "lens_merge_tables",
"type": "function",
},
Object {
"arguments": Object {},
"function": "vis",
"type": "function",
},
],
"type": "expression",
}
"kibana
| lens_merge_tables layerIds=\\"first\\" tables={datasource}
| vis"
`);
});
@ -346,12 +313,10 @@ describe('workspace_panel', () => {
/>
);
expect(
(instance.find(expressionRendererMock).prop('expression') as Ast).chain[1].arguments.layerIds
).toEqual(['first', 'second', 'third']);
expect(
(instance.find(expressionRendererMock).prop('expression') as Ast).chain[1].arguments.tables
).toMatchInlineSnapshot(`
const ast = fromExpression(instance.find(expressionRendererMock).prop('expression') as string);
expect(ast.chain[1].arguments.layerIds).toEqual(['first', 'second', 'third']);
expect(ast.chain[1].arguments.tables).toMatchInlineSnapshot(`
Array [
Object {
"chain": Array [

View file

@ -8,7 +8,7 @@
import React, { useState, useEffect, useMemo, useContext, useCallback } from 'react';
import classNames from 'classnames';
import { FormattedMessage } from '@kbn/i18n/react';
import { Ast } from '@kbn/interpreter/common';
import { toExpression } from '@kbn/interpreter/common';
import { i18n } from '@kbn/i18n';
import {
EuiEmptyPrompt,
@ -159,13 +159,21 @@ export const InnerWorkspacePanel = React.memo(function InnerWorkspacePanel({
() => {
if (!configurationValidationError?.length) {
try {
return buildExpression({
const ast = buildExpression({
visualization: activeVisualization,
visualizationState,
datasourceMap,
datasourceStates,
datasourceLayers: framePublicAPI.datasourceLayers,
});
if (ast) {
// expression has to be turned into a string for dirty checking - if the ast is rebuilt,
// turning it into a string will make sure the expression renderer only re-renders if the
// expression actually changed.
return toExpression(ast);
} else {
return null;
}
} catch (e) {
const buildMessages = activeVisualization?.getErrorMessages(visualizationState);
const defaultMessage = {
@ -347,7 +355,7 @@ export const InnerVisualizationWrapper = ({
ExpressionRendererComponent,
dispatch,
}: {
expression: Ast | null | undefined;
expression: string | null | undefined;
framePublicAPI: FramePublicAPI;
timefilter: TimefilterContract;
onEvent: (event: ExpressionRendererEvent) => void;

View file

@ -299,6 +299,45 @@ describe('embeddable', () => {
expect(expressionRenderer).toHaveBeenCalledTimes(2);
});
it('should re-render once if session id changes and ', async () => {
const embeddable = new Embeddable(
{
timefilter: dataPluginMock.createSetupContract().query.timefilter.timefilter,
attributeService,
expressionRenderer,
basePath,
indexPatternService: {} as IndexPatternsContract,
editable: true,
getTrigger,
documentToExpression: () =>
Promise.resolve({
ast: {
type: 'expression',
chain: [
{ type: 'function', function: 'my', arguments: {} },
{ type: 'function', function: 'expression', arguments: {} },
],
},
errors: undefined,
}),
},
{ id: '123' } as LensEmbeddableInput
);
await embeddable.initializeSavedVis({ id: '123' } as LensEmbeddableInput);
embeddable.render(mountpoint);
expect(expressionRenderer).toHaveBeenCalledTimes(1);
embeddable.updateInput({
searchSessionId: 'newSession',
});
embeddable.reload();
await new Promise((resolve) => setTimeout(resolve, 0));
expect(expressionRenderer).toHaveBeenCalledTimes(2);
});
it('should re-render when dashboard view/edit mode changes', async () => {
const embeddable = new Embeddable(
{

View file

@ -178,7 +178,8 @@ export class Embeddable
});
// Update search context and reload on changes related to search
input$
this.getUpdated$()
.pipe(map(() => this.getInput()))
.pipe(
distinctUntilChanged((a, b) =>
isEqual(