diff --git a/app/models/topic.rb b/app/models/topic.rb index 7f5b34b3988..8533894aa0f 100644 --- a/app/models/topic.rb +++ b/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 diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index d8188c1544a..1cee451bafe 100644 --- a/spec/models/topic_spec.rb +++ b/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 '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) + 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 - 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 - + 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 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 - 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 + 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 - it "should increase the new category's topic count" do - expect(@new_category.topic_count).to eq(1) - end + describe 'when new category is set to auto close by default' do + before do + new_category.update!(auto_close_hours: 5) + end - it "should lower the original category's topic count" do - expect(@category.topic_count).to eq(0) + 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 + + 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