From 477f352e8f0d53ddde1249666c5f033b3225f609 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9gis=20Hanol?= <regis@hanol.fr>
Date: Fri, 29 May 2015 20:08:39 +0200
Subject: [PATCH] FIX: remove latest empty revision
---
lib/post_revisor.rb | 51 ++++++++++++++++++----------
spec/components/post_revisor_spec.rb | 20 +++++++++--
2 files changed, 51 insertions(+), 20 deletions(-)
diff --git a/lib/post_revisor.rb b/lib/post_revisor.rb
index 3d85fc25c..6e4278c17 100644
--- a/lib/post_revisor.rb
+++ b/lib/post_revisor.rb
@@ -91,7 +91,7 @@ class PostRevisor
@fields[:category_id] = @fields[:category_id].to_i if @fields.has_key?(:category_id)
# always reset edit_reason unless provided
- @fields[:edit_reason] = nil unless @fields.has_key?(:edit_reason)
+ @fields[:edit_reason] = nil unless @fields[:edit_reason].present?
return false unless should_revise?
@@ -199,17 +199,18 @@ class PostRevisor
create_or_update_revision
end
- def update_post
- prev_owner, new_owner = nil, nil
- if @fields.has_key?("user_id") && @fields["user_id"] != @post.user_id
- prev_owner = User.find_by_id(@post.user_id)
- new_owner = User.find_by_id(@fields["user_id"])
+ USER_ACTIONS_TO_REMOVE ||= [UserAction::NEW_TOPIC, UserAction::REPLY, UserAction::RESPONSE]
+
+ def update_post
+ if @fields.has_key?("user_id") && @fields["user_id"] != @post.user_id
+ prev_owner = User.find(@post.user_id)
+ new_owner = User.find(@fields["user_id"])
- UserAction.where( target_post_id: @post.id,
- user_id: prev_owner.id,
- action_type: [UserAction::NEW_TOPIC, UserAction::REPLY, UserAction::RESPONSE] )
- .find_each { |ua| ua.destroy }
# UserActionObserver will create new UserAction records for the new owner
+ UserAction.where(target_post_id: @post.id)
+ .where(user_id: prev_owner.id)
+ .where(action_type: USER_ACTIONS_TO_REMOVE)
+ .destroy_all
end
POST_TRACKED_FIELDS.each do |field|
@@ -229,17 +230,20 @@ class PostRevisor
# post owner changed
if prev_owner && new_owner && prev_owner != new_owner
- likes = UserAction.where( target_post_id: @post.id,
- user_id: prev_owner.id,
- action_type: UserAction::WAS_LIKED ).update_all(user_id: new_owner.id)
+ likes = UserAction.where(target_post_id: @post.id)
+ .where(user_id: prev_owner.id)
+ .where(action_type: UserAction::WAS_LIKED)
+ .update_all(user_id: new_owner.id)
prev_owner.user_stat.post_count -= 1
prev_owner.user_stat.topic_count -= 1 if @post.is_first_post?
prev_owner.user_stat.likes_received -= likes
prev_owner.user_stat.update_topic_reply_count
+
if @post.created_at == prev_owner.user_stat.first_post_created_at
prev_owner.user_stat.first_post_created_at = prev_owner.posts.order('created_at ASC').first.try(:created_at)
end
+
prev_owner.user_stat.save
new_owner.user_stat.post_count += 1
@@ -302,14 +306,26 @@ class PostRevisor
modifications = post_changes.merge(@topic_changes.diff)
modifications.each_key do |field|
if revision.modifications.has_key?(field)
- old_value = revision.modifications[field][0]
- new_value = modifications[field][1]
- revision.modifications[field] = [old_value, new_value]
+ old_value = revision.modifications[field][0].to_s
+ new_value = modifications[field][1].to_s
+ if old_value != new_value
+ revision.modifications[field] = [old_value, new_value]
+ else
+ revision.modifications.delete(field)
+ end
else
revision.modifications[field] = modifications[field]
end
end
- revision.save if modifications.length
+ # should probably do this before saving the post!
+ if revision.modifications.empty?
+ revision.destroy
+ @post.version -= 1
+ @post.public_version -= 1
+ @post.save
+ else
+ revision.save
+ end
end
def post_changes
@@ -410,4 +426,3 @@ class PostRevisor
end
end
-
diff --git a/spec/components/post_revisor_spec.rb b/spec/components/post_revisor_spec.rb
index 099721585..efcd03054 100644
--- a/spec/components/post_revisor_spec.rb
+++ b/spec/components/post_revisor_spec.rb
@@ -5,7 +5,7 @@ describe PostRevisor do
let(:topic) { Fabricate(:topic) }
let(:newuser) { Fabricate(:newuser) }
- let(:post_args) { {user: newuser, topic: topic} }
+ let(:post_args) { { user: newuser, topic: topic } }
context 'TopicChanges' do
let(:topic) { Fabricate(:topic) }
@@ -74,7 +74,8 @@ describe PostRevisor do
describe 'ninja editing' do
it 'correctly applies edits' do
- SiteSetting.ninja_edit_window = 1.minute.to_i
+ SiteSetting.stubs(:ninja_edit_window).returns(1.minute)
+
subject.revise!(post.user, { raw: 'updated body' }, revised_at: post.updated_at + 10.seconds)
post.reload
@@ -84,6 +85,21 @@ describe PostRevisor do
expect(post.last_version_at).to eq(first_version_at)
expect(subject.category_changed).to be_blank
end
+
+ it "doesn't create a new version" do
+ SiteSetting.stubs(:ninja_edit_window).returns(1.minute)
+
+ # making a revision
+ subject.revise!(post.user, { raw: 'updated body' }, revised_at: post.updated_at + SiteSetting.ninja_edit_window + 1.seconds)
+ # "roll back"
+ subject.revise!(post.user, { raw: 'Hello world' }, revised_at: post.updated_at + SiteSetting.ninja_edit_window + 2.seconds)
+
+ post.reload
+
+ expect(post.version).to eq(1)
+ expect(post.public_version).to eq(1)
+ expect(post.revisions.size).to eq(0)
+ end
end
describe 'revision much later' do