FIX: Topic Timer auto opening closed topics (#10524)

This commit is addressing an issue where it is possible that there could
be multiple topic timer jobs running to close a topic or a weird race
condition state causing a topic that was just closed to be re-opened.

By removing the logic from the Topic Timer model into the Topic Timer
controller endpoint we isolate the code that is used for setting an
auto-open or an auto-close timer to just that functionality making the
topic timer background jobs safer if multiple are running.

Possibly in the future if we would like this logic back in the model a
refactor will be needed where we actually pass in the auto-close and
auto-open action instead of mixing it with the close and open
action that is currently being passed to the controller.

app/controllers/topics_controller.rb

@@ -463,6 +463,15 @@ class TopicsController < ApplicationController
     options.merge!(category_id: params[:category_id]) if !params[:category_id].blank?
     options.merge!(duration: params[:duration].to_i) if params[:duration].present?
 
+    # Be sure to close/open the topic when setting an auto-open/auto-close timer
+    if status_type == TopicTimer.types[:open]
+      topic.update_status('closed', true, current_user) if !topic.closed
+    end
+
+    if status_type == TopicTimer.types[:close]
+      topic.update_status('closed', false, current_user) if topic.closed
+    end
+
     topic_status_update = topic.set_or_create_timer(
       status_type,
       params[:time],

app/models/topic_timer.rb

@@ -127,7 +127,6 @@ class TopicTimer < ActiveRecord::Base
 
   def schedule_auto_open_job(time)
     return unless topic
-    topic.update_status('closed', true, user) if !topic.closed
 
     Jobs.enqueue_at(time, :toggle_topic_closed,
       topic_timer_id: id,
@@ -137,7 +136,6 @@ class TopicTimer < ActiveRecord::Base
 
   def schedule_auto_close_job(time)
     return unless topic
-    topic.update_status('closed', false, user) if topic.closed
 
     Jobs.enqueue_at(time, :toggle_topic_closed,
       topic_timer_id: id,

spec/models/topic_timer_spec.rb

@@ -138,7 +138,7 @@ RSpec.describe TopicTimer, type: :model do
       end
     end
 
-    describe 'when a open topic status update is created for an open topic' do
+    describe 'when a topic has been deleted' do
       fab!(:topic) { Fabricate(:topic, closed: false) }
       fab!(:topic_timer) do
         Fabricate(:topic_timer,
@@ -151,46 +151,11 @@ RSpec.describe TopicTimer, type: :model do
         Jobs.run_immediately!
       end
 
-      it 'should close the topic' do
+      it 'should not queue the job' do
+        topic.trash!
         topic_timer
-        expect(topic.reload.closed).to eq(true)
-      end
 
-      describe 'when topic has been deleted' do
-        it 'should not queue the job' do
-          topic.trash!
-          topic_timer
-
-          expect(Jobs::ToggleTopicClosed.jobs).to eq([])
-        end
-      end
-    end
-
-    describe 'when a close topic status update is created for a closed topic' do
-      fab!(:topic) { Fabricate(:topic, closed: true) }
-      fab!(:topic_timer) do
-        Fabricate(:topic_timer,
-          status_type: described_class.types[:close],
-          topic: topic
-        )
-      end
-
-      before do
-        Jobs.run_immediately!
-      end
-
-      it 'should open the topic' do
-        topic_timer
-        expect(topic.reload.closed).to eq(false)
-      end
-
-      describe 'when topic has been deleted' do
-        it 'should not queue the job' do
-          topic.trash!
-          topic_timer
-
-          expect(Jobs::ToggleTopicClosed.jobs).to eq([])
-        end
+        expect(Jobs::ToggleTopicClosed.jobs).to eq([])
       end
     end
 

spec/requests/topics_controller_spec.rb

@@ -2925,6 +2925,29 @@ RSpec.describe TopicsController do
           expect(response.body).to include('status_type')
         end
       end
+
+      it 'should close the topic when setting an auto-open timer' do
+        post "/t/#{topic.id}/timer.json", params: {
+          time: 24,
+          status_type: "open"
+        }
+        expect(response.status).to eq(200)
+        topic_status_update = TopicTimer.last
+        topic = topic_status_update.topic
+        expect(topic.closed).to eq(true)
+      end
+
+      it 'should open the topic when setting an auto-close timer' do
+        topic = Fabricate(:topic, user: user, closed: true)
+        post "/t/#{topic.id}/timer.json", params: {
+          time: 24,
+          status_type: "close"
+        }
+        expect(response.status).to eq(200)
+        topic_status_update = TopicTimer.last
+        topic = topic_status_update.topic
+        expect(topic.closed).to eq(false)
+      end
     end
   end