From 71ce9ced474595ffa9eaf11d6029972698fad624 Mon Sep 17 00:00:00 2001
From: Guo Xiang Tan <tgx_world@hotmail.com>
Date: Thu, 13 Dec 2018 16:39:36 +0800
Subject: [PATCH] FIX: Incorrect arguments were being passed to
 `Jobs::ClosePoll`.

Also fix spec that wasn't testing anything.
---
 plugins/poll/jobs/regular/close_poll.rb       | 14 +++++++++++-
 plugins/poll/plugin.rb                        |  9 ++++++--
 .../spec/controllers/posts_controller_spec.rb | 22 ++++++++++++++-----
 .../poll/spec/jobs/regular/close_poll_spec.rb | 15 +++++++++++--
 4 files changed, 50 insertions(+), 10 deletions(-)

diff --git a/plugins/poll/jobs/regular/close_poll.rb b/plugins/poll/jobs/regular/close_poll.rb
index ccfac109314..6e75241aabf 100644
--- a/plugins/poll/jobs/regular/close_poll.rb
+++ b/plugins/poll/jobs/regular/close_poll.rb
@@ -3,7 +3,19 @@ module Jobs
   class ClosePoll < Jobs::Base
 
     def execute(args)
-      DiscoursePoll::Poll.toggle_status(args[:post_id], args[:poll_name], "closed", Discourse.system_user)
+      %i{
+        post_id
+        poll_name
+      }.each do |key|
+        raise Discourse::InvalidParameters.new(key) if args[key].blank?
+      end
+
+      DiscoursePoll::Poll.toggle_status(
+        args[:post_id],
+        args[:poll_name],
+        "closed",
+        Discourse.system_user
+      )
     end
 
   end
diff --git a/plugins/poll/plugin.rb b/plugins/poll/plugin.rb
index cbf7ab43582..9d62323c55f 100644
--- a/plugins/poll/plugin.rb
+++ b/plugins/poll/plugin.rb
@@ -221,10 +221,15 @@ after_initialize do
 
       def schedule_jobs(post)
         Poll.where(post: post).find_each do |poll|
-          Jobs.cancel_scheduled_job(:close_poll, poll_id: poll.id)
+          job_args = {
+            post_id: post.id,
+            poll_name: poll.name
+          }
+
+          Jobs.cancel_scheduled_job(:close_poll, job_args)
 
           if poll.open? && poll.close_at && poll.close_at > Time.zone.now
-            Jobs.enqueue_at(poll.close_at, :close_poll, poll_id: poll.id)
+            Jobs.enqueue_at(poll.close_at, :close_poll, job_args)
           end
         end
       end
diff --git a/plugins/poll/spec/controllers/posts_controller_spec.rb b/plugins/poll/spec/controllers/posts_controller_spec.rb
index 912e58c017c..30fcad9cde3 100644
--- a/plugins/poll/spec/controllers/posts_controller_spec.rb
+++ b/plugins/poll/spec/controllers/posts_controller_spec.rb
@@ -35,17 +35,29 @@ describe PostsController do
     end
 
     it "schedules auto-close job" do
+      freeze_time
       name = "auto_close"
       close_date = 1.month.from_now
 
-      post :create, params: {
-        title: title, raw: "[poll name=#{name} close=#{close_date.iso8601}]\n- A\n- B\n[/poll]"
-      }, format: :json
+      expect do
+        post :create, params: {
+          title: title,
+          raw: "[poll name=#{name} close=#{close_date.iso8601}]\n- A\n- B\n[/poll]"
+        }, format: :json
+      end.to change { Jobs::ClosePoll.jobs.size }.by(1) &
+             change { Poll.count }.by(1)
 
       expect(response.status).to eq(200)
       json = ::JSON.parse(response.body)
-      expect(Poll.find_by(post_id: json["id"]).close_at).to be
-      expect(Jobs.scheduled_for(:close_poll, post_id: json["id"], poll_name: name)).to be
+      post_id = json["id"]
+
+      expect(Poll.find_by(post_id: post_id).close_at).to eq(1.month.from_now)
+
+      job = Jobs::ClosePoll.jobs.first
+      job_args = job["args"].first
+
+      expect(job_args["post_id"]).to eq(post_id)
+      expect(job_args["poll_name"]).to eq(name)
     end
 
     it "should have different options" do
diff --git a/plugins/poll/spec/jobs/regular/close_poll_spec.rb b/plugins/poll/spec/jobs/regular/close_poll_spec.rb
index 3dfe2188967..8e5c2b282cc 100644
--- a/plugins/poll/spec/jobs/regular/close_poll_spec.rb
+++ b/plugins/poll/spec/jobs/regular/close_poll_spec.rb
@@ -1,10 +1,21 @@
 require "rails_helper"
 
 describe Jobs::ClosePoll do
+  let(:post) { Fabricate(:post, raw: "[poll]\n- A\n- B\n[/poll]") }
+
+  describe 'missing arguments' do
+    it 'should raise the right error' do
+      expect do
+        Jobs::ClosePoll.new.execute(post_id: post.id)
+      end.to raise_error(Discourse::InvalidParameters, "poll_name")
+
+      expect do
+        Jobs::ClosePoll.new.execute(poll_name: "poll")
+      end.to raise_error(Discourse::InvalidParameters, "post_id")
+    end
+  end
 
   it "automatically closes a poll" do
-    post = Fabricate(:post, raw: "[poll]\n- A\n- B\n[/poll]")
-
     expect(post.polls.first.closed?).to eq(false)
 
     Jobs::ClosePoll.new.execute(post_id: post.id, poll_name: "poll")