Merge branch 'master' into elastic#63480

.github/CODEOWNERS

@@ -163,6 +163,7 @@
 /packages/kbn-analytics/ @elastic/pulse
 /src/legacy/core_plugins/ui_metric/ @elastic/pulse
 /src/plugins/kibana_usage_collection/ @elastic/pulse
+/src/plugins/newsfeed/ @elastic/pulse
 /src/plugins/telemetry/ @elastic/pulse
 /src/plugins/telemetry_collection_manager/ @elastic/pulse
 /src/plugins/telemetry_management_section/ @elastic/pulse

src/core/CONVENTIONS.md

@@ -201,6 +201,36 @@ export class MyPlugin implements Plugin {
 }
 ```
 
+Prefer the pattern shown above, using `core.getStartServices()`, rather than store local references retrieved from `start`. 
+
+**Bad:**
+```ts
+export class MyPlugin implements Plugin {
+ // Anti pattern
+  private coreStart?: CoreStart;
+  private depsStart?: DepsStart;  
+
+  public setup(core) {
+    core.application.register({
+      id: 'my-app',
+      async mount(params) {
+        const { renderApp } = await import('./application/my_app');
+        // Anti pattern - use `core.getStartServices()` instead!
+        return renderApp(this.coreStart, this.depsStart, params);
+      }
+    });
+  }  
+
+  public start(core, deps) {
+    // Anti pattern
+    this.coreStart = core;
+    this.depsStart = deps;
+  }
+}
+```
+
+The main reason to prefer the provided async accessor, is that it doesn't requires the developer to understand and reason about when that function can be called. Having an API that fails sometimes isn't a good API design, and it makes accurately testing this difficult.
+
 #### Services
 
 Service structure should mirror the plugin lifecycle to make reasoning about how the service is executed more clear.

test/common/fixtures/plugins/newsfeed/index.ts → src/plugins/newsfeed/common/constants.ts

@@ -17,17 +17,10 @@
  * under the License.
  */
 
-import Hapi from 'hapi';
-import { initPlugin as initNewsfeed } from './newsfeed_simulation';
+export const NEWSFEED_FALLBACK_LANGUAGE = 'en';
+export const NEWSFEED_LAST_FETCH_STORAGE_KEY = 'newsfeed.lastfetchtime';
+export const NEWSFEED_HASH_SET_STORAGE_KEY = 'newsfeed.hashes';
 
-const NAME = 'newsfeed-FTS-external-service-simulators';
-
-// eslint-disable-next-line import/no-default-export
-export default function(kibana: any) {
-  return new kibana.Plugin({
-    name: NAME,
-    init: (server: Hapi.Server) => {
-      initNewsfeed(server, `/api/_${NAME}`);
-    },
-  });
-}
+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';

src/plugins/newsfeed/kibana.json

@@ -1,6 +1,6 @@
 {
   "id": "newsfeed",
   "version": "kibana",
-  "server": false,
+  "server": true,
   "ui": true
 }

src/plugins/newsfeed/public/components/flyout_list.tsx

@@ -29,11 +29,11 @@ import {
   EuiButtonEmpty,
   EuiText,
   EuiBadge,
+  EuiHeaderAlert,
 } from '@elastic/eui';
 import { FormattedMessage } from '@kbn/i18n/react';
-import { EuiHeaderAlert } from '../../../../legacy/core_plugins/newsfeed/public/np_ready/components/header_alert/header_alert';
 import { NewsfeedContext } from './newsfeed_header_nav_button';
-import { NewsfeedItem } from '../../types';
+import { NewsfeedItem } from '../types';
 import { NewsEmptyPrompt } from './empty_news';
 import { NewsLoadingPrompt } from './loading_news';
 

src/plugins/newsfeed/public/components/newsfeed_header_nav_button.tsx

@@ -21,7 +21,7 @@ import React, { useState, Fragment, useEffect } from 'react';
 import * as Rx from 'rxjs';
 import { EuiHeaderSectionItemButton, EuiIcon, EuiNotificationBadge } from '@elastic/eui';
 import { NewsfeedFlyout } from './flyout_list';
-import { FetchResult } from '../../types';
+import { FetchResult } from '../types';
 
 export interface INewsfeedContext {
   setFlyoutVisible: React.Dispatch<React.SetStateAction<boolean>>;

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

@@ -22,8 +22,11 @@ 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 '../../constants';
-import { ApiItem, NewsfeedItem, NewsfeedPluginInjectedConfig } from '../../types';
+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();
@@ -458,23 +461,20 @@ describe('getApi', () => {
     }
     return Promise.reject('wrong args!');
   };
-  let configMock: NewsfeedPluginInjectedConfig;
+  let configMock: NewsfeedPluginBrowserConfig;
 
   afterEach(() => {
     jest.resetAllMocks();
   });
 
   beforeEach(() => {
     configMock = {
-      newsfeed: {
-        service: {
-          urlRoot: 'http://fakenews.co',
-          pathTemplate: '/kibana-test/v{VERSION}.json',
-        },
-        defaultLanguage: 'en',
-        mainInterval: 86400000,
-        fetchInterval: 86400000,
+      service: {
+        urlRoot: 'http://fakenews.co',
+        pathTemplate: '/kibana-test/v{VERSION}.json',
       },
+      mainInterval: moment.duration(86400000),
+      fetchInterval: moment.duration(86400000),
     };
     httpMock = ({
       fetch: mockHttpGet,
@@ -483,7 +483,7 @@ describe('getApi', () => {
 
   it('creates a result', done => {
     mockHttpGet.mockImplementationOnce(() => Promise.resolve({ items: [] }));
-    getApi(httpMock, configMock.newsfeed, '6.8.2').subscribe(result => {
+    getApi(httpMock, configMock, '6.8.2').subscribe(result => {
       expect(result).toMatchInlineSnapshot(`
         Object {
           "error": null,
@@ -528,7 +528,7 @@ describe('getApi', () => {
 
     mockHttpGet.mockImplementationOnce(getHttpMockWithItems(mockApiItems));
 
-    getApi(httpMock, configMock.newsfeed, '6.8.2').subscribe(result => {
+    getApi(httpMock, configMock, '6.8.2').subscribe(result => {
       expect(result).toMatchInlineSnapshot(`
         Object {
           "error": null,
@@ -568,7 +568,7 @@ describe('getApi', () => {
       },
     ];
     mockHttpGet.mockImplementationOnce(getHttpMockWithItems(mockApiItems));
-    getApi(httpMock, configMock.newsfeed, '6.8.2').subscribe(result => {
+    getApi(httpMock, configMock, '6.8.2').subscribe(result => {
       expect(result).toMatchInlineSnapshot(`
         Object {
           "error": null,
@@ -595,7 +595,7 @@ describe('getApi', () => {
   it('forwards an error', done => {
     mockHttpGet.mockImplementationOnce((arg1, arg2) => Promise.reject('sorry, try again later!'));
 
-    getApi(httpMock, configMock.newsfeed, '6.8.2').subscribe(result => {
+    getApi(httpMock, configMock, '6.8.2').subscribe(result => {
       expect(result).toMatchInlineSnapshot(`
         Object {
           "error": "sorry, try again later!",
@@ -623,14 +623,14 @@ describe('getApi', () => {
     ];
 
     it("retries until fetch doesn't error", done => {
-      configMock.newsfeed.mainInterval = 10; // fast retry for testing
+      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.newsfeed, '6.8.2')
+      getApi(httpMock, configMock, '6.8.2')
         .pipe(take(4), toArray())
         .subscribe(result => {
           expect(result).toMatchInlineSnapshot(`
@@ -677,13 +677,13 @@ describe('getApi', () => {
     });
 
     it("doesn't retry if fetch succeeds", done => {
-      configMock.newsfeed.mainInterval = 10; // fast retry for testing
+      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.newsfeed, '6.8.2').pipe(
+      const get$ = getApi(httpMock, configMock, '6.8.2').pipe(
         tap(() => {
           timesFetched++;
         })