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

[Search Sessions] Improve session restoration back button #87635

Merged
Merged
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
e3aa170
wip: search session restoration and back button
Dosant Jan 7, 2021
41cbf7d
clean up
Dosant Jan 7, 2021
1603eaa
unit tests
Dosant Jan 7, 2021
1dbb919
fix
Dosant Jan 7, 2021
235451a
Merge branch 'master' of github.com:elastic/kibana into dev/search/re…
Dosant Jan 7, 2021
ec36856
Merge branch 'master' into dev/search/restore-search-session-back-button
kibanamachine Jan 11, 2021
a41ee33
Merge branch 'master' of github.com:elastic/kibana into dev/search/re…
Dosant Jan 12, 2021
44998d5
Merge branch 'master' of github.com:elastic/kibana into dev/search/re…
Dosant Jan 12, 2021
16cadae
Merge branch 'master' of github.com:elastic/kibana into dev/search/re…
Dosant Jan 12, 2021
9a638da
Merge branch 'master' into dev/search/restore-search-session-back-button
kibanamachine Jan 13, 2021
c1f3d79
Merge branch 'dev/search/restore-search-session-back-button' of githu…
Dosant Jan 15, 2021
573640c
Merge branch 'master' of github.com:elastic/kibana into dev/search/re…
Dosant Jan 15, 2021
01b2f0f
improve
Dosant Jan 15, 2021
c4ff5a1
add tests
Dosant Jan 15, 2021
e75f381
Merge branch 'master' of github.com:elastic/kibana into dev/search/re…
Dosant Jan 18, 2021
3967bdc
fix session id and back button in discover
Dosant Jan 18, 2021
586ed47
remove usused functions
Dosant Jan 18, 2021
419f443
Merge branch 'master' of github.com:elastic/kibana into dev/search/re…
Dosant Jan 18, 2021
931615e
improve
Dosant Jan 18, 2021
db9119d
remove index pattern workaround from session test
Dosant Jan 18, 2021
822a3cc
fix loadOnInitialLoad prop
Dosant Jan 18, 2021
9ff8dab
Merge branch 'master' of github.com:elastic/kibana into dev/search/re…
Dosant Jan 18, 2021
0d2425e
fix initial query param emit
Dosant Jan 19, 2021
d2b0664
Merge branch 'master' of github.com:elastic/kibana into dev/search/re…
Dosant Jan 19, 2021
1f55a8e
fix typo in test names
Dosant Jan 19, 2021
978eb3b
fix edge case with initial emit
Dosant Jan 19, 2021
8076a9b
Merge branch 'master' of github.com:elastic/kibana into dev/search/re…
Dosant Jan 20, 2021
9f35e91
license header
Dosant Jan 20, 2021
15fa884
shouldSearchOnPageLoad -> true in case there is a searchSessionid in …
Dosant Jan 20, 2021
0ce788c
better comments
Dosant Jan 20, 2021
8c6d8e7
Merge branch 'master' of github.com:elastic/kibana into dev/search/re…
Dosant Jan 21, 2021
c726a86
fix bad tests merge
Dosant Jan 21, 2021
35a6b8e
Merge branch 'master' into dev/search/restore-search-session-back-button
kibanamachine Jan 25, 2021
6944ac1
Merge branch 'master' into dev/search/restore-search-session-back-button
kibanamachine Jan 26, 2021
2e4854a
Merge branch 'master' into dev/search/restore-search-session-back-button
kibanamachine Jan 28, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<!-- Do not edit this file. It is automatically generated by API Documenter. -->

[Home](./index.md) &gt; [kibana-plugin-plugins-kibana\_utils-public-state\_sync](./kibana-plugin-plugins-kibana_utils-public-state_sync.md) &gt; [IKbnUrlStateStorage](./kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.md) &gt; [kbnUrlControls](./kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.kbnurlcontrols.md)

## IKbnUrlStateStorage.kbnUrlControls property

Lower level wrapper around history library that handles batching multiple URL updates into one history change

<b>Signature:</b>

```typescript
kbnUrlControls: IKbnUrlControls;
```
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,8 @@ export interface IKbnUrlStateStorage extends IStateStorage

| Property | Type | Description |
| --- | --- | --- |
| [cancel](./kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.cancel.md) | <code>() =&gt; void</code> | cancels any pending url updates |
| [change$](./kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.change_.md) | <code>&lt;State = unknown&gt;(key: string) =&gt; Observable&lt;State &#124; null&gt;</code> | |
| [flush](./kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.flush.md) | <code>(opts?: {</code><br/><code> replace?: boolean;</code><br/><code> }) =&gt; boolean</code> | Synchronously runs any pending url updates, returned boolean indicates if change occurred. |
| [get](./kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.get.md) | <code>&lt;State = unknown&gt;(key: string) =&gt; State &#124; null</code> | |
| [kbnUrlControls](./kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.kbnurlcontrols.md) | <code>IKbnUrlControls</code> | Lower level wrapper around history library that handles batching multiple URL updates into one history change |
| [set](./kibana-plugin-plugins-kibana_utils-public-state_sync.ikbnurlstatestorage.set.md) | <code>&lt;State&gt;(key: string, state: State, opts?: {</code><br/><code> replace: boolean;</code><br/><code> }) =&gt; Promise&lt;string &#124; undefined&gt;</code> | |

96 changes: 70 additions & 26 deletions src/plugins/dashboard/public/application/dashboard_app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,22 @@
* Public License, v 1.
*/

import _ from 'lodash';
import { History } from 'history';
import { merge, Subscription } from 'rxjs';
import React, { useEffect, useCallback, useState } from 'react';
import { merge, Subject, Subscription } from 'rxjs';
import React, { useCallback, useEffect, useMemo, useState } from 'react';

import { debounceTime, tap } from 'rxjs/operators';
import { useKibana } from '../../../kibana_react/public';
import { DashboardConstants } from '../dashboard_constants';
import { DashboardTopNav } from './top_nav/dashboard_top_nav';
import { DashboardAppServices, DashboardEmbedSettings, DashboardRedirect } from './types';
import {
getChangesFromAppStateForContainerState,
getDashboardContainerInput,
getFiltersSubscription,
getInputSubscription,
getOutputSubscription,
getFiltersSubscription,
getSearchSessionIdFromURL,
getDashboardContainerInput,
getChangesFromAppStateForContainerState,
} from './dashboard_app_functions';
import {
useDashboardBreadcrumbs,
Expand All @@ -30,11 +30,11 @@ import {
useSavedDashboard,
} from './hooks';

import { removeQueryParam } from '../services/kibana_utils';
import { IndexPattern } from '../services/data';
import { EmbeddableRenderer } from '../services/embeddable';
import { DashboardContainerInput } from '.';
import { leaveConfirmStrings } from '../dashboard_strings';
import { createQueryParamObservable, replaceUrlHashQuery } from '../../../kibana_utils/public';

export interface DashboardAppProps {
history: History;
Expand All @@ -59,7 +59,7 @@ export function DashboardApp({
indexPatterns: indexPatternService,
} = useKibana<DashboardAppServices>().services;

const [lastReloadTime, setLastReloadTime] = useState(0);
const triggerRefresh$ = useMemo(() => new Subject<{ force?: boolean }>(), []);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a nice improvement. Much better than using a lastReloadTime to trigger forced resets.

const [indexPatterns, setIndexPatterns] = useState<IndexPattern[]>([]);

const savedDashboard = useSavedDashboard(savedDashboardId, history);
Expand All @@ -68,9 +68,13 @@ export function DashboardApp({
history
);
const dashboardContainer = useDashboardContainer(dashboardStateManager, history, false);
const searchSessionIdQuery$ = useMemo(
() => createQueryParamObservable(history, DashboardConstants.SEARCH_SESSION_ID),
[history]
);

const refreshDashboardContainer = useCallback(
(lastReloadRequestTime?: number) => {
(force?: boolean) => {
if (!dashboardContainer || !dashboardStateManager) {
return;
}
Expand All @@ -80,7 +84,7 @@ export function DashboardApp({
appStateDashboardInput: getDashboardContainerInput({
isEmbeddedExternally: Boolean(embedSettings),
dashboardStateManager,
lastReloadRequestTime,
lastReloadRequestTime: force ? Date.now() : undefined,
dashboardCapabilities,
query: data.query,
}),
Expand All @@ -100,19 +104,43 @@ export function DashboardApp({
const shouldRefetch = Object.keys(changes).some(
(changeKey) => !noRefetchKeys.includes(changeKey as keyof DashboardContainerInput)
);
if (getSearchSessionIdFromURL(history)) {
// going away from a background search results
removeQueryParam(history, DashboardConstants.SEARCH_SESSION_ID, true);
}

const newSearchSessionId: string | undefined = (() => {
// do not update session id if this is irrelevant state change to prevent excessive searches
if (!shouldRefetch) return;

let searchSessionIdFromURL = getSearchSessionIdFromURL(history);
if (searchSessionIdFromURL) {
if (
data.search.session.isRestore() &&
data.search.session.isCurrentSession(searchSessionIdFromURL)
) {
// navigating away from a restored session
dashboardStateManager.kbnUrlStateStorage.kbnUrlControls.updateAsync((nextUrl) => {
if (nextUrl.includes(DashboardConstants.SEARCH_SESSION_ID)) {
return replaceUrlHashQuery(nextUrl, (query) => {
delete query[DashboardConstants.SEARCH_SESSION_ID];
return query;
});
}
return nextUrl;
});
searchSessionIdFromURL = undefined;
} else {
data.search.session.restore(searchSessionIdFromURL);
}
}

return searchSessionIdFromURL ?? data.search.session.start();
})();

if (changes.viewMode) {
setViewMode(changes.viewMode);
}

dashboardContainer.updateInput({
...changes,
// do not start a new session if this is irrelevant state change to prevent excessive searches
...(shouldRefetch && { searchSessionId: data.search.session.start() }),
...(newSearchSessionId && { searchSessionId: newSearchSessionId }),
});
}
},
Expand Down Expand Up @@ -159,23 +187,42 @@ export function DashboardApp({
subscriptions.add(
merge(
...[timeFilter.getRefreshIntervalUpdate$(), timeFilter.getTimeUpdate$()]
).subscribe(() => refreshDashboardContainer())
).subscribe(() => triggerRefresh$.next())
);

subscriptions.add(
merge(
data.search.session.onRefresh$,
data.query.timefilter.timefilter.getAutoRefreshFetch$()
data.query.timefilter.timefilter.getAutoRefreshFetch$(),
searchSessionIdQuery$
).subscribe(() => {
setLastReloadTime(() => new Date().getTime());
triggerRefresh$.next({ force: true });
})
);

dashboardStateManager.registerChangeListener(() => {
// we aren't checking dirty state because there are changes the container needs to know about
// that won't make the dashboard "dirty" - like a view mode change.
refreshDashboardContainer();
triggerRefresh$.next();
});

// debounce `refreshDashboardContainer()`
// use `forceRefresh=true` in case at least one debounced trigger asked for it
let forceRefresh: boolean = false;
subscriptions.add(
triggerRefresh$
.pipe(
tap((trigger) => {
forceRefresh = forceRefresh || (trigger?.force ?? false);
}),
debounceTime(50)
)
.subscribe(() => {
refreshDashboardContainer(forceRefresh);
forceRefresh = false;
})
);

return () => {
subscriptions.unsubscribe();
};
Expand All @@ -187,6 +234,8 @@ export function DashboardApp({
data.search.session,
indexPatternService,
dashboardStateManager,
searchSessionIdQuery$,
triggerRefresh$,
refreshDashboardContainer,
]);

Expand Down Expand Up @@ -216,11 +265,6 @@ export function DashboardApp({
};
}, [dashboardStateManager, dashboardContainer, onAppLeave, embeddable]);

// Refresh the dashboard container when lastReloadTime changes
useEffect(() => {
refreshDashboardContainer(lastReloadTime);
}, [lastReloadTime, refreshDashboardContainer]);

return (
<div className="app-container dshAppContainer">
{savedDashboard && dashboardStateManager && dashboardContainer && viewMode && (
Expand All @@ -242,7 +286,7 @@ export function DashboardApp({
// The user can still request a reload in the query bar, even if the
// query is the same, and in that case, we have to explicitly ask for
// a reload, since no state changes will cause it.
setLastReloadTime(() => new Date().getTime());
triggerRefresh$.next({ force: true });
}
}}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ export class DashboardStateManager {
>;
private readonly stateContainerChangeSub: Subscription;
private readonly STATE_STORAGE_KEY = '_a';
private readonly kbnUrlStateStorage: IKbnUrlStateStorage;
public readonly kbnUrlStateStorage: IKbnUrlStateStorage;
private readonly stateSyncRef: ISyncStateRef;
private readonly history: History;
private readonly usageCollection: UsageCollectionSetup | undefined;
Expand Down Expand Up @@ -596,7 +596,7 @@ export class DashboardStateManager {
this.toUrlState(this.stateContainer.get())
);
// immediately forces scheduled updates and changes location
return this.kbnUrlStateStorage.flush({ replace });
return !!this.kbnUrlStateStorage.kbnUrlControls.flush(replace);
}

// TODO: find nicer solution for this
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading