#5791: Address errors found by lgtm.com

superset/assets/spec/javascripts/sqllab/actions/sqlLab_spec.js

@@ -85,6 +85,7 @@ describe('async actions', () => {
       });
     });
 
+    /* eslint jest/no-disabled-tests: 0 */
     xit('parses large number result without losing precision', () =>
       makeRequest().then(() => {
         expect(fetchMock.calls(fetchQueryEndpoint)).toHaveLength(1);
@@ -138,6 +139,7 @@ describe('async actions', () => {
       });
     });
 
+    /* eslint jest/no-disabled-tests: 0 */
     xit('parses large number result without losing precision', () =>
       makeRequest().then(() => {
         expect(fetchMock.calls(runQueryEndpoint)).toHaveLength(1);

superset/assets/src/SqlLab/components/ScheduleQueryButton.jsx

@@ -125,8 +125,8 @@ class ScheduleQueryButton extends React.PureComponent {
   onDescriptionChange(e) {
     this.setState({ description: e.target.value });
   }
-  toggleSchedule(e) {
-    this.setState({ target: e.target, showSchedule: !this.state.showSchedule });
+  toggleSchedule() {
+    this.setState({ showSchedule: !this.state.showSchedule });
   }
   renderModalBody() {
     return (

superset/assets/src/explore/components/ExploreViewContainer.jsx

@@ -99,7 +99,7 @@ class ExploreViewContainer extends React.Component {
     // Trigger the chart if there are no errors
     const { chart } = this.props;
     if (!this.hasErrors()) {
-      this.props.actions.triggerQuery(true, this.props.chart.id);
+      this.props.actions.triggerQuery(true, chart.id);
     }
   }
 
@@ -112,7 +112,7 @@ class ExploreViewContainer extends React.Component {
       (this.props.controls.datasource == null ||
         nextProps.controls.datasource.value !== this.props.controls.datasource.value)
     ) {
-      fetchDatasourceMetadata(nextProps.form_data.datasource, true);
+      fetchDatasourceMetadata(nextProps.form_data.datasource);
     }
 
     const changedControlKeys = this.findChangedControlKeys(this.props.controls, nextProps.controls);
@@ -273,8 +273,8 @@ class ExploreViewContainer extends React.Component {
     // Returns an error message as a node if any errors are in the store
     const errors = [];
     const ctrls = this.props.controls;
-    for (const controlName in this.props.controls) {
-      const control = this.props.controls[controlName];
+    for (const controlName in ctrls) {
+      const control = ctrls[controlName];
       if (control.validationErrors && control.validationErrors.length > 0) {
         errors.push(
           <div key={controlName}>

superset/assets/src/explore/components/controls/withVerification.jsx

@@ -56,8 +56,9 @@ export default function withVerification(WrappedComponent, optionLabel, optionsN
           endpoint,
         }).then(({ json }) => {
           if (Array.isArray(json)) {
-            this.setState({ validOptions: new Set(json) || new Set() });
+            this.setState({ validOptions: new Set(json) });
           }
+          /* eslint no-console: 0 */
         }).catch(error => console.log(error));
 
         if (!this.state.hasRunVerification) {

superset/assets/src/explore/controls.jsx

@@ -1470,7 +1470,7 @@ export const controls = {
     renderTrigger: true,
     default: true,
     description: (
-      'Whether to apply filters as they change, or wait for' +
+      'Whether to apply filters as they change, or wait for ' +
       'users to hit an [Apply] button'
     ),
   },

superset/assets/src/explore/exploreUtils.js

@@ -47,8 +47,7 @@ export function getAnnotationJsonUrl(slice_id, form_data, isNative) {
   const endpoint = isNative ? 'annotation_json' : 'slice_json';
   return uri.pathname(`/superset/${endpoint}/${slice_id}`)
     .search({
-      form_data: safeStringify(form_data,
-        (key, value) => value === null ? undefined : value),
+      form_data: safeStringify(form_data),
     }).toString();
 }
 

superset/connectors/druid/models.py

@@ -80,6 +80,7 @@ def _fetch_metadata_for(datasource):
 
 class JavascriptPostAggregator(Postaggregator):
     def __init__(self, name, field_names, function):
+        super(JavascriptPostAggregator, self).__init__(function, field_names, name)
         self.post_aggregator = {
             "type": "javascript",
             "fieldNames": field_names,
@@ -93,6 +94,7 @@ class CustomPostAggregator(Postaggregator):
     """A way to allow users to specify completely custom PostAggregators"""
 
     def __init__(self, name, post_aggregator):
+        super(CustomPostAggregator, self).__init__(None, None, name)
         self.name = name
         self.post_aggregator = post_aggregator
 

superset/connectors/druid/views.py

@@ -433,7 +433,6 @@ def refresh_datasources(self, refreshAll=True):
                     "danger",
                 )
                 logging.exception(e)
-                pass
             if valid_cluster:
                 cluster.metadata_last_refreshed = datetime.now()
                 flash(

superset/data/energy.py

@@ -26,7 +26,6 @@
 from superset.connectors.sqla.models import SqlMetric
 from superset.utils import core as utils
 from .helpers import (
-    DATA_FOLDER,
     get_example_data,
     merge_slice,
     misc_dash_slices,

superset/data/tabbed_dashboard.py

@@ -17,25 +17,12 @@
 """Loads datasets, dashboards and slices in a new superset instance"""
 # pylint: disable=C,R,W
 import json
-import os
 import textwrap
 
-import pandas as pd
-from sqlalchemy import DateTime, String
-
 from superset import db
-from superset.connectors.sqla.models import SqlMetric
-from superset.utils import core as utils
 from .helpers import (
-    config,
     Dash,
-    DATA_FOLDER,
-    get_example_data,
-    get_slice_json,
-    merge_slice,
-    misc_dash_slices,
     Slice,
-    TBL,
     update_slice_ids,
 )
 

superset/migrations/versions/45e7da7cfeba_.py

@@ -26,9 +26,6 @@
 revision = "45e7da7cfeba"
 down_revision = ("e553e78e90c5", "c82ee8a39623")
 
-from alembic import op
-import sqlalchemy as sa
-
 
 def upgrade():
     pass

superset/migrations/versions/80aa3f04bc82_add_parent_ids_in_dashboard_layout.py

@@ -27,9 +27,7 @@
 from alembic import op
 from sqlalchemy import Column, Integer, Text
 from sqlalchemy.ext.declarative import declarative_base
-from sqlalchemy.orm import relationship
 from alembic import op
-import sqlalchemy as sa
 
 from superset import db
 

superset/stats_logger.py

@@ -83,6 +83,7 @@ def __init__(
             If statsd_client argument is given, all other arguments are ignored and the
             supplied client will be used to emit metrics.
             """
+            super(StatsdStatsLogger, self).__init__(prefix)
             if statsd_client:
                 self.client = statsd_client
             else:

superset/tasks/cache.py

@@ -262,6 +262,7 @@ def cache_warmup(strategy_name, *args, **kwargs):
 
     """
     logger.info("Loading strategy")
+
     class_ = None
     for class_ in strategies:
         if class_.name == strategy_name:

superset/utils/core.py

@@ -121,7 +121,7 @@ def __repr__(self):
 
     def __get__(self, obj, objtype):
         if not self.is_method:
-            self.is_method = True
+            self.is_method = True  # lgtm[py/mutable-descriptor]
         """Support instance methods."""
         return functools.partial(self.__call__, obj)
 

superset/views/core.py

@@ -1074,8 +1074,10 @@ def request_access(self):
         dashboard_id = request.args.get("dashboard_id")
         if dashboard_id:
             dash = (
-                db.session.query(models.Dashboard).filter_by(id=int(dashboard_id)).one()
+                db.session.query(models.Dashboard).filter_by(id=int(dashboard_id)).one_or_none()
             )
+            if not dash:
+                abort(404)
             datasources |= dash.datasources
         datasource_id = request.args.get("datasource_id")
         datasource_type = request.args.get("datasource_type")
@@ -1879,7 +1881,7 @@ def save_dash(self, dashboard_id):
         dash = session.query(models.Dashboard).filter_by(id=dashboard_id).first()
         check_ownership(dash, raise_if_false=True)
         data = json.loads(request.form.get("data"))
-        self._set_dash_metadata(dash, data)
+        Superset._set_dash_metadata(dash, data)
         session.merge(dash)
         session.commit()
         session.close()
@@ -2580,7 +2582,6 @@ def table(self, database_id, table_name, schema):
             except Exception:
                 # sqla.types.JSON __str__ has a bug, so using __class__.
                 dtype = col["type"].__class__.__name__
-                pass
             payload_columns.append(
                 {
                     "name": col["name"],

tests/access_tests.py

@@ -479,6 +479,14 @@ def test_approve(self, mock_send_mime):
             session.delete(security_manager.find_role(TEST_ROLE_NAME))
             session.commit()
 
+    def test_dashboard_endpoint_malicious_redirect(self):
+        unused_dashboard_id = 9999
+        resp = self.client.get(
+            '/superset/request_access?dashboard_id={}&action=go&'.format(
+                unused_dashboard_id),
+            follow_redirects=True)
+        assert resp.status_code == 404
+
     def test_request_access(self):
         if app.config.get("ENABLE_ACCESS_REQUEST"):
             session = db.session