diff --git a/app/models/category.rb b/app/models/category.rb index 9450e44e962..3721e2bc9f3 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -199,7 +199,7 @@ SQL t = Topic.new(title: I18n.t("category.topic_prefix", category: name), user: user, pinned_at: Time.now, category_id: id) t.skip_callbacks = true t.ignore_category_auto_close = true - t.set_or_create_timer(TopicTimer.types[:close], nil) + t.delete_topic_timer(TopicTimer.types[:close]) t.save!(validate: false) update_column(:topic_id, t.id) t.posts.create(raw: post_template, user: user) diff --git a/app/models/topic.rb b/app/models/topic.rb index 2dd51661d25..80c8b492f71 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -994,6 +994,12 @@ SQL @public_topic_timer ||= topic_timers.find_by(deleted_at: nil, public_type: true) end + def delete_topic_timer(status_type, by_user: Discourse.system_user) + options = { status_type: status_type } + options.merge!(user: by_user) unless TopicTimer.public_types[status_type] + self.topic_timers.find_by(options)&.trash!(by_user) + end + # Valid arguments for the time: # * An integer, which is the number of hours from now to update the topic's status. # * A timestamp, like "2013-11-25 13:00", when the topic's status should update. @@ -1005,16 +1011,13 @@ SQL # * based_on_last_post: True if time should be based on timestamp of the last post. # * category_id: Category that the update will apply to. def set_or_create_timer(status_type, time, by_user: nil, timezone_offset: 0, based_on_last_post: false, category_id: SiteSetting.uncategorized_category_id) + return delete_topic_timer(status_type, by_user: by_user) if time.blank? + topic_timer_options = { topic: self } topic_timer_options.merge!(user: by_user) unless TopicTimer.public_types[status_type] topic_timer = TopicTimer.find_or_initialize_by(topic_timer_options) topic_timer.status_type = status_type - if time.blank? - topic_timer.trash!(by_user || Discourse.system_user) - return - end - time_now = Time.zone.now topic_timer.based_on_last_post = !based_on_last_post.blank? diff --git a/app/services/topic_status_updater.rb b/app/services/topic_status_updater.rb index 80b5781ea73..ed088d5a8ea 100644 --- a/app/services/topic_status_updater.rb +++ b/app/services/topic_status_updater.rb @@ -38,9 +38,9 @@ TopicStatusUpdater = Struct.new(:topic, :user) do if @topic_status_update if status.manually_closing_topic? || status.closing_topic? - topic.set_or_create_timer(TopicTimer.types[:close], nil) + topic.delete_topic_timer(TopicTimer.types[:close]) elsif status.manually_opening_topic? || status.opening_topic? - topic.set_or_create_timer(TopicTimer.types[:open], nil) + topic.delete_topic_timer(TopicTimer.types[:open]) end end diff --git a/spec/controllers/topics_controller_spec.rb b/spec/controllers/topics_controller_spec.rb index 11a28ed94ea..b34445c4168 100644 --- a/spec/controllers/topics_controller_spec.rb +++ b/spec/controllers/topics_controller_spec.rb @@ -420,6 +420,7 @@ describe TopicsController do expect(response).to be_success expect(@topic.reload.closed).to eq(false) + expect(@topic.topic_timers).to eq([]) body = JSON.parse(response.body) diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index 6dbcbdd340a..0b6bc4949d5 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -1177,8 +1177,6 @@ describe Topic do let(:admin) { Fabricate(:admin) } let(:trust_level_4) { Fabricate(:trust_level_4) } - before { Discourse.stubs(:system_user).returns(admin) } - it 'can take a number of hours as an integer' do freeze_time now @@ -1248,12 +1246,12 @@ describe Topic do it 'sets topic status update user to system user if given user is not staff or a TL4 user' do topic.set_or_create_timer(TopicTimer.types[:close], 3, by_user: Fabricate.build(:user, id: 444)) - expect(topic.topic_timers.first.user).to eq(admin) + expect(topic.topic_timers.first.user).to eq(Discourse.system_user) end it 'sets topic status update user to system user if user is not given and topic creator is not staff nor TL4 user' do topic.set_or_create_timer(TopicTimer.types[:close], 3) - expect(topic.topic_timers.first.user).to eq(admin) + expect(topic.topic_timers.first.user).to eq(Discourse.system_user) end it 'sets topic status update user to topic creator if it is a staff user' do @@ -1280,6 +1278,15 @@ describe Topic do expect(closing_topic.reload.public_topic_timer.execute_at).to eq(2.days.from_now) end + it 'should not delete topic_timer of another status_type' do + freeze_time + closing_topic.set_or_create_timer(TopicTimer.types[:open], nil) + topic_timer = closing_topic.public_topic_timer + + expect(topic_timer.execute_at).to eq(5.hours.from_now) + expect(topic_timer.status_type).to eq(TopicTimer.types[:close]) + end + it 'should allow status_type to be updated' do freeze_time