Merge pull request #196 from EverythingMe/fixes

Bug fixes (#91, #195)

rd_ui/app/scripts/controllers/query_source.js

@@ -1,7 +1,7 @@
 (function() {
   'use strict';
 
-  function QuerySourceCtrl(Events, $controller, $scope, $location, Query, Visualization, KeyboardShortcuts) {
+  function QuerySourceCtrl(Events, growl, $controller, $scope, $location, Query, Visualization, KeyboardShortcuts) {
     // extends QueryViewCtrl
     $controller('QueryViewCtrl', {$scope: $scope});
     // TODO:
@@ -67,15 +67,19 @@
       if (confirm('Are you sure you want to delete ' + vis.name + ' ?')) {
         Events.record(currentUser, 'delete', 'visualization', vis.id);
 
-        Visualization.delete(vis);
-        if ($scope.selectedTab == vis.id) {
-          $scope.selectedTab = DEFAULT_TAB;
-          $location.hash($scope.selectedTab);
-        }
-        $scope.query.visualizations =
-          $scope.query.visualizations.filter(function(v) {
-            return vis.id !== v.id;
-          });
+        Visualization.delete(vis, function() {
+          if ($scope.selectedTab == vis.id) {
+            $scope.selectedTab = DEFAULT_TAB;
+            $location.hash($scope.selectedTab);
+          }
+          $scope.query.visualizations =
+            $scope.query.visualizations.filter(function (v) {
+              return vis.id !== v.id;
+            });
+        }, function () {
+          growl.addErrorMessage("Error deleting visualization. Maybe it's used in a dashboard?");
+        });
+
       }
     };
 
@@ -99,7 +103,7 @@
   }
 
   angular.module('redash.controllers').controller('QuerySourceCtrl', [
-    'Events', '$controller', '$scope', '$location', 'Query',
+    'Events', 'growl', '$controller', '$scope', '$location', 'Query',
     'Visualization', 'KeyboardShortcuts', QuerySourceCtrl
     ]);
 })();

rd_ui/app/scripts/services/resources.js

@@ -205,19 +205,35 @@
       return this.columns;
     }
 
-    QueryResult.prototype.getColumnCleanName = function (column) {
+
+    QueryResult.prototype.getColumnNameWithoutType = function (column) {
       var parts = column.split('::');
-      var name = parts[1];
-      if (parts[0] != '') {
-        // TODO: it's probably time to generalize this.
-        // see also getColumnFriendlyName
-        name = parts[0].replace(/%/g, '__pct').replace(/ /g, '_').replace(/\?/g, '');
+      return parts[0];
+    };
+
+    var charConversionMap = {
+      '__pct': /%/g,
+      '_': / /g,
+      '__qm': /\?/g,
+      '__brkt': /[\(\)\[\]]/g,
+      '__dash': /-/g,
+      '__amp': /&/g
+    };
+
+    QueryResult.prototype.getColumnCleanName = function (column) {
+      var name = this.getColumnNameWithoutType(column);
+
+      if (name != '') {
+        _.each(charConversionMap, function(regex, replacement) {
+          name = name.replace(regex, replacement);
+        });
       }
+
       return name;
     }
 
     QueryResult.prototype.getColumnFriendlyName = function (column) {
-      return this.getColumnCleanName(column).replace('__pct', '%').replace(/_/g, ' ').replace(/(?:^|\s)\S/g, function (a) {
+      return this.getColumnNameWithoutType(column).replace(/(?:^|\s)\S/g, function (a) {
         return a.toUpperCase();
       });
     }

rd_ui/app/scripts/visualizations/chart.js

@@ -101,6 +101,8 @@
               }
             });
 
+            scope.xAxisType = (scope.visualization.options.xAxis && scope.visualization.options.xAxis.type) || scope.xAxisType;
+
             xAxisUnwatch = scope.$watch("xAxisType", function (xAxisType) {
               scope.visualization.options.xAxis = scope.visualization.options.xAxis || {};
               scope.visualization.options.xAxis.type = xAxisType;

redash/data/query_runner_pg.py

@@ -43,12 +43,26 @@ def pg(connection_string):
             cursor.execute(query)
             wait(connection)
 
-            column_names = [col.name for col in cursor.description]
+            column_names = set()
+            columns = []
+            duplicates_counter = 1
+
+            for column in cursor.description:
+                # TODO: this deduplication needs to be generalized and reused in all query runners.
+                column_name = column.name
+                if column_name in column_names:
+                    column_name = column_name + str(duplicates_counter)
+                    duplicates_counter += 1
+
+                column_names.add(column_name)
+
+                columns.append({
+                    'name': column_name,
+                    'friendly_name': column_friendly_name(column_name),
+                    'type': None
+                })
 
             rows = [dict(zip(column_names, row)) for row in cursor]
-            columns = [{'name': col.name,
-                        'friendly_name': column_friendly_name(col.name),
-                        'type': None} for col in cursor.description]
 
             data = {'columns': columns, 'rows': rows}
             json_data = json.dumps(data, cls=JSONEncoder)

redash/data/worker.py

@@ -314,7 +314,7 @@ class Worker(multiprocessing.Process):
                              self.name, job_id)
                 job.done(None, "Interrupted/Cancelled while running.")
 
-            job.expire(24 * 3600)
+            job.expire(settings.JOB_EXPIRY_TIME)
 
             logging.info("[%s] Finished Processing %s (pid: %d status: %d)",
                          self.name, job_id, self.child_pid, status)

redash/models.py

@@ -133,8 +133,13 @@ class QueryResult(BaseModel):
     def get_latest(cls, data_source, query, ttl=0):
         query_hash = utils.gen_query_hash(query)
 
-        query = cls.select().where(cls.query_hash == query_hash, cls.data_source == data_source,
-                                   peewee.SQL("retrieved_at + interval '%s second' >= now() at time zone 'utc'", ttl)).order_by(cls.retrieved_at.desc())
+        if ttl == -1:
+            query = cls.select().where(cls.query_hash == query_hash,
+                                       cls.data_source == data_source).order_by(cls.retrieved_at.desc())
+        else:
+            query = cls.select().where(cls.query_hash == query_hash, cls.data_source == data_source,
+                                       peewee.SQL("retrieved_at + interval '%s second' >= now() at time zone 'utc'",
+                                                  ttl)).order_by(cls.retrieved_at.desc())
 
         return query.first()
 
@@ -261,7 +266,23 @@ class Dashboard(BaseModel):
                 .switch(Query)\
                 .join(QueryResult, join_type=peewee.JOIN_LEFT_OUTER)
             widgets = {w.id: w.to_dict() for w in widgets}
-            widgets_layout = map(lambda row: map(lambda widget_id: widgets.get(widget_id, None), row), layout)
+
+            # The following is a workaround for cases when the widget object gets deleted without the dashboard layout
+            # updated. This happens for users with old databases that didn't have a foreign key relationship between
+            # visualizations and widgets.
+            # It's temporary until better solution is implemented (we probably should move the position information
+            # to the widget).
+            widgets_layout = []
+            for row in layout:
+                new_row = []
+                for widget_id in row:
+                    widget = widgets.get(widget_id, None)
+                    if widget:
+                        new_row.append(widget)
+
+                widgets_layout.append(new_row)
+
+            # widgets_layout = map(lambda row: map(lambda widget_id: widgets.get(widget_id, None), row), layout)
         else:
             widgets_layout = None
 

redash/settings.py

@@ -63,6 +63,7 @@ ADMINS = array_from_string(os.environ.get("REDASH_ADMINS", ''))
 ALLOWED_EXTERNAL_USERS = array_from_string(os.environ.get("REDASH_ALLOWED_EXTERNAL_USERS", ''))
 STATIC_ASSETS_PATH = fix_assets_path(os.environ.get("REDASH_STATIC_ASSETS_PATH", "../rd_ui/app/"))
 WORKERS_COUNT = int(os.environ.get("REDASH_WORKERS_COUNT", "2"))
+JOB_EXPIRY_TIME = int(os.environ.get("REDASH_JOB_EXPIRY_TIME", 3600*24))
 COOKIE_SECRET = os.environ.get("REDASH_COOKIE_SECRET", "c292a0a3aa32397cdb050e233733900f")
 LOG_LEVEL = os.environ.get("REDASH_LOG_LEVEL", "INFO")
 EVENTS_LOG_PATH = os.environ.get("REDASH_EVENTS_LOG_PATH", "")

tests/test_models.py

@@ -80,4 +80,14 @@ class QueryResultTest(BaseTestCase):
 
         found_query_result = models.QueryResult.get_latest(qr.data_source, qr.query, 60)
 
+        self.assertEqual(found_query_result.id, qr.id)
+
+    def test_get_latest_returns_the_last_cached_result_for_negative_ttl(self):
+        yesterday = datetime.datetime.now() + datetime.timedelta(days=-100)
+        very_old = query_result_factory.create(retrieved_at=yesterday)
+
+        yesterday = datetime.datetime.now() + datetime.timedelta(days=-1)
+        qr = query_result_factory.create(retrieved_at=yesterday)
+        found_query_result = models.QueryResult.get_latest(qr.data_source, qr.query, -1)
+
         self.assertEqual(found_query_result.id, qr.id)