Merge pull request #505 from alphagov/fix-off-by-one-closing-dates

Make sure jobs run at midnight in the application time zone

Author: h-lame

app/jobs/close_petitions_job.rb

@@ -1,7 +1,7 @@
 class ClosePetitionsJob < ActiveJob::Base
   queue_as :high_priority
 
-  def perform
-    Petition.close_petitions!
+  def perform(time)
+    Petition.close_petitions!(time.in_time_zone)
   end
 end

app/jobs/debated_petitions_job.rb

@@ -1,7 +1,7 @@
 class DebatedPetitionsJob < ActiveJob::Base
   queue_as :high_priority
 
-  def perform
-    Petition.mark_petitions_as_debated!
+  def perform(date)
+    Petition.mark_petitions_as_debated!(date.to_date)
   end
 end

app/models/petition.rb

@@ -281,12 +281,12 @@ def sanitized_tag(tag)
       "[#{tag.gsub(/[\[\]%]/,'')}]"
     end
 
-    def in_need_of_marking_as_debated
-      where(scheduled_debate_state.and(debate_date_in_the_past))
+    def in_need_of_marking_as_debated(date = Date.current)
+      where(scheduled_debate_state.and(debate_date_in_the_past(date)))
     end
 
-    def mark_petitions_as_debated!
-      in_need_of_marking_as_debated.update_all(debate_state: 'debated')
+    def mark_petitions_as_debated!(date = Date.current)
+      in_need_of_marking_as_debated(date).update_all(debate_state: 'debated')
     end
 
     private
@@ -303,8 +303,8 @@ def awaiting_debate_state
       arel_table[:debate_state].eq('awaiting')
     end
 
-    def debate_date_in_the_past
-      arel_table[:scheduled_debate_date].lt(Date.current)
+    def debate_date_in_the_past(date)
+      arel_table[:scheduled_debate_date].lt(date)
     end
 
     def scheduled_debate_state

config/schedule.rb

@@ -35,10 +35,10 @@
   runner "PetitionCountJob.perform_later"
 end
 
-every :day, at: '0.00am' do
-  runner "ClosePetitionsJob.perform_later"
+every :day, at: '7.00am' do
+  rake "epets:petitions:close", output: nil
 end
 
-every :day, at: '0.00am' do
-  runner "DebatedPetitionsJob.perform_later"
+every :day, at: '7.15am' do
+  rake "epets:petitions:debated", output: nil
 end

lib/tasks/petitions.rake

@@ -0,0 +1,15 @@
+namespace :epets do
+  namespace :petitions do
+    desc "Add task to the queue to close petitions at midnight"
+    task :close => :environment do
+      time = Date.tomorrow.beginning_of_day
+      ClosePetitionsJob.set(wait_until: time).perform_later(time.iso8601)
+    end
+
+    desc "Add task to the queue to mark petitions as debated at midnight"
+    task :debated => :environment do
+      date = Date.tomorrow
+      DebatedPetitionsJob.set(wait_until: date.beginning_of_day).perform_later(date.iso8601)
+    end
+  end
+end

spec/jobs/close_petitions_job_spec.rb

@@ -1,29 +1,75 @@
 require 'rails_helper'
 
 RSpec.describe ClosePetitionsJob, type: :job do
-  let!(:petition) { FactoryGirl.create(:open_petition, open_at: open_at) }
+  context "for a petition opened in the winter" do
+    let!(:petition) {
+      FactoryGirl.create(:open_petition, open_at: "2015-12-29T10:00:00Z")
+    }
 
-  context "when a petition is in the open state and closing date has not passed" do
-    let(:open_at) { Site.opened_at_for_closing(1.day.from_now) }
+    around do |example|
+      travel_to(now)
+      example.run
+      travel_back
+    end
+
+    context "and the closing date has not passed" do
+      let(:now) { "2016-06-28T07:00:00Z".in_time_zone }
+
+      it "does not change the petition state" do
+        expect{
+          perform_enqueued_jobs {
+            described_class.perform_later(Date.tomorrow.beginning_of_day.iso8601)
+          }
+        }.not_to change{ petition.reload.state }
+      end
+    end
+
+    context "and the closing date has passed" do
+      let(:now) { "2016-06-29T07:00:00Z".in_time_zone }
 
-    it "does not close the petition" do
-      expect{
-        perform_enqueued_jobs {
-          described_class.perform_later
-        }
-      }.not_to change{ petition.reload.state }
+      it "does change the petition debate state" do
+        expect{
+          perform_enqueued_jobs {
+            described_class.perform_later(Date.tomorrow.beginning_of_day.iso8601)
+          }
+        }.to change{ petition.reload.state }.from("open").to("closed")
+      end
     end
   end
 
-  context "when a petition is in the open state and closed_at has passed" do
-    let(:open_at) { Site.opened_at_for_closing(1.day.ago) }
+  context "for a petition opened in the summer" do
+    let!(:petition) {
+      FactoryGirl.create(:open_petition, open_at: "2016-06-29T10:00:00Z")
+    }
+
+    around do |example|
+      travel_to(now)
+      example.run
+      travel_back
+    end
+
+    context "and the debate date has not passed" do
+      let(:now) { "2016-12-28T07:00:00Z".in_time_zone }
+
+      it "does not change the petition state" do
+        expect{
+          perform_enqueued_jobs {
+            described_class.perform_later(Date.tomorrow.beginning_of_day.iso8601)
+          }
+        }.not_to change{ petition.reload.state }
+      end
+    end
+
+    context "and the debate date has passed" do
+      let(:now) { "2016-12-29T07:00:00Z".in_time_zone }
 
-    it "does close the petition" do
-      expect{
-        perform_enqueued_jobs {
-          described_class.perform_later
-        }
-      }.to change{ petition.reload.state }.from('open').to('closed')
+      it "does change the petition state" do
+        expect{
+          perform_enqueued_jobs {
+            described_class.perform_later(Date.tomorrow.beginning_of_day.iso8601)
+          }
+        }.to change{ petition.reload.state }.from("open").to("closed")
+      end
     end
   end
 end

spec/jobs/debated_petitions_job_spec.rb

@@ -1,47 +1,89 @@
 require 'rails_helper'
 
 RSpec.describe DebatedPetitionsJob, type: :job do
-  context "when a petition is in the scheduled debate state and the debate date has not passed" do
+  context "for a petition with a scheduled debate date in the winter" do
     let(:petition) {
       FactoryGirl.build(:open_petition,
         debate_state: 'scheduled',
-        scheduled_debate_date: Date.tomorrow
+        scheduled_debate_date: "2015-12-29"
       )
     }
 
-    before do
-      petition.save
+    let(:open_at) { "2015-12-01T10:00:00Z".in_time_zone }
+
+    around do |example|
+      travel_to(open_at) { petition.save }
+      travel_to(now)
+      example.run
+      travel_back
+    end
+
+    context "and the debate date has not passed" do
+      let(:now) { "2015-12-28T07:15:00Z".in_time_zone }
+
+      it "does not change the petition debate state" do
+        expect{
+          perform_enqueued_jobs {
+            described_class.perform_later(Date.tomorrow.iso8601)
+          }
+        }.not_to change{ petition.reload.debate_state }
+      end
     end
 
-    it "does not close the petition" do
-      expect{
-        perform_enqueued_jobs {
-          described_class.perform_later
-        }
-      }.not_to change{ petition.reload.debate_state }
+    context "and the debate date has passed" do
+      let(:now) { "2015-12-29T07:15:00Z".in_time_zone }
+
+      it "does change the petition debate state" do
+        expect{
+          perform_enqueued_jobs {
+            described_class.perform_later(Date.tomorrow.iso8601)
+          }
+        }.to change{ petition.reload.debate_state }.from("scheduled").to("debated")
+      end
     end
   end
 
-  context "when a petition is in the scheduled debate state and the debate date has passed" do
+  context "for a petition with a scheduled debate date in the summer" do
     let(:petition) {
       FactoryGirl.build(:open_petition,
         debate_state: 'scheduled',
-        scheduled_debate_date: Date.tomorrow
+        scheduled_debate_date: "2016-06-29"
       )
     }
 
+    let(:open_at) { "2016-06-01T10:00:00Z".in_time_zone }
+
     before do
-      travel_to(2.days.ago) do
-        petition.save
+      travel_to(open_at) { petition.save }
+      travel_to(now)
+    end
+
+    after do
+      travel_back
+    end
+
+    context "and the debate date has not passed" do
+      let(:now) { "2016-06-28T07:15:00Z".in_time_zone }
+
+      it "does not change the petition debate state" do
+        expect{
+          perform_enqueued_jobs {
+            described_class.perform_later(Date.tomorrow.iso8601)
+          }
+        }.not_to change{ petition.reload.debate_state }
       end
     end
 
-    it "does close the petition" do
-      expect{
-        perform_enqueued_jobs {
-          described_class.perform_later
-        }
-      }.to change{ petition.reload.debate_state }.from("scheduled").to("debated")
+    context "and the debate date has passed" do
+      let(:now) { "2016-06-29T07:15:00Z".in_time_zone }
+
+      it "does change the petition debate state" do
+        expect{
+          perform_enqueued_jobs {
+            described_class.perform_later(Date.tomorrow.iso8601)
+          }
+        }.to change{ petition.reload.debate_state }.from("scheduled").to("debated")
+      end
     end
   end
 end