Skip to content

Commit

Permalink
FIX: TopicTimestampChanger should not allow timestamps in the future.
Browse files Browse the repository at this point in the history
  • Loading branch information
tgxworld committed May 22, 2017
1 parent 4382a0b commit 238a156
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 33 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import ModalFunctionality from 'discourse/mixins/modal-functionality';
import computed from 'ember-addons/ember-computed-decorators';
import DiscourseURL from 'discourse/lib/url';
import Topic from 'discourse/models/topic';

// Modal related to changing the timestamp of posts
export default Ember.Controller.extend(ModalFunctionality, {
Expand Down Expand Up @@ -43,7 +44,7 @@ export default Ember.Controller.extend(ModalFunctionality, {
const self = this,
topic = this.get('topicController.model');

Discourse.Topic.changeTimestamp(
Topic.changeTimestamp(
topic.get('id'),
this.get('createdAt').unix()
).then(function() {
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/topics_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -560,11 +560,11 @@ def change_timestamps
begin
TopicTimestampChanger.new(
topic_id: params[:topic_id].to_i,
timestamp: params[:timestamp].to_i
timestamp: params[:timestamp].to_f
).change!

render json: success_json
rescue ActiveRecord::RecordInvalid
rescue ActiveRecord::RecordInvalid, TopicTimestampChanger::InvalidTimestampError
render json: failed_json, status: 422
end
end
Expand Down
15 changes: 10 additions & 5 deletions app/services/topic_timestamp_changer.rb
Original file line number Diff line number Diff line change
@@ -1,22 +1,27 @@
class TopicTimestampChanger
def initialize(params)
@topic = params[:topic] || Topic.with_deleted.find(params[:topic_id])
class InvalidTimestampError < StandardError; end

def initialize(timestamp:, topic: nil, topic_id: nil)
@topic = topic || Topic.with_deleted.find(topic_id)
@posts = @topic.posts
@timestamp = Time.at(params[:timestamp])
@current_timestamp = Time.zone.now
@timestamp = Time.zone.at(timestamp)

raise InvalidTimestampError if @timestamp.to_f > @current_timestamp.to_f

@time_difference = calculate_time_difference
end

def change!
ActiveRecord::Base.transaction do
last_posted_at = @timestamp
now = Time.zone.now

@posts.each do |post|
if post.is_first_post?
update_post(post, @timestamp)
else
new_created_at = Time.at(post.created_at.to_f + @time_difference)
new_created_at = now if new_created_at > now
new_created_at = @current_timestamp if new_created_at > @current_timestamp
last_posted_at = new_created_at if new_created_at > last_posted_at
update_post(post, new_created_at)
end
Expand Down
57 changes: 32 additions & 25 deletions spec/services/topic_timestamp_changer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,53 +4,60 @@
describe "change!" do
let(:old_timestamp) { Time.zone.now }
let(:new_timestamp) { old_timestamp + 1.day }
let!(:topic) { Fabricate(:topic, created_at: old_timestamp) }
let(:topic) { Fabricate(:topic, created_at: old_timestamp) }
let!(:p1) { Fabricate(:post, topic: topic, created_at: old_timestamp) }
let!(:p2) { Fabricate(:post, topic: topic, created_at: old_timestamp + 1.day) }
let(:params) { { topic_id: topic.id, timestamp: new_timestamp.to_f } }

context 'new timestamp is in the future' do
let(:new_timestamp) { old_timestamp + 2.day }

it 'should raise the right error' do
expect { TopicTimestampChanger.new(topic: topic, timestamp: new_timestamp.to_f).change! }
.to raise_error(TopicTimestampChanger::InvalidTimestampError)
end
end

context 'new timestamp is in the past' do
let(:new_timestamp) { old_timestamp - 2.day }

it 'changes the timestamp of the topic and opening post' do
Timecop.freeze do
TopicTimestampChanger.new(params).change!
TopicTimestampChanger.new(topic: topic, timestamp: new_timestamp.to_f).change!

topic.reload
[:created_at, :updated_at, :bumped_at].each do |column|
expect(topic.public_send(column)).to be_within_one_second_of(new_timestamp)
expect(topic.public_send(column)).to be_within(1.second).of(new_timestamp)
end

p1.reload
[:created_at, :updated_at].each do |column|
expect(p1.public_send(column)).to be_within_one_second_of(new_timestamp)
expect(p1.public_send(column)).to be_within(1.second).of(new_timestamp)
end

expect(topic.last_posted_at).to be_within_one_second_of(p2.reload.created_at)
end
end
end

describe 'predated timestamp' do
it 'updates the timestamp of posts in the topic with the time difference applied' do
TopicTimestampChanger.new(params).change!
p2.reload
[:created_at, :updated_at].each do |column|
expect(p2.public_send(column)).to be_within(1.second).of(new_timestamp + 1.day)
end

p2.reload
[:created_at, :updated_at].each do |column|
expect(p2.public_send(column)).to be_within_one_second_of(old_timestamp + 2.day)
expect(topic.last_posted_at).to be_within(1.second).of(p2.reload.created_at)
end
end
end

describe 'backdated timestamp' do
let(:new_timestamp) { old_timestamp - 1.day }
describe 'when posts have timestamps in the future' do
let(:new_timestamp) { Time.zone.now }
let(:p3) { Fabricate(:post, topic: topic, created_at: new_timestamp + 3.day) }

it 'should set the new timestamp as the default timestamp' do
Timecop.freeze do
p3
TopicTimestampChanger.new(topic: topic, timestamp: new_timestamp.to_f).change!

it 'updates the timestamp of posts in the topic with the time difference applied' do
TopicTimestampChanger.new(params).change!
p3.reload

p2.reload
[:created_at, :updated_at].each do |column|
expect(p2.public_send(column)).to be_within_one_second_of(old_timestamp)
[:created_at, :updated_at].each do |column|
expect(p3.public_send(column)).to be_within(1.second).of(new_timestamp)
end
end
end
end
end
Expand All @@ -59,7 +66,7 @@
$redis.set AdminDashboardData.stats_cache_key, "X"
$redis.set About.stats_cache_key, "X"

TopicTimestampChanger.new(params).change!
TopicTimestampChanger.new(topic: topic, timestamp: Time.zone.now.to_f).change!

expect($redis.get(AdminDashboardData.stats_cache_key)).to eq(nil)
expect($redis.get(About.stats_cache_key)).to eq(nil)
Expand Down

0 comments on commit 238a156

Please sign in to comment.