[NP] Get rid of usage redirectWhenMissing service (#59777)

* Move redirect_when_missing to kibana utils * Replace redirectWhenMissing in dashboard * Replace redirectWhenMissing in discover * Remove redirect in monitoring * Remove extra import * Move invalid vistype check into editor.js * Mock the history folder * Fix redirect when missing index or saved object * Move history to discover services * Use redirect to listing page Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
elastic · Mar 17, 2020 · eddbdc8 · eddbdc8
1 parent 55003b6
commit eddbdc8
Show file tree

Hide file tree

Showing 20 changed files with 179 additions and 56 deletions.
diff --git a/src/legacy/core_plugins/kibana/public/dashboard/legacy_imports.ts b/src/legacy/core_plugins/kibana/public/dashboard/legacy_imports.ts
@@ -28,7 +28,7 @@ export { npSetup, npStart } from 'ui/new_platform';
 
 export { KbnUrl } from 'ui/url/kbn_url';
 // @ts-ignore
-export { KbnUrlProvider, RedirectWhenMissingProvider } from 'ui/url/index';
+export { KbnUrlProvider } from 'ui/url/index';
 export { IInjector } from 'ui/chrome';
 export { absoluteToParsedUrl } from 'ui/url/absolute_to_parsed_url';
 export {

diff --git a/src/legacy/core_plugins/kibana/public/dashboard/np_ready/application.ts b/src/legacy/core_plugins/kibana/public/dashboard/np_ready/application.ts
@@ -35,7 +35,6 @@ import {
   KbnUrlProvider,
   PrivateProvider,
   PromiseServiceCreator,
-  RedirectWhenMissingProvider,
 } from '../legacy_imports';
 // @ts-ignore
 import { initDashboardApp } from './legacy_app';
@@ -146,8 +145,7 @@ function createLocalIconModule() {
 function createLocalKbnUrlModule() {
   angular
     .module('app/dashboard/KbnUrl', ['app/dashboard/Private', 'ngRoute'])
-    .service('kbnUrl', (Private: IPrivate) => Private(KbnUrlProvider))
-    .service('redirectWhenMissing', (Private: IPrivate) => Private(RedirectWhenMissingProvider));
+    .service('kbnUrl', (Private: IPrivate) => Private(KbnUrlProvider));
 }
 
 function createLocalConfigModule(core: AppMountContext['core']) {

diff --git a/src/legacy/core_plugins/kibana/public/dashboard/np_ready/legacy_app.js b/src/legacy/core_plugins/kibana/public/dashboard/np_ready/legacy_app.js
@@ -28,6 +28,7 @@ import { initDashboardAppDirective } from './dashboard_app';
 import { createDashboardEditUrl, DashboardConstants } from './dashboard_constants';
 import {
   createKbnUrlStateStorage,
+  redirectWhenMissing,
   InvalidJSONProperty,
   SavedObjectNotFound,
 } from '../../../../../../plugins/kibana_utils/public';
@@ -136,7 +137,7 @@ export function initDashboardApp(app, deps) {
           });
         },
         resolve: {
-          dash: function($rootScope, $route, redirectWhenMissing, kbnUrl, history) {
+          dash: function($rootScope, $route, kbnUrl, history) {
             return ensureDefaultIndexPattern(deps.core, deps.data, $rootScope, kbnUrl).then(() => {
               const savedObjectsClient = deps.savedObjectsClient;
               const title = $route.current.params.title;
@@ -171,14 +172,18 @@ export function initDashboardApp(app, deps) {
         controller: createNewDashboardCtrl,
         requireUICapability: 'dashboard.createNew',
         resolve: {
-          dash: function(redirectWhenMissing, $rootScope, kbnUrl) {
+          dash: function($rootScope, kbnUrl, history) {
             return ensureDefaultIndexPattern(deps.core, deps.data, $rootScope, kbnUrl)
               .then(() => {
                 return deps.savedDashboards.get();
               })
               .catch(
                 redirectWhenMissing({
-                  dashboard: DashboardConstants.LANDING_PAGE_PATH,
+                  history,
+                  mapping: {
+                    dashboard: DashboardConstants.LANDING_PAGE_PATH,
+                  },
+                  toastNotifications: deps.core.notifications.toasts,
                 })
               );
           },
@@ -189,7 +194,7 @@ export function initDashboardApp(app, deps) {
         template: dashboardTemplate,
         controller: createNewDashboardCtrl,
         resolve: {
-          dash: function($rootScope, $route, redirectWhenMissing, kbnUrl, history) {
+          dash: function($rootScope, $route, kbnUrl, history) {
             const id = $route.current.params.id;
 
             return ensureDefaultIndexPattern(deps.core, deps.data, $rootScope, kbnUrl)
@@ -207,7 +212,7 @@ export function initDashboardApp(app, deps) {
               .catch(error => {
                 // A corrupt dashboard was detected (e.g. with invalid JSON properties)
                 if (error instanceof InvalidJSONProperty) {
-                  deps.toastNotifications.addDanger(error.message);
+                  deps.core.notifications.toasts.addDanger(error.message);
                   kbnUrl.redirect(DashboardConstants.LANDING_PAGE_PATH);
                   return;
                 }
@@ -221,7 +226,7 @@ export function initDashboardApp(app, deps) {
                     pathname: DashboardConstants.CREATE_NEW_DASHBOARD_URL,
                   });
 
-                  deps.toastNotifications.addWarning(
+                  deps.core.notifications.toasts.addWarning(
                     i18n.translate('kbn.dashboard.urlWasRemovedInSixZeroWarningMessage', {
                       defaultMessage:
                         'The url "dashboard/create" was removed in 6.0. Please update your bookmarks.',
@@ -234,7 +239,11 @@ export function initDashboardApp(app, deps) {
               })
               .catch(
                 redirectWhenMissing({
-                  dashboard: DashboardConstants.LANDING_PAGE_PATH,
+                  history,
+                  mapping: {
+                    dashboard: DashboardConstants.LANDING_PAGE_PATH,
+                  },
+                  toastNotifications: deps.core.notifications.toasts,
                 })
               );
           },

diff --git a/src/legacy/core_plugins/kibana/public/discover/build_services.ts b/src/legacy/core_plugins/kibana/public/discover/build_services.ts
@@ -16,6 +16,8 @@
  * specific language governing permissions and limitations
  * under the License.
  */
+import { createHashHistory, History } from 'history';
+
 import {
   Capabilities,
   ChromeStart,
@@ -46,6 +48,7 @@ export interface DiscoverServices {
   data: DataPublicPluginStart;
   docLinks: DocLinksStart;
   docViewsRegistry: DocViewsRegistry;
+  history: History;
   theme: ChartsPluginStart['theme'];
   filterManager: FilterManager;
   indexPatterns: IndexPatternsContract;
@@ -79,6 +82,7 @@ export async function buildServices(
     data: plugins.data,
     docLinks: core.docLinks,
     docViewsRegistry,
+    history: createHashHistory(),
     theme: plugins.charts.theme,
     filterManager: plugins.data.query.filterManager,
     getSavedSearchById: async (id: string) => savedObjectService.get(id),

diff --git a/src/legacy/core_plugins/kibana/public/discover/get_inner_angular.ts b/src/legacy/core_plugins/kibana/public/discover/get_inner_angular.ts
@@ -27,7 +27,7 @@ import { CoreStart, LegacyCoreStart, IUiSettingsClient } from 'kibana/public';
 // @ts-ignore
 import { StateManagementConfigProvider } from 'ui/state_management/config_provider';
 // @ts-ignore
-import { KbnUrlProvider, RedirectWhenMissingProvider } from 'ui/url';
+import { KbnUrlProvider } from 'ui/url';
 import { DataPublicPluginStart } from '../../../../../plugins/data/public';
 import { Storage } from '../../../../../plugins/kibana_utils/public';
 import { NavigationPublicPluginStart as NavigationStart } from '../../../../../plugins/navigation/public';
@@ -173,8 +173,7 @@ export function initializeInnerAngularModule(
 function createLocalKbnUrlModule() {
   angular
     .module('discoverKbnUrl', ['discoverPrivate', 'ngRoute'])
-    .service('kbnUrl', (Private: IPrivate) => Private(KbnUrlProvider))
-    .service('redirectWhenMissing', (Private: IPrivate) => Private(RedirectWhenMissingProvider));
+    .service('kbnUrl', (Private: IPrivate) => Private(KbnUrlProvider));
 }
 
 function createLocalConfigModule(uiSettings: IUiSettingsClient) {

diff --git a/src/legacy/core_plugins/kibana/public/discover/kibana_services.ts b/src/legacy/core_plugins/kibana/public/discover/kibana_services.ts
@@ -59,7 +59,7 @@ export { intervalOptions } from 'ui/agg_types';
 export { subscribeWithScope } from '../../../../../plugins/kibana_legacy/public';
 // @ts-ignore
 export { timezoneProvider } from 'ui/vis/lib/timezone';
-export { unhashUrl } from '../../../../../plugins/kibana_utils/public';
+export { unhashUrl, redirectWhenMissing } from '../../../../../plugins/kibana_utils/public';
 export {
   ensureDefaultIndexPattern,
   formatMsg,

diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js
@@ -50,13 +50,15 @@ import {
   tabifyAggResponse,
   getAngularModule,
   ensureDefaultIndexPattern,
+  redirectWhenMissing,
 } from '../../kibana_services';
 
 const {
   core,
   chrome,
   data,
   docTitle,
+  history,
   indexPatterns,
   filterManager,
   share,
@@ -113,10 +115,10 @@ app.config($routeProvider => {
     template: indexTemplate,
     reloadOnSearch: false,
     resolve: {
-      savedObjects: function(redirectWhenMissing, $route, kbnUrl, Promise, $rootScope) {
+      savedObjects: function($route, kbnUrl, Promise, $rootScope) {
         const savedSearchId = $route.current.params.id;
         return ensureDefaultIndexPattern(core, data, $rootScope, kbnUrl).then(() => {
-          const { appStateContainer } = getState({});
+          const { appStateContainer } = getState({ history });
           const { index } = appStateContainer.getState();
           return Promise.props({
             ip: indexPatterns.getCache().then(indexPatternList => {
@@ -151,9 +153,13 @@ app.config($routeProvider => {
               })
               .catch(
                 redirectWhenMissing({
-                  search: '/discover',
-                  'index-pattern':
-                    '/management/kibana/objects/savedSearches/' + $route.current.params.id,
+                  history,
+                  mapping: {
+                    search: '/discover',
+                    'index-pattern':
+                      '/management/kibana/objects/savedSearches/' + $route.current.params.id,
+                  },
+                  toastNotifications,
                 })
               ),
           });
@@ -207,6 +213,7 @@ function discoverController(
   } = getState({
     defaultAppState: getStateDefaults(),
     storeInSessionStorage: config.get('state:storeInSessionStorage'),
+    history,
   });
   if (appStateContainer.getState().index !== $scope.indexPattern.id) {
     //used index pattern is different than the given by url/state which is invalid

diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.test.ts b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.test.ts
@@ -30,7 +30,7 @@ describe('Test discover state', () => {
     history.push('/');
     state = getState({
       defaultAppState: { index: 'test' },
-      hashHistory: history,
+      history,
     });
     await state.replaceUrlAppState({});
     await state.startSync();

diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts
@@ -17,7 +17,7 @@
  * under the License.
  */
 import { isEqual } from 'lodash';
-import { createHashHistory, History } from 'history';
+import { History } from 'history';
 import {
   createStateContainer,
   createKbnUrlStateStorage,
@@ -65,9 +65,9 @@ interface GetStateParams {
    */
   storeInSessionStorage?: boolean;
   /**
-   * Browser history used for testing
+   * Browser history
    */
-  hashHistory?: History;
+  history: History;
 }
 
 export interface GetStateReturn {
@@ -121,11 +121,11 @@ const APP_STATE_URL_KEY = '_a';
 export function getState({
   defaultAppState = {},
   storeInSessionStorage = false,
-  hashHistory,
+  history,
 }: GetStateParams): GetStateReturn {
   const stateStorage = createKbnUrlStateStorage({
     useHash: storeInSessionStorage,
-    history: hashHistory ? hashHistory : createHashHistory(),
+    history,
   });
 
   const appStateFromUrl = stateStorage.get(APP_STATE_URL_KEY) as AppState;

diff --git a/src/legacy/core_plugins/kibana/public/visualize/legacy_imports.ts b/src/legacy/core_plugins/kibana/public/visualize/legacy_imports.ts
@@ -25,7 +25,7 @@
  */
 
 // @ts-ignore
-export { KbnUrlProvider, RedirectWhenMissingProvider } from 'ui/url';
+export { KbnUrlProvider } from 'ui/url';
 export { absoluteToParsedUrl } from 'ui/url/absolute_to_parsed_url';
 export { KibanaParsedUrl } from 'ui/url/kibana_parsed_url';
 export { wrapInI18nContext } from 'ui/i18n';

diff --git a/src/legacy/core_plugins/kibana/public/visualize/np_ready/application.ts b/src/legacy/core_plugins/kibana/public/visualize/np_ready/application.ts
@@ -24,7 +24,6 @@ import { AppMountContext } from 'kibana/public';
 import {
   configureAppAngularModule,
   KbnUrlProvider,
-  RedirectWhenMissingProvider,
   IPrivate,
   PrivateProvider,
   PromiseServiceCreator,
@@ -102,8 +101,7 @@ function createLocalAngularModule(core: AppMountContext['core'], navigation: Nav
 function createLocalKbnUrlModule() {
   angular
     .module('app/visualize/KbnUrl', ['app/visualize/Private', 'ngRoute'])
-    .service('kbnUrl', (Private: IPrivate) => Private(KbnUrlProvider))
-    .service('redirectWhenMissing', (Private: IPrivate) => Private(RedirectWhenMissingProvider));
+    .service('kbnUrl', (Private: IPrivate) => Private(KbnUrlProvider));
 }
 
 function createLocalPromiseModule() {

diff --git a/src/legacy/core_plugins/kibana/public/visualize/np_ready/editor/editor.js b/src/legacy/core_plugins/kibana/public/visualize/np_ready/editor/editor.js
@@ -31,6 +31,7 @@ import { getEditBreadcrumbs } from '../breadcrumbs';
 
 import { addHelpMenuToAppChrome } from '../help_menu/help_menu_util';
 import { unhashUrl } from '../../../../../../../plugins/kibana_utils/public';
+import { MarkdownSimple, toMountPoint } from '../../../../../../../plugins/kibana_react/public';
 import { addFatalError, kbnBaseUrl } from '../../../../../../../plugins/kibana_legacy/public';
 import {
   SavedObjectSaveModal,
@@ -75,7 +76,6 @@ function VisualizeAppController(
   $injector,
   $timeout,
   kbnUrl,
-  redirectWhenMissing,
   kbnUrlStateStorage,
   history
 ) {
@@ -313,16 +313,33 @@ function VisualizeAppController(
     }
   );
 
+  const stopAllSyncing = () => {
+    stopStateSync();
+    stopSyncingQueryServiceStateWithUrl();
+    stopSyncingAppFilters();
+  };
+
   // The savedVis is pulled from elasticsearch, but the appState is pulled from the url, with the
   // defaults applied. If the url was from a previous session which included modifications to the
   // appState then they won't be equal.
   if (!_.isEqual(stateContainer.getState().vis, stateDefaults.vis)) {
     try {
       vis.setState(stateContainer.getState().vis);
-    } catch {
-      redirectWhenMissing({
-        'index-pattern-field': '/visualize',
+    } catch (error) {
+      // stop syncing url updtes with the state to prevent extra syncing
+      stopAllSyncing();
+
+      toastNotifications.addWarning({
+        title: i18n.translate('kbn.visualize.visualizationTypeInvalidNotificationMessage', {
+          defaultMessage: 'Invalid visualization type',
+        }),
+        text: toMountPoint(<MarkdownSimple>{error.message}</MarkdownSimple>),
       });
+
+      history.replace(`${VisualizeConstants.LANDING_PAGE_PATH}?notFound=visualization`);
+
+      // prevent further controller execution
+      return;
     }
   }
 
@@ -529,9 +546,8 @@ function VisualizeAppController(
 
       unsubscribePersisted();
       unsubscribeStateUpdates();
-      stopStateSync();
-      stopSyncingQueryServiceStateWithUrl();
-      stopSyncingAppFilters();
+
+      stopAllSyncing();
     });
 
     $timeout(() => {

diff --git a/src/legacy/core_plugins/kibana/public/visualize/np_ready/editor/visualization.js b/src/legacy/core_plugins/kibana/public/visualize/np_ready/editor/visualization.js
@@ -59,7 +59,9 @@ export function initVisualizationDirective(app, deps) {
         });
 
         $scope.$on('$destroy', () => {
-          $scope._handler.destroy();
+          if ($scope._handler) {
+            $scope._handler.destroy();
+          }
         });
       },
     };