From 08c36fa96839da7ff4d4eb2396bce70db0a452e2 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Wed, 17 May 2017 09:37:11 +0800 Subject: [PATCH] REFACTOR: Clean up some code associated with topic timers. --- app/models/topic.rb | 19 ++++++------------- spec/jobs/topic_reminder_spec.rb | 4 ++-- spec/models/topic_spec.rb | 28 ---------------------------- spec/models/topic_timer_spec.rb | 13 ++++++++----- 4 files changed, 16 insertions(+), 48 deletions(-) diff --git a/app/models/topic.rb b/app/models/topic.rb index 703392c9fc5..7f5b34b3988 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -397,7 +397,7 @@ class Topic < ActiveRecord::Base def reload(options=nil) @post_numbers = nil - @topic_timer = nil + @public_topic_timer = nil super(options) end @@ -957,7 +957,7 @@ SQL end def public_topic_timer - topic_timers.where(deleted_at: nil, public_type: true).first + @public_topic_timer ||= topic_timers.find_by(deleted_at: nil, public_type: true) end # Valid arguments for the time: @@ -973,11 +973,9 @@ 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) - topic_timer = if TopicTimer.public_types[status_type] - TopicTimer.find_or_initialize_by( status_type: status_type, topic: self ) - else - TopicTimer.find_or_initialize_by( status_type: status_type, topic: self, user: by_user ) - end + topic_timer_options = { status_type: status_type, 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) if time.blank? topic_timer.trash!(trashed_by: by_user || Discourse.system_user) @@ -1004,12 +1002,7 @@ SQL is_timestamp = time.is_a?(String) now = utc.now - if is_timestamp && m = /^(\d{1,2}):(\d{2})(?:\s*[AP]M)?$/i.match(time.strip) - # a time of day in client's time zone, like "15:00" - topic_timer.execute_at = utc.local(now.year, now.month, now.day, m[1].to_i, m[2].to_i) - topic_timer.execute_at += timezone_offset * 60 if timezone_offset - topic_timer.execute_at += 1.day if topic_timer.execute_at < now - elsif is_timestamp && time.include?("-") && timestamp = utc.parse(time) + if is_timestamp && time.include?("-") && timestamp = utc.parse(time) # a timestamp in client's time zone, like "2015-5-27 12:00" topic_timer.execute_at = timestamp topic_timer.execute_at += timezone_offset * 60 if timezone_offset diff --git a/spec/jobs/topic_reminder_spec.rb b/spec/jobs/topic_reminder_spec.rb index da84b680fad..fd8b71697e6 100644 --- a/spec/jobs/topic_reminder_spec.rb +++ b/spec/jobs/topic_reminder_spec.rb @@ -50,5 +50,5 @@ describe Jobs::TopicReminder do }.to_not change { Notification.count } end end - -end \ No newline at end of file + +end diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index 15bea596814..d8188c1544a 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -1154,34 +1154,6 @@ describe Topic do end end - it "can take a time later in the day" do - Timecop.freeze(now) do - topic.set_or_create_timer(TopicTimer.types[:close], '13:00', {by_user: admin}) - expect(topic.topic_timers.first.execute_at).to eq(Time.zone.local(2013,11,20,13,0)) - end - end - - it "can take a time later in the day, with timezone offset" do - Timecop.freeze(now) do - topic.set_or_create_timer(TopicTimer.types[:close], '13:00', {by_user: admin, timezone_offset: 240}) - expect(topic.topic_timers.first.execute_at).to eq(Time.zone.local(2013,11,20,17,0)) - end - end - - it "can take a time for the next day" do - Timecop.freeze(now) do - topic.set_or_create_timer(TopicTimer.types[:close], '5:00', {by_user: admin}) - expect(topic.topic_timers.first.execute_at).to eq(Time.zone.local(2013,11,21,5,0)) - end - end - - it "can take a time for the next day, with timezone offset" do - Timecop.freeze(now) do - topic.set_or_create_timer(TopicTimer.types[:close], '1:00', {by_user: admin, timezone_offset: 240}) - expect(topic.topic_timers.first.execute_at).to eq(Time.zone.local(2013,11,21,5,0)) - end - end - it "can take a timestamp for a future time" do Timecop.freeze(now) do topic.set_or_create_timer(TopicTimer.types[:close], '2013-11-22 5:00', {by_user: admin}) diff --git a/spec/models/topic_timer_spec.rb b/spec/models/topic_timer_spec.rb index 4d32df14b6f..7ffd78d2d63 100644 --- a/spec/models/topic_timer_spec.rb +++ b/spec/models/topic_timer_spec.rb @@ -27,10 +27,13 @@ RSpec.describe TopicTimer, type: :model do end it 'should allow users to have their own private topic timer' do - Fabricate(:topic_timer, topic: topic, user: admin, status_type: TopicTimer.types[:reminder]) - expect { - Fabricate(:topic_timer, topic: topic, user: Fabricate(:admin), status_type: TopicTimer.types[:reminder]) - }.to_not raise_error + expect do + Fabricate(:topic_timer, + topic: topic, + user: Fabricate(:admin), + status_type: TopicTimer.types[:reminder] + ) + end.to_not raise_error end end @@ -220,7 +223,7 @@ RSpec.describe TopicTimer, type: :model do end end - describe 'public_type' do + describe '#public_type' do [:close, :open, :delete].each do |public_type| it "is true for #{public_type}" do timer = Fabricate(:topic_timer, status_type: described_class.types[public_type])