diff --git a/app/services/topic_status_updater.rb b/app/services/topic_status_updater.rb index 5bd72a7b4bb..795751a0d79 100644 --- a/app/services/topic_status_updater.rb +++ b/app/services/topic_status_updater.rb @@ -4,23 +4,36 @@ TopicStatusUpdater = Struct.new(:topic, :user) do @topic_status_update = topic.topic_status_update + updated = nil Topic.transaction do - change(status, opts) - highest_post_number = topic.highest_post_number - create_moderator_post_for(status, opts[:message]) - update_read_state_for(status, highest_post_number) + updated = change(status, opts) + if updated + highest_post_number = topic.highest_post_number + create_moderator_post_for(status, opts[:message]) + update_read_state_for(status, highest_post_number) + end end + + updated end private def change(status, opts={}) + result = true + if status.pinned? || status.pinned_globally? topic.update_pinned(status.enabled?, status.pinned_globally?, opts[:until]) elsif status.autoclosed? - topic.update_column('closed', status.enabled?) + rc = Topic.where(id: topic.id, closed: !status.enabled?).update_all(closed: status.enabled?) + topic.closed = status.enabled? + result = false if rc == 0 else - topic.update_column(status.name, status.enabled?) + rc = Topic.where(:id => topic.id, status.name => !status.enabled) + .update_all(status.name => status.enabled?) + + topic.send("#{status.name}=", status.enabled?) + result = false if rc == 0 end if @topic_status_update @@ -38,6 +51,8 @@ TopicStatusUpdater = Struct.new(:topic, :user) do (status.disabled? && status.visible?)) CategoryFeaturedTopic.where(topic_id: topic.id).delete_all end + + result end def create_moderator_post_for(status, message=nil) diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index a25f4db326b..5feef5180db 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -689,14 +689,16 @@ describe Topic do context 'archived' do context 'disable' do before do - @topic.update_status('archived', false, @user) - @topic.reload + @archived_topic = Fabricate(:topic, archived: true, bumped_at: 1.hour.ago) + @original_bumped_at = @archived_topic.bumped_at.to_f + @archived_topic.update_status('archived', false, @user) + @archived_topic.reload end it 'should archive correctly' do - expect(@topic).not_to be_archived - expect(@topic.bumped_at.to_f).to eq(@original_bumped_at) - expect(@topic.moderator_posts_count).to eq(1) + expect(@archived_topic).not_to be_archived + expect(@archived_topic.bumped_at.to_f).to eq(@original_bumped_at) + expect(@archived_topic.moderator_posts_count).to eq(1) end end @@ -719,14 +721,16 @@ describe Topic do shared_examples_for 'a status that closes a topic' do context 'disable' do before do - @topic.update_status(status, false, @user) - @topic.reload + @closed_topic = Fabricate(:topic, closed: true, bumped_at: 1.hour.ago) + @original_bumped_at = @closed_topic.bumped_at.to_f + @closed_topic.update_status(status, false, @user) + @closed_topic.reload end it 'should not be pinned' do - expect(@topic).not_to be_closed - expect(@topic.moderator_posts_count).to eq(1) - expect(@topic.bumped_at.to_f).not_to eq(@original_bumped_at) + expect(@closed_topic).not_to be_closed + expect(@closed_topic.moderator_posts_count).to eq(1) + expect(@closed_topic.bumped_at.to_f).not_to eq(@original_bumped_at) end end diff --git a/spec/services/topic_status_updater_spec.rb b/spec/services/topic_status_updater_spec.rb index 3609082ec64..17f3be231d6 100644 --- a/spec/services/topic_status_updater_spec.rb +++ b/spec/services/topic_status_updater_spec.rb @@ -56,4 +56,60 @@ describe TopicStatusUpdater do expect(last_post.raw).to eq(I18n.t("topic_statuses.autoclosed_enabled_lastpost_hours", count: 10)) end + describe "repeat actions" do + + shared_examples "an action that doesn't repeat" do + it "does not perform the update twice" do + topic = Fabricate(:topic, status_name => false) + updated = TopicStatusUpdater.new(topic, admin).update!(status_name, true) + expect(updated).to eq(true) + expect(topic.send("#{status_name}?")).to eq(true) + + updated = TopicStatusUpdater.new(topic, admin).update!(status_name, true) + expect(updated).to eq(false) + expect(topic.posts.where(post_type: Post.types[:small_action]).count).to eq(1) + + updated = TopicStatusUpdater.new(topic, admin).update!(status_name, false) + expect(updated).to eq(true) + expect(topic.send("#{status_name}?")).to eq(false) + + updated = TopicStatusUpdater.new(topic, admin).update!(status_name, false) + expect(updated).to eq(false) + expect(topic.posts.where(post_type: Post.types[:small_action]).count).to eq(2) + end + + end + + it_behaves_like "an action that doesn't repeat" do + let(:status_name) { "closed" } + end + + it_behaves_like "an action that doesn't repeat" do + let(:status_name) { "visible" } + end + + it_behaves_like "an action that doesn't repeat" do + let(:status_name) { "archived" } + end + + it "updates autoclosed" do + topic = Fabricate(:topic) + updated = TopicStatusUpdater.new(topic, admin).update!('autoclosed', true) + expect(updated).to eq(true) + expect(topic.closed?).to eq(true) + + updated = TopicStatusUpdater.new(topic, admin).update!('autoclosed', true) + expect(updated).to eq(false) + expect(topic.posts.where(post_type: Post.types[:small_action]).count).to eq(1) + + updated = TopicStatusUpdater.new(topic, admin).update!('autoclosed', false) + expect(updated).to eq(true) + expect(topic.closed?).to eq(false) + + updated = TopicStatusUpdater.new(topic, admin).update!('autoclosed', false) + expect(updated).to eq(false) + expect(topic.posts.where(post_type: Post.types[:small_action]).count).to eq(2) + end + + end end