FIX: topic timer offset applied two times

timezone offset was calculated and sent from browser to server, it would be applied on utc time generated from '2013-11-22 5:00' format for example and then sent back to browser which would display it thinking it's UTC time using `moment(utc time)` when it's in fact an UTC time we have offseted with the initial user timezone.

This is impossible to automatically test in the current app state. Easiest reproduction is in live browser after setting your timezone to `America/New_York`, when setting a topic timer to later_today, after save, the time under the topic should be off to something roughly equal +1/-1  hour to your timezone offset.
This commit is contained in:
Joffrey JAFFEUX
2017-12-07 14:42:58 +01:00
committed by GitHub
parent eda30c4cf2
commit f0ef307d2d
4 changed files with 3 additions and 25 deletions

View File

@ -6,8 +6,7 @@ const TopicTimer = RestModel.extend({});
TopicTimer.reopenClass({ TopicTimer.reopenClass({
updateStatus(topicId, time, basedOnLastPost, statusType, categoryId) { updateStatus(topicId, time, basedOnLastPost, statusType, categoryId) {
let data = { let data = {
time: time, time,
timezone_offset: (new Date().getTimezoneOffset()),
status_type: statusType status_type: statusType
}; };

View File

@ -287,7 +287,7 @@ class TopicsController < ApplicationController
end end
def timer def timer
params.permit(:time, :timezone_offset, :based_on_last_post, :category_id) params.permit(:time, :based_on_last_post, :category_id)
params.require(:status_type) params.require(:status_type)
status_type = status_type =
@ -302,7 +302,6 @@ class TopicsController < ApplicationController
options = { options = {
by_user: current_user, by_user: current_user,
timezone_offset: params[:timezone_offset]&.to_i,
based_on_last_post: params[:based_on_last_post] based_on_last_post: params[:based_on_last_post]
} }

View File

@ -1032,10 +1032,9 @@ SQL
# * `nil` to delete the topic's status update. # * `nil` to delete the topic's status update.
# Options: # Options:
# * by_user: User who is setting the topic's status update. # * by_user: User who is setting the topic's status update.
# * timezone_offset: (Integer) offset from UTC in minutes of the given argument.
# * based_on_last_post: True if time should be based on timestamp of the last post. # * 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. # * 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) def set_or_create_timer(status_type, time, by_user: nil, based_on_last_post: false, category_id: SiteSetting.uncategorized_category_id)
return delete_topic_timer(status_type, by_user: by_user) if time.blank? return delete_topic_timer(status_type, by_user: by_user) if time.blank?
public_topic_timer = !!TopicTimer.public_types[status_type] public_topic_timer = !!TopicTimer.public_types[status_type]
@ -1067,7 +1066,6 @@ SQL
if 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" # a timestamp in client's time zone, like "2015-5-27 12:00"
topic_timer.execute_at = timestamp topic_timer.execute_at = timestamp
topic_timer.execute_at += timezone_offset * 60 if timezone_offset
topic_timer.errors.add(:execute_at, :invalid) if timestamp < now topic_timer.errors.add(:execute_at, :invalid) if timestamp < now
else else
num_hours = time.to_f num_hours = time.to_f

View File

@ -1236,24 +1236,12 @@ describe Topic do
expect(topic.topic_timers.first.execute_at).to eq(3.days.from_now) expect(topic.topic_timers.first.execute_at).to eq(3.days.from_now)
end end
it 'can take a number of hours as an integer, with timezone offset' do
freeze_time now
topic.set_or_create_timer(TopicTimer.types[:close], 72, by_user: admin, timezone_offset: 240)
expect(topic.topic_timers.first.execute_at).to eq(3.days.from_now)
end
it 'can take a number of hours as a string' do it 'can take a number of hours as a string' do
freeze_time now freeze_time now
topic.set_or_create_timer(TopicTimer.types[:close], '18', by_user: admin) topic.set_or_create_timer(TopicTimer.types[:close], '18', by_user: admin)
expect(topic.topic_timers.first.execute_at).to eq(18.hours.from_now) expect(topic.topic_timers.first.execute_at).to eq(18.hours.from_now)
end end
it 'can take a number of hours as a string, with timezone offset' do
freeze_time now
topic.set_or_create_timer(TopicTimer.types[:close], '18', by_user: admin, timezone_offset: 240)
expect(topic.topic_timers.first.execute_at).to eq(18.hours.from_now)
end
it 'can take a number of hours as a string and can handle based on last post' do it 'can take a number of hours as a string and can handle based on last post' do
freeze_time now freeze_time now
topic.set_or_create_timer(TopicTimer.types[:close], '18', by_user: admin, based_on_last_post: true) topic.set_or_create_timer(TopicTimer.types[:close], '18', by_user: admin, based_on_last_post: true)
@ -1266,12 +1254,6 @@ describe Topic do
expect(topic.topic_timers.first.execute_at).to eq(Time.zone.local(2013, 11, 22, 5, 0)) expect(topic.topic_timers.first.execute_at).to eq(Time.zone.local(2013, 11, 22, 5, 0))
end end
it "can take a timestamp for a future time, with timezone offset" do
freeze_time now
topic.set_or_create_timer(TopicTimer.types[:close], '2013-11-22 5:00', by_user: admin, timezone_offset: 240)
expect(topic.topic_timers.first.execute_at).to eq(Time.zone.local(2013, 11, 22, 9, 0))
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
freeze_time now freeze_time now
topic.set_or_create_timer(TopicTimer.types[:close], '2013-11-19 5:00', by_user: admin) topic.set_or_create_timer(TopicTimer.types[:close], '2013-11-19 5:00', by_user: admin)