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/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;
-};