Warn if the title is a duplicate

src/ui/public/courier/__tests__/saved_object.js

@@ -69,6 +69,9 @@ describe('Saved Object', function () {
    * @param {Object} mockDocResponse
    */
   function stubESResponse(mockDocResponse) {
+    // Stub out search for duplicate title:
+    sinon.stub(esAdminStub, 'search').returns(BluebirdPromise.resolve({ hits: { total: 0 } }));
+
     sinon.stub(esDataStub, 'mget').returns(BluebirdPromise.resolve({ docs: [mockDocResponse] }));
     sinon.stub(esDataStub, 'index').returns(BluebirdPromise.resolve(mockDocResponse));
     sinon.stub(esAdminStub, 'mget').returns(BluebirdPromise.resolve({ docs: [mockDocResponse] }));
@@ -85,6 +88,7 @@ describe('Saved Object', function () {
    */
   function createInitializedSavedObject(config = {}) {
     const savedObject = new SavedObject(config);
+    savedObject.title = 'my saved object';
     return savedObject.init();
   }
 
@@ -278,6 +282,7 @@ describe('Saved Object', function () {
       });
 
       it('on failure', function () {
+        stubESResponse(getMockedDocResponse('id'));
         return createInitializedSavedObject({ type: 'dashboard' }).then(savedObject => {
           sinon.stub(DocSource.prototype, 'doIndex', () => {
             expect(savedObject.isSaving).to.be(true);

src/ui/public/courier/saved_object/get_title_already_exists.js

@@ -0,0 +1,36 @@
+import _ from 'lodash';
+/**
+ * Returns true if the given saved object has a title that already exists, false otherwise. Search is case
+ * insensitive.
+ * @param savedObject {SavedObject} The object with the title to check.
+ * @param esAdmin {Object} Used to query es
+ * @returns {Promise<string|undefined>} Returns the title that matches. Because this search is not case
+ * sensitive, it may not exactly match the title of the object.
+ */
+export function getTitleAlreadyExists(savedObject, esAdmin) {
+  const { index, title, id } = savedObject;
+  const esType = savedObject.getEsType();
+  if (!title) {
+    throw new Error('Title must be supplied');
+  }
+
+  const body = {
+    query: {
+      bool: {
+        must: { match_phrase: { title } },
+        must_not: { match: { id } }
+      }
+    }
+  };
+
+  // Elastic search will return the most relevant results first, which means exact matches should come
+  // first, and so we shouldn't need to request everything. Using 10 just to be on the safe side.
+  const size = 10;
+  return esAdmin.search({ index, type: esType, body, size })
+    .then((response) => {
+      const match = _.find(response.hits.hits, function currentVersion(hit) {
+        return hit._source.title.toLowerCase() === title.toLowerCase();
+      });
+      return match ? match._source.title : undefined;
+    });
+}

src/ui/public/courier/saved_object/saved_object.js

@@ -18,6 +18,27 @@ import MappingSetupProvider from 'ui/utils/mapping_setup';
 
 import DocSourceProvider from '../data_source/admin_doc_source';
 import SearchSourceProvider from '../data_source/search_source';
+import { getTitleAlreadyExists } from './get_title_already_exists';
+
+/**
+ * An error message to be used when the user rejects a confirm overwrite.
+ * @type {string}
+ */
+const OVERWRITE_REJECTED = 'Overwrite confirmation was rejected';
+/**
+ * An error message to be used when the user rejects a confirm save with duplicate title.
+ * @type {string}
+ */
+const SAVE_DUPLICATE_REJECTED = 'Save with duplicate title confirmation was rejected';
+
+/**
+ * @param error {Error} the error
+ * @return {boolean}
+ */
+function isErrorNonFatal(error) {
+  if (!error) return false;
+  return error.message === OVERWRITE_REJECTED || error.message === SAVE_DUPLICATE_REJECTED;
+}
 
 export default function SavedObjectFactory(esAdmin, kbnIndex, Promise, Private, Notifier, confirmModalPromise, indexPatterns) {
 
@@ -35,10 +56,17 @@ export default function SavedObjectFactory(esAdmin, kbnIndex, Promise, Private,
     const docSource = new DocSource();
 
     // type name for this object, used as the ES-type
-    const type = config.type;
+    const esType = config.type;
+    this.index = kbnIndex;
 
     this.getDisplayName = function () {
-      return type;
+      return esType;
+    };
+
+    // NOTE: this.type (not set in this file, but somewhere else) is the sub type, e.g. 'area' or
+    // 'data table', while esType is the more generic type - e.g. 'visualization' or 'saved search'.
+    this.getEsType = function () {
+      return esType;
     };
 
     /**
@@ -51,7 +79,7 @@ export default function SavedObjectFactory(esAdmin, kbnIndex, Promise, Private,
 
     // Create a notifier for sending alerts
     const notify = new Notifier({
-      location: 'Saved ' + type
+      location: 'Saved ' + this.getDisplayName()
     });
 
     // mapping definition for the fields that this object will expose
@@ -96,7 +124,9 @@ export default function SavedObjectFactory(esAdmin, kbnIndex, Promise, Private,
      * @return {Promise<IndexPattern | null>}
      */
     const hydrateIndexPattern = () => {
-      if (!this.searchSource) { return Promise.resolve(null); }
+      if (!this.searchSource) {
+        return Promise.resolve(null);
+      }
 
       if (config.clearSavedIndexPattern) {
         this.searchSource.set('index', undefined);
@@ -105,7 +135,9 @@ export default function SavedObjectFactory(esAdmin, kbnIndex, Promise, Private,
 
       let index = config.indexPattern || this.searchSource.getOwn('index');
 
-      if (!index) { return Promise.resolve(null); }
+      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)) {
@@ -128,17 +160,17 @@ export default function SavedObjectFactory(esAdmin, kbnIndex, Promise, Private,
      * @resolved {SavedObject}
      */
     this.init = _.once(() => {
-      // ensure that the type is defined
-      if (!type) throw new Error('You must define a type name to use SavedObject objects.');
+      // ensure that the esType is defined
+      if (!esType) throw new Error('You must define a type name to use SavedObject objects.');
 
       // tell the docSource where to find the doc
       docSource
         .index(kbnIndex)
-        .type(type)
+        .type(esType)
         .id(this.id);
 
-      // check that the mapping for this type is defined
-      return mappingSetup.isDefined(type)
+      // check that the mapping for this esType is defined
+      return mappingSetup.isDefined(esType)
         .then(function (defined) {
           // if it is already defined skip this step
           if (defined) return true;
@@ -152,8 +184,8 @@ export default function SavedObjectFactory(esAdmin, kbnIndex, Promise, Private,
             }
           };
 
-          // tell mappingSetup to set type
-          return mappingSetup.setup(type, mapping);
+          // tell mappingSetup to set esType
+          return mappingSetup.setup(esType, mapping);
         })
         .then(() => {
           // If there is not id, then there is no document to fetch from elasticsearch
@@ -180,7 +212,7 @@ export default function SavedObjectFactory(esAdmin, kbnIndex, Promise, Private,
     this.applyESResp = (resp) => {
       this._source = _.cloneDeep(resp._source);
 
-      if (resp.found != null && !resp.found) throw new errors.SavedObjectNotFound(type, this.id);
+      if (resp.found != null && !resp.found) throw new errors.SavedObjectNotFound(esType, this.id);
 
       const meta = resp._source.kibanaSavedObjectMeta || {};
       delete resp._source.kibanaSavedObjectMeta;
@@ -258,12 +290,6 @@ export default function SavedObjectFactory(esAdmin, kbnIndex, Promise, Private,
       return esAdmin.indices.refresh({ index: kbnIndex });
     }
 
-    /**
-     * An error message to be used when the user rejects a confirm overwrite.
-     * @type {string}
-     */
-    const OVERWRITE_REJECTED = 'Overwrite confirmation was rejected';
-
     /**
      * Attempts to create the current object using the serialized source. If an object already
      * exists, a warning message requests an overwrite confirmation.
@@ -290,6 +316,27 @@ export default function SavedObjectFactory(esAdmin, kbnIndex, Promise, Private,
         });
     };
 
+    /**
+     * Returns a promise that resolves to true if either the title is unique, or if the user confirmed they
+     * wished to save the duplicate title.  Promise is rejected if the user rejects the confirmation.
+     */
+    const warnIfDuplicateTitle = () => {
+      // Don't warn if the user isn't updating the title, otherwise that would become very annoying to have
+      // to confirm the save every time, except when copyOnSave is true, then we do want to check.
+      if (this.title === this.lastSavedTitle && !this.copyOnSave) {
+        return Promise.resolve();
+      }
+
+      return getTitleAlreadyExists(this, esAdmin)
+        .then((duplicateTitle) => {
+          if (!duplicateTitle) return true;
+          const confirmMessage =
+            `A ${this.getDisplayName()} with the title '${duplicateTitle}' already exists. Would you like to save anyway?`;
+
+          return confirmModalPromise(confirmMessage, { confirmButtonText: `Save ${this.getDisplayName()}` })
+            .catch(() => Promise.reject(new Error(SAVE_DUPLICATE_REJECTED)));
+        });
+    };
 
     /**
      * @typedef {Object} SaveOptions
@@ -325,9 +372,14 @@ export default function SavedObjectFactory(esAdmin, kbnIndex, Promise, Private,
       const source = this.serialize();
 
       this.isSaving = true;
-      const doSave = saveOptions.confirmOverwrite ? createSource(source) : docSource.doIndex(source);
-      return doSave
-        .then((id) => { this.id = id; })
+
+      return warnIfDuplicateTitle()
+        .then(() => {
+          return saveOptions.confirmOverwrite ? createSource(source) : docSource.doIndex(source);
+        })
+        .then((id) => {
+          this.id = id;
+        })
         .then(refreshIndex)
         .then(() => {
           this.isSaving = false;
@@ -337,7 +389,9 @@ export default function SavedObjectFactory(esAdmin, kbnIndex, Promise, Private,
         .catch((err) => {
           this.isSaving = false;
           this.id = originalId;
-          if (err && err.message === OVERWRITE_REJECTED) return;
+          if (isErrorNonFatal(err)) {
+            return;
+          }
           return Promise.reject(err);
         });
     };
@@ -357,10 +411,12 @@ export default function SavedObjectFactory(esAdmin, kbnIndex, Promise, Private,
       return esAdmin.delete(
         {
           index: kbnIndex,
-          type: type,
+          type: esType,
           id: this.id
         })
-        .then(() => { return refreshIndex(); });
+        .then(() => {
+          return refreshIndex();
+        });
     };
   }
 

src/ui/public/courier/saved_object/ui/saved_object_save_as_checkbox.html

@@ -3,7 +3,12 @@
     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.
   </div>
   <label>
-    <input type="checkbox" ng-model="savedObject.copyOnSave" ng-checked="savedObject.copyOnSave">
+    <input
+      type="checkbox"
+      data-test-subj="saveAsNewCheckbox"
+      ng-model="savedObject.copyOnSave"
+      ng-checked="savedObject.copyOnSave"
+    >
     Save as a new {{savedObject.getDisplayName()}}
   </label>
 </div>

test/functional/apps/dashboard/_dashboard_save.js

@@ -0,0 +1,86 @@
+import expect from 'expect.js';
+import { bdd } from '../../../support';
+
+import PageObjects from '../../../support/page_objects';
+
+bdd.describe('dashboard save', function describeIndexTests() {
+  const dashboardName = 'Dashboard Save Test';
+
+  bdd.before(async function () {
+    return PageObjects.dashboard.initTests();
+  });
+
+  bdd.it('warns on duplicate name for new dashboard', async function() {
+    await PageObjects.dashboard.clickNewDashboard();
+    await PageObjects.dashboard.saveDashboard(dashboardName);
+
+    let isConfirmOpen = await PageObjects.common.isConfirmModalOpen();
+    expect(isConfirmOpen).to.equal(false);
+
+    await PageObjects.dashboard.gotoDashboardLandingPage();
+    await PageObjects.dashboard.clickNewDashboard();
+    await PageObjects.dashboard.enterDashboardTitleAndClickSave(dashboardName);
+
+    isConfirmOpen = await PageObjects.common.isConfirmModalOpen();
+    expect(isConfirmOpen).to.equal(true);
+  });
+
+  bdd.it('does not save on reject confirmation', async function() {
+    await PageObjects.common.clickCancelOnModal();
+
+    const countOfDashboards = await PageObjects.dashboard.getDashboardCountWithName(dashboardName);
+    expect(countOfDashboards).to.equal(1);
+  });
+
+  bdd.it('Saves on confirm duplicate title warning', async function() {
+    await PageObjects.dashboard.gotoDashboardLandingPage();
+    await PageObjects.dashboard.clickNewDashboard();
+    await PageObjects.dashboard.enterDashboardTitleAndClickSave(dashboardName);
+
+    await PageObjects.common.clickConfirmOnModal();
+
+    // This is important since saving a new dashboard will cause a refresh of the page.  We have to
+    // wait till it finishes reloading or it might reload the url after simulating the
+    // dashboard landing page click.
+    await PageObjects.header.waitUntilLoadingHasFinished();
+
+    const countOfDashboards = await PageObjects.dashboard.getDashboardCountWithName(dashboardName);
+    expect(countOfDashboards).to.equal(2);
+  });
+
+  bdd.it('Does not warn when you save an existing dashboard with the title it already has, and that title is a duplicate',
+    async function() {
+      await PageObjects.dashboard.clickDashboardByLinkText(dashboardName);
+      await PageObjects.header.isGlobalLoadingIndicatorHidden();
+      await PageObjects.dashboard.saveDashboard(dashboardName);
+
+      const isConfirmOpen = await PageObjects.common.isConfirmModalOpen();
+      expect(isConfirmOpen).to.equal(false);
+    }
+  );
+
+  bdd.it('Warns you when you Save as New Dashboard, and the title is a duplicate', async function() {
+    await PageObjects.dashboard.enterDashboardTitleAndClickSave(dashboardName, { saveAsNew: true });
+
+    const isConfirmOpen = await PageObjects.common.isConfirmModalOpen();
+    expect(isConfirmOpen).to.equal(true);
+
+    await PageObjects.common.clickCancelOnModal();
+  });
+
+  bdd.it('Does not warn when only the prefix matches', async function() {
+    await PageObjects.dashboard.saveDashboard(dashboardName.split(' ')[0]);
+
+    const isConfirmOpen = await PageObjects.common.isConfirmModalOpen();
+    expect(isConfirmOpen).to.equal(false);
+  });
+
+  bdd.it('Warns when case is different', async function() {
+    await PageObjects.dashboard.enterDashboardTitleAndClickSave(dashboardName.toUpperCase());
+
+    const isConfirmOpen = await PageObjects.common.isConfirmModalOpen();
+    expect(isConfirmOpen).to.equal(true);
+
+    await PageObjects.common.clickCancelOnModal();
+  });
+});

test/functional/apps/dashboard/_dashboard_time.js

@@ -17,7 +17,7 @@ bdd.describe('dashboard time', function dashboardSaveWithTime() {
     bdd.it('is saved', async function () {
       await PageObjects.dashboard.clickNewDashboard();
       await PageObjects.dashboard.addVisualizations([PageObjects.dashboard.getTestVisualizationNames()[0]]);
-      await PageObjects.dashboard.saveDashboard(dashboardName, false);
+      await PageObjects.dashboard.saveDashboard(dashboardName, { storeTimeWithDashboard: false });
     });
 
     bdd.it('Does not set the time picker on open', async function () {
@@ -35,7 +35,7 @@ bdd.describe('dashboard time', function dashboardSaveWithTime() {
   bdd.describe('dashboard with stored timed', async function () {
     bdd.it('is saved with quick time', async function () {
       await PageObjects.header.setQuickTime('Today');
-      await PageObjects.dashboard.saveDashboard(dashboardName, true);
+      await PageObjects.dashboard.saveDashboard(dashboardName, { storeTimeWithDashboard: true });
     });
 
     bdd.it('sets quick time on open', async function () {
@@ -49,7 +49,7 @@ bdd.describe('dashboard time', function dashboardSaveWithTime() {
 
     bdd.it('is saved with absolute time', async function () {
       await PageObjects.header.setAbsoluteRange(fromTime, toTime);
-      await PageObjects.dashboard.saveDashboard(dashboardName, true);
+      await PageObjects.dashboard.saveDashboard(dashboardName, { storeTimeWithDashboard: true });
     });
 
     bdd.it('sets absolute time on open', async function () {