Skip to content

Commit 3679e94

Browse files
pgayvalletpgayvallet
committed
use fetch instead of core.http and add tests
1 parent 5c8c499 commit 3679e94

6 files changed

Lines changed: 122 additions & 24 deletions

File tree

‎src/plugins/newsfeed/public/lib/api.test.ts‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88

99
import { driverInstanceMock, storageInstanceMock } from './api.test.mocks';
1010
import moment from 'moment';
11-
import { httpServiceMock } from '../../../../core/public/mocks';
1211
import { getApi } from './api';
1312
import { TestScheduler } from 'rxjs/testing';
1413
import { FetchResult, NewsfeedPluginBrowserConfig } from '../types';
@@ -40,10 +39,7 @@ const createFetchResult = (parts: Partial<FetchResult>): FetchResult => ({
4039
});
4140

4241
describe('getApi', () => {
43-
let http: ReturnType<typeof httpServiceMock.createSetupContract>;
44-
4542
beforeEach(() => {
46-
http = httpServiceMock.createSetupContract();
4743
driverInstanceMock.shouldFetch.mockReturnValue(true);
4844
});
4945

@@ -64,7 +60,7 @@ describe('getApi', () => {
6460
a: createFetchResult({ feedItems: ['item' as any] }),
6561
})
6662
);
67-
const api = getApi(http, createConfig(1000), kibanaVersion, newsfeedId);
63+
const api = getApi(createConfig(1000), kibanaVersion, newsfeedId);
6864

6965
expectObservable(api.fetchResults$.pipe(take(1))).toBe('(a|)', {
7066
a: createFetchResult({
@@ -87,7 +83,7 @@ describe('getApi', () => {
8783
a: createFetchResult({ feedItems: ['item' as any] }),
8884
})
8985
);
90-
const api = getApi(http, createConfig(2), kibanaVersion, newsfeedId);
86+
const api = getApi(createConfig(2), kibanaVersion, newsfeedId);
9187

9288
expectObservable(api.fetchResults$.pipe(take(2))).toBe('a-(b|)', {
9389
a: createFetchResult({
@@ -115,7 +111,7 @@ describe('getApi', () => {
115111
a: createFetchResult({}),
116112
})
117113
);
118-
const api = getApi(http, createConfig(10), kibanaVersion, newsfeedId);
114+
const api = getApi(createConfig(10), kibanaVersion, newsfeedId);
119115

120116
expectObservable(api.fetchResults$.pipe(take(2))).toBe('a--(b|)', {
121117
a: createFetchResult({

‎src/plugins/newsfeed/public/lib/api.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
import { combineLatest, Observable, timer, of } from 'rxjs';
1010
import { map, catchError, filter, mergeMap, tap } from 'rxjs/operators';
1111
import { i18n } from '@kbn/i18n';
12-
import { HttpSetup } from 'src/core/public';
1312
import { FetchResult, NewsfeedPluginBrowserConfig } from '../types';
1413
import { NewsfeedApiDriver } from './driver';
1514
import { NewsfeedStorage } from './storage';
@@ -39,7 +38,6 @@ export interface NewsfeedApi {
3938
* Computes hasNew value from new item hashes saved in localStorage
4039
*/
4140
export function getApi(
42-
http: HttpSetup,
4341
config: NewsfeedPluginBrowserConfig,
4442
kibanaVersion: string,
4543
newsfeedId: string
@@ -53,7 +51,7 @@ export function getApi(
5351
const results$ = timer(0, mainInterval).pipe(
5452
filter(() => driver.shouldFetch()),
5553
mergeMap(() =>
56-
driver.fetchNewsfeedItems(http, config.service).pipe(
54+
driver.fetchNewsfeedItems(config.service).pipe(
5755
catchError((err) => {
5856
window.console.error(err);
5957
return of({

‎src/plugins/newsfeed/public/lib/driver.test.mocks.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0 and the Server Side Public License, v 1; you may not use this file except
5+
* in compliance with, at your election, the Elastic License 2.0 or the Server
6+
* Side Public License, v 1.
7+
*/
8+
9+
export const convertItemsMock = jest.fn();
10+
jest.doMock('./convert_items', () => ({
11+
convertItems: convertItemsMock,
12+
}));

‎src/plugins/newsfeed/public/lib/driver.test.ts‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@
66
* Side Public License, v 1.
77
*/
88

9+
import { convertItemsMock } from './driver.test.mocks';
10+
// @ts-expect-error
11+
import fetchMock from 'fetch-mock/es5/client';
12+
import { take } from 'rxjs/operators';
913
import { NewsfeedApiDriver } from './driver';
1014
import { storageMock } from './storage.mock';
1115

@@ -20,6 +24,16 @@ describe('NewsfeedApiDriver', () => {
2024
beforeEach(() => {
2125
storage = storageMock.create();
2226
driver = new NewsfeedApiDriver(kibanaVersion, userLanguage, fetchInterval, storage);
27+
convertItemsMock.mockReturnValue([]);
28+
});
29+
30+
afterEach(() => {
31+
fetchMock.reset();
32+
convertItemsMock.mockReset();
33+
});
34+
35+
afterAll(() => {
36+
fetchMock.restore();
2337
});
2438

2539
describe('shouldFetch', () => {
@@ -39,4 +53,81 @@ describe('NewsfeedApiDriver', () => {
3953
expect(driver.shouldFetch()).toBe(false);
4054
});
4155
});
56+
57+
describe('fetchNewsfeedItems', () => {
58+
it('calls `window.fetch` with the correct parameters', async () => {
59+
fetchMock.get('*', { items: [] });
60+
await driver
61+
.fetchNewsfeedItems({
62+
urlRoot: 'http://newsfeed.com',
63+
pathTemplate: '/{VERSION}/news',
64+
})
65+
.pipe(take(1))
66+
.toPromise();
67+
68+
expect(fetchMock.lastUrl()).toEqual('http://newsfeed.com/8.0.0/news');
69+
expect(fetchMock.lastOptions()).toEqual({
70+
method: 'GET',
71+
});
72+
});
73+
74+
it('calls `convertItems` with the correct parameters', async () => {
75+
fetchMock.get('*', { items: ['foo', 'bar'] });
76+
77+
await driver
78+
.fetchNewsfeedItems({
79+
urlRoot: 'http://newsfeed.com',
80+
pathTemplate: '/{VERSION}/news',
81+
})
82+
.pipe(take(1))
83+
.toPromise();
84+
85+
expect(convertItemsMock).toHaveBeenCalledTimes(1);
86+
expect(convertItemsMock).toHaveBeenCalledWith(['foo', 'bar'], userLanguage);
87+
});
88+
89+
it('calls `storage.setFetchedItems` with the correct parameters', async () => {
90+
fetchMock.get('*', { items: [] });
91+
convertItemsMock.mockReturnValue([
92+
{ id: '1', hash: 'hash1' },
93+
{ id: '2', hash: 'hash2' },
94+
]);
95+
96+
await driver
97+
.fetchNewsfeedItems({
98+
urlRoot: 'http://newsfeed.com',
99+
pathTemplate: '/{VERSION}/news',
100+
})
101+
.pipe(take(1))
102+
.toPromise();
103+
104+
expect(storage.setFetchedItems).toHaveBeenCalledTimes(1);
105+
expect(storage.setFetchedItems).toHaveBeenCalledWith(['hash1', 'hash2']);
106+
});
107+
108+
it('returns the expected values', async () => {
109+
fetchMock.get('*', { items: [] });
110+
const feedItems = [
111+
{ id: '1', hash: 'hash1' },
112+
{ id: '2', hash: 'hash2' },
113+
];
114+
convertItemsMock.mockReturnValue(feedItems);
115+
storage.setFetchedItems.mockReturnValue(true);
116+
117+
const result = await driver
118+
.fetchNewsfeedItems({
119+
urlRoot: 'http://newsfeed.com',
120+
pathTemplate: '/{VERSION}/news',
121+
})
122+
.pipe(take(1))
123+
.toPromise();
124+
125+
expect(result).toEqual({
126+
error: null,
127+
kibanaVersion,
128+
hasNew: true,
129+
feedItems,
130+
});
131+
});
132+
});
42133
});

‎src/plugins/newsfeed/public/lib/driver.ts‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
*/
88

99
import moment from 'moment';
10-
import { HttpSetup } from 'kibana/public';
1110
import * as Rx from 'rxjs';
1211
import { NEWSFEED_DEFAULT_SERVICE_BASE_URL } from '../../common/constants';
1312
import { ApiItem, FetchResult, NewsfeedPluginBrowserConfig } from '../types';
@@ -16,6 +15,10 @@ import type { NewsfeedStorage } from './storage';
1615

1716
type ApiConfig = NewsfeedPluginBrowserConfig['service'];
1817

18+
interface NewsfeedResponse {
19+
items: ApiItem[];
20+
}
21+
1922
export class NewsfeedApiDriver {
2023
private readonly kibanaVersion: string;
2124
private readonly loadedTime = moment().utc(); // the date is compared to time in UTC format coming from the service
@@ -47,18 +50,18 @@ export class NewsfeedApiDriver {
4750
return duration.asMilliseconds() > this.fetchInterval;
4851
}
4952

50-
fetchNewsfeedItems(http: HttpSetup, config: ApiConfig): Rx.Observable<FetchResult> {
53+
fetchNewsfeedItems(config: ApiConfig): Rx.Observable<FetchResult> {
5154
const urlPath = config.pathTemplate.replace('{VERSION}', this.kibanaVersion);
5255
const fullUrl = (config.urlRoot || NEWSFEED_DEFAULT_SERVICE_BASE_URL) + urlPath;
56+
const request = new Request(fullUrl, {
57+
method: 'GET',
58+
});
5359

5460
return Rx.from(
55-
http
56-
.fetch(fullUrl, {
57-
method: 'GET',
58-
})
59-
.then(({ items }: { items: ApiItem[] }) => {
60-
return this.convertResponse(items);
61-
})
61+
window.fetch(request).then(async (response) => {
62+
const { items } = (await response.json()) as NewsfeedResponse;
63+
return this.convertResponse(items);
64+
})
6265
);
6366
}
6467

‎src/plugins/newsfeed/public/plugin.tsx‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ export class NewsfeedPublicPlugin
4242
}
4343

4444
public start(core: CoreStart) {
45-
const api = this.createNewsfeedApi(core, this.config, NewsfeedApiEndpoint.KIBANA);
45+
const api = this.createNewsfeedApi(this.config, NewsfeedApiEndpoint.KIBANA);
4646
core.chrome.navControls.registerRight({
4747
order: 1000,
4848
mount: (target) => this.mount(api, target),
@@ -56,7 +56,7 @@ export class NewsfeedPublicPlugin
5656
pathTemplate: `/${endpoint}/v{VERSION}.json`,
5757
},
5858
});
59-
const { fetchResults$ } = this.createNewsfeedApi(core, config, endpoint);
59+
const { fetchResults$ } = this.createNewsfeedApi(config, endpoint);
6060
return fetchResults$;
6161
},
6262
};
@@ -67,12 +67,10 @@ export class NewsfeedPublicPlugin
6767
}
6868

6969
private createNewsfeedApi(
70-
core: CoreStart,
7170
config: NewsfeedPluginBrowserConfig,
7271
newsfeedId: NewsfeedApiEndpoint
7372
): NewsfeedApi {
74-
const { http } = core;
75-
const api = getApi(http, config, this.kibanaVersion, newsfeedId);
73+
const api = getApi(config, this.kibanaVersion, newsfeedId);
7674
return {
7775
markAsRead: api.markAsRead,
7876
fetchResults$: api.fetchResults$.pipe(

0 commit comments

Comments
 (0)