diff --git a/NEWS.md b/NEWS.md index 17751105..cc6fdc7d 100644 --- a/NEWS.md +++ b/NEWS.md @@ -7,6 +7,8 @@ Announcements: - Logging all errors. - Add support for aggregated visualizations. - Allow vector-only map-config creation. + - Histograms: Now they accept a `no_filters` parameter. + ## 4.4.0 Released 2017-12-12 diff --git a/lib/cartodb/backends/dataview.js b/lib/cartodb/backends/dataview.js index 9df22868..c110f328 100644 --- a/lib/cartodb/backends/dataview.js +++ b/lib/cartodb/backends/dataview.js @@ -37,12 +37,19 @@ DataviewBackend.prototype.getDataview = function (mapConfigProvider, user, param throw new Error("Dataview '" + dataviewName + "' does not exists"); } - var pg = new PSQL(dbParamsFromReqParams(params)); - var ownFilter = +params.own_filter; - ownFilter = !!ownFilter; + var noFilters = +params.no_filters; + if (Number.isFinite(ownFilter) && Number.isFinite(noFilters)) { + err = new Error(); + err.message = 'Both own_filter and no_filters cannot be sent in the same request'; + err.type = 'dataview'; + err.http_status = 400; + return callback(err); + } - var query = (ownFilter) ? dataviewDefinition.sql.own_filter_on : dataviewDefinition.sql.own_filter_off; + var pg = new PSQL(dbParamsFromReqParams(params)); + + var query = getDataviewQuery(dataviewDefinition, ownFilter, noFilters); if (params.bbox) { var bboxFilter = new BBoxFilter({column: 'the_geom_webmercator', srid: 3857}, {bbox: params.bbox}); query = bboxFilter.sql(query); @@ -55,7 +62,7 @@ DataviewBackend.prototype.getDataview = function (mapConfigProvider, user, param ); var dataview = dataviewFactory.getDataview(query, dataviewDefinition); - dataview.getResult(pg, getOverrideParams(params, ownFilter), this); + dataview.getResult(pg, getOverrideParams(params, !!ownFilter), this); }, function returnCallback(err, result) { return callback(err, result); @@ -63,6 +70,16 @@ DataviewBackend.prototype.getDataview = function (mapConfigProvider, user, param ); }; +function getDataviewQuery(dataviewDefinition, ownFilter, noFilters) { + if (noFilters) { + return dataviewDefinition.sql.no_filters; + } else if (ownFilter === 1) { + return dataviewDefinition.sql.own_filter_on; + } else { + return dataviewDefinition.sql.own_filter_off; + } +} + function getQueryRewriteData(mapConfig, dataviewDefinition, params) { var sourceId = dataviewDefinition.source.id; // node.id var layer = _.find(mapConfig.obj().layers, function(l) { diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 23c0f6cd..0276527d 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -106,6 +106,7 @@ LayergroupController.prototype.register = function(app) { var allowedDataviewQueryParams = [ 'filters', // json 'own_filter', // 0, 1 + 'no_filters', // 0, 1 'bbox', // w,s,e,n 'start', // number 'end', // number diff --git a/lib/cartodb/models/aggregation/aggregation-query.js b/lib/cartodb/models/aggregation/aggregation-query.js index a43aead0..3fc68c8e 100644 --- a/lib/cartodb/models/aggregation/aggregation-query.js +++ b/lib/cartodb/models/aggregation/aggregation-query.js @@ -153,7 +153,7 @@ const aggregationQueryTemplates = { ${aggregateColumnDefs(ctx)} FROM (${ctx.sourceQuery}) _cdb_query, _cdb_params WHERE the_geom_webmercator && _cdb_params.bbox - GROUP BY _cdb_gx, _cdb_gy ${dimensionNames} + GROUP BY _cdb_gx, _cdb_gy ${dimensionNames(ctx)} ) SELECT ST_SetSRID(ST_MakePoint(_cdb_gx*(res+0.5), _cdb_gy*(res+0.5)), 3857) AS the_geom_webmercator diff --git a/lib/cartodb/models/mapconfig/adapter/analysis-mapconfig-adapter.js b/lib/cartodb/models/mapconfig/adapter/analysis-mapconfig-adapter.js index 8cb63f48..6cd0241d 100644 --- a/lib/cartodb/models/mapconfig/adapter/analysis-mapconfig-adapter.js +++ b/lib/cartodb/models/mapconfig/adapter/analysis-mapconfig-adapter.js @@ -58,6 +58,13 @@ AnalysisMapConfigAdapter.prototype.getMapConfig = function(user, requestMapConfi requestMapConfig = appendFiltersToNodes(requestMapConfig, dataviewsFiltersBySourceId); + // Expected format for analyses filters + // filters = {analyses: { + // a1: [{min, max}, {accept, reject}], + // b1: [{range, column, min, max}, {category, column, accept, reject}] + // }} + requestMapConfig = appendFiltersToNodes(requestMapConfig, filters.analyses); + function createAnalysis(analysisDefinition, done) { self.analysisBackend.create(analysisConfiguration, analysisDefinition, function (err, analysis) { if (err) { @@ -200,6 +207,7 @@ function dataviewQuery(node, dataviewName, ownFilter) { function appendFiltersToNodes(requestMapConfig, dataviewsFiltersBySourceId) { var analyses = requestMapConfig.analyses || []; + dataviewsFiltersBySourceId = dataviewsFiltersBySourceId || {}; requestMapConfig.analyses = analyses.map(function(analysisDefinition) { var analysisGraph = new camshaft.reference.AnalysisGraph(analysisDefinition); diff --git a/package.json b/package.json index 26bb0405..f4ed0914 100644 --- a/package.json +++ b/package.json @@ -19,7 +19,9 @@ "Sandro Santilli ", "Carlos Matallín ", "Daniel Garcia Aubert ", - "Mario de Frutos " + "Mario de Frutos ", + "Ivan Malagon ", + "Simon Martin " ], "dependencies": { "body-parser": "^1.18.2", diff --git a/test/acceptance/aggregation.js b/test/acceptance/aggregation.js index aa245fac..561291dc 100644 --- a/test/acceptance/aggregation.js +++ b/test/acceptance/aggregation.js @@ -560,6 +560,132 @@ describe('aggregation', function () { done(); }); }); + + it('aggregates with point-grid placement', function (done) { + this.mapConfig = createVectorMapConfig([ + { + type: 'cartodb', + options: { + sql: POINTS_SQL_1, + aggregation: { + placement: 'point-grid', + columns: { + total: { + aggregate_function: 'sum', + aggregated_column: 'value' + }, + v_avg: { + aggregate_function: 'avg', + aggregated_column: 'value' + } + }, + threshold: 1 + }, + cartocss: '#layer { marker-width: [total]; }', + cartocss_version: '2.3.0' + } + } + ]); + + this.testClient = new TestClient(this.mapConfig); + this.testClient.getLayergroup((err, body) => { + if (err) { + return done(err); + } + + 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)); + + done(); + }); + }); + + it('aggregates with point-sample placement', function (done) { + this.mapConfig = createVectorMapConfig([ + { + type: 'cartodb', + options: { + sql: POINTS_SQL_1, + aggregation: { + placement: 'point-sample', + columns: { + total: { + aggregate_function: 'sum', + aggregated_column: 'value' + }, + v_avg: { + aggregate_function: 'avg', + aggregated_column: 'value' + } + }, + threshold: 1 + }, + cartocss: '#layer { marker-width: [total]; }', + cartocss_version: '2.3.0' + } + } + ]); + + this.testClient = new TestClient(this.mapConfig); + this.testClient.getLayergroup((err, body) => { + if (err) { + return done(err); + } + + 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)); + + done(); + }); + }); + + it('aggregates with centroid placement', function (done) { + this.mapConfig = createVectorMapConfig([ + { + type: 'cartodb', + options: { + sql: POINTS_SQL_1, + aggregation: { + placement: 'centroid', + columns: { + total: { + aggregate_function: 'sum', + aggregated_column: 'value' + }, + v_avg: { + aggregate_function: 'avg', + aggregated_column: 'value' + } + }, + threshold: 1 + }, + cartocss: '#layer { marker-width: [total]; }', + cartocss_version: '2.3.0' + } + } + ]); + + this.testClient = new TestClient(this.mapConfig); + this.testClient.getLayergroup((err, body) => { + if (err) { + return done(err); + } + + 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)); + + done(); + }); + }); }); }); }); diff --git a/test/acceptance/analysis/analyses-filters-params.js b/test/acceptance/analysis/analyses-filters-params.js new file mode 100644 index 00000000..c4a17ae4 --- /dev/null +++ b/test/acceptance/analysis/analyses-filters-params.js @@ -0,0 +1,150 @@ +require('../../support/test_helper'); + +const assert = require('../../support/assert'); +const TestClient = require('../../support/test-client'); + +describe('analysis-filters-params', () => { + + const CARTOCSS = `#layer { + marker-fill-opacity: 1; + marker-line-color: white; + marker-line-width: 0.5; + marker-line-opacity: 1; + marker-placement: point; + marker-type: ellipse; + marker-width: 8; + marker-fill: red; + marker-allow-overlap: true; + }`; + + const mapConfig = { + version: '1.6.0', + layers: [ + { + "type": "cartodb", + "options": { + "source": { + "id": "a1" + }, + "cartocss": CARTOCSS, + "cartocss_version": "2.3.0" + } + } + ], + dataviews: { + pop_max_histogram: { + source: { + id: 'a1' + }, + type: 'histogram', + options: { + column: 'pop_max' + } + }, + pop_min_histogram: { + source: { + id: 'a1' + }, + type: 'histogram', + options: { + column: 'pop_min' + } + } + }, + analyses: [ + { + "id": "a1", + "type": "source", + "params": { + "query": "select * from populated_places_simple_reduced" + } + } + ] + }; + + var params = { + filters: { + dataviews: { + pop_max_histogram: { + min: 2e6 + }, + pop_min_histogram: { + max: 2e6 + } + } + } + }; + + + it('should get a filtered histogram dataview with all filters', function(done) { + const testClient = new TestClient(mapConfig, 1234); + const testParams = Object.assign({}, params, { + own_filter: 1 + }); + + testClient.getDataview('pop_max_histogram', testParams, (err, dataview) => { + assert.ok(!err, err); + + assert.equal(dataview.type, 'histogram'); + assert.equal(dataview.bins_count, 6); + + testClient.drain(done); + }); + }); + + it('should get a filtered histogram dataview with all filters except my own filter', function(done) { + const testClient = new TestClient(mapConfig, 1234); + const testParams = Object.assign({}, params, { + own_filter: 0 + }); + + testClient.getDataview('pop_max_histogram', testParams, (err, dataview) => { + assert.ok(!err, err); + + assert.equal(dataview.type, 'histogram'); + assert.equal(dataview.bins_count, 24); + + testClient.drain(done); + }); + }); + + it('should get a filtered histogram dataview without filters', function(done) { + const testClient = new TestClient(mapConfig, 1234); + const testParams = Object.assign({}, params, { + no_filters: 1 + }); + + testClient.getDataview('pop_max_histogram', testParams, (err, dataview) => { + assert.ok(!err, err); + + assert.equal(dataview.type, 'histogram'); + assert.equal(dataview.bins_count, 48); + + testClient.drain(done); + }); + }); + + it('should return an error if both no_filters and own_filter params are present', function (done) { + const testClient = new TestClient(mapConfig, 1234); + const expectedError = { + errors: ['Both own_filter and no_filters cannot be sent in the same request'], + errors_with_context: [{ + type: 'dataview', + message: 'Both own_filter and no_filters cannot be sent in the same request' + }] + }; + const testParams = Object.assign({}, params, { + no_filters: 1, + own_filter: 0, + response: { + status: 400 + } + }); + + testClient.getDataview('pop_max_histogram', testParams, (err, dataview) => { + assert.deepEqual(dataview, expectedError); + + testClient.drain(done); + }); + }); +}); diff --git a/test/acceptance/analysis/analyses-filters.js b/test/acceptance/analysis/analyses-filters.js new file mode 100644 index 00000000..152b29e6 --- /dev/null +++ b/test/acceptance/analysis/analyses-filters.js @@ -0,0 +1,84 @@ +require('../../support/test_helper'); + +const assert = require('../../support/assert'); +const TestClient = require('../../support/test-client'); + +describe('analysis-layers-dataviews', () => { + + const CARTOCSS = `#layer { + marker-fill-opacity: 1; + marker-line-color: white; + marker-line-width: 0.5; + marker-line-opacity: 1; + marker-placement: point; + marker-type: ellipse; + marker-width: 8; + marker-fill: red; + marker-allow-overlap: true; + }`; + + const mapConfig = { + version: '1.6.0', + layers: [ + { + "type": "cartodb", + "options": { + "source": { + "id": "a1" + }, + "cartocss": CARTOCSS, + "cartocss_version": "2.3.0" + } + } + ], + dataviews: { + pop_max_histogram: { + source: { + id: 'a1' + }, + type: 'histogram', + options: { + column: 'pop_max' + } + } + }, + analyses: [ + { + "id": "a1", + "type": "source", + "params": { + "query": "select * from populated_places_simple_reduced" + } + } + ] + }; + + it('should get a filtered histogram dataview', function(done) { + const testClient = new TestClient(mapConfig, 1234); + + const params = { + filters: { + analyses: { + 'a1': [ + { + type: 'range', + column: 'pop_max', + params: { + min: 2e6 + } + } + ] + } + } + }; + + testClient.getDataview('pop_max_histogram', params, (err, dataview) => { + assert.ok(!err, err); + + assert.equal(dataview.type, 'histogram'); + assert.equal(dataview.bins_start, 2008000); + + testClient.drain(done); + }); + }); +}); diff --git a/test/acceptance/analysis/analysis-layers-dataviews.js b/test/acceptance/analysis/analysis-layers-dataviews.js index a328ed09..8e95096a 100644 --- a/test/acceptance/analysis/analysis-layers-dataviews.js +++ b/test/acceptance/analysis/analysis-layers-dataviews.js @@ -109,7 +109,8 @@ describe('analysis-layers-dataviews', function() { min: 2e6 } } - } + }, + own_filter: 1 }; testClient.getDataview('pop_max_histogram', params, function(err, dataview) { diff --git a/test/acceptance/dataviews/overviews.js b/test/acceptance/dataviews/overviews.js index dcff687f..552b9ac6 100644 --- a/test/acceptance/dataviews/overviews.js +++ b/test/acceptance/dataviews/overviews.js @@ -393,7 +393,8 @@ describe('dataviews using tables with overviews', function() { var params = { filters: { dataviews: { test_histogram: { min: 2 } } - } + }, + own_filter: 1 }; var testClient = new TestClient(overviewsMapConfig); testClient.getDataview('test_histogram', params, function (err, histogram) { @@ -412,7 +413,8 @@ describe('dataviews using tables with overviews', function() { var params = { filters: { dataviews: { test_histogram: { max: -1 } } - } + }, + own_filter: 1 }; var testClient = new TestClient(overviewsMapConfig); testClient.getDataview('test_histogram', params, function (err, histogram) { @@ -433,7 +435,8 @@ describe('dataviews using tables with overviews', function() { var params = { filters: { dataviews: { test_histogram_date: { max: -1 } } - } + }, + own_filter: 1 }; var testClient = new TestClient(overviewsMapConfig); testClient.getDataview('test_histogram_date', params, function (err, histogram) { diff --git a/test/support/test-client.js b/test/support/test-client.js index f4437a39..6b200f2e 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -411,9 +411,13 @@ TestClient.prototype.getDataview = function(dataviewName, params, callback) { self.keysToDelete['map_cfg|' + LayergroupToken.parse(layergroupId).token] = 0; self.keysToDelete['user:localhost:mapviews:global'] = 5; - var urlParams = { - own_filter: params.hasOwnProperty('own_filter') ? params.own_filter : 1 - }; + var urlParams = {}; + if (params.hasOwnProperty('no_filters')) { + urlParams.no_filters = params.no_filters; + } + if (params.hasOwnProperty('own_filter')) { + urlParams.own_filter = params.own_filter; + } ['bbox', 'bins', 'start', 'end', 'aggregation', 'offset', 'categories'].forEach(function(extraParam) { if (params.hasOwnProperty(extraParam)) {