Move due date counting code into Holiday class

4 files changed, 84 insertions, 78 deletions

diff --git a/app/models/holiday.rb b/app/models/holiday.rb
index 478e0b086..5837753f1 100644
--- a/app/models/holiday.rb
+++ b/app/models/holiday.rb
@@ -9,13 +9,49 @@
 #
 
 # models/holiday.rb:
-# Store details on public holidays on which the clock for answering FOI
-# requests do not run
+#
+# Store details on, and perform calculations with, public holidays on which 
+# the clock for answering FOI requests does not run:
+#
+#   ... "working day" means any day other than a Saturday, a Sunday, Christmas
+#   Day, Good Friday or a day which is a bank holiday under the [1971 c. 80.]
+#   Banking and Financial Dealings Act 1971 in any part of the United Kingdom.
+#    -- Freedom of Information Act 2000 section 10
 #
 # Copyright (c) 2009 UK Citizens Online Democracy. All rights reserved.
 # Email: francis@mysociety.org; WWW: http://www.mysociety.org/
 #
-# $Id: holiday.rb,v 1.2 2009-03-09 15:48:32 tony Exp $
+# $Id: holiday.rb,v 1.3 2009-03-10 08:16:43 tony Exp $
 
 class Holiday < ActiveRecord::Base
+
+  # Calculate the date on which a request made on a given date falls due.
+  def Holiday.due_date_from(start_date)
+
+      # TODO only fetch holidays after the start_date
+      holidays = self.all.collect { |h| h.day }.to_set
+
+      # Count forward 20 working days. We start with today (or if not a working day,
+      # the next working day*) as "day zero". The first of the twenty full
+      # working days is the next day. We return the date of the last of the twenty.
+      #
+      # * See this response for example of a public authority complaining when we got
+      # that detail wrong: http://www.whatdotheyknow.com/request/policy_regarding_body_scans#incoming-1100
+     
+      # We have to skip non-working days at start to find day zero, so start at
+      # day -1 and at yesterday, so we can do that.
+      days_passed = -1 
+      response_required_by = start_date - 1.day
+
+      # Now step forward into day zero, and then each of the 20 days.
+      while days_passed < 20
+          response_required_by += 1.day
+          next if response_required_by.wday == 0 || response_required_by.wday == 6 # weekend
+          next if holidays.include?(response_required_by)
+          days_passed += 1
+      end
+
+      return response_required_by
+  end
+
 end
diff --git a/app/models/info_request.rb b/app/models/info_request.rb
index e1e6ba4f3..e265b19d8 100644
--- a/app/models/info_request.rb
+++ b/app/models/info_request.rb
@@ -23,7 +23,7 @@
 # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved.
 # Email: francis@mysociety.org; WWW: http://www.mysociety.org/
 #
-# $Id: info_request.rb,v 1.176 2009-03-09 17:13:07 tony Exp $
+# $Id: info_request.rb,v 1.177 2009-03-10 08:16:43 tony Exp $
 
 require 'digest/sha1'
 require File.join(File.dirname(__FILE__),'../../vendor/plugins/acts_as_xapian/lib/acts_as_xapian')
@@ -438,51 +438,14 @@ public
         return last_sent
     end
 
-    # Calculate date by end of which response is required by law.
-    #
-    #   ... "working day” means any day other than a Saturday, a Sunday, Christmas
-    #   Day, Good Friday or a day which is a bank holiday under the [1971 c. 80.]
-    #   Banking and Financial Dealings Act 1971 in any part of the United Kingdom.
-    #
-    # Freedom of Information Act 2000 section 10
-    #
     # How do we cope with case where extra info was required from the requester
     # by the public body in order to fulfill the request, as per sections 1(3)
     # and 10(6b) ? For clarifications this is covered by
     # last_event_forming_initial_request. There may be more obscure
     # things, e.g. fees, not properly covered.
-
     def date_response_required_by
         last_sent = last_event_forming_initial_request
-        return due_date_for_request_date(last_sent.outgoing_message.last_sent_at)
-    end
-
-    # TODO move this into Holiday class 
-    def due_date_for_request_date(request_date)
-        # TODO only fetch holidays after the request_date
-        holidays = Holiday.all.collect { |h| h.day }.to_set
-
-        # Count forward 20 working days. We start with today (or if not a working day,
-        # the next working day*) as "day zero". The first of the twenty full
-        # working days is the next day. We return the date of the last of the twenty.
-        #
-        # * See this response for example of a public authority complaining when we got
-        # that detail wrong: http://www.whatdotheyknow.com/request/policy_regarding_body_scans#incoming-1100
-       
-        # We have to skip non-working days at start to find day zero, so start at
-        # day -1 and at yesterday, so we can do that.
-        days_passed = -1 
-        response_required_by = request_date - 1.day
-
-        # Now step forward into day zero, and then each of the 20 days.
-        while days_passed < 20
-            response_required_by += 1.day
-            next if response_required_by.wday == 0 || response_required_by.wday == 6 # weekend
-            next if holidays.include?(response_required_by)
-            days_passed += 1
-        end
-
-        return response_required_by
+        return Holiday.due_date_from(last_sent.outgoing_message.last_sent_at)
     end
 
     def days_overdue
diff --git a/spec/models/holiday_spec.rb b/spec/models/holiday_spec.rb
new file mode 100644
index 000000000..f4dfc7476
--- /dev/null
+++ b/spec/models/holiday_spec.rb
@@ -0,0 +1,43 @@
+require File.dirname(__FILE__) + '/../spec_helper'
+
+describe Holiday, " when calculating due date" do
+    fixtures :holidays
+
+    def due_date(ymd) 
+        return Holiday.due_date_from(Date.strptime(ymd)).strftime("%F")
+    end
+
+    it "handles no holidays" do
+      due_date('2008-10-01').should == '2008-10-29'
+    end
+
+    it "handles non leap years" do
+      due_date('2007-02-01').should == '2007-03-01'
+    end
+
+    it "handles leap years" do
+      due_date('2008-02-01').should == '2008-02-29'
+    end
+
+    it "handles Thursday start" do
+      due_date('2009-03-12').should == '2009-04-14'
+    end
+
+    it "handles Friday start" do
+      due_date('2009-03-13').should == '2009-04-15'
+    end
+
+    it "handles Saturday start" do
+      due_date('2009-03-14').should == '2009-04-16'
+    end
+
+    it "handles Sunday start" do
+      due_date('2009-03-15').should == '2009-04-16'
+    end
+
+    it "handles Monday start" do
+      due_date('2009-03-16').should == '2009-04-16' 
+    end
+
+end
+
diff --git a/spec/models/info_request_spec.rb b/spec/models/info_request_spec.rb
index acb046e03..5866e4bca 100644
--- a/spec/models/info_request_spec.rb
+++ b/spec/models/info_request_spec.rb
@@ -78,42 +78,6 @@ describe InfoRequest, " when emailing" do
 
 end 
 
-describe InfoRequest, " when calculating due date" do
-    fixtures :info_requests, :info_request_events, :public_bodies, :users, :holidays
-
-    before do
-        @ir = info_requests(:fancy_dog_request)
-    end
-
-    it "knows when it needs answered by" do
-        @ir.date_response_required_by.strftime("%F").should == '2007-11-22'
-    end
-
-    # These ones should all move when the underlying method moves
-    # I'm not sure what the best way is in RSpec to do this sort of data
-    # driven test so that it reports which one is failing rather than
-    # breaking out on first failure
-
-    test_dates = {
-        'no_holidays'   => ['2008-10-01' , '2008-10-29' ],
-        'not_leap_year' => ['2007-02-01' , '2007-03-01' ],
-        'leap_year'     => ['2008-02-01' , '2008-02-29' ],
-        'on_thu'        => ['2009-03-12' , '2009-04-14' ],
-        'on_fri'        => ['2009-03-13' , '2009-04-15' ],
-        'on_sat'        => ['2009-03-14' , '2009-04-16' ],
-        'on_sun'        => ['2009-03-15' , '2009-04-16' ],
-        'on_mon'        => ['2009-03-16' , '2009-04-16' ],
-    }
-
-    it "gets it right" do
-        test_dates.each_pair do |name, date|
-            reqdate = Date.strptime(date[0])
-            @ir.due_date_for_request_date(reqdate).strftime("%F").should == date[1]
-        end
-    end
-
-end
-
 describe InfoRequest, "when calculating status" do
     fixtures :public_bodies, :users