Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Lens] Integrate searchSessionId into Lens app #86297

Merged
merged 11 commits into from
Dec 29, 2020
133 changes: 131 additions & 2 deletions x-pack/plugins/lens/public/app_plugin/app.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,16 @@ function createMockFrame(): jest.Mocked<EditorFrameInstance> {
};
}

function createMockSearchService() {
let sessionIdCounter = 1;
return {
session: {
start: jest.fn(() => `sessionId-${sessionIdCounter++}`),
clear: jest.fn(),
},
};
}

function createMockFilterManager() {
const unsubscribe = jest.fn();

Expand Down Expand Up @@ -118,16 +128,29 @@ function createMockQueryString() {
function createMockTimefilter() {
const unsubscribe = jest.fn();

let timeFilter = { from: 'now-7d', to: 'now' };
let subscriber: () => void;
return {
getTime: jest.fn(() => ({ from: 'now-7d', to: 'now' })),
setTime: jest.fn(),
getTime: jest.fn(() => timeFilter),
setTime: jest.fn((newTimeFilter) => {
timeFilter = newTimeFilter;
if (subscriber) {
subscriber();
}
}),
getTimeUpdate$: () => ({
subscribe: ({ next }: { next: () => void }) => {
subscriber = next;
return unsubscribe;
},
}),
getRefreshInterval: () => {},
getRefreshIntervalDefaults: () => {},
getAutoRefreshFetch$: () => ({
subscribe: ({ next }: { next: () => void }) => {
return next;
},
}),
};
}

Expand Down Expand Up @@ -209,6 +232,7 @@ describe('Lens App', () => {
return new Promise((resolve) => resolve({ id }));
}),
},
search: createMockSearchService(),
} as unknown) as DataPublicPluginStart,
storage: {
get: jest.fn(),
Expand Down Expand Up @@ -295,6 +319,7 @@ describe('Lens App', () => {
"query": "",
},
"savedQuery": undefined,
"searchSessionId": "sessionId-1",
"showNoDataPopover": [Function],
},
],
Expand Down Expand Up @@ -1072,6 +1097,53 @@ describe('Lens App', () => {
})
);
});

it('updates the searchSessionId when the user changes query or time in the search bar', () => {
const { component, frame, services } = mountWith({});
act(() =>
component.find(TopNavMenu).prop('onQuerySubmit')!({
dateRange: { from: 'now-14d', to: 'now-7d' },
query: { query: '', language: 'lucene' },
})
);
component.update();
expect(frame.mount).toHaveBeenCalledWith(
expect.any(Element),
expect.objectContaining({
searchSessionId: `sessionId-1`,
})
);

// trigger again, this time changing just the query
act(() =>
component.find(TopNavMenu).prop('onQuerySubmit')!({
dateRange: { from: 'now-14d', to: 'now-7d' },
query: { query: 'new', language: 'lucene' },
})
);
component.update();
expect(frame.mount).toHaveBeenCalledWith(
expect.any(Element),
expect.objectContaining({
searchSessionId: `sessionId-2`,
})
);

const indexPattern = ({ id: 'index1' } as unknown) as IIndexPattern;
const field = ({ name: 'myfield' } as unknown) as IFieldType;
act(() =>
services.data.query.filterManager.setFilters([
esFilters.buildExistsFilter(field, indexPattern),
])
);
component.update();
expect(frame.mount).toHaveBeenCalledWith(
expect.any(Element),
expect.objectContaining({
searchSessionId: `sessionId-3`,
})
);
});
});

describe('saved query handling', () => {
Expand Down Expand Up @@ -1165,6 +1237,37 @@ describe('Lens App', () => {
);
});

it('updates the searchSessionId when the query is updated', () => {
const { component, frame } = mountWith({});
act(() => {
component.find(TopNavMenu).prop('onSaved')!({
id: '1',
attributes: {
title: '',
description: '',
query: { query: '', language: 'lucene' },
},
});
});
act(() => {
component.find(TopNavMenu).prop('onSavedQueryUpdated')!({
id: '2',
attributes: {
title: 'new title',
description: '',
query: { query: '', language: 'lucene' },
},
});
});
component.update();
expect(frame.mount).toHaveBeenCalledWith(
expect.any(Element),
expect.objectContaining({
searchSessionId: `sessionId-1`,
})
);
});

it('clears all existing unpinned filters when the active saved query is cleared', () => {
const { component, frame, services } = mountWith({});
act(() =>
Expand All @@ -1190,6 +1293,32 @@ describe('Lens App', () => {
})
);
});

it('updates the searchSessionId when the active saved query is cleared', () => {
const { component, frame, services } = mountWith({});
act(() =>
component.find(TopNavMenu).prop('onQuerySubmit')!({
dateRange: { from: 'now-14d', to: 'now-7d' },
query: { query: 'new', language: 'lucene' },
})
);
const indexPattern = ({ id: 'index1' } as unknown) as IIndexPattern;
const field = ({ name: 'myfield' } as unknown) as IFieldType;
const pinnedField = ({ name: 'pinnedField' } as unknown) as IFieldType;
const unpinned = esFilters.buildExistsFilter(field, indexPattern);
const pinned = esFilters.buildExistsFilter(pinnedField, indexPattern);
FilterManager.setFiltersStore([pinned], esFilters.FilterStateStore.GLOBAL_STATE);
act(() => services.data.query.filterManager.setFilters([pinned, unpinned]));
component.update();
act(() => component.find(TopNavMenu).prop('onClearSavedQuery')!());
component.update();
expect(frame.mount).toHaveBeenCalledWith(
expect.any(Element),
expect.objectContaining({
searchSessionId: `sessionId-2`,
})
);
});
});

describe('showing a confirm message when leaving', () => {
Expand Down
68 changes: 38 additions & 30 deletions x-pack/plugins/lens/public/app_plugin/app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import './app.scss';

import _ from 'lodash';
import React, { useState, useEffect, useCallback } from 'react';
import React, { useState, useEffect, useCallback, useMemo } from 'react';
import { i18n } from '@kbn/i18n';
import { NotificationsStart } from 'kibana/public';
import { EuiBreadcrumb } from '@elastic/eui';
Expand Down Expand Up @@ -71,7 +71,6 @@ export function App({
} = useKibana<LensAppServices>().services;

const [state, setState] = useState<LensAppState>(() => {
const currentRange = data.query.timefilter.timefilter.getTime();
return {
query: data.query.queryString.getQuery(),
// Do not use app-specific filters from previous app,
Expand All @@ -81,14 +80,11 @@ export function App({
: data.query.filterManager.getFilters(),
isLoading: Boolean(initialInput),
indexPatternsForTopNav: [],
dateRange: {
fromDate: currentRange.from,
toDate: currentRange.to,
},
isLinkedToOriginatingApp: Boolean(incomingState?.originatingApp),
isSaveModalVisible: false,
indicateNoData: false,
isSaveable: false,
searchSessionId: data.search.session.start(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remove dateRange from the LensAppState now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved the dateRange out of state in the last commit. An object reference needs to be still hold for the frame component, but there's a single source of truth now for it.

};
});

Expand All @@ -107,10 +103,14 @@ export function App({
state.indicateNoData,
state.query,
state.filters,
state.dateRange,
state.indexPatternsForTopNav,
state.searchSessionId,
]);

// Need a stable reference for the frame component of the dateRange
const { from: fromDate, to: toDate } = data.query.timefilter.timefilter.getTime();
const currentDateRange = useMemo(() => ({ fromDate, toDate }), [fromDate, toDate]);

const onError = useCallback(
(e: { message: string }) =>
notifications.toasts.addDanger({
Expand Down Expand Up @@ -160,24 +160,35 @@ export function App({

const filterSubscription = data.query.filterManager.getUpdates$().subscribe({
next: () => {
setState((s) => ({ ...s, filters: data.query.filterManager.getFilters() }));
setState((s) => ({
...s,
filters: data.query.filterManager.getFilters(),
searchSessionId: data.search.session.start(),
}));
trackUiEvent('app_filters_updated');
},
});

const timeSubscription = data.query.timefilter.timefilter.getTimeUpdate$().subscribe({
next: () => {
const currentRange = data.query.timefilter.timefilter.getTime();
setState((s) => ({
...s,
dateRange: {
fromDate: currentRange.from,
toDate: currentRange.to,
},
searchSessionId: data.search.session.start(),
}));
},
});

const autoRefreshSubscription = data.query.timefilter.timefilter
.getAutoRefreshFetch$()
.subscribe({
next: () => {
setState((s) => ({
...s,
searchSessionId: data.search.session.start(),
}));
},
});

const kbnUrlStateStorage = createKbnUrlStateStorage({
history,
useHash: uiSettings.get('state:storeInSessionStorage'),
Expand All @@ -192,10 +203,12 @@ export function App({
stopSyncingQueryServiceStateWithUrl();
filterSubscription.unsubscribe();
timeSubscription.unsubscribe();
autoRefreshSubscription.unsubscribe();
};
}, [
data.query.filterManager,
data.query.timefilter.timefilter,
data.search.session,
notifications.toasts,
uiSettings,
data.query,
Expand Down Expand Up @@ -594,21 +607,21 @@ export function App({
appName={'lens'}
onQuerySubmit={(payload) => {
const { dateRange, query } = payload;
if (
dateRange.from !== state.dateRange.fromDate ||
dateRange.to !== state.dateRange.toDate
) {
const currentRange = data.query.timefilter.timefilter.getTime();
if (dateRange.from !== currentRange.from || dateRange.to !== currentRange.to) {
data.query.timefilter.timefilter.setTime(dateRange);
trackUiEvent('app_date_change');
} else {
// Query has changed, renew the session id.
// Time change will be picked up by the time subscription
setState((s) => ({
...s,
searchSessionId: data.search.session.start(),
}));
trackUiEvent('app_query_change');
}
setState((s) => ({
...s,
dateRange: {
fromDate: dateRange.from,
toDate: dateRange.to,
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we're using the timeFilter manager instead now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code was already using the timeFilter, but this code here was covering the scenario where the data didn't change but we want the state to trigger a new render ("Refresh" click).
Previously when the dateRange values changed the state was update twice with the same content, triggering a double rendering. Now this scenario is covered by the timeFilter subscription.
In case of no dateRange change ("Refresh" button), the session id is regenerated triggering a re-render now.

We still need the dateRange state, but we can reduce the number of time we update it.

query: query || s.query,
}));
}}
Expand All @@ -622,12 +635,6 @@ export function App({
setState((s) => ({
...s,
savedQuery: { ...savedQuery }, // Shallow query for reference issues
dateRange: savedQuery.attributes.timefilter
? {
fromDate: savedQuery.attributes.timefilter.from,
toDate: savedQuery.attributes.timefilter.to,
}
: s.dateRange,
}));
}}
onClearSavedQuery={() => {
Expand All @@ -640,8 +647,8 @@ export function App({
}));
}}
query={state.query}
dateRangeFrom={state.dateRange.fromDate}
dateRangeTo={state.dateRange.toDate}
dateRangeFrom={fromDate}
dateRangeTo={toDate}
indicateNoData={state.indicateNoData}
/>
</div>
Expand All @@ -650,7 +657,8 @@ export function App({
className="lnsApp__frame"
render={editorFrame.mount}
nativeProps={{
dateRange: state.dateRange,
searchSessionId: state.searchSessionId,
dateRange: currentDateRange,
query: state.query,
filters: state.filters,
savedQuery: state.savedQuery,
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/lens/public/app_plugin/mounter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,7 @@ export async function mountApp(
params.element
);
return () => {
data.search.session.clear();
instance.unmount();
unmountComponentAtNode(params.element);
unlistenParentHistory();
Expand Down
6 changes: 1 addition & 5 deletions x-pack/plugins/lens/public/app_plugin/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,16 +55,12 @@ export interface LensAppState {
// Determines whether the lens editor shows the 'save and return' button, and the originating app breadcrumb.
isLinkedToOriginatingApp?: boolean;

// Properties needed to interface with TopNav
dateRange: {
fromDate: string;
toDate: string;
};
query: Query;
filters: Filter[];
savedQuery?: SavedQuery;
isSaveable: boolean;
activeData?: TableInspectorAdapter;
searchSessionId: string;
}

export interface RedirectToOriginProps {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ function getDefaultProps() {
},
palettes: chartPluginMock.createPaletteRegistry(),
showNoDataPopover: jest.fn(),
searchSessionId: 'sessionId',
};
}

Expand Down Expand Up @@ -264,6 +265,7 @@ describe('editor_frame', () => {
filters: [],
dateRange: { fromDate: 'now-7d', toDate: 'now' },
availablePalettes: defaultProps.palettes,
searchSessionId: 'sessionId',
});
});

Expand Down
Loading