diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index 7f5e36a4570..7089f89ee01 100644 --- a/app/controllers/topics_controller.rb +++ b/app/controllers/topics_controller.rb @@ -435,6 +435,8 @@ class TopicsController < ApplicationController guardian.ensure_can_moderate!(@topic) end + params[:until] === '' ? params[:until] = nil : params[:until] + @topic.update_status(status, enabled, current_user, until: params[:until]) render json: success_json.merge!( diff --git a/app/jobs/regular/remove_banner.rb b/app/jobs/regular/remove_banner.rb new file mode 100644 index 00000000000..880c8696c94 --- /dev/null +++ b/app/jobs/regular/remove_banner.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module Jobs + + class RemoveBanner < ::Jobs::Base + + def execute(args) + topic_id = args[:topic_id] + + return unless topic_id.present? + + topic = Topic.find_by(id: topic_id) + topic.remove_banner!(Discourse.system_user) if topic.present? + end + + end + +end diff --git a/app/jobs/regular/unpin_topic.rb b/app/jobs/regular/unpin_topic.rb index b99e505904b..9d4f6b93eed 100644 --- a/app/jobs/regular/unpin_topic.rb +++ b/app/jobs/regular/unpin_topic.rb @@ -7,7 +7,7 @@ module Jobs def execute(args) topic_id = args[:topic_id] - raise Discourse::InvalidParameters.new(:topic_id) unless topic_id.present? + return unless topic_id.present? topic = Topic.find_by(id: topic_id) topic.update_pinned(false) if topic.present? diff --git a/app/models/topic.rb b/app/models/topic.rb index 32c004ed0ad..00a3eacaa80 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -1093,7 +1093,15 @@ class Topic < ActiveRecord::Base @participants_summary ||= TopicParticipantsSummary.new(self, options).summary end - def make_banner!(user) + def make_banner!(user, bannered_until = nil) + if bannered_until + bannered_until = begin + Time.parse(bannered_until) + rescue ArgumentError + raise Discourse::InvalidParameters.new(:bannered_until) + end + end + # only one banner at the same time previous_banner = Topic.where(archetype: Archetype.banner).first previous_banner.remove_banner!(user) if previous_banner.present? @@ -1102,18 +1110,25 @@ class Topic < ActiveRecord::Base .update_all(dismissed_banner_key: nil) self.archetype = Archetype.banner + self.bannered_until = bannered_until self.add_small_action(user, "banner.enabled") self.save MessageBus.publish('/site/banner', banner) + + Jobs.cancel_scheduled_job(:remove_banner, topic_id: self.id) + Jobs.enqueue_at(bannered_until, :remove_banner, topic_id: self.id) if bannered_until end def remove_banner!(user) self.archetype = Archetype.default + self.bannered_until = nil self.add_small_action(user, "banner.disabled") self.save MessageBus.publish('/site/banner', nil) + + Jobs.cancel_scheduled_job(:remove_banner, topic_id: self.id) end def banner @@ -1199,12 +1214,13 @@ class Topic < ActiveRecord::Base TopicUser.change(user.id, id, cleared_pinned_at: nil) end - def update_pinned(status, global = false, pinned_until = "") - pinned_until ||= '' - - pinned_until = begin - Time.parse(pinned_until) - rescue ArgumentError + def update_pinned(status, global = false, pinned_until = nil) + if pinned_until + pinned_until = begin + Time.parse(pinned_until) + rescue ArgumentError + raise Discourse::InvalidParameters.new(:pinned_until) + end end update_columns( @@ -1233,7 +1249,10 @@ class Topic < ActiveRecord::Base def self.ensure_consistency! # unpin topics that might have been missed - Topic.where("pinned_until < now()").update_all(pinned_at: nil, pinned_globally: false, pinned_until: nil) + Topic.where('pinned_until < ?', Time.now).update_all(pinned_at: nil, pinned_globally: false, pinned_until: nil) + Topic.where('bannered_until < ?', Time.now).find_each do |topic| + topic.remove_banner!(Discourse.system_user) + end end def inherit_auto_close_from_category(timer_type: :close) diff --git a/db/migrate/20210621103509_add_bannered_until.rb b/db/migrate/20210621103509_add_bannered_until.rb new file mode 100644 index 00000000000..6a48cf416e1 --- /dev/null +++ b/db/migrate/20210621103509_add_bannered_until.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddBanneredUntil < ActiveRecord::Migration[6.1] + def change + add_column :topics, :bannered_until, :datetime, null: true + + add_index :topics, :bannered_until, where: 'bannered_until IS NOT NULL' + end +end diff --git a/db/migrate/20210624080131_add_partial_index_pinned_until.rb b/db/migrate/20210624080131_add_partial_index_pinned_until.rb new file mode 100644 index 00000000000..c91cdb45236 --- /dev/null +++ b/db/migrate/20210624080131_add_partial_index_pinned_until.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class AddPartialIndexPinnedUntil < ActiveRecord::Migration[6.1] + disable_ddl_transaction! + + def change + add_index :topics, :pinned_until, + where: 'pinned_until IS NOT NULL', + algorithm: :concurrently + end +end diff --git a/spec/jobs/remove_banner_spec.rb b/spec/jobs/remove_banner_spec.rb new file mode 100644 index 00000000000..13c60b87e93 --- /dev/null +++ b/spec/jobs/remove_banner_spec.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe Jobs::RemoveBanner do + fab!(:topic) { Fabricate(:topic) } + fab!(:user) { topic.user } + + context 'topic is not bannered until' do + it 'doesn’t enqueue a future job to remove it' do + expect do + topic.make_banner!(user) + end.to change { Jobs::RemoveBanner.jobs.size }.by(0) + end + end + + context 'topic is bannered until' do + context 'bannered_until is a valid date' do + it 'enqueues a future job to remove it' do + bannered_until = 5.days.from_now + + expect(topic.archetype).to eq(Archetype.default) + + expect do + topic.make_banner!(user, bannered_until.to_s) + end.to change { Jobs::RemoveBanner.jobs.size }.by(1) + + topic.reload + expect(topic.archetype).to eq(Archetype.banner) + + job = Jobs::RemoveBanner.jobs[0] + expect(Time.at(job['at'])).to be_within_one_minute_of(bannered_until) + expect(job['args'][0]['topic_id']).to eq(topic.id) + + job['class'].constantize.new.perform(*job['args']) + topic.reload + expect(topic.archetype).to eq(Archetype.default) + end + end + + context 'bannered_until is an invalid date' do + it 'doesn’t enqueue a future job to remove it' do + expect do + expect do + topic.make_banner!(user, 'xxx') + end.to raise_error(Discourse::InvalidParameters) + end.to change { Jobs::RemoveBanner.jobs.size }.by(0) + end + end + end +end diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index b797e1cb07b..efd5a6026ca 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -1380,6 +1380,24 @@ describe Topic do end + context "bannered_until date" do + + it 'sets bannered_until to be caught by ensure_consistency' do + bannered_until = 5.days.from_now + topic.make_banner!(user, bannered_until.to_s) + + freeze_time 6.days.from_now do + expect(topic.archetype).to eq(Archetype.banner) + + Topic.ensure_consistency! + topic.reload + + expect(topic.archetype).to eq(Archetype.default) + end + end + + end + end context 'last_poster info' do