From fd9534797c519bf6be33cc9df07a62a3aa1abc4d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?=
 <danielgarciaaubert@gmail.com>
Date: Mon, 11 Sep 2017 18:44:14 +0200
Subject: [PATCH] Minor refactors

---
 .../dataview/histograms/date-histogram.js     | 32 ++++++++++---------
 .../dataview/histograms/numeric-histogram.js  |  3 ++
 2 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/lib/cartodb/models/dataview/histograms/date-histogram.js b/lib/cartodb/models/dataview/histograms/date-histogram.js
index 20523a46..a14e90e9 100644
--- a/lib/cartodb/models/dataview/histograms/date-histogram.js
+++ b/lib/cartodb/models/dataview/histograms/date-histogram.js
@@ -1,4 +1,5 @@
 const BaseHistogram = require('./base-histogram');
+const debug = require('debug')('windshaft:dataview:date-histogram');
 
 const dateIntervalQueryTpl = ctx => `
     WITH
@@ -186,16 +187,13 @@ module.exports = class DateHistogram extends BaseHistogram {
     }
 
     _buildQuery (psql, override, callback) {
-        const _aggregation = override && override.aggregation ? override.aggregation : this.aggregation;
-        const _offset = override && Number.isFinite(override.offset) ? override.offset : this.offset;
-
-        if (!DATE_AGGREGATIONS.hasOwnProperty(_aggregation)) {
+        if (!DATE_AGGREGATIONS.hasOwnProperty(this._getAggregation(override))) {
             return callback(new Error('Invalid aggregation value. Valid ones: ' +
                 Object.keys(DATE_AGGREGATIONS).join(', ')
             ));
         }
 
-        if (_aggregation === 'auto') {
+        if (this._getAggregation(override) === 'auto') {
             this._getAutomaticAggregation(psql, function (err, aggregation) {
                 if (err || aggregation === 'none') {
                     this.aggregation = 'day';
@@ -212,12 +210,14 @@ module.exports = class DateHistogram extends BaseHistogram {
             _override: override,
             _query: this.query,
             _column: this.column,
-            _aggregation: _aggregation,
+            _aggregation: this._getAggregation(override),
             _start: this._getBinStart(override),
             _end: this._getBinEnd(override),
-            _offset: this._parseOffset(_offset, _aggregation)
+            _offset: this._parseOffset(override)
         });
 
+        debug(histogramSql);
+
         return callback(null, histogramSql);
     }
 
@@ -285,22 +285,24 @@ module.exports = class DateHistogram extends BaseHistogram {
     }
 
     _getAggregation (override = {}) {
-        return override && override.aggregation ? override.aggregation : this.aggregation;
+        return override.aggregation ? override.aggregation : this.aggregation;
     }
 
     _getOffset (override = {}) {
-        return override.offset || this.offset || 0;
+        return Number.isFinite(override.offset) ? override.offset : (this.offset || 0);
     }
 
-    _parseOffset (offset, aggregation) {
-        if (!offset) {
-            return '0';
-        }
-        if (aggregation === 'hour' || aggregation === 'minute') {
+    _parseOffset (override) {
+        if (this._shouldIgnoreOffset(override)) {
             return '0';
         }
 
-        const offsetInHours = Math.ceil(offset / 3600);
+        const offsetInHours = Math.ceil(this._getOffset(override) / 3600);
+
         return '' + offsetInHours;
     }
+
+    _shouldIgnoreOffset (override) {
+        return (this._getAggregation(override) === 'hour' || this._getAggregation(override) === 'minute');
+    }
 };
diff --git a/lib/cartodb/models/dataview/histograms/numeric-histogram.js b/lib/cartodb/models/dataview/histograms/numeric-histogram.js
index ed14d78f..60716eb6 100644
--- a/lib/cartodb/models/dataview/histograms/numeric-histogram.js
+++ b/lib/cartodb/models/dataview/histograms/numeric-histogram.js
@@ -1,4 +1,5 @@
 const BaseHistogram = require('./base-histogram');
+const debug = require('debug')('windshaft:dataview:numeric-histogram');
 
 const columnCastTpl = ctx => `date_part('epoch', ${ctx.column})`;
 
@@ -176,6 +177,8 @@ module.exports = class NumericHistogram extends BaseHistogram {
             _maxBins: BIN_MAX_NUMBER,
         });
 
+        debug(histogramSql);
+
         return callback(null, histogramSql);
     }