From 30611c343c389d3d7504fd94e56fbd5e94ced7c1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9gis=20Hanol?= <regis@hanol.fr>
Date: Mon, 16 Jun 2014 19:21:21 +0200
Subject: [PATCH] ensures only one banner topic at all time

---
 app/controllers/topics_controller.rb       | 12 ++-----
 app/models/topic.rb                        | 16 +++++++++
 spec/controllers/topics_controller_spec.rb | 24 +++++---------
 spec/fabricators/topic_fabricator.rb       |  4 +++
 spec/models/topic_spec.rb                  | 38 ++++++++++++++++++++++
 5 files changed, 68 insertions(+), 26 deletions(-)

diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb
index 16335f2e299..b3622f4ff33 100644
--- a/app/controllers/topics_controller.rb
+++ b/app/controllers/topics_controller.rb
@@ -177,12 +177,7 @@ class TopicsController < ApplicationController
     topic = Topic.find_by(id: params[:topic_id].to_i)
     guardian.ensure_can_moderate!(topic)
 
-    # TODO: only one banner at the same time
-
-    topic.archetype = Archetype.banner
-    topic.add_moderator_post(current_user, I18n.t("archetypes.banner.message.make"))
-
-    topic.save
+    topic.make_banner!(current_user)
 
     render nothing: true
   end
@@ -191,10 +186,7 @@ class TopicsController < ApplicationController
     topic = Topic.find_by(id: params[:topic_id].to_i)
     guardian.ensure_can_moderate!(topic)
 
-    topic.archetype = Archetype.default
-    topic.add_moderator_post(current_user, I18n.t("archetypes.banner.message.remove"))
-
-    topic.save
+    topic.remove_banner!(current_user)
 
     render nothing: true
   end
diff --git a/app/models/topic.rb b/app/models/topic.rb
index 2eb8e220297..0da1ad8a38a 100644
--- a/app/models/topic.rb
+++ b/app/models/topic.rb
@@ -626,6 +626,22 @@ class Topic < ActiveRecord::Base
     end
   end
 
+  def make_banner!(user)
+    # only one banner at the same time
+    previous_banner = Topic.where(archetype: Archetype.banner).first
+    previous_banner.remove_banner!(user) if previous_banner.present?
+
+    self.archetype = Archetype.banner
+    self.add_moderator_post(user, I18n.t("archetypes.banner.message.make"))
+    self.save
+  end
+
+  def remove_banner!(user)
+    self.archetype = Archetype.default
+    self.add_moderator_post(user, I18n.t("archetypes.banner.message.remove"))
+    self.save
+  end
+
   def self.starred_counts_per_day(sinceDaysAgo=30)
     TopicUser.starred_since(sinceDaysAgo).by_date_starred.count
   end
diff --git a/spec/controllers/topics_controller_spec.rb b/spec/controllers/topics_controller_spec.rb
index 69692a818e8..e69c6b5169e 100644
--- a/spec/controllers/topics_controller_spec.rb
+++ b/spec/controllers/topics_controller_spec.rb
@@ -857,15 +857,11 @@ describe TopicsController do
 
     describe 'when logged in' do
 
-      before do
-        @admin = log_in(:admin)
-        @topic = Fabricate(:topic, user: @admin)
-      end
-
       it "changes the topic archetype to 'banner'" do
-        Topic.any_instance.expects(:archetype=).with(Archetype.banner)
-        Topic.any_instance.expects(:add_moderator_post)
-        xhr :put, :make_banner, topic_id: @topic.id
+        topic = Fabricate(:topic, user: log_in(:admin))
+        Topic.any_instance.expects(:make_banner!)
+
+        xhr :put, :make_banner, topic_id: topic.id
         response.should be_success
       end
 
@@ -883,15 +879,11 @@ describe TopicsController do
 
     describe 'when logged in' do
 
-      before do
-        @admin = log_in(:admin)
-        @topic = Fabricate(:topic, user: @admin)
-      end
-
       it "resets the topic archetype" do
-        Topic.any_instance.expects(:archetype=).with(Archetype.default)
-        Topic.any_instance.expects(:add_moderator_post)
-        xhr :put, :remove_banner, topic_id: @topic.id
+        topic = Fabricate(:topic, user: log_in(:admin))
+        Topic.any_instance.expects(:remove_banner!)
+
+        xhr :put, :remove_banner, topic_id: topic.id
         response.should be_success
       end
 
diff --git a/spec/fabricators/topic_fabricator.rb b/spec/fabricators/topic_fabricator.rb
index c05fe83f0ce..3e2632cdae5 100644
--- a/spec/fabricators/topic_fabricator.rb
+++ b/spec/fabricators/topic_fabricator.rb
@@ -11,6 +11,10 @@ end
 Fabricator(:topic_allowed_user) do
 end
 
+Fabricator(:banner_topic, from: :topic) do
+  archetype Archetype.banner
+end
+
 Fabricator(:private_message_topic, from: :topic) do
   user
   title { sequence(:title) { |i| "This is a private message #{i}" } }
diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb
index 5cec3aedbb5..aea4dad51ca 100644
--- a/spec/models/topic_spec.rb
+++ b/spec/models/topic_spec.rb
@@ -638,6 +638,44 @@ describe Topic do
     end
   end
 
+  describe "make_banner!" do
+
+    before do
+      @topic = Fabricate(:topic)
+      @user = @topic.user
+    end
+
+    it "changes the topic archetype to 'banner'" do
+      @topic.expects(:add_moderator_post)
+      @topic.make_banner!(@user)
+      @topic.archetype.should == Archetype.banner
+    end
+
+    it "ensures only one banner topic at all time" do
+      banner_topic = Fabricate(:banner_topic)
+      Topic.where(archetype: Archetype.banner).count.should == 1
+
+      @topic.make_banner!(@user)
+      Topic.where(archetype: Archetype.banner).count.should == 1
+    end
+
+  end
+
+  describe "remove_banner!" do
+
+    before do
+      @topic = Fabricate(:topic)
+      @user = @topic.user
+    end
+
+    it "resets the topic archetype" do
+      @topic.expects(:add_moderator_post)
+      @topic.remove_banner!(@user)
+      @topic.archetype.should == Archetype.default
+    end
+
+  end
+
   context 'last_poster info' do
 
     before do