FIX: topic auto-close uses the client's time zone

This commit is contained in:
Neil Lalonde 2015-05-27 12:22:34 -04:00
parent 23eadc3fb1
commit ea8cf1a208
6 changed files with 67 additions and 21 deletions

View file

@ -36,6 +36,7 @@ export default ObjectController.extend(ModalFunctionality, {
data: { data: {
auto_close_time: time, auto_close_time: time,
auto_close_based_on_last_post: this.get("model.details.auto_close_based_on_last_post"), auto_close_based_on_last_post: this.get("model.details.auto_close_based_on_last_post"),
timezone_offset: (new Date().getTimezoneOffset())
} }
}).then(function(result){ }).then(function(result){
if (result.success) { if (result.success) {

View file

@ -195,14 +195,14 @@ class TopicsController < ApplicationController
end end
def autoclose def autoclose
params.permit(:auto_close_time) params.permit(:auto_close_time, :timezone_offset)
params.require(:auto_close_based_on_last_post) params.require(:auto_close_based_on_last_post)
topic = Topic.find_by(id: params[:topic_id].to_i) topic = Topic.find_by(id: params[:topic_id].to_i)
guardian.ensure_can_moderate!(topic) guardian.ensure_can_moderate!(topic)
topic.auto_close_based_on_last_post = params[:auto_close_based_on_last_post] topic.auto_close_based_on_last_post = params[:auto_close_based_on_last_post]
topic.set_auto_close(params[:auto_close_time], current_user) topic.set_auto_close(params[:auto_close_time], {by_user: current_user, timezone_offset: params[:timezone_offset] ? params[:timezone_offset].to_i : nil})
if topic.save if topic.save
render json: success_json.merge!({ render json: success_json.merge!({

View file

@ -760,8 +760,13 @@ class Topic < ActiveRecord::Base
# * A timestamp, like "2013-11-25 13:00", when the topic should close. # * A timestamp, like "2013-11-25 13:00", when the topic should close.
# * A timestamp with timezone in JSON format. (e.g., "2013-11-26T21:00:00.000Z") # * A timestamp with timezone in JSON format. (e.g., "2013-11-26T21:00:00.000Z")
# * nil, to prevent the topic from automatically closing. # * nil, to prevent the topic from automatically closing.
def set_auto_close(arg, by_user=nil) # Options:
# * by_user: User who is setting the auto close time
# * timezone_offset: (Integer) offset from UTC in minutes of the given argument. Default 0.
def set_auto_close(arg, opts={})
self.auto_close_hours = nil self.auto_close_hours = nil
by_user = opts[:by_user]
offset_minutes = opts[:timezone_offset]
if self.auto_close_based_on_last_post if self.auto_close_based_on_last_post
num_hours = arg.to_f num_hours = arg.to_f
@ -773,12 +778,17 @@ class Topic < ActiveRecord::Base
self.auto_close_at = nil self.auto_close_at = nil
end end
else else
utc = Time.find_zone("UTC")
if arg.is_a?(String) && m = /^(\d{1,2}):(\d{2})(?:\s*[AP]M)?$/i.match(arg.strip) if arg.is_a?(String) && m = /^(\d{1,2}):(\d{2})(?:\s*[AP]M)?$/i.match(arg.strip)
now = Time.zone.now # a time of day in client's time zone, like "15:00"
self.auto_close_at = Time.zone.local(now.year, now.month, now.day, m[1].to_i, m[2].to_i) now = utc.now
self.auto_close_at = utc.local(now.year, now.month, now.day, m[1].to_i, m[2].to_i)
self.auto_close_at += offset_minutes * 60 if offset_minutes
self.auto_close_at += 1.day if self.auto_close_at < now self.auto_close_at += 1.day if self.auto_close_at < now
elsif arg.is_a?(String) && arg.include?("-") && timestamp = Time.zone.parse(arg) elsif arg.is_a?(String) && arg.include?("-") && timestamp = utc.parse(arg)
# a timestamp in client's time zone, like "2015-5-27 12:00"
self.auto_close_at = timestamp self.auto_close_at = timestamp
self.auto_close_at += offset_minutes * 60 if offset_minutes
self.errors.add(:auto_close_at, :invalid) if timestamp < Time.zone.now self.errors.add(:auto_close_at, :invalid) if timestamp < Time.zone.now
else else
num_hours = arg.to_f num_hours = arg.to_f

View file

@ -107,7 +107,7 @@ class TopicCreator
def setup_auto_close_time(topic) def setup_auto_close_time(topic)
return unless @opts[:auto_close_time].present? return unless @opts[:auto_close_time].present?
return unless @guardian.can_moderate?(topic) return unless @guardian.can_moderate?(topic)
topic.set_auto_close(@opts[:auto_close_time], @user) topic.set_auto_close(@opts[:auto_close_time], {by_user: @user})
end end
def process_private_message(topic) def process_private_message(topic)

View file

@ -871,8 +871,8 @@ describe TopicsController do
end end
it "can set a topic's auto close time and 'based on last post' property" do it "can set a topic's auto close time and 'based on last post' property" do
Topic.any_instance.expects(:set_auto_close).with("24", @admin) Topic.any_instance.expects(:set_auto_close).with("24", {by_user: @admin, timezone_offset: -240})
xhr :put, :autoclose, topic_id: @topic.id, auto_close_time: '24', auto_close_based_on_last_post: true xhr :put, :autoclose, topic_id: @topic.id, auto_close_time: '24', auto_close_based_on_last_post: true, timezone_offset: -240
json = ::JSON.parse(response.body) json = ::JSON.parse(response.body)
expect(json).to have_key('auto_close_at') expect(json).to have_key('auto_close_at')
expect(json).to have_key('auto_close_hours') expect(json).to have_key('auto_close_hours')
@ -880,7 +880,7 @@ describe TopicsController do
it "can remove a topic's auto close time" do it "can remove a topic's auto close time" do
Topic.any_instance.expects(:set_auto_close).with(nil, anything) Topic.any_instance.expects(:set_auto_close).with(nil, anything)
xhr :put, :autoclose, topic_id: @topic.id, auto_close_time: nil, auto_close_based_on_last_post: false xhr :put, :autoclose, topic_id: @topic.id, auto_close_time: nil, auto_close_based_on_last_post: false, timezone_offset: -240
end end
end end

View file

@ -951,7 +951,7 @@ describe Topic do
job_args[:user_id] == topic_closer.id job_args[:user_id] == topic_closer.id
end end
topic = Fabricate(:topic, user: topic_creator) topic = Fabricate(:topic, user: topic_creator)
topic.set_auto_close(7, topic_closer).save topic.set_auto_close(7, {by_user: topic_closer}).save
end end
it "ignores the category's default auto-close" do it "ignores the category's default auto-close" do
@ -1079,42 +1079,77 @@ describe Topic do
it 'can take a number of hours as an integer' do it 'can take a number of hours as an integer' do
Timecop.freeze(now) do Timecop.freeze(now) do
topic.set_auto_close(72, admin) topic.set_auto_close(72, {by_user: admin})
expect(topic.auto_close_at).to eq(3.days.from_now)
end
end
it 'can take a number of hours as an integer, with timezone offset' do
Timecop.freeze(now) do
topic.set_auto_close(72, {by_user: admin, timezone_offset: 240})
expect(topic.auto_close_at).to eq(3.days.from_now) expect(topic.auto_close_at).to eq(3.days.from_now)
end end
end end
it 'can take a number of hours as a string' do it 'can take a number of hours as a string' do
Timecop.freeze(now) do Timecop.freeze(now) do
topic.set_auto_close('18', admin) topic.set_auto_close('18', {by_user: admin})
expect(topic.auto_close_at).to eq(18.hours.from_now)
end
end
it 'can take a number of hours as a string, with timezone offset' do
Timecop.freeze(now) do
topic.set_auto_close('18', {by_user: admin, timezone_offset: 240})
expect(topic.auto_close_at).to eq(18.hours.from_now) expect(topic.auto_close_at).to eq(18.hours.from_now)
end end
end end
it "can take a time later in the day" do it "can take a time later in the day" do
Timecop.freeze(now) do Timecop.freeze(now) do
topic.set_auto_close('13:00', admin) topic.set_auto_close('13:00', {by_user: admin})
expect(topic.auto_close_at).to eq(Time.zone.local(2013,11,20,13,0)) expect(topic.auto_close_at).to eq(Time.zone.local(2013,11,20,13,0))
end end
end end
it "can take a time later in the day, with timezone offset" do
Timecop.freeze(now) do
topic.set_auto_close('13:00', {by_user: admin, timezone_offset: 240})
expect(topic.auto_close_at).to eq(Time.zone.local(2013,11,20,17,0))
end
end
it "can take a time for the next day" do it "can take a time for the next day" do
Timecop.freeze(now) do Timecop.freeze(now) do
topic.set_auto_close('5:00', admin) topic.set_auto_close('5:00', {by_user: admin})
expect(topic.auto_close_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_auto_close('1:00', {by_user: admin, timezone_offset: 240})
expect(topic.auto_close_at).to eq(Time.zone.local(2013,11,21,5,0)) expect(topic.auto_close_at).to eq(Time.zone.local(2013,11,21,5,0))
end end
end end
it "can take a timestamp for a future time" do it "can take a timestamp for a future time" do
Timecop.freeze(now) do Timecop.freeze(now) do
topic.set_auto_close('2013-11-22 5:00', admin) topic.set_auto_close('2013-11-22 5:00', {by_user: admin})
expect(topic.auto_close_at).to eq(Time.zone.local(2013,11,22,5,0)) expect(topic.auto_close_at).to eq(Time.zone.local(2013,11,22,5,0))
end end
end end
it "can take a timestamp for a future time, with timezone offset" do
Timecop.freeze(now) do
topic.set_auto_close('2013-11-22 5:00', {by_user: admin, timezone_offset: 240})
expect(topic.auto_close_at).to eq(Time.zone.local(2013,11,22,9,0))
end
end
it "sets a validation error when given a timestamp in the past" do it "sets a validation error when given a timestamp in the past" do
Timecop.freeze(now) do Timecop.freeze(now) do
topic.set_auto_close('2013-11-19 5:00', admin) topic.set_auto_close('2013-11-19 5:00', {by_user: admin})
expect(topic.auto_close_at).to eq(Time.zone.local(2013,11,19,5,0)) expect(topic.auto_close_at).to eq(Time.zone.local(2013,11,19,5,0))
expect(topic.errors[:auto_close_at]).to be_present expect(topic.errors[:auto_close_at]).to be_present
end end
@ -1122,23 +1157,23 @@ describe Topic do
it "can take a timestamp with timezone" do it "can take a timestamp with timezone" do
Timecop.freeze(now) do Timecop.freeze(now) do
topic.set_auto_close('2013-11-25T01:35:00-08:00', admin) topic.set_auto_close('2013-11-25T01:35:00-08:00', {by_user: admin})
expect(topic.auto_close_at).to eq(Time.utc(2013,11,25,9,35)) expect(topic.auto_close_at).to eq(Time.utc(2013,11,25,9,35))
end end
end end
it 'sets auto_close_user to given user if it is a staff or TL4 user' do it 'sets auto_close_user to given user if it is a staff or TL4 user' do
topic.set_auto_close(3, admin) topic.set_auto_close(3, {by_user: admin})
expect(topic.auto_close_user_id).to eq(admin.id) expect(topic.auto_close_user_id).to eq(admin.id)
end end
it 'sets auto_close_user to given user if it is a TL4 user' do it 'sets auto_close_user to given user if it is a TL4 user' do
topic.set_auto_close(3, trust_level_4) topic.set_auto_close(3, {by_user: trust_level_4})
expect(topic.auto_close_user_id).to eq(trust_level_4.id) expect(topic.auto_close_user_id).to eq(trust_level_4.id)
end end
it 'sets auto_close_user to system user if given user is not staff or a TL4 user' do it 'sets auto_close_user to system user if given user is not staff or a TL4 user' do
topic.set_auto_close(3, Fabricate.build(:user, id: 444)) topic.set_auto_close(3, {by_user: Fabricate.build(:user, id: 444)})
expect(topic.auto_close_user_id).to eq(admin.id) expect(topic.auto_close_user_id).to eq(admin.id)
end end