Fix edge case when start month is 12 (December) -- get_history_for_da…

…tes was using incorrect branch, resulting in a SQL query with: "...WHERE month IN ()..." which caused an exception.
papercheck · May 3, 2011 · bd94a8f · bd94a8f
1 parent edf2576
commit bd94a8f
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 6 deletions.
diff --git a/app/controllers/rails_admin/history_controller.rb b/app/controllers/rails_admin/history_controller.rb
@@ -15,7 +15,7 @@ def list
     end
 
     def slider
-      if params[:from].nil? or params[:to].nil?
+      if params[:from].blank? or params[:to].blank?
         not_found
       else
         render :json => AbstractHistory.history_summaries(params[:from], params[:to])

diff --git a/app/models/rails_admin/history.rb b/app/models/rails_admin/history.rb
@@ -9,19 +9,18 @@ class History < ActiveRecord::Base
     }
 
     def self.get_history_for_dates(mstart, mstop, ystart, ystop)
-      sql_in = ""
-      if mstart > mstop
-        sql_in = (mstart + 1..12).to_a.join(", ")
+      if mstart > mstop && mstart < 12
+        sql_in = ((mstart + 1)..12).to_a.join(", ")
         sql_in_two = (1..mstop).to_a.join(", ")
 
         results = History.find_by_sql("select count(*) as number, year, month from rails_admin_histories where month IN (#{sql_in}) and year = #{ystart} group by year, month")
         results_two = History.find_by_sql("select count(*) as number, year, month from rails_admin_histories where month IN (#{sql_in_two}) and year = #{ystop} group by year, month")
 
         results.concat(results_two)
       else
-        sql_in =  (mstart + 1..mstop).to_a.join(", ")
+        sql_in = ((mstart == 12 ? 1 : mstart + 1)..mstop).to_a.join(", ")
 
-        results = History.find_by_sql("select count(*) as number, year, month from rails_admin_histories where month IN (#{sql_in}) and year = #{ystart} group by year, month")
+        results = History.find_by_sql("select count(*) as number, year, month from rails_admin_histories where month IN (#{sql_in}) and year = #{ystop} group by year, month")
       end
 
       results.each do |result|

diff --git a/spec/requests/history/rails_admin_history_spec.rb b/spec/requests/history/rails_admin_history_spec.rb
@@ -13,6 +13,13 @@
     end
   end
 
+  describe "when range starts in December" do
+    it "does not produce SQL with empty IN () range" do
+      RailsAdmin::History.should_receive(:find_by_sql).with("select count(*) as number, year, month from rails_admin_histories where month IN (1, 2, 3, 4) and year = 2011 group by year, month").and_return([])
+      RailsAdmin::History.get_history_for_dates(12, 4, 2010, 2011)
+    end
+  end
+
   describe "history blank results single year" do
     before(:each) do
       @months = RailsAdmin::History.add_blank_results([RailsAdmin::BlankHistory.new(7, 2010), RailsAdmin::BlankHistory.new(9, 2011)], 5, 2010)