From 924f00939000ee99adc8284abae59ed1086b32ab Mon Sep 17 00:00:00 2001 From: Raul Ochoa Date: Mon, 13 Mar 2017 18:36:43 +0100 Subject: [PATCH 1/3] Test for #606: function avg(timestamp with time zone) does not exist --- test/acceptance/widgets/ported/histogram.js | 31 +++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/test/acceptance/widgets/ported/histogram.js b/test/acceptance/widgets/ported/histogram.js index 0f5c9d8e..90dfe840 100644 --- a/test/acceptance/widgets/ported/histogram.js +++ b/test/acceptance/widgets/ported/histogram.js @@ -304,6 +304,37 @@ describe('widgets', function() { }); }); + it('can use a datetime filtered column with no results', function(done) { + this.testClient = new TestClient(histogramsMapConfig({ + updated_at: { + type: 'histogram', + options: { + column: 'updated_at' + } + } + })); + var params = { + own_filter: 1, + filters: { + layers: [{ + updated_at: { + // this will remove all results + max: -1 + } + }] + } + }; + this.testClient.getWidget('updated_at', params, function (err, res, histogram) { + assert.ok(!err, err); + assert.ok(histogram); + assert.equal(histogram.type, 'histogram'); + + assert.equal(histogram.bins.length, 0); + + done(); + }); + }); + it('can getTile with datetime filtered column', function(done) { this.testClient = new TestClient(histogramsMapConfig({ updated_at: { From 992b2b6ba72c24d74cc589ac63605330cfbafba7 Mon Sep 17 00:00:00 2001 From: Raul Ochoa Date: Mon, 13 Mar 2017 18:40:29 +0100 Subject: [PATCH 2/3] Histogram column type discovery query uses non-filtered query Pass all queries to the dataview and use the no filters one for discovering what is the column type associated to the histogram dataview. --- lib/cartodb/models/dataview/factory.js | 2 +- lib/cartodb/models/dataview/histogram.js | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/cartodb/models/dataview/factory.js b/lib/cartodb/models/dataview/factory.js index 464a0d44..50814f23 100644 --- a/lib/cartodb/models/dataview/factory.js +++ b/lib/cartodb/models/dataview/factory.js @@ -11,7 +11,7 @@ var DataviewFactory = { if (!this.dataviews[type]) { throw new Error('Invalid dataview type: "' + type + '"'); } - return new this.dataviews[type](query, dataviewDefinition.options); + return new this.dataviews[type](query, dataviewDefinition.options, dataviewDefinition.sql); } }; diff --git a/lib/cartodb/models/dataview/histogram.js b/lib/cartodb/models/dataview/histogram.js index 7b3e29bc..5d102bf5 100644 --- a/lib/cartodb/models/dataview/histogram.js +++ b/lib/cartodb/models/dataview/histogram.js @@ -109,12 +109,13 @@ var TYPE = 'histogram'; } } */ -function Histogram(query, options) { +function Histogram(query, options, queries) { if (!_.isString(options.column)) { throw new Error('Histogram expects `column` in widget options'); } this.query = query; + this.queries = queries; this.column = options.column; this.bins = options.bins; @@ -143,7 +144,7 @@ Histogram.prototype.sql = function(psql, override, callback) { var _column = this.column; var columnTypeQuery = columnTypeQueryTpl({ - column: _column, query: this.query + column: _column, query: this.queries.no_filters }); if (this._columnType === null) { From fa945502618d81edd1333eb0d77a585fb37da8f8 Mon Sep 17 00:00:00 2001 From: Raul Ochoa Date: Thu, 16 Mar 2017 19:15:34 +0100 Subject: [PATCH 3/3] Include changes for overviews implementation --- .../models/dataview/overviews/factory.js | 3 +- .../models/dataview/overviews/histogram.js | 5 +- test/acceptance/dataviews/overviews.js | 91 +++++++++++++++++++ 3 files changed, 96 insertions(+), 3 deletions(-) diff --git a/lib/cartodb/models/dataview/overviews/factory.js b/lib/cartodb/models/dataview/overviews/factory.js index b63b004a..d7a07d54 100644 --- a/lib/cartodb/models/dataview/overviews/factory.js +++ b/lib/cartodb/models/dataview/overviews/factory.js @@ -14,7 +14,8 @@ OverviewsDataviewFactory.prototype.getDataview = function(query, dataviewDefinit return parentFactory.getDataview(query, dataviewDefinition); } return new dataviews[type]( - query, dataviewDefinition.options, this.queryRewriter, this.queryRewriteData, this.options + query, dataviewDefinition.options, this.queryRewriter, this.queryRewriteData, this.options, + dataviewDefinition.sql ); }; diff --git a/lib/cartodb/models/dataview/overviews/histogram.js b/lib/cartodb/models/dataview/overviews/histogram.js index 5f4d97d9..67da4514 100644 --- a/lib/cartodb/models/dataview/overviews/histogram.js +++ b/lib/cartodb/models/dataview/overviews/histogram.js @@ -96,10 +96,11 @@ var histogramQueryTpl = dot.template([ 'ORDER BY bin' ].join('\n')); -function Histogram(query, options, queryRewriter, queryRewriteData, params) { +function Histogram(query, options, queryRewriter, queryRewriteData, params, queries) { BaseOverviewsDataview.call(this, query, options, BaseDataview, queryRewriter, queryRewriteData, params); this.query = query; + this.queries = queries; this.column = options.column; this.bins = options.bins; @@ -129,7 +130,7 @@ Histogram.prototype.sql = function(psql, override, callback) { var _column = this.column; var columnTypeQuery = columnTypeQueryTpl({ - column: _column, query: this.rewrittenQuery(this.query) + column: _column, query: this.rewrittenQuery(this.queries.no_filters) }); if (this._columnType === null) { diff --git a/test/acceptance/dataviews/overviews.js b/test/acceptance/dataviews/overviews.js index 601124a7..d23519ae 100644 --- a/test/acceptance/dataviews/overviews.js +++ b/test/acceptance/dataviews/overviews.js @@ -144,6 +144,22 @@ describe('dataviews using tables with overviews', function() { aggregationColumn: 'name', } }, + test_histogram: { + type: 'histogram', + source: {id: 'data-source'}, + options: { + column: 'value', + bins: 2 + } + }, + test_histogram_date: { + type: 'histogram', + source: {id: 'data-source'}, + options: { + column: 'updated_at', + bins: 2 + } + }, test_avg: { type: 'formula', source: {id: 'data-source'}, @@ -265,8 +281,83 @@ describe('dataviews using tables with overviews', function() { }); }); + it("should expose a histogram", function (done) { + var testClient = new TestClient(overviewsMapConfig); + testClient.getDataview('test_histogram', function (err, histogram) { + if (err) { + return done(err); + } + assert.ok(histogram); + assert.equal(histogram.type, 'histogram'); + assert.ok(Array.isArray(histogram.bins)); + testClient.drain(done); + }); + }); + describe('filters', function() { + describe('histogram', function () { + + it("should expose a filtered histogram", function (done) { + var params = { + filters: { + dataviews: { test_histogram: { min: 2 } } + } + }; + var testClient = new TestClient(overviewsMapConfig); + testClient.getDataview('test_histogram', params, function (err, histogram) { + if (err) { + return done(err); + } + assert.ok(histogram); + assert.equal(histogram.type, 'histogram'); + assert.ok(Array.isArray(histogram.bins)); + assert.equal(histogram.bins.length, 4); + testClient.drain(done); + }); + }); + + it("should expose a filtered histogram with no results", function (done) { + var params = { + filters: { + dataviews: { test_histogram: { max: -1 } } + } + }; + var testClient = new TestClient(overviewsMapConfig); + testClient.getDataview('test_histogram', params, function (err, histogram) { + if (err) { + return done(err); + } + assert.ok(histogram); + assert.equal(histogram.type, 'histogram'); + assert.ok(Array.isArray(histogram.bins)); + assert.equal(histogram.bins.length, 0); + testClient.drain(done); + }); + }); + + it("should expose a filtered date histogram with no results", function (done) { + // This most likely works because the overviews will pass + // the responsibility to the normal dataviews. + var params = { + filters: { + dataviews: { test_histogram_date: { max: -1 } } + } + }; + var testClient = new TestClient(overviewsMapConfig); + testClient.getDataview('test_histogram_date', params, function (err, histogram) { + if (err) { + return done(err); + } + assert.ok(histogram); + assert.equal(histogram.type, 'histogram'); + assert.ok(Array.isArray(histogram.bins)); + assert.equal(histogram.bins.length, 0); + testClient.drain(done); + }); + }); + }); + describe('category', function () { var params = {