Fixed bug: when a report was run with a limit, and then run again with a bigger limit, the results would not include the data for the periods prior to what was in the cache from the first report run. - reportable - Fork of reportable required by WarVox, from hdm/reportable.
(DIR) Log
(DIR) Files
(DIR) Refs
(DIR) README
---
(DIR) commit c56e40b20ceea3a9098f19c675bf7790c0002569
(DIR) parent 5dd3e6e1a3c3851b6fe2b7c0d20abc65425ed268
(HTM) Author: Myron Marston <myron.marston@gmail.com>
Date: Sat, 4 Apr 2009 02:21:55 +0800
Fixed bug: when a report was run with a limit, and then run again with a bigger limit, the results would not include the data for the periods prior to what was in the cache from the first report run.
Signed-off-by: Marco Otte-Witte <marco.otte-witte@simplabs.com>
Diffstat:
M lib/kvlr/reports_as_sparkline/repo… | 42 ++++++++++++++++++++++++++------
M spec/classes/report_cache_spec.rb | 47 ++++++++++++++++++-------------
M spec/classes/report_spec.rb | 5 +++++
3 files changed, 68 insertions(+), 26 deletions(-)
---
(DIR) diff --git a/lib/kvlr/reports_as_sparkline/report_cache.rb b/lib/kvlr/reports_as_sparkline/report_cache.rb
@@ -18,32 +18,60 @@ module Kvlr #:nodoc:
end
# Get any missing data that comes after our cached data...
- new_data = if !options[:live_data] && last_cached_reporting_period == ReportingPeriod.new(options[:grouping]).previous
+ new_after_cache_data = if !options[:live_data] && last_cached_reporting_period == ReportingPeriod.new(options[:grouping]).previous
[]
else
end_date = options[:live_data] ? nil : last_reporting_period && last_reporting_period.date_time
yield((last_cached_reporting_period.next rescue first_reporting_period).date_time, end_date)
end
- prepare_result(new_data, cached_data, report, options, cache)
+ # Get any mising data that comes before our cached data....
+ new_before_cache_data = if cached_data.empty? || # after_cache_data will contain all the data if the cache was empty.
+ first_cached_reporting_period.date_time == first_reporting_period.date_time
+ []
+ else
+ yield(first_reporting_period.date_time, first_cached_reporting_period.date_time)
+ end
+
+ prepare_result(new_before_cache_data, new_after_cache_data, cached_data, report, options, cache)
end
end
private
- def self.prepare_result(new_data, cached_data, report, options, cache = true)
- new_data = new_data.map { |data| [ReportingPeriod.from_db_string(options[:grouping], data[0]), data[1]] }
+ def self.prepare_result(new_before_cache_data, new_after_cache_data, cached_data, report, options, cache = true)
+ cache_map_proc = lambda { |data| [ReportingPeriod.from_db_string(options[:grouping], data[0]), data[1]] }
+
+ new_after_cache_data = new_after_cache_data.map &cache_map_proc
+ new_before_cache_data = new_before_cache_data.map &cache_map_proc
result = cached_data.map { |cached| [cached.reporting_period, cached.value] }
+
+ first_reporting_period = ReportingPeriod.first(options[:grouping], options[:limit], options[:end_date])
last_reporting_period = ReportingPeriod.new(options[:grouping], options[:end_date])
- reporting_period = cached_data.empty? ? ReportingPeriod.first(options[:grouping], options[:limit], options[:end_date]) : ReportingPeriod.new(options[:grouping], cached_data.last.reporting_period).next
+ first_cached_reporting_period = cached_data.empty? ? nil : ReportingPeriod.new(options[:grouping], cached_data.first.reporting_period)
+ last_cached_reporting_period = cached_data.empty? ? nil : ReportingPeriod.new(options[:grouping], cached_data.last.reporting_period)
+
+ if first_cached_reporting_period
+ reporting_period = first_reporting_period
+ while reporting_period < first_cached_reporting_period
+ cached = build_cached_data(report, options[:grouping], reporting_period, find_value(new_before_cache_data, reporting_period))
+ cached.save! if cache
+ result.insert(0, [reporting_period.date_time, cached.value])
+ reporting_period = reporting_period.next
+ end
+ end
+
+ reporting_period = cached_data.empty? ? first_reporting_period : last_cached_reporting_period.next
+
while reporting_period < last_reporting_period
- cached = build_cached_data(report, options[:grouping], reporting_period, find_value(new_data, reporting_period))
+ cached = build_cached_data(report, options[:grouping], reporting_period, find_value(new_after_cache_data, reporting_period))
cached.save! if cache
result << [reporting_period.date_time, cached.value]
reporting_period = reporting_period.next
end
+
if options[:live_data]
- result << [last_reporting_period.date_time, find_value(new_data, last_reporting_period)]
+ result << [last_reporting_period.date_time, find_value(new_after_cache_data, last_reporting_period)]
end
result
end
(DIR) diff --git a/spec/classes/report_cache_spec.rb b/spec/classes/report_cache_spec.rb
@@ -37,7 +37,7 @@ describe Kvlr::ReportsAsSparkline::ReportCache do
}.should raise_error(YieldMatchException)
end
- it 'should yield the reporting period after the last one in the cache if data was read from cache' do
+ it 'should yield the reporting period after the last one in the cache, and before the first one in the cache if data was read from cache' do
reporting_period = Kvlr::ReportsAsSparkline::ReportingPeriod.new(
@report.options[:grouping],
Time.now - 3.send(@report.options[:grouping].identifier)
@@ -46,11 +46,15 @@ describe Kvlr::ReportsAsSparkline::ReportCache do
cached.stub!(:reporting_period).and_return(reporting_period.date_time)
Kvlr::ReportsAsSparkline::ReportCache.stub!(:find).and_return([cached])
+ expected_dates = [[reporting_period.next.date_time, nil], [reporting_period.offset(-7).date_time, reporting_period.date_time]]
+ yield_count = 0
Kvlr::ReportsAsSparkline::ReportCache.process(@report, @options) do |begin_at, end_at|
- begin_at.should == reporting_period.next.date_time
- end_at.should == nil
+ [begin_at, end_at].should == expected_dates[yield_count]
+ yield_count += 1
[]
end
+
+ yield_count.should == 2
end
end
@@ -72,7 +76,7 @@ describe Kvlr::ReportsAsSparkline::ReportCache do
}.should raise_error(YieldMatchException)
end
- it 'should yield the reporting period after the last one in the cache if data was read from cache' do
+ it 'should yield the reporting period after the last one in the cache, and before the first one in the cache if data was read from cache' do
reporting_period = Kvlr::ReportsAsSparkline::ReportingPeriod.new(
@report.options[:grouping],
Time.now - 3.send(@report.options[:grouping].identifier)
@@ -81,11 +85,15 @@ describe Kvlr::ReportsAsSparkline::ReportCache do
cached.stub!(:reporting_period).and_return(reporting_period.date_time)
Kvlr::ReportsAsSparkline::ReportCache.stub!(:find).and_return([cached])
+ expected_dates = [[reporting_period.next.date_time, nil], [reporting_period.offset(-7).date_time, reporting_period.date_time]]
+ yield_count = 0
Kvlr::ReportsAsSparkline::ReportCache.process(@report, @report.options) do |begin_at, end_at|
- begin_at.should == reporting_period.next.date_time
- end_at.should == nil
+ [begin_at, end_at].should == expected_dates[yield_count]
+ yield_count += 1
[]
end
+
+ yield_count.should == 2
end
end
@@ -148,12 +156,13 @@ describe Kvlr::ReportsAsSparkline::ReportCache do
end
it 'should prepare the results before it returns them' do
- new_data = []
+ new_after_cache_data = []
+ new_before_cache_data = []
cached_data = []
Kvlr::ReportsAsSparkline::ReportCache.stub!(:find).and_return(cached_data)
- Kvlr::ReportsAsSparkline::ReportCache.should_receive(:prepare_result).once.with(new_data, cached_data, @report, @report.options, true)
+ Kvlr::ReportsAsSparkline::ReportCache.should_receive(:prepare_result).once.with(new_before_cache_data, new_after_cache_data, cached_data, @report, @report.options, true)
- Kvlr::ReportsAsSparkline::ReportCache.process(@report, @report.options) { new_data }
+ Kvlr::ReportsAsSparkline::ReportCache.process(@report, @report.options) { new_after_cache_data }
end
it 'should yield the first reporting period if the cache is empty' do
@@ -188,7 +197,7 @@ describe Kvlr::ReportsAsSparkline::ReportCache do
before do
@current_reporting_period = Kvlr::ReportsAsSparkline::ReportingPeriod.new(@report.options[:grouping])
- @new_data = [[@current_reporting_period.previous.date_time, 1.0]]
+ @new_after_cache_data = [[@current_reporting_period.previous.date_time, 1.0]]
Kvlr::ReportsAsSparkline::ReportingPeriod.stub!(:from_db_string).and_return(@current_reporting_period.previous)
@cached = Kvlr::ReportsAsSparkline::ReportCache.new
@cached.stub!(:save!)
@@ -196,9 +205,9 @@ describe Kvlr::ReportsAsSparkline::ReportCache do
end
it 'should convert the date strings from the newly read data to reporting periods' do
- Kvlr::ReportsAsSparkline::ReportingPeriod.should_receive(:from_db_string).once.with(@report.options[:grouping], @new_data[0][0]).and_return(Kvlr::ReportsAsSparkline::ReportingPeriod.new(@report.options[:grouping]))
+ Kvlr::ReportsAsSparkline::ReportingPeriod.should_receive(:from_db_string).once.with(@report.options[:grouping], @new_after_cache_data[0][0]).and_return(Kvlr::ReportsAsSparkline::ReportingPeriod.new(@report.options[:grouping]))
- Kvlr::ReportsAsSparkline::ReportCache.send(:prepare_result, @new_data, [], @report, @report.options)
+ Kvlr::ReportsAsSparkline::ReportCache.send(:prepare_result, [], @new_after_cache_data, [], @report, @report.options)
end
it 'should create :limit instances of Kvlr::ReportsAsSparkline::ReportCache with value 0.0 if no new data has been read and nothing was cached' do
@@ -209,7 +218,7 @@ describe Kvlr::ReportsAsSparkline::ReportCache do
0.0
).and_return(@cached)
- Kvlr::ReportsAsSparkline::ReportCache.send(:prepare_result, [], [], @report, @report.options)
+ Kvlr::ReportsAsSparkline::ReportCache.send(:prepare_result, [], [], [], @report, @report.options)
end
it 'should create a new Kvlr::ReportsAsSparkline::ReportCache with the correct value if new data has been read' do
@@ -226,17 +235,17 @@ describe Kvlr::ReportsAsSparkline::ReportCache do
1.0
).and_return(@cached)
- Kvlr::ReportsAsSparkline::ReportCache.send(:prepare_result, @new_data, [], @report, @report.options)
+ Kvlr::ReportsAsSparkline::ReportCache.send(:prepare_result, [], @new_after_cache_data, [], @report, @report.options)
end
it 'should save the created Kvlr::ReportsAsSparkline::ReportCache' do
@cached.should_receive(:save!).once
- Kvlr::ReportsAsSparkline::ReportCache.send(:prepare_result, @new_data, [], @report, @report.options)
+ Kvlr::ReportsAsSparkline::ReportCache.send(:prepare_result, [], @new_after_cache_data, [], @report, @report.options)
end
it 'should return an array of arrays of Dates and Floats' do
- result = Kvlr::ReportsAsSparkline::ReportCache.send(:prepare_result, @new_data, [], @report, @report.options, true)
+ result = Kvlr::ReportsAsSparkline::ReportCache.send(:prepare_result, [], @new_after_cache_data, [], @report, @report.options, true)
result.should be_kind_of(Array)
result[0].should be_kind_of(Array)
@@ -247,7 +256,7 @@ describe Kvlr::ReportsAsSparkline::ReportCache do
describe 'with :live_data = false' do
before do
- @result = Kvlr::ReportsAsSparkline::ReportCache.send(:prepare_result, @new_data, [], @report, @report.options, true)
+ @result = Kvlr::ReportsAsSparkline::ReportCache.send(:prepare_result, [], @new_after_cache_data, [], @report, @report.options, true)
end
it 'should return an array of length :limit' do
@@ -264,7 +273,7 @@ describe Kvlr::ReportsAsSparkline::ReportCache do
before do
options = @report.options.merge(:live_data => true)
- @result = Kvlr::ReportsAsSparkline::ReportCache.send(:prepare_result, @new_data, [], @report, options, true)
+ @result = Kvlr::ReportsAsSparkline::ReportCache.send(:prepare_result, [], @new_after_cache_data, [], @report, options, true)
end
it 'should return an array of length (:limit + 1)' do
@@ -282,7 +291,7 @@ describe Kvlr::ReportsAsSparkline::ReportCache do
it 'should not save the created Kvlr::ReportsAsSparkline::ReportCache' do
@cached.should_not_receive(:save!)
- Kvlr::ReportsAsSparkline::ReportCache.send(:prepare_result, @new_data, [], @report, @report.options, false)
+ Kvlr::ReportsAsSparkline::ReportCache.send(:prepare_result, [], @new_after_cache_data, [], @report, @report.options, false)
end
end
(DIR) diff --git a/spec/classes/report_spec.rb b/spec/classes/report_spec.rb
@@ -89,6 +89,11 @@ describe Kvlr::ReportsAsSparkline::Report do
@report2.run.should == [[@two_months_ago, 0.0], [@one_month_ago, 1.0]]
end
+ it "should go back further into history on a second report run if the limit is higher" do
+ @report2.run(:limit => 2)
+ @report2.run(:limit => 3).should == [[@three_months_ago, 2.0], [@two_months_ago.to_time, 0.0], [@one_month_ago.to_time, 1.0]]
+ end
+
it "should return data for two months prior to the end date" do
@report2.run(:end_date => 1.month.ago).should == [[@three_months_ago, 2.0], [@two_months_ago, 0.0]]
end