Add comments and inline docs for visualization saving and editing pro…

…cess.

The goal is to clarify where URL state is coming from, when working with
visualizations. Some of the classes touched upon are: SavedVis,
PersistedState, AppState, and base classes.

Explored files:
- src/core_plugins/kibana/public/visualize/editor/editor.js
- src/core_plugins/kibana/public/visualize/saved_visualizations/_saved_vis.js
- src/ui/public/courier/data_source/_doc_send_to_es.js
- src/ui/public/courier/data_source/doc_source.js
- src/ui/public/courier/saved_object/saved_object.js
- src/ui/public/es.js
- src/ui/public/events.js
- src/ui/public/persisted_state/persisted_state.js
- src/ui/public/state_management/app_state.js
- src/ui/public/state_management/state.js
- src/ui/public/vis/agg_config.js
- src/ui/public/vis/agg_configs.js
- src/ui/public/vis/vis.js

src/core_plugins/kibana/public/visualize/editor/editor.js

@@ -66,16 +66,27 @@ uiModules
     location: 'Visualization Editor'
   });
 
+  // Retrieve the resolved SavedVis instance.
   const savedVis = $route.current.locals.savedVis;
 
+  // Instance of vis.js.
   const vis = savedVis.vis;
+
+  // Clone the _vis instance.
   const editableVis = vis.createEditableVis();
+
+  // NOTE: Why is this instance method being overwritten?
   vis.requesting = function () {
     const requesting = editableVis.requesting;
+    // Invoking requesting() calls onRequest on each agg's type param.
+    // NOTE: What is the structure of an agg_config, and the role of type
+    // and type.params?
     requesting.call(vis);
     requesting.call(editableVis);
   };
 
+  // Boolean by default, but then something else later
+  // NOTE: What is the role of searchSource? Why does it default to a boolean?
   const searchSource = savedVis.searchSource;
 
   $scope.topNavMenu = [{
@@ -104,8 +115,12 @@ uiModules
     docTitle.change(savedVis.title);
   }
 
+  // Instance of app_state.js.
   let $state = $scope.$state = (function initState() {
+    // Extract visualization state with filtered aggs.
+    // Consists of: aggs, params, listeners, title, and type.
     const savedVisState = vis.getState();
+
     const stateDefaults = {
       uiState: savedVis.uiStateJSON ? JSON.parse(savedVis.uiStateJSON) : {},
       linked: !!savedVis.savedSearchId,
@@ -114,19 +129,22 @@ uiModules
       vis: savedVisState
     };
 
-    $state = new AppState(stateDefaults);
+    // This is used to generate a 'uiState' PersistedState instance, and
+    // implictly add 'change' and 'fetch_with_changes' event handlers.
+    const appState = new AppState(stateDefaults);
 
-    if (!angular.equals($state.vis, savedVisState)) {
+    // NOTE: Why would appState.vis not equal the savedVisState?
+    if (!angular.equals(appState.vis, savedVisState)) {
       Promise.try(function () {
-        editableVis.setState($state.vis);
+        editableVis.setState(appState.vis);
         vis.setState(editableVis.getEnabledState());
       })
       .catch(courier.redirectWhenMissing({
         'index-pattern-field': '/visualize'
       }));
     }
 
-    return $state;
+    return appState;
   }());
 
   function init() {
@@ -137,8 +155,14 @@ uiModules
     $scope.indexPattern = vis.indexPattern;
     $scope.editableVis = editableVis;
     $scope.state = $state;
+
+    // Create a PersistedState instance.
     $scope.uiState = $state.makeStateful('uiState');
+    // Associate PersistedState instance with the Vis instance, so that
+    // `uiStateVal` can be called on it. Currently this is only used to extract
+    // map-specific information (e.g. mapZoom, mapCenter).
     vis.setUiState($scope.uiState);
+
     $scope.timefilter = timefilter;
     $scope.opts = _.pick($scope, 'doSave', 'savedVis', 'shareData', 'timefilter');
 
@@ -239,6 +263,9 @@ uiModules
     kbnUrl.change('/visualize', {});
   };
 
+  /**
+   * 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

src/core_plugins/kibana/public/visualize/saved_visualizations/_saved_vis.js

@@ -1,6 +1,16 @@
+
+/**
+ * @name SavedVis
+ *
+ * @extends SavedObject.
+ *
+ * @description It's a type of SavedObject, but specific to visualizations.
+ */
+
 import _ from 'lodash';
 import VisProvider from 'ui/vis';
 import uiModules from 'ui/modules';
+
 uiModules
 .get('app/visualize')
 .factory('SavedVis', function (config, $injector, courier, Promise, savedSearches, Private, Notifier) {

src/ui/public/courier/data_source/_doc_send_to_es.js

@@ -1,3 +1,11 @@
+
+/**
+ * @name _doc_send_to_es
+ *
+ * @description Depends upon the es object to make ES requests, and also
+ * interacts with courier objects.
+ */
+
 import _ from 'lodash';
 
 import errors from 'ui/errors';

src/ui/public/courier/data_source/doc_source.js

@@ -1,3 +1,14 @@
+
+/**
+ * @name DocSource
+ *
+ * @description This class is tightly coupled with _doc_send_to_es. Its primary
+ * methods (`doUpdate`, `doIndex`, `doCreate`) are all proxies for methods
+ * exposed by _doc_send_to_es (`update`, `index`, `create`). These methods are
+ * called with DocSource as the context. When called, they depend on “private”
+ * DocSource methods within their execution.
+ */
+
 import _ from 'lodash';
 
 import 'ui/es';

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

@@ -1,3 +1,15 @@
+
+/**
+ * @name SavedObject
+ *
+ * @description SavedObject seems to track a reference to an object in ES,
+ * and surface methods for CRUD functionality (save and delete). This seems
+ * similar to how Backbone Models work.
+ *
+ * This class seems to interface with ES primarily through the es Angular
+ * service and a DocSource instance.
+ */
+
 import angular from 'angular';
 import _ from 'lodash';
 

src/ui/public/es.js

@@ -2,6 +2,12 @@ import 'elasticsearch-browser';
 import _ from 'lodash';
 import uiModules from 'ui/modules';
 
+/**
+ * @name es
+ * @description This is the result of calling esFactory. Where is esFactory
+ * defined?
+ */
+
 let es; // share the client amongst all apps
 uiModules
   .get('kibana', ['elasticsearch', 'kibana/config'])

src/ui/public/events.js

@@ -1,3 +1,10 @@
+
+/**
+ * @name Events
+ *
+ * @extends SimpleEmitter
+ */
+
 import _ from 'lodash';
 import Notifier from 'ui/notify/notifier';
 import SimpleEmitter from 'ui/utils/simple_emitter';

src/ui/public/persisted_state/persisted_state.js

@@ -1,3 +1,10 @@
+
+/**
+ * @name PersistedState
+ *
+ * @extends Events
+ */
+
 import _ from 'lodash';
 import toPath from 'lodash/internal/toPath';
 import errors from 'ui/errors';
@@ -269,4 +276,4 @@ export default function (Private) {
   };
 
   return PersistedState;
-};
+};

src/ui/public/state_management/app_state.js

@@ -1,3 +1,14 @@
+
+/**
+ * @name AppState
+ *
+ * @extends State
+ *
+ * @description Inherits State, which inherits Events. This class seems to be
+ * concerned with mapping "props" to PersistedState instances, and surfacing the
+ * ability to destroy those mappings.
+ */
+
 import _ from 'lodash';
 import modules from 'ui/modules';
 import StateManagementStateProvider from 'ui/state_management/state';
@@ -12,7 +23,13 @@ function AppStateProvider(Private, $rootScope, $location) {
 
   _.class(AppState).inherits(State);
   function AppState(defaults) {
+    // Initialize persistedStates. This object maps "prop" names to
+    // PersistedState instances. These are used to make properties "stateful".
     persistedStates = {};
+
+    // Initialize eventUnsubscribers. These will be called in `destroy`, to
+    // remove handlers for the 'change' and 'fetch_with_changes' events which
+    // are dispatched via the rootScope.
     eventUnsubscribers = [];
 
     AppState.Super.call(this, urlParam, defaults);
@@ -28,6 +45,9 @@ function AppStateProvider(Private, $rootScope, $location) {
     _.callEach(eventUnsubscribers);
   };
 
+  /**
+   * @returns PersistedState instance.
+   */
   AppState.prototype.makeStateful = function (prop) {
     if (persistedStates[prop]) return persistedStates[prop];
     let self = this;
@@ -38,8 +58,8 @@ function AppStateProvider(Private, $rootScope, $location) {
     // update the app state when the stateful instance changes
     let updateOnChange = function () {
       let replaceState = false; // TODO: debouncing logic
-
       self[prop] = persistedStates[prop].getChanges();
+      // Save state to the URL.
       self.save(replaceState);
     };
     let handlerOnChange = (method) => persistedStates[prop][method]('change', updateOnChange);

src/ui/public/state_management/state.js

@@ -1,3 +1,12 @@
+
+/**
+ * @name State
+ *
+ * @extends Events
+ *
+ * @description Persists generic "state" to and reads it from the URL.
+ */
+
 import _ from 'lodash';
 import rison from 'rison-node';
 import applyDiff from 'ui/utils/diff_object';

src/ui/public/vis/agg_config.js

@@ -1,3 +1,10 @@
+
+/**
+ * @name AggConfig
+ *
+ * @description This class represents an aggregation.
+ */
+
 import _ from 'lodash';
 import RegistryFieldFormatsProvider from 'ui/registry/field_formats';
 export default function AggConfigFactory(Private, fieldTypeFilter) {
@@ -177,7 +184,7 @@ export default function AggConfigFactory(Private, fieldTypeFilter) {
 
   /**
    * Hook into param onRequest handling, and tell the aggConfig that it
-   * is being sent to elasticsearc.
+   * is being sent to elasticsearch.
    *
    * @return {[type]} [description]
    */
@@ -189,7 +196,7 @@ export default function AggConfigFactory(Private, fieldTypeFilter) {
   };
 
   /**
-   * Convert this aggConfig to it's dsl syntax.
+   * Convert this aggConfig to its dsl syntax.
    *
    * Adds params and adhoc subaggs to a pojo, then returns it
    *

src/ui/public/vis/agg_configs.js

@@ -1,3 +1,13 @@
+
+/**
+ * @name AggConfig
+ *
+ * @extends IndexedArray
+ *
+ * @description A "data structure"-like class with methods for indexing and
+ * accessing instances of AggConfig.
+ */
+
 import _ from 'lodash';
 import IndexedArray from 'ui/indexed_array';
 import VisAggConfigProvider from 'ui/vis/agg_config';

src/ui/public/vis/vis.js

@@ -1,3 +1,14 @@
+
+/**
+ * @name Vis
+ *
+ * @description This class consists of aggs, params, listeners, title, and type.
+ *  - Aggs: Instances of AggConfig.
+ *  - Params: Are these the settings in the Options tab?
+ *
+ * Not to be confused with vislib/vis.js.
+ */
+
 import _ from 'lodash';
 import AggTypesIndexProvider from 'ui/agg_types/index';
 import RegistryVisTypesProvider from 'ui/registry/vis_types';
@@ -24,7 +35,6 @@ export default function VisFactory(Notifier, Private) {
 
     this.indexPattern = indexPattern;
 
-    // http://aphyr.com/data/posts/317/state.gif
     this.setState(state);
     this.setUiState(uiState);
   }
@@ -36,6 +46,7 @@ export default function VisFactory(Notifier, Private) {
 
     let schemas = type.schemas;
 
+    // NOTE: What is this doing?
     let aggs = _.transform(oldState, function (newConfigs, oldConfigs, oldGroupName) {
       let schema = schemas.all.byName[oldGroupName];
 
@@ -119,6 +130,7 @@ export default function VisFactory(Notifier, Private) {
   };
 
   Vis.prototype.requesting = function () {
+    // Invoke requesting() on each agg. Aggs is an instance of AggConfigs.
     _.invoke(this.aggs.getRequestAggs(), 'requesting');
   };
 
@@ -149,6 +161,11 @@ export default function VisFactory(Notifier, Private) {
   Vis.prototype.getUiState = function () {
     return this.__uiState;
   };
+
+  /**
+   * Currently this is only used to extract map-specific information
+   * (e.g. mapZoom, mapCenter).
+   */
   Vis.prototype.uiStateVal = function (key, val) {
     if (this.hasUiState()) {
       if (_.isUndefined(val)) {