Merge pull request #262 from GoogleCloudPlatform/joemu-dev-fix-warnings

Fix Closure Compiler Warnings

Author: jmuharsky

client/components/config/config-service_test.js

@@ -192,7 +192,6 @@ describe('configService', function() {
       provided_other_project = 'PROVIDED_OTHER_PROJECT';
 
       provided_object = {
-        'other_property': provided_other_value,
         'default_project': provided_other_project
       };
 
@@ -206,7 +205,6 @@ describe('configService', function() {
       };
 
       expected_data = {
-        'other_property': provided_other_value,
         'default_project': provided_default_project,
         'default_dataset': provided_default_dataset,
         'default_table': provided_default_table,
@@ -219,7 +217,9 @@ describe('configService', function() {
       };
 
       svc.populate(provided_data);
-      expect(svc.toJSON(provided_object)).toEqual(expected_data);
+      var actualData = svc.toJSON(provided_object);
+
+      expect(actualData).toEqual(expected_data);
     });
   });
 

client/components/dashboard/dashboard-data-service.js

@@ -195,7 +195,7 @@ DashboardDataService.prototype.update = function(dashboardConfig) {
 /**
  * Deletes the given dashboard on the server based on its id.
  *
- * @param {!number} dashboard_id
+ * @param {string} dashboard_id
  * @return {!angular.$q.Promise}
  */
 DashboardDataService.prototype.delete = function(dashboard_id) {
@@ -206,53 +206,64 @@ DashboardDataService.prototype.delete = function(dashboard_id) {
 /**
  * Creates a copy of the dashboard, with an optional new name.
  *
- * @param {!number} dashboard_id
+ * @param {?string} dashboardId
  * @param {?string=} opt_title The name of the new dashboard.  If omitted,
  *     will have the same name as the original.
  * @return {!angular.$q.Promise}
  */
-DashboardDataService.prototype.copy = function(dashboard_id, opt_title) {
+DashboardDataService.prototype.copy = function(dashboardId, opt_title) {
+  if (!dashboardId) {
+    throw 'DashboadDataService.copy() failed: Dashboard id is required.';
+  }
+
   return this.post(
-      '/dashboard/copy', {id: dashboard_id, title: opt_title}, null);
+      '/dashboard/copy', {id: dashboardId, title: opt_title}, null);
 };
 
 
 /**
  * Changes the name of a dashboard.
  *
- * @param {!number} dashboard_id
- * @param {!string} title The title of the new dashboard.  If omitted, will
- *     have the same title as the original.
+ * @param {?string} dashboardId
+ * @param {string} title The title of the new dashboard.
  * @return {!angular.$q.Promise}
  */
-DashboardDataService.prototype.rename = function(dashboard_id, title) {
+DashboardDataService.prototype.rename = function(dashboardId, title) {
+  if (!dashboardId) {
+    throw 'DashboadDataService.rename() failed: Dashboard id is required.';
+  }
+
   return this.post(
-      '/dashboard/rename', {id: dashboard_id, title: title}, null);
+      '/dashboard/rename', {id: dashboardId, title: title}, null);
 };
 
 
 /**
  * Transfers a dashboard to a new owner, based on the provided email address.
  *
- * @param {!number} dashboard_id
- * @param {!string} new_owner_email The email address of the new owner.  If
+ * @param {?string} dashboardId
+ * @param {string} newOwnerEmail The email address of the new owner.  If
  *     the email address cannot be resolved, no change will take place.
  * @return {!angular.$q.Promise}
  */
 DashboardDataService.prototype.editOwner = function(
-    dashboard_id, new_owner_email) {
+    dashboardId, newOwnerEmail) {
+  if (!dashboardId) {
+    throw 'DashboadDataService.editOwner() failed: Dashboard id is required.';
+  }
+
   return this.post(
       '/dashboard/edit-owner',
-      {id: dashboard_id, email: new_owner_email}, null);
+      {id: dashboardId, email: newOwnerEmail}, null);
 };
 
 
 /**
  * Returns a list of dashboards.
  *
- * @param {boolean=} opt_mine If true, limits the list to items owned by the
+ * @param {?boolean=} opt_mine If true, limits the list to items owned by the
  *     current user.  This setting is not useful is opt_owner is specified.
- * @param {string=} opt_owner If provided, limits the list to dashboards owned
+ * @param {?string=} opt_owner If provided, limits the list to dashboards owned
  *     by the provided email address.
  *
  * @return {!angular.$q.Promise}

client/components/dashboard/dashboard-directive.js

@@ -20,13 +20,17 @@
 
 goog.provide('p3rf.perfkit.explorer.components.dashboard.DashboardDirective');
 
+goog.require('p3rf.perfkit.explorer.components.container.ContainerWidgetConfig');
 goog.require('p3rf.perfkit.explorer.components.dashboard.DashboardService');
 goog.require('p3rf.perfkit.explorer.models.ChartType');
+goog.require('p3rf.perfkit.explorer.models.WidgetConfig');
 
 
 goog.scope(function() {
 const explorer = p3rf.perfkit.explorer;
 const ChartType = explorer.models.ChartType;
+const ContainerWidgetConfig = explorer.components.container.ContainerWidgetConfig;
+const WidgetConfig = explorer.models.WidgetConfig;
 
 
 /**
@@ -112,7 +116,7 @@ explorer.components.dashboard.DashboardDirective = function() {
       /**
        * Returns true if the widget should scroll its overflow, otherwise stretch.
        * @param {!WidgetConfig} widget
-       * @param {!ContainerConfig} container
+       * @param {!ContainerWidgetConfig} container
        */
       $scope.isWidgetScrollable = function(widget, container) {
         // TODO: Replace with data-driven constraints for visualizations that support scrolling.
@@ -132,7 +136,13 @@ explorer.components.dashboard.DashboardDirective = function() {
         }
       }
 
-      /** @export {number} */
+      /**
+       * Returns the effective width of a widget within a container. 
+       * @param {!WidgetConfig} widget
+       * @param {!ContainerWidgetConfig} container
+       * @return {number}
+       * @export
+       */
       $scope.getWidgetFlexWidth = function(widget, container) {
         let widgetSpan = widget.model.layout.columnspan;
         let totalSpan = container.model.container.columns;

client/components/dashboard/dashboard-service.js

@@ -51,6 +51,7 @@ const ChartWidgetConfig = explorer.models.ChartWidgetConfig;
 const ConfigService = explorer.components.config.ConfigService;
 const ContainerWidgetConfig = explorer.components.container.ContainerWidgetConfig;
 const DashboardInstance = explorer.components.dashboard.DashboardInstance;
+const DashboardModel = explorer.components.dashboard.DashboardModel;
 const DashboardParam = explorer.components.dashboard.DashboardParam;
 const DashboardDataService = explorer.components.dashboard.DashboardDataService;
 const ExplorerStateService = explorer.components.explorer.ExplorerStateService;
@@ -156,7 +157,7 @@ explorer.components.dashboard.DashboardService = function(arrayUtilService,
   ];
 
   Object.defineProperty(this, 'current', {
-    /** @export {function(): explorer.components.dashboard.DashboardModel} */
+    /** @export {function(): explorer.components.dashboard.DashboardInstance} */
     get: function() {
       return explorerStateService.selectedDashboard;
     }
@@ -308,16 +309,16 @@ DashboardService.prototype.saveDashboardCopy = function() {
  * Set the current dashboard and the set the widgets array to reference the
  * dashboard's widgets.
  *
- * @param {!DashboardInstance} dashboardConfig
+ * @param {!DashboardInstance} dashboard
  * @export
  */
-DashboardService.prototype.setDashboard = function(dashboardConfig) {
-  this.explorerStateService_.selectedDashboard = dashboardConfig;
-  if (dashboardConfig) {
+DashboardService.prototype.setDashboard = function(dashboard) {
+  this.explorerStateService_.selectedDashboard = dashboard;
+  if (dashboard) {
     this.explorerStateService_.widgets.clear();
     this.explorerStateService_.containers.clear();
 
-    for (let container of dashboardConfig.model.children) {
+    for (let container of dashboard.model.children) {
       this.explorerStateService_.containers.all[container.model.id] = container;
       if (container.model.id ===
           this.explorerStateService_.containers.selectedId) {
@@ -416,17 +417,17 @@ DashboardService.prototype.selectWidget = function(
     }
   }
 
-  if (widget) {
+  if (goog.isDefAndNotNull(widget)) {
     this.sidebarTabService_.resolveSelectedTabForWidget();
 
     this.timeout_(() => {
-      this.scrollWidgetIntoView(widget);
+      this.scrollWidgetIntoView(/** @type {!WidgetConfig} */ (widget));
     });
-  } else if (container) {
+  } else if (goog.isDefAndNotNull(container)) {
     this.sidebarTabService_.resolveSelectedTabForContainer();
 
     this.timeout_(() => {
-      this.scrollContainerIntoView(container);
+      this.scrollContainerIntoView(/** @type {!ContainerWidgetConfig} */ (container));
     });
   } else {
     this.timeout_(() => {
@@ -535,7 +536,7 @@ DashboardService.prototype.selectContainer = function(
 
 /**
  * Rewrites the current widget's query based on the config.
- * @param {!Widget} widget The widget to rewrite the query against.
+ * @param {!WidgetConfig} widget The widget to rewrite the query against.
  * @param {boolean=} replaceParams If true, parameters (%%NAME%%) will be
  *     replaced with the current param value (from the dashboard or url).
  *     Defaults to false.
@@ -547,45 +548,47 @@ DashboardService.prototype.rewriteQuery = function(widget, replaceParams) {
 
   let widgetConfig = widget.model.datasource.config;
 
-  let project_name = this.arrayUtilService_.getFirst([
+  let project_name = (this.arrayUtilService_.getFirst([
       widgetConfig.results.project_id,
       this.current.model.project_id,
-      this.config.default_project], false);
-  if (project_name === null) {
-    this.errorService_.addError(ErrorTypes.DANGER, 'Project name not found.');
+      this.config.default_project], false));
+  if (goog.isNull(project_name)) {
+    throw 'Project name not found.';
   }
 
-  let dataset_name = this.arrayUtilService_.getFirst([
+  let dataset_name = (this.arrayUtilService_.getFirst([
       widgetConfig.results.dataset_name,
       this.current.model.dataset_name,
-      this.config.default_dataset], false);
-  if (project_name === null) {
-    this.errorService_.addError(ErrorTypes.DANGER, 'Dataset name not found.');
+      this.config.default_dataset], false));
+  if (goog.isNull(dataset_name)) {
+    throw 'Dataset name not found.';
   }
 
-  let table_name = this.arrayUtilService_.getFirst([
+  let table_name = (this.arrayUtilService_.getFirst([
       widgetConfig.results.table_name,
       this.current.model.table_name,
-      this.config.default_table], false);
-  if (project_name === null) {
-    this.errorService_.addError(ErrorTypes.DANGER, 'Table name not found.');
+      this.config.default_table], false));
+  if (goog.isNull(table_name)) {
+    throw 'Table name not found.';
   }
 
-  let table_partition = this.arrayUtilService_.getFirst([
+  let table_partition = (this.arrayUtilService_.getFirst([
       widgetConfig.results.table_partition,
       this.current.model.table_partition,
-      this.DEFAULT_TABLE_PARTITION], false);
-  if (project_name === null) {
-    this.errorService_.addError(ErrorTypes.DANGER,
-                                'Table partition not found.');
+      this.DEFAULT_TABLE_PARTITION], false));
+  if (goog.isNull(table_partition)) {
+    throw 'Table partition not found.';
   }
 
   this.initializeParams_();
   let params = replaceParams ? this.params : null;
 
   return this.queryBuilderService_.getSql(
         widget.model.datasource.config,
-        project_name, dataset_name, table_name, table_partition, params);
+        /** @type {string} */ (project_name),
+        /** @type {string} */ (dataset_name),
+        /** @type {string} */ (table_name),
+        /** @type {!QueryTablePartitioning} */ (table_partition), params);
 };
 
 /**

client/components/dashboard/dashboard-version-model.js

@@ -50,7 +50,7 @@ const DashboardModel = explorer.components.dashboard.DashboardModel;
 explorer.components.dashboard.DashboardVersionModel = function(
     opt_version, opt_verify, opt_update) {
   /**
-   * @type {?string}
+   * @type {string}
    * @export
    */
   this.version = opt_version || '';

client/components/dashboard/dashboard-version-service.js

@@ -87,7 +87,7 @@ explorer.components.dashboard.DashboardVersionService = function($filter) {
 
   /**
    * The version to target for updates.  By default, this is the most recent version.
-   * @type {?DashboardVersionModel}
+   * @type {!DashboardVersionModel}
    * @export
    */
   this.currentVersion = this.versions[0];
@@ -112,15 +112,15 @@ DashboardVersionService.prototype.verifyAndUpdateModel = function(dashboard) {
   // If the version is not current, run the update script to bring the version
   // current.
   if (version == this.currentVersion) {
-    if (!goog.isDef(dashboard.version)) {
-      dashboard.version = version.version;
-    }
+    dashboard.version = version.version;
   } else {
     let dashboard_version_index = this.versions.indexOf(version);
+    
     let current_version_index = this.versions.indexOf(this.currentVersion);
 
     for (let i = dashboard_version_index - 1; i >= current_version_index; --i) {
       let update_version = this.versions[i];
+
       update_version.update(dashboard);
       dashboard.version = update_version.version;
     }

client/components/dashboard/dashboard-version-service_test.js

@@ -101,11 +101,10 @@ describe('dashboardVersionService', function() {
         expect(svc.currentVersion).not.toBeNull();
       });
 
-      it('should update the email, config, custom_query and querystring.',
+      it('should update the email, config and custom_query.',
           function() {
             providedDatasource = {
-              'query': 'TEST_QUERY',
-              'querystring': 'product_name=SAMPLE_PRODUCT'};
+              'query': 'TEST_QUERY'};
 
             providedDashboard = {
               'owner': 'TEST_OWNER',
@@ -123,7 +122,6 @@ describe('dashboardVersionService', function() {
 
             expectedOwner = 'TEST_OWNER';
             expectedConfig = new QueryConfigModel();
-            expectedConfig.filters.product_name = 'SAMPLE_PRODUCT';
 
             svc.verifyAndUpdateModel(providedDashboard);
 
@@ -137,8 +135,7 @@ describe('dashboardVersionService', function() {
 
       it('should add a custom_query flag when no query is present.',
          function() {
-           providedDatasource = {
-             'querystring': 'product_name=SAMPLE_PRODUCT'};
+           providedDatasource = {};
 
            providedDashboard = {
              'owner': 'TEST_OWNER',
@@ -156,7 +153,6 @@ describe('dashboardVersionService', function() {
 
            expectedOwner = 'TEST_OWNER';
            expectedConfig = new QueryConfigModel();
-           expectedConfig.filters.product_name = 'SAMPLE_PRODUCT';
 
            svc.verifyAndUpdateModel(providedDashboard);
 

client/components/dashboard/versions/dashboard-schema_02.js

@@ -65,14 +65,6 @@ goog.scope(function() {
       if (!widget.datasource.config) {
         widget.datasource.config = new QueryConfigModel();
       }
-
-      // If a querystring is present, apply it to the config object.
-      if (widget.datasource.querystring) {
-        QueryConfigModel.applyQueryString(
-            widget.datasource.config,
-            widget.datasource.querystring);
-        delete widget.datasource.querystring;
-      }
     });
   };
 });