Skip to content
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
a1c86ec
Initial version
kertal Dec 13, 2019
70d7cf8
Further migrations
kertal Dec 16, 2019
9b337a8
merge upstream/master
kertal Dec 16, 2019
9374470
Remove ES2015 public types
kertal Dec 16, 2019
f94e5d3
Code improvements & cleanups
kertal Dec 16, 2019
ca96123
Fix test
kertal Dec 16, 2019
ae596fe
Merge remote-tracking branch 'upstream/master' into kertal-pr-2019-12…
kertal Dec 17, 2019
03b8efb
Improve types
kertal Dec 17, 2019
a568b28
Merge remote-tracking branch 'upstream/master' into kertal-pr-2019-12…
kertal Dec 17, 2019
e2515bf
remove global angular references in graph
flash1293 Dec 17, 2019
59004ff
fix test
flash1293 Dec 17, 2019
e3b108c
Remove accidentally commited files
kertal Dec 17, 2019
c09e7ca
Add missing doc files
kertal Dec 17, 2019
e392307
Cleanup Discover
kertal Dec 18, 2019
3372685
Cleanup visualize
kertal Dec 18, 2019
78d4822
Cleanup visualize - remove registry
kertal Dec 18, 2019
1ecab13
Cleanup visualize - plugin.ts
kertal Dec 18, 2019
9cc80c2
Cleanup dashboard
kertal Dec 18, 2019
17b3737
Cleanup dashboard - register
kertal Dec 18, 2019
e7cd3b8
merge
kertal Dec 18, 2019
105748f
Merge branch 'flash1293-graph/deangularize' into kertal-pr-2019-12-18…
kertal Dec 18, 2019
c3a8cab
Merge remote-tracking branch 'upstream/master' into kertal-pr-2019-12…
kertal Dec 18, 2019
2dcbe20
Cleanup timelion
kertal Dec 18, 2019
032900d
Migrate saved_object_finder to timelion
kertal Dec 18, 2019
d8b384e
Remove unused SavedObjectRegistryProvider
kertal Dec 18, 2019
7987d95
Fix i18n issues
kertal Dec 18, 2019
4cf7a3b
merge master / fix conflicts
kertal Dec 19, 2019
013e5f4
Cleanup SavedObjectRegistryProvider from discover (merge leftover)
kertal Dec 19, 2019
e8764c3
Merge branch 'master' into kertal-pr-2019-12-18-np-remove-SavedObject…
elasticmachine Dec 20, 2019
a36d340
Dashboard review changes
kertal Dec 20, 2019
0f12cfe
Visualize review changes
kertal Dec 20, 2019
c539e7a
Merge remote-tracking branch 'upstream/master' into kertal-pr-2019-12…
kertal Dec 20, 2019
f21b8d9
Fix potential linting conflict in application.ts
kertal Dec 20, 2019
6f9e2d5
Merge branch 'kertal-pr-2019-12-18-np-remove-SavedObjectRegistryProvi…
kertal Dec 20, 2019
a6eade5
Merge master/fix conflicts
kertal Dec 21, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ export interface RenderDeps {
npDataStart: NpDataStart;
navigation: NavigationStart;
savedObjectsClient: SavedObjectsClientContract;
savedObjectRegistry: any;
dashboardConfig: any;
savedDashboards: any;
dashboardCapabilities: any;
Expand Down
13 changes: 1 addition & 12 deletions src/legacy/core_plugins/kibana/public/dashboard/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,7 @@
* under the License.
*/

import {
npSetup,
npStart,
SavedObjectRegistryProvider,
legacyChrome,
IPrivate,
} from './legacy_imports';
import { npSetup, npStart, legacyChrome } from './legacy_imports';
import { DashboardPlugin, LegacyAngularInjectedDependencies } from './plugin';
import { start as data } from '../../../data/public/legacy';
import { start as embeddables } from '../../../embeddable_api/public/np_ready/public/legacy';
Expand All @@ -37,13 +31,8 @@ import './dashboard_config';
async function getAngularDependencies(): Promise<LegacyAngularInjectedDependencies> {
const injector = await legacyChrome.dangerouslyGetActiveInjector();

const Private = injector.get<IPrivate>('Private');

const savedObjectRegistry = Private(SavedObjectRegistryProvider);

return {
dashboardConfig: injector.get('dashboardConfig'),
Copy link
Contributor

@flash1293 flash1293 Dec 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉Down to one, nice! We should probably just move that flag into kibana_legacy, then dashboard is almost ready to go (not in this PR though)

savedObjectRegistry,
savedDashboards: injector.get('savedDashboards'),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like we also don't need to inject saved dashboards anymore because we can create the class inside the app using the helper (might be done in a separate PR)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thx, I've taken care of this, planned to do it in a 2020 PR, but the time is now!
did it the same way with visualize, so the injected variables are just used for savedObjectManagementRegistry. now the first 2020 PR will be cleanup of this 🔨

};
}
Expand Down
8 changes: 5 additions & 3 deletions src/legacy/core_plugins/kibana/public/dashboard/legacy_app.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import { DashboardListing, EMPTY_FILTER } from './listing/dashboard_listing';
import { addHelpMenuToAppChrome } from './help_menu/help_menu_util';
import { registerTimefilterWithGlobalStateFactory } from '../../../../ui/public/timefilter/setup_router';
import { syncOnMount } from './global_state_sync';
import { savedObjectLoaderDashboard } from './saved_dashboard/saved_dashboards';
Copy link
Contributor

@flash1293 flash1293 Dec 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This import isn't allowed because it uses services directly without going through plugin.ts (the linting PR will start flagging this).

Suggestion to do this cleanly:

  • Create the saved_dashboard class and service in the start phase of the dashboard plugin and
  • expose it through the contract.
  • Then have a little wrapper in legacy world registering the class from the shimmed contract into angular for the management view.

This would put the class creation where it belongs, doesn't cross legacy/shimmed boundaries, wouldn't block moving the dashboard app into NP and is still compatible with the angular setup.

Basically it would mean moving this part into the plugin.ts:

const savedObjectsClient = npStart.core.savedObjects.client;
const services = {
  savedObjectsClient,
  indexPatterns: npStart.plugins.data.indexPatterns,
  chrome: npStart.core.chrome,
  overlays: npStart.core.overlays,
};

const SavedDashboard = createSavedDashboardClass(services);
return {
  savedObjectLoaderDashboard: new SavedObjectLoader(
  SavedDashboard,
  savedObjectsClient,
  npStart.core.chrome
)
};

And keeping just this part in legacy saved_dasboard:

// This is the only thing that gets injected into controllers
module.service('savedDashboards', () => dashboardStart.savedObjectLoaderDashboard);

Copy link
Member Author

@kertal kertal Dec 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx, yes, the upcoming linting improvement will complain about this changes. So I've solved it the "Discover way"™" for now, services created twice, and will clean this by removing module.service('savedDashboards'... in a follow up PR


export function initDashboardApp(app, deps) {
initDashboardAppDirective(app, deps);
Expand Down Expand Up @@ -86,7 +87,8 @@ export function initDashboardApp(app, deps) {
...defaults,
template: dashboardListingTemplate,
controller($injector, $location, $scope) {
const services = deps.savedObjectRegistry.byLoaderPropertiesName;
const service = savedObjectLoaderDashboard;

const kbnUrl = $injector.get('kbnUrl');
const dashboardConfig = deps.dashboardConfig;

Expand All @@ -95,7 +97,7 @@ export function initDashboardApp(app, deps) {
kbnUrl.redirect(DashboardConstants.CREATE_NEW_DASHBOARD_URL);
};
$scope.find = search => {
return services.dashboards.find(search, $scope.listingLimit);
return service.find(search, $scope.listingLimit);
};
$scope.editItem = ({ id }) => {
kbnUrl.redirect(`${createDashboardEditUrl(id)}?_a=(viewMode:edit)`);
Expand All @@ -104,7 +106,7 @@ export function initDashboardApp(app, deps) {
return deps.addBasePath(`#${createDashboardEditUrl(id)}`);
};
$scope.delete = dashboards => {
return services.dashboards.delete(dashboards.map(d => d.id));
return service.delete(dashboards.map(d => d.id));
};
$scope.hideWriteControls = dashboardConfig.getHideWriteControls();
$scope.initialFilter = $location.search().filter || EMPTY_FILTER;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ export { AppState } from 'ui/state_management/app_state';
export { AppStateClass } from 'ui/state_management/app_state';
export { SavedObjectSaveOpts } from 'ui/saved_objects/types';
export { npSetup, npStart } from 'ui/new_platform';
export { SavedObjectRegistryProvider } from 'ui/saved_objects';
export { IPrivate } from 'ui/private';
export { SavedObjectSaveModal } from 'ui/saved_objects/components/saved_object_save_modal';
export { subscribeWithScope } from 'ui/utils/subscribe_with_scope';
Expand Down
1 change: 0 additions & 1 deletion src/legacy/core_plugins/kibana/public/dashboard/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ import { KibanaLegacySetup } from '../../../../../plugins/kibana_legacy/public';

export interface LegacyAngularInjectedDependencies {
dashboardConfig: any;
savedObjectRegistry: any;
savedDashboards: any;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,4 @@
* specific language governing permissions and limitations
* under the License.
*/

import { SavedObjectRegistryProvider } from 'ui/saved_objects/saved_object_registry';
import './saved_dashboards';

SavedObjectRegistryProvider.register((savedDashboards: any) => {
return savedDashboards;
});
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,20 @@ savedObjectManagementRegistry.register({
defaultMessage: 'dashboards',
}),
});
const savedObjectsClient = npStart.core.savedObjects.client;
const services = {
savedObjectsClient,
indexPatterns: npStart.plugins.data.indexPatterns,
chrome: npStart.core.chrome,
overlays: npStart.core.overlays,
};

// This is the only thing that gets injected into controllers
module.service('savedDashboards', function() {
const savedObjectsClient = npStart.core.savedObjects.client;
const services = {
savedObjectsClient,
indexPatterns: npStart.plugins.data.indexPatterns,
chrome: npStart.core.chrome,
overlays: npStart.core.overlays,
};
const SavedDashboard = createSavedDashboardClass(services);
export const savedObjectLoaderDashboard = new SavedObjectLoader(
SavedDashboard,
savedObjectsClient,
npStart.core.chrome
);

const SavedDashboard = createSavedDashboardClass(services);
return new SavedObjectLoader(SavedDashboard, savedObjectsClient, npStart.core.chrome);
});
// This is the only thing that gets injected into controllers
module.service('savedDashboards', () => savedObjectLoaderDashboard);
5 changes: 0 additions & 5 deletions src/legacy/core_plugins/kibana/public/discover/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
*/
import { PluginInitializer, PluginInitializerContext } from 'kibana/public';
import { npSetup, npStart } from 'ui/new_platform';
import { SavedObjectRegistryProvider } from 'ui/saved_objects';
import { DiscoverPlugin, DiscoverSetup, DiscoverStart } from './plugin';

// Core will be looking for this when loading our plugin in the new platform
Expand All @@ -33,8 +32,4 @@ export const pluginInstance = plugin({} as PluginInitializerContext);
pluginInstance.start(npStart.core, npStart.plugins);
})();

SavedObjectRegistryProvider.register((savedSearches: any) => {
return savedSearches;
});

export { createSavedSearchesService } from './saved_searches/saved_searches';
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@ savedObjectManagementRegistry.register({
service: 'savedSearches',
title: 'searches',
});
const services = {
savedObjectsClient: npStart.core.savedObjects.client,
indexPatterns: npStart.plugins.data.indexPatterns,
chrome: npStart.core.chrome,
overlays: npStart.core.overlays,
};
const savedSearches = createSavedSearchesService(services);

const module = uiModules.get('discover/saved_searches');
module.service('savedSearches', () => {
const services = {
savedObjectsClient: npStart.core.savedObjects.client,
indexPatterns: npStart.plugins.data.indexPatterns,
chrome: npStart.core.chrome,
overlays: npStart.core.overlays,
};
return createSavedSearchesService(services);
});
module.service('savedSearches', () => savedSearches);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could use the same pattern as suggested above for dashboard here (only creating the service once and exposing through the contract, but that way is also fine because it's just created twice, once for "inside" the app and once for the global registry. That approach would also be OK for me for dashboard.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♩ ♪ ♫ ♬ ♭ ♮ ♯ 🎼 🎵 🎶 I did it discover way ♩ ♪ ♫ ♬ ♭ ♮ ♯ 🎼 🎵 🎶

5 changes: 2 additions & 3 deletions src/legacy/core_plugins/kibana/public/visualize/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import {
legacyChrome,
npSetup,
npStart,
SavedObjectRegistryProvider,
VisEditorTypesRegistryProvider,
} from './legacy_imports';
import { VisualizePlugin, LegacyAngularInjectedDependencies } from './plugin';
Expand All @@ -42,12 +41,10 @@ async function getAngularDependencies(): Promise<LegacyAngularInjectedDependenci
const Private = injector.get<IPrivate>('Private');

const editorTypes = Private(VisEditorTypesRegistryProvider);
const savedObjectRegistry = Private(SavedObjectRegistryProvider);

return {
legacyChrome,
editorTypes,
savedObjectRegistry,
savedVisualizations: injector.get('savedVisualizations'),
};
}
Expand All @@ -66,3 +63,5 @@ async function getAngularDependencies(): Promise<LegacyAngularInjectedDependenci
visualizations,
});
})();

export { createSavedVisLoader } from './saved_visualizations/saved_visualizations';
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ export interface VisualizeKibanaServices {
navigation: NavigationStart;
toastNotifications: ToastsStart;
savedObjectsClient: SavedObjectsClientContract;
savedObjectRegistry: any;
savedQueryService: DataPublicPluginStart['query']['savedQueries'];
savedVisualizations: SavedVisualizations;
share: SharePluginStart;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ export { IPrivate } from 'ui/private';
// @ts-ignore
export { PrivateProvider } from 'ui/private/private';

export { SavedObjectRegistryProvider } from 'ui/saved_objects';
export { SavedObjectSaveModal } from 'ui/saved_objects/components/saved_object_save_modal';
export { showSaveModal } from 'ui/saved_objects/show_saved_object_save_modal';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { i18n } from '@kbn/i18n';

import { getServices } from '../kibana_services';
import { wrapInI18nContext } from '../legacy_imports';
import { savedObjectLoaderVisualize } from '../saved_visualizations/saved_visualization_register';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same problem as with dashboard here


export function initListingDirective(app) {
app.directive('visualizeListingTable', reactDirective =>
Expand All @@ -47,7 +48,6 @@ export function VisualizeListingController($injector, createNewVis) {
addBasePath,
chrome,
legacyChrome,
savedObjectRegistry,
savedObjectsClient,
data: {
query: {
Expand Down Expand Up @@ -94,10 +94,7 @@ export function VisualizeListingController($injector, createNewVis) {
// In case the user navigated to the page via the /visualize/new URL we start the dialog immediately
this.createNewVis();
}

// TODO: Extract this into an external service.
const services = savedObjectRegistry.byLoaderPropertiesName;
const visualizationService = services.visualizations;
const visualizationService = savedObjectLoaderVisualize;
this.visTypeRegistry = visualizations.types;

this.fetchItems = filter => {
Expand Down
1 change: 0 additions & 1 deletion src/legacy/core_plugins/kibana/public/visualize/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ import { SavedVisualizations } from './types';
export interface LegacyAngularInjectedDependencies {
legacyChrome: any;
editorTypes: any;
savedObjectRegistry: any;
savedVisualizations: SavedVisualizations;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,28 @@
* specific language governing permissions and limitations
* under the License.
*/

import { SavedObjectRegistryProvider } from 'ui/saved_objects/saved_object_registry';
import { npStart } from 'ui/new_platform';
// @ts-ignore
import { uiModules } from 'ui/modules';
// @ts-ignore
import { savedObjectManagementRegistry } from '../../management/saved_object_registry';
import './saved_visualizations';
import { createSavedVisLoader } from './saved_visualizations';

SavedObjectRegistryProvider.register((savedVisualizations: any) => {
return savedVisualizations;
});
const services = {
savedObjectsClient: npStart.core.savedObjects.client,
indexPatterns: npStart.plugins.data.indexPatterns,
chrome: npStart.core.chrome,
overlays: npStart.core.overlays,
};

export const savedObjectLoaderVisualize = createSavedVisLoader(services);

// Register this service with the saved object registry so it can be
// edited by the object editor.
savedObjectManagementRegistry.register({
service: 'savedVisualizations',
title: 'visualizations',
});

uiModules.get('app/visualize').service('savedVisualizations', () => savedObjectLoaderVisualize);
Original file line number Diff line number Diff line change
Expand Up @@ -16,26 +16,18 @@
* specific language governing permissions and limitations
* under the License.
*/
import { npStart } from 'ui/new_platform';
// @ts-ignore
import { uiModules } from 'ui/modules';
import { SavedObjectLoader } from 'ui/saved_objects';
import { SavedObjectKibanaServices } from 'ui/saved_objects/types';

import { start as visualizations } from '../../../../visualizations/public/np_ready/public/legacy';
import { createVisualizeEditUrl } from '../visualize_constants';
// @ts-ignore
import { findListItems } from './find_list_items';
import { createSavedVisClass } from './_saved_vis';
const app = uiModules.get('app/visualize');

app.service('savedVisualizations', function() {
const savedObjectsClient = npStart.core.savedObjects.client;
const services = {
savedObjectsClient,
indexPatterns: npStart.plugins.data.indexPatterns,
chrome: npStart.core.chrome,
overlays: npStart.core.overlays,
};
export function createSavedVisLoader(services: SavedObjectKibanaServices) {
const { savedObjectsClient } = services;

class SavedObjectLoaderVisualize extends SavedObjectLoader {
mapHitSource = (source: Record<string, any>, id: string) => {
const visTypes = visualizations.types;
Expand Down Expand Up @@ -81,6 +73,5 @@ app.service('savedVisualizations', function() {
}
}
const SavedVis = createSavedVisClass(services);

return new SavedObjectLoaderVisualize(SavedVis, savedObjectsClient, npStart.core.chrome);
});
return new SavedObjectLoaderVisualize(SavedVis, savedObjectsClient, services.chrome);
}
14 changes: 8 additions & 6 deletions src/legacy/core_plugins/timelion/public/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import { i18n } from '@kbn/i18n';

import { capabilities } from 'ui/capabilities';
import { docTitle } from 'ui/doc_title';
import { SavedObjectRegistryProvider } from 'ui/saved_objects/saved_object_registry';
import { fatalError, toastNotifications } from 'ui/notify';
import { timezoneProvider } from 'ui/vis/lib/timezone';
import { timefilter } from 'ui/timefilter';
Expand All @@ -36,15 +35,14 @@ require('ui/autoload/all');

// TODO: remove ui imports completely (move to plugins)
import 'ui/directives/input_focus';
import 'ui/directives/saved_object_finder';
import './directives/saved_object_finder';
import 'ui/directives/listen';
import 'ui/kbn_top_nav';
import 'ui/saved_objects/ui/saved_object_save_as_checkbox';
import './services/saved_sheets';
import './services/_saved_sheet';
import './services/saved_sheet_register';

import rootTemplate from 'plugins/timelion/index.html';
import { createSavedVisLoader } from '../../kibana/public/visualize';

require('plugins/timelion/directives/cells/cells');
require('plugins/timelion/directives/fixed_element');
Expand Down Expand Up @@ -129,8 +127,12 @@ app.controller('timelion', function(
timefilter.enableAutoRefreshSelector();
timefilter.enableTimeRangeSelector();

const savedVisualizations = Private(SavedObjectRegistryProvider).byLoaderPropertiesName
.visualizations;
const savedVisualizations = createSavedVisLoader({
savedObjectsClient: npStart.core.savedObjects.client,
indexPatterns: npStart.plugins.data.indexPatterns,
chrome: npStart.core.chrome,
overlays: npStart.core.overlays,
});
const timezone = Private(timezoneProvider)();

const defaultExpression = '.es(*)';
Expand Down
Loading