From 03338f9086e65e8020598cd3346ed23a3d69a01e Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Tue, 29 Jun 2021 09:16:25 +1000 Subject: [PATCH] FIX: Remove legacy topic timer code (#13544) The new topic timer backend code introduced six months ago in 0034cbd is now used instead of this legacy code. It can be safely removed now. --- app/jobs/regular/topic_reminder.rb | 31 -------------- app/models/topic_timer.rb | 65 ------------------------------ spec/models/topic_timer_spec.rb | 65 ------------------------------ 3 files changed, 161 deletions(-) delete mode 100644 app/jobs/regular/topic_reminder.rb diff --git a/app/jobs/regular/topic_reminder.rb b/app/jobs/regular/topic_reminder.rb deleted file mode 100644 index e4b103d66d9..00000000000 --- a/app/jobs/regular/topic_reminder.rb +++ /dev/null @@ -1,31 +0,0 @@ -# frozen_string_literal: true - -module Jobs - class TopicReminder < ::Jobs::Base - - def execute(args) - # noop, TODO(martin 2021-03-11): Remove this after timers migrated and outstanding jobs cancelled - return - - topic_timer = TopicTimer.find_by(id: args[:topic_timer_id]) - - topic = topic_timer&.topic - user = topic_timer&.user - - if topic_timer.blank? || topic.blank? || user.blank? || - topic_timer.execute_at > Time.zone.now - return - end - - user.notifications.create!( - notification_type: Notification.types[:topic_reminder], - topic_id: topic.id, - post_number: 1, - data: { topic_title: topic.title, display_username: user.username }.to_json - ) - - topic_timer.trash!(Discourse.system_user) - end - - end -end diff --git a/app/models/topic_timer.rb b/app/models/topic_timer.rb index 2227004d566..2a9a690adef 100644 --- a/app/models/topic_timer.rb +++ b/app/models/topic_timer.rb @@ -36,10 +36,6 @@ class TopicTimer < ActiveRecord::Base if (will_save_change_to_execute_at? && !attribute_in_database(:execute_at).nil?) || will_save_change_to_user_id? - - # TODO(martin - 2021-05-01) - Remove this backwards compatability for outstanding - # jobs once they have all been run and after Jobs::TopicTimerEnqueuer is in place - self.send("cancel_auto_#{self.class.types[status_type]}_job") end end @@ -67,24 +63,9 @@ class TopicTimer < ActiveRecord::Base end def enqueue_typed_job(time: nil) - return if typed_job_scheduled? self.send("schedule_auto_#{status_type_name}_job") end - # TODO(martin - 2021-05-01) - Remove this backwards compatability for outstanding - # jobs once they have all been run and after Jobs::TopicTimerEnqueuer is in place - def typed_job_scheduled? - scheduled = Jobs.scheduled_for( - TopicTimer.type_job_map[status_type_name], topic_timer_id: id - ).any? - - if [:close, :silent_close, :open].include?(status_type_name) - return scheduled || Jobs.scheduled_for(:toggle_topic_closed, topic_timer_id: id).any? - end - - scheduled - end - def self.type_job_map { close: :close_topic, @@ -165,48 +146,6 @@ class TopicTimer < ActiveRecord::Base self.status_type.to_i == TopicTimer.types[:publish_to_category] 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 - - # TODO(martin - 2021-05-01) - Remove cancels for toggle_topic_closed once topic timer revamp completed. - def cancel_auto_open_job - Jobs.cancel_scheduled_job(:toggle_topic_closed, topic_timer_id: id) - Jobs.cancel_scheduled_job(:open_topic, topic_timer_id: id) - end - - # TODO(martin - 2021-05-01) - Remove cancels for toggle_topic_closed once topic timer revamp completed. - def cancel_auto_silent_close_job - Jobs.cancel_scheduled_job(:toggle_topic_closed, topic_timer_id: id) - Jobs.cancel_scheduled_job(:close_topic, topic_timer_id: id) - end - - def cancel_auto_publish_to_category_job - Jobs.cancel_scheduled_job(TopicTimer.type_job_map[:publish_to_category], topic_timer_id: id) - end - - def cancel_auto_delete_job - Jobs.cancel_scheduled_job(TopicTimer.type_job_map[:delete], topic_timer_id: id) - end - - def cancel_auto_reminder_job - Jobs.cancel_scheduled_job(TopicTimer.type_job_map[:reminder], topic_timer_id: id) - end - - def cancel_auto_bump_job - Jobs.cancel_scheduled_job(TopicTimer.type_job_map[:bump], topic_timer_id: id) - end - - def cancel_auto_delete_replies_job - Jobs.cancel_scheduled_job(TopicTimer.type_job_map[:delete_replies], topic_timer_id: id) - end - - def cancel_auto_clear_slow_mode_job - Jobs.cancel_scheduled_job(TopicTimer.type_job_map[:clear_slow_mode], topic_timer_id: id) - end - def schedule_auto_delete_replies_job Jobs.enqueue(TopicTimer.type_job_map[:delete_replies], topic_timer_id: id) end @@ -235,10 +174,6 @@ class TopicTimer < ActiveRecord::Base Jobs.enqueue(TopicTimer.type_job_map[:delete], topic_timer_id: id) end - def schedule_auto_reminder_job - # noop, TODO(martin 2021-03-11): Remove this after timers migrated and outstanding jobs cancelled - end - def schedule_auto_clear_slow_mode_job Jobs.enqueue(TopicTimer.type_job_map[:clear_slow_mode], topic_timer_id: id) end diff --git a/spec/models/topic_timer_spec.rb b/spec/models/topic_timer_spec.rb index cff5316d5cc..fde80ad8565 100644 --- a/spec/models/topic_timer_spec.rb +++ b/spec/models/topic_timer_spec.rb @@ -48,44 +48,6 @@ RSpec.describe TopicTimer, type: :model do end end - describe "#typed_job_scheduled?" do - let(:scheduled_jobs) { Sidekiq::ScheduledSet.new } - - after do - scheduled_jobs.clear - end - - it "returns true if the job is scheduled" do - Sidekiq::Testing.disable! do - scheduled_jobs.clear - Jobs.enqueue_at(3.hours.from_now, :close_topic, topic_timer_id: topic_timer.id) - expect(topic_timer.typed_job_scheduled?).to eq(true) - end - end - - it "returns false if the job is not already scheduled" do - Sidekiq::Testing.disable! do - scheduled_jobs.clear - expect(topic_timer.typed_job_scheduled?).to eq(false) - end - end - - it "returns true if the toggle_topic_closed job is scheduled for close,open,silent_close types" do - Sidekiq::Testing.disable! do - scheduled_jobs.clear - topic_timer1 = Fabricate(:topic_timer, status_type: TopicTimer.types[:close]) - Jobs.enqueue_at(3.hours.from_now, :toggle_topic_closed, topic_timer_id: topic_timer1.id) - topic_timer2 = Fabricate(:topic_timer, status_type: TopicTimer.types[:open]) - Jobs.enqueue_at(3.hours.from_now, :toggle_topic_closed, topic_timer_id: topic_timer2.id) - topic_timer3 = Fabricate(:topic_timer, status_type: TopicTimer.types[:silent_close]) - Jobs.enqueue_at(3.hours.from_now, :toggle_topic_closed, topic_timer_id: topic_timer3.id) - expect(topic_timer1.typed_job_scheduled?).to eq(true) - expect(topic_timer2.typed_job_scheduled?).to eq(true) - expect(topic_timer3.typed_job_scheduled?).to eq(true) - end - end - end - describe '#execute_at' do describe 'when #execute_at is greater than #created_at' do it 'should be valid' do @@ -146,38 +108,11 @@ RSpec.describe TopicTimer, type: :model do describe 'when #execute_at and #user_id are not changed' do it 'should not schedule another to update topic' do Jobs.expects(:enqueue_at).never - Jobs.expects(:cancel_scheduled_job).never topic_timer.update!(topic: Fabricate(:topic)) end end - describe 'when #execute_at value is changed' do - it 'reschedules the job' do - Jobs.expects(:cancel_scheduled_job).with( - :toggle_topic_closed, topic_timer_id: topic_timer.id - ) - Jobs.expects(:cancel_scheduled_job).with( - :close_topic, topic_timer_id: topic_timer.id - ) - - topic_timer.update!(execute_at: 3.days.from_now, created_at: Time.zone.now) - end - end - - describe 'when user is changed' do - it 'should update the job' do - Jobs.expects(:cancel_scheduled_job).with( - :toggle_topic_closed, topic_timer_id: topic_timer.id - ) - Jobs.expects(:cancel_scheduled_job).with( - :close_topic, topic_timer_id: topic_timer.id - ) - - topic_timer.update!(user: admin) - end - end - describe 'when a open topic status update is created for an open topic' do fab!(:topic) { Fabricate(:topic, closed: false) } fab!(:topic_timer) do