Skip to content

Commit

Permalink
Merge branch 'main' into fieldlist-error
Browse files Browse the repository at this point in the history
  • Loading branch information
jughosta committed Apr 26, 2023
2 parents b847be6 + c0033a1 commit fc94cb0
Show file tree
Hide file tree
Showing 79 changed files with 2,785 additions and 246 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@
*/

import {
ContactCardEmbeddable,
ContactCardEmbeddableFactory,
ContactCardEmbeddableInput,
ContactCardEmbeddableOutput,
CONTACT_CARD_EMBEDDABLE,
} from '@kbn/embeddable-plugin/public/lib/test_samples';
import {
Expand Down Expand Up @@ -253,3 +256,63 @@ test('creates a control group from the control group factory and waits for it to
);
expect(mockControlGroupContainer.untilInitialized).toHaveBeenCalled();
});

/*
* dashboard.getInput$() subscriptions are used to update:
* 1) dashboard instance searchSessionId state
* 2) child input on parent input changes
*
* Rxjs subscriptions are executed in the order that they are created.
* This test ensures that searchSessionId update subscription is created before child input subscription
* to ensure child input subscription includes updated searchSessionId.
*/
test('searchSessionId is updated prior to child embeddable parent subscription execution', async () => {
const embeddableFactory = {
create: new ContactCardEmbeddableFactory((() => null) as any, {} as any),
getDefaultInput: jest.fn().mockResolvedValue({
timeRange: {
to: 'now',
from: 'now-15m',
},
}),
};
pluginServices.getServices().embeddable.getEmbeddableFactory = jest
.fn()
.mockReturnValue(embeddableFactory);
let sessionCount = 0;
pluginServices.getServices().data.search.session.start = () => {
sessionCount++;
return `searchSessionId${sessionCount}`;
};
const dashboard = await createDashboard(embeddableId, {
searchSessionSettings: {
getSearchSessionIdFromURL: () => undefined,
removeSessionIdFromUrl: () => {},
createSessionRestorationDataProvider: () => {},
} as unknown as DashboardCreationOptions['searchSessionSettings'],
});
const embeddable = await dashboard.addNewEmbeddable<
ContactCardEmbeddableInput,
ContactCardEmbeddableOutput,
ContactCardEmbeddable
>(CONTACT_CARD_EMBEDDABLE, {
firstName: 'Bob',
});

expect(embeddable.getInput().searchSessionId).toBe('searchSessionId1');

dashboard.updateInput({
timeRange: {
to: 'now',
from: 'now-7d',
},
});

expect(sessionCount).toBeGreaterThan(1);
const embeddableInput = embeddable.getInput();
expect((embeddableInput as any).timeRange).toEqual({
to: 'now',
from: 'now-7d',
});
expect(embeddableInput.searchSessionId).toBe(`searchSessionId${sessionCount}`);
});
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,7 @@ export const createDashboard = async (
// --------------------------------------------------------------------------------------
// Set up search sessions integration.
// --------------------------------------------------------------------------------------
let initialSearchSessionId;
if (searchSessionSettings) {
const { sessionIdToRestore } = searchSessionSettings;

Expand All @@ -229,7 +230,7 @@ export const createDashboard = async (
}
const existingSession = session.getSessionId();

const initialSearchSessionId =
initialSearchSessionId =
sessionIdToRestore ??
(existingSession && incomingEmbeddable ? existingSession : session.start());

Expand All @@ -238,7 +239,6 @@ export const createDashboard = async (
creationOptions?.searchSessionSettings
);
});
initialInput.searchSessionId = initialSearchSessionId;
}

// --------------------------------------------------------------------------------------
Expand Down Expand Up @@ -284,6 +284,7 @@ export const createDashboard = async (
const dashboardContainer = new DashboardContainer(
initialInput,
reduxEmbeddablePackage,
initialSearchSessionId,
savedObjectResult?.dashboardInput,
dashboardCreationStartTime,
undefined,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,13 @@
* Side Public License, v 1.
*/

import { debounceTime, pairwise, skip } from 'rxjs/operators';
import { pairwise, skip } from 'rxjs/operators';

import { noSearchSessionStorageCapabilityMessage } from '@kbn/data-plugin/public';

import { DashboardContainer } from '../../dashboard_container';
import { DashboardContainerInput } from '../../../../../common';
import { pluginServices } from '../../../../services/plugin_services';
import { CHANGE_CHECK_DEBOUNCE } from '../../../../dashboard_constants';
import { DashboardCreationOptions } from '../../dashboard_container_factory';
import { getShouldRefresh } from '../../../state/diffing/dashboard_diffing_integration';

Expand Down Expand Up @@ -57,10 +56,10 @@ export function startDashboardSearchSessionIntegration(

// listen to and compare states to determine when to launch a new session.
this.getInput$()
.pipe(pairwise(), debounceTime(CHANGE_CHECK_DEBOUNCE))
.subscribe(async (states) => {
.pipe(pairwise())
.subscribe((states) => {
const [previous, current] = states as DashboardContainerInput[];
const shouldRefetch = await getShouldRefresh.bind(this)(previous, current);
const shouldRefetch = getShouldRefresh.bind(this)(previous, current);
if (!shouldRefetch) return;

const currentSearchSessionId = this.getState().explicitInput.searchSessionId;
Expand All @@ -83,7 +82,7 @@ export function startDashboardSearchSessionIntegration(
})();

if (updatedSearchSessionId && updatedSearchSessionId !== currentSearchSessionId) {
this.dispatch.setSearchSessionId(updatedSearchSessionId);
this.searchSessionId = updatedSearchSessionId;
}
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import React from 'react';
import { mount } from 'enzyme';

import { mockedReduxEmbeddablePackage } from '@kbn/presentation-util-plugin/public/mocks';
import { findTestSubject, nextTick } from '@kbn/test-jest-helpers';
import { I18nProvider } from '@kbn/i18n-react';
import {
Expand All @@ -29,9 +30,10 @@ import { applicationServiceMock, coreMock } from '@kbn/core/public/mocks';
import { uiActionsPluginMock } from '@kbn/ui-actions-plugin/public/mocks';
import { createEditModeActionDefinition } from '@kbn/embeddable-plugin/public/lib/test_samples';

import { buildMockDashboard, getSampleDashboardPanel } from '../../mocks';
import { buildMockDashboard, getSampleDashboardInput, getSampleDashboardPanel } from '../../mocks';
import { pluginServices } from '../../services/plugin_services';
import { ApplicationStart } from '@kbn/core-application-browser';
import { DashboardContainer } from './dashboard_container';

const theme = coreMock.createStart().theme;
let application: ApplicationStart | undefined;
Expand Down Expand Up @@ -171,7 +173,11 @@ test('Container view mode change propagates to new children', async () => {

test('searchSessionId propagates to children', async () => {
const searchSessionId1 = 'searchSessionId1';
const container = buildMockDashboard({ searchSessionId: searchSessionId1 });
const container = new DashboardContainer(
getSampleDashboardInput(),
mockedReduxEmbeddablePackage,
searchSessionId1
);
const embeddable = await container.addNewEmbeddable<
ContactCardEmbeddableInput,
ContactCardEmbeddableOutput,
Expand All @@ -181,11 +187,6 @@ test('searchSessionId propagates to children', async () => {
});

expect(embeddable.getInput().searchSessionId).toBe(searchSessionId1);

const searchSessionId2 = 'searchSessionId2';
container.updateInput({ searchSessionId: searchSessionId2 });

expect(embeddable.getInput().searchSessionId).toBe(searchSessionId2);
});

test('DashboardContainer in edit mode shows edit mode actions', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ export class DashboardContainer extends Container<InheritedChildInput, Dashboard
public subscriptions: Subscription = new Subscription();
public controlGroup?: ControlGroupContainer;

public searchSessionId?: string;

// cleanup
public stopSyncingWithUnifiedSearch?: () => void;
private cleanupStateTools: () => void;
Expand All @@ -117,6 +119,7 @@ export class DashboardContainer extends Container<InheritedChildInput, Dashboard
constructor(
initialInput: DashboardContainerInput,
reduxToolsPackage: ReduxToolsPackage,
initialSessionId?: string,
initialLastSavedInput?: DashboardContainerInput,
dashboardCreationStartTime?: number,
parent?: Container,
Expand Down Expand Up @@ -146,6 +149,7 @@ export class DashboardContainer extends Container<InheritedChildInput, Dashboard
} = pluginServices.getServices());

this.creationOptions = creationOptions;
this.searchSessionId = initialSessionId;
this.dashboardCreationStartTime = dashboardCreationStartTime;

// start diffing dashboard state
Expand Down Expand Up @@ -244,7 +248,6 @@ export class DashboardContainer extends Container<InheritedChildInput, Dashboard
syncColors,
syncTooltips,
hidePanelTitles,
searchSessionId,
refreshInterval,
executionContext,
} = this.input;
Expand All @@ -254,10 +257,10 @@ export class DashboardContainer extends Container<InheritedChildInput, Dashboard
combinedFilters = combineDashboardFiltersWithControlGroupFilters(filters, this.controlGroup);
}
return {
searchSessionId: this.searchSessionId,
refreshConfig: refreshInterval,
filters: combinedFilters,
hidePanelTitles,
searchSessionId,
executionContext,
syncTooltips,
syncColors,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,13 +99,6 @@ export const dashboardContainerReducers = {
state.explicitInput.title = action.payload;
},

setSearchSessionId: (
state: DashboardReduxState,
action: PayloadAction<DashboardContainerInput['searchSessionId']>
) => {
state.explicitInput.searchSessionId = action.payload;
},

// ------------------------------------------------------------------------------
// Unsaved Changes Reducers
// ------------------------------------------------------------------------------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export type DashboardDiffFunctions = {
) => boolean | Promise<boolean>;
};

export const isKeyEqual = async (
export const isKeyEqualAsync = async (
key: keyof DashboardContainerInput,
diffFunctionProps: DiffFunctionProps<typeof key>,
diffingFunctions: DashboardDiffFunctions
Expand All @@ -52,6 +52,25 @@ export const isKeyEqual = async (
return fastIsEqual(diffFunctionProps.currentValue, diffFunctionProps.lastValue);
};

export const isKeyEqual = (
key: keyof Omit<DashboardContainerInput, 'panels'>, // only Panels is async
diffFunctionProps: DiffFunctionProps<typeof key>,
diffingFunctions: DashboardDiffFunctions
) => {
const propsAsNever = diffFunctionProps as never; // todo figure out why props has conflicting types in some constituents.
const diffingFunction = diffingFunctions[key];
if (!diffingFunction) {
return fastIsEqual(diffFunctionProps.currentValue, diffFunctionProps.lastValue);
}

if (diffingFunction?.prototype?.name === 'AsyncFunction') {
throw new Error(
`The function for key "${key}" is async, must use isKeyEqualAsync for asynchronous functions`
);
}
return diffingFunction(propsAsNever);
};

/**
* A collection of functions which diff individual keys of dashboard state. If a key is missing from this list it is
* diffed by the default diffing function, fastIsEqual.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,55 +29,55 @@ describe('getShouldRefresh', () => {
);

describe('filter changes', () => {
test('should return false when filters do not change', async () => {
test('should return false when filters do not change', () => {
const lastInput = {
filters: [existsFilter],
} as unknown as DashboardContainerInput;
expect(await getShouldRefresh.bind(dashboardContainerMock)(lastInput, lastInput)).toBe(false);
expect(getShouldRefresh.bind(dashboardContainerMock)(lastInput, lastInput)).toBe(false);
});

test('should return true when pinned filters change', async () => {
test('should return true when pinned filters change', () => {
const pinnedFilter = pinFilter(existsFilter);
const lastInput = {
filters: [pinnedFilter],
} as unknown as DashboardContainerInput;
const input = {
filters: [toggleFilterNegated(pinnedFilter)],
} as unknown as DashboardContainerInput;
expect(await getShouldRefresh.bind(dashboardContainerMock)(lastInput, input)).toBe(true);
expect(getShouldRefresh.bind(dashboardContainerMock)(lastInput, input)).toBe(true);
});

test('should return false when disabled filters change', async () => {
test('should return false when disabled filters change', () => {
const disabledFilter = disableFilter(existsFilter);
const lastInput = {
filters: [disabledFilter],
} as unknown as DashboardContainerInput;
const input = {
filters: [toggleFilterNegated(disabledFilter)],
} as unknown as DashboardContainerInput;
expect(await getShouldRefresh.bind(dashboardContainerMock)(lastInput, input)).toBe(false);
expect(getShouldRefresh.bind(dashboardContainerMock)(lastInput, input)).toBe(false);
});

test('should return false when pinned filter changes to unpinned', async () => {
test('should return false when pinned filter changes to unpinned', () => {
const lastInput = {
filters: [existsFilter],
} as unknown as DashboardContainerInput;
const input = {
filters: [pinFilter(existsFilter)],
} as unknown as DashboardContainerInput;
expect(await getShouldRefresh.bind(dashboardContainerMock)(lastInput, input)).toBe(false);
expect(getShouldRefresh.bind(dashboardContainerMock)(lastInput, input)).toBe(false);
});
});

describe('timeRange changes', () => {
test('should return false when timeRange does not change', async () => {
test('should return false when timeRange does not change', () => {
const lastInput = {
timeRange: { from: 'now-15m', to: 'now' },
} as unknown as DashboardContainerInput;
expect(await getShouldRefresh.bind(dashboardContainerMock)(lastInput, lastInput)).toBe(false);
expect(getShouldRefresh.bind(dashboardContainerMock)(lastInput, lastInput)).toBe(false);
});

test('should return true when timeRange changes (timeRestore is true)', async () => {
test('should return true when timeRange changes (timeRestore is true)', () => {
const lastInput = {
timeRange: { from: 'now-15m', to: 'now' },
timeRestore: true,
Expand All @@ -86,10 +86,10 @@ describe('getShouldRefresh', () => {
timeRange: { from: 'now-30m', to: 'now' },
timeRestore: true,
} as unknown as DashboardContainerInput;
expect(await getShouldRefresh.bind(dashboardContainerMock)(lastInput, input)).toBe(true);
expect(getShouldRefresh.bind(dashboardContainerMock)(lastInput, input)).toBe(true);
});

test('should return true when timeRange changes (timeRestore is false)', async () => {
test('should return true when timeRange changes (timeRestore is false)', () => {
const lastInput = {
timeRange: { from: 'now-15m', to: 'now' },
timeRestore: false,
Expand All @@ -98,7 +98,26 @@ describe('getShouldRefresh', () => {
timeRange: { from: 'now-30m', to: 'now' },
timeRestore: false,
} as unknown as DashboardContainerInput;
expect(await getShouldRefresh.bind(dashboardContainerMock)(lastInput, input)).toBe(true);
expect(getShouldRefresh.bind(dashboardContainerMock)(lastInput, input)).toBe(true);
});
});

describe('key without custom diffing function (syncColors)', () => {
test('should return false when syncColors do not change', () => {
const lastInput = {
syncColors: false,
} as unknown as DashboardContainerInput;
expect(getShouldRefresh.bind(dashboardContainerMock)(lastInput, lastInput)).toBe(false);
});

test('should return true when syncColors change', () => {
const lastInput = {
syncColors: false,
} as unknown as DashboardContainerInput;
const input = {
syncColors: true,
} as unknown as DashboardContainerInput;
expect(getShouldRefresh.bind(dashboardContainerMock)(lastInput, input)).toBe(true);
});
});
});
Loading

0 comments on commit fc94cb0

Please sign in to comment.