From acf0b082b40908ab7e8cb4725f7254a5f91563b0 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 12 Sep 2018 11:25:21 +0200 Subject: [PATCH 1/2] Perform some tests for all placements The "only the_geom" and other aggregation tests were perform only for default aggregation. --- test/acceptance/aggregation.js | 107 +++++++++++++++++---------------- 1 file changed, 56 insertions(+), 51 deletions(-) diff --git a/test/acceptance/aggregation.js b/test/acceptance/aggregation.js index 17944029..4053054e 100644 --- a/test/acceptance/aggregation.js +++ b/test/acceptance/aggregation.js @@ -1640,7 +1640,7 @@ describe('aggregation', function () { }); }); - ['centroid', 'point-sample', 'point-grid'].forEach(placement => { + ['centroid', 'point-sample', 'point-grid', 'default'].forEach(placement => { it(`cartodb_id should be present in ${placement} aggregation`, function(done) { this.mapConfig = createVectorMapConfig([ { @@ -1648,7 +1648,6 @@ describe('aggregation', function () { options: { sql: POINTS_SQL_1, aggregation: { - placement: placement, threshold: 1 }, cartocss: '#layer { marker-width: 1; }', @@ -1657,6 +1656,9 @@ describe('aggregation', function () { } } ]); + if (placement !== 'default') { + this.mapConfig.layers[0].options.aggregation.placement = placement; + } this.testClient = new TestClient(this.mapConfig); this.testClient.getLayergroup((err, body) => { @@ -1673,71 +1675,74 @@ describe('aggregation', function () { done(); }); }); - }); - it('should only require the_geom_webmercator for aggregation', function (done) { - this.mapConfig = createVectorMapConfig([ - { - type: 'cartodb', - options: { - sql: POINTS_SQL_ONLY_WEBMERCATOR, - aggregation: { - threshold: 1 + it(`should only require the_geom_webmercator for ${placement} aggregation`, function (done) { + this.mapConfig = createVectorMapConfig([ + { + type: 'cartodb', + options: { + sql: POINTS_SQL_ONLY_WEBMERCATOR, + aggregation: { + threshold: 1 + } } } + ]); + if (placement !== 'default') { + this.mapConfig.layers[0].options.aggregation.placement = placement; } - ]); - this.testClient = new TestClient(this.mapConfig); + this.testClient = new TestClient(this.mapConfig); - this.testClient.getLayergroup((err, body) => { - if (err) { - return done(err); - } + this.testClient.getLayergroup((err, body) => { + if (err) { + return done(err); + } - assert.equal(typeof body.metadata, 'object'); - assert.ok(Array.isArray(body.metadata.layers)); + assert.equal(typeof body.metadata, 'object'); + assert.ok(Array.isArray(body.metadata.layers)); - body.metadata.layers.forEach(layer => assert.ok(layer.meta.aggregation.mvt)); - body.metadata.layers.forEach(layer => assert.ok(!layer.meta.aggregation.png)); + body.metadata.layers.forEach(layer => assert.ok(layer.meta.aggregation.mvt)); + body.metadata.layers.forEach(layer => assert.ok(!layer.meta.aggregation.png)); - done(); + done(); + }); }); - }); - it('aggregation should work with attributes', function (done) { - this.mapConfig = createVectorMapConfig([ - { - type: 'cartodb', - options: { - sql: POINTS_SQL_1, - cartocss: '#layer { marker-width: 7; }', - cartocss_version: '2.3.0', - aggregation: { - threshold: 1 - }, - attributes: { - id: 'cartodb_id', - columns: [ - 'value' - ] + it(`${placement} aggregation should work with attributes`, function (done) { + this.mapConfig = createVectorMapConfig([ + { + type: 'cartodb', + options: { + sql: POINTS_SQL_1, + cartocss: '#layer { marker-width: 7; }', + cartocss_version: '2.3.0', + aggregation: { + threshold: 1 + }, + attributes: { + id: 'cartodb_id', + columns: [ + 'value' + ] + } } } - } - ]); - this.testClient = new TestClient(this.mapConfig); + ]); + this.testClient = new TestClient(this.mapConfig); - this.testClient.getLayergroup((err, body) => { - if (err) { - return done(err); - } + this.testClient.getLayergroup((err, body) => { + if (err) { + return done(err); + } - assert.equal(typeof body.metadata, 'object'); - assert.ok(Array.isArray(body.metadata.layers)); + assert.equal(typeof body.metadata, 'object'); + assert.ok(Array.isArray(body.metadata.layers)); - body.metadata.layers.forEach(layer => assert.ok(layer.meta.aggregation.mvt)); - body.metadata.layers.forEach(layer => assert.ok(layer.meta.aggregation.png)); + body.metadata.layers.forEach(layer => assert.ok(layer.meta.aggregation.mvt)); + body.metadata.layers.forEach(layer => assert.ok(layer.meta.aggregation.png)); - done(); + done(); + }); }); }); From 1e65804a1b4a62c3796640fc70d4e3e85e1ecb5b Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 12 Sep 2018 11:26:14 +0200 Subject: [PATCH 2/2] Avoid requiring the_geom for point-sample aggregation --- lib/cartodb/models/aggregation/aggregation-query.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cartodb/models/aggregation/aggregation-query.js b/lib/cartodb/models/aggregation/aggregation-query.js index 4ae3bc33..4d964171 100644 --- a/lib/cartodb/models/aggregation/aggregation-query.js +++ b/lib/cartodb/models/aggregation/aggregation-query.js @@ -381,7 +381,7 @@ const aggregationQueryTemplates = { ) SELECT _cdb_clusters.cartodb_id, - the_geom, the_geom_webmercator + the_geom_webmercator ${dimensionNames(ctx, '_cdb_clusters')} ${aggregateColumnNames(ctx, '_cdb_clusters')} FROM