From a1e024e228973b4d6b4fa2260e9e4e485d54847d Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 18 May 2016 17:49:09 +0200 Subject: [PATCH 1/4] Fix dataview problem for bbox with no query rewrite data Fixes #457 --- lib/cartodb/backends/dataview.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/cartodb/backends/dataview.js b/lib/cartodb/backends/dataview.js index 44db254e..89b811bb 100644 --- a/lib/cartodb/backends/dataview.js +++ b/lib/cartodb/backends/dataview.js @@ -149,7 +149,9 @@ DataviewBackend.prototype.getDataview = function (mapConfigProvider, user, param bbox: params.bbox } }; - queryRewriteData = _.extend(queryRewriteData, { bbox_filter: bbox_filter_definition }); + if ( queryRewriteData ) { + queryRewriteData = _.extend(queryRewriteData, { bbox_filter: bbox_filter_definition }); + } } var dataviewFactory = DataviewFactoryWithOverviews.getFactory( From 5989ab344d450155f71885e8e32d6a6b59b156ac Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 18 May 2016 18:02:08 +0200 Subject: [PATCH 2/4] Add test to detect problem #457 --- test/acceptance/dataviews/overviews.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/acceptance/dataviews/overviews.js b/test/acceptance/dataviews/overviews.js index 9acda126..0ab7e8bd 100644 --- a/test/acceptance/dataviews/overviews.js +++ b/test/acceptance/dataviews/overviews.js @@ -58,6 +58,21 @@ describe('dataviews using tables without overviews', function() { }); }); + it("should admit a bbox", function(done) { + var params = { + bbox: "-170,-80,170,80" + }; + var testClient = new TestClient(nonOverviewsMapConfig); + testClient.getDataview('country_places_count', params, function(err, formula_result) { + if (err) { + return done(err); + } + assert.deepEqual(formula_result, { operation: 'count', result: 7253, nulls: 0, type: 'formula' }); + + testClient.drain(done); + }); + }); + describe('filters', function() { describe('category', function () { From 9206b1a1b502bf0d032de8cfc0c740d4a273233f Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 18 May 2016 18:16:32 +0200 Subject: [PATCH 3/4] Fix dataviews/overviews tests and add some new cases --- test/acceptance/dataviews/overviews.js | 161 ++++++++++++++++--------- 1 file changed, 106 insertions(+), 55 deletions(-) diff --git a/test/acceptance/dataviews/overviews.js b/test/acceptance/dataviews/overviews.js index 0ab7e8bd..601124a7 100644 --- a/test/acceptance/dataviews/overviews.js +++ b/test/acceptance/dataviews/overviews.js @@ -92,6 +92,23 @@ describe('dataviews using tables without overviews', function() { testClient.drain(done); }); }); + + it("should expose a filtered formula and admit a bbox", function (done) { + var params = { + filters: { + dataviews: {country_categories: {accept: ['CAN']}} + }, + bbox: "-170,-80,170,80" + }; + var testClient = new TestClient(nonOverviewsMapConfig); + testClient.getDataview('country_places_count', params, function (err, formula_result) { + if (err) { + return done(err); + } + assert.deepEqual(formula_result, { operation: 'count', result: 254, nulls: 0, type: 'formula' }); + testClient.drain(done); + }); + }); }); }); @@ -233,16 +250,32 @@ describe('dataviews using tables with overviews', function() { }); }); + it("should admit a bbox", function(done) { + var params = { + bbox: "-170,-80,170,80" + }; + var testClient = new TestClient(overviewsMapConfig); + testClient.getDataview('test_sum', params, function(err, formula_result) { + if (err) { + return done(err); + } + assert.deepEqual(formula_result, {"operation":"sum","result":15,"nulls":0,"type":"formula"}); + + testClient.drain(done); + }); + }); + describe('filters', function() { describe('category', function () { - it("should expose a filtered formula", function (done) { - var params = { - filters: { - dataviews: {test_categories: {accept: ['Hawai']}} - } - }; + var params = { + filters: { + dataviews: {test_categories: {accept: ['Hawai']}} + } + }; + + it("should expose a filtered sum formula", function (done) { var testClient = new TestClient(overviewsMapConfig); testClient.getDataview('test_sum', params, function (err, formula_result) { if (err) { @@ -251,56 +284,74 @@ describe('dataviews using tables with overviews', function() { assert.deepEqual(formula_result, {"operation":"sum","result":1,"nulls":0,"type":"formula"}); testClient.drain(done); }); - - it("should expose an avg formula", function(done) { - var testClient = new TestClient(overviewsMapConfig); - testClient.getDataview('test_avg', { own_filter: 0 }, function(err, formula_result) { - if (err) { - return done(err); - } - assert.deepEqual(formula_result, {"operation":"avg","result":1,"nulls":0,"type":"formula"}); - - testClient.drain(done); - }); - }); - - it("should expose a count formula", function(done) { - var testClient = new TestClient(overviewsMapConfig); - testClient.getDataview('test_count', { own_filter: 0 }, function(err, formula_result) { - if (err) { - return done(err); - } - assert.deepEqual(formula_result, {"operation":"count","result":1,"nulls":0,"type":"formula"}); - - testClient.drain(done); - }); - }); - - it("should expose a max formula", function(done) { - var testClient = new TestClient(overviewsMapConfig); - testClient.getDataview('test_max', { own_filter: 0 }, function(err, formula_result) { - if (err) { - return done(err); - } - assert.deepEqual(formula_result, {"operation":"max","result":1,"nulls":0,"type":"formula"}); - - testClient.drain(done); - }); - }); - - it("should expose a min formula", function(done) { - var testClient = new TestClient(overviewsMapConfig); - testClient.getDataview('test_min', { own_filter: 0 }, function(err, formula_result) { - if (err) { - return done(err); - } - assert.deepEqual(formula_result, {"operation":"min","result":1,"nulls":0,"type":"formula"}); - - testClient.drain(done); - }); - }); - }); + + it("should expose a filtered avg formula", function(done) { + var testClient = new TestClient(overviewsMapConfig); + testClient.getDataview('test_avg', params, function(err, formula_result) { + if (err) { + return done(err); + } + assert.deepEqual(formula_result, {"operation":"avg","result":1,"nulls":0,"type":"formula"}); + + testClient.drain(done); + }); + }); + + it("should expose a filtered count formula", function(done) { + var testClient = new TestClient(overviewsMapConfig); + testClient.getDataview('test_count', params, function(err, formula_result) { + if (err) { + return done(err); + } + assert.deepEqual(formula_result, {"operation":"count","result":1,"nulls":0,"type":"formula"}); + + testClient.drain(done); + }); + }); + + it("should expose a filterd max formula", function(done) { + var testClient = new TestClient(overviewsMapConfig); + testClient.getDataview('test_max', params, function(err, formula_result) { + if (err) { + return done(err); + } + assert.deepEqual(formula_result, {"operation":"max","result":1,"nulls":0,"type":"formula"}); + + testClient.drain(done); + }); + }); + + it("should expose a filterd min formula", function(done) { + var testClient = new TestClient(overviewsMapConfig); + testClient.getDataview('test_min', params, function(err, formula_result) { + if (err) { + return done(err); + } + assert.deepEqual(formula_result, {"operation":"min","result":1,"nulls":0,"type":"formula"}); + + testClient.drain(done); + }); + }); + + it("should expose a filtered sum formula with bbox", function (done) { + var bboxparams = { + filters: { + dataviews: {test_categories: {accept: ['Hawai']}} + }, + bbox: "-170,-80,170,80" + }; + var testClient = new TestClient(overviewsMapConfig); + testClient.getDataview('test_sum', bboxparams, function (err, formula_result) { + if (err) { + return done(err); + } + assert.deepEqual(formula_result, {"operation":"sum","result":1,"nulls":0,"type":"formula"}); + testClient.drain(done); + }); + }); + + }); }); From 2a06405a58b7fae3fa2b4d72d7df739112c2f5d3 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 18 May 2016 18:21:17 +0200 Subject: [PATCH 4/4] Move definition to the scope where it's needed --- lib/cartodb/backends/dataview.js | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/cartodb/backends/dataview.js b/lib/cartodb/backends/dataview.js index 89b811bb..b7bb6d0b 100644 --- a/lib/cartodb/backends/dataview.js +++ b/lib/cartodb/backends/dataview.js @@ -139,17 +139,17 @@ DataviewBackend.prototype.getDataview = function (mapConfigProvider, user, param if (params.bbox) { var bboxFilter = new BBoxFilter({column: 'the_geom', srid: 4326}, {bbox: params.bbox}); query = bboxFilter.sql(query); - var bbox_filter_definition = { - type: 'bbox', - options: { - column: 'the_geom', - srid: 4326, - }, - params: { - bbox: params.bbox - } - }; if ( queryRewriteData ) { + var bbox_filter_definition = { + type: 'bbox', + options: { + column: 'the_geom', + srid: 4326, + }, + params: { + bbox: params.bbox + } + }; queryRewriteData = _.extend(queryRewriteData, { bbox_filter: bbox_filter_definition }); } }