FIX: Inherit topic auto close when changing topic's category.

app/models/topic.rb

@@ -170,7 +170,6 @@ class Topic < ActiveRecord::Base
 
   before_create do
     initialize_default_values
-    inherit_auto_close_from_category
   end
 
   after_create do
@@ -187,6 +186,10 @@ class Topic < ActiveRecord::Base
     if title_changed?
       write_attribute :fancy_title, Topic.fancy_title(title)
     end
+
+    if category_id_changed? || new_record?
+      inherit_auto_close_from_category
+    end
   end
 
   after_save do
@@ -614,8 +617,7 @@ SQL
       old_category = category
 
       if self.category_id != new_category.id
-        self.category_id = new_category.id
-        self.update_column(:category_id, new_category.id)
+        self.update!(category_id: new_category.id)
         Category.where(id: old_category.id).update_all("topic_count = topic_count - 1") if old_category
 
         # when a topic changes category we may have to start watching it

spec/models/topic_spec.rb

@@ -966,74 +966,92 @@ describe Topic do
     end
   end
 
-  describe 'change_category' do
-
-    before do
-      @topic = Fabricate(:topic)
-      @category = Fabricate(:category, user: @topic.user)
-      @user = @topic.user
-    end
+  describe '#change_category_to_id' do
+    let(:topic) { Fabricate(:topic) }
+    let(:user) { topic.user }
+    let(:category) { Fabricate(:category, user: user) }
 
     describe 'without a previous category' do
+      it 'changes the category' do
+        topic.change_category_to_id(category.id)
+        category.reload
+        expect(topic.category).to eq(category)
+        expect(category.topic_count).to eq(1)
+      end
 
       it 'should not change the topic_count when not changed' do
-       expect { @topic.change_category_to_id(@topic.category.id); @category.reload }.not_to change(@category, :topic_count)
-      end
-
-      describe 'changed category' do
-        before do
-          @topic.change_category_to_id(@category.id)
-          @category.reload
-        end
-
-        it 'changes the category' do
-          expect(@topic.category).to eq(@category)
-          expect(@category.topic_count).to eq(1)
-        end
-
+       expect { topic.change_category_to_id(topic.category.id); category.reload }.not_to change(category, :topic_count)
       end
 
       it "doesn't change the category when it can't be found" do
-        @topic.change_category_to_id(12312312)
-        expect(@topic.category_id).to eq(SiteSetting.uncategorized_category_id)
+        topic.change_category_to_id(12312312)
+        expect(topic.category_id).to eq(SiteSetting.uncategorized_category_id)
       end
     end
 
     describe 'with a previous category' do
       before do
-        @topic.change_category_to_id(@category.id)
-        @topic.reload
-        @category.reload
+        topic.change_category_to_id(category.id)
+        topic.reload
+        category.reload
       end
 
       it 'increases the topic_count' do
-        expect(@category.topic_count).to eq(1)
+        expect(category.topic_count).to eq(1)
       end
 
       it "doesn't change the topic_count when the value doesn't change" do
-        expect { @topic.change_category_to_id(@category.id); @category.reload }.not_to change(@category, :topic_count)
+        expect { topic.change_category_to_id(category.id); category.reload }.not_to change(category, :topic_count)
       end
 
-      it "doesn't reset the category when given a name that doesn't exist" do
-        @topic.change_category_to_id(55556)
-        expect(@topic.category_id).to be_present
+      it "doesn't reset the category when an id that doesn't exist" do
+        topic.change_category_to_id(55556)
+        expect(topic.category_id).to eq(category.id)
       end
 
       describe 'to a different category' do
+        let(:new_category) { Fabricate(:category, user: user, name: '2nd category') }
+
+        it 'should work' do
+          topic.change_category_to_id(new_category.id)
+
+          expect(topic.reload.category).to eq(new_category)
+          expect(new_category.reload.topic_count).to eq(1)
+          expect(category.reload.topic_count).to eq(0)
+        end
+
+        describe 'when new category is set to auto close by default' do
           before do
-          @new_category = Fabricate(:category, user: @user, name: '2nd category')
-          @topic.change_category_to_id(@new_category.id)
-          @topic.reload
-          @new_category.reload
-          @category.reload
+            new_category.update!(auto_close_hours: 5)
           end
 
-        it "should increase the new category's topic count" do
-          expect(@new_category.topic_count).to eq(1)
+          it 'should set a topic timer' do
+            expect { topic.change_category_to_id(new_category.id) }
+              .to change { TopicTimer.count }.by(1)
+
+            expect(topic.reload.category).to eq(new_category)
+
+            topic_timer = TopicTimer.last
+
+            expect(topic_timer.topic).to eq(topic)
+            expect(topic_timer.execute_at).to be_within(1.second).of(Time.zone.now + 5.hours)
           end
 
-        it "should lower the original category's topic count" do
-          expect(@category.topic_count).to eq(0)
+          describe 'when topic has an existing topic timer' do
+            let(:topic_timer) { Fabricate(:topic_timer, topic: topic) }
+
+            it "should not inherit category's auto close hours" do
+              topic_timer
+              topic.change_category_to_id(new_category.id)
+
+              expect(topic.reload.category).to eq(new_category)
+
+              expect(topic.public_topic_timer).to eq(topic_timer)
+
+              expect(topic.public_topic_timer.execute_at)
+                .to be_within(1.second).of(topic_timer.execute_at)
+            end
+          end
         end
       end
 
@@ -1051,13 +1069,13 @@ describe Topic do
 
       describe 'when the category exists' do
         before do
-          @topic.change_category_to_id(nil)
-          @category.reload
+          topic.change_category_to_id(nil)
+          category.reload
         end
 
         it "resets the category" do
-          expect(@topic.category_id).to eq(SiteSetting.uncategorized_category_id)
-          expect(@category.topic_count).to eq(0)
+          expect(topic.category_id).to eq(SiteSetting.uncategorized_category_id)
+          expect(category.topic_count).to eq(0)
         end
       end