| plugin should be feature complete now; some specs, cleanup, refactorings still … | |
| Log | |
| Files | |
| Refs | |
| README | |
| --- | |
| commit 686d110e727d3a6144e037d4791f520c434409df | |
| parent 7f02d5870811625ca57691a29a3c9f749203f550 | |
| Author: marcoow <[email protected]> | |
| Date: Wed, 10 Dec 2008 04:10:00 +0800 | |
| plugin should be feature complete now; some specs, cleanup, refactorings still … | |
| Signed-off-by: Marco Otte-Witte <[email protected]> | |
| Diffstat: | |
| M lib/kvlr/reports_as_sparkline/grou… | 38 +++++++--------------------… | |
| M lib/kvlr/reports_as_sparkline/repo… | 86 ++++++++++++++++++---------… | |
| M spec/models/report_method_spec.rb | 17 ++++++++++------- | |
| M spec/other/cumulated_report_spec.rb | 15 +++++++++------ | |
| M spec/other/grouping_spec.rb | 76 ++++++++++++-----------------… | |
| M spec/other/report_cache_spec.rb | 19 ++++--------------- | |
| M spec/other/report_spec.rb | 26 +++++++++++++++++++------- | |
| 7 files changed, 129 insertions(+), 148 deletions(-) | |
| --- | |
| diff --git a/lib/kvlr/reports_as_sparkline/grouping.rb b/lib/kvlr/reports_as_sp… | |
| @@ -4,43 +4,21 @@ module Kvlr #:nodoc: | |
| class Grouping | |
| - @@allowed_groupings = | |
| - | |
| def initialize(grouping) | |
| raise ArgumentError.new("Invalid grouping #{grouping}") unless [:hour,… | |
| @identifier = grouping | |
| end | |
| def identifier | |
| - @identifier.to_s | |
| - end | |
| - | |
| - def to_reporting_period(date_time) | |
| - return case @identifier | |
| - when :hour | |
| - DateTime.new(date_time.year, date_time.month, date_time.day, date_… | |
| - when :day | |
| - date_time.to_date | |
| - when :week | |
| - date_time = (date_time - date_time.wday.days) + 1 | |
| - Date.new(date_time.year, date_time.month, date_time.day) | |
| - when :month | |
| - Date.new(date_time.year, date_time.month) | |
| - end | |
| + @identifier | |
| end | |
| - def first_reporting_period(limit) | |
| - return case @identifier | |
| - when :hour | |
| - to_reporting_period(Time.now - limit.hours) | |
| - when :day | |
| - to_reporting_period(Time.now - limit.days) | |
| - when :week | |
| - to_reporting_period(Time.now - limit.weeks) | |
| - when :month | |
| - date = Time.now - limit.months | |
| - Date.new(date.year, date.month, 1) | |
| + def date_parts_from_db_string(db_string) | |
| + parts = db_string.split('/').map(&:to_i) | |
| + if @identifier == :week | |
| + parts[1] += 1 | |
| end | |
| + parts | |
| end | |
| def to_sql(date_column_name) | |
| @@ -63,7 +41,7 @@ module Kvlr #:nodoc: | |
| when :day | |
| "DATE_FORMAT(#{date_column_name}, '%Y/%m/%d')" | |
| when :week | |
| - "DATE_FORMAT(#{date_column_name}, '%Y-%u')" | |
| + "DATE_FORMAT(#{date_column_name}, '%Y/%u')" | |
| when :month | |
| "DATE_FORMAT(#{date_column_name}, '%Y/%m')" | |
| end | |
| @@ -76,7 +54,7 @@ module Kvlr #:nodoc: | |
| when :day | |
| "strftime('%Y/%m/%d', #{date_column_name})" | |
| when :week | |
| - "strftime('%Y-%W', #{date_column_name})" | |
| + "strftime('%Y/%W', #{date_column_name})" | |
| when :month | |
| "strftime('%Y/%m', #{date_column_name})" | |
| end | |
| diff --git a/lib/kvlr/reports_as_sparkline/report_cache.rb b/lib/kvlr/reports_a… | |
| @@ -4,51 +4,67 @@ module Kvlr #:nodoc: | |
| class ReportCache < ActiveRecord::Base | |
| + serialize :reporting_period, Kvlr::ReportsAsSparkline::ReportingPeriod | |
| + | |
| def self.cached_transaction(report, limit, no_cache = false, &block) | |
| raise ArgumentError.new('A block must be given') unless block_given? | |
| - return yield(report.grouping.first_reporting_period(limit)) if no_cache | |
| self.transaction do | |
| - cached_data = self.find( | |
| - :all, | |
| - :conditions => { | |
| - :model_name => report.klass.to_s, | |
| - :report_name => report.name.to_s, | |
| - :grouping => report.grouping.identifier.to_s, | |
| - :aggregation => report.aggregation.to_s | |
| - }, | |
| - :limit => limit, | |
| - :order => 'reporting_period DESC' | |
| - ) | |
| - last_reporting_period_to_read = if cached_data.empty? | |
| - report.grouping.first_reporting_period(limit) | |
| + cached_data = if no_cache | |
| + [] | |
| + else | |
| + self.find( | |
| + :all, | |
| + :conditions => { | |
| + :model_name => report.klass.to_s, | |
| + :report_name => report.name.to_s, | |
| + :grouping => report.grouping.identifier.to_s, | |
| + :aggregation => report.aggregation.to_s | |
| + }, | |
| + :limit => limit, | |
| + :order => 'reporting_period DESC' | |
| + ) | |
| + end | |
| + last_reporting_period_to_read = if cached_data.empty? | |
| + ReportingPeriod.first(report.grouping, limit) | |
| else | |
| - report.grouping.to_reporting_period(cached_data.last.reporting_p… | |
| + cached_data.last.reporting_period | |
| end | |
| - new_data = yield(last_reporting_period_to_read) | |
| - return update_cache(new_data, cached_data, report) | |
| + new_data = yield(last_reporting_period_to_read.date_time) | |
| + update_cache(new_data, cached_data, last_reporting_period_to_read, r… | |
| end | |
| end | |
| private | |
| - def self.update_cache(new_data, cached_data, report) | |
| - rows_to_write = (0..-1) | |
| - if cached_data.size > 0 && new_data.size > 0 | |
| - cached_data.last.update_attributes!(:value => new_data.first[1]) | |
| - rows_to_write = (1..-1) | |
| - end | |
| - for row in (new_data[rows_to_write] || []) | |
| - self.create!( | |
| - :model_name => report.klass.to_s, | |
| - :report_name => report.name.to_s, | |
| - :grouping => report.grouping.identifier.to_s, | |
| - :aggregation => report.aggregation.to_s, | |
| - :reporting_period => report.grouping.to_reporting_period(DateTim… | |
| - :value => row[1] | |
| - ) | |
| - end | |
| - result = cached_data.map { |cached| [cached.reporting_period, cached… | |
| - result += new_data.map { |data| [DateTime.parse(data[0]), data[1]] } | |
| + def self.update_cache(new_data, cached_data, last_reporting_period_to_… | |
| + new_data.map! { |data| [ReportingPeriod.from_db_string(report.groupi… | |
| + current_reporting_period = ReportingPeriod.new(report.grouping) | |
| + reporting_period = current_reporting_period | |
| + result = [] | |
| + begin | |
| + data = new_data.detect { |data| data[0] == reporting_period } | |
| + if data && cached = cached_data.detect { |cached| cached.reporting… | |
| + if no_cache | |
| + cached.update_attributes!(:value => data[1]) | |
| + else | |
| + cached.value = data[1] | |
| + end | |
| + else | |
| + data = self.new( | |
| + :model_name => report.klass.to_s, | |
| + :report_name => report.name.to_s, | |
| + :grouping => report.grouping.identifier.to_s, | |
| + :aggregation => report.aggregation.to_s, | |
| + :reporting_period => reporting_period, | |
| + :value => (data ? data[1] : 0) | |
| + ) | |
| + data.save! unless no_cache | |
| + data = [data.reporting_period, data.value] | |
| + end | |
| + result << data | |
| + reporting_period = reporting_period.previous | |
| + end while (reporting_period != last_reporting_period_to_read) | |
| + result.map { |r| [r[0].date_time, r[1]] } | |
| end | |
| end | |
| diff --git a/spec/models/report_method_spec.rb b/spec/models/report_method_spec… | |
| @@ -35,30 +35,33 @@ describe Kvlr::ReportsAsSparkline do | |
| it 'should include all data when invoked on the base model class' do | |
| result = User.registrations_report.to_a | |
| - result.length.should == 2 | |
| - result[0][1].should == 1 | |
| - result[1][1].should == 2 | |
| + result.length.should == 20 | |
| + result[7][1].should == 1 | |
| + result[14][1].should == 2 | |
| end | |
| it 'should include all data when invoked on the base model class' do | |
| result = SpecialUser.registrations_report.to_a | |
| - result.length.should == 1 | |
| - result[0][1].should == 1 | |
| + result.length.should == 20 | |
| + result[14][1].should == 1 | |
| end | |
| after(:all) do | |
| User.destroy_all | |
| SpecialUser.destroy_all | |
| - Kvlr::ReportsAsSparkline::ReportCache.destroy_all | |
| end | |
| end | |
| + after do | |
| + Kvlr::ReportsAsSparkline::ReportCache.destroy_all | |
| + end | |
| + | |
| end | |
| class User < ActiveRecord::Base | |
| - report_as_sparkline :registrations, :cumulate => true | |
| + report_as_sparkline :registrations, :cumulate => true, :limit => 20 | |
| end | |
| class SpecialUser < User; end | |
| diff --git a/spec/other/cumulated_report_spec.rb b/spec/other/cumulated_report_… | |
| @@ -23,27 +23,30 @@ describe Kvlr::ReportsAsSparkline::CumulatedReport do | |
| it 'should return correct data for :aggregation => :count' do | |
| result = @report.run.to_a | |
| - result[0][1].should == 1 | |
| - result[1][1].should == 3 | |
| + result[7][1].should == 1 | |
| + result[14][1].should == 3 | |
| end | |
| it 'should return correct data for :aggregation => :sum' do | |
| @report = Kvlr::ReportsAsSparkline::CumulatedReport.new(User, :registrat… | |
| result = @report.run().to_a | |
| - result[0][1].should == 1 | |
| - result[1][1].should == 6 | |
| + result[7][1].should == 1 | |
| + result[14][1].should == 6 | |
| end | |
| it 'should return correct data with custom conditions' do | |
| result = @report.run(:conditions => ['login IN (?)', ['test 1', 'test 2'… | |
| - result[0][1].should == 1 | |
| - result[1][1].should == 2 | |
| + result[7][1].should == 1 | |
| + result[14][1].should == 2 | |
| end | |
| after(:all) do | |
| User.destroy_all | |
| + end | |
| + | |
| + after do | |
| Kvlr::ReportsAsSparkline::ReportCache.destroy_all | |
| end | |
| diff --git a/spec/other/grouping_spec.rb b/spec/other/grouping_spec.rb | |
| @@ -10,53 +10,12 @@ describe Kvlr::ReportsAsSparkline::Grouping do | |
| end | |
| - describe '.to_reporting_period' do | |
| - | |
| - it 'should return the date with day = 1 for grouping :month' do | |
| - datetime = Time.now | |
| - grouping = Kvlr::ReportsAsSparkline::Grouping.new(:month) | |
| - | |
| - grouping.to_reporting_period(datetime).should == Date.new(datetime.year,… | |
| - end | |
| - | |
| - it 'should return the date of the first day of the week date_time is in (w… | |
| - grouping = Kvlr::ReportsAsSparkline::Grouping.new(:week) | |
| - | |
| - datetime = DateTime.new(2008, 11, 27) #this is a thursday | |
| - grouping.to_reporting_period(datetime).should == DateTime.new(datetime.y… | |
| - | |
| - datetime = DateTime.new(2008, 11, 24) #this is a monday already, should … | |
| - grouping.to_reporting_period(datetime).should == DateTime.new(datetime.y… | |
| - | |
| - datetime = DateTime.new(2008, 11, 1) #this is a saturday | |
| - grouping.to_reporting_period(datetime).should == DateTime.new(datetime.y… | |
| - | |
| - datetime = DateTime.new(2009, 1, 1) #this is a thursday | |
| - grouping.to_reporting_period(datetime).should == DateTime.new(2008, 12, … | |
| - end | |
| - | |
| - it 'should return the date for grouping :day' do | |
| - datetime = Time.now | |
| - grouping = Kvlr::ReportsAsSparkline::Grouping.new(:day) | |
| - | |
| - grouping.to_reporting_period(datetime).should == datetime.to_date | |
| - end | |
| - | |
| - it 'should return the date and time with minutes = seconds = 0 for groupin… | |
| - datetime = Time.now | |
| - grouping = Kvlr::ReportsAsSparkline::Grouping.new(:hour) | |
| - | |
| - grouping.to_reporting_period(datetime).should == DateTime.new(datetime.y… | |
| - end | |
| - | |
| - end | |
| - | |
| describe '.to_sql' do | |
| describe 'for MySQL' do | |
| before do | |
| - ActiveRecord::Base.connection.class.stub!(:to_s).and_return('ActiveRec… | |
| + ActiveRecord::Base.connection.stub!(:class).and_return(ActiveRecord::C… | |
| end | |
| it 'should use DATE_FORMAT with format string "%Y/%m/%d/%H" for grouping… | |
| @@ -67,8 +26,8 @@ describe Kvlr::ReportsAsSparkline::Grouping do | |
| Kvlr::ReportsAsSparkline::Grouping.new(:day).send(:to_sql, 'created_at… | |
| end | |
| - it 'should use DATE_FORMAT with format string "%Y-%u" for grouping :week… | |
| - Kvlr::ReportsAsSparkline::Grouping.new(:week).send(:to_sql, 'created_a… | |
| + it 'should use DATE_FORMAT with format string "%Y/%u" for grouping :week… | |
| + Kvlr::ReportsAsSparkline::Grouping.new(:week).send(:to_sql, 'created_a… | |
| end | |
| it 'should use DATE_FORMAT with format string "%Y/%m" for grouping :mont… | |
| @@ -80,7 +39,7 @@ describe Kvlr::ReportsAsSparkline::Grouping do | |
| describe 'for PostgreSQL' do | |
| before do | |
| - ActiveRecord::Base.connection.class.stub!(:to_s).and_return('ActiveRec… | |
| + ActiveRecord::Base.connection.stub!(:class).and_return(ActiveRecord::C… | |
| end | |
| it 'should use date_trunc with trunc "hour" for grouping :hour' do | |
| @@ -104,7 +63,7 @@ describe Kvlr::ReportsAsSparkline::Grouping do | |
| describe 'for SQLite3' do | |
| before do | |
| - ActiveRecord::Base.connection.class.stub!(:to_s).and_return('ActiveRec… | |
| + ActiveRecord::Base.connection.stub!(:class).and_return(ActiveRecord::C… | |
| end | |
| it 'should use strftime with format string "%Y/%m/%d/%H" for grouping :h… | |
| @@ -115,8 +74,8 @@ describe Kvlr::ReportsAsSparkline::Grouping do | |
| Kvlr::ReportsAsSparkline::Grouping.new(:day).send(:to_sql, 'created_at… | |
| end | |
| - it 'should use strftime with format string "%Y-%W" for grouping :week' do | |
| - Kvlr::ReportsAsSparkline::Grouping.new(:week).send(:to_sql, 'created_a… | |
| + it 'should use strftime with format string "%Y/%W" for grouping :week' do | |
| + Kvlr::ReportsAsSparkline::Grouping.new(:week).send(:to_sql, 'created_a… | |
| end | |
| it 'should use strftime with format string "%Y/%m" for grouping :month' … | |
| @@ -127,6 +86,27 @@ describe Kvlr::ReportsAsSparkline::Grouping do | |
| end | |
| + describe '#date_parts_from_db_string' do | |
| + | |
| + for grouping in [[:hour, '2008/12/31/12'], [:day, '2008/12/31'], [:month, … | |
| + | |
| + it "should split the string with '/' for grouping :#{grouping[0].to_s}" … | |
| + db_string = grouping[1] | |
| + | |
| + Kvlr::ReportsAsSparkline::Grouping.new(grouping[0]).date_parts_from_db… | |
| + end | |
| + | |
| + end | |
| + | |
| + it 'should split the string with "/" and increment the week by 1 for group… | |
| + db_string = '2008/2' | |
| + expected = [2008, 3] | |
| + | |
| + Kvlr::ReportsAsSparkline::Grouping.new(:week).date_parts_from_db_string(… | |
| + end | |
| + | |
| + end | |
| + | |
| end | |
| class ActiveRecord::ConnectionAdapters::MysqlAdapter; end | |
| diff --git a/spec/other/report_cache_spec.rb b/spec/other/report_cache_spec.rb | |
| @@ -52,48 +52,37 @@ describe Kvlr::ReportsAsSparkline::ReportCache do | |
| it 'should yield the first reporting period if the cache is empty' do | |
| Kvlr::ReportsAsSparkline::ReportCache.cached_transaction(@report, 100) d… | |
| - begin_at.should == @report.grouping.first_reporting_period(100) | |
| + begin_at.should == Kvlr::ReportsAsSparkline::ReportingPeriod.first(@re… | |
| [] | |
| end | |
| end | |
| it 'should yield the last reporting period in the cache if the cache is no… | |
| + reporting_period = Kvlr::ReportsAsSparkline::ReportingPeriod.new(@report… | |
| cached = Kvlr::ReportsAsSparkline::ReportCache.new({ | |
| :model_name => @report.klass, | |
| :report_name => @report.name, | |
| :grouping => @report.grouping.identifier.to_s, | |
| :aggregation => @report.aggregation.to_s, | |
| :value => 1, | |
| - :reporting_period => DateTime.now | |
| + :reporting_period => reporting_period | |
| }) | |
| Kvlr::ReportsAsSparkline::ReportCache.stub!(:find).and_return([cached]) | |
| Kvlr::ReportsAsSparkline::ReportCache.cached_transaction(@report, 100) d… | |
| - begin_at.should == @report.grouping.to_reporting_period(cached.reporti… | |
| + begin_at.should == reporting_period.date_time | |
| [] | |
| end | |
| end | |
| describe 'with no_cache = true' do | |
| - it 'should not start a transaction' do | |
| - Kvlr::ReportsAsSparkline::ReportCache.should_not_receive(:transaction) | |
| - | |
| - Kvlr::ReportsAsSparkline::ReportCache.cached_transaction(@report, 100,… | |
| - end | |
| - | |
| it 'should not read any data from cache' do | |
| Kvlr::ReportsAsSparkline::ReportCache.should_not_receive(:find) | |
| Kvlr::ReportsAsSparkline::ReportCache.cached_transaction(@report, 100,… | |
| end | |
| - it 'should not update the cache' do | |
| - Kvlr::ReportsAsSparkline::ReportCache.should_not_receive(:update_cache) | |
| - | |
| - Kvlr::ReportsAsSparkline::ReportCache.cached_transaction(@report, 100,… | |
| - end | |
| - | |
| end | |
| end | |
| diff --git a/spec/other/report_spec.rb b/spec/other/report_spec.rb | |
| @@ -34,30 +34,42 @@ describe Kvlr::ReportsAsSparkline::Report do | |
| result = @report.run(:limit => 3) | |
| end | |
| - it 'should return correct data for :aggregation => :count' do | |
| + it 'should return correct data for :aggregation => :count and grouping :… | |
| + @report = Kvlr::ReportsAsSparkline::Report.new(User, :registrations, :… | |
| result = @report.run.to_a | |
| - result[0][1].should == 1 | |
| - result[1][1].should == 2 | |
| + result[7][1].should == 1 | |
| + result[14][1].should == 2 | |
| + end | |
| + | |
| + it 'should return correct data for :aggregation => :count and grouping :… | |
| + @report = Kvlr::ReportsAsSparkline::Report.new(User, :registrations, :… | |
| + result = @report.run.to_a | |
| + | |
| + result[1][1].should == 1 | |
| + result[2][1].should == 2 | |
| end | |
| it 'should return correct data for :aggregation => :sum' do | |
| @report = Kvlr::ReportsAsSparkline::Report.new(User, :registrations, :… | |
| result = @report.run().to_a | |
| - result[0][1].should == 1 | |
| - result[1][1].should == 5 | |
| + result[7][1].should == 1 | |
| + result[14][1].should == 5 | |
| end | |
| it 'should return correct data with custom conditions' do | |
| result = @report.run(:conditions => ['login IN (?)', ['test 1', 'test … | |
| - result[0][1].should == 1 | |
| - result[1][1].should == 1 | |
| + result[7][1].should == 1 | |
| + result[14][1].should == 1 | |
| end | |
| after(:all) do | |
| User.destroy_all | |
| + end | |
| + | |
| + after do | |
| Kvlr::ReportsAsSparkline::ReportCache.destroy_all | |
| end | |