From c54eb3f32729dd188a3fdb7326cedb9970adca8b Mon Sep 17 00:00:00 2001 From: Stacey Gammon Date: Wed, 30 Nov 2016 15:55:28 -0500 Subject: [PATCH 1/7] make scope isolate --- .../dashboard/components/panel/panel.js | 85 +++++++++---------- .../public/dashboard/directives/grid.js | 31 +++++-- 2 files changed, 62 insertions(+), 54 deletions(-) diff --git a/src/core_plugins/kibana/public/dashboard/components/panel/panel.js b/src/core_plugins/kibana/public/dashboard/components/panel/panel.js index 776f1f04fbcb9..7477a98d3f6f8 100644 --- a/src/core_plugins/kibana/public/dashboard/components/panel/panel.js +++ b/src/core_plugins/kibana/public/dashboard/components/panel/panel.js @@ -27,60 +27,55 @@ uiModules return { restrict: 'E', template: panelTemplate, - requires: '^dashboardGrid', + scope: { + chrome: '=', + parentUiState: '=', + panel: '=', + remove: '&' + }, link: function ($scope) { - // using $scope inheritance, panels are available in AppState - const $state = $scope.state; - // receives $scope.panel from the dashboard grid directive, seems like should be isolate? - $scope.$watch('id', function () { - if (!$scope.panel.id || !$scope.panel.type) return; + if (!$scope.panel.id || !$scope.panel.type) return; - loadPanel($scope.panel, $scope) - .then(function (panelConfig) { - // These could be done in loadPanel, putting them here to make them more explicit - $scope.savedObj = panelConfig.savedObj; - $scope.editUrl = panelConfig.editUrl; - $scope.$on('$destroy', function () { - panelConfig.savedObj.destroy(); - $scope.parentUiState.removeChild(getPanelId(panelConfig.panel)); - }); - - // create child ui state from the savedObj - const uiState = panelConfig.uiState || {}; - $scope.uiState = $scope.parentUiState.createChild(getPanelId(panelConfig.panel), uiState, true); - const panelSavedVis = _.get(panelConfig, 'savedObj.vis'); // Sometimes this will be a search, and undef - if (panelSavedVis) { - panelSavedVis.setUiState($scope.uiState); - } + loadPanel($scope.panel, $scope) + .then(function (panelConfig) { + // These could be done in loadPanel, putting them here to make them more explicit + $scope.savedObj = panelConfig.savedObj; + $scope.editUrl = panelConfig.editUrl; + $scope.$on('$destroy', function () { + panelConfig.savedObj.destroy(); + $scope.parentUiState.removeChild(getPanelId(panelConfig.panel)); + }); - $scope.filter = function (field, value, operator) { - const index = $scope.savedObj.searchSource.get('index').id; - filterManager.add(field, value, operator, index); - }; - }) - .catch(function (e) { - $scope.error = e.message; + // create child ui state from the savedObj + const uiState = panelConfig.uiState || {}; + $scope.uiState = $scope.parentUiState.createChild(getPanelId(panelConfig.panel), uiState, true); + const panelSavedVis = _.get(panelConfig, 'savedObj.vis'); // Sometimes this will be a search, and undef + if (panelSavedVis) { + panelSavedVis.setUiState($scope.uiState); + } - // If the savedObjectType matches the panel type, this means the object itself has been deleted, - // so we shouldn't even have an edit link. If they don't match, it means something else is wrong - // with the object (but the object still exists), so we link to the object editor instead. - const objectItselfDeleted = e.savedObjectType === $scope.panel.type; - if (objectItselfDeleted) return; + $scope.filter = function (field, value, operator) { + const index = $scope.savedObj.searchSource.get('index').id; + filterManager.add(field, value, operator, index); + }; + }) + .catch(function (e) { + $scope.error = e.message; - const type = $scope.panel.type; - const id = $scope.panel.id; - const service = _.find(services, { type: type }); - if (!service) return; + // If the savedObjectType matches the panel type, this means the object itself has been deleted, + // so we shouldn't even have an edit link. If they don't match, it means something else is wrong + // with the object (but the object still exists), so we link to the object editor instead. + const objectItselfDeleted = e.savedObjectType === $scope.panel.type; + if (objectItselfDeleted) return; - $scope.editUrl = '#management/kibana/objects/' + service.name + '/' + id + '?notFound=' + e.savedObjectType; - }); + const type = $scope.panel.type; + const id = $scope.panel.id; + const service = _.find(services, { type: type }); + if (!service) return; + $scope.editUrl = '#management/kibana/objects/' + service.name + '/' + id + '?notFound=' + e.savedObjectType; }); - - $scope.remove = function () { - _.pull($state.panels, $scope.panel); - }; } }; }); diff --git a/src/core_plugins/kibana/public/dashboard/directives/grid.js b/src/core_plugins/kibana/public/dashboard/directives/grid.js index 7b744ba9a1572..29ebf24f656cb 100644 --- a/src/core_plugins/kibana/public/dashboard/directives/grid.js +++ b/src/core_plugins/kibana/public/dashboard/directives/grid.js @@ -73,6 +73,12 @@ app.directive('dashboardGrid', function ($compile, Notifier) { gridster.enable().enable_resize(); }); + // Does not remove the panel from the ui - that will be handled by the watch below, which + // will be triggered by the removal. + $scope.removePanelFromState = (panel) => { + _.pull($scope.state.panels, panel); + }; + $scope.$watchCollection('state.panels', function (panels) { const currentPanels = gridster.$widgets.toArray().map(function (el) { return getPanelFor(el); @@ -126,7 +132,7 @@ app.directive('dashboardGrid', function ($compile, Notifier) { const panel = $panel.data('panel'); panel.$el = $panel; - panel.$scope = $panel.data('$scope'); + // panel.$scope = $panel.data('$scope'); return panel; } @@ -136,7 +142,7 @@ app.directive('dashboardGrid', function ($compile, Notifier) { // want them to show up in the url) function makePanelSerializeable(panel) { delete panel.$el; - delete panel.$scope; + // delete panel.$scope; } // tell gridster to remove the panel, and cleanup our metadata @@ -145,7 +151,7 @@ app.directive('dashboardGrid', function ($compile, Notifier) { gridster.remove_widget(panel.$el, silent); // destroy the scope - panel.$scope.$destroy(); + // panel.$scope.$destroy(); panel.$el.removeData('panel'); panel.$el.removeData('$scope'); @@ -170,11 +176,19 @@ app.directive('dashboardGrid', function ($compile, Notifier) { } } - panel.$scope = $scope.$new(); - panel.$scope.panel = panel; - panel.$scope.parentUiState = $scope.uiState; + // panel.$scope = $scope.$new(); + // panel.$scope.panel = panel; + // panel.$scope.parentUiState = $scope.uiState; + const panelIndex = _.indexOf($scope.state.panels, panel); - panel.$el = $compile('
  • ')(panel.$scope); + const panelHtml = ` +
  • + +
  • `; + panel.$el = $compile(panelHtml)($scope); // tell gridster to use the widget gridster.add_widget(panel.$el, panel.size_x, panel.size_y, panel.col, panel.row); @@ -184,7 +198,7 @@ app.directive('dashboardGrid', function ($compile, Notifier) { // stash the panel and it's scope in the element's data panel.$el.data('panel', panel); - panel.$el.data('$scope', panel.$scope); + // panel.$el.data('$scope', panel.$scope); } // ensure that the panel object has the latest size/pos info @@ -202,7 +216,6 @@ app.directive('dashboardGrid', function ($compile, Notifier) { gridster.$widgets.each(function (i, el) { const panel = getPanelFor(el); refreshPanelStats(panel); - panel.$scope.$broadcast('resize'); makePanelSerializeable(panel); $scope.$root.$broadcast('change:vis'); }); From e76f638a65ebd63e64bac35773f8df5d485bc32e Mon Sep 17 00:00:00 2001 From: Stacey Gammon Date: Thu, 1 Dec 2016 13:54:50 -0500 Subject: [PATCH 2/7] Make panel scope isolate Also clean up and simplify scope destroying Fix sorting on scripted date and boolean fields (#9261) The elasticsearch API only [supports][1][2] sort scripts of type `number` and `string`. Since dates need to be returned as millis since the epoch for visualizations to work anyway, we can simply add a condition to send dates as type number in the sort API. ES will cast booleans if we tell them its a string, so we can add a similar condition there as well. [1]: https://www.elastic.co/guide/en/elasticsearch/reference/5.0/search-request-sort.html#_script_based_sorting [2]: https://github.com/elastic/elasticsearch/blob/aeb97ff41298e26b107a733837dfe17f123c0c9b/core/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java#L359 Fixes: https://github.com/elastic/kibana/issues/9257 Add docs on all available console server config options (#9288) settings: do not query ES for settings in non-green status (#9308) If the ui settings status is not green, that means there is at least one dependency (so elasticsearch at the moment) that is not met in order for it to function correctly, so we shouldn't attempt to determine user settings at all. This ensures that when something like the version check fails in the elasticsearch plugin, Kibana correctly behaves by not attempting requests to elasticsearch, which prevents 500 errors and allows users to see the error status on the status page. We also now periodically check for compatible elasticsearch versions so that Kibana can automatically recover if the elasticsearch node is upgraded to the appropriate version. Change loading screen background to white to make it less distracting when navigating between apps. (#9313) more refactoring Fix sorting on scripted date and boolean fields (#9261) The elasticsearch API only [supports][1][2] sort scripts of type `number` and `string`. Since dates need to be returned as millis since the epoch for visualizations to work anyway, we can simply add a condition to send dates as type number in the sort API. ES will cast booleans if we tell them its a string, so we can add a similar condition there as well. [1]: https://www.elastic.co/guide/en/elasticsearch/reference/5.0/search-request-sort.html#_script_based_sorting [2]: https://github.com/elastic/elasticsearch/blob/aeb97ff41298e26b107a733837dfe17f123c0c9b/core/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java#L359 Fixes: https://github.com/elastic/kibana/issues/9257 Add docs on all available console server config options (#9288) settings: do not query ES for settings in non-green status (#9308) If the ui settings status is not green, that means there is at least one dependency (so elasticsearch at the moment) that is not met in order for it to function correctly, so we shouldn't attempt to determine user settings at all. This ensures that when something like the version check fails in the elasticsearch plugin, Kibana correctly behaves by not attempting requests to elasticsearch, which prevents 500 errors and allows users to see the error status on the status page. We also now periodically check for compatible elasticsearch versions so that Kibana can automatically recover if the elasticsearch node is upgraded to the appropriate version. Change loading screen background to white to make it less distracting when navigating between apps. (#9313) Skip assertion when the test blips. (#9251) Tests fails intermittently, and for identical reasons. When this occurs, skip the test. This only occurs in CI and cannot be reproduced locally. Additional changes: - Use async/await to improve legibility. - Move async behaviour to beforeEach and keep test stubs synchronous to improve labeling of tests. [git] ignore extra files in the root config/ directory (#9296) upgrade makelogs (#9295) build: remove deepModules hackery (#9327) The deepModules hacks in the build system were added to support the long paths that resulted from npm2, but npm3 fundamentally addresses that problem, so deepModules is no longer necessary. In practical terms, npm3 shouldn't ever cause path lengths to become so long that they trigger path length problems on certain operating systems. more cleanup and tests Save/rename flow fix (#9087) * Save/rename flow fix - Use auto generated ids instead of coupling the id to the title which creates problems. - Adjust UI to make the save flow more understandable. - Remove confirmation on overwrite since we will now be creating duplicate objects even if they have the same title. * use undefined instead of null * Change titleChanged function name * address code review comments * Add isSaving flag to avoid checkbox flicker, fix regression bug from refactor. Added tests and fixed a couple bugs Updated info message * Update doc and nav title with new name on rename don't hardcode Dashboard --- .gitignore | 3 +- Gruntfile.js | 18 +- docs/console.asciidoc | 2 +- docs/console/configuring-console.asciidoc | 57 +++ docs/console/disabling-console.asciidoc | 10 - docs/setup/settings.asciidoc | 4 + package.json | 2 +- .../elasticsearch/lib/health_check.js | 9 +- .../dashboard/__tests__/dashboard_panels.js | 33 +- .../dashboard/components/panel/lib/panel.js | 74 +++ .../components/panel/lib/panel_utils.js | 45 ++ .../dashboard/components/panel/panel.html | 8 +- .../dashboard_panel_directive.js} | 39 +- .../public/dashboard/directives/grid.js | 90 +--- .../kibana/public/dashboard/index.html | 2 +- .../kibana/public/dashboard/index.js | 36 +- .../dashboard/partials/save_dashboard.html | 14 +- .../public/discover/controllers/discover.js | 2 +- .../kibana/public/discover/index.html | 2 +- .../public/discover/partials/save_search.html | 2 + .../public/visualize/editor/editor.html | 2 +- .../kibana/public/visualize/editor/editor.js | 8 +- .../public/visualize/editor/panels/save.html | 3 + .../tagcloud/public/__tests__/tag_cloud.js | 460 +++++++++++------- src/core_plugins/tagcloud/public/tag_cloud.js | 16 +- src/core_plugins/timelion/public/app.js | 4 +- src/core_plugins/timelion/public/index.html | 2 +- .../timelion/public/partials/save_sheet.html | 6 +- src/ui/public/autoload/modules.js | 1 + .../public/courier/__tests__/saved_object.js | 110 ++++- .../data_source/_normalize_sort_request.js | 19 +- .../courier/saved_object/saved_object.js | 189 +++---- .../ui/saved_object_save_as_checkbox.html | 9 + .../ui/saved_object_save_as_checkbox.js | 14 + src/ui/public/styles/input.less | 1 + src/ui/public/utils/__tests__/slugify_id.js | 42 -- src/ui/public/utils/slugify_id.js | 19 - src/ui/settings/__tests__/index.js | 13 +- src/ui/settings/index.js | 7 + src/ui/views/chrome.jade | 2 +- src/ui/views/ui_app.jade | 3 +- tasks/build/index.js | 2 - tasks/build/package_json.js | 5 +- tasks/config/clean.js | 3 - 44 files changed, 915 insertions(+), 477 deletions(-) create mode 100644 docs/console/configuring-console.asciidoc delete mode 100644 docs/console/disabling-console.asciidoc create mode 100644 src/core_plugins/kibana/public/dashboard/components/panel/lib/panel.js create mode 100644 src/core_plugins/kibana/public/dashboard/components/panel/lib/panel_utils.js rename src/core_plugins/kibana/public/dashboard/{components/panel/panel.js => directives/dashboard_panel_directive.js} (72%) create mode 100644 src/ui/public/courier/saved_object/ui/saved_object_save_as_checkbox.html create mode 100644 src/ui/public/courier/saved_object/ui/saved_object_save_as_checkbox.js delete mode 100644 src/ui/public/utils/__tests__/slugify_id.js delete mode 100644 src/ui/public/utils/slugify_id.js diff --git a/.gitignore b/.gitignore index bb293595e316e..2d58640087d9c 100644 --- a/.gitignore +++ b/.gitignore @@ -26,7 +26,8 @@ target data disabledPlugins webpackstats.json -config/kibana.dev.yml +config/* +!config/kibana.yml coverage selenium .babelcache.json diff --git a/Gruntfile.js b/Gruntfile.js index d4246608d8d21..6c025cd1b94a0 100644 --- a/Gruntfile.js +++ b/Gruntfile.js @@ -49,23 +49,7 @@ module.exports = function (grunt) { '!<%= src %>/core_plugins/timelion/vendor_components/**/*.js', '!<%= src %>/fixtures/**/*.js', '!<%= root %>/test/fixtures/scenarios/**/*.js' - ], - deepModules: { - 'caniuse-db': '1.0.30000265', - 'chalk': '1.1.0', - 'glob': '4.5.3', - 'har-validator': '1.8.0', - 'json5': '0.4.0', - 'loader-utils': '0.2.11', - 'micromatch': '2.2.0', - 'postcss-normalize-url': '2.1.1', - 'postcss-reduce-idents': '1.0.2', - 'postcss-unique-selectors': '1.0.0', - 'postcss-minify-selectors': '1.4.6', - 'postcss-single-charset': '0.3.0', - 'regenerator': '0.8.36', - 'readable-stream': '2.1.0' - } + ] }; grunt.config.merge(config); diff --git a/docs/console.asciidoc b/docs/console.asciidoc index 11d28df26531f..895a632b45d06 100644 --- a/docs/console.asciidoc +++ b/docs/console.asciidoc @@ -83,4 +83,4 @@ include::console/history.asciidoc[] include::console/settings.asciidoc[] -include::console/disabling-console.asciidoc[] +include::console/configuring-console.asciidoc[] diff --git a/docs/console/configuring-console.asciidoc b/docs/console/configuring-console.asciidoc new file mode 100644 index 0000000000000..deb9d6e190d1b --- /dev/null +++ b/docs/console/configuring-console.asciidoc @@ -0,0 +1,57 @@ +[[configuring-console]] +== Configuring Console + +You can add the following options in the `config/kibana.yml` file: + +`console.enabled`:: *Default: true* Set to false to disable Console. Toggling this will cause the server to regenerate assets on the next startup, which may cause a delay before pages start being served. + +`console.proxyFilter`:: *Default: `.*`* A list of regular expressions that are used to validate any outgoing request from Console. If none + of these match, the request will be rejected. See <> for more details. + +`console.proxyConfig`:: A list of configuration options that are based on the proxy target. Use this to set custom timeouts or SSL settings for specific hosts. This is done by defining a set of `match` criteria using wildcards/globs which will be checked against each request. The configuration from all matching rules will then be merged together to configure the proxy used for that request. ++ +The valid match keys are `match.protocol`, `match.host`, `match.port`, and `match.path`. All of these keys default to `*`, which means they will match any value. ++ +Example: ++ +[source,yaml] +-------- +console.proxyConfig: + - match: + host: "*.internal.org" # allow any host that ends in .internal.org + port: "{9200..9299}" # allow any port from 9200-9299 + + ssl: + ca: "/opt/certs/internal.ca" + # "key" and "cert" are also valid options here + + - match: + protocol: "https" + + ssl: + verify: false # allows any certificate to be used, even self-signed certs + + # since this rule has no "match" section it matches everything + - timeout: 180000 # 3 minutes +-------- + +[[securing-console]] +=== Securing Console + +Console is meant to be used as a local development tool. As such, it will send requests to any host & port combination, +just as a local curl command would. To overcome the CORS limitations enforced by browsers, Console's Node.js backend +serves as a proxy to send requests on behalf of the browser. However, if put on a server and exposed to the internet +this can become a security risk. In those cases, we highly recommend you lock down the proxy by setting the +`console.proxyFilter` setting. The setting accepts a list of regular expressions that are evaluated against each URL + the proxy is requested to retrieve. If none of the regular expressions match the proxy will reject the request. + +Here is an example configuration the only allows Console to connect to localhost: + +[source,yaml] +-------- +console.proxyFilter: + - ^https?://(localhost|127\.0\.0\.1|\[::0\]).* +-------- + +You will need to restart Kibana for these changes to take effect. + diff --git a/docs/console/disabling-console.asciidoc b/docs/console/disabling-console.asciidoc deleted file mode 100644 index 7aa1fa56e77f7..0000000000000 --- a/docs/console/disabling-console.asciidoc +++ /dev/null @@ -1,10 +0,0 @@ -[[disabling-console]] -== Disable Console - -If the users of Kibana have no requirements or need to access any of the Console functionality, it can -be disabled completely and not even show up as an available app by setting the `console.enabled` Kibana server setting to `false`: - -[source,yaml] --------- -console.enabled: false --------- diff --git a/docs/setup/settings.asciidoc b/docs/setup/settings.asciidoc index 4cdc402e562c5..bf31ba692dec9 100644 --- a/docs/setup/settings.asciidoc +++ b/docs/setup/settings.asciidoc @@ -65,3 +65,7 @@ The minimum value is 100. `status.allowAnonymous`:: *Default: false* If authentication is enabled, setting this to `true` allows unauthenticated users to access the Kibana server status API and status page. `console.enabled`:: *Default: true* Set to false to disable Console. Toggling this will cause the server to regenerate assets on the next startup, which may cause a delay before pages start being served. +`console.proxyFilter`:: *Default: `.*`* A list of regular expressions that are used to validate any outgoing request from Console. If none of these match, the request will be rejected. +`console.proxyConfig`:: A list of configuration options that are based on the proxy target. Use this to set custom timeouts or SSL settings for specific hosts. This is done by defining a set of `match` criteria using wildcards/globs which will be checked against each request. The configuration from all matching rules will then be merged together to configure the proxy used for that request. ++ +The valid match keys are `match.protocol`, `match.host`, `match.port`, and `match.path`. All of these keys default to `*`, which means they will match any value. See <> for an example. diff --git a/package.json b/package.json index b26563945552c..01ac395810b7d 100644 --- a/package.json +++ b/package.json @@ -205,7 +205,7 @@ "karma-safari-launcher": "0.1.1", "license-checker": "5.1.2", "load-grunt-config": "0.19.2", - "makelogs": "3.0.2", + "makelogs": "3.1.1", "marked-text-renderer": "0.1.0", "mocha": "2.5.3", "murmurhash3js": "3.0.1", diff --git a/src/core_plugins/elasticsearch/lib/health_check.js b/src/core_plugins/elasticsearch/lib/health_check.js index 4f1555893d871..00ab4e1686730 100644 --- a/src/core_plugins/elasticsearch/lib/health_check.js +++ b/src/core_plugins/elasticsearch/lib/health_check.js @@ -81,13 +81,20 @@ module.exports = function (plugin, server) { }); } + function waitForEsVersion() { + return checkEsVersion(server, kibanaVersion.get()).catch(err => { + plugin.status.red(err); + return Promise.delay(REQUEST_DELAY).then(waitForEsVersion); + }); + } + function setGreenStatus() { return plugin.status.green('Kibana index ready'); } function check() { return waitForPong() - .then(() => checkEsVersion(server, kibanaVersion.get())) + .then(waitForEsVersion) .then(waitForShards) .then(setGreenStatus) .then(_.partial(migrateConfig, server)) diff --git a/src/core_plugins/kibana/public/dashboard/__tests__/dashboard_panels.js b/src/core_plugins/kibana/public/dashboard/__tests__/dashboard_panels.js index 0611c8eba587a..7c685e2bdb488 100644 --- a/src/core_plugins/kibana/public/dashboard/__tests__/dashboard_panels.js +++ b/src/core_plugins/kibana/public/dashboard/__tests__/dashboard_panels.js @@ -2,12 +2,13 @@ import angular from 'angular'; import expect from 'expect.js'; import ngMock from 'ng_mock'; import 'plugins/kibana/dashboard/services/_saved_dashboard'; +import { DEFAULT_PANEL_WIDTH, DEFAULT_PANEL_HEIGHT } from '../components/panel/lib/panel'; describe('dashboard panels', function () { let $scope; let $el; - const compile = (dashboard) => { + function compile(dashboard) { ngMock.inject(($rootScope, $controller, $compile, $route) => { $scope = $rootScope.$new(); $route.current = { @@ -19,12 +20,16 @@ describe('dashboard panels', function () { $el = angular.element(` - `); + `); $compile($el)($scope); $scope.$digest(); }); }; + function findPanelWithVisualizationId(id) { + return $scope.state.panels.find((panel) => { return panel.id === id; }); + } + beforeEach(() => { ngMock.module('kibana'); }); @@ -77,10 +82,30 @@ describe('dashboard panels', function () { compile(dash); }); expect($scope.state.panels.length).to.be(16); - const foo8Panel = $scope.state.panels.find( - (panel) => { return panel.id === 'foo8'; }); + const foo8Panel = findPanelWithVisualizationId('foo8'); expect(foo8Panel).to.not.be(null); expect(foo8Panel.row).to.be(8); expect(foo8Panel.col).to.be(1); }); + + it('initializes visualizations with the default size', function () { + ngMock.inject((SavedDashboard) => { + let dash = new SavedDashboard(); + dash.init(); + dash.panelsJSON = `[ + {"col":3,"id":"foo1","row":1,"type":"visualization"}, + {"col":5,"id":"foo2","row":1,"size_x":5,"size_y":9,"type":"visualization"}]`; + compile(dash); + }); + expect($scope.state.panels.length).to.be(2); + const foo1Panel = findPanelWithVisualizationId('foo1'); + expect(foo1Panel).to.not.be(null); + expect(foo1Panel.size_x).to.be(DEFAULT_PANEL_WIDTH); + expect(foo1Panel.size_y).to.be(DEFAULT_PANEL_HEIGHT); + + const foo2Panel = findPanelWithVisualizationId('foo2'); + expect(foo2Panel).to.not.be(null); + expect(foo2Panel.size_x).to.be(5); + expect(foo2Panel.size_y).to.be(9); + }); }); diff --git a/src/core_plugins/kibana/public/dashboard/components/panel/lib/panel.js b/src/core_plugins/kibana/public/dashboard/components/panel/lib/panel.js new file mode 100644 index 0000000000000..07aad05710086 --- /dev/null +++ b/src/core_plugins/kibana/public/dashboard/components/panel/lib/panel.js @@ -0,0 +1,74 @@ +export const DEFAULT_PANEL_WIDTH = 3; +export const DEFAULT_PANEL_HEIGHT = 2; + +/** + * Represents a panel on a grid. Keeps track of position in the grid and what visualization it + * contains. + * + * @param id - Id of the visualization contained in the panel. + * @param type - Type of the visualization in the panel. + * @param panelId - Unique id to represent this panel in the grid. + * @constructor + */ +function Panel(id, type, panelId) { + /** + * A reference to the gridster widget holding this panel. Used to + * update the size and column attributes. + */ + this.$el; + + /** + * Width of the panel. + * @type {number} + */ + this.size_x = DEFAULT_PANEL_WIDTH; + + /** + * Height of the panel. + * @type {number} + */ + this.size_y = DEFAULT_PANEL_HEIGHT; + + /** + * Column index in the grid. + * @type {number} + */ + this.col; + + /** + * Row index of the panel in the grid. + * @type {number} + */ + this.row; + + /** + * Height of the panel. + * @type {number} + */ + this.id = id; + + /** + * Unique identifier for this panel. Guaranteed to be unique among the current + * panels in the grid. + * @type {number} + */ + this.panelId = panelId; + + /** + * Type of visualization this panel contains. + * @type {string} + */ + this.type = type; +} + +/** + * Factory function to create a panel object. + * + * @param id {string} - The id of the visualization this panel contains + * @param type {string} - The type of visualization this panel contains + * @param panelId {number} - A unique identifier for this panel in the grid + * @returns {Panel} + */ +export function createNewPanel(id, type, panelId) { + return new Panel(id, type, panelId); +} diff --git a/src/core_plugins/kibana/public/dashboard/components/panel/lib/panel_utils.js b/src/core_plugins/kibana/public/dashboard/components/panel/lib/panel_utils.js new file mode 100644 index 0000000000000..dc2a80f25e931 --- /dev/null +++ b/src/core_plugins/kibana/public/dashboard/components/panel/lib/panel_utils.js @@ -0,0 +1,45 @@ +import { DEFAULT_PANEL_WIDTH, DEFAULT_PANEL_HEIGHT } from 'plugins/kibana/dashboard/components/panel/lib/panel'; + +export class PanelUtils { + /** + * Fills in default parameters where not specified. + * @param panel {Panel} + */ + static initializeDefaults(panel) { + panel.size_x = panel.size_x || DEFAULT_PANEL_WIDTH; + panel.size_y = panel.size_y || DEFAULT_PANEL_HEIGHT; + + if (!panel.id) { + // In the interest of backwards comparability + if (panel.visId) { + panel.id = panel.visId; + panel.type = 'visualization'; + delete panel.visId; + } else { + throw new Error('Missing object id on panel'); + } + } + } + + /** + * Ensures that the panel object has the latest size/pos info. + * @param panel {Panel} + */ + static refreshSizeAndPosition(panel) { + const data = panel.$el.coords().grid; + panel.size_x = data.size_x; + panel.size_y = data.size_y; + panel.col = data.col; + panel.row = data.row; + } + + /** + * $el is a circular structure because it contains a reference to it's parent panel, + * so it needs to be removed before it can be serialized (we also don't + * want it to show up in the url). + * @param panel + */ + static makeSerializeable(panel) { + delete panel.$el; + } +} diff --git a/src/core_plugins/kibana/public/dashboard/components/panel/panel.html b/src/core_plugins/kibana/public/dashboard/components/panel/panel.html index 874a62b75522a..1cba6ed1d7ca5 100644 --- a/src/core_plugins/kibana/public/dashboard/components/panel/panel.html +++ b/src/core_plugins/kibana/public/dashboard/components/panel/panel.html @@ -4,13 +4,13 @@ {{::savedObj.title}} @@ -26,7 +26,7 @@ ng-switch-when="visualization" vis="savedObj.vis" search-source="savedObj.searchSource" - show-spy-panel="chrome.getVisible()" + show-spy-panel="!isFullScreenMode" ui-state="uiState" render-counter class="panel-content"> diff --git a/src/core_plugins/kibana/public/dashboard/components/panel/panel.js b/src/core_plugins/kibana/public/dashboard/directives/dashboard_panel_directive.js similarity index 72% rename from src/core_plugins/kibana/public/dashboard/components/panel/panel.js rename to src/core_plugins/kibana/public/dashboard/directives/dashboard_panel_directive.js index 7477a98d3f6f8..bc0abc9799883 100644 --- a/src/core_plugins/kibana/public/dashboard/components/panel/panel.js +++ b/src/core_plugins/kibana/public/dashboard/directives/dashboard_panel_directive.js @@ -20,21 +20,42 @@ uiModules }; }); - const getPanelId = function (panel) { - return ['P', panel.panelIndex].join('-'); + /** + * Returns a unique id for storing the panel state in the persistent ui. + * @param panel + * @returns {string} + */ + const getPersistedStateId = function (panel) { + return `P-${panel.panelId}`; }; return { restrict: 'E', template: panelTemplate, scope: { - chrome: '=', + /** + * Whether or not the dashboard this panel is contained on is in 'full screen mode'. + * @type {boolean} + */ + isFullScreenMode: '=', + /** + * The parent's persisted state is used to create a child persisted state for the + * panel. + * @type {PersistedState} + */ parentUiState: '=', + /** + * Contains information about this panel. + * @type {Panel} + */ panel: '=', + /** + * Handles removing this panel from the grid. + * @type {() => void} + */ remove: '&' }, - link: function ($scope) { - + link: function ($scope, element) { if (!$scope.panel.id || !$scope.panel.type) return; loadPanel($scope.panel, $scope) @@ -42,14 +63,16 @@ uiModules // These could be done in loadPanel, putting them here to make them more explicit $scope.savedObj = panelConfig.savedObj; $scope.editUrl = panelConfig.editUrl; - $scope.$on('$destroy', function () { + + element.on('$destroy', function () { panelConfig.savedObj.destroy(); - $scope.parentUiState.removeChild(getPanelId(panelConfig.panel)); + $scope.parentUiState.removeChild(getPersistedStateId(panelConfig.panel)); + $scope.$destroy(); }); // create child ui state from the savedObj const uiState = panelConfig.uiState || {}; - $scope.uiState = $scope.parentUiState.createChild(getPanelId(panelConfig.panel), uiState, true); + $scope.uiState = $scope.parentUiState.createChild(getPersistedStateId(panelConfig.panel), uiState, true); const panelSavedVis = _.get(panelConfig, 'savedObj.vis'); // Sometimes this will be a search, and undef if (panelSavedVis) { panelSavedVis.setUiState($scope.uiState); diff --git a/src/core_plugins/kibana/public/dashboard/directives/grid.js b/src/core_plugins/kibana/public/dashboard/directives/grid.js index 29ebf24f656cb..42c6296bc2130 100644 --- a/src/core_plugins/kibana/public/dashboard/directives/grid.js +++ b/src/core_plugins/kibana/public/dashboard/directives/grid.js @@ -3,6 +3,7 @@ import $ from 'jquery'; import Binder from 'ui/binder'; import 'gridster'; import uiModules from 'ui/modules'; +import { PanelUtils } from 'plugins/kibana/dashboard/components/panel/lib/panel_utils'; const app = uiModules.get('app/dashboard'); @@ -33,6 +34,18 @@ app.directive('dashboardGrid', function ($compile, Notifier) { // debounced layout function is safe to call as much as possible const safeLayout = _.debounce(layout, 200); + $scope.removePanelFromState = (panelId) => { + _.remove($scope.state.panels, function (panel) { + return panel.panelId === panelId; + }); + }; + + $scope.getPanelByPanelId = (panelId) => { + return _.find($scope.state.panels, function (panel) { + return panel.panelId === panelId; + }); + }; + function init() { $el.addClass('gridster'); @@ -73,12 +86,6 @@ app.directive('dashboardGrid', function ($compile, Notifier) { gridster.enable().enable_resize(); }); - // Does not remove the panel from the ui - that will be handled by the watch below, which - // will be triggered by the removal. - $scope.removePanelFromState = (panel) => { - _.pull($scope.state.panels, panel); - }; - $scope.$watchCollection('state.panels', function (panels) { const currentPanels = gridster.$widgets.toArray().map(function (el) { return getPanelFor(el); @@ -94,7 +101,7 @@ app.directive('dashboardGrid', function ($compile, Notifier) { if (added.length) added.forEach(addPanel); // ensure that every panel can be serialized now that we are done - $state.panels.forEach(makePanelSerializeable); + $state.panels.forEach(PanelUtils.makeSerializeable); // alert interested parties that we have finished processing changes to the panels // TODO: change this from event based to calling a method on dashboardApp @@ -112,7 +119,7 @@ app.directive('dashboardGrid', function ($compile, Notifier) { panel.$el.stop(); removePanel(panel, true); // not that we will, but lets be safe - makePanelSerializeable(panel); + PanelUtils.makeSerializeable(panel); }); }); @@ -125,67 +132,31 @@ app.directive('dashboardGrid', function ($compile, Notifier) { // return the panel object for an element. // // !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! - // ALWAYS CALL makePanelSerializeable AFTER YOU ARE DONE WITH IT + // ALWAYS CALL PanelUtils.makeSerializeable AFTER YOU ARE DONE WITH IT // !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! function getPanelFor(el) { const $panel = el.jquery ? el : $(el); const panel = $panel.data('panel'); - panel.$el = $panel; - // panel.$scope = $panel.data('$scope'); - return panel; } - // since the $el and $scope are circular structures, they need to be - // removed from panel before it can be serialized (we also wouldn't - // want them to show up in the url) - function makePanelSerializeable(panel) { - delete panel.$el; - // delete panel.$scope; - } - // tell gridster to remove the panel, and cleanup our metadata function removePanel(panel, silent) { // remove from grister 'silently' (don't reorganize after) gridster.remove_widget(panel.$el, silent); - - // destroy the scope - // panel.$scope.$destroy(); - panel.$el.removeData('panel'); - panel.$el.removeData('$scope'); } // tell gridster to add the panel, and create additional meatadata like $scope function addPanel(panel) { - _.defaults(panel, { - size_x: 3, - size_y: 2 - }); - - // ignore panels that don't have vis id's - if (!panel.id) { - // In the interest of backwards compat - if (panel.visId) { - panel.id = panel.visId; - panel.type = 'visualization'; - delete panel.visId; - } else { - throw new Error('missing object id on panel'); - } - } - - // panel.$scope = $scope.$new(); - // panel.$scope.panel = panel; - // panel.$scope.parentUiState = $scope.uiState; - const panelIndex = _.indexOf($scope.state.panels, panel); + PanelUtils.initializeDefaults(panel); const panelHtml = `
  • -
  • `; panel.$el = $compile(panelHtml)($scope); @@ -193,21 +164,12 @@ app.directive('dashboardGrid', function ($compile, Notifier) { // tell gridster to use the widget gridster.add_widget(panel.$el, panel.size_x, panel.size_y, panel.col, panel.row); - // update size/col/etc. - refreshPanelStats(panel); + // Gridster may change the position of the widget when adding it, make sure the panel + // contains the latest info. + PanelUtils.refreshSizeAndPosition(panel); - // stash the panel and it's scope in the element's data + // stash the panel in the element's data panel.$el.data('panel', panel); - // panel.$el.data('$scope', panel.$scope); - } - - // ensure that the panel object has the latest size/pos info - function refreshPanelStats(panel) { - const data = panel.$el.coords().grid; - panel.size_x = data.size_x; - panel.size_y = data.size_y; - panel.col = data.col; - panel.row = data.row; } // when gridster tell us it made a change, update each of the panel objects @@ -215,8 +177,8 @@ app.directive('dashboardGrid', function ($compile, Notifier) { // ensure that our panel objects keep their size in sync gridster.$widgets.each(function (i, el) { const panel = getPanelFor(el); - refreshPanelStats(panel); - makePanelSerializeable(panel); + PanelUtils.refreshSizeAndPosition(panel); + PanelUtils.makeSerializeable(panel); $scope.$root.$broadcast('change:vis'); }); } diff --git a/src/core_plugins/kibana/public/dashboard/index.html b/src/core_plugins/kibana/public/dashboard/index.html index e1b29a59c5c7b..767fcfcb7a6bf 100644 --- a/src/core_plugins/kibana/public/dashboard/index.html +++ b/src/core_plugins/kibana/public/dashboard/index.html @@ -10,7 +10,7 @@ > diff --git a/src/core_plugins/kibana/public/dashboard/index.js b/src/core_plugins/kibana/public/dashboard/index.js index 84b6fad27ac0a..5a1eb82559f15 100644 --- a/src/core_plugins/kibana/public/dashboard/index.js +++ b/src/core_plugins/kibana/public/dashboard/index.js @@ -7,7 +7,7 @@ import 'ui/notify'; import 'ui/typeahead'; import 'ui/share'; import 'plugins/kibana/dashboard/directives/grid'; -import 'plugins/kibana/dashboard/components/panel/panel'; +import 'plugins/kibana/dashboard/directives/dashboard_panel_directive'; import 'plugins/kibana/dashboard/services/saved_dashboards'; import 'plugins/kibana/dashboard/styles/main.less'; import FilterBarQueryFilterProvider from 'ui/filter_bar/query_filter'; @@ -17,6 +17,7 @@ import uiRoutes from 'ui/routes'; import uiModules from 'ui/modules'; import indexTemplate from 'plugins/kibana/dashboard/index.html'; import { savedDashboardRegister } from 'plugins/kibana/dashboard/services/saved_dashboard_register'; +import { createNewPanel } from 'plugins/kibana/dashboard/components/panel/lib/panel'; require('ui/saved_objects/saved_object_registry').register(savedDashboardRegister); const app = uiModules.get('app/dashboard', [ @@ -143,15 +144,16 @@ app.directive('dashboardApp', function (Notifier, courier, AppState, timefilter, courier.setRootSearchSource(dash.searchSource); + const docTitle = Private(DocTitleProvider); + function init() { updateQueryOnRootSource(); - const docTitle = Private(DocTitleProvider); if (dash.id) { docTitle.change(dash.title); } - initPanelIndices(); + initPanelIds(); // watch for state changes and update the appStatus.dirty value stateMonitor = stateMonitorFactory.create($state, stateDefaults); @@ -170,24 +172,23 @@ app.directive('dashboardApp', function (Notifier, courier, AppState, timefilter, $scope.$emit('application.load'); } - function initPanelIndices() { - // find the largest panelIndex in all the panels - let maxIndex = getMaxPanelIndex(); + function initPanelIds() { + // find the largest panelId in all the panels + let maxIndex = getMaxPanelId(); - // ensure that all panels have a panelIndex + // ensure that all panels have a panelId $scope.state.panels.forEach(function (panel) { - if (!panel.panelIndex) { - panel.panelIndex = maxIndex++; + if (!panel.panelId) { + panel.panelId = maxIndex++; } }); } - function getMaxPanelIndex() { - let index = $scope.state.panels.reduce(function (idx, panel) { - // if panel is missing an index, add one and increment the index - return Math.max(idx, panel.panelIndex || idx); + function getMaxPanelId() { + let maxId = $scope.state.panels.reduce(function (id, panel) { + return Math.max(id, panel.panelId || id); }, 0); - return ++index; + return ++maxId; } function updateQueryOnRootSource() { @@ -227,7 +228,6 @@ app.directive('dashboardApp', function (Notifier, courier, AppState, timefilter, }; $scope.save = function () { - $state.title = dash.id = dash.title; $state.save(); const timeRestoreObj = _.pick(timefilter.refreshInterval, ['display', 'pause', 'section', 'value']); @@ -246,6 +246,8 @@ app.directive('dashboardApp', function (Notifier, courier, AppState, timefilter, notify.info('Saved Dashboard as "' + dash.title + '"'); if (dash.id !== $routeParams.id) { kbnUrl.change('/dashboard/{{id}}', {id: dash.id}); + } else { + docTitle.change(dash.lastSavedTitle); } } }) @@ -270,12 +272,12 @@ app.directive('dashboardApp', function (Notifier, courier, AppState, timefilter, // called by the saved-object-finder when a user clicks a vis $scope.addVis = function (hit) { pendingVis++; - $state.panels.push({ id: hit.id, type: 'visualization', panelIndex: getMaxPanelIndex() }); + $state.panels.push(createNewPanel(hit.id, 'visualization', getMaxPanelId())); }; $scope.addSearch = function (hit) { pendingVis++; - $state.panels.push({ id: hit.id, type: 'search', panelIndex: getMaxPanelIndex() }); + $state.panels.push(createNewPanel(hit.id, 'search', getMaxPanelId())); }; // Setup configurable values for config directive, after objects are initialized diff --git a/src/core_plugins/kibana/public/dashboard/partials/save_dashboard.html b/src/core_plugins/kibana/public/dashboard/partials/save_dashboard.html index 641aad5c81172..1cd22b45337e7 100644 --- a/src/core_plugins/kibana/public/dashboard/partials/save_dashboard.html +++ b/src/core_plugins/kibana/public/dashboard/partials/save_dashboard.html @@ -2,7 +2,7 @@ role="form" ng-submit="opts.save()" > -
    Save Dashboard
    +
    Save {{opts.dashboard.getDisplayName()}}
    + + +
    - + + diff --git a/src/core_plugins/kibana/public/discover/controllers/discover.js b/src/core_plugins/kibana/public/discover/controllers/discover.js index 52659bf8296ab..2010edfe8dd16 100644 --- a/src/core_plugins/kibana/public/discover/controllers/discover.js +++ b/src/core_plugins/kibana/public/discover/controllers/discover.js @@ -318,7 +318,6 @@ function discoverController($scope, config, courier, $route, $window, Notifier, $scope.opts.saveDataSource = function () { return $scope.updateDataSource() .then(function () { - savedSearch.id = savedSearch.title; savedSearch.columns = $scope.state.columns; savedSearch.sort = $scope.state.sort; @@ -334,6 +333,7 @@ function discoverController($scope, config, courier, $route, $window, Notifier, } else { // Update defaults so that "reload saved query" functions correctly $state.setDefaults(getStateDefaults()); + docTitle.change(savedSearch.lastSavedTitle); } } }); diff --git a/src/core_plugins/kibana/public/discover/index.html b/src/core_plugins/kibana/public/discover/index.html index 0f2faa466bc67..375621208fb43 100644 --- a/src/core_plugins/kibana/public/discover/index.html +++ b/src/core_plugins/kibana/public/discover/index.html @@ -7,7 +7,7 @@
    - +   {{(hits || 0) | number:0}} diff --git a/src/core_plugins/kibana/public/discover/partials/save_search.html b/src/core_plugins/kibana/public/discover/partials/save_search.html index 875c47fa66094..b81723ae70bf3 100644 --- a/src/core_plugins/kibana/public/discover/partials/save_search.html +++ b/src/core_plugins/kibana/public/discover/partials/save_search.html @@ -10,6 +10,8 @@ input-focus="select" placeholder="Name this search..." > + + diff --git a/src/core_plugins/kibana/public/visualize/editor/editor.html b/src/core_plugins/kibana/public/visualize/editor/editor.html index 44d62bb5a3fc2..c9ec9f27c0e84 100644 --- a/src/core_plugins/kibana/public/visualize/editor/editor.html +++ b/src/core_plugins/kibana/public/visualize/editor/editor.html @@ -10,7 +10,7 @@ >
    diff --git a/src/core_plugins/kibana/public/visualize/editor/editor.js b/src/core_plugins/kibana/public/visualize/editor/editor.js index 78905b03597b8..ccb17996dc08b 100644 --- a/src/core_plugins/kibana/public/visualize/editor/editor.js +++ b/src/core_plugins/kibana/public/visualize/editor/editor.js @@ -292,7 +292,6 @@ function VisEditor($scope, $route, timefilter, AppState, $location, kbnUrl, $tim * Called when the user clicks "Save" button. */ $scope.doSave = function () { - savedVis.id = savedVis.title; // vis.title was not bound and it's needed to reflect title into visState $state.vis.title = savedVis.title; savedVis.visState = $state.vis; @@ -305,8 +304,11 @@ function VisEditor($scope, $route, timefilter, AppState, $location, kbnUrl, $tim if (id) { notify.info('Saved Visualization "' + savedVis.title + '"'); - if (savedVis.id === $route.current.params.id) return; - kbnUrl.change('/visualize/edit/{{id}}', {id: savedVis.id}); + if (savedVis.id === $route.current.params.id) { + docTitle.change(savedVis.lastSavedTitle); + } else { + kbnUrl.change('/visualize/edit/{{id}}', {id: savedVis.id}); + } } }, notify.fatal); }; diff --git a/src/core_plugins/kibana/public/visualize/editor/panels/save.html b/src/core_plugins/kibana/public/visualize/editor/panels/save.html index f3a38f5c273bd..9bf4eb534afdf 100644 --- a/src/core_plugins/kibana/public/visualize/editor/panels/save.html +++ b/src/core_plugins/kibana/public/visualize/editor/panels/save.html @@ -11,6 +11,9 @@ ng-model="opts.savedVis.title" required > + + +
    - +
    diff --git a/src/ui/public/autoload/modules.js b/src/ui/public/autoload/modules.js index aae542cf95911..e9b3de0f0eed5 100644 --- a/src/ui/public/autoload/modules.js +++ b/src/ui/public/autoload/modules.js @@ -33,3 +33,4 @@ import 'ui/typeahead'; import 'ui/url'; import 'ui/validate_date_interval'; import 'ui/watch_multi'; +import 'ui/courier/saved_object/ui/saved_object_save_as_checkbox'; diff --git a/src/ui/public/courier/__tests__/saved_object.js b/src/ui/public/courier/__tests__/saved_object.js index 829dc26e35fe1..23adcde6eab50 100644 --- a/src/ui/public/courier/__tests__/saved_object.js +++ b/src/ui/public/courier/__tests__/saved_object.js @@ -5,11 +5,14 @@ import ngMock from 'ng_mock'; import expect from 'expect.js'; import sinon from 'auto-release-sinon'; - import BluebirdPromise from 'bluebird'; + import SavedObjectFactory from '../saved_object/saved_object'; -import { stubMapper } from 'test_utils/stub_mapper'; import IndexPatternFactory from 'ui/index_patterns/_index_pattern'; +import DocSourceProvider from '../data_source/doc_source'; + +import { stubMapper } from 'test_utils/stub_mapper'; + describe('Saved Object', function () { require('test_utils/no_digest_promises').activateForSuite(); @@ -17,6 +20,7 @@ describe('Saved Object', function () { let SavedObject; let IndexPattern; let esStub; + let DocSource; /** * Some default es stubbing to avoid timeouts and allow a default type of 'dashboard'. @@ -35,6 +39,7 @@ describe('Saved Object', function () { // Necessary to avoid a timeout condition. sinon.stub(esStub.indices, 'putMapping').returns(BluebirdPromise.resolve()); + sinon.stub(esStub.indices, 'refresh').returns(BluebirdPromise.resolve()); } /** @@ -83,11 +88,112 @@ describe('Saved Object', function () { SavedObject = Private(SavedObjectFactory); IndexPattern = Private(IndexPatternFactory); esStub = es; + DocSource = Private(DocSourceProvider); mockEsService(); stubMapper(Private); })); + describe('save', function () { + describe(' with copyOnSave', function () { + it('as true creates a copy on save success', function () { + const mockDocResponse = getMockedDocResponse('myId'); + stubESResponse(mockDocResponse); + let newUniqueId; + return createInitializedSavedObject({type: 'dashboard', id: 'myId'}).then(savedObject => { + sinon.stub(DocSource.prototype, 'doIndex', function () { + newUniqueId = savedObject.id; + expect(newUniqueId).to.not.be('myId'); + mockDocResponse._id = newUniqueId; + return BluebirdPromise.resolve(newUniqueId); + }); + savedObject.copyOnSave = true; + return savedObject.save() + .then((id) => { + expect(id).to.be(newUniqueId); + }); + }); + }); + + it('as true does not create a copy when save fails', function () { + const mockDocResponse = getMockedDocResponse('myId'); + stubESResponse(mockDocResponse); + let originalId = 'id1'; + return createInitializedSavedObject({type: 'dashboard', id: originalId}).then(savedObject => { + sinon.stub(DocSource.prototype, 'doIndex', function () { + return BluebirdPromise.reject('simulated error'); + }); + savedObject.copyOnSave = true; + return savedObject.save().then(() => { + throw new Error('Expected a rejection'); + }).catch(() => { + expect(savedObject.id).to.be(originalId); + }); + }); + }); + + it('as false does not create a copy', function () { + const mockDocResponse = getMockedDocResponse('myId'); + stubESResponse(mockDocResponse); + const id = 'myId'; + return createInitializedSavedObject({type: 'dashboard', id: id}).then(savedObject => { + sinon.stub(DocSource.prototype, 'doIndex', function () { + expect(savedObject.id).to.be(id); + return BluebirdPromise.resolve(id); + }); + savedObject.copyOnSave = false; + return savedObject.save() + .then((id) => { + expect(id).to.be(id); + }); + }); + }); + }); + + it('returns id from server on success', function () { + return createInitializedSavedObject({type: 'dashboard'}).then(savedObject => { + const mockDocResponse = getMockedDocResponse('myId'); + stubESResponse(mockDocResponse); + return savedObject.save() + .then((id) => { + expect(id).to.be('myId'); + }); + }); + }); + + describe('updates isSaving variable', function () { + it('on success', function () { + let id = 'id'; + stubESResponse(getMockedDocResponse(id)); + return createInitializedSavedObject({type: 'dashboard', id: id}).then(savedObject => { + sinon.stub(DocSource.prototype, 'doIndex', () => { + expect(savedObject.isSaving).to.be(true); + return BluebirdPromise.resolve(id); + }); + expect(savedObject.isSaving).to.be(false); + return savedObject.save() + .then((id) => { + expect(savedObject.isSaving).to.be(false); + }); + }); + }); + + it('on failure', function () { + return createInitializedSavedObject({type: 'dashboard'}).then(savedObject => { + sinon.stub(DocSource.prototype, 'doIndex', () => { + expect(savedObject.isSaving).to.be(true); + return BluebirdPromise.reject(); + }); + expect(savedObject.isSaving).to.be(false); + return savedObject.save() + .catch(() => { + expect(savedObject.isSaving).to.be(false); + }); + }); + }); + }); + }); + describe('applyESResp', function () { it('throws error if not found', function () { return createInitializedSavedObject({ type: 'dashboard' }).then(savedObject => { diff --git a/src/ui/public/courier/data_source/_normalize_sort_request.js b/src/ui/public/courier/data_source/_normalize_sort_request.js index 7bab633bf1d4c..beb9f3f6effa8 100644 --- a/src/ui/public/courier/data_source/_normalize_sort_request.js +++ b/src/ui/public/courier/data_source/_normalize_sort_request.js @@ -38,7 +38,7 @@ export default function normalizeSortRequest(config) { inline: indexField.script, lang: indexField.lang }, - type: indexField.type, + type: castSortType(indexField.type), order: direction }; } else { @@ -56,3 +56,20 @@ export default function normalizeSortRequest(config) { return normalized; } }; + +// The ES API only supports sort scripts of type 'number' and 'string' +function castSortType(type) { + const typeCastings = { + number: 'number', + string: 'string', + date: 'number', + boolean: 'string' + }; + + const castedType = typeCastings[type]; + if (!castedType) { + throw new Error(`Unsupported script sort type: ${type}`); + } + + return castedType; +} diff --git a/src/ui/public/courier/saved_object/saved_object.js b/src/ui/public/courier/saved_object/saved_object.js index 8cfe0fe1bb39f..38d53eb12e625 100644 --- a/src/ui/public/courier/saved_object/saved_object.js +++ b/src/ui/public/courier/saved_object/saved_object.js @@ -13,7 +13,7 @@ import angular from 'angular'; import _ from 'lodash'; import errors from 'ui/errors'; -import slugifyId from 'ui/utils/slugify_id'; +import uuid from 'node-uuid'; import MappingSetupProvider from 'ui/utils/mapping_setup'; import DocSourceProvider from '../data_source/doc_source'; @@ -38,7 +38,19 @@ export default function SavedObjectFactory(es, kbnIndex, Promise, Private, Notif let docSource = new DocSource(); // type name for this object, used as the ES-type - let type = config.type; + const type = config.type; + + self.getDisplayName = function () { + return type; + }; + + /** + * Flips to true during a save operation, and back to false once the save operation + * completes. + * @type {boolean} + */ + self.isSaving = false; + self.defaults = config.defaults || {}; // Create a notifier for sending alerts let notify = new Notifier({ @@ -48,18 +60,18 @@ export default function SavedObjectFactory(es, kbnIndex, Promise, Private, Notif // mapping definition for the fields that this object will expose let mapping = mappingSetup.expandShorthand(config.mapping); - // default field values, assigned when the source is loaded - let defaults = config.defaults || {}; - let afterESResp = config.afterESResp || _.noop; let customInit = config.init || _.noop; // optional search source which this object configures - self.searchSource = config.searchSource && new SearchSource(); + self.searchSource = config.searchSource ? new SearchSource() : undefined; // the id of the document self.id = config.id || void 0; - self.defaults = config.defaults; + + // Whether to create a copy when the object is saved. This should eventually go away + // in favor of a better rename/save flow. + self.copyOnSave = false; /** * Asynchronously initialize this object - will only run @@ -74,9 +86,9 @@ export default function SavedObjectFactory(es, kbnIndex, Promise, Private, Notif // tell the docSource where to find the doc docSource - .index(kbnIndex) - .type(type) - .id(self.id); + .index(kbnIndex) + .type(type) + .id(self.id); // check that the mapping for this type is defined return mappingSetup.isDefined(type) @@ -100,15 +112,14 @@ export default function SavedObjectFactory(es, kbnIndex, Promise, Private, Notif // If there is not id, then there is no document to fetch from elasticsearch if (!self.id) { // just assign the defaults and be done - _.assign(self, defaults); - return hydrateIndexPattern().then(function () { + _.assign(self, self.defaults); + return hydrateIndexPattern().then(() => { return afterESResp.call(self); }); } // fetch the object from ES - return docSource.fetch() - .then(self.applyESResp); + return docSource.fetch().then(self.applyESResp); }) .then(function () { return customInit.call(self); @@ -133,7 +144,7 @@ export default function SavedObjectFactory(es, kbnIndex, Promise, Private, Notif } // assign the defaults to the response - _.defaults(self._source, defaults); + _.defaults(self._source, self.defaults); // transform the source using _deserializers _.forOwn(mapping, function ittr(fieldMapping, fieldName) { @@ -144,15 +155,16 @@ export default function SavedObjectFactory(es, kbnIndex, Promise, Private, Notif // Give obj all of the values in _source.fields _.assign(self, self._source); + self.lastSavedTitle = self.title; - return Promise.try(function () { + return Promise.try(() => { parseSearchSource(meta.searchSourceJSON); + return hydrateIndexPattern(); }) - .then(hydrateIndexPattern) - .then(function () { + .then(() => { return Promise.cast(afterESResp.call(self, resp)); }) - .then(function () { + .then(() => { // Any time obj is updated, re-call applyESResp docSource.onUpdate().then(self.applyESResp, notify.fatal); }); @@ -181,28 +193,31 @@ export default function SavedObjectFactory(es, kbnIndex, Promise, Private, Notif * After creation or fetching from ES, ensure that the searchSources index indexPattern * is an bonafide IndexPattern object. * - * @return {[type]} [description] + * @return {Promise} */ function hydrateIndexPattern() { - return Promise.try(function () { - if (self.searchSource) { - - let index = config.indexPattern || self.searchSource.getOwn('index'); - if (!index) return; - if (config.clearSavedIndexPattern) { - self.searchSource.set('index', undefined); - return; - } + if (!self.searchSource) { return Promise.resolve(null); } - if (!(index instanceof indexPatterns.IndexPattern)) { - index = indexPatterns.get(index); - } + if (config.clearSavedIndexPattern) { + self.searchSource.set('index', undefined); + return Promise.resolve(null); + } - return Promise.resolve(index).then(function (indexPattern) { - self.searchSource.set('index', indexPattern); - }); - } - }); + let index = config.indexPattern || self.searchSource.getOwn('index'); + + if (!index) { return Promise.resolve(null); } + + // If index is not an IndexPattern object at this point, then it's a string id of an index. + if (!(index instanceof indexPatterns.IndexPattern)) { + index = indexPatterns.get(index); + } + + // At this point index will either be an IndexPattern, if cached, or a promise that + // will return an IndexPattern, if not cached. + return Promise.resolve(index) + .then((indexPattern) => { + self.searchSource.set('index', indexPattern); + }); } /** @@ -231,59 +246,55 @@ export default function SavedObjectFactory(es, kbnIndex, Promise, Private, Notif }; /** - * Save this object + * Returns true if the object's original title has been changed. New objects return false. + * @return {boolean} + */ + self.isTitleChanged = function () { + return self._source && self._source.title !== self.title; + }; + + /** + * Saves this object. * * @return {Promise} * @resolved {String} - The id of the doc */ self.save = function () { + // Save the original id in case the save fails. + let originalId = self.id; + // Read https://github.com/elastic/kibana/issues/9056 and + // https://github.com/elastic/kibana/issues/9012 for some background into why this copyOnSave variable + // exists. + // The goal is to move towards a better rename flow, but since our users have been conditioned + // to expect a 'save as' flow during a rename, we are keeping the logic the same until a better + // UI/UX can be worked out. + if (this.copyOnSave) { + self.id = null; + } - let body = self.serialize(); - - // Slugify the object id - self.id = slugifyId(self.id); - - // ensure that the docSource has the current self.id + // Create a unique id for this object if it doesn't have one already. + self.id = this.id || uuid.v1(); + // ensure that the docSource has the current id docSource.id(self.id); - // index the document - return self.saveSource(body); - }; + let source = self.serialize(); - self.saveSource = function (source) { - let finish = function (id) { - self.id = id; - return es.indices.refresh({ - index: kbnIndex - }) - .then(function () { + self.isSaving = true; + return docSource.doIndex(source) + .then((id) => { self.id = id; }) + .then(self.refreshIndex) + .then(() => { + self.isSaving = false; + self.lastSavedTitle = self.title; return self.id; + }) + .catch(function (err) { + self.isSaving = false; + self.id = originalId; + return Promise.reject(err); }); - }; - - return docSource.doCreate(source) - .then(finish) - .catch(function (err) { - // record exists, confirm overwriting - if (_.get(err, 'origError.status') === 409) { - let confirmMessage = 'Are you sure you want to overwrite ' + self.title + '?'; - - return safeConfirm(confirmMessage).then( - function () { - return docSource.doIndex(source).then(finish); - }, - _.noop // if the user doesn't overwrite record, just swallow the error - ); - } - return Promise.reject(err); - }); }; - /** - * Destroy this object - * - * @return {undefined} - */ self.destroy = function () { docSource.cancelQueued(); if (self.searchSource) { @@ -291,20 +302,26 @@ export default function SavedObjectFactory(es, kbnIndex, Promise, Private, Notif } }; + /** + * Queries es to refresh the index. + * @returns {Promise} + */ + self.refreshIndex = function () { + return es.indices.refresh({ index: kbnIndex }); + }; + /** * Delete this object from Elasticsearch * @return {promise} */ self.delete = function () { - return es.delete({ - index: kbnIndex, - type: type, - id: this.id - }).then(function () { - return es.indices.refresh({ - index: kbnIndex - }); - }); + return es.delete( + { + index: kbnIndex, + type: type, + id: this.id + }) + .then(() => { return this.refreshIndex(); }); }; } diff --git a/src/ui/public/courier/saved_object/ui/saved_object_save_as_checkbox.html b/src/ui/public/courier/saved_object/ui/saved_object_save_as_checkbox.html new file mode 100644 index 0000000000000..466afdf55f9c4 --- /dev/null +++ b/src/ui/public/courier/saved_object/ui/saved_object_save_as_checkbox.html @@ -0,0 +1,9 @@ +
    +
    + In previous versions of Kibana, changing the name of a {{savedObject.getDisplayName()}} would make a copy with the new name. Use the 'Save as a new {{savedObject.getDisplayName()}}' checkbox to do this now. +
    + +
    diff --git a/src/ui/public/courier/saved_object/ui/saved_object_save_as_checkbox.js b/src/ui/public/courier/saved_object/ui/saved_object_save_as_checkbox.js new file mode 100644 index 0000000000000..5d159fbf234a6 --- /dev/null +++ b/src/ui/public/courier/saved_object/ui/saved_object_save_as_checkbox.js @@ -0,0 +1,14 @@ +import uiModules from 'ui/modules'; +import saveObjectSaveAsCheckboxTemplate from './saved_object_save_as_checkbox.html'; + +uiModules + .get('kibana') + .directive('savedObjectSaveAsCheckBox', function () { + return { + restrict: 'E', + template: saveObjectSaveAsCheckboxTemplate, + scope: { + savedObject: '=' + } + }; + }); diff --git a/src/ui/public/styles/input.less b/src/ui/public/styles/input.less index 0503b3933fd34..7d3a4a19b7cbd 100644 --- a/src/ui/public/styles/input.less +++ b/src/ui/public/styles/input.less @@ -12,3 +12,4 @@ select { color: @input-color; background-color: @input-bg; } + diff --git a/src/ui/public/utils/__tests__/slugify_id.js b/src/ui/public/utils/__tests__/slugify_id.js deleted file mode 100644 index 57f7f2181501b..0000000000000 --- a/src/ui/public/utils/__tests__/slugify_id.js +++ /dev/null @@ -1,42 +0,0 @@ -import _ from 'lodash'; -import slugifyId from 'ui/utils/slugify_id'; -import expect from 'expect.js'; - -describe('slugifyId()', function () { - - let fixtures = [ - ['test/test', 'test-slash-test'], - ['test?test', 'test-questionmark-test'], - ['test=test', 'test-equal-test'], - ['test&test', 'test-ampersand-test'], - ['test%test', 'test-percent-test'], - ['test / test', 'test-slash-test'], - ['test ? test', 'test-questionmark-test'], - ['test = test', 'test-equal-test'], - ['test & test', 'test-ampersand-test'], - ['test % test', 'test-percent-test'], - ['test / ^test', 'test-slash-^test'], - ['test ? test', 'test-questionmark-test'], - ['test = test', 'test-equal-test'], - ['test & test', 'test-ampersand-test'], - ['test % test', 'test-percent-test'], - ['test/test/test', 'test-slash-test-slash-test'], - ['test?test?test', 'test-questionmark-test-questionmark-test'], - ['test&test&test', 'test-ampersand-test-ampersand-test'], - ['test=test=test', 'test-equal-test-equal-test'], - ['test%test%test', 'test-percent-test-percent-test'] - ]; - - _.each(fixtures, function (fixture) { - let msg = 'should convert ' + fixture[0] + ' to ' + fixture[1]; - it(msg, function () { - let results = slugifyId(fixture[0]); - expect(results).to.be(fixture[1]); - }); - }); - - it('should do nothing if the id is undefined', function () { - expect(slugifyId(undefined)).to.be(undefined); - }); - -}); diff --git a/src/ui/public/utils/slugify_id.js b/src/ui/public/utils/slugify_id.js deleted file mode 100644 index 0894d9356593e..0000000000000 --- a/src/ui/public/utils/slugify_id.js +++ /dev/null @@ -1,19 +0,0 @@ -import _ from 'lodash'; -export default function (id) { - if (id == null) return; - - let trans = { - '/' : '-slash-', - '\\?' : '-questionmark-', - '\\&' : '-ampersand-', - '=' : '-equal-', - '%' : '-percent-' - }; - _.each(trans, function (val, key) { - let regex = new RegExp(key, 'g'); - id = id.replace(regex, val); - }); - id = id.replace(/[\s]+/g, '-'); - id = id.replace(/[\-]+/g, '-'); - return id; -}; diff --git a/src/ui/settings/__tests__/index.js b/src/ui/settings/__tests__/index.js index 344552abdce81..d2ffc25764641 100644 --- a/src/ui/settings/__tests__/index.js +++ b/src/ui/settings/__tests__/index.js @@ -158,6 +158,14 @@ describe('ui settings', function () { })).to.equal(true); }); + it('returns an empty object when status is not green', async function () { + const { uiSettings, req } = instantiate({ + settingsStatusOverrides: { state: 'yellow' } + }); + + expect(await uiSettings.getUserProvided(req)).to.eql({}); + }); + it('returns an empty object on 404 responses', async function () { const { uiSettings, req } = instantiate({ async callWithRequest() { @@ -361,7 +369,7 @@ function expectElasticsearchUpdateQuery(server, req, configGet, doc) { }); } -function instantiate({ getResult, callWithRequest } = {}) { +function instantiate({ getResult, callWithRequest, settingsStatusOverrides } = {}) { const esStatus = { state: 'green', on: sinon.spy() @@ -370,7 +378,8 @@ function instantiate({ getResult, callWithRequest } = {}) { state: 'green', red: sinon.spy(), yellow: sinon.spy(), - green: sinon.spy() + green: sinon.spy(), + ...settingsStatusOverrides }; const kbnServer = { status: { diff --git a/src/ui/settings/index.js b/src/ui/settings/index.js index 29f5959d67038..986f2f8428fd9 100644 --- a/src/ui/settings/index.js +++ b/src/ui/settings/index.js @@ -68,6 +68,13 @@ export default function setupSettings(kbnServer, server, config) { assertRequest(req); const { callWithRequest, errors } = server.plugins.elasticsearch; + // If the ui settings status isn't green, we shouldn't be attempting to get + // user settings, since we can't be sure that all the necessary conditions + // (e.g. elasticsearch being available) are met. + if (status.state !== 'green') { + return hydrateUserSettings({}); + } + const params = getClientSettings(config); const allowedErrors = [errors[404], errors[403], errors.NoConnections]; if (ignore401Errors) allowedErrors.push(errors[401]); diff --git a/src/ui/views/chrome.jade b/src/ui/views/chrome.jade index 2163ebd650499..542f2b0f10797 100644 --- a/src/ui/views/chrome.jade +++ b/src/ui/views/chrome.jade @@ -92,7 +92,7 @@ html(lang='en') -webkit-justify-content: center; -ms-flex-pack: center; justify-content: center; - background: #5A5A5A; + background: #FFFFFF; } .kibanaWelcomeLogo { diff --git a/src/ui/views/ui_app.jade b/src/ui/views/ui_app.jade index 975dec6ecd77a..83541bf027825 100644 --- a/src/ui/views/ui_app.jade +++ b/src/ui/views/ui_app.jade @@ -27,7 +27,6 @@ block content margin-top: 130px; /* 1 */ text-align: center; background: #3caed2; - box-shadow: 0 0 2em #444444; } .kibanaLoader__logo { @@ -100,7 +99,7 @@ block content */ .kibanaLoadingMessage { font-family: "Open Sans", Helvetica, Arial, sans-serif; - color: white; + color: #8c8c8c; max-width: 540px; height: 50px; /* 1 */ margin-top: 80px; /* 1 */ diff --git a/tasks/build/index.js b/tasks/build/index.js index 1fa50df44073c..84eb30a21c0c1 100644 --- a/tasks/build/index.js +++ b/tasks/build/index.js @@ -18,8 +18,6 @@ module.exports = function (grunt) { '_build:installNpmDeps', '_build:removePkgJsonDeps', 'clean:testsFromModules', - 'clean:deepModuleBins', - 'clean:deepModules', 'run:optimizeBuild', 'stop:optimizeBuild', '_build:versionedLinks', diff --git a/tasks/build/package_json.js b/tasks/build/package_json.js index a9038a2dd6d5b..2b58a4353f942 100644 --- a/tasks/build/package_json.js +++ b/tasks/build/package_json.js @@ -1,8 +1,5 @@ module.exports = function (grunt) { - let { defaults } = require('lodash'); - let pkg = grunt.config.get('pkg'); - let deepModules = grunt.config.get('deepModules'); grunt.registerTask('_build:packageJson', function () { const { sha, number, version } = grunt.config.get('build'); @@ -22,7 +19,7 @@ module.exports = function (grunt) { engines: { node: pkg.engines.node }, - dependencies: defaults({}, pkg.dependencies, deepModules) + dependencies: pkg.dependencies }, null, ' ') ); }); diff --git a/tasks/config/clean.js b/tasks/config/clean.js index 4128505fd189e..096ffae86cdf9 100644 --- a/tasks/config/clean.js +++ b/tasks/config/clean.js @@ -1,11 +1,8 @@ module.exports = function (grunt) { - let modules = Object.keys(grunt.config.get('deepModules')); return { build: 'build', target: 'target', screenshots: 'test/screenshots/session', testsFromModules: 'build/kibana/node_modules/**/{test,tests}/**', - deepModuleBins: 'build/kibana/node_modules/*/node_modules/**/.bin/{' + modules.join(',') + '}', - deepModules: 'build/kibana/node_modules/*/node_modules/**/{' + modules.join(',') + '}/', }; }; From 3a09d660f0807d9c28404a014a4a6c7b7a615da5 Mon Sep 17 00:00:00 2001 From: Stacey Gammon Date: Fri, 2 Dec 2016 10:31:09 -0500 Subject: [PATCH 3/7] namespace panel factory cleanup --- .../dashboard/components/panel/lib/panel.js | 23 +++++++++++-------- .../public/dashboard/directives/grid.js | 6 +++++ .../kibana/public/dashboard/index.js | 6 ++--- 3 files changed, 22 insertions(+), 13 deletions(-) diff --git a/src/core_plugins/kibana/public/dashboard/components/panel/lib/panel.js b/src/core_plugins/kibana/public/dashboard/components/panel/lib/panel.js index 07aad05710086..38e08660a7fb7 100644 --- a/src/core_plugins/kibana/public/dashboard/components/panel/lib/panel.js +++ b/src/core_plugins/kibana/public/dashboard/components/panel/lib/panel.js @@ -61,14 +61,17 @@ function Panel(id, type, panelId) { this.type = type; } -/** - * Factory function to create a panel object. - * - * @param id {string} - The id of the visualization this panel contains - * @param type {string} - The type of visualization this panel contains - * @param panelId {number} - A unique identifier for this panel in the grid - * @returns {Panel} - */ -export function createNewPanel(id, type, panelId) { - return new Panel(id, type, panelId); + +export class PanelFactory { + /** + * Factory function to create a panel object. + * + * @param id {string} - The id of the visualization this panel contains + * @param type {string} - The type of visualization this panel contains + * @param panelId {number} - A unique identifier for this panel in the grid + * @returns {Panel} + */ + static create(id, type, panelId) { + return new Panel(id, type, panelId); + } } diff --git a/src/core_plugins/kibana/public/dashboard/directives/grid.js b/src/core_plugins/kibana/public/dashboard/directives/grid.js index 42c6296bc2130..eef9f69b89d25 100644 --- a/src/core_plugins/kibana/public/dashboard/directives/grid.js +++ b/src/core_plugins/kibana/public/dashboard/directives/grid.js @@ -40,6 +40,12 @@ app.directive('dashboardGrid', function ($compile, Notifier) { }); }; + /** + * Removes the panel with the given id from the $scope.state.panels array. Does not + * remove the ui element from gridster - that is triggered by a watcher that is + * triggered on changes made to $scope.state.panels. + * @param panelId {number} + */ $scope.getPanelByPanelId = (panelId) => { return _.find($scope.state.panels, function (panel) { return panel.panelId === panelId; diff --git a/src/core_plugins/kibana/public/dashboard/index.js b/src/core_plugins/kibana/public/dashboard/index.js index 5a1eb82559f15..08a6cab3de0da 100644 --- a/src/core_plugins/kibana/public/dashboard/index.js +++ b/src/core_plugins/kibana/public/dashboard/index.js @@ -17,7 +17,7 @@ import uiRoutes from 'ui/routes'; import uiModules from 'ui/modules'; import indexTemplate from 'plugins/kibana/dashboard/index.html'; import { savedDashboardRegister } from 'plugins/kibana/dashboard/services/saved_dashboard_register'; -import { createNewPanel } from 'plugins/kibana/dashboard/components/panel/lib/panel'; +import { PanelFactory } from 'plugins/kibana/dashboard/components/panel/lib/panel'; require('ui/saved_objects/saved_object_registry').register(savedDashboardRegister); const app = uiModules.get('app/dashboard', [ @@ -272,12 +272,12 @@ app.directive('dashboardApp', function (Notifier, courier, AppState, timefilter, // called by the saved-object-finder when a user clicks a vis $scope.addVis = function (hit) { pendingVis++; - $state.panels.push(createNewPanel(hit.id, 'visualization', getMaxPanelId())); + $state.panels.push(PanelFactory.create(hit.id, 'visualization', getMaxPanelId())); }; $scope.addSearch = function (hit) { pendingVis++; - $state.panels.push(createNewPanel(hit.id, 'search', getMaxPanelId())); + $state.panels.push(PanelFactory.create(hit.id, 'search', getMaxPanelId())); }; // Setup configurable values for config directive, after objects are initialized From 93afafa81098e5248844b046051035fc0c822b5f Mon Sep 17 00:00:00 2001 From: Stacey Gammon Date: Tue, 6 Dec 2016 09:25:51 -0500 Subject: [PATCH 4/7] Fix parameter name in html --- src/core_plugins/kibana/public/dashboard/directives/grid.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core_plugins/kibana/public/dashboard/directives/grid.js b/src/core_plugins/kibana/public/dashboard/directives/grid.js index eef9f69b89d25..d01b787ce47a6 100644 --- a/src/core_plugins/kibana/public/dashboard/directives/grid.js +++ b/src/core_plugins/kibana/public/dashboard/directives/grid.js @@ -162,7 +162,7 @@ app.directive('dashboardGrid', function ($compile, Notifier) {
  • `; panel.$el = $compile(panelHtml)($scope); From aa8d6c8056e80cce8cec572f1dd707cb77e7852c Mon Sep 17 00:00:00 2001 From: Stacey Gammon Date: Tue, 6 Dec 2016 10:27:58 -0500 Subject: [PATCH 5/7] Address comments - Get rid of factory function and Panel class. - Rename panel.js to panel_state.js - Rename dashboard_panel_directive to dashboard_panel --- .../dashboard/components/panel/lib/panel.js | 77 ------------------- .../components/panel/lib/panel_state.js | 35 +++++++++ .../components/panel/lib/panel_utils.js | 2 +- ..._panel_directive.js => dashboard_panel.js} | 4 +- .../kibana/public/dashboard/index.js | 8 +- 5 files changed, 42 insertions(+), 84 deletions(-) delete mode 100644 src/core_plugins/kibana/public/dashboard/components/panel/lib/panel.js create mode 100644 src/core_plugins/kibana/public/dashboard/components/panel/lib/panel_state.js rename src/core_plugins/kibana/public/dashboard/directives/{dashboard_panel_directive.js => dashboard_panel.js} (98%) diff --git a/src/core_plugins/kibana/public/dashboard/components/panel/lib/panel.js b/src/core_plugins/kibana/public/dashboard/components/panel/lib/panel.js deleted file mode 100644 index 38e08660a7fb7..0000000000000 --- a/src/core_plugins/kibana/public/dashboard/components/panel/lib/panel.js +++ /dev/null @@ -1,77 +0,0 @@ -export const DEFAULT_PANEL_WIDTH = 3; -export const DEFAULT_PANEL_HEIGHT = 2; - -/** - * Represents a panel on a grid. Keeps track of position in the grid and what visualization it - * contains. - * - * @param id - Id of the visualization contained in the panel. - * @param type - Type of the visualization in the panel. - * @param panelId - Unique id to represent this panel in the grid. - * @constructor - */ -function Panel(id, type, panelId) { - /** - * A reference to the gridster widget holding this panel. Used to - * update the size and column attributes. - */ - this.$el; - - /** - * Width of the panel. - * @type {number} - */ - this.size_x = DEFAULT_PANEL_WIDTH; - - /** - * Height of the panel. - * @type {number} - */ - this.size_y = DEFAULT_PANEL_HEIGHT; - - /** - * Column index in the grid. - * @type {number} - */ - this.col; - - /** - * Row index of the panel in the grid. - * @type {number} - */ - this.row; - - /** - * Height of the panel. - * @type {number} - */ - this.id = id; - - /** - * Unique identifier for this panel. Guaranteed to be unique among the current - * panels in the grid. - * @type {number} - */ - this.panelId = panelId; - - /** - * Type of visualization this panel contains. - * @type {string} - */ - this.type = type; -} - - -export class PanelFactory { - /** - * Factory function to create a panel object. - * - * @param id {string} - The id of the visualization this panel contains - * @param type {string} - The type of visualization this panel contains - * @param panelId {number} - A unique identifier for this panel in the grid - * @returns {Panel} - */ - static create(id, type, panelId) { - return new Panel(id, type, panelId); - } -} diff --git a/src/core_plugins/kibana/public/dashboard/components/panel/lib/panel_state.js b/src/core_plugins/kibana/public/dashboard/components/panel/lib/panel_state.js new file mode 100644 index 0000000000000..f3b9c137adc51 --- /dev/null +++ b/src/core_plugins/kibana/public/dashboard/components/panel/lib/panel_state.js @@ -0,0 +1,35 @@ +export const DEFAULT_PANEL_WIDTH = 3; +export const DEFAULT_PANEL_HEIGHT = 2; + +/** + * Represents a panel on a grid. Keeps track of position in the grid and what visualization it + * contains. + * + * @typedef PanelState + * @property {number} id - Id of the visualization contained in the panel. + * @property {Element} $el - A reference to the gridster widget holding this panel. Used to + * update the size and column attributes. TODO: move out of panel state as this couples state to ui. + * @property {string} type - Type of the visualization in the panel. + * @property {number} panelId - Unique id to represent this panel in the grid. + * @property {number} size_x - Width of the panel. + * @property {number} size_y - Height of the panel. + * @property {number} col - Column index in the grid. + * @property {number} row - Row index in the grid. + */ + +/** + * Creates and initializes a basic panel state. + * @param {number} id + * @param {string} type + * @param {number} panelId + * @return {PanelState} + */ +export function createPanelState(id, type, panelId) { + return { + size_x: DEFAULT_PANEL_WIDTH, + size_y: DEFAULT_PANEL_HEIGHT, + panelId: panelId, + type: type, + id: id + }; +} diff --git a/src/core_plugins/kibana/public/dashboard/components/panel/lib/panel_utils.js b/src/core_plugins/kibana/public/dashboard/components/panel/lib/panel_utils.js index dc2a80f25e931..2b789179ba322 100644 --- a/src/core_plugins/kibana/public/dashboard/components/panel/lib/panel_utils.js +++ b/src/core_plugins/kibana/public/dashboard/components/panel/lib/panel_utils.js @@ -1,4 +1,4 @@ -import { DEFAULT_PANEL_WIDTH, DEFAULT_PANEL_HEIGHT } from 'plugins/kibana/dashboard/components/panel/lib/panel'; +import { DEFAULT_PANEL_WIDTH, DEFAULT_PANEL_HEIGHT } from 'plugins/kibana/dashboard/components/panel/lib/panel_state'; export class PanelUtils { /** diff --git a/src/core_plugins/kibana/public/dashboard/directives/dashboard_panel_directive.js b/src/core_plugins/kibana/public/dashboard/directives/dashboard_panel.js similarity index 98% rename from src/core_plugins/kibana/public/dashboard/directives/dashboard_panel_directive.js rename to src/core_plugins/kibana/public/dashboard/directives/dashboard_panel.js index bc0abc9799883..ed1736349991d 100644 --- a/src/core_plugins/kibana/public/dashboard/directives/dashboard_panel_directive.js +++ b/src/core_plugins/kibana/public/dashboard/directives/dashboard_panel.js @@ -22,7 +22,7 @@ uiModules /** * Returns a unique id for storing the panel state in the persistent ui. - * @param panel + * @param {PanelState} panel * @returns {string} */ const getPersistedStateId = function (panel) { @@ -46,7 +46,7 @@ uiModules parentUiState: '=', /** * Contains information about this panel. - * @type {Panel} + * @type {PanelState} */ panel: '=', /** diff --git a/src/core_plugins/kibana/public/dashboard/index.js b/src/core_plugins/kibana/public/dashboard/index.js index 08a6cab3de0da..607633f747aff 100644 --- a/src/core_plugins/kibana/public/dashboard/index.js +++ b/src/core_plugins/kibana/public/dashboard/index.js @@ -7,7 +7,7 @@ import 'ui/notify'; import 'ui/typeahead'; import 'ui/share'; import 'plugins/kibana/dashboard/directives/grid'; -import 'plugins/kibana/dashboard/directives/dashboard_panel_directive'; +import 'plugins/kibana/dashboard/directives/dashboard_panel'; import 'plugins/kibana/dashboard/services/saved_dashboards'; import 'plugins/kibana/dashboard/styles/main.less'; import FilterBarQueryFilterProvider from 'ui/filter_bar/query_filter'; @@ -17,7 +17,7 @@ import uiRoutes from 'ui/routes'; import uiModules from 'ui/modules'; import indexTemplate from 'plugins/kibana/dashboard/index.html'; import { savedDashboardRegister } from 'plugins/kibana/dashboard/services/saved_dashboard_register'; -import { PanelFactory } from 'plugins/kibana/dashboard/components/panel/lib/panel'; +import { createPanelState } from 'plugins/kibana/dashboard/components/panel/lib/panel_state'; require('ui/saved_objects/saved_object_registry').register(savedDashboardRegister); const app = uiModules.get('app/dashboard', [ @@ -272,12 +272,12 @@ app.directive('dashboardApp', function (Notifier, courier, AppState, timefilter, // called by the saved-object-finder when a user clicks a vis $scope.addVis = function (hit) { pendingVis++; - $state.panels.push(PanelFactory.create(hit.id, 'visualization', getMaxPanelId())); + $state.panels.push(createPanelState(hit.id, 'visualization', getMaxPanelId())); }; $scope.addSearch = function (hit) { pendingVis++; - $state.panels.push(PanelFactory.create(hit.id, 'search', getMaxPanelId())); + $state.panels.push(createPanelState(hit.id, 'search', getMaxPanelId())); }; // Setup configurable values for config directive, after objects are initialized From 317e76eee53da48cd88566e3fe6ba29f9a966457 Mon Sep 17 00:00:00 2001 From: Stacey Gammon Date: Tue, 6 Dec 2016 13:11:42 -0500 Subject: [PATCH 6/7] Fix file path reference in tests. --- .../kibana/public/dashboard/__tests__/dashboard_panels.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core_plugins/kibana/public/dashboard/__tests__/dashboard_panels.js b/src/core_plugins/kibana/public/dashboard/__tests__/dashboard_panels.js index 7c685e2bdb488..fd7d982cd7194 100644 --- a/src/core_plugins/kibana/public/dashboard/__tests__/dashboard_panels.js +++ b/src/core_plugins/kibana/public/dashboard/__tests__/dashboard_panels.js @@ -2,7 +2,7 @@ import angular from 'angular'; import expect from 'expect.js'; import ngMock from 'ng_mock'; import 'plugins/kibana/dashboard/services/_saved_dashboard'; -import { DEFAULT_PANEL_WIDTH, DEFAULT_PANEL_HEIGHT } from '../components/panel/lib/panel'; +import { DEFAULT_PANEL_WIDTH, DEFAULT_PANEL_HEIGHT } from '../components/panel/lib/panel_state'; describe('dashboard panels', function () { let $scope; From c87f45b192f1060c924a0fb3045f0c308e06f8f5 Mon Sep 17 00:00:00 2001 From: Stacey Gammon Date: Wed, 7 Dec 2016 09:32:08 -0500 Subject: [PATCH 7/7] Panel => PanelState --- .../public/dashboard/components/panel/lib/panel_state.js | 2 +- .../public/dashboard/components/panel/lib/panel_utils.js | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/core_plugins/kibana/public/dashboard/components/panel/lib/panel_state.js b/src/core_plugins/kibana/public/dashboard/components/panel/lib/panel_state.js index f3b9c137adc51..55d58c88d7e21 100644 --- a/src/core_plugins/kibana/public/dashboard/components/panel/lib/panel_state.js +++ b/src/core_plugins/kibana/public/dashboard/components/panel/lib/panel_state.js @@ -5,7 +5,7 @@ export const DEFAULT_PANEL_HEIGHT = 2; * Represents a panel on a grid. Keeps track of position in the grid and what visualization it * contains. * - * @typedef PanelState + * @typedef {Object} PanelState * @property {number} id - Id of the visualization contained in the panel. * @property {Element} $el - A reference to the gridster widget holding this panel. Used to * update the size and column attributes. TODO: move out of panel state as this couples state to ui. diff --git a/src/core_plugins/kibana/public/dashboard/components/panel/lib/panel_utils.js b/src/core_plugins/kibana/public/dashboard/components/panel/lib/panel_utils.js index 2b789179ba322..5856d71f884f6 100644 --- a/src/core_plugins/kibana/public/dashboard/components/panel/lib/panel_utils.js +++ b/src/core_plugins/kibana/public/dashboard/components/panel/lib/panel_utils.js @@ -3,7 +3,7 @@ import { DEFAULT_PANEL_WIDTH, DEFAULT_PANEL_HEIGHT } from 'plugins/kibana/dashbo export class PanelUtils { /** * Fills in default parameters where not specified. - * @param panel {Panel} + * @param {PanelState} panel */ static initializeDefaults(panel) { panel.size_x = panel.size_x || DEFAULT_PANEL_WIDTH; @@ -23,7 +23,7 @@ export class PanelUtils { /** * Ensures that the panel object has the latest size/pos info. - * @param panel {Panel} + * @param {PanelState} panel */ static refreshSizeAndPosition(panel) { const data = panel.$el.coords().grid; @@ -37,7 +37,7 @@ export class PanelUtils { * $el is a circular structure because it contains a reference to it's parent panel, * so it needs to be removed before it can be serialized (we also don't * want it to show up in the url). - * @param panel + * @param {PanelState} panel */ static makeSerializeable(panel) { delete panel.$el;