FIX: Auto close topic from category settings based on topic created_at (#12082)

Previously when inheriting category auto-close settings for a topic, those settings were disrupted if another topic timer was assigned or if a topic was closed then manually re-opened.

This PR makes it so that when a topic is manually re-opened the topic auto-close settings are inherited from the category. However, they will now be based on the topic created_at date. As an example, for a topic with a category auto close hours setting of 72 (3 days):

* Topic was created on 2021-02-15 08:00
* Topic was closed on 2021-02-16 10:00
* Topic was opened again on 2021-02-17 06:00

Now, the topic will inherit the auto close timer again and will close automatically at **2021-02-18 08:00**, which is based on the creation date. If the current date and time is greater than the original auto-close time (e.g. we were at 2021-02-20 13:45) then no auto-close timer is created.

Note, this will not happen if the topic category auto-close setting is "based on last post".
This commit is contained in:
Martin Brennan 2021-02-17 07:51:39 +10:00 committed by GitHub
parent f17e745fe3
commit fb83757edb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 132 additions and 27 deletions

View File

@ -370,26 +370,6 @@ class Topic < ActiveRecord::Base
self.last_post_user_id ||= user_id self.last_post_user_id ||= user_id
end end
def inherit_auto_close_from_category(timer_type: :close)
if !self.closed &&
!@ignore_category_auto_close &&
self.category &&
self.category.auto_close_hours &&
!public_topic_timer&.execute_at
based_on_last_post = self.category.auto_close_based_on_last_post
duration_minutes = based_on_last_post ? self.category.auto_close_hours * 60 : nil
self.set_or_create_timer(
TopicTimer.types[timer_type],
self.category.auto_close_hours,
by_user: Discourse.system_user,
based_on_last_post: based_on_last_post,
duration_minutes: duration_minutes
)
end
end
def advance_draft_sequence def advance_draft_sequence
if self.private_message? if self.private_message?
DraftSequence.next!(user, Draft::NEW_PRIVATE_MESSAGE) DraftSequence.next!(user, Draft::NEW_PRIVATE_MESSAGE)
@ -1287,6 +1267,45 @@ class Topic < ActiveRecord::Base
Topic.where("pinned_until < now()").update_all(pinned_at: nil, pinned_globally: false, pinned_until: nil) Topic.where("pinned_until < now()").update_all(pinned_at: nil, pinned_globally: false, pinned_until: nil)
end end
def inherit_auto_close_from_category(timer_type: :close)
auto_close_hours = self.category&.auto_close_hours
if self.open? &&
!@ignore_category_auto_close &&
auto_close_hours.present? &&
public_topic_timer&.execute_at.blank?
based_on_last_post = self.category.auto_close_based_on_last_post
duration_minutes = based_on_last_post ? auto_close_hours * 60 : nil
# the timer time can be a timestamp or an integer based
# on the number of hours
auto_close_time = auto_close_hours
if !based_on_last_post
# set auto close to the original time it should have been
# when the topic was first created.
start_time = self.created_at || Time.zone.now
auto_close_time = start_time + auto_close_hours.hours
# if we have already passed the original close time then
# we should not recreate the auto-close timer for the topic
return if auto_close_time < Time.zone.now
# timestamp must be a string for set_or_create_timer
auto_close_time = auto_close_time.to_s
end
self.set_or_create_timer(
TopicTimer.types[timer_type],
auto_close_time,
by_user: Discourse.system_user,
based_on_last_post: based_on_last_post,
duration_minutes: duration_minutes
)
end
end
def public_topic_timer def public_topic_timer
@public_topic_timer ||= topic_timers.find_by(deleted_at: nil, public_type: true) @public_topic_timer ||= topic_timers.find_by(deleted_at: nil, public_type: true)
end end
@ -1295,6 +1314,7 @@ class Topic < ActiveRecord::Base
options = { status_type: status_type } options = { status_type: status_type }
options.merge!(user: by_user) unless TopicTimer.public_types[status_type] options.merge!(user: by_user) unless TopicTimer.public_types[status_type]
self.topic_timers.find_by(options)&.trash!(by_user) self.topic_timers.find_by(options)&.trash!(by_user)
@public_topic_timer = nil
nil nil
end end

View File

@ -56,6 +56,7 @@ TopicStatusUpdater = Struct.new(:topic, :user) do
topic.delete_topic_timer(TopicTimer.types[:silent_close]) topic.delete_topic_timer(TopicTimer.types[:silent_close])
elsif status.manually_opening_topic? || status.opening_topic? elsif status.manually_opening_topic? || status.opening_topic?
topic.delete_topic_timer(TopicTimer.types[:open]) topic.delete_topic_timer(TopicTimer.types[:open])
topic.inherit_auto_close_from_category
end end
end end

View File

@ -41,7 +41,7 @@ describe Topic do
topic_status_update = TopicTimer.last topic_status_update = TopicTimer.last
expect(topic_status_update.topic).to eq(topic) expect(topic_status_update.topic).to eq(topic)
expect(topic.public_topic_timer.execute_at).to eq_time(2.hours.from_now) expect(topic.public_topic_timer.execute_at).to be_within_one_second_of(2.hours.from_now)
end end
context 'topic was created by staff user' do context 'topic was created by staff user' do
@ -54,7 +54,7 @@ describe Topic do
topic_status_update = TopicTimer.last topic_status_update = TopicTimer.last
expect(topic_status_update.topic).to eq(staff_topic) expect(topic_status_update.topic).to eq(staff_topic)
expect(topic_status_update.execute_at).to eq_time(2.hours.from_now) expect(topic_status_update.execute_at).to be_within_one_second_of(2.hours.from_now)
expect(topic_status_update.user).to eq(Discourse.system_user) expect(topic_status_update.user).to eq(Discourse.system_user)
end end
@ -65,7 +65,7 @@ describe Topic do
staff_topic.update_status('closed', true, admin) staff_topic.update_status('closed', true, admin)
expect(TopicTimer.with_deleted.find(topic_timer_id).deleted_at) expect(TopicTimer.with_deleted.find(topic_timer_id).deleted_at)
.to eq_time(Time.zone.now) .to be_within_one_second_of(Time.zone.now)
end end
end end
end end
@ -80,7 +80,7 @@ describe Topic do
topic_status_update = TopicTimer.last topic_status_update = TopicTimer.last
expect(topic_status_update.topic).to eq(regular_user_topic) expect(topic_status_update.topic).to eq(regular_user_topic)
expect(topic_status_update.execute_at).to eq_time(2.hours.from_now) expect(topic_status_update.execute_at).to be_within_one_second_of(2.hours.from_now)
expect(topic_status_update.user).to eq(Discourse.system_user) expect(topic_status_update.user).to eq(Discourse.system_user)
end end
end end

View File

@ -133,7 +133,7 @@ RSpec.describe Jobs::PublishTopicToCategory do
topic_timer = topic.public_topic_timer topic_timer = topic.public_topic_timer
expect(topic.category).to eq(another_category) expect(topic.category).to eq(another_category)
expect(topic_timer.status_type).to eq(TopicTimer.types[:close]) expect(topic_timer.status_type).to eq(TopicTimer.types[:close])
expect(topic_timer.execute_at).to eq_time(5.hours.from_now) expect(topic_timer.execute_at).to be_within_one_second_of(5.hours.from_now)
end end
end end
end end

View File

@ -1628,7 +1628,7 @@ describe Topic do
expect(topic_timer.user).to eq(Discourse.system_user) expect(topic_timer.user).to eq(Discourse.system_user)
expect(topic_timer.topic).to eq(topic) expect(topic_timer.topic).to eq(topic)
expect(topic_timer.execute_at).to eq_time(5.hours.from_now) expect(topic_timer.execute_at).to be_within_one_second_of(5.hours.from_now)
end end
describe 'when topic is already closed' do describe 'when topic is already closed' do
@ -1885,7 +1885,7 @@ describe Topic do
freeze_time freeze_time
Jobs.run_immediately! Jobs.run_immediately!
expect(topic.topic_timers.first.execute_at).to eq_time(topic.created_at + 4.hours) expect(topic.topic_timers.first.execute_at).to be_within_one_second_of(topic.created_at + 4.hours)
topic.set_or_create_timer(TopicTimer.types[:close], 2, by_user: admin) topic.set_or_create_timer(TopicTimer.types[:close], 2, by_user: admin)

View File

@ -70,6 +70,90 @@ describe TopicStatusUpdater do
expect(last_post.raw).to eq(I18n.t("topic_statuses.autoclosed_enabled_lastpost_hours", count: 10)) expect(last_post.raw).to eq(I18n.t("topic_statuses.autoclosed_enabled_lastpost_hours", count: 10))
end end
describe "opening the topic" do
it "opens the topic and deletes the timer" do
topic = create_topic
topic.set_or_create_timer(
TopicTimer.types[:open], 10.hours.from_now
)
TopicStatusUpdater.new(topic, admin).update!("closed", false)
timer = TopicTimer.find_by(topic: topic)
expect(timer).to eq(nil)
end
context "when the category has auto close settings" do
let(:topic) { create_topic }
let(:based_on_last_post) { false }
before do
# auto close after 3 days, topic was created a day ago
topic.update(
category: Fabricate(:category, auto_close_hours: 72, auto_close_based_on_last_post: based_on_last_post),
created_at: 1.day.ago
)
end
it "inherits auto close from the topic category, based on the created_at date of the topic" do
# close the topic manually, and set a timer to automatically open
TopicStatusUpdater.new(topic, admin).update!("closed", true)
topic.set_or_create_timer(
TopicTimer.types[:open], 10.hours.from_now
)
# manually open the topic. it has been 1 days since creation so the
# topic should auto-close 2 days from now, the original auto close time
TopicStatusUpdater.new(topic, admin).update!("closed", false)
timer = TopicTimer.find_by(topic: topic)
expect(timer).not_to eq(nil)
expect(timer.execute_at).to be_within_one_second_of(topic.created_at + 72.hours)
end
it "does not inherit auto close from the topic category if it has already been X hours since topic creation" do
topic.category.update(auto_close_hours: 1)
# close the topic manually, and set a timer to automatically open
TopicStatusUpdater.new(topic, admin).update!("closed", true)
topic.set_or_create_timer(
TopicTimer.types[:open], 10.hours.from_now
)
# manually open the topic. it has been over a day since creation and
# the auto close hours was 1 so a new timer should not be made
TopicStatusUpdater.new(topic, admin).update!("closed", false)
timer = TopicTimer.find_by(topic: topic)
expect(timer).to eq(nil)
end
context "when category setting is based_on_last_post" do
let(:based_on_last_post) { true }
it "inherits auto close from the topic category, using the duration because the close is based_on_last_post" do
# close the topic manually, and set a timer to automatically open
TopicStatusUpdater.new(topic, admin).update!("closed", true)
topic.set_or_create_timer(
TopicTimer.types[:open], 10.hours.from_now
)
# manually open the topic. it should re open 3 days from now, NOT
# 3 days from creation
TopicStatusUpdater.new(topic, admin).update!("closed", false)
timer = TopicTimer.find_by(topic: topic)
expect(timer).not_to eq(nil)
expect(timer.duration_minutes).to eq(72 * 60)
expect(timer.execute_at).to be_within_one_second_of(Time.zone.now + 72.hours)
end
end
end
end
describe "repeat actions" do describe "repeat actions" do
shared_examples "an action that doesn't repeat" do shared_examples "an action that doesn't repeat" do