DEV: adds support for bannered until (#13417)

DEV: adds support for bannered until (#13417)

ATM it only implements server side of it, as my need is for automation purposes. However it should probably be added in the UI too as it’s unexpected to have pinned_until and no bannered_until.

diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb
index 7f5e36a..7089f89 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 0000000..880c869
--- /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 b99e505..9d4f6b9 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 32c004e..00a3eac 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 0000000..6a48cf4
--- /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 0000000..c91cdb4
--- /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 0000000..13c60b8
--- /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 b797e1c..efd5a60 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

GitHub sha: 2654a6685cea512ef8e3f8aea29079dc617775fd

This commit appears in #13417 which was approved by ZogStriP, tgxworld, and martin. It was merged by jjaffeux.