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
elastic · Dec 2, 2016 · f432698 · f432698
1 parent 9d79d8b
commit f432698
Show file tree

Hide file tree

Showing 20 changed files with 269 additions and 169 deletions.
diff --git a/src/core_plugins/kibana/public/dashboard/index.html b/src/core_plugins/kibana/public/dashboard/index.html
@@ -10,7 +10,7 @@
       >
         <span
           ng-show="dash.id"
-          ng-bind="::dash.title"
+          ng-bind="dash.lastSavedTitle"
         ></span>
       </div>
 

diff --git a/src/core_plugins/kibana/public/dashboard/index.js b/src/core_plugins/kibana/public/dashboard/index.js
@@ -143,10 +143,11 @@ 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);
         }
@@ -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);
             }
           }
         })

diff --git 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()"
 >
-  <div class="localDropdownTitle">Save Dashboard</div>
+  <div class="localDropdownTitle">Save {{opts.dashboard.getDisplayName()}}</div>
   <input
     class="localDropdownInput"
     id="dashboardTitle"
@@ -11,12 +11,18 @@
     placeholder="Dashboard title"
     input-focus="select"
   >
+
+  <saved-object-save-as-check-box saved-object="opts.dashboard"></saved-object-save-as-check-box>
+
   <div class="form-group">
     <label>
       <input type="checkbox" ng-model="opts.dashboard.timeRestore" ng-checked="opts.dashboard.timeRestore">
-      Store time with dashboard
-      <kbn-info info="Change the time filter to the currently selected time each time this dashboard is loaded"></kbn-info>
+      Store time with {{opts.dashboard.getDisplayName()}}
+      <kbn-info placement="bottom" info="Change the time filter to the currently selected time each time this dashboard is loaded"></kbn-info>
     </label>
   </div>
-  <button type="submit" ng-disabled="!opts.dashboard.title" class="btn btn-primary" aria-label="Save dashboard">Save</button>
+
+  <button type="submit" ng-disabled="!opts.dashboard.title" class="btn btn-primary" aria-label="Save dashboard">
+    Save
+  </button>
 </form>
diff --git 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
@@ -7,7 +7,7 @@
       <div data-transclude-slot="topLeftCorner" class="localBreadcrumbs">
         <div class="localBreadcrumb">
           <span ng-show="opts.savedSearch.id" class="localBreadcrumb__emphasis">
-            <span data-test-subj="discoverCurrentQuery" ng-bind="::opts.savedSearch.title"></span>
+            <span data-test-subj="discoverCurrentQuery" ng-bind="opts.savedSearch.lastSavedTitle"></span>
             <i aria-label="Reload Saved Search" tooltip="Reload Saved Search" ng-click="resetQuery();" class="fa fa-undo small"></i>&nbsp;
           </span>
           <span data-test-subj="discoverQueryHits" class="localBreadcrumb__emphasis">{{(hits || 0) | number:0}}</span>

diff --git 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..."
   >
+
+  <saved-object-save-as-check-box saved-object="opts.savedSearch"></saved-object-save-as-check-box>
   <button ng-disabled="!opts.savedSearch.title" data-test-subj="discover-save-search-btn" type="submit" class="btn btn-primary">
     Save
   </button>

diff --git a/src/core_plugins/kibana/public/visualize/editor/editor.html b/src/core_plugins/kibana/public/visualize/editor/editor.html
@@ -10,7 +10,7 @@
       >
         <span
           ng-show="savedVis.id"
-          ng-bind="::savedVis.title"
+          ng-bind="savedVis.lastSavedTitle"
         ></span>
       </div>
 

diff --git 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
@@ -11,6 +11,9 @@
     ng-model="opts.savedVis.title"
     required
   >
+
+  <saved-object-save-as-check-box saved-object="opts.savedVis"></saved-object-save-as-check-box>
+
   <button
     data-test-subj="saveVisualizationButton"
     type="submit"

diff --git a/src/core_plugins/timelion/public/app.js b/src/core_plugins/timelion/public/app.js
@@ -163,7 +163,7 @@ app.controller('timelion', function (
     }
   });
 
-  $scope.$watch(function () { return savedSheet.title; }, function (newTitle) {
+  $scope.$watch(function () { return savedSheet.lastSavedTitle; }, function (newTitle) {
     docTitle.change(savedSheet.id ? newTitle : undefined);
   });
 
@@ -222,7 +222,6 @@ app.controller('timelion', function (
   $scope.safeSearch = _.debounce($scope.search, 500);
 
   function saveSheet() {
-    savedSheet.id = savedSheet.title;
     savedSheet.timelion_sheet = $scope.state.sheet;
     savedSheet.timelion_interval = $scope.state.interval;
     savedSheet.timelion_columns = $scope.state.columns;
@@ -239,7 +238,6 @@ app.controller('timelion', function (
 
   function saveExpression(title) {
     savedVisualizations.get({type: 'timelion'}).then(function (savedExpression) {
-      savedExpression.id = title;
       savedExpression.visState.params = {
         expression: $scope.state.sheet[$scope.state.selected],
         interval: $scope.state.interval

diff --git a/src/core_plugins/timelion/public/index.html b/src/core_plugins/timelion/public/index.html
@@ -5,7 +5,7 @@
     <div data-transclude-slots>
       <div data-transclude-slot="topLeftCorner">
         <span class="localTitle" ng-show="opts.savedSheet.id">
-          {{opts.savedSheet.title}}
+          {{opts.savedSheet.lastSavedTitle}}
           &nbsp;
           <span class="fa fa-bolt" ng-click="showStats = !showStats"></span>
           &nbsp;

diff --git a/src/core_plugins/timelion/public/partials/save_sheet.html b/src/core_plugins/timelion/public/partials/save_sheet.html
@@ -14,6 +14,8 @@ <h4 class="list-group-item-heading">Save entire Timelion sheet</h4>
         <label for="savedSheet" class="control-label">Save sheet as</label>
         <input id="savedSheet" ng-model="opts.savedSheet.title" input-focus="select" class="form-control" placeholder="Name this sheet...">
       </div>
+
+      <saved-object-save-as-check-box saved-object="opts.savedSheet"></saved-object-save-as-check-box>
       <div class="form-group">
         <button ng-disabled="!opts.savedSheet.title" type="submit" class="btn btn-primary">
           Save
@@ -43,9 +45,7 @@ <h4 class="list-group-item-heading">Save current expression as Kibana dashboard
         <input id="savedExpression" ng-model="panelTitle" input-focus="select" class="form-control" placeholder="Name this panel">
       </div>
       <div class="form-group">
-        <button ng-disabled="!panelTitle" type="submit" class="btn btn-primary">
-          Save
-        </button>
+        <button ng-disabled="!panelTitle" type="submit" class="btn btn-primary">Save</button>
       </div>
     </form>
   </div>

diff --git 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
@@ -5,18 +5,22 @@
 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();
 
   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 => {