Merge pull request #491 from CartoDB/issue-375

Sort start and end override params to correct bins width

lib/cartodb/models/dataview/histogram.js

@@ -174,8 +174,8 @@ Histogram.prototype.sql = function(psql, override, callback) {
         basicsQuery = overrideBasicsQueryTpl({
             _query: _query,
             _column: _column,
-            _start: override.start,
+            _start: getBinStart(override),
-            _end: override.end
+            _end: getBinEnd(override)
         });
 
         binsQuery = [
@@ -248,7 +248,7 @@ Histogram.prototype.format = function(result, override) {
         width = firstRow.bin_width || width;
         avg = firstRow.avg_val;
         nulls = firstRow.nulls_count;
-        binsStart = override.hasOwnProperty('start') ? override.start : firstRow.min;
+        binsStart = override.hasOwnProperty('start') ? getBinStart(override) : firstRow.min;
 
         buckets = result.rows.map(function(row) {
             return _.omit(row, 'bins_number', 'bin_width', 'nulls_count', 'avg_val');
@@ -266,9 +266,19 @@ Histogram.prototype.format = function(result, override) {
 };
 
 function getBinStart(override) {
+    if (override.hasOwnProperty('start') && override.hasOwnProperty('end')) {
+        return Math.min(override.start, override.end);
+    }
     return override.start || 0;
 }
 
+function getBinEnd(override) {
+    if (override.hasOwnProperty('start') && override.hasOwnProperty('end')) {
+        return Math.max(override.start, override.end);
+    }
+    return override.end || 0;
+}
+
 function getBinsCount(override) {
     return override.bins || 0;
 }

test/acceptance/dataviews/histogram.js

@@ -0,0 +1,80 @@
+require('../../support/test_helper');
+
+var assert = require('../../support/assert');
+var TestClient = require('../../support/test-client');
+
+describe('histogram-dataview', function() {
+
+    afterEach(function(done) {
+        if (this.testClient) {
+            this.testClient.drain(done);
+        } else {
+            done();
+        }
+    });
+
+    function createMapConfig(layers, dataviews, analysis) {
+        return {
+            version: '1.5.0',
+            layers: layers,
+            dataviews: dataviews || {},
+            analyses: analysis || []
+        };
+    }
+
+    var mapConfig = createMapConfig(
+        [
+            {
+                "type": "cartodb",
+                "options": {
+                    "source": {
+                        "id": "2570e105-7b37-40d2-bdf4-1af889598745"
+                    },
+                    "cartocss": "#points { marker-width: 10; marker-fill: red; }",
+                    "cartocss_version": "2.3.0"
+                }
+            }
+        ],
+        {
+            pop_max_histogram: {
+                source: {
+                    id: '2570e105-7b37-40d2-bdf4-1af889598745'
+                },
+                type: 'histogram',
+                options: {
+                    column: 'x'
+                }
+            }
+        },
+        [
+            {
+                "id": "2570e105-7b37-40d2-bdf4-1af889598745",
+                "type": "source",
+                "params": {
+                    "query": "select null::geometry the_geom_webmercator, x from generate_series(0,1000) x"
+                }
+            }
+        ]
+    );
+
+    it('should get bin_width right when max > min in filter', function(done) {
+        var params = {
+            bins: 10,
+            start: 1e3,
+            end: 0
+        };
+
+        this.testClient = new TestClient(mapConfig, 1234);
+        this.testClient.getDataview('pop_max_histogram', params, function(err, dataview) {
+            assert.ok(!err, err);
+
+            assert.equal(dataview.type, 'histogram');
+            assert.ok(dataview.bin_width > 0, 'Unexpected bin width: ' + dataview.bin_width);
+            dataview.bins.forEach(function(bin) {
+                assert.ok(bin.min <= bin.max, 'bin min < bin max: ' + JSON.stringify(bin));
+            });
+
+            done();
+        });
+    });
+});

test/support/test-client.js

@@ -310,9 +310,13 @@ TestClient.prototype.getDataview = function(dataviewName, params, callback) {
             var urlParams = {
                 own_filter: params.hasOwnProperty('own_filter') ? params.own_filter : 1
             };
-            if (params && params.bbox) {
+
-                urlParams.bbox = params.bbox;
+            ['bbox', 'bins', 'start', 'end'].forEach(function(extraParam) {
-            }
+                if (params.hasOwnProperty(extraParam)) {
+                    urlParams[extraParam] = params[extraParam];
+                }
+            });
+
             if (self.apiKey) {
                 urlParams.api_key = self.apiKey;
             }
@@ -341,10 +345,10 @@ TestClient.prototype.getDataview = function(dataviewName, params, callback) {
                 }
             );
         },
-        function finish(err, res) {
+        function finish(err, dataview) {
             self.keysToDelete['map_cfg|' + LayergroupToken.parse(layergroupId).token] = 0;
             self.keysToDelete['user:localhost:mapviews:global'] = 5;
-            return callback(err, res);
+            return callback(err, dataview);
         }
     );
 };