Merge remote-tracking branch 'openaustralia_github/calendar_days' into develop

6 files changed, 119 insertions, 47 deletions

diff --git a/app/controllers/holiday_controller.rb b/app/controllers/holiday_controller.rb
index 7f62aa26d..9430c0756 100644
--- a/app/controllers/holiday_controller.rb
+++ b/app/controllers/holiday_controller.rb
@@ -14,7 +14,9 @@ class HolidayController < ApplicationController
     def due_date
         if params[:holiday]
             @request_date = Date.strptime(params[:holiday]) or raise "Invalid date"
-            @due_date = Holiday.due_date_from(@request_date, 20)
+            days_later = MySociety::Config.get('REPLY_LATE_AFTER_DAYS', 20)
+            working_or_calendar_days = MySociety::Config.get('WORKING_OR_CALENDAR_DAYS', 'working')
+            @due_date = Holiday.due_date_from(@request_date, days_later, working_or_calendar_days)
             @skipped = Holiday.all(
                 :conditions => [ 'day >= ? AND day <= ?',
                     @request_date.strftime("%F"), @due_date.strftime("%F")
diff --git a/app/models/holiday.rb b/app/models/holiday.rb
index debd88dec..2a697a7a2 100644
--- a/app/models/holiday.rb
+++ b/app/models/holiday.rb
@@ -25,15 +25,31 @@
 
 class Holiday < ActiveRecord::Base
 
-    # Calculate the date on which a request made on a given date falls due.
+    def Holiday.weekend_or_holiday?(date)
+        # TODO only fetch holidays after the start_date
+        holidays = self.all.collect { |h| h.day }.to_set
+
+        date.wday == 0 || date.wday == 6 || holidays.include?(date)
+    end
+
+    def Holiday.due_date_from(start_date, days, type_of_days)
+        case type_of_days
+        when "working"
+            Holiday.due_date_from_working_days(start_date, days)
+        when "calendar"
+            Holiday.due_date_from_calendar_days(start_date, days)
+        else
+            raise "Unexpected value for type_of_days: #{type_of_days}"
+        end
+    end
+
+    # Calculate the date on which a request made on a given date falls due when
+    # days are given in working days
     # i.e. it is due by the end of that day.
-    def Holiday.due_date_from(start_date, working_days)
+    def Holiday.due_date_from_working_days(start_date, working_days)
         # convert date/times into dates
         start_date = start_date.to_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 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.
@@ -48,13 +64,25 @@ class Holiday < ActiveRecord::Base
 
         # Now step forward into each of the 20 days.
         while days_passed < working_days
-            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
+            response_required_by += 1
+            days_passed += 1 unless weekend_or_holiday?(response_required_by)
         end
 
-        return response_required_by
+        response_required_by
     end
 
+    # Calculate the date on which a request made on a given date falls due when
+    # the days are given in calendar days (rather than working days)
+    # If the due date falls on a weekend or a holiday then the due date is the next
+    # weekday that isn't a holiday.
+    def Holiday.due_date_from_calendar_days(start_date, days)
+        # convert date/times into dates
+        start_date = start_date.to_date
+
+        response_required_by = start_date + days
+        while weekend_or_holiday?(response_required_by)
+            response_required_by += 1
+        end
+        response_required_by
+    end
 end
diff --git a/app/models/info_request.rb b/app/models/info_request.rb
index b62f67ee1..141440c6d 100644
--- a/app/models/info_request.rb
+++ b/app/models/info_request.rb
@@ -690,7 +690,8 @@ public
     # things, e.g. fees, not properly covered.
     def date_response_required_by
         days_later = MySociety::Config.get('REPLY_LATE_AFTER_DAYS', 20)
-        return Holiday.due_date_from(self.date_initial_request_last_sent_at, days_later)
+        working_or_calendar_days = MySociety::Config.get('WORKING_OR_CALENDAR_DAYS', 'working')
+        return Holiday.due_date_from(self.date_initial_request_last_sent_at, days_later, working_or_calendar_days)
     end
     # This is a long stop - even with UK public interest test extensions, 40
     # days is a very long time.
@@ -698,12 +699,13 @@ public
         last_sent = last_event_forming_initial_request
         very_late_days_later = MySociety::Config.get('REPLY_VERY_LATE_AFTER_DAYS', 40)
         school_very_late_days_later = MySociety::Config.get('SPECIAL_REPLY_VERY_LATE_AFTER_DAYS', 60)
+        working_or_calendar_days = MySociety::Config.get('WORKING_OR_CALENDAR_DAYS', 'working')    
         if self.public_body.is_school?
             # schools have 60 working days maximum (even over a long holiday)
-            return Holiday.due_date_from(self.date_initial_request_last_sent_at, 60)
+            return Holiday.due_date_from(self.date_initial_request_last_sent_at, school_very_late_days_later, working_or_calendar_days)
         else
             # public interest test ICO guidance gives 40 working maximum
-            return Holiday.due_date_from(self.date_initial_request_last_sent_at, 40)
+            return Holiday.due_date_from(self.date_initial_request_last_sent_at, very_late_days_later, working_or_calendar_days)
         end
     end
 
diff --git a/config/general.yml-example b/config/general.yml-example
index a6f657d96..fd27b151a 100644
--- a/config/general.yml-example
+++ b/config/general.yml-example
@@ -30,6 +30,9 @@ REPLY_LATE_AFTER_DAYS: 20
 REPLY_VERY_LATE_AFTER_DAYS: 40
 # We give some types of authority like schools a bit longer than everyone else
 SPECIAL_REPLY_VERY_LATE_AFTER_DAYS: 60
+# Whether the days above are given in working or calendar days. Value can be "working" or "calendar".
+# Default is "working".
+WORKING_OR_CALENDAR_DAYS: working
 
 # example public bodies for the home page, semicolon delimited - short_names
 FRONTPAGE_PUBLICBODY_EXAMPLES: 'tgq'
diff --git a/spec/models/customstates.rb b/spec/models/customstates.rb
index 3488e6730..bffbe86fb 100644
--- a/spec/models/customstates.rb
+++ b/spec/models/customstates.rb
@@ -13,7 +13,7 @@ module InfoRequestCustomStates
             return 'deadline_extended' if
                 Time.now.strftime("%Y-%m-%d") < self.date_deadline_extended.strftime("%Y-%m-%d")
             return 'waiting_response_very_overdue'  if 
-                Time.now.strftime("%Y-%m-%d") > Holiday.due_date_from(self.date_deadline_extended, 15).strftime("%Y-%m-%d")
+                Time.now.strftime("%Y-%m-%d") > Holiday.due_date_from_working_days(self.date_deadline_extended, 15).strftime("%Y-%m-%d")
             return 'waiting_response_overdue'
         end
         return 'waiting_response_very_overdue' if
@@ -27,7 +27,7 @@ module InfoRequestCustomStates
         # XXX shouldn't this be 15 days after the date the status was
         # changed to "deadline extended"? Or perhaps 15 days ater the
         # initial request due date?
-        return Holiday.due_date_from(self.date_response_required_by, 15)
+        return Holiday.due_date_from_working_days(self.date_response_required_by, 15)
     end
 
     module ClassMethods 
diff --git a/spec/models/holiday_spec.rb b/spec/models/holiday_spec.rb
index 00ebc7279..5d3f76d24 100644
--- a/spec/models/holiday_spec.rb
+++ b/spec/models/holiday_spec.rb
@@ -3,47 +3,84 @@ require File.expand_path(File.dirname(__FILE__) + '/../spec_helper')
 describe Holiday, " when calculating due date" do
 
     def due_date(ymd) 
-        return Holiday.due_date_from(Date.strptime(ymd), 20).strftime("%F")
+        return Holiday.due_date_from_working_days(Date.strptime(ymd), 20).strftime("%F")
     end
 
-    it "handles no holidays" do
-      due_date('2008-10-01').should == '2008-10-29'
-    end
+    context "in working days" do
+        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 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 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 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 Friday start" do
+        due_date('2009-03-13').should == '2009-04-15'
+        end
 
-    # Delivery at the weekend ends up the same due day as if it had arrived on
-    # the Friday before. This is because the next working day (Monday) counts
-    # as day 1.
-    # See http://www.whatdotheyknow.com/help/officers#days
-    it "handles Saturday start" do
-      due_date('2009-03-14').should == '2009-04-15'
-    end
-    it "handles Sunday start" do
-      due_date('2009-03-15').should == '2009-04-15'
-    end
+        # Delivery at the weekend ends up the same due day as if it had arrived on
+        # the Friday before. This is because the next working day (Monday) counts
+        # as day 1.
+        # See http://www.whatdotheyknow.com/help/officers#days
+        it "handles Saturday start" do
+        due_date('2009-03-14').should == '2009-04-15'
+        end
+        it "handles Sunday start" do
+        due_date('2009-03-15').should == '2009-04-15'
+        end
 
-    it "handles Monday start" do
-      due_date('2009-03-16').should == '2009-04-16' 
-    end
+        it "handles Monday start" do
+        due_date('2009-03-16').should == '2009-04-16' 
+        end
 
-    it "handles Time objects" do
-      Holiday.due_date_from(Time.utc(2009, 03, 16, 12, 0, 0), 20).strftime('%F').should == '2009-04-16' 
+        it "handles Time objects" do
+        Holiday.due_date_from_working_days(Time.utc(2009, 03, 16, 12, 0, 0), 20).strftime('%F').should == '2009-04-16' 
+        end
     end
 
+    context "in calendar days" do
+        it "handles no holidays" do
+            Holiday.due_date_from_calendar_days(Date.new(2008, 10, 1), 20).should == Date.new(2008, 10, 21)
+        end
+
+        it "handles the due date falling on a Friday" do
+            Holiday.due_date_from_calendar_days(Date.new(2008, 10, 4), 20).should == Date.new(2008, 10, 24)
+        end
+
+        # If the due date would fall on a Saturday it should in fact fall on the next day that isn't a weekend
+        # or a holiday
+        it "handles the due date falling on a Saturday" do
+            Holiday.due_date_from_calendar_days(Date.new(2008, 10, 5), 20).should == Date.new(2008, 10, 27)
+        end
+
+        it "handles the due date falling on a Sunday" do
+            Holiday.due_date_from_calendar_days(Date.new(2008, 10, 6), 20).should == Date.new(2008, 10, 27)
+        end
+
+        it "handles the due date falling on a Monday" do
+            Holiday.due_date_from_calendar_days(Date.new(2008, 10, 7), 20).should == Date.new(2008, 10, 27)
+        end
+
+        it "handles the due date falling on a day before a Holiday" do
+            Holiday.due_date_from_calendar_days(Date.new(2008, 12, 4), 20).should == Date.new(2008, 12, 24)
+        end
+
+        it "handles the due date falling on a Holiday" do
+            Holiday.due_date_from_calendar_days(Date.new(2008, 12, 5), 20).should == Date.new(2008, 12, 29)
+        end
+
+        it "handles Time objects" do
+            Holiday.due_date_from_calendar_days(Time.utc(2009, 03, 17, 12, 0, 0), 20).should == Date.new(2009, 4, 6)
+        end
+    end
 end