From 50ecdb5feef135718ab17bd53cfb1845877d8fcb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 7 Nov 2017 12:51:48 +0100 Subject: [PATCH 1/5] Add test to ensure that categories param is used to compose the aggregation dataview --- test/acceptance/dataviews/aggregation.js | 77 ++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/test/acceptance/dataviews/aggregation.js b/test/acceptance/dataviews/aggregation.js index d8d03177..4a274d80 100644 --- a/test/acceptance/dataviews/aggregation.js +++ b/test/acceptance/dataviews/aggregation.js @@ -324,3 +324,80 @@ describe('aggregation-dataview: special float values', function() { }); }); }); + +describe('categories param', function () { + afterEach(function(done) { + if (this.testClient) { + this.testClient.drain(done); + } else { + done(); + } + }); + + const mapConfig = { + version: '1.5.0', + layers: [ + { + type: "cartodb", + options: { + source: { + "id": "a0" + }, + cartocss: "#points { marker-width: 10; marker-fill: red; }", + cartocss_version: "2.3.0" + } + } + ], + dataviews: { + categories: { + source: { + id: 'a0' + }, + type: 'aggregation', + options: { + column: 'cat', + aggregation: 'sum', + aggregationColumn: 'val' + } + } + }, + analyses: [ + { + id: "a0", + type: "source", + params: { + query: ` + SELECT + null::geometry the_geom_webmercator, + CASE + WHEN x % 4 = 0 THEN 1 + WHEN x % 4 = 1 THEN 2 + WHEN x % 4 = 2 THEN 3 + ELSE 4 + END AS val, + CASE + WHEN x % 4 = 0 THEN 'category_1' + WHEN x % 4 = 1 THEN 'category_2' + WHEN x % 4 = 2 THEN 'category_3' + ELSE 'category_4' + END AS cat + FROM generate_series(1, 1000) x + ` + } + } + ] + }; + + it('should accept cartegories param to customize aggregation dataview', function (done) { + this.testClient = new TestClient(mapConfig, 1234); + const params = { + categories: 2 + }; + + this.testClient.getDataview('categories', params, (err, dataview) => { + assert.ifError(err); + assert.equal(dataview.categoriesCount, 2); + done(); + }); + }); +}); From 743bb0723b88b2b2ac43e4af22cfebbd8b111593 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 7 Nov 2017 16:14:47 +0100 Subject: [PATCH 2/5] Add query param to define the number of categories to be ranked --- lib/cartodb/backends/dataview.js | 4 +- lib/cartodb/controllers/layergroup.js | 3 +- lib/cartodb/models/dataview/aggregation.js | 6 ++- test/acceptance/dataviews/aggregation.js | 45 +++++++++++++++++----- test/support/test-client.js | 2 +- 5 files changed, 45 insertions(+), 15 deletions(-) diff --git a/lib/cartodb/backends/dataview.js b/lib/cartodb/backends/dataview.js index b6037ae6..9df22868 100644 --- a/lib/cartodb/backends/dataview.js +++ b/lib/cartodb/backends/dataview.js @@ -24,7 +24,7 @@ module.exports = DataviewBackend; DataviewBackend.prototype.getDataview = function (mapConfigProvider, user, params, callback) { - var dataviewName = params.dataviewName; + var dataviewName = params.dataviewName; step( function getMapConfig() { mapConfigProvider.getMapConfig(this); @@ -94,7 +94,7 @@ function getQueryRewriteData(mapConfig, dataviewDefinition, params) { } function getOverrideParams(params, ownFilter) { - var overrideParams = _.reduce(_.pick(params, 'start', 'end', 'bins', 'offset'), + var overrideParams = _.reduce(_.pick(params, 'start', 'end', 'bins', 'offset', 'categories'), function castNumbers(overrides, val, k) { if (!Number.isFinite(+val)) { throw new Error('Invalid number format for parameter \'' + k + '\''); diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index dc6f85ff..458951c2 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -109,7 +109,8 @@ LayergroupController.prototype.register = function(app) { 'bins', // number 'aggregation', //string 'offset', // number - 'q' // widgets search + 'q', // widgets search + 'categories', // number ]; app.get( diff --git a/lib/cartodb/models/dataview/aggregation.js b/lib/cartodb/models/dataview/aggregation.js index b4c59af2..40774353 100644 --- a/lib/cartodb/models/dataview/aggregation.js +++ b/lib/cartodb/models/dataview/aggregation.js @@ -245,6 +245,10 @@ module.exports = class Aggregation extends BaseDataview { return null; } + const limit = Number.isFinite(override.categories) && override.categories > 0 ? + override.categories : + CATEGORIES_LIMIT; + const aggregationSql = aggregationDataviewQueryTpl({ override: override, query: this.query, @@ -256,7 +260,7 @@ module.exports = class Aggregation extends BaseDataview { aggregationColumn: this.aggregationColumn || 1 }), isFloatColumn: this._isFloatColumn, - limit: CATEGORIES_LIMIT + limit }); debug(aggregationSql); diff --git a/test/acceptance/dataviews/aggregation.js b/test/acceptance/dataviews/aggregation.js index 4a274d80..45189213 100644 --- a/test/acceptance/dataviews/aggregation.js +++ b/test/acceptance/dataviews/aggregation.js @@ -325,7 +325,7 @@ describe('aggregation-dataview: special float values', function() { }); }); -describe('categories param', function () { +describe('aggregation dataview tuned by categories query param', function () { afterEach(function(done) { if (this.testClient) { this.testClient.drain(done); @@ -388,16 +388,41 @@ describe('categories param', function () { ] }; - it('should accept cartegories param to customize aggregation dataview', function (done) { - this.testClient = new TestClient(mapConfig, 1234); - const params = { - categories: 2 - }; + var scenarios = [ + { + params: { own_filter: 0, categories: -1 }, + categoriesExpected: 4 + }, + { + params: { own_filter: 0, categories: 0 }, + categoriesExpected: 4 + }, + { + params: { own_filter: 0, categories: 1 }, + categoriesExpected: 1 + }, + { + params: { own_filter: 0, categories: 2 }, + categoriesExpected: 2 + }, + { + params: { own_filter: 0, categories: 4 }, + categoriesExpected: 4 + }, + { + params: { own_filter: 0, categories: 5 }, + categoriesExpected: 4 + } + ]; - this.testClient.getDataview('categories', params, (err, dataview) => { - assert.ifError(err); - assert.equal(dataview.categoriesCount, 2); - done(); + scenarios.forEach(function (scenario) { + it(`should handle cartegories to customize aggregations: ${JSON.stringify(scenario.params)}`, function (done) { + this.testClient = new TestClient(mapConfig, 1234); + this.testClient.getDataview('categories', scenario.params, (err, dataview) => { + assert.ifError(err); + assert.equal(dataview.categories.length, scenario.categoriesExpected); + done(); + }); }); }); }); diff --git a/test/support/test-client.js b/test/support/test-client.js index cd7b4f60..f31f9db4 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -415,7 +415,7 @@ TestClient.prototype.getDataview = function(dataviewName, params, callback) { own_filter: params.hasOwnProperty('own_filter') ? params.own_filter : 1 }; - ['bbox', 'bins', 'start', 'end', 'aggregation', 'offset'].forEach(function(extraParam) { + ['bbox', 'bins', 'start', 'end', 'aggregation', 'offset', 'categories'].forEach(function(extraParam) { if (params.hasOwnProperty(extraParam)) { urlParams[extraParam] = params[extraParam]; } From 9149f72f42d5405c6a96c8df3187e825a2536a34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 7 Nov 2017 16:21:55 +0100 Subject: [PATCH 3/5] Update NEWS --- NEWS.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 6312c5bb..78a1be0a 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,10 +1,11 @@ # Changelog -## 4.0.2 +## 4.1.0 Released 2017-mm-dd Announcements: - Upgrades windshaft to [4.0.1](https://github.com/CartoDB/windshaft/releases/tag/4.0.1). + - Add `categories` query param to define the number of cateries to be ranked for aggregation dataviews. ## 4.0.1 From fc9dce0cca7699b532b8f2a8243627a2a953556a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 7 Nov 2017 16:22:36 +0100 Subject: [PATCH 4/5] Fix typo --- NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 78a1be0a..409b5ebc 100644 --- a/NEWS.md +++ b/NEWS.md @@ -5,7 +5,7 @@ Released 2017-mm-dd Announcements: - Upgrades windshaft to [4.0.1](https://github.com/CartoDB/windshaft/releases/tag/4.0.1). - - Add `categories` query param to define the number of cateries to be ranked for aggregation dataviews. + - Add `categories` query param to define the number of categories to be ranked for aggregation dataviews. ## 4.0.1 From 2aee357006b3ccec62e1f5fbdfb3ac818d99c5f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 7 Nov 2017 16:28:37 +0100 Subject: [PATCH 5/5] Improve test structure --- test/acceptance/dataviews/aggregation.js | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/test/acceptance/dataviews/aggregation.js b/test/acceptance/dataviews/aggregation.js index 45189213..e29d9ff4 100644 --- a/test/acceptance/dataviews/aggregation.js +++ b/test/acceptance/dataviews/aggregation.js @@ -326,14 +326,6 @@ describe('aggregation-dataview: special float values', function() { }); describe('aggregation dataview tuned by categories query param', function () { - afterEach(function(done) { - if (this.testClient) { - this.testClient.drain(done); - } else { - done(); - } - }); - const mapConfig = { version: '1.5.0', layers: [ @@ -388,6 +380,14 @@ describe('aggregation dataview tuned by categories query param', function () { ] }; + beforeEach(function () { + this.testClient = new TestClient(mapConfig, 1234); + }); + + afterEach(function (done) { + this.testClient.drain(done); + }); + var scenarios = [ { params: { own_filter: 0, categories: -1 }, @@ -417,7 +417,6 @@ describe('aggregation dataview tuned by categories query param', function () { scenarios.forEach(function (scenario) { it(`should handle cartegories to customize aggregations: ${JSON.stringify(scenario.params)}`, function (done) { - this.testClient = new TestClient(mapConfig, 1234); this.testClient.getDataview('categories', scenario.params, (err, dataview) => { assert.ifError(err); assert.equal(dataview.categories.length, scenario.categoriesExpected);