diff --git a/app/models/post.rb b/app/models/post.rb
index 2e71eb425..066cac96b 100644
--- a/app/models/post.rb
+++ b/app/models/post.rb
@@ -1,6 +1,7 @@
require_dependency 'jobs'
require_dependency 'pretty_text'
require_dependency 'rate_limiter'
+require_dependency 'post_revisor'
require 'archetype'
require 'hpricot'
@@ -14,12 +15,6 @@ class Post < ActiveRecord::Base
FLAG_THRESHOLD_REACHED_AGAIN = 2
end
- # A custom rate limiter for edits
- class EditRateLimiter < RateLimiter
- def initialize(user)
- super(user, "edit-post:#{Date.today.to_s}", SiteSetting.max_edits_per_day, 1.day.to_i)
- end
- end
versioned
@@ -115,6 +110,11 @@ class Post < ActiveRecord::Base
@cooked_document ||= Nokogiri::HTML.fragment(self.cooked)
end
+ def reset_cooked
+ @cooked_document = nil
+ self.cooked = nil
+ end
+
def image_count
return 0 unless self.raw.present?
cooked_document.search("img.emoji").remove
@@ -290,77 +290,14 @@ class Post < ActiveRecord::Base
self.save
end
- # Update the body of a post. Will create a new version when appropriate
- def revise(updated_by, new_raw, opts={})
-
- # Only update if it changes
- return false if self.raw == new_raw
-
- updater = lambda do |new_version=false|
-
- # Raw is about to change, enable validations
- @cooked_document = nil
- self.cooked = nil
-
- self.raw = new_raw
- self.updated_by = updated_by
- self.last_editor_id = updated_by.id
-
- if self.hidden && self.hidden_reason_id == HiddenReason::FLAG_THRESHOLD_REACHED
- self.hidden = false
- self.hidden_reason_id = nil
- self.topic.update_attributes(visible: true)
-
- PostAction.clear_flags!(self, -1)
- end
-
- self.save
- end
-
- # We can optionally specify when this version was revised. Defaults to now.
- revised_at = opts[:revised_at] || Time.now
- new_version = false
-
- # We always create a new version if the poster has changed
- new_version = true if (self.last_editor_id != updated_by.id)
-
- # We always create a new version if it's been greater than the ninja edit window
- new_version = true if (revised_at - last_version_at) > SiteSetting.ninja_edit_window.to_i
-
- new_version = true if opts[:force_new_version]
-
- # Create the new version (or don't)
- if new_version
-
- self.cached_version = version + 1
-
- Post.transaction do
- self.last_version_at = revised_at
- updater.call(true)
- EditRateLimiter.new(updated_by).performed! unless opts[:bypass_rate_limiter]
-
- # If a new version is created of the last post, bump it.
- unless Post.where('post_number > ? and topic_id = ?', self.post_number, self.topic_id).exists?
- topic.update_column(:bumped_at, Time.now) unless opts[:bypass_bump]
- end
- end
-
- else
- skip_version(&updater)
- end
-
- # Invalidate any oneboxes
- self.invalidate_oneboxes = true
- trigger_post_process
-
- true
- end
-
-
def url
"/t/#{Slug.for(topic.title)}/#{topic.id}/#{post_number}"
end
+ def revise(updated_by, new_raw, opts={})
+ PostRevisor.new(self).revise!(updated_by, new_raw, opts)
+ end
+
# Various callbacks
before_create do
self.post_number ||= Topic.next_post_number(topic_id, reply_to_post_number.present?)
diff --git a/lib/edit_rate_limiter.rb b/lib/edit_rate_limiter.rb
new file mode 100644
index 000000000..f91291eda
--- /dev/null
+++ b/lib/edit_rate_limiter.rb
@@ -0,0 +1,6 @@
+require 'rate_limiter'
+class EditRateLimiter < RateLimiter
+ def initialize(user)
+ super(user, "edit-post:#{Date.today.to_s}", SiteSetting.max_edits_per_day, 1.day.to_i)
+ end
+end
diff --git a/lib/post_revisor.rb b/lib/post_revisor.rb
new file mode 100644
index 000000000..c8ec6449e
--- /dev/null
+++ b/lib/post_revisor.rb
@@ -0,0 +1,83 @@
+require 'edit_rate_limiter'
+class PostRevisor
+ def initialize(post)
+ @post = post
+ end
+
+ def revise!(user, new_raw, opts = {})
+ @user, @new_raw, @opts = user, new_raw, opts
+ return false if not should_revise?
+ revise_post
+ post_process_post
+ true
+ end
+
+ private
+
+ def should_revise?
+ @post.raw != @new_raw
+ end
+
+ def revise_post
+ if should_create_new_version?
+ revise_and_create_new_version
+ else
+ revise_without_creating_a_new_version
+ end
+ end
+
+ def get_revised_at
+ @opts[:revised_at] || Time.now
+ end
+
+ def should_create_new_version?
+ (@post.last_editor_id != @user.id) or
+ ((get_revised_at - @post.last_version_at) > SiteSetting.ninja_edit_window.to_i) or
+ @opts[:force_new_version] == true
+ end
+
+ def revise_and_create_new_version
+ Post.transaction do
+ @post.cached_version = @post.version + 1
+ @post.last_version_at = get_revised_at
+ update_post
+ EditRateLimiter.new(@post.user).performed! unless @opts[:bypass_rate_limiter] == true
+ bump_topic unless @opts[:bypass_bump]
+ end
+ end
+
+ def revise_without_creating_a_new_version
+ @post.skip_version do
+ update_post
+ end
+ end
+
+ def bump_topic
+ unless Post.where('post_number > ? and topic_id = ?', @post.post_number, @post.topic_id).exists?
+ @post.topic.update_column(:bumped_at, Time.now)
+ end
+ end
+
+ def update_post
+ @post.reset_cooked
+
+ @post.raw = @new_raw
+ @post.updated_by = @user
+ @post.last_editor_id = @user.id
+
+ if @post.hidden && @post.hidden_reason_id == Post::HiddenReason::FLAG_THRESHOLD_REACHED
+ @post.hidden = false
+ @post.hidden_reason_id = nil
+ @post.topic.update_attributes(visible: true)
+
+ PostAction.clear_flags!(@post, -1)
+ end
+
+ @post.save
+ end
+
+ def post_process_post
+ @post.invalidate_oneboxes = true
+ @post.trigger_post_process
+ end
+end
diff --git a/spec/components/post_revisor_spec.rb b/spec/components/post_revisor_spec.rb
new file mode 100644
index 000000000..386211f47
--- /dev/null
+++ b/spec/components/post_revisor_spec.rb
@@ -0,0 +1,163 @@
+require 'spec_helper'
+require 'post_revisor'
+
+describe PostRevisor do
+
+ let(:topic) { Fabricate(:topic) }
+ let(:post_args) { {user: topic.user, topic: topic} }
+
+ context 'revise' do
+
+ let(:post) { Fabricate(:post, post_args) }
+ let(:first_version_at) { post.last_version_at }
+
+ subject { described_class.new(post) }
+
+ describe 'with the same body' do
+
+ it 'returns false' do
+ subject.revise!(post.user, post.raw).should be_false
+ end
+
+ it "doesn't change cached_version" do
+ lambda { subject.revise!(post.user, post.raw); post.reload }.should_not change(post, :cached_version)
+ end
+
+ end
+
+ describe 'ninja editing' do
+ before do
+ SiteSetting.expects(:ninja_edit_window).returns(1.minute.to_i)
+ subject.revise!(post.user, 'updated body', revised_at: post.updated_at + 10.seconds)
+ post.reload
+ end
+
+ it 'does not update cached_version' do
+ post.cached_version.should == 1
+ end
+
+ it 'does not create a new version' do
+ post.all_versions.size.should == 1
+ end
+
+ it "doesn't change the last_version_at" do
+ post.last_version_at.should == first_version_at
+ end
+ end
+
+ describe 'revision much later' do
+
+ let!(:revised_at) { post.updated_at + 2.minutes }
+
+ before do
+ SiteSetting.stubs(:ninja_edit_window).returns(1.minute.to_i)
+ subject.revise!(post.user, 'updated body', revised_at: revised_at)
+ post.reload
+ end
+
+ it 'updates the cached_version' do
+ post.cached_version.should == 2
+ end
+
+ it 'creates a new version' do
+ post.all_versions.size.should == 2
+ end
+
+ it "updates the last_version_at" do
+ post.last_version_at.to_i.should == revised_at.to_i
+ end
+
+ describe "new edit window" do
+
+ before do
+ subject.revise!(post.user, 'yet another updated body', revised_at: revised_at)
+ post.reload
+ end
+
+ it "doesn't create a new version if you do another" do
+ post.cached_version.should == 2
+ end
+
+ it "doesn't change last_version_at" do
+ post.last_version_at.to_i.should == revised_at.to_i
+ end
+
+ context "after second window" do
+
+ let!(:new_revised_at) {revised_at + 2.minutes}
+
+ before do
+ subject.revise!(post.user, 'yet another, another updated body', revised_at: new_revised_at)
+ post.reload
+ end
+
+ it "does create a new version after the edit window" do
+ post.cached_version.should == 3
+ end
+
+ it "does create a new version after the edit window" do
+ post.last_version_at.to_i.should == new_revised_at.to_i
+ end
+ end
+ end
+ end
+
+ describe 'rate limiter' do
+ let(:changed_by) { Fabricate(:coding_horror) }
+
+ it "triggers a rate limiter" do
+ EditRateLimiter.any_instance.expects(:performed!)
+ subject.revise!(changed_by, 'updated body')
+ end
+ end
+
+ describe 'with a new body' do
+ let(:changed_by) { Fabricate(:coding_horror) }
+ let!(:result) { subject.revise!(changed_by, 'updated body') }
+
+ it 'returns true' do
+ result.should be_true
+ end
+
+ it 'updates the body' do
+ post.raw.should == 'updated body'
+ end
+
+ it 'sets the invalidate oneboxes attribute' do
+ post.invalidate_oneboxes.should == true
+ end
+
+ it 'increased the cached_version' do
+ post.cached_version.should == 2
+ end
+
+ it 'has the new version in all_versions' do
+ post.all_versions.size.should == 2
+ end
+
+ it 'has versions' do
+ post.versions.should be_present
+ end
+
+ it "saved the user who made the change in the version" do
+ post.versions.first.user.should be_present
+ end
+
+ context 'second poster posts again quickly' do
+ before do
+ SiteSetting.expects(:ninja_edit_window).returns(1.minute.to_i)
+ subject.revise!(changed_by, 'yet another updated body', revised_at: post.updated_at + 10.seconds)
+ post.reload
+ end
+
+ it 'is a ninja edit, because the second poster posted again quickly' do
+ post.cached_version.should == 2
+ end
+
+ it 'is a ninja edit, because the second poster posted again quickly' do
+ post.all_versions.size.should == 2
+ end
+ end
+ end
+ end
+end
diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb
index c58f44c2b..88c943ec0 100644
--- a/spec/models/post_spec.rb
+++ b/spec/models/post_spec.rb
@@ -370,7 +370,7 @@ describe Post do
let(:changed_by) { Fabricate(:coding_horror) }
it "triggers a rate limiter" do
- Post::EditRateLimiter.any_instance.expects(:performed!)
+ EditRateLimiter.any_instance.expects(:performed!)
post.revise(changed_by, 'updated body')
end
end