Warn if the title is a duplicate

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

@@ -85,6 +85,7 @@ describe('Saved Object', function () {
  */
  function createInitializedSavedObject(config = {}) {
  const savedObject = new SavedObject(config);
+ savedObject.title = 'my saved object';
  return savedObject.init();
  }
 

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

@@ -0,0 +1,27 @@
+/**
+ * Returns true if the given saved object has a title that already exists, false otherwise.
+ * @param savedObject {SavedObject} The object with the title to check.
+ * @param esAdmin {Object} Used to query es
+ * @returns {Promise<bool>}
+ */
+export function getTitleAlreadyExists(savedObject, esAdmin) {
+ const { index, title, type, id } = savedObject;
+ if (!title) {
+ throw new Error('Title must be supplied');
+ }
+
+ const body = {
+ query: {
+ bool: {
+ must: [{ match: { title } }],
+ must_not: [{ match: { id } }]
+ }
+ }
+ };
+
+ const size = 0;
+ return esAdmin.search({ index, type, body, size })
+ .then((response) => {
+ return response.hits.total > 0 ? true : false;
+ });
+}

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

@@ -18,6 +18,7 @@ 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';
 
 export default function SavedObjectFactory(esAdmin, kbnIndex, Promise, Private, Notifier, confirmModalPromise, indexPatterns) {
 
@@ -36,6 +37,8 @@ export default function SavedObjectFactory(esAdmin, kbnIndex, Promise, Private,
 
  // type name for this object, used as the ES-type
  const type = config.type;
+ this.type = type;
+ this.index = kbnIndex;
 
  this.getDisplayName = function () {
  return type;
@@ -96,7 +99,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 +110,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)) {
@@ -263,6 +270,11 @@ export default function SavedObjectFactory(esAdmin, kbnIndex, Promise, Private,
  * @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';
 
  /**
  * Attempts to create the current object using the serialized source. If an object already
@@ -290,6 +302,26 @@ 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.
+ if (this.title === this.lastSavedTitle) {
+ return Promise.resolve();
+ }
+
+ return getTitleAlreadyExists(this, esAdmin)
+ .then((isDuplicate) => {
+ if (!isDuplicate) return true;
+ const confirmMessage = `A ${type} with the title '${this.title}' already exists. Would you like to save anyway?`;
+
+ return confirmModalPromise(confirmMessage, { confirmButtonText: `Save ${type}` })
+ .catch(() => Promise.reject(new Error(SAVE_DUPLICATE_REJECTED)));
+ });
+ };
 
  /**
  * @typedef {Object} SaveOptions
@@ -325,9 +357,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 +374,11 @@ export default function SavedObjectFactory(esAdmin, kbnIndex, Promise, Private,
  .catch((err) => {
  this.isSaving = false;
  this.id = originalId;
- if (err && err.message === OVERWRITE_REJECTED) return;
+ if (err && (
+ err.message === OVERWRITE_REJECTED ||
+ err.message === SAVE_DUPLICATE_REJECTED)) {
+ return;
+ }
  return Promise.reject(err);
  });
  };
@@ -360,7 +401,9 @@ export default function SavedObjectFactory(esAdmin, kbnIndex, Promise, Private,
  type: type,
  id: this.id
  })
- .then(() => { return refreshIndex(); });
+ .then(() => {
+ return refreshIndex();
+ });
  };
  }
 

test/functional/apps/dashboard/_dashboard_save.js

@@ -0,0 +1,55 @@
+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', 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();
+
+ const countOfDashboards = await PageObjects.dashboard.getDashboardCountWithName(dashboardName);
+ expect(countOfDashboards).to.equal(2);
+ });
+
+ bdd.it('Does not warn when saving a duplicate title that remains unchanged', 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);
+ });
+});

test/functional/apps/dashboard/index.js

@@ -14,5 +14,6 @@ bdd.describe('dashboard app', function () {
  });
 
  require('./_dashboard');
+ require('./_dashboard_save');
  require('./_dashboard_time');
 });

test/support/page_objects/common.js

@@ -299,10 +299,10 @@ export default class Common {
  return exists;
  }
 
- findTestSubject(selector) {
+ findTestSubject(selector, findTimeout = defaultFindTimeout) {
  this.debug('in findTestSubject: ' + testSubjSelector(selector));
  return this.remote
- .setFindTimeout(defaultFindTimeout)
+ .setFindTimeout(findTimeout)
  .findDisplayedByCssSelector(testSubjSelector(selector));
  }
 
@@ -321,4 +321,21 @@ export default class Common {
  this.debug(`Found ${elements.length} for selector ${selector}`);
  return elements;
  }
+
+ async clickConfirmOnModal() {
+ this.debug('Clicking modal confirm');
+ await this.findTestSubject('confirmModalConfirmButton').click();
+ }
+
+ async clickCancelOnModal() {
+ this.debug('Clicking modal cancel');
+ await this.findTestSubject('confirmModalCancelButton').click();
+ }
+
+ async isConfirmModalOpen() {
+ let isOpen = true;
+ await this.findTestSubject('confirmModalCancelButton', 2000).catch(() => isOpen = false);
+ await this.remote.setFindTimeout(defaultFindTimeout);
+ return isOpen;
+ }
 }

test/support/page_objects/dashboard_page.js

@@ -41,7 +41,9 @@ export default class DashboardPage {
  PageObjects.common.debug('Go to dashboard landing page');
  const onPage = await this.onDashboardLandingPage();
  if (!onPage) {
- return PageObjects.common.findByCssSelector('a[href="#/dashboard"]').click();
+ return PageObjects.common.try(() =>
+ PageObjects.common.findByCssSelector('a[href="#/dashboard"]').click()
+ );
  }
  }
 
@@ -131,13 +133,28 @@ export default class DashboardPage {
  }
 
  async saveDashboard(dashName, storeTimeWithDash) {
+ await this.enterDashboardTitleAndClickSave(dashName, storeTimeWithDash);
+
+ await PageObjects.header.isGlobalLoadingIndicatorHidden();
+
+ // verify that green message at the top of the page.
+ // it's only there for about 5 seconds
+ await PageObjects.common.try(() => {
+ PageObjects.common.debug('verify toast-message for saved dashboard');
+ return this.findTimeout
+ .findByCssSelector('kbn-truncated.toast-message.ng-isolate-scope')
+ .getVisibleText();
+ });
+ }
+
+ async enterDashboardTitleAndClickSave(dashboardTitle, storeTimeWithDash) {
  await PageObjects.common.findTestSubject('dashboardSaveButton').click();
 
  await PageObjects.header.isGlobalLoadingIndicatorHidden();
  await PageObjects.common.sleep(1000);
 
  PageObjects.common.debug('entering new title');
- await this.findTimeout.findById('dashboardTitle').type(dashName);
+ await this.findTimeout.findById('dashboardTitle').type(dashboardTitle);
 
  if (storeTimeWithDash !== undefined) {
  await this.storeTimeWithDashboard(storeTimeWithDash);
@@ -150,17 +167,6 @@ export default class DashboardPage {
  PageObjects.common.debug('clicking final Save button for named dashboard');
  return this.findTimeout.findByCssSelector('.btn-primary').click();
  });
-
- await PageObjects.header.isGlobalLoadingIndicatorHidden();
-
- // verify that green message at the top of the page.
- // it's only there for about 5 seconds
- await PageObjects.common.try(() => {
- PageObjects.common.debug('verify toast-message for saved dashboard');
- return this.findTimeout
- .findByCssSelector('kbn-truncated.toast-message.ng-isolate-scope')
- .getVisibleText();
- });
  }
 
  clickDashboardByLinkText(dashName) {
@@ -169,18 +175,33 @@ export default class DashboardPage {
  .click();
  }
 
- // use the search filter box to narrow the results down to a single
- // entry, or at least to a single page of results
- async loadSavedDashboard(dashName) {
- PageObjects.common.debug(`Load Saved Dashboard ${dashName}`);
- const self = this;
+ async searchForDashboardWithName(dashName) {
+ PageObjects.common.debug(`searchForDashboardWithName: ${dashName}`);
+
  await this.gotoDashboardLandingPage();
  const searchBox = await PageObjects.common.findTestSubject('searchFilter');
  await searchBox.click();
+
+ // Note: this replacement of - to space is to preserve original logic but I'm not sure why or if it's needed.
  await searchBox.type(dashName.replace('-',' '));
 
- await PageObjects.header.isGlobalLoadingIndicatorHidden();
- await PageObjects.common.sleep(1000);
+ return await PageObjects.header.isGlobalLoadingIndicatorHidden();
+ }
+
+ async getDashboardCountWithName(dashName) {
+ PageObjects.common.debug(`getDashboardCountWithName: ${dashName}`);
+
+ await this.searchForDashboardWithName(dashName);
+ const links = await this.findTimeout.findAllByLinkText(dashName);
+ return links.length;
+ }
+
+ // use the search filter box to narrow the results down to a single
+ // entry, or at least to a single page of results
+ async loadSavedDashboard(dashName) {
+ PageObjects.common.debug(`Load Saved Dashboard ${dashName}`);
+
+ await this.searchForDashboardWithName(dashName);
  await this.clickDashboardByLinkText(dashName);
  return PageObjects.header.isGlobalLoadingIndicatorHidden();
  }