DEV: Split toggle topic close job (#11679)
Splits the ToggleTopicClosed
job into two distinct OpenTopic
and CloseTopic
jobs to make the code clearer. The old job cannot be deleted yet because of outstanding sidekiq schedules, so a todo has been added to do so later this year.
Also replaced mentions of topic_status_update
with topic_timer
in some files, because the topic_status_update
model is obsolete and replaced by topic timer.
Added some shortcut methods for checking if a topic is open/whether a user can change an open topic.
diff --git a/app/jobs/regular/bump_topic.rb b/app/jobs/regular/bump_topic.rb
index e795592..5a60883 100644
--- a/app/jobs/regular/bump_topic.rb
+++ b/app/jobs/regular/bump_topic.rb
@@ -4,7 +4,7 @@ module Jobs
class BumpTopic < ::Jobs::Base
def execute(args)
- topic_timer = TopicTimer.find_by(id: args[:topic_timer_id] || args[:topic_status_update_id])
+ topic_timer = TopicTimer.find_by(id: args[:topic_timer_id])
topic = topic_timer&.topic
diff --git a/app/jobs/regular/clear_slow_mode.rb b/app/jobs/regular/clear_slow_mode.rb
index 0c45421..0e7f0cf 100644
--- a/app/jobs/regular/clear_slow_mode.rb
+++ b/app/jobs/regular/clear_slow_mode.rb
@@ -4,7 +4,7 @@ module Jobs
class ClearSlowMode < ::Jobs::Base
def execute(args)
- topic_timer = TopicTimer.find_by(id: args[:topic_timer_id] || args[:topic_status_update_id])
+ topic_timer = TopicTimer.find_by(id: args[:topic_timer_id])
if topic_timer.nil? || topic_timer.execute_at > Time.zone.now
return
diff --git a/app/jobs/regular/close_topic.rb b/app/jobs/regular/close_topic.rb
new file mode 100644
index 0000000..96364d5
--- /dev/null
+++ b/app/jobs/regular/close_topic.rb
@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+
+module Jobs
+ class CloseTopic < ::Jobs::Base
+ def execute(args)
+ topic_timer = TopicTimer.find_by(id: args[:topic_timer_id])
+ return if !topic_timer&.runnable?
+
+ topic = topic_timer.topic
+ user = topic_timer.user
+ silent = args[:silent]
+
+ if topic.blank? || topic.closed?
+ topic_timer.destroy!
+ return
+ end
+
+ if !Guardian.new(user).can_close_topic?(topic)
+ topic_timer.destroy!
+ topic.reload
+
+ if topic_timer.based_on_last_post
+ topic.inherit_auto_close_from_category(timer_type: silent ? :silent_close : :close)
+ end
+
+ return
+ end
+
+ # this handles deleting the topic timer as wel, see TopicStatusUpdater
+ topic.update_status('autoclosed', true, user, { silent: silent })
+ end
+ end
+end
diff --git a/app/jobs/regular/delete_replies.rb b/app/jobs/regular/delete_replies.rb
index 9ec384e..0e5605a 100644
--- a/app/jobs/regular/delete_replies.rb
+++ b/app/jobs/regular/delete_replies.rb
@@ -4,7 +4,7 @@ module Jobs
class DeleteReplies < ::Jobs::Base
def execute(args)
- topic_timer = TopicTimer.find_by(id: args[:topic_timer_id] || args[:topic_status_update_id])
+ topic_timer = TopicTimer.find_by(id: args[:topic_timer_id])
topic = topic_timer&.topic
diff --git a/app/jobs/regular/delete_topic.rb b/app/jobs/regular/delete_topic.rb
index be6ab19..829a833 100644
--- a/app/jobs/regular/delete_topic.rb
+++ b/app/jobs/regular/delete_topic.rb
@@ -4,7 +4,7 @@ module Jobs
class DeleteTopic < ::Jobs::Base
def execute(args)
- topic_timer = TopicTimer.find_by(id: args[:topic_timer_id] || args[:topic_status_update_id])
+ topic_timer = TopicTimer.find_by(id: args[:topic_timer_id])
topic = topic_timer&.topic
diff --git a/app/jobs/regular/open_topic.rb b/app/jobs/regular/open_topic.rb
new file mode 100644
index 0000000..36246cb
--- /dev/null
+++ b/app/jobs/regular/open_topic.rb
@@ -0,0 +1,45 @@
+# frozen_string_literal: true
+
+module Jobs
+ class OpenTopic < ::Jobs::Base
+ def execute(args)
+ topic_timer = TopicTimer.find_by(id: args[:topic_timer_id])
+ return if !topic_timer&.runnable?
+
+ topic = topic_timer.topic
+ user = topic_timer.user
+
+ if topic.blank?
+ topic_timer.destroy!
+ return
+ end
+
+ if !Guardian.new(user).can_open_topic?(topic) || topic.open?
+ topic_timer.destroy!
+ topic.reload
+
+ topic.inherit_auto_close_from_category(timer_type: :close)
+
+ return
+ end
+
+ # guards against reopening a topic too early if the topic has
+ # been auto closed because of reviewables/reports, this will
+ # just update the existing topic timer and push it down the line
+ if topic.auto_close_threshold_reached?
+ topic.set_or_create_timer(
+ TopicTimer.types[:open],
+ SiteSetting.num_hours_to_close_topic,
+ by_user: Discourse.system_user
+ )
+ else
+
+ # autoclosed, false is just another way of saying open.
+ # this handles deleting the topic timer as wel, see TopicStatusUpdater
+ topic.update_status('autoclosed', false, user)
+ end
+
+ topic.inherit_auto_close_from_category(timer_type: :close)
+ end
+ end
+end
diff --git a/app/jobs/regular/publish_topic_to_category.rb b/app/jobs/regular/publish_topic_to_category.rb
index ca0010e..f3c4114 100644
--- a/app/jobs/regular/publish_topic_to_category.rb
+++ b/app/jobs/regular/publish_topic_to_category.rb
@@ -3,7 +3,7 @@
module Jobs
class PublishTopicToCategory < ::Jobs::Base
def execute(args)
- topic_timer = TopicTimer.find_by(id: args[:topic_timer_id] || args[:topic_status_update_id])
+ topic_timer = TopicTimer.find_by(id: args[:topic_timer_id])
return if topic_timer.blank?
topic = topic_timer.topic
diff --git a/app/jobs/regular/toggle_topic_closed.rb b/app/jobs/regular/toggle_topic_closed.rb
index cc10d5a..55f7bce 100644
--- a/app/jobs/regular/toggle_topic_closed.rb
+++ b/app/jobs/regular/toggle_topic_closed.rb
@@ -1,9 +1,14 @@
# frozen_string_literal: true
module Jobs
+ # TODO: DEPRECATED - Use OpenTopic and CloseTopic instead.
+ # (martin - 2021-05-01) - Delete once topic timer revamp is completed.
class ToggleTopicClosed < ::Jobs::Base
def execute(args)
topic_timer = TopicTimer.find_by(id: args[:topic_timer_id] || args[:topic_status_update_id])
+
+ # state false is Open Topic
+ # state true is Close Topic
state = !!args[:state]
timer_type = args[:silent] ? :silent_close : :close
diff --git a/app/models/topic.rb b/app/models/topic.rb
index 2e910dc..fa21ad9 100644
--- a/app/models/topic.rb
+++ b/app/models/topic.rb
@@ -576,13 +576,17 @@ class Topic < ActiveRecord::Base
end
def private_message?
- archetype == Archetype.private_message
+ self.archetype == Archetype.private_message
end
def regular?
self.archetype == Archetype.default
end
+ def open?
+ !self.closed?
+ end
+
MAX_SIMILAR_BODY_LENGTH ||= 200
def self.similar_to(title, raw, user = nil)
diff --git a/app/models/topic_timer.rb b/app/models/topic_timer.rb
index 0d7be78..053742e 100644
--- a/app/models/topic_timer.rb
+++ b/app/models/topic_timer.rb
@@ -15,7 +15,7 @@ class TopicTimer < ActiveRecord::Base
validates :status_type, uniqueness: { scope: [:topic_id, :deleted_at, :user_id] }, if: :private_type?
validates :category_id, presence: true, if: :publishing_to_category?
- validate :ensure_update_will_happen
+ validate :executed_at_in_future?
scope :scheduled_bump_topics, -> { where(status_type: TopicTimer.types[:bump], deleted_at: nil).pluck(:topic_id) }
@@ -84,23 +84,38 @@ class TopicTimer < ActiveRecord::Base
!!self.class.private_types[self.status_type]
end
+ def runnable?
+ return false if deleted_at.present?
+ return false if execute_at > Time.zone.now
+ true
+ end
+
private
- def ensure_update_will_happen
- if created_at && (execute_at < created_at)
- errors.add(:execute_at, I18n.t(
- 'activerecord.errors.models.topic_timer.attributes.execute_at.in_the_past'
- ))
- end
+ def executed_at_in_future?
+ return if created_at.blank? || (execute_at > created_at)
+
+ errors.add(:execute_at, I18n.t(
+ 'activerecord.errors.models.topic_timer.attributes.execute_at.in_the_past'
+ ))
end
+ # TODO(martin - 2021-05-01) - Remove cancels for toggle_topic_closed once topic timer revamp completed.
def cancel_auto_close_job
Jobs.cancel_scheduled_job(:toggle_topic_closed, topic_timer_id: id)
+ Jobs.cancel_scheduled_job(:close_topic, topic_timer_id: id)
+ end
+
[... diff too long, it was truncated ...]
GitHub sha: 2404fa7a