Fixed a bug with cumulated reports. To be a true cumulated total, they should include the totals from before the first reported periods. - reportable - Fork of reportable required by WarVox, from hdm/reportable.
(DIR) Log
(DIR) Files
(DIR) Refs
(DIR) README
---
(DIR) commit 5af0207e497c3a9cbf3c60dd5d6f0f0914ecef28
(DIR) parent c56e40b20ceea3a9098f19c675bf7790c0002569
(HTM) Author: Myron Marston <myron.marston@gmail.com>
Date: Sat, 4 Apr 2009 05:48:41 +0800
Fixed a bug with cumulated reports. To be a true cumulated total, they should include the totals from before the first reported periods.
Signed-off-by: Marco Otte-Witte <marco.otte-witte@simplabs.com>
Diffstat:
M lib/kvlr/reports_as_sparkline/cumu… | 12 +++++++++---
M lib/kvlr/reports_as_sparkline/repo… | 29 ++++++++++++++++++++---------
M spec/classes/cumulated_report_spec… | 38 ++++++++++++++++++++++++++++++-
M spec/classes/report_spec.rb | 8 ++++++++
4 files changed, 74 insertions(+), 13 deletions(-)
---
(DIR) diff --git a/lib/kvlr/reports_as_sparkline/cumulated_report.rb b/lib/kvlr/reports_as_sparkline/cumulated_report.rb
@@ -17,13 +17,14 @@ module Kvlr #:nodoc:
# Runs the report (see Kvlr::ReportsAsSparkline::Report#run)
def run(options = {})
- cumulate(super)
+ cumulate(super, options_for_run(options))
end
protected
- def cumulate(data) #:nodoc:
- acc = 0.0
+ def cumulate(data, options) #:nodoc:
+ first_reporting_period = ReportingPeriod.first(options[:grouping], options[:limit], options[:end_date])
+ acc = initial_cumulative_value(first_reporting_period.date_time, options)
result = []
data.each do |element|
acc += element[1].to_f
@@ -32,6 +33,11 @@ module Kvlr #:nodoc:
result
end
+ def initial_cumulative_value(date, options)
+ conditions = setup_conditions(nil, date, options[:conditions])
+ @klass.send(@aggregation, @value_column, :conditions => conditions)
+ end
+
end
end
(DIR) diff --git a/lib/kvlr/reports_as_sparkline/report.rb b/lib/kvlr/reports_as_sparkline/report.rb
@@ -48,11 +48,8 @@ module Kvlr #:nodoc:
# * <tt>:live_data</tt> - Specified whether data for the current reporting period is read; if :live_data is true, you will experience a performance hit since the request cannot be satisfied from the cache only (defaults to false)
# * <tt>:end_date</tt> - When specified, the report will be for the periods before this date.
def run(options = {})
- options = options.dup
- ensure_valid_options(options, :run)
custom_conditions = options.key?(:conditions)
- options.reverse_merge!(@options)
- options[:grouping] = Grouping.new(options[:grouping]) unless options[:grouping].is_a?(Grouping)
+ options = options_for_run(options)
ReportCache.process(self, options, !custom_conditions) do |begin_at, end_at|
read_data(begin_at, end_at, options)
end
@@ -60,6 +57,14 @@ module Kvlr #:nodoc:
private
+ def options_for_run(options = {})
+ options = options.dup
+ ensure_valid_options(options, :run)
+ options.reverse_merge!(@options)
+ options[:grouping] = Grouping.new(options[:grouping]) unless options[:grouping].is_a?(Grouping)
+ return options
+ end
+
def read_data(begin_at, end_at, options)
conditions = setup_conditions(begin_at, end_at, options[:conditions])
@klass.send(@aggregation,
@@ -85,14 +90,20 @@ module Kvlr #:nodoc:
elsif custom_conditions.size > 0
conditions = [(custom_conditions[0] || ''), *custom_conditions[1..-1]]
end
- conditions[0] += "#{(conditions[0].blank? ? '' : ' AND ') + @date_column.to_s} >= ?"
- conditions << begin_at
+ conditions[0] += "#{(conditions[0].blank? ? '' : ' AND ') + @date_column.to_s} "
- if end_at
- conditions[0].sub!(/>= \?\z/, 'BETWEEN ? AND ?')
- conditions << end_at
+ conditions[0] += if begin_at && end_at
+ 'BETWEEN ? AND ?'
+ elsif begin_at
+ '>= ?'
+ elsif end_at
+ '<= ?'
+ else
+ raise ArgumentError.new('You must pass either begin_at, end_at or both to setup_conditions.')
end
+ conditions << begin_at if begin_at
+ conditions << end_at if end_at
conditions
end
(DIR) diff --git a/spec/classes/cumulated_report_spec.rb b/spec/classes/cumulated_report_spec.rb
@@ -25,6 +25,41 @@ describe Kvlr::ReportsAsSparkline::CumulatedReport do
@report.run.length.should == 11
end
+
+ describe "a month report with a limit of 2" do
+ before(:all) do
+ User.delete_all
+ User.create!(:login => 'test 1', :created_at => Time.now, :profile_visits => 2)
+ User.create!(:login => 'test 2', :created_at => Time.now - 1.month, :profile_visits => 1)
+ User.create!(:login => 'test 3', :created_at => Time.now - 3.month, :profile_visits => 2)
+ User.create!(:login => 'test 4', :created_at => Time.now - 3.month, :profile_visits => 3)
+ User.create!(:login => 'test 5', :created_at => Time.now - 4.month, :profile_visits => 4)
+
+ @report2 = Kvlr::ReportsAsSparkline::CumulatedReport.new(User, :cumulated_registrations,
+ :grouping => :month,
+ :limit => 2
+ )
+
+ @one_month_ago = Date.new(DateTime.now.year, DateTime.now.month, 1) - 1.month
+ @two_months_ago = Date.new(DateTime.now.year, DateTime.now.month, 1) - 2.months
+ @three_months_ago = Date.new(DateTime.now.year, DateTime.now.month, 1) - 3.months
+ end
+
+ it 'should include the counts from before the first period in the cumulated totals' do
+ @report2.run.should == [[@two_months_ago, 3.0], [@one_month_ago, 4.0]]
+ end
+
+ it 'should return the initial count for initial_cumulative_value' do
+ options = @report2.send(:options_for_run, {})
+ tomorrow = 1.day.from_now
+ @report2.send(:initial_cumulative_value, tomorrow, options).should == 5
+ @report2.send(:initial_cumulative_value, tomorrow - 1.month, options).should == 4
+ @report2.send(:initial_cumulative_value, tomorrow - 2.months, options).should == 3
+ @report2.send(:initial_cumulative_value, tomorrow - 3.months, options).should == 3
+ @report2.send(:initial_cumulative_value, tomorrow - 4.months, options).should == 1
+ @report2.send(:initial_cumulative_value, tomorrow - 5.months, options).should == 0
+ end
+ end
for grouping in [:hour, :day, :week, :month] do
@@ -35,6 +70,7 @@ describe Kvlr::ReportsAsSparkline::CumulatedReport do
describe "with :live_data = #{live_data}" do
before(:all) do
+ User.delete_all
User.create!(:login => 'test 1', :created_at => Time.now - 1.send(grouping), :profile_visits => 1)
User.create!(:login => 'test 2', :created_at => Time.now - 3.send(grouping), :profile_visits => 2)
User.create!(:login => 'test 3', :created_at => Time.now - 3.send(grouping), :profile_visits => 3)
@@ -159,7 +195,7 @@ describe Kvlr::ReportsAsSparkline::CumulatedReport do
second = Time.now.to_s
data = [[first, 1], [second, 2]]
- @report.send(:cumulate, data).should == [[first, 1.0], [second, 3.0]]
+ @report.send(:cumulate, data, @report.send(:options_for_run, {})).should == [[first, 1.0], [second, 3.0]]
end
end
(DIR) diff --git a/spec/classes/report_spec.rb b/spec/classes/report_spec.rb
@@ -386,6 +386,14 @@ describe Kvlr::ReportsAsSparkline::Report do
it 'should return conditions for date_column >= begin_at when no custom conditions and a begin_at are specified' do
@report.send(:setup_conditions, @begin_at, nil).should == ['created_at >= ?', @begin_at]
end
+
+ it 'should return conditions for date_column <= end_at when no custom conditions and a end_at are specified' do
+ @report.send(:setup_conditions, nil, @end_at).should == ['created_at <= ?', @end_at]
+ end
+
+ it 'should raise an argument error when neither begin_at or end_at are specified' do
+ lambda {@report.send(:setup_conditions, nil, nil)}.should raise_error(ArgumentError)
+ end
it 'should return conditions for date_column BETWEEN begin_at and end_date only when an empty Hash of custom conditions is specified' do
@report.send(:setup_conditions, @begin_at, @end_at, {}).should == ['created_at BETWEEN ? AND ?', @begin_at, @end_at]