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

[7.x] [Search] Remove long-running query pop-up (#75385) #77294

Merged
merged 1 commit into from
Sep 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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.

Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ export declare class SearchInterceptor
| Property | Modifiers | Type | Description |
| --- | --- | --- | --- |
| [deps](./kibana-plugin-plugins-data-public.searchinterceptor.deps.md) | | <code>SearchInterceptorDeps</code> | |
| [showTimeoutError](./kibana-plugin-plugins-data-public.searchinterceptor.showtimeouterror.md) | | <code>((e: Error) =&gt; void) &amp; import(&quot;lodash&quot;).Cancelable</code> | |

## Methods

| Method | Modifiers | Description |
| --- | --- | --- |
| [getPendingCount$()](./kibana-plugin-plugins-data-public.searchinterceptor.getpendingcount_.md) | | Returns an <code>Observable</code> over the current number of pending searches. This could mean that one of the search requests is still in flight, or that it has only received partial responses. |
| [search(request, options)](./kibana-plugin-plugins-data-public.searchinterceptor.search.md) | | Searches using the given <code>search</code> method. Overrides the <code>AbortSignal</code> with one that will abort either when <code>cancelPending</code> is called, when the request times out, or when the original <code>AbortSignal</code> is aborted. Updates <code>pendingCount$</code> when the request is started/finalized. |

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

[Home](./index.md) &gt; [kibana-plugin-plugins-data-public](./kibana-plugin-plugins-data-public.md) &gt; [SearchInterceptor](./kibana-plugin-plugins-data-public.searchinterceptor.md) &gt; [showTimeoutError](./kibana-plugin-plugins-data-public.searchinterceptor.showtimeouterror.md)

## SearchInterceptor.showTimeoutError property

<b>Signature:</b>

```typescript
protected showTimeoutError: ((e: Error) => void) & import("lodash").Cancelable;
```
10 changes: 2 additions & 8 deletions src/plugins/data/public/public.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ import { Search } from '@elastic/elasticsearch/api/requestParams';
import { SearchResponse } from 'elasticsearch';
import { SerializedFieldFormat as SerializedFieldFormat_2 } from 'src/plugins/expressions/common';
import { Subscription } from 'rxjs';
import { Toast } from 'kibana/public';
import { ToastInputFields } from 'src/core/public/notifications';
import { ToastsSetup } from 'kibana/public';
import { TransportRequestOptions } from '@elastic/elasticsearch/lib/Transport';
Expand Down Expand Up @@ -1740,11 +1739,6 @@ export class SearchInterceptor {
protected application: CoreStart['application'];
// (undocumented)
protected readonly deps: SearchInterceptorDeps;
getPendingCount$(): Observable<number>;
// @internal (undocumented)
protected hideToast: () => void;
// @internal
protected longRunningToast?: Toast;
// @internal
protected pendingCount$: BehaviorSubject<number>;
// @internal (undocumented)
Expand All @@ -1758,8 +1752,8 @@ export class SearchInterceptor {
combinedSignal: AbortSignal;
cleanup: () => void;
};
// @internal (undocumented)
protected showToast: () => void;
// (undocumented)
protected showTimeoutError: ((e: Error) => void) & import("lodash").Cancelable;
// @internal
protected timeoutSubscriptions: Subscription;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,31 +63,4 @@ describe('Search Usage Collector', () => {
SEARCH_EVENT_TYPE.QUERIES_CANCELLED
);
});

test('tracks long popups', async () => {
await usageCollector.trackLongQueryPopupShown();
expect(mockUsageCollectionSetup.reportUiStats).toHaveBeenCalled();
expect(mockUsageCollectionSetup.reportUiStats.mock.calls[0][1]).toBe(METRIC_TYPE.LOADED);
expect(mockUsageCollectionSetup.reportUiStats.mock.calls[0][2]).toBe(
SEARCH_EVENT_TYPE.LONG_QUERY_POPUP_SHOWN
);
});

test('tracks long popups dismissed', async () => {
await usageCollector.trackLongQueryDialogDismissed();
expect(mockUsageCollectionSetup.reportUiStats).toHaveBeenCalled();
expect(mockUsageCollectionSetup.reportUiStats.mock.calls[0][1]).toBe(METRIC_TYPE.CLICK);
expect(mockUsageCollectionSetup.reportUiStats.mock.calls[0][2]).toBe(
SEARCH_EVENT_TYPE.LONG_QUERY_DIALOG_DISMISSED
);
});

test('tracks run query beyond timeout', async () => {
await usageCollector.trackLongQueryRunBeyondTimeout();
expect(mockUsageCollectionSetup.reportUiStats).toHaveBeenCalled();
expect(mockUsageCollectionSetup.reportUiStats.mock.calls[0][1]).toBe(METRIC_TYPE.CLICK);
expect(mockUsageCollectionSetup.reportUiStats.mock.calls[0][2]).toBe(
SEARCH_EVENT_TYPE.LONG_QUERY_RUN_BEYOND_TIMEOUT
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -48,29 +48,5 @@ export const createUsageCollector = (
SEARCH_EVENT_TYPE.QUERIES_CANCELLED
);
},
trackLongQueryPopupShown: async () => {
const currentApp = await getCurrentApp();
return usageCollection?.reportUiStats(
currentApp!,
METRIC_TYPE.LOADED,
SEARCH_EVENT_TYPE.LONG_QUERY_POPUP_SHOWN
);
},
trackLongQueryDialogDismissed: async () => {
const currentApp = await getCurrentApp();
return usageCollection?.reportUiStats(
currentApp!,
METRIC_TYPE.CLICK,
SEARCH_EVENT_TYPE.LONG_QUERY_DIALOG_DISMISSED
);
},
trackLongQueryRunBeyondTimeout: async () => {
const currentApp = await getCurrentApp();
return usageCollection?.reportUiStats(
currentApp!,
METRIC_TYPE.CLICK,
SEARCH_EVENT_TYPE.LONG_QUERY_RUN_BEYOND_TIMEOUT
);
},
};
};
6 changes: 0 additions & 6 deletions src/plugins/data/public/search/collectors/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,9 @@
export enum SEARCH_EVENT_TYPE {
QUERY_TIMED_OUT = 'queryTimedOut',
QUERIES_CANCELLED = 'queriesCancelled',
LONG_QUERY_POPUP_SHOWN = 'longQueryPopupShown',
LONG_QUERY_DIALOG_DISMISSED = 'longQueryDialogDismissed',
LONG_QUERY_RUN_BEYOND_TIMEOUT = 'longQueryRunBeyondTimeout',
}

export interface SearchUsageCollector {
trackQueryTimedOut: () => Promise<void>;
trackQueriesCancelled: () => Promise<void>;
trackLongQueryPopupShown: () => Promise<void>;
trackLongQueryDialogDismissed: () => Promise<void>;
trackLongQueryRunBeyondTimeout: () => Promise<void>;
}
59 changes: 0 additions & 59 deletions src/plugins/data/public/search/long_query_notification.tsx

This file was deleted.

75 changes: 34 additions & 41 deletions src/plugins/data/public/search/search_interceptor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,39 @@ describe('SearchInterceptor', () => {
await flushPromises();
});

test('Should not timeout if requestTimeout is undefined', async () => {
searchInterceptor = new SearchInterceptor({
startServices: mockCoreSetup.getStartServices(),
uiSettings: mockCoreSetup.uiSettings,
http: mockCoreSetup.http,
toasts: mockCoreSetup.notifications.toasts,
});
mockCoreSetup.http.fetch.mockImplementationOnce((options: any) => {
return new Promise((resolve, reject) => {
options.signal.addEventListener('abort', () => {
reject(new AbortError());
});

setTimeout(resolve, 5000);
});
});
const mockRequest: IEsSearchRequest = {
params: {},
};
const response = searchInterceptor.search(mockRequest);

expect.assertions(1);
const next = jest.fn();
const complete = () => {
expect(next).toBeCalled();
};
response.subscribe({ next, complete });

jest.advanceTimersByTime(5000);

await flushPromises();
});

test('Observable should fail if user aborts (test merged signal)', async () => {
const abortController = new AbortController();
mockCoreSetup.http.fetch.mockImplementationOnce((options: any) => {
Expand Down Expand Up @@ -125,7 +158,7 @@ describe('SearchInterceptor', () => {
await flushPromises();
});

test('Immediatelly aborts if passed an aborted abort signal', async (done) => {
test('Immediately aborts if passed an aborted abort signal', async (done) => {
const abort = new AbortController();
const mockRequest: IEsSearchRequest = {
params: {},
Expand All @@ -141,44 +174,4 @@ describe('SearchInterceptor', () => {
response.subscribe({ error });
});
});

describe('getPendingCount$', () => {
test('should observe the number of pending requests', () => {
const pendingCount$ = searchInterceptor.getPendingCount$();
const pendingNext = jest.fn();
pendingCount$.subscribe(pendingNext);

const mockResponse: any = { result: 200 };
mockCoreSetup.http.fetch.mockResolvedValue(mockResponse);
const mockRequest: IEsSearchRequest = {
params: {},
};
const response = searchInterceptor.search(mockRequest);

response.subscribe({
complete: () => {
expect(pendingNext.mock.calls).toEqual([[0], [1], [0]]);
},
});
});

test('should observe the number of pending requests on error', () => {
const pendingCount$ = searchInterceptor.getPendingCount$();
const pendingNext = jest.fn();
pendingCount$.subscribe(pendingNext);

const mockResponse: any = { result: 500 };
mockCoreSetup.http.fetch.mockRejectedValue(mockResponse);
const mockRequest: IEsSearchRequest = {
params: {},
};
const response = searchInterceptor.search(mockRequest);

response.subscribe({
complete: () => {
expect(pendingNext.mock.calls).toEqual([[0], [1], [0]]);
},
});
});
});
});
Loading