Merge branch '7.x' into backport/7.x/pr-72331

.ci/end2end.groovy

@@ -37,22 +37,31 @@ pipeline {
         deleteDir()
         gitCheckout(basedir: "${BASE_DIR}", githubNotifyFirstTimeContributor: false,
                     shallow: false, reference: "/var/lib/jenkins/.git-references/kibana.git")
+
+        // Filter when to run based on the below reasons:
+        //  - On a PRs when:
+        //    - There are changes related to the APM UI project
+        //      - only when the owners of those changes are members of the given GitHub teams
+        //  - On merges to branches when:
+        //    - There are changes related to the APM UI project
+        //  - FORCE parameter is set to true.
         script {
+          def apm_updated = false
           dir("${BASE_DIR}"){
-            def regexps =[ "^x-pack/plugins/apm/.*" ]
-            env.APM_UPDATED = isGitRegionMatch(patterns: regexps)
+            apm_updated = isGitRegionMatch(patterns: [ "^x-pack/plugins/apm/.*" ])
+          }
+          if (isPR()) {
+            def isMember = isMemberOf(user: env.CHANGE_AUTHOR, team: ['apm-ui', 'uptime'])
+            setEnvVar('RUN_APM_E2E', params.FORCE || (apm_updated && isMember))
+          } else {
+            setEnvVar('RUN_APM_E2E', params.FORCE || apm_updated)
           }
         }
       }
     }
     stage('Prepare Kibana') {
       options { skipDefaultCheckout() }
-      when {
-        anyOf {
-          expression { return params.FORCE }
-          expression { return env.APM_UPDATED != "false" }
-        }
-      }
+      when { expression { return env.RUN_APM_E2E != "false" } }
       environment {
         JENKINS_NODE_COOKIE = 'dontKillMe'
       }
@@ -70,12 +79,7 @@ pipeline {
     }
     stage('Smoke Tests'){
       options { skipDefaultCheckout() }
-      when {
-        anyOf {
-          expression { return params.FORCE }
-          expression { return env.APM_UPDATED != "false" }
-        }
-      }
+      when { expression { return env.RUN_APM_E2E != "false" } }
       steps{
         notifyTestStatus('Running smoke tests', 'PENDING')
         dir("${BASE_DIR}"){

docs/developer/best-practices/index.asciidoc

@@ -122,6 +122,14 @@ In addition, if users are relying on state stored in your app’s URL as
 part of your public contract, keep in mind that you may also need to
 provide backwards compatibility for bookmarked URLs.
 
+[discrete]
+=== Routing, Navigation and URL
+
+The {kib} platform provides a set of tools to help developers build consistent experience around routing and browser navigation.
+Some of that tooling is inside `core`, some is available as part of various plugins.
+
+<<kibana-navigation, Follow this guide>> to get an idea of available tools and common approaches for handling routing and browser navigation.
+
 [discrete]
 === Testing & stability
 
@@ -131,6 +139,8 @@ Review:
 * <<stability>>
 * <<security-best-practices>>
 
+include::navigation.asciidoc[leveloffset=+1]
+
 include::stability.asciidoc[leveloffset=+1]
 
 include::security.asciidoc[leveloffset=+1]

docs/developer/best-practices/navigation.asciidoc

@@ -0,0 +1,226 @@
+[[kibana-navigation]]
+== Routing, Navigation and URL
+
+The {kib} platform provides a set of tools to help developers build consistent experience around routing and browser navigation.
+Some of that tooling is inside `core`, some is available as part of various plugins.
+
+The purpose of this guide is to give a high-level overview of available tools and to explain common approaches for handling routing and browser navigation.
+
+This guide covers following topics:
+
+* <<deep-linking>>
+* <<navigating-between-kibana-apps>>
+* <<routing>>
+* <<history-and-location>>
+* <<state-sync>>
+* <<preserve-state>>
+
+[[deep-linking]]
+=== Deep-linking into {kib} apps
+
+Assuming you want to link from your app to *Discover*. When building such URL there are two things to consider:
+
+1. Prepending a proper `basePath`.
+2. Specifying *Discover* state. 
+
+==== Prepending a proper `basePath`
+
+To prepend {kib}'s `basePath` use {kib-repo}tree/{branch}/docs/development/core/public/kibana-plugin-core-public.ibasepath.prepend.md[core.http.basePath.prepend] helper: 
+
+[source,typescript jsx]
+----
+const discoverUrl = core.http.basePath.prepend(`/discover`);
+
+console.log(discoverUrl); // http://localhost:5601/bpr/s/space/app/discover
+----
+
+==== Specifying state
+
+**Consider a {kib} app URL a part of app's plugin contract:**
+
+. Avoid hardcoding other app's URL in your app's code.
+. Avoid generating other app's state and serializing it into URL query params.
+
+[source,typescript jsx]
+----
+// Avoid relying on other app's state structure in your app's code:
+const discoverUrlWithSomeState = core.http.basePath.prepend(`/discover#/?_g=(filters:!(),refreshInterval:(pause:!t,value:0),time:(from:'2020-09-10T11:39:50.203Z',to:'2020-09-10T11:40:20.249Z'))&_a=(columns:!(_source),filters:!(),index:'90943e30-9a47-11e8-b64d-95841ca0b247',interval:auto,query:(language:kuery,query:''),sort:!())`);
+----
+
+Instead, each app should expose {kib-repo}tree/{branch}/src/plugins/share/public/url_generators/README.md[a URL generator].
+Other apps should use those URL generators for creating URLs. 
+
+[source,typescript jsx]
+----
+// Properly generated URL to *Discover* app. Generator code is owned by *Discover* app and available on *Discover*'s plugin contract.
+const discoverUrl = discoverUrlGenerator.createUrl({filters, timeRange});
+----
+
+To get a better idea, take a look at *Discover* URL generator {kib-repo}tree/{branch}/src/plugins/discover/public/url_generator.ts[implementation].
+It allows specifying various **Discover** app state pieces like: index pattern, filters, query, time range and more.
+
+There are two ways to access other's app URL generator in your code:
+
+1. From a plugin contract of a destination app *(preferred)*. 
+2. Using URL generator service instance on `share` plugin contract (in case an explicit plugin dependency is not possible).
+
+In case you want other apps to link to your app, then you should create a URL generator and expose it on your plugin's contract.
+
+
+[[navigating-between-kibana-apps]]
+=== Navigating between {kib} apps
+
+{kib} is a single page application and there is a set of simple rules developers should follow
+to make sure there is no page reload when navigating from one place in {kib} to another. 
+
+For example, navigation using native browser APIs would cause a full page reload.  
+
+[source,js]
+----
+const urlToADashboard = core.http.basePath.prepend(`/dashboard/my-dashboard`);
+
+// this would cause a full page reload: 
+window.location.href = urlToADashboard; 
+----
+
+To navigate between different {kib} apps without a page reload there are APIs in `core`:
+
+* {kib-repo}tree/{branch}/docs/development/core/public/kibana-plugin-core-public.applicationstart.navigatetoapp.md[core.application.navigateToApp]
+* {kib-repo}tree/{branch}/docs/development/core/public/kibana-plugin-core-public.applicationstart.navigatetourl.md[core.application.navigateToUrl]
+
+*Rendering a link to a different {kib} app on its own would also cause a full page reload:*
+
+[source,typescript jsx]
+----
+const myLink = () => 
+  <a href={urlToADashboard}>Go to Dashboard</a>; 
+----
+
+A workaround could be to handle a click, prevent browser navigation and use `core.application.navigateToApp` API:
+
+[source,typescript jsx]
+----
+const MySPALink = () => 
+  <a 
+    href={urlToADashboard} 
+    onClick={(e) => {
+      e.preventDefault();
+      core.application.navigateToApp('dashboard', { path: '/my-dashboard' }); 
+    }}
+  > 
+    Go to Dashboard 
+  </a>;
+----
+
+As it would be too much boilerplate to do this for each {kib} link in your app, there is a handy wrapper that helps with it: 
+{kib-repo}tree/{branch}/src/plugins/kibana_react/public/app_links/redirect_app_link.tsx#L49[RedirectAppLinks].
+
+[source,typescript jsx]
+----
+const MyApp = () => 
+  <RedirectAppLinks application={core.application}>
+    {/*...*/}
+    {/* navigations using this link will happen in SPA friendly way */}
+      <a href={urlToADashboard}>Go to Dashboard</a>
+    {/*...*/}
+  </RedirectAppLinks>
+----
+
+[[routing]]
+=== Setting up internal app routing
+
+It is very common for {kib} apps to use React and React Router.
+Common rules to follow in this scenario:
+
+* Set up `BrowserRouter` and not `HashRouter`.
+* *Initialize your router with `history` instance provided by the `core`.*
+
+This is required to make sure `core` is aware of navigations triggered inside your app, so it could act accordingly when needed.
+
+* `Core`'s {kib-repo}tree/{branch}/docs/development/core/public/kibana-plugin-core-public.scopedhistory.md[ScopedHistory] instance.
+* {kib-repo}tree/{branch}/docs/development/core/public/kibana-plugin-core-public.appmountparameters.history.md[Example usage]
+* {kib-repo}tree/{branch}/test/plugin_functional/plugins/core_plugin_a/public/application.tsx#L120[Example plugin]
+
+Relative links will be resolved relative to your app's route (e.g.: `http://localhost5601/app/{your-app-id}`)
+and setting up internal links in your app in SPA friendly way would look something like:
+
+[source,typescript jsx]
+----
+import {Link} from 'react-router-dom';
+
+const MyInternalLink = () => <Link to="/my-other-page"></Link>
+----
+
+[[history-and-location]]
+=== Using history and browser location
+
+Try to avoid using `window.location` and `window.history` directly. +  
+Instead, consider using {kib-repo}tree/{branch}/docs/development/core/public/kibana-plugin-core-public.scopedhistory.md[ScopedHistory]
+instance provided by `core`.
+
+* This way `core` will know about location changes triggered within your app, and it would act accordingly.
+* Some plugins are listening to location changes. Triggering location change manually could lead to unpredictable and hard-to-catch bugs.
+
+Common use-case for using 
+`core`'s {kib-repo}tree/{branch}/docs/development/core/public/kibana-plugin-core-public.scopedhistory.md[ScopedHistory] directly: 
+
+* Reading/writing query params or hash.
+* Imperatively triggering internal navigations within your app.
+* Listening to browser location changes.
+
+
+[[state-sync]]
+=== Syncing state with URL 
+
+Historically {kib} apps store _a lot_ of application state in the URL.
+The most common pattern that {kib} apps follow today is storing state in `_a` and `_g` query params in https://github.com/w33ble/rison-node#readme[rison] format.
+[[query-params]]
+Those query params follow the convention: 
+
+* `_g` (*global*) - global UI state that should be shared and synced across multiple apps. common example from Analyze group apps: time range, refresh interval, *pinned* filters.
+* `_a` (*application*) - UI state scoped to current app.
+
+NOTE: After migrating to KP platform we got navigations without page reloads. Since then there is no real need to follow `_g` and `_a` separation anymore. It's up you to decide if you want to follow this pattern or if you prefer a single query param or something else. The need for this separation earlier is explained in <<preserve-state>>. 
+
+There are utils to help you to implement such kind of state syncing.
+
+**When you should consider using state syncing utils:**
+
+* You want to sync your application state with URL in similar manner Analyze group applications do.
+* You want to follow platform's <<history-and-location, working with browser history and location best practices>> out of the box. 
+* You want to support `state:storeInSessionStore` escape hatch for URL overflowing out of the box.
+* You should also consider using them if you'd like to serialize state to different (not `rison`) format. Utils are composable, and you can implement your own `storage`.
+* In case you want to sync part of your state with URL, but other part of it with browser storage. 
+
+**When you shouldn't use state syncing utils:**
+
+* Adding a query param flag or simple key/value to the URL. 
+
+Follow {kib-repo}tree/{branch}/src/plugins/kibana_utils/docs/state_sync#state-syncing-utilities[these] docs to learn more.
+
+
+[[preserve-state]]
+=== Preserving state between navigations
+
+Consider the scenario: 
+
+1. You are in *Dashboard* app looking at a dashboard with some filters applied;
+2. Navigate to *Discover* using in-app navigation;
+3. Change the time filter'
+4. Navigate to *Dashboard* using in-app navigation.
+
+You'd notice that you were navigated to *Dashboard* app with the *same state* that you left it with,
+except that the time filter has changed to the one you applied on *Discover* app.
+
+Historically {kib} Analyze groups apps achieve that behavior relying on state in the URL. 
+If you'd have a closer look on a link in the navigation,
+you'd notice that state is stored inside that link, and it also gets updated whenever relevant state changes happen:
+
+[role="screenshot"]
+image:images/state_inside_the_link.png[State is stored inside the navigation link]
+
+This is where <<query-params, separation>> into `_a` and `_g` query params comes into play. What is considered a *global* state gets constantly updated in those navigation links. In the example above it was a time filter.
+This is backed by {kib-repo}tree/{branch}/src/plugins/kibana_utils/public/state_management/url/kbn_url_tracker.ts#L57[KbnUrlTracker] util. You can use it to achieve similar behavior.
+
+NOTE: After migrating to KP navigation works without page reloads and all plugins are loaded simultaneously.
+Hence, likely there are simpler ways to preserve state of your application, unless you want to do it through URL.

src/plugins/dashboard/public/attribute_service/attribute_service.tsx

@@ -57,7 +57,7 @@ export interface AttributeServiceOptions<A extends { title: string }> {
     type: string,
     attributes: A,
     savedObjectId?: string
-  ) => Promise<{ id: string }>;
+  ) => Promise<{ id?: string } | { error: Error }>;
   customUnwrapMethod?: (savedObject: SimpleSavedObject<A>) => A;
 }
 
@@ -124,7 +124,10 @@ export class AttributeService<
           newAttributes,
           savedObjectId
         );
-        return { ...originalInput, savedObjectId: savedItem.id } as RefType;
+        if ('id' in savedItem) {
+          return { ...originalInput, savedObjectId: savedItem.id } as RefType;
+        }
+        return { ...originalInput } as RefType;
       }
 
       if (savedObjectId) {
@@ -208,7 +211,6 @@ export class AttributeService<
           return { error };
         }
       };
-
       if (saveOptions && (saveOptions as { showSaveModal: boolean }).showSaveModal) {
         this.showSaveModal(
           <SavedObjectSaveModal

src/plugins/dashboard/public/mocks.tsx

@@ -0,0 +1,35 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { DashboardStart } from './plugin';
+
+export type Start = jest.Mocked<DashboardStart>;
+
+const createStartContract = (): DashboardStart => {
+  // @ts-ignore
+  const startContract: DashboardStart = {
+    getAttributeService: jest.fn(),
+  };
+
+  return startContract;
+};
+
+export const dashboardPluginMock = {
+  createStartContract,
+};

src/plugins/embeddable/public/lib/embeddables/default_embeddable_factory_provider.ts

@@ -37,11 +37,11 @@ export const defaultEmbeddableFactoryProvider = <
     getExplicitInput: def.getExplicitInput
       ? def.getExplicitInput.bind(def)
       : () => Promise.resolve({}),
-    createFromSavedObject:
-      def.createFromSavedObject ??
-      ((savedObjectId: string, input: Partial<I>, parent?: IContainer) => {
-        throw new Error(`Creation from saved object not supported by type ${def.type}`);
-      }),
+    createFromSavedObject: def.createFromSavedObject
+      ? def.createFromSavedObject.bind(def)
+      : (savedObjectId: string, input: Partial<I>, parent?: IContainer) => {
+          throw new Error(`Creation from saved object not supported by type ${def.type}`);
+        },
     create: def.create.bind(def),
     type: def.type,
     isEditable: def.isEditable.bind(def),

src/plugins/expressions/common/expression_types/specs/boolean.ts

@@ -41,7 +41,6 @@ export const boolean: ExpressionTypeDefinition<'boolean', boolean> = {
     },
     datatable: (value): Datatable => ({
       type: 'datatable',
-      meta: {},
       columns: [{ id: 'value', name: 'value', meta: { type: name } }],
       rows: [{ value }],
     }),