From 90809988493384cf0311ad70aaf9a2c60da6c005 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Wed, 19 May 2021 14:20:58 +0200 Subject: [PATCH 1/7] fix the implementation --- src/plugins/newsfeed/common/constants.ts | 5 - .../components/newsfeed_header_nav_button.tsx | 41 ++-- src/plugins/newsfeed/public/lib/api.ts | 198 ++++-------------- .../newsfeed/public/lib/convert_items.ts | 72 +++++++ src/plugins/newsfeed/public/lib/driver.ts | 80 +++++++ src/plugins/newsfeed/public/lib/storage.ts | 86 ++++++++ src/plugins/newsfeed/public/plugin.tsx | 37 ++-- 7 files changed, 320 insertions(+), 199 deletions(-) create mode 100644 src/plugins/newsfeed/public/lib/convert_items.ts create mode 100644 src/plugins/newsfeed/public/lib/driver.ts create mode 100644 src/plugins/newsfeed/public/lib/storage.ts diff --git a/src/plugins/newsfeed/common/constants.ts b/src/plugins/newsfeed/common/constants.ts index 7ff078e82810b..6ba5e07ea873e 100644 --- a/src/plugins/newsfeed/common/constants.ts +++ b/src/plugins/newsfeed/common/constants.ts @@ -7,11 +7,6 @@ */ export const NEWSFEED_FALLBACK_LANGUAGE = 'en'; -export const NEWSFEED_FALLBACK_FETCH_INTERVAL = 86400000; // 1 day -export const NEWSFEED_FALLBACK_MAIN_INTERVAL = 120000; // 2 minutes -export const NEWSFEED_LAST_FETCH_STORAGE_KEY = 'newsfeed.lastfetchtime'; -export const NEWSFEED_HASH_SET_STORAGE_KEY = 'newsfeed.hashes'; - export const NEWSFEED_DEFAULT_SERVICE_BASE_URL = 'https://feeds.elastic.co'; export const NEWSFEED_DEV_SERVICE_BASE_URL = 'https://feeds-staging.elastic.co'; export const NEWSFEED_DEFAULT_SERVICE_PATH = '/kibana/v{VERSION}.json'; diff --git a/src/plugins/newsfeed/public/components/newsfeed_header_nav_button.tsx b/src/plugins/newsfeed/public/components/newsfeed_header_nav_button.tsx index 142d4286b363b..8a8a70dee49a0 100644 --- a/src/plugins/newsfeed/public/components/newsfeed_header_nav_button.tsx +++ b/src/plugins/newsfeed/public/components/newsfeed_header_nav_button.tsx @@ -6,57 +6,58 @@ * Side Public License, v 1. */ -import React, { useState, Fragment, useEffect } from 'react'; +import React, { useState, useEffect, useMemo, useCallback } from 'react'; import * as Rx from 'rxjs'; import { EuiHeaderSectionItemButton, EuiIcon } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { NewsfeedFlyout } from './flyout_list'; import { FetchResult } from '../types'; +import { NewsfeedApi } from '../lib/api'; export interface INewsfeedContext { setFlyoutVisible: React.Dispatch>; newsFetchResult: FetchResult | void | null; } + export const NewsfeedContext = React.createContext({} as INewsfeedContext); export type NewsfeedApiFetchResult = Rx.Observable; export interface Props { - apiFetchResult: NewsfeedApiFetchResult; + newsfeedApi: NewsfeedApi; } -export const NewsfeedNavButton = ({ apiFetchResult }: Props) => { - const [showBadge, setShowBadge] = useState(false); +export const NewsfeedNavButton = ({ newsfeedApi }: Props) => { const [flyoutVisible, setFlyoutVisible] = useState(false); const [newsFetchResult, setNewsFetchResult] = useState(null); + const hasNew = useMemo(() => { + return newsFetchResult ? newsFetchResult.hasNew : false; + }, [newsFetchResult]); useEffect(() => { - function handleStatusChange(fetchResult: FetchResult | void | null) { - if (fetchResult) { - setShowBadge(fetchResult.hasNew); - } - setNewsFetchResult(fetchResult); - } - - const subscription = apiFetchResult.subscribe((res) => handleStatusChange(res)); + const subscription = newsfeedApi.fetchResults$.subscribe((results) => { + setNewsFetchResult(results); + }); return () => subscription.unsubscribe(); - }, [apiFetchResult]); + }, [newsfeedApi]); - function showFlyout() { - setShowBadge(false); + const showFlyout = useCallback(() => { + if (newsFetchResult) { + newsfeedApi.markAsRead(newsFetchResult.feedItems.map((item) => item.hash)); + } setFlyoutVisible(!flyoutVisible); - } + }, [newsfeedApi, newsFetchResult, flyoutVisible]); return ( - + <> { defaultMessage: 'Newsfeed menu - all items read', }) } - notification={showBadge ? true : null} + notification={hasNew ? true : null} onClick={showFlyout} > {flyoutVisible ? : null} - + ); }; diff --git a/src/plugins/newsfeed/public/lib/api.ts b/src/plugins/newsfeed/public/lib/api.ts index 9b1274a25d486..febce9021afb4 100644 --- a/src/plugins/newsfeed/public/lib/api.ts +++ b/src/plugins/newsfeed/public/lib/api.ts @@ -6,21 +6,13 @@ * Side Public License, v 1. */ -import * as Rx from 'rxjs'; -import moment from 'moment'; -import uuid from 'uuid'; +import { combineLatest, Observable, timer, of } from 'rxjs'; +import { map, catchError, filter, mergeMap, tap } from 'rxjs/operators'; import { i18n } from '@kbn/i18n'; -import { catchError, filter, mergeMap, tap } from 'rxjs/operators'; import { HttpSetup } from 'src/core/public'; -import { - NEWSFEED_DEFAULT_SERVICE_BASE_URL, - NEWSFEED_FALLBACK_LANGUAGE, - NEWSFEED_LAST_FETCH_STORAGE_KEY, - NEWSFEED_HASH_SET_STORAGE_KEY, -} from '../../common/constants'; -import { ApiItem, NewsfeedItem, FetchResult, NewsfeedPluginBrowserConfig } from '../types'; - -type ApiConfig = NewsfeedPluginBrowserConfig['service']; +import { FetchResult, NewsfeedPluginBrowserConfig } from '../types'; +import { NewsfeedApiDriver } from './driver'; +import { NewsfeedStorage } from './storage'; export enum NewsfeedApiEndpoint { KIBANA = 'kibana', @@ -29,145 +21,17 @@ export enum NewsfeedApiEndpoint { OBSERVABILITY = 'observability', } -export class NewsfeedApiDriver { - private readonly id = uuid.v4(); - private readonly kibanaVersion: string; - private readonly loadedTime = moment().utc(); // the date is compared to time in UTC format coming from the service - private readonly lastFetchStorageKey: string; - private readonly hashSetStorageKey: string; - - constructor( - kibanaVersion: string, - private readonly userLanguage: string, - private readonly fetchInterval: number - ) { - // The API only accepts versions in the format `X.Y.Z`, so we need to drop the `-SNAPSHOT` or any other label after it - this.kibanaVersion = kibanaVersion.replace(/^(\d+\.\d+\.\d+).*/, '$1'); - this.lastFetchStorageKey = `${NEWSFEED_LAST_FETCH_STORAGE_KEY}.${this.id}`; - this.hashSetStorageKey = `${NEWSFEED_HASH_SET_STORAGE_KEY}.${this.id}`; - } - - shouldFetch(): boolean { - const lastFetchUtc: string | null = sessionStorage.getItem(this.lastFetchStorageKey); - if (lastFetchUtc == null) { - return true; - } - const last = moment(lastFetchUtc, 'x'); // parse as unix ms timestamp (already is UTC) - - // does the last fetch time precede the time that the page was loaded? - if (this.loadedTime.diff(last) > 0) { - return true; - } - - const now = moment.utc(); // always use UTC to compare timestamps that came from the service - const duration = moment.duration(now.diff(last)); - - return duration.asMilliseconds() > this.fetchInterval; - } - - updateLastFetch() { - sessionStorage.setItem(this.lastFetchStorageKey, Date.now().toString()); - } - - updateHashes(items: NewsfeedItem[]): { previous: string[]; current: string[] } { - // replace localStorage hashes with new hashes - const stored: string | null = localStorage.getItem(this.hashSetStorageKey); - let old: string[] = []; - if (stored != null) { - old = stored.split(','); - } - - const newHashes = items.map((i) => i.hash); - const updatedHashes = [...new Set(old.concat(newHashes))]; - localStorage.setItem(this.hashSetStorageKey, updatedHashes.join(',')); - - return { previous: old, current: updatedHashes }; - } - - fetchNewsfeedItems(http: HttpSetup, config: ApiConfig): Rx.Observable { - const urlPath = config.pathTemplate.replace('{VERSION}', this.kibanaVersion); - const fullUrl = (config.urlRoot || NEWSFEED_DEFAULT_SERVICE_BASE_URL) + urlPath; - - return Rx.from( - http - .fetch(fullUrl, { - method: 'GET', - }) - .then(({ items }: { items: ApiItem[] }) => { - return this.modelItems(items); - }) - ); - } - - validateItem(item: Partial) { - const hasMissing = [ - item.title, - item.description, - item.linkText, - item.linkUrl, - item.publishOn, - item.hash, - ].includes(undefined); - - return !hasMissing; - } - - modelItems(items: ApiItem[]): FetchResult { - const feedItems: NewsfeedItem[] = items.reduce((accum: NewsfeedItem[], it: ApiItem) => { - let chosenLanguage = this.userLanguage; - const { - expire_on: expireOnUtc, - publish_on: publishOnUtc, - languages, - title, - description, - link_text: linkText, - link_url: linkUrl, - badge, - hash, - } = it; - - if (moment(expireOnUtc).isBefore(Date.now())) { - return accum; // ignore item if expired - } - - if (moment(publishOnUtc).isAfter(Date.now())) { - return accum; // ignore item if publish date hasn't occurred yet (pre-published) - } - - if (languages && !languages.includes(chosenLanguage)) { - chosenLanguage = NEWSFEED_FALLBACK_LANGUAGE; // don't remove the item: fallback on a language - } - - const tempItem: NewsfeedItem = { - title: title[chosenLanguage], - description: description[chosenLanguage], - linkText: linkText != null ? linkText[chosenLanguage] : null, - linkUrl: linkUrl[chosenLanguage], - badge: badge != null ? badge![chosenLanguage] : null, - publishOn: moment(publishOnUtc), - expireOn: moment(expireOnUtc), - hash: hash.slice(0, 10), // optimize for storage and faster parsing - }; - - if (!this.validateItem(tempItem)) { - return accum; // ignore if title, description, etc is missing - } - - return [...accum, tempItem]; - }, []); - - // calculate hasNew - const { previous, current } = this.updateHashes(feedItems); - const hasNew = current.length > previous.length; - - return { - error: null, - kibanaVersion: this.kibanaVersion, - hasNew, - feedItems, - }; - } +export interface NewsfeedApi { + /** + * The current fetch results + */ + fetchResults$: Observable; + + /** + * Mark the given items as read. + * Will refresh the `hasNew` value of the emitted FetchResult accordingly + */ + markAsRead(itemHashes: string[]): void; } /* @@ -177,20 +41,22 @@ export class NewsfeedApiDriver { export function getApi( http: HttpSetup, config: NewsfeedPluginBrowserConfig, - kibanaVersion: string -): Rx.Observable { + kibanaVersion: string, + newsfeedId: string +): NewsfeedApi { const userLanguage = i18n.getLocale(); const fetchInterval = config.fetchInterval.asMilliseconds(); - const mainInterval = config.mainInterval.asMilliseconds(); - const driver = new NewsfeedApiDriver(kibanaVersion, userLanguage, fetchInterval); + const mainInterval = 2000; // config.mainInterval.asMilliseconds(); + const storage = new NewsfeedStorage(newsfeedId); + const driver = new NewsfeedApiDriver(kibanaVersion, userLanguage, fetchInterval, storage); - return Rx.timer(0, mainInterval).pipe( + const results$ = timer(0, mainInterval).pipe( filter(() => driver.shouldFetch()), mergeMap(() => driver.fetchNewsfeedItems(http, config.service).pipe( catchError((err) => { window.console.error(err); - return Rx.of({ + return of({ error: err, kibanaVersion, hasNew: false, @@ -201,4 +67,20 @@ export function getApi( ), tap(() => driver.updateLastFetch()) ); + + const merged$ = combineLatest([results$, storage.getUnreadStatus$()]).pipe( + map(([results, unreadStatus]) => { + return { + ...results, + hasNew: results.error ? false : unreadStatus, + }; + }) + ); + + return { + fetchResults$: merged$, + markAsRead: (itemHashes) => { + storage.markItemsAsRead(itemHashes); + }, + }; } diff --git a/src/plugins/newsfeed/public/lib/convert_items.ts b/src/plugins/newsfeed/public/lib/convert_items.ts new file mode 100644 index 0000000000000..1052487e677e5 --- /dev/null +++ b/src/plugins/newsfeed/public/lib/convert_items.ts @@ -0,0 +1,72 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import moment from 'moment'; +import { ApiItem, NewsfeedItem } from '../types'; +import { NEWSFEED_FALLBACK_LANGUAGE } from '../../common/constants'; + +export const convertItems = (items: ApiItem[], userLanguage: string): NewsfeedItem[] => { + return items + .filter(validatePublishedDate) + .map((item) => convertItem(item, userLanguage)) + .filter(validateIntegrity); +}; + +const validatePublishedDate = (item: ApiItem): boolean => { + if (moment(item.expire_on).isBefore(Date.now())) { + return false; // ignore item if expired + } + + if (moment(item.publish_on).isAfter(Date.now())) { + return false; // ignore item if publish date hasn't occurred yet (pre-published) + } + return true; +}; + +const convertItem = (rawItem: ApiItem, userLanguage: string): NewsfeedItem => { + const { + expire_on: expireOnUtc, + publish_on: publishOnUtc, + languages, + title, + description, + link_text: linkText, + link_url: linkUrl, + badge, + hash, + } = rawItem; + + let chosenLanguage = userLanguage; + if (languages && !languages.includes(chosenLanguage)) { + chosenLanguage = NEWSFEED_FALLBACK_LANGUAGE; // don't remove the item: fallback on a language + } + + return { + title: title[chosenLanguage], + description: description[chosenLanguage], + linkText: linkText != null ? linkText[chosenLanguage] : null, + linkUrl: linkUrl[chosenLanguage], + badge: badge != null ? badge![chosenLanguage] : null, + publishOn: moment(publishOnUtc), + expireOn: moment(expireOnUtc), + hash: hash.slice(0, 10), // optimize for storage and faster parsing + }; +}; + +const validateIntegrity = (item: Partial): boolean => { + const hasMissing = [ + item.title, + item.description, + item.linkText, + item.linkUrl, + item.publishOn, + item.hash, + ].includes(undefined); + + return !hasMissing; +}; diff --git a/src/plugins/newsfeed/public/lib/driver.ts b/src/plugins/newsfeed/public/lib/driver.ts new file mode 100644 index 0000000000000..e45863d603765 --- /dev/null +++ b/src/plugins/newsfeed/public/lib/driver.ts @@ -0,0 +1,80 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import moment from 'moment'; +import { HttpSetup } from 'kibana/public'; +import * as Rx from 'rxjs'; +import { NEWSFEED_DEFAULT_SERVICE_BASE_URL } from '../../common/constants'; +import { ApiItem, FetchResult, NewsfeedPluginBrowserConfig } from '../types'; +import { convertItems } from './convert_items'; +import type { NewsfeedStorage } from './storage'; + +type ApiConfig = NewsfeedPluginBrowserConfig['service']; + +export class NewsfeedApiDriver { + private readonly kibanaVersion: string; + private readonly loadedTime = moment().utc(); // the date is compared to time in UTC format coming from the service + + constructor( + kibanaVersion: string, + private readonly userLanguage: string, + private readonly fetchInterval: number, + private readonly storage: NewsfeedStorage + ) { + // The API only accepts versions in the format `X.Y.Z`, so we need to drop the `-SNAPSHOT` or any other label after it + this.kibanaVersion = kibanaVersion.replace(/^(\d+\.\d+\.\d+).*/, '$1'); + } + + shouldFetch(): boolean { + const lastFetchUtc = this.storage.getLastFetchTime(); + if (!lastFetchUtc) { + return true; + } + const last = moment(lastFetchUtc); + + // does the last fetch time precede the time that the page was loaded? + if (this.loadedTime.diff(last) > 0) { + return true; + } + + const now = moment.utc(); // always use UTC to compare timestamps that came from the service + const duration = moment.duration(now.diff(last)); + return duration.asMilliseconds() > this.fetchInterval; + } + + updateLastFetch() { + this.storage.setLastFetchTime(new Date()); + } + + fetchNewsfeedItems(http: HttpSetup, config: ApiConfig): Rx.Observable { + const urlPath = config.pathTemplate.replace('{VERSION}', this.kibanaVersion); + const fullUrl = (config.urlRoot || NEWSFEED_DEFAULT_SERVICE_BASE_URL) + urlPath; + + return Rx.from( + http + .fetch(fullUrl, { + method: 'GET', + }) + .then(({ items }: { items: ApiItem[] }) => { + return this.convertResponse(items); + }) + ); + } + + private convertResponse(items: ApiItem[]): FetchResult { + const feedItems = convertItems(items, this.userLanguage); + const hasNew = this.storage.setFetchedItems(feedItems.map((item) => item.hash)); + + return { + error: null, + kibanaVersion: this.kibanaVersion, + hasNew, + feedItems, + }; + } +} diff --git a/src/plugins/newsfeed/public/lib/storage.ts b/src/plugins/newsfeed/public/lib/storage.ts new file mode 100644 index 0000000000000..9f78fc977b67a --- /dev/null +++ b/src/plugins/newsfeed/public/lib/storage.ts @@ -0,0 +1,86 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import moment from 'moment'; +import { Observable, BehaviorSubject } from 'rxjs'; + +/** + * Persistence layer for the newsfeed driver + */ +export class NewsfeedStorage { + private readonly lastFetchStorageKey: string; + private readonly readStatusStorageKey: string; + private readonly unreadStatus$: BehaviorSubject; + + constructor(storagePrefix: string) { + this.lastFetchStorageKey = `newsfeed.${storagePrefix}.lastFetch`; + this.readStatusStorageKey = `newsfeed.${storagePrefix}.readStatus`; + this.unreadStatus$ = new BehaviorSubject(anyUnread(this.getReadStatus())); + } + + getLastFetchTime(): Date | undefined { + const lastFetchUtc = localStorage.getItem(this.lastFetchStorageKey); + if (!lastFetchUtc) { + return undefined; + } + + return moment(lastFetchUtc, 'x').toDate(); // parse as unix ms timestamp (already is UTC) + } + + setLastFetchTime(date: Date) { + localStorage.setItem(this.lastFetchStorageKey, JSON.stringify(date.getTime())); + } + + setFetchedItems(itemHashes: string[]): boolean { + const currentReadStatus = this.getReadStatus(); + + const newReadStatus: Record = {}; + itemHashes.forEach((hash) => { + newReadStatus[hash] = currentReadStatus[hash] ?? false; + }); + + this.setReadStatus(newReadStatus); + return anyUnread(newReadStatus); + } + + /** + * Marks given items as read, and return the overall unread status. + */ + markItemsAsRead(itemHashes: string[]): boolean { + const updatedReadStatus = this.getReadStatus(); + itemHashes.forEach((hash) => { + updatedReadStatus[hash] = true; + }); + this.setReadStatus(updatedReadStatus); + return anyUnread(updatedReadStatus); + } + + getUnreadStatus(): boolean { + return this.unreadStatus$.value; + } + + getUnreadStatus$(): Observable { + return this.unreadStatus$.asObservable(); + } + + private getReadStatus(): Record { + try { + return JSON.parse(localStorage.getItem(this.readStatusStorageKey) || '{}'); + } catch (e) { + return {}; + } + } + + private setReadStatus(status: Record) { + this.unreadStatus$.next(anyUnread(status)); + localStorage.setItem(this.readStatusStorageKey, JSON.stringify(status)); + } +} + +const anyUnread = (status: Record): boolean => + Object.values(status).some((read) => !read); diff --git a/src/plugins/newsfeed/public/plugin.tsx b/src/plugins/newsfeed/public/plugin.tsx index a788b3c4d0b59..782a51377311c 100644 --- a/src/plugins/newsfeed/public/plugin.tsx +++ b/src/plugins/newsfeed/public/plugin.tsx @@ -7,15 +7,15 @@ */ import * as Rx from 'rxjs'; -import { catchError, takeUntil, share } from 'rxjs/operators'; +import { catchError, takeUntil } from 'rxjs/operators'; import ReactDOM from 'react-dom'; import React from 'react'; import moment from 'moment'; import { I18nProvider } from '@kbn/i18n/react'; import { PluginInitializerContext, CoreSetup, CoreStart, Plugin } from 'src/core/public'; -import { NewsfeedPluginBrowserConfig, FetchResult } from './types'; -import { NewsfeedNavButton, NewsfeedApiFetchResult } from './components/newsfeed_header_nav_button'; -import { getApi, NewsfeedApiEndpoint } from './lib/api'; +import { NewsfeedPluginBrowserConfig } from './types'; +import { NewsfeedNavButton } from './components/newsfeed_header_nav_button'; +import { getApi, NewsfeedApi, NewsfeedApiEndpoint } from './lib/api'; export type NewsfeedPublicPluginSetup = ReturnType; export type NewsfeedPublicPluginStart = ReturnType; @@ -42,10 +42,10 @@ export class NewsfeedPublicPlugin } public start(core: CoreStart) { - const api$ = this.fetchNewsfeed(core, this.config).pipe(share()); + const api = this.createNewsfeedApi(core, this.config, NewsfeedApiEndpoint.KIBANA); core.chrome.navControls.registerRight({ order: 1000, - mount: (target) => this.mount(api$, target), + mount: (target) => this.mount(api, target), }); return { @@ -56,7 +56,7 @@ export class NewsfeedPublicPlugin pathTemplate: `/${endpoint}/v{VERSION}.json`, }, }); - return this.fetchNewsfeed(core, config); + return this.createNewsfeedApi(core, config, endpoint); }, }; } @@ -65,21 +65,26 @@ export class NewsfeedPublicPlugin this.stop$.next(); } - private fetchNewsfeed( + private createNewsfeedApi( core: CoreStart, - config: NewsfeedPluginBrowserConfig - ): Rx.Observable { + config: NewsfeedPluginBrowserConfig, + newsfeedId: NewsfeedApiEndpoint + ): NewsfeedApi { const { http } = core; - return getApi(http, config, this.kibanaVersion).pipe( - takeUntil(this.stop$), // stop the interval when stop method is called - catchError(() => Rx.of(null)) // do not throw error - ); + const api = getApi(http, config, this.kibanaVersion, newsfeedId); + return { + markAsRead: api.markAsRead, + fetchResults$: api.fetchResults$.pipe( + takeUntil(this.stop$), // stop the interval when stop method is called + catchError(() => Rx.of(null)) // do not throw error + ), + }; } - private mount(api$: NewsfeedApiFetchResult, targetDomElement: HTMLElement) { + private mount(api: NewsfeedApi, targetDomElement: HTMLElement) { ReactDOM.render( - + , targetDomElement ); From 464d82435a2699663c1759d8b68b26a0fa6197fb Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Wed, 19 May 2021 16:04:31 +0200 Subject: [PATCH 2/7] add unit tests --- src/plugins/newsfeed/public/lib/api.test.ts | 695 ------------------ src/plugins/newsfeed/public/lib/api.ts | 10 +- .../newsfeed/public/lib/convert_items.test.ts | 144 ++++ .../newsfeed/public/lib/convert_items.ts | 6 +- .../newsfeed/public/lib/driver.test.ts | 42 ++ src/plugins/newsfeed/public/lib/driver.ts | 4 - .../newsfeed/public/lib/storage.mock.ts | 26 + .../newsfeed/public/lib/storage.test.ts | 165 +++++ src/plugins/newsfeed/public/lib/storage.ts | 11 +- 9 files changed, 392 insertions(+), 711 deletions(-) delete mode 100644 src/plugins/newsfeed/public/lib/api.test.ts create mode 100644 src/plugins/newsfeed/public/lib/convert_items.test.ts create mode 100644 src/plugins/newsfeed/public/lib/driver.test.ts create mode 100644 src/plugins/newsfeed/public/lib/storage.mock.ts create mode 100644 src/plugins/newsfeed/public/lib/storage.test.ts diff --git a/src/plugins/newsfeed/public/lib/api.test.ts b/src/plugins/newsfeed/public/lib/api.test.ts deleted file mode 100644 index e142ffb4f6989..0000000000000 --- a/src/plugins/newsfeed/public/lib/api.test.ts +++ /dev/null @@ -1,695 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0 and the Server Side Public License, v 1; you may not use this file except - * in compliance with, at your election, the Elastic License 2.0 or the Server - * Side Public License, v 1. - */ - -import { take, tap, toArray } from 'rxjs/operators'; -import { interval, race } from 'rxjs'; -import sinon, { stub } from 'sinon'; -import moment from 'moment'; -import { HttpSetup } from 'src/core/public'; -import { - NEWSFEED_HASH_SET_STORAGE_KEY, - NEWSFEED_LAST_FETCH_STORAGE_KEY, -} from '../../common/constants'; -import { ApiItem, NewsfeedItem, NewsfeedPluginBrowserConfig } from '../types'; -import { NewsfeedApiDriver, getApi } from './api'; - -const localStorageGet = sinon.stub(); -const sessionStoragetGet = sinon.stub(); - -Object.defineProperty(window, 'localStorage', { - value: { - getItem: localStorageGet, - setItem: stub(), - }, - writable: true, -}); -Object.defineProperty(window, 'sessionStorage', { - value: { - getItem: sessionStoragetGet, - setItem: stub(), - }, - writable: true, -}); - -jest.mock('uuid', () => ({ - v4: () => 'NEW_UUID', -})); - -describe('NewsfeedApiDriver', () => { - const kibanaVersion = '99.999.9-test_version'; // It'll remove the `-test_version` bit - const userLanguage = 'en'; - const fetchInterval = 2000; - const getDriver = () => new NewsfeedApiDriver(kibanaVersion, userLanguage, fetchInterval); - - afterEach(() => { - sinon.reset(); - }); - - describe('shouldFetch', () => { - it('defaults to true', () => { - const driver = getDriver(); - expect(driver.shouldFetch()).toBe(true); - }); - - it('returns true if last fetch time precedes page load time', () => { - sessionStoragetGet.throws('Wrong key passed!'); - sessionStoragetGet - .withArgs(`${NEWSFEED_LAST_FETCH_STORAGE_KEY}.NEW_UUID`) - .returns(322642800000); // 1980-03-23 - const driver = getDriver(); - expect(driver.shouldFetch()).toBe(true); - }); - - it('returns false if last fetch time is recent enough', () => { - sessionStoragetGet.throws('Wrong key passed!'); - sessionStoragetGet - .withArgs(`${NEWSFEED_LAST_FETCH_STORAGE_KEY}.NEW_UUID`) - .returns(3005017200000); // 2065-03-23 - const driver = getDriver(); - expect(driver.shouldFetch()).toBe(false); - }); - }); - - describe('updateHashes', () => { - it('returns previous and current storage', () => { - const driver = getDriver(); - const items: NewsfeedItem[] = [ - { - title: 'Good news, everyone!', - description: 'good item description', - linkText: 'click here', - linkUrl: 'about:blank', - badge: 'test', - publishOn: moment(1572489035150), - expireOn: moment(1572489047858), - hash: 'hash1oneoneoneone', - }, - ]; - expect(driver.updateHashes(items)).toMatchInlineSnapshot(` - Object { - "current": Array [ - "hash1oneoneoneone", - ], - "previous": Array [], - } - `); - }); - - it('concatenates the previous hashes with the current', () => { - localStorageGet.throws('Wrong key passed!'); - localStorageGet.withArgs(`${NEWSFEED_HASH_SET_STORAGE_KEY}.NEW_UUID`).returns('happyness'); - const driver = getDriver(); - const items: NewsfeedItem[] = [ - { - title: 'Better news, everyone!', - description: 'better item description', - linkText: 'click there', - linkUrl: 'about:blank', - badge: 'concatentated', - publishOn: moment(1572489035150), - expireOn: moment(1572489047858), - hash: 'three33hash', - }, - ]; - expect(driver.updateHashes(items)).toMatchInlineSnapshot(` - Object { - "current": Array [ - "happyness", - "three33hash", - ], - "previous": Array [ - "happyness", - ], - } - `); - }); - }); - - it('Validates items for required fields', () => { - const driver = getDriver(); - expect(driver.validateItem({})).toBe(false); - expect( - driver.validateItem({ - title: 'Gadzooks!', - description: 'gadzooks item description', - linkText: 'click here', - linkUrl: 'about:blank', - badge: 'test', - publishOn: moment(1572489035150), - expireOn: moment(1572489047858), - hash: 'hash2twotwotwotwotwo', - }) - ).toBe(true); - expect( - driver.validateItem({ - title: 'Gadzooks!', - description: 'gadzooks item description', - linkText: 'click here', - linkUrl: 'about:blank', - publishOn: moment(1572489035150), - hash: 'hash2twotwotwotwotwo', - }) - ).toBe(true); - expect( - driver.validateItem({ - title: 'Gadzooks!', - description: 'gadzooks item description', - linkText: 'click here', - linkUrl: 'about:blank', - publishOn: moment(1572489035150), - // hash: 'hash2twotwotwotwotwo', // should fail because this is missing - }) - ).toBe(false); - }); - - describe('modelItems', () => { - it('Models empty set with defaults', () => { - const driver = getDriver(); - const apiItems: ApiItem[] = []; - expect(driver.modelItems(apiItems)).toMatchInlineSnapshot(` - Object { - "error": null, - "feedItems": Array [], - "hasNew": false, - "kibanaVersion": "99.999.9", - } - `); - }); - - it('Selects default language', () => { - const driver = getDriver(); - const apiItems: ApiItem[] = [ - { - title: { - en: 'speaking English', - es: 'habla Espanol', - }, - description: { - en: 'language test', - es: 'idiomas', - }, - languages: ['en', 'es'], - link_text: { - en: 'click here', - es: 'aqui', - }, - link_url: { - en: 'xyzxyzxyz', - es: 'abcabc', - }, - badge: { - en: 'firefighter', - es: 'bombero', - }, - publish_on: new Date('2014-10-31T04:23:47Z'), - expire_on: new Date('2049-10-31T04:23:47Z'), - hash: 'abcabc1231123123hash', - }, - ]; - expect(driver.modelItems(apiItems)).toMatchObject({ - error: null, - feedItems: [ - { - badge: 'firefighter', - description: 'language test', - hash: 'abcabc1231', - linkText: 'click here', - linkUrl: 'xyzxyzxyz', - title: 'speaking English', - }, - ], - hasNew: true, - kibanaVersion: '99.999.9', - }); - }); - - it("Falls back to English when user language isn't present", () => { - // Set Language to French - const driver = new NewsfeedApiDriver(kibanaVersion, 'fr', fetchInterval); - const apiItems: ApiItem[] = [ - { - title: { - en: 'speaking English', - fr: 'Le Title', - }, - description: { - en: 'not French', - fr: 'Le Description', - }, - languages: ['en', 'fr'], - link_text: { - en: 'click here', - fr: 'Le Link Text', - }, - link_url: { - en: 'xyzxyzxyz', - fr: 'le_url', - }, - badge: { - en: 'firefighter', - fr: 'le_badge', - }, - publish_on: new Date('2014-10-31T04:23:47Z'), - expire_on: new Date('2049-10-31T04:23:47Z'), - hash: 'frfrfrfr1231123123hash', - }, // fallback: no - { - title: { - en: 'speaking English', - es: 'habla Espanol', - }, - description: { - en: 'not French', - es: 'no Espanol', - }, - languages: ['en', 'es'], - link_text: { - en: 'click here', - es: 'aqui', - }, - link_url: { - en: 'xyzxyzxyz', - es: 'abcabc', - }, - badge: { - en: 'firefighter', - es: 'bombero', - }, - publish_on: new Date('2014-10-31T04:23:47Z'), - expire_on: new Date('2049-10-31T04:23:47Z'), - hash: 'enenenen1231123123hash', - }, // fallback: yes - ]; - expect(driver.modelItems(apiItems)).toMatchObject({ - error: null, - feedItems: [ - { - badge: 'le_badge', - description: 'Le Description', - hash: 'frfrfrfr12', - linkText: 'Le Link Text', - linkUrl: 'le_url', - title: 'Le Title', - }, - { - badge: 'firefighter', - description: 'not French', - hash: 'enenenen12', - linkText: 'click here', - linkUrl: 'xyzxyzxyz', - title: 'speaking English', - }, - ], - hasNew: true, - kibanaVersion: '99.999.9', - }); - }); - - it('Models multiple items into an API FetchResult', () => { - const driver = getDriver(); - const apiItems: ApiItem[] = [ - { - title: { - en: 'guess what', - }, - description: { - en: 'this tests the modelItems function', - }, - link_text: { - en: 'click here', - }, - link_url: { - en: 'about:blank', - }, - publish_on: new Date('2014-10-31T04:23:47Z'), - expire_on: new Date('2049-10-31T04:23:47Z'), - hash: 'abcabc1231123123hash', - }, - { - title: { - en: 'guess when', - }, - description: { - en: 'this also tests the modelItems function', - }, - link_text: { - en: 'click here', - }, - link_url: { - en: 'about:blank', - }, - badge: { - en: 'hero', - }, - publish_on: new Date('2014-10-31T04:23:47Z'), - expire_on: new Date('2049-10-31T04:23:47Z'), - hash: 'defdefdef456456456', - }, - ]; - expect(driver.modelItems(apiItems)).toMatchObject({ - error: null, - feedItems: [ - { - badge: null, - description: 'this tests the modelItems function', - hash: 'abcabc1231', - linkText: 'click here', - linkUrl: 'about:blank', - title: 'guess what', - }, - { - badge: 'hero', - description: 'this also tests the modelItems function', - hash: 'defdefdef4', - linkText: 'click here', - linkUrl: 'about:blank', - title: 'guess when', - }, - ], - hasNew: true, - kibanaVersion: '99.999.9', - }); - }); - - it('Filters expired', () => { - const driver = getDriver(); - const apiItems: ApiItem[] = [ - { - title: { - en: 'guess what', - }, - description: { - en: 'this tests the modelItems function', - }, - link_text: { - en: 'click here', - }, - link_url: { - en: 'about:blank', - }, - publish_on: new Date('2013-10-31T04:23:47Z'), - expire_on: new Date('2014-10-31T04:23:47Z'), // too old - hash: 'abcabc1231123123hash', - }, - ]; - expect(driver.modelItems(apiItems)).toMatchInlineSnapshot(` - Object { - "error": null, - "feedItems": Array [], - "hasNew": false, - "kibanaVersion": "99.999.9", - } - `); - }); - - it('Filters pre-published', () => { - const driver = getDriver(); - const apiItems: ApiItem[] = [ - { - title: { - en: 'guess what', - }, - description: { - en: 'this tests the modelItems function', - }, - link_text: { - en: 'click here', - }, - link_url: { - en: 'about:blank', - }, - publish_on: new Date('2055-10-31T04:23:47Z'), // too new - expire_on: new Date('2056-10-31T04:23:47Z'), - hash: 'abcabc1231123123hash', - }, - ]; - expect(driver.modelItems(apiItems)).toMatchInlineSnapshot(` - Object { - "error": null, - "feedItems": Array [], - "hasNew": false, - "kibanaVersion": "99.999.9", - } - `); - }); - }); -}); - -describe('getApi', () => { - const mockHttpGet = jest.fn(); - let httpMock = ({ - fetch: mockHttpGet, - } as unknown) as HttpSetup; - const getHttpMockWithItems = (mockApiItems: ApiItem[]) => ( - arg1: string, - arg2: { method: string } - ) => { - if ( - arg1 === 'http://fakenews.co/kibana-test/v6.8.2.json' && - arg2.method && - arg2.method === 'GET' - ) { - return Promise.resolve({ items: mockApiItems }); - } - return Promise.reject('wrong args!'); - }; - let configMock: NewsfeedPluginBrowserConfig; - - afterEach(() => { - jest.resetAllMocks(); - }); - - beforeEach(() => { - configMock = { - service: { - urlRoot: 'http://fakenews.co', - pathTemplate: '/kibana-test/v{VERSION}.json', - }, - mainInterval: moment.duration(86400000), - fetchInterval: moment.duration(86400000), - }; - httpMock = ({ - fetch: mockHttpGet, - } as unknown) as HttpSetup; - }); - - it('creates a result', (done) => { - mockHttpGet.mockImplementationOnce(() => Promise.resolve({ items: [] })); - getApi(httpMock, configMock, '6.8.2').subscribe((result) => { - expect(result).toMatchInlineSnapshot(` - Object { - "error": null, - "feedItems": Array [], - "hasNew": false, - "kibanaVersion": "6.8.2", - } - `); - done(); - }); - }); - - it('hasNew is true when the service returns hashes not in the cache', (done) => { - const mockApiItems: ApiItem[] = [ - { - title: { - en: 'speaking English', - es: 'habla Espanol', - }, - description: { - en: 'language test', - es: 'idiomas', - }, - languages: ['en', 'es'], - link_text: { - en: 'click here', - es: 'aqui', - }, - link_url: { - en: 'xyzxyzxyz', - es: 'abcabc', - }, - badge: { - en: 'firefighter', - es: 'bombero', - }, - publish_on: new Date('2014-10-31T04:23:47Z'), - expire_on: new Date('2049-10-31T04:23:47Z'), - hash: 'abcabc1231123123hash', - }, - ]; - - mockHttpGet.mockImplementationOnce(getHttpMockWithItems(mockApiItems)); - - getApi(httpMock, configMock, '6.8.2').subscribe((result) => { - expect(result).toMatchInlineSnapshot(` - Object { - "error": null, - "feedItems": Array [ - Object { - "badge": "firefighter", - "description": "language test", - "expireOn": "2049-10-31T04:23:47.000Z", - "hash": "abcabc1231", - "linkText": "click here", - "linkUrl": "xyzxyzxyz", - "publishOn": "2014-10-31T04:23:47.000Z", - "title": "speaking English", - }, - ], - "hasNew": true, - "kibanaVersion": "6.8.2", - } - `); - done(); - }); - }); - - it('hasNew is false when service returns hashes that are all stored', (done) => { - localStorageGet.throws('Wrong key passed!'); - localStorageGet.withArgs(`${NEWSFEED_HASH_SET_STORAGE_KEY}.NEW_UUID`).returns('happyness'); - const mockApiItems: ApiItem[] = [ - { - title: { en: 'hasNew test' }, - description: { en: 'test' }, - link_text: { en: 'click here' }, - link_url: { en: 'xyzxyzxyz' }, - badge: { en: 'firefighter' }, - publish_on: new Date('2014-10-31T04:23:47Z'), - expire_on: new Date('2049-10-31T04:23:47Z'), - hash: 'happyness', - }, - ]; - mockHttpGet.mockImplementationOnce(getHttpMockWithItems(mockApiItems)); - getApi(httpMock, configMock, '6.8.2').subscribe((result) => { - expect(result).toMatchInlineSnapshot(` - Object { - "error": null, - "feedItems": Array [ - Object { - "badge": "firefighter", - "description": "test", - "expireOn": "2049-10-31T04:23:47.000Z", - "hash": "happyness", - "linkText": "click here", - "linkUrl": "xyzxyzxyz", - "publishOn": "2014-10-31T04:23:47.000Z", - "title": "hasNew test", - }, - ], - "hasNew": false, - "kibanaVersion": "6.8.2", - } - `); - done(); - }); - }); - - it('forwards an error', (done) => { - mockHttpGet.mockImplementationOnce((arg1, arg2) => Promise.reject('sorry, try again later!')); - - getApi(httpMock, configMock, '6.8.2').subscribe((result) => { - expect(result).toMatchInlineSnapshot(` - Object { - "error": "sorry, try again later!", - "feedItems": Array [], - "hasNew": false, - "kibanaVersion": "6.8.2", - } - `); - done(); - }); - }); - - describe('Retry fetching', () => { - const successItems: ApiItem[] = [ - { - title: { en: 'hasNew test' }, - description: { en: 'test' }, - link_text: { en: 'click here' }, - link_url: { en: 'xyzxyzxyz' }, - badge: { en: 'firefighter' }, - publish_on: new Date('2014-10-31T04:23:47Z'), - expire_on: new Date('2049-10-31T04:23:47Z'), - hash: 'happyness', - }, - ]; - - it("retries until fetch doesn't error", (done) => { - configMock.mainInterval = moment.duration(10); // fast retry for testing - mockHttpGet - .mockImplementationOnce(() => Promise.reject('Sorry, try again later!')) - .mockImplementationOnce(() => Promise.reject('Sorry, internal server error!')) - .mockImplementationOnce(() => Promise.reject("Sorry, it's too cold to go outside!")) - .mockImplementationOnce(getHttpMockWithItems(successItems)); - - getApi(httpMock, configMock, '6.8.2') - .pipe(take(4), toArray()) - .subscribe((result) => { - expect(result).toMatchInlineSnapshot(` - Array [ - Object { - "error": "Sorry, try again later!", - "feedItems": Array [], - "hasNew": false, - "kibanaVersion": "6.8.2", - }, - Object { - "error": "Sorry, internal server error!", - "feedItems": Array [], - "hasNew": false, - "kibanaVersion": "6.8.2", - }, - Object { - "error": "Sorry, it's too cold to go outside!", - "feedItems": Array [], - "hasNew": false, - "kibanaVersion": "6.8.2", - }, - Object { - "error": null, - "feedItems": Array [ - Object { - "badge": "firefighter", - "description": "test", - "expireOn": "2049-10-31T04:23:47.000Z", - "hash": "happyness", - "linkText": "click here", - "linkUrl": "xyzxyzxyz", - "publishOn": "2014-10-31T04:23:47.000Z", - "title": "hasNew test", - }, - ], - "hasNew": false, - "kibanaVersion": "6.8.2", - }, - ] - `); - done(); - }); - }); - - it("doesn't retry if fetch succeeds", (done) => { - configMock.mainInterval = moment.duration(10); // fast retry for testing - mockHttpGet.mockImplementation(getHttpMockWithItems(successItems)); - - const timeout$ = interval(1000); // lets us capture some results after a short time - let timesFetched = 0; - - const get$ = getApi(httpMock, configMock, '6.8.2').pipe( - tap(() => { - timesFetched++; - }) - ); - - race(get$, timeout$).subscribe(() => { - expect(timesFetched).toBe(1); // first fetch was successful, so there was no retry - done(); - }); - }); - }); -}); diff --git a/src/plugins/newsfeed/public/lib/api.ts b/src/plugins/newsfeed/public/lib/api.ts index febce9021afb4..b9cab3e71353c 100644 --- a/src/plugins/newsfeed/public/lib/api.ts +++ b/src/plugins/newsfeed/public/lib/api.ts @@ -46,7 +46,7 @@ export function getApi( ): NewsfeedApi { const userLanguage = i18n.getLocale(); const fetchInterval = config.fetchInterval.asMilliseconds(); - const mainInterval = 2000; // config.mainInterval.asMilliseconds(); + const mainInterval = config.mainInterval.asMilliseconds(); const storage = new NewsfeedStorage(newsfeedId); const driver = new NewsfeedApiDriver(kibanaVersion, userLanguage, fetchInterval, storage); @@ -65,14 +65,14 @@ export function getApi( }) ) ), - tap(() => driver.updateLastFetch()) + tap(() => storage.setLastFetchTime(new Date())) ); - const merged$ = combineLatest([results$, storage.getUnreadStatus$()]).pipe( - map(([results, unreadStatus]) => { + const merged$ = combineLatest([results$, storage.isAnyUnread$()]).pipe( + map(([results, isAnyUnread]) => { return { ...results, - hasNew: results.error ? false : unreadStatus, + hasNew: results.error ? false : isAnyUnread, }; }) ); diff --git a/src/plugins/newsfeed/public/lib/convert_items.test.ts b/src/plugins/newsfeed/public/lib/convert_items.test.ts new file mode 100644 index 0000000000000..c55a46bed5d38 --- /dev/null +++ b/src/plugins/newsfeed/public/lib/convert_items.test.ts @@ -0,0 +1,144 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import moment from 'moment'; +import { omit } from 'lodash'; +import { validateIntegrity, validatePublishedDate, convertItem } from './convert_items'; +import type { ApiItem, NewsfeedItem } from '../types'; + +const createApiItem = (parts: Partial = {}): ApiItem => ({ + hash: 'hash', + expire_on: new Date(), + publish_on: new Date(), + title: {}, + description: {}, + link_url: {}, + ...parts, +}); + +const createNewsfeedItem = (parts: Partial = {}): NewsfeedItem => ({ + title: 'title', + description: 'description', + linkText: 'linkText', + linkUrl: 'linkUrl', + badge: 'badge', + publishOn: moment(), + expireOn: moment(), + hash: 'hash', + ...parts, +}); + +describe('convertItem', () => { + const item = createApiItem({ + languages: ['en', 'fr'], + title: { + en: 'en title', + fr: 'fr title', + }, + description: { + en: 'en desc', + fr: 'fr desc', + }, + link_text: { + en: 'en link text', + fr: 'fr link text', + }, + link_url: { + en: 'en link url', + fr: 'fr link url', + }, + badge: { + en: 'en badge', + fr: 'fr badge', + }, + publish_on: new Date('2014-10-31T04:23:47Z'), + expire_on: new Date('2049-10-31T04:23:47Z'), + hash: 'hash', + }); + + it('converts api items to newsfeed items using the specified language', () => { + expect(convertItem(item, 'fr')).toMatchObject({ + title: 'fr title', + description: 'fr desc', + linkText: 'fr link text', + linkUrl: 'fr link url', + badge: 'fr badge', + hash: 'hash', + }); + }); + + it('fallbacks to `en` is the language is not present', () => { + expect(convertItem(item, 'de')).toMatchObject({ + title: 'en title', + description: 'en desc', + linkText: 'en link text', + linkUrl: 'en link url', + badge: 'en badge', + hash: 'hash', + }); + }); +}); + +describe('validatePublishedDate', () => { + it('returns false when the publish date is not reached yet', () => { + expect( + validatePublishedDate( + createApiItem({ + publish_on: new Date('2055-10-31T04:23:47Z'), // too new + expire_on: new Date('2056-10-31T04:23:47Z'), + }) + ) + ).toBe(false); + }); + + it('returns false when the expire date is already reached', () => { + expect( + validatePublishedDate( + createApiItem({ + publish_on: new Date('2013-10-31T04:23:47Z'), + expire_on: new Date('2014-10-31T04:23:47Z'), // too old + }) + ) + ).toBe(false); + }); + + it('returns true when current date is between the publish and expire dates', () => { + expect( + validatePublishedDate( + createApiItem({ + publish_on: new Date('2014-10-31T04:23:47Z'), + expire_on: new Date('2049-10-31T04:23:47Z'), + }) + ) + ).toBe(true); + }); +}); + +describe('validateIntegrity', () => { + it('returns false if `title` is missing', () => { + expect(validateIntegrity(omit(createNewsfeedItem(), 'title'))).toBe(false); + }); + it('returns false if `description` is missing', () => { + expect(validateIntegrity(omit(createNewsfeedItem(), 'description'))).toBe(false); + }); + it('returns false if `linkText` is missing', () => { + expect(validateIntegrity(omit(createNewsfeedItem(), 'linkText'))).toBe(false); + }); + it('returns false if `linkUrl` is missing', () => { + expect(validateIntegrity(omit(createNewsfeedItem(), 'linkUrl'))).toBe(false); + }); + it('returns false if `publishOn` is missing', () => { + expect(validateIntegrity(omit(createNewsfeedItem(), 'publishOn'))).toBe(false); + }); + it('returns false if `hash` is missing', () => { + expect(validateIntegrity(omit(createNewsfeedItem(), 'hash'))).toBe(false); + }); + it('returns true if all mandatory fields are present', () => { + expect(validateIntegrity(createNewsfeedItem())).toBe(true); + }); +}); diff --git a/src/plugins/newsfeed/public/lib/convert_items.ts b/src/plugins/newsfeed/public/lib/convert_items.ts index 1052487e677e5..f80c9397aa0a0 100644 --- a/src/plugins/newsfeed/public/lib/convert_items.ts +++ b/src/plugins/newsfeed/public/lib/convert_items.ts @@ -17,7 +17,7 @@ export const convertItems = (items: ApiItem[], userLanguage: string): NewsfeedIt .filter(validateIntegrity); }; -const validatePublishedDate = (item: ApiItem): boolean => { +export const validatePublishedDate = (item: ApiItem): boolean => { if (moment(item.expire_on).isBefore(Date.now())) { return false; // ignore item if expired } @@ -28,7 +28,7 @@ const validatePublishedDate = (item: ApiItem): boolean => { return true; }; -const convertItem = (rawItem: ApiItem, userLanguage: string): NewsfeedItem => { +export const convertItem = (rawItem: ApiItem, userLanguage: string): NewsfeedItem => { const { expire_on: expireOnUtc, publish_on: publishOnUtc, @@ -58,7 +58,7 @@ const convertItem = (rawItem: ApiItem, userLanguage: string): NewsfeedItem => { }; }; -const validateIntegrity = (item: Partial): boolean => { +export const validateIntegrity = (item: Partial): boolean => { const hasMissing = [ item.title, item.description, diff --git a/src/plugins/newsfeed/public/lib/driver.test.ts b/src/plugins/newsfeed/public/lib/driver.test.ts new file mode 100644 index 0000000000000..2f3dcfcb51b97 --- /dev/null +++ b/src/plugins/newsfeed/public/lib/driver.test.ts @@ -0,0 +1,42 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { NewsfeedApiDriver } from './driver'; +import { storageMock } from './storage.mock'; + +const kibanaVersion = '8.0.0'; +const userLanguage = 'en'; +const fetchInterval = 2000; + +describe('NewsfeedApiDriver', () => { + let driver: NewsfeedApiDriver; + let storage: ReturnType; + + beforeEach(() => { + storage = storageMock.create(); + driver = new NewsfeedApiDriver(kibanaVersion, userLanguage, fetchInterval, storage); + }); + + describe('shouldFetch', () => { + it('returns true if no value is present in the storage', () => { + storage.getLastFetchTime.mockReturnValue(undefined); + expect(driver.shouldFetch()).toBe(true); + expect(storage.getLastFetchTime).toHaveBeenCalledTimes(1); + }); + + it('returns true if last fetch time precedes page load time', () => { + storage.getLastFetchTime.mockReturnValue(new Date(322642800000)); // 1980-03-23 + expect(driver.shouldFetch()).toBe(true); + }); + + it('returns false if last fetch time is recent enough', () => { + storage.getLastFetchTime.mockReturnValue(new Date(3005017200000)); // 2065-03-23 + expect(driver.shouldFetch()).toBe(false); + }); + }); +}); diff --git a/src/plugins/newsfeed/public/lib/driver.ts b/src/plugins/newsfeed/public/lib/driver.ts index e45863d603765..ae0052acebfe7 100644 --- a/src/plugins/newsfeed/public/lib/driver.ts +++ b/src/plugins/newsfeed/public/lib/driver.ts @@ -47,10 +47,6 @@ export class NewsfeedApiDriver { return duration.asMilliseconds() > this.fetchInterval; } - updateLastFetch() { - this.storage.setLastFetchTime(new Date()); - } - fetchNewsfeedItems(http: HttpSetup, config: ApiConfig): Rx.Observable { const urlPath = config.pathTemplate.replace('{VERSION}', this.kibanaVersion); const fullUrl = (config.urlRoot || NEWSFEED_DEFAULT_SERVICE_BASE_URL) + urlPath; diff --git a/src/plugins/newsfeed/public/lib/storage.mock.ts b/src/plugins/newsfeed/public/lib/storage.mock.ts new file mode 100644 index 0000000000000..98681e20b0665 --- /dev/null +++ b/src/plugins/newsfeed/public/lib/storage.mock.ts @@ -0,0 +1,26 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { PublicMethodsOf } from '@kbn/utility-types'; +import type { NewsfeedStorage } from './storage'; + +const createStorageMock = () => { + const mock: jest.Mocked> = { + getLastFetchTime: jest.fn(), + setLastFetchTime: jest.fn(), + setFetchedItems: jest.fn(), + markItemsAsRead: jest.fn(), + isAnyUnread: jest.fn(), + isAnyUnread$: jest.fn(), + }; + return mock as jest.Mocked; +}; + +export const storageMock = { + create: createStorageMock, +}; diff --git a/src/plugins/newsfeed/public/lib/storage.test.ts b/src/plugins/newsfeed/public/lib/storage.test.ts new file mode 100644 index 0000000000000..1c424d8247e86 --- /dev/null +++ b/src/plugins/newsfeed/public/lib/storage.test.ts @@ -0,0 +1,165 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { NewsfeedStorage, getStorageKey } from './storage'; +import { take } from 'rxjs/operators'; + +describe('NewsfeedStorage', () => { + const storagePrefix = 'test'; + let mockStorage: Record; + let storage: NewsfeedStorage; + + const getKey = (key: string) => getStorageKey(storagePrefix, key); + + beforeAll(() => { + Object.defineProperty(window, 'localStorage', { + value: { + getItem: (key: string) => { + return mockStorage[key] || null; + }, + setItem: (key: string, value: string) => { + mockStorage[key] = value; + }, + }, + writable: true, + }); + }); + + afterAll(() => { + delete (window as any).localStorage; + }); + + beforeEach(() => { + mockStorage = {}; + storage = new NewsfeedStorage(storagePrefix); + }); + + describe('getLastFetchTime', () => { + it('returns undefined if not set', () => { + expect(storage.getLastFetchTime()).toBeUndefined(); + }); + + it('returns the last value that was set', () => { + const date = new Date(); + storage.setLastFetchTime(date); + expect(storage.getLastFetchTime()!.getTime()).toEqual(date.getTime()); + }); + }); + + describe('setFetchedItems', () => { + it('updates the value in the storage', () => { + storage.setFetchedItems(['a', 'b', 'c']); + expect(JSON.parse(localStorage.getItem(getKey('readStatus'))!)).toEqual({ + a: false, + b: false, + c: false, + }); + }); + + it('preserves the read status if present', () => { + const initialValue = { a: true, b: false }; + window.localStorage.setItem(getKey('readStatus'), JSON.stringify(initialValue)); + storage.setFetchedItems(['a', 'b', 'c']); + expect(JSON.parse(localStorage.getItem(getKey('readStatus'))!)).toEqual({ + a: true, + b: false, + c: false, + }); + }); + + it('removes the old keys from the storage', () => { + const initialValue = { a: true, b: false, old: false }; + window.localStorage.setItem(getKey('readStatus'), JSON.stringify(initialValue)); + storage.setFetchedItems(['a', 'b', 'c']); + expect(JSON.parse(localStorage.getItem(getKey('readStatus'))!)).toEqual({ + a: true, + b: false, + c: false, + }); + }); + }); + + describe('markItemsAsRead', () => { + it('flags the entries as read', () => { + const initialValue = { a: true, b: false, c: false }; + window.localStorage.setItem(getKey('readStatus'), JSON.stringify(initialValue)); + storage.markItemsAsRead(['b']); + expect(JSON.parse(localStorage.getItem(getKey('readStatus'))!)).toEqual({ + a: true, + b: true, + c: false, + }); + }); + + it('add the entries when not present', () => { + const initialValue = { a: true, b: false, c: false }; + window.localStorage.setItem(getKey('readStatus'), JSON.stringify(initialValue)); + storage.markItemsAsRead(['b', 'new']); + expect(JSON.parse(localStorage.getItem(getKey('readStatus'))!)).toEqual({ + a: true, + b: true, + c: false, + new: true, + }); + }); + }); + + describe('isAnyUnread', () => { + it('returns true if any item was not read', () => { + storage.setFetchedItems(['a', 'b', 'c']); + storage.markItemsAsRead(['a']); + expect(storage.isAnyUnread()).toBe(true); + }); + + it('returns true if all item are unread', () => { + storage.setFetchedItems(['a', 'b', 'c']); + expect(storage.isAnyUnread()).toBe(true); + }); + + it('returns false if all item are unread', () => { + storage.setFetchedItems(['a', 'b', 'c']); + storage.markItemsAsRead(['a', 'b', 'c']); + expect(storage.isAnyUnread()).toBe(false); + }); + + it('loads the value initially present in localStorage', () => { + const initialValue = { a: true, b: false }; + window.localStorage.setItem(getKey('readStatus'), JSON.stringify(initialValue)); + storage = new NewsfeedStorage(storagePrefix); + expect(storage.isAnyUnread()).toBe(true); + }); + }); + + describe('isAnyUnread$', () => { + it('emits an initial value at subscription', async () => { + const initialValue = { a: true, b: false, c: false }; + window.localStorage.setItem(getKey('readStatus'), JSON.stringify(initialValue)); + storage = new NewsfeedStorage(storagePrefix); + + expect(await storage.isAnyUnread$().pipe(take(1)).toPromise()).toBe(true); + }); + + it('emits when `setFetchedItems` is called', () => { + const emissions: boolean[] = []; + storage.isAnyUnread$().subscribe((unread) => emissions.push(unread)); + + storage.setFetchedItems(['a', 'b', 'c']); + expect(emissions).toEqual([false, true]); + }); + + it('emits when `markItemsAsRead` is called', () => { + const emissions: boolean[] = []; + storage.isAnyUnread$().subscribe((unread) => emissions.push(unread)); + + storage.setFetchedItems(['a', 'b', 'c']); + storage.markItemsAsRead(['a', 'b']); + storage.markItemsAsRead(['c']); + expect(emissions).toEqual([false, true, true, false]); + }); + }); +}); diff --git a/src/plugins/newsfeed/public/lib/storage.ts b/src/plugins/newsfeed/public/lib/storage.ts index 9f78fc977b67a..3259323def41c 100644 --- a/src/plugins/newsfeed/public/lib/storage.ts +++ b/src/plugins/newsfeed/public/lib/storage.ts @@ -18,8 +18,8 @@ export class NewsfeedStorage { private readonly unreadStatus$: BehaviorSubject; constructor(storagePrefix: string) { - this.lastFetchStorageKey = `newsfeed.${storagePrefix}.lastFetch`; - this.readStatusStorageKey = `newsfeed.${storagePrefix}.readStatus`; + this.lastFetchStorageKey = getStorageKey(storagePrefix, 'lastFetch'); + this.readStatusStorageKey = getStorageKey(storagePrefix, 'readStatus'); this.unreadStatus$ = new BehaviorSubject(anyUnread(this.getReadStatus())); } @@ -60,11 +60,11 @@ export class NewsfeedStorage { return anyUnread(updatedReadStatus); } - getUnreadStatus(): boolean { + isAnyUnread(): boolean { return this.unreadStatus$.value; } - getUnreadStatus$(): Observable { + isAnyUnread$(): Observable { return this.unreadStatus$.asObservable(); } @@ -84,3 +84,6 @@ export class NewsfeedStorage { const anyUnread = (status: Record): boolean => Object.values(status).some((read) => !read); + +/** @internal */ +export const getStorageKey = (prefix: string, key: string) => `newsfeed.${prefix}.${key}`; From 442c4802772fc23eb7a35146d9e26dd770c5d53d Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Wed, 19 May 2021 20:45:08 +0200 Subject: [PATCH 3/7] add API unit tests --- .../newsfeed/public/lib/api.test.mocks.ts | 20 +++ src/plugins/newsfeed/public/lib/api.test.ts | 130 ++++++++++++++++++ .../newsfeed/public/lib/driver.mock.ts | 22 +++ 3 files changed, 172 insertions(+) create mode 100644 src/plugins/newsfeed/public/lib/api.test.mocks.ts create mode 100644 src/plugins/newsfeed/public/lib/api.test.ts create mode 100644 src/plugins/newsfeed/public/lib/driver.mock.ts diff --git a/src/plugins/newsfeed/public/lib/api.test.mocks.ts b/src/plugins/newsfeed/public/lib/api.test.mocks.ts new file mode 100644 index 0000000000000..677bc203cbef3 --- /dev/null +++ b/src/plugins/newsfeed/public/lib/api.test.mocks.ts @@ -0,0 +1,20 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { storageMock } from './storage.mock'; +import { driverMock } from './driver.mock'; + +export const storageInstanceMock = storageMock.create(); +jest.doMock('./storage', () => ({ + NewsfeedStorage: jest.fn().mockImplementation(() => storageInstanceMock), +})); + +export const driverInstanceMock = driverMock.create(); +jest.doMock('./driver', () => ({ + NewsfeedApiDriver: jest.fn().mockImplementation(() => driverInstanceMock), +})); diff --git a/src/plugins/newsfeed/public/lib/api.test.ts b/src/plugins/newsfeed/public/lib/api.test.ts new file mode 100644 index 0000000000000..9727557587cd1 --- /dev/null +++ b/src/plugins/newsfeed/public/lib/api.test.ts @@ -0,0 +1,130 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { driverInstanceMock, storageInstanceMock } from './api.test.mocks'; +import moment from 'moment'; +import { httpServiceMock } from '../../../../core/public/mocks'; +import { getApi } from './api'; +import { TestScheduler } from 'rxjs/testing'; +import { FetchResult, NewsfeedPluginBrowserConfig } from '../types'; +import { take } from 'rxjs/operators'; + +const kibanaVersion = '8.0.0'; +const newsfeedId = 'test'; + +const getTestScheduler = () => + new TestScheduler((actual, expected) => { + expect(actual).toEqual(expected); + }); + +const createConfig = (mainInternal: number): NewsfeedPluginBrowserConfig => ({ + mainInterval: moment.duration(mainInternal, 'ms'), + fetchInterval: moment.duration(mainInternal, 'ms'), + service: { + urlRoot: 'urlRoot', + pathTemplate: 'pathTemplate', + }, +}); + +const createFetchResult = (parts: Partial): FetchResult => ({ + feedItems: [], + hasNew: false, + error: null, + kibanaVersion, + ...parts, +}); + +describe('getApi', () => { + let http: ReturnType; + + beforeEach(() => { + http = httpServiceMock.createSetupContract(); + driverInstanceMock.shouldFetch.mockReturnValue(true); + }); + + afterEach(() => { + storageInstanceMock.isAnyUnread$.mockReset(); + driverInstanceMock.fetchNewsfeedItems.mockReset(); + }); + + it('merges the newsfeed and unread observables', () => { + getTestScheduler().run(({ expectObservable, cold }) => { + storageInstanceMock.isAnyUnread$.mockImplementation(() => { + return cold('a|', { + a: true, + }); + }); + driverInstanceMock.fetchNewsfeedItems.mockReturnValue( + cold('a|', { + a: createFetchResult({ feedItems: ['item' as any] }), + }) + ); + const api = getApi(http, createConfig(1000), kibanaVersion, newsfeedId); + + expectObservable(api.fetchResults$.pipe(take(1))).toBe('(a|)', { + a: createFetchResult({ + feedItems: ['item' as any], + hasNew: true, + }), + }); + }); + }); + + it('emits based on the predefined interval', () => { + getTestScheduler().run(({ expectObservable, cold }) => { + storageInstanceMock.isAnyUnread$.mockImplementation(() => { + return cold('a|', { + a: true, + }); + }); + driverInstanceMock.fetchNewsfeedItems.mockReturnValue( + cold('a|', { + a: createFetchResult({ feedItems: ['item' as any] }), + }) + ); + const api = getApi(http, createConfig(2), kibanaVersion, newsfeedId); + + expectObservable(api.fetchResults$.pipe(take(2))).toBe('a-(b|)', { + a: createFetchResult({ + feedItems: ['item' as any], + hasNew: true, + }), + b: createFetchResult({ + feedItems: ['item' as any], + hasNew: true, + }), + }); + }); + }); + + it('re-emits when the unread status changes', () => { + getTestScheduler().run(({ expectObservable, cold }) => { + storageInstanceMock.isAnyUnread$.mockImplementation(() => { + return cold('a--b', { + a: true, + b: false, + }); + }); + driverInstanceMock.fetchNewsfeedItems.mockReturnValue( + cold('(a|)', { + a: createFetchResult({}), + }) + ); + const api = getApi(http, createConfig(10), kibanaVersion, newsfeedId); + + expectObservable(api.fetchResults$.pipe(take(2))).toBe('a--(b|)', { + a: createFetchResult({ + hasNew: true, + }), + b: createFetchResult({ + hasNew: false, + }), + }); + }); + }); +}); diff --git a/src/plugins/newsfeed/public/lib/driver.mock.ts b/src/plugins/newsfeed/public/lib/driver.mock.ts new file mode 100644 index 0000000000000..8ae4ad1a82c4d --- /dev/null +++ b/src/plugins/newsfeed/public/lib/driver.mock.ts @@ -0,0 +1,22 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { PublicMethodsOf } from '@kbn/utility-types'; +import type { NewsfeedApiDriver } from './driver'; + +const createDriverMock = () => { + const mock: jest.Mocked> = { + shouldFetch: jest.fn(), + fetchNewsfeedItems: jest.fn(), + }; + return mock as jest.Mocked; +}; + +export const driverMock = { + create: createDriverMock, +}; From 82723cb10f5a3a24667fd53880d0444fe2a83a00 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Wed, 19 May 2021 21:30:13 +0200 Subject: [PATCH 4/7] fix public interface --- src/plugins/newsfeed/public/plugin.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/plugins/newsfeed/public/plugin.tsx b/src/plugins/newsfeed/public/plugin.tsx index 782a51377311c..10b0fc51084ab 100644 --- a/src/plugins/newsfeed/public/plugin.tsx +++ b/src/plugins/newsfeed/public/plugin.tsx @@ -56,7 +56,8 @@ export class NewsfeedPublicPlugin pathTemplate: `/${endpoint}/v{VERSION}.json`, }, }); - return this.createNewsfeedApi(core, config, endpoint); + const { fetchResults$ } = this.createNewsfeedApi(core, config, endpoint); + return fetchResults$; }, }; } From 79ae8baa64a34da9e7b765230fab4ac9fbd5fd4b Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Thu, 27 May 2021 07:42:38 +0200 Subject: [PATCH 5/7] address review comments --- .../public/components/newsfeed_header_nav_button.tsx | 5 +---- src/plugins/newsfeed/public/lib/driver.test.ts | 4 ++-- src/plugins/newsfeed/public/lib/storage.ts | 8 ++++---- 3 files changed, 7 insertions(+), 10 deletions(-) diff --git a/src/plugins/newsfeed/public/components/newsfeed_header_nav_button.tsx b/src/plugins/newsfeed/public/components/newsfeed_header_nav_button.tsx index 8a8a70dee49a0..7060adcc2a4ec 100644 --- a/src/plugins/newsfeed/public/components/newsfeed_header_nav_button.tsx +++ b/src/plugins/newsfeed/public/components/newsfeed_header_nav_button.tsx @@ -7,12 +7,11 @@ */ import React, { useState, useEffect, useMemo, useCallback } from 'react'; -import * as Rx from 'rxjs'; import { EuiHeaderSectionItemButton, EuiIcon } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; +import type { NewsfeedApi } from '../lib/api'; import { NewsfeedFlyout } from './flyout_list'; import { FetchResult } from '../types'; -import { NewsfeedApi } from '../lib/api'; export interface INewsfeedContext { setFlyoutVisible: React.Dispatch>; @@ -21,8 +20,6 @@ export interface INewsfeedContext { export const NewsfeedContext = React.createContext({} as INewsfeedContext); -export type NewsfeedApiFetchResult = Rx.Observable; - export interface Props { newsfeedApi: NewsfeedApi; } diff --git a/src/plugins/newsfeed/public/lib/driver.test.ts b/src/plugins/newsfeed/public/lib/driver.test.ts index 2f3dcfcb51b97..12bf0a3895601 100644 --- a/src/plugins/newsfeed/public/lib/driver.test.ts +++ b/src/plugins/newsfeed/public/lib/driver.test.ts @@ -30,12 +30,12 @@ describe('NewsfeedApiDriver', () => { }); it('returns true if last fetch time precedes page load time', () => { - storage.getLastFetchTime.mockReturnValue(new Date(322642800000)); // 1980-03-23 + storage.getLastFetchTime.mockReturnValue(new Date(Date.now() - 456789)); expect(driver.shouldFetch()).toBe(true); }); it('returns false if last fetch time is recent enough', () => { - storage.getLastFetchTime.mockReturnValue(new Date(3005017200000)); // 2065-03-23 + storage.getLastFetchTime.mockReturnValue(new Date(Date.now() + 745678)); expect(driver.shouldFetch()).toBe(false); }); }); diff --git a/src/plugins/newsfeed/public/lib/storage.ts b/src/plugins/newsfeed/public/lib/storage.ts index 3259323def41c..f3df242ad9423 100644 --- a/src/plugins/newsfeed/public/lib/storage.ts +++ b/src/plugins/newsfeed/public/lib/storage.ts @@ -44,8 +44,7 @@ export class NewsfeedStorage { newReadStatus[hash] = currentReadStatus[hash] ?? false; }); - this.setReadStatus(newReadStatus); - return anyUnread(newReadStatus); + return this.setReadStatus(newReadStatus); } /** @@ -56,8 +55,7 @@ export class NewsfeedStorage { itemHashes.forEach((hash) => { updatedReadStatus[hash] = true; }); - this.setReadStatus(updatedReadStatus); - return anyUnread(updatedReadStatus); + return this.setReadStatus(updatedReadStatus); } isAnyUnread(): boolean { @@ -77,8 +75,10 @@ export class NewsfeedStorage { } private setReadStatus(status: Record) { + const hasUnread = anyUnread(status); this.unreadStatus$.next(anyUnread(status)); localStorage.setItem(this.readStatusStorageKey, JSON.stringify(status)); + return hasUnread; } } From 5c8c4990efb1c10c3ea34a190bb13d216aa57fa5 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Mon, 31 May 2021 09:07:59 +0200 Subject: [PATCH 6/7] name convertItem to localizeItem --- src/plugins/newsfeed/public/lib/convert_items.test.ts | 8 ++++---- src/plugins/newsfeed/public/lib/convert_items.ts | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/plugins/newsfeed/public/lib/convert_items.test.ts b/src/plugins/newsfeed/public/lib/convert_items.test.ts index c55a46bed5d38..8b599d841935c 100644 --- a/src/plugins/newsfeed/public/lib/convert_items.test.ts +++ b/src/plugins/newsfeed/public/lib/convert_items.test.ts @@ -8,7 +8,7 @@ import moment from 'moment'; import { omit } from 'lodash'; -import { validateIntegrity, validatePublishedDate, convertItem } from './convert_items'; +import { validateIntegrity, validatePublishedDate, localizeItem } from './convert_items'; import type { ApiItem, NewsfeedItem } from '../types'; const createApiItem = (parts: Partial = {}): ApiItem => ({ @@ -33,7 +33,7 @@ const createNewsfeedItem = (parts: Partial = {}): NewsfeedItem => ...parts, }); -describe('convertItem', () => { +describe('localizeItem', () => { const item = createApiItem({ languages: ['en', 'fr'], title: { @@ -62,7 +62,7 @@ describe('convertItem', () => { }); it('converts api items to newsfeed items using the specified language', () => { - expect(convertItem(item, 'fr')).toMatchObject({ + expect(localizeItem(item, 'fr')).toMatchObject({ title: 'fr title', description: 'fr desc', linkText: 'fr link text', @@ -73,7 +73,7 @@ describe('convertItem', () => { }); it('fallbacks to `en` is the language is not present', () => { - expect(convertItem(item, 'de')).toMatchObject({ + expect(localizeItem(item, 'de')).toMatchObject({ title: 'en title', description: 'en desc', linkText: 'en link text', diff --git a/src/plugins/newsfeed/public/lib/convert_items.ts b/src/plugins/newsfeed/public/lib/convert_items.ts index f80c9397aa0a0..38ea2cc895f3e 100644 --- a/src/plugins/newsfeed/public/lib/convert_items.ts +++ b/src/plugins/newsfeed/public/lib/convert_items.ts @@ -13,7 +13,7 @@ import { NEWSFEED_FALLBACK_LANGUAGE } from '../../common/constants'; export const convertItems = (items: ApiItem[], userLanguage: string): NewsfeedItem[] => { return items .filter(validatePublishedDate) - .map((item) => convertItem(item, userLanguage)) + .map((item) => localizeItem(item, userLanguage)) .filter(validateIntegrity); }; @@ -28,7 +28,7 @@ export const validatePublishedDate = (item: ApiItem): boolean => { return true; }; -export const convertItem = (rawItem: ApiItem, userLanguage: string): NewsfeedItem => { +export const localizeItem = (rawItem: ApiItem, userLanguage: string): NewsfeedItem => { const { expire_on: expireOnUtc, publish_on: publishOnUtc, From 3679e941ebb865409182fa78093fbe5f76b0b648 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Mon, 31 May 2021 10:11:16 +0200 Subject: [PATCH 7/7] use fetch instead of core.http and add tests --- src/plugins/newsfeed/public/lib/api.test.ts | 10 +- src/plugins/newsfeed/public/lib/api.ts | 4 +- .../newsfeed/public/lib/driver.test.mocks.ts | 12 +++ .../newsfeed/public/lib/driver.test.ts | 91 +++++++++++++++++++ src/plugins/newsfeed/public/lib/driver.ts | 21 +++-- src/plugins/newsfeed/public/plugin.tsx | 8 +- 6 files changed, 122 insertions(+), 24 deletions(-) create mode 100644 src/plugins/newsfeed/public/lib/driver.test.mocks.ts diff --git a/src/plugins/newsfeed/public/lib/api.test.ts b/src/plugins/newsfeed/public/lib/api.test.ts index 9727557587cd1..a4894573932e6 100644 --- a/src/plugins/newsfeed/public/lib/api.test.ts +++ b/src/plugins/newsfeed/public/lib/api.test.ts @@ -8,7 +8,6 @@ import { driverInstanceMock, storageInstanceMock } from './api.test.mocks'; import moment from 'moment'; -import { httpServiceMock } from '../../../../core/public/mocks'; import { getApi } from './api'; import { TestScheduler } from 'rxjs/testing'; import { FetchResult, NewsfeedPluginBrowserConfig } from '../types'; @@ -40,10 +39,7 @@ const createFetchResult = (parts: Partial): FetchResult => ({ }); describe('getApi', () => { - let http: ReturnType; - beforeEach(() => { - http = httpServiceMock.createSetupContract(); driverInstanceMock.shouldFetch.mockReturnValue(true); }); @@ -64,7 +60,7 @@ describe('getApi', () => { a: createFetchResult({ feedItems: ['item' as any] }), }) ); - const api = getApi(http, createConfig(1000), kibanaVersion, newsfeedId); + const api = getApi(createConfig(1000), kibanaVersion, newsfeedId); expectObservable(api.fetchResults$.pipe(take(1))).toBe('(a|)', { a: createFetchResult({ @@ -87,7 +83,7 @@ describe('getApi', () => { a: createFetchResult({ feedItems: ['item' as any] }), }) ); - const api = getApi(http, createConfig(2), kibanaVersion, newsfeedId); + const api = getApi(createConfig(2), kibanaVersion, newsfeedId); expectObservable(api.fetchResults$.pipe(take(2))).toBe('a-(b|)', { a: createFetchResult({ @@ -115,7 +111,7 @@ describe('getApi', () => { a: createFetchResult({}), }) ); - const api = getApi(http, createConfig(10), kibanaVersion, newsfeedId); + const api = getApi(createConfig(10), kibanaVersion, newsfeedId); expectObservable(api.fetchResults$.pipe(take(2))).toBe('a--(b|)', { a: createFetchResult({ diff --git a/src/plugins/newsfeed/public/lib/api.ts b/src/plugins/newsfeed/public/lib/api.ts index b9cab3e71353c..4fbbd8687b73f 100644 --- a/src/plugins/newsfeed/public/lib/api.ts +++ b/src/plugins/newsfeed/public/lib/api.ts @@ -9,7 +9,6 @@ import { combineLatest, Observable, timer, of } from 'rxjs'; import { map, catchError, filter, mergeMap, tap } from 'rxjs/operators'; import { i18n } from '@kbn/i18n'; -import { HttpSetup } from 'src/core/public'; import { FetchResult, NewsfeedPluginBrowserConfig } from '../types'; import { NewsfeedApiDriver } from './driver'; import { NewsfeedStorage } from './storage'; @@ -39,7 +38,6 @@ export interface NewsfeedApi { * Computes hasNew value from new item hashes saved in localStorage */ export function getApi( - http: HttpSetup, config: NewsfeedPluginBrowserConfig, kibanaVersion: string, newsfeedId: string @@ -53,7 +51,7 @@ export function getApi( const results$ = timer(0, mainInterval).pipe( filter(() => driver.shouldFetch()), mergeMap(() => - driver.fetchNewsfeedItems(http, config.service).pipe( + driver.fetchNewsfeedItems(config.service).pipe( catchError((err) => { window.console.error(err); return of({ diff --git a/src/plugins/newsfeed/public/lib/driver.test.mocks.ts b/src/plugins/newsfeed/public/lib/driver.test.mocks.ts new file mode 100644 index 0000000000000..2d7123ebc2d1f --- /dev/null +++ b/src/plugins/newsfeed/public/lib/driver.test.mocks.ts @@ -0,0 +1,12 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +export const convertItemsMock = jest.fn(); +jest.doMock('./convert_items', () => ({ + convertItems: convertItemsMock, +})); diff --git a/src/plugins/newsfeed/public/lib/driver.test.ts b/src/plugins/newsfeed/public/lib/driver.test.ts index 12bf0a3895601..38ec90cf20101 100644 --- a/src/plugins/newsfeed/public/lib/driver.test.ts +++ b/src/plugins/newsfeed/public/lib/driver.test.ts @@ -6,6 +6,10 @@ * Side Public License, v 1. */ +import { convertItemsMock } from './driver.test.mocks'; +// @ts-expect-error +import fetchMock from 'fetch-mock/es5/client'; +import { take } from 'rxjs/operators'; import { NewsfeedApiDriver } from './driver'; import { storageMock } from './storage.mock'; @@ -20,6 +24,16 @@ describe('NewsfeedApiDriver', () => { beforeEach(() => { storage = storageMock.create(); driver = new NewsfeedApiDriver(kibanaVersion, userLanguage, fetchInterval, storage); + convertItemsMock.mockReturnValue([]); + }); + + afterEach(() => { + fetchMock.reset(); + convertItemsMock.mockReset(); + }); + + afterAll(() => { + fetchMock.restore(); }); describe('shouldFetch', () => { @@ -39,4 +53,81 @@ describe('NewsfeedApiDriver', () => { expect(driver.shouldFetch()).toBe(false); }); }); + + describe('fetchNewsfeedItems', () => { + it('calls `window.fetch` with the correct parameters', async () => { + fetchMock.get('*', { items: [] }); + await driver + .fetchNewsfeedItems({ + urlRoot: 'http://newsfeed.com', + pathTemplate: '/{VERSION}/news', + }) + .pipe(take(1)) + .toPromise(); + + expect(fetchMock.lastUrl()).toEqual('http://newsfeed.com/8.0.0/news'); + expect(fetchMock.lastOptions()).toEqual({ + method: 'GET', + }); + }); + + it('calls `convertItems` with the correct parameters', async () => { + fetchMock.get('*', { items: ['foo', 'bar'] }); + + await driver + .fetchNewsfeedItems({ + urlRoot: 'http://newsfeed.com', + pathTemplate: '/{VERSION}/news', + }) + .pipe(take(1)) + .toPromise(); + + expect(convertItemsMock).toHaveBeenCalledTimes(1); + expect(convertItemsMock).toHaveBeenCalledWith(['foo', 'bar'], userLanguage); + }); + + it('calls `storage.setFetchedItems` with the correct parameters', async () => { + fetchMock.get('*', { items: [] }); + convertItemsMock.mockReturnValue([ + { id: '1', hash: 'hash1' }, + { id: '2', hash: 'hash2' }, + ]); + + await driver + .fetchNewsfeedItems({ + urlRoot: 'http://newsfeed.com', + pathTemplate: '/{VERSION}/news', + }) + .pipe(take(1)) + .toPromise(); + + expect(storage.setFetchedItems).toHaveBeenCalledTimes(1); + expect(storage.setFetchedItems).toHaveBeenCalledWith(['hash1', 'hash2']); + }); + + it('returns the expected values', async () => { + fetchMock.get('*', { items: [] }); + const feedItems = [ + { id: '1', hash: 'hash1' }, + { id: '2', hash: 'hash2' }, + ]; + convertItemsMock.mockReturnValue(feedItems); + storage.setFetchedItems.mockReturnValue(true); + + const result = await driver + .fetchNewsfeedItems({ + urlRoot: 'http://newsfeed.com', + pathTemplate: '/{VERSION}/news', + }) + .pipe(take(1)) + .toPromise(); + + expect(result).toEqual({ + error: null, + kibanaVersion, + hasNew: true, + feedItems, + }); + }); + }); }); diff --git a/src/plugins/newsfeed/public/lib/driver.ts b/src/plugins/newsfeed/public/lib/driver.ts index ae0052acebfe7..0efa981e8c89d 100644 --- a/src/plugins/newsfeed/public/lib/driver.ts +++ b/src/plugins/newsfeed/public/lib/driver.ts @@ -7,7 +7,6 @@ */ import moment from 'moment'; -import { HttpSetup } from 'kibana/public'; import * as Rx from 'rxjs'; import { NEWSFEED_DEFAULT_SERVICE_BASE_URL } from '../../common/constants'; import { ApiItem, FetchResult, NewsfeedPluginBrowserConfig } from '../types'; @@ -16,6 +15,10 @@ import type { NewsfeedStorage } from './storage'; type ApiConfig = NewsfeedPluginBrowserConfig['service']; +interface NewsfeedResponse { + items: ApiItem[]; +} + export class NewsfeedApiDriver { private readonly kibanaVersion: string; 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 { return duration.asMilliseconds() > this.fetchInterval; } - fetchNewsfeedItems(http: HttpSetup, config: ApiConfig): Rx.Observable { + fetchNewsfeedItems(config: ApiConfig): Rx.Observable { const urlPath = config.pathTemplate.replace('{VERSION}', this.kibanaVersion); const fullUrl = (config.urlRoot || NEWSFEED_DEFAULT_SERVICE_BASE_URL) + urlPath; + const request = new Request(fullUrl, { + method: 'GET', + }); return Rx.from( - http - .fetch(fullUrl, { - method: 'GET', - }) - .then(({ items }: { items: ApiItem[] }) => { - return this.convertResponse(items); - }) + window.fetch(request).then(async (response) => { + const { items } = (await response.json()) as NewsfeedResponse; + return this.convertResponse(items); + }) ); } diff --git a/src/plugins/newsfeed/public/plugin.tsx b/src/plugins/newsfeed/public/plugin.tsx index 10b0fc51084ab..fdda0a24b8bd5 100644 --- a/src/plugins/newsfeed/public/plugin.tsx +++ b/src/plugins/newsfeed/public/plugin.tsx @@ -42,7 +42,7 @@ export class NewsfeedPublicPlugin } public start(core: CoreStart) { - const api = this.createNewsfeedApi(core, this.config, NewsfeedApiEndpoint.KIBANA); + const api = this.createNewsfeedApi(this.config, NewsfeedApiEndpoint.KIBANA); core.chrome.navControls.registerRight({ order: 1000, mount: (target) => this.mount(api, target), @@ -56,7 +56,7 @@ export class NewsfeedPublicPlugin pathTemplate: `/${endpoint}/v{VERSION}.json`, }, }); - const { fetchResults$ } = this.createNewsfeedApi(core, config, endpoint); + const { fetchResults$ } = this.createNewsfeedApi(config, endpoint); return fetchResults$; }, }; @@ -67,12 +67,10 @@ export class NewsfeedPublicPlugin } private createNewsfeedApi( - core: CoreStart, config: NewsfeedPluginBrowserConfig, newsfeedId: NewsfeedApiEndpoint ): NewsfeedApi { - const { http } = core; - const api = getApi(http, config, this.kibanaVersion, newsfeedId); + const api = getApi(config, this.kibanaVersion, newsfeedId); return { markAsRead: api.markAsRead, fetchResults$: api.fetchResults$.pipe(