FIX: Topic timers being incorrectly trashed!.

https://meta.discourse.org/t/temporary-timed-closure-of-topic-not-re-opening-topic/67877

app/models/category.rb

@@ -199,7 +199,7 @@ SQL
     t = Topic.new(title: I18n.t("category.topic_prefix", category: name), user: user, pinned_at: Time.now, category_id: id)
     t.skip_callbacks = true
     t.ignore_category_auto_close = true
-    t.set_or_create_timer(TopicTimer.types[:close], nil)
+    t.delete_topic_timer(TopicTimer.types[:close])
     t.save!(validate: false)
     update_column(:topic_id, t.id)
     t.posts.create(raw: post_template, user: user)

app/models/topic.rb

@@ -994,6 +994,12 @@ SQL
     @public_topic_timer ||= topic_timers.find_by(deleted_at: nil, public_type: true)
   end
 
+  def delete_topic_timer(status_type, by_user: Discourse.system_user)
+    options = { status_type: status_type }
+    options.merge!(user: by_user) unless TopicTimer.public_types[status_type]
+    self.topic_timers.find_by(options)&.trash!(by_user)
+  end
+
   # Valid arguments for the time:
   #  * An integer, which is the number of hours from now to update the topic's status.
   #  * A timestamp, like "2013-11-25 13:00", when the topic's status should update.
@@ -1005,16 +1011,13 @@ SQL
   #  * based_on_last_post: True if time should be based on timestamp of the last post.
   #  * category_id: Category that the update will apply to.
   def set_or_create_timer(status_type, time, by_user: nil, timezone_offset: 0, based_on_last_post: false, category_id: SiteSetting.uncategorized_category_id)
+    return delete_topic_timer(status_type, by_user: by_user) if time.blank?
+
     topic_timer_options = { topic: self }
     topic_timer_options.merge!(user: by_user) unless TopicTimer.public_types[status_type]
     topic_timer = TopicTimer.find_or_initialize_by(topic_timer_options)
     topic_timer.status_type = status_type
 
-    if time.blank?
-      topic_timer.trash!(by_user || Discourse.system_user)
-      return
-    end
-
     time_now = Time.zone.now
     topic_timer.based_on_last_post = !based_on_last_post.blank?
 

app/services/topic_status_updater.rb

@@ -38,9 +38,9 @@ TopicStatusUpdater = Struct.new(:topic, :user) do
 
     if @topic_status_update
       if status.manually_closing_topic? || status.closing_topic?
-        topic.set_or_create_timer(TopicTimer.types[:close], nil)
+        topic.delete_topic_timer(TopicTimer.types[:close])
       elsif status.manually_opening_topic? || status.opening_topic?
-        topic.set_or_create_timer(TopicTimer.types[:open], nil)
+        topic.delete_topic_timer(TopicTimer.types[:open])
       end
     end
 

spec/controllers/topics_controller_spec.rb

@@ -420,6 +420,7 @@ describe TopicsController do
 
         expect(response).to be_success
         expect(@topic.reload.closed).to eq(false)
+        expect(@topic.topic_timers).to eq([])
 
         body = JSON.parse(response.body)
 

spec/models/topic_spec.rb

@@ -1177,8 +1177,6 @@ describe Topic do
     let(:admin) { Fabricate(:admin) }
     let(:trust_level_4) { Fabricate(:trust_level_4) }
 
-    before { Discourse.stubs(:system_user).returns(admin) }
-
     it 'can take a number of hours as an integer' do
       freeze_time now
 
@@ -1248,12 +1246,12 @@ describe Topic do
 
     it 'sets topic status update user to system user if given user is not staff or a TL4 user' do
       topic.set_or_create_timer(TopicTimer.types[:close], 3, by_user: Fabricate.build(:user, id: 444))
-      expect(topic.topic_timers.first.user).to eq(admin)
+      expect(topic.topic_timers.first.user).to eq(Discourse.system_user)
     end
 
     it 'sets topic status update user to system user if user is not given and topic creator is not staff nor TL4 user' do
       topic.set_or_create_timer(TopicTimer.types[:close], 3)
-      expect(topic.topic_timers.first.user).to eq(admin)
+      expect(topic.topic_timers.first.user).to eq(Discourse.system_user)
     end
 
     it 'sets topic status update user to topic creator if it is a staff user' do
@@ -1280,6 +1278,15 @@ describe Topic do
       expect(closing_topic.reload.public_topic_timer.execute_at).to eq(2.days.from_now)
     end
 
+    it 'should not delete topic_timer of another status_type' do
+      freeze_time
+      closing_topic.set_or_create_timer(TopicTimer.types[:open], nil)
+      topic_timer = closing_topic.public_topic_timer
+
+      expect(topic_timer.execute_at).to eq(5.hours.from_now)
+      expect(topic_timer.status_type).to eq(TopicTimer.types[:close])
+    end
+
     it 'should allow status_type to be updated' do
       freeze_time