FEATURE: editing_grace_period_max_diff to force revisions in grace period

If a user performs a substantive edit of 20 chars or more during grace period
we will store a revision to track the change

This allows for better auditing of changes that happen during the grace period

config/locales/server.en.yml

@@ -1023,6 +1023,7 @@ en:
     download_remote_images_max_days_old: "Don't download remote images for posts that are more than n days old."
     disabled_image_download_domains: "Remote images will never be downloaded from these domains. Pipe-delimited list."
     editing_grace_period: "For (n) seconds after posting, editing will not create a new version in the post history."
+    editing_grace_period_max_diff: "Maximum number of character changes allowed in editing grace period, if more changed store another post revision"
     staff_edit_locks_post: "Posts will be locked from editing if they are edited by staff members"
     post_edit_time_limit: "The author can edit or delete their post for (n) minutes after posting. Set to 0 for forever."
     edit_history_visible_to_public: "Allow everyone to see previous versions of an edited post. When disabled, only staff members can view."

config/site_settings.yml

@@ -534,6 +534,7 @@ posting:
     client: true
     validator: "EnablePrivateEmailMessagesValidator"
   editing_grace_period: 300
+  editing_grace_period_max_diff: 20
   staff_edit_locks_post: false
   post_edit_time_limit:
     default: 86400

lib/post_revisor.rb

@@ -214,7 +214,15 @@ class PostRevisor
   end
 
   def revise_post
-    should_create_new_version? ? revise_and_create_new_version : revise
+    if should_create_new_version?
+      revise_and_create_new_version
+    else
+      unless cached_original_raw
+        self.original_raw = @post.raw
+        self.original_cooked = @post.cooked
+      end
+      revise
+    end
   end
 
   def should_create_new_version?
@@ -226,9 +234,57 @@ class PostRevisor
     @post.last_editor_id != @editor.id
   end
 
+  def original_raw_key
+    "original_raw_#{(@last_version_at.to_f * 1000).to_i}#{@post.id}"
+  end
+
+  def original_cooked_key
+    "original_cooked_#{(@last_version_at.to_f * 1000).to_i}#{@post.id}"
+  end
+
+  def cached_original_raw
+    @cached_original_raw ||= $redis.get(original_raw_key)
+  end
+
+  def cached_original_cooked
+    @cached_original_cooked ||= $redis.get(original_cooked_key)
+  end
+
+  def original_raw
+    cached_original_raw || @post.raw
+  end
+
+  def original_raw=(val)
+    @cached_original_raw = val
+    $redis.setex(original_raw_key, SiteSetting.editing_grace_period + 1, val)
+  end
+
+  def original_cooked=(val)
+    @cached_original_cooked = val
+    $redis.setex(original_cooked_key, SiteSetting.editing_grace_period + 1, val)
+  end
+
+  def diff_size(before, after)
+    changes = 0
+    ONPDiff.new(before, after).short_diff.each do |str, type|
+      next if type == :common
+      changes += str.length
+    end
+    changes
+  end
+
   def ninja_edit?
     return false if @post.has_active_flag?
-    @revised_at - @last_version_at <= SiteSetting.editing_grace_period.to_i
+    return false if (@revised_at - @last_version_at) > SiteSetting.editing_grace_period.to_i
+
+    if new_raw = @fields[:raw]
+      if (original_raw.length - new_raw.length).abs > SiteSetting.editing_grace_period_max_diff.to_i ||
+        diff_size(original_raw, new_raw) > SiteSetting.editing_grace_period_max_diff.to_i
+        return false
+      end
+    end
+
+    true
   end
 
   def owner_changed?
@@ -368,6 +424,15 @@ class PostRevisor
 
   def create_revision
     modifications = post_changes.merge(@topic_changes.diff)
+
+    if modifications["raw"]
+      modifications["raw"][0] = cached_original_raw || modifications["raw"][0]
+    end
+
+    if modifications["cooked"]
+      modifications["cooked"][0] = cached_original_cooked || modifications["cooked"][0]
+    end
+
     PostRevision.create!(
       user_id: @post.last_editor_id,
       post_id: @post.id,
@@ -380,6 +445,7 @@ class PostRevisor
     return unless revision = PostRevision.find_by(post_id: @post.id, number: @post.version)
     revision.user_id = @post.last_editor_id
     modifications = post_changes.merge(@topic_changes.diff)
+
     modifications.each_key do |field|
       if revision.modifications.has_key?(field)
         old_value = revision.modifications[field][0].to_s

spec/components/post_revisor_spec.rb

@@ -111,8 +111,30 @@ describe PostRevisor do
         expect(subject.category_changed).to be_blank
       end
 
+      it "does create a new version if a large diff happens" do
+        SiteSetting.editing_grace_period_max_diff = 10
+
+        post = Fabricate(:post, raw: 'hello world')
+        revisor = PostRevisor.new(post)
+        revisor.revise!(post.user, { raw: 'hello world123456789' }, revised_at: post.updated_at + 1.second)
+
+        post.reload
+
+        expect(post.version).to eq(1)
+
+        revisor = PostRevisor.new(post)
+        revisor.revise!(post.user, { raw: 'hello world12345678901' }, revised_at: post.updated_at + 1.second)
+
+        post.reload
+        expect(post.version).to eq(2)
+
+        expect(post.revisions.first.modifications["raw"][0]).to eq("hello world")
+        expect(post.revisions.first.modifications["cooked"][0]).to eq("<p>hello world</p>")
+      end
+
       it "doesn't create a new version" do
         SiteSetting.editing_grace_period = 1.minute
+        SiteSetting.editing_grace_period_max_diff = 100
 
         # making a revision
         subject.revise!(post.user, { raw: 'updated body' }, revised_at: post.updated_at + SiteSetting.editing_grace_period + 1.seconds)
@@ -317,48 +339,34 @@ describe PostRevisor do
     end
 
     describe 'with a new body' do
+      before do
+        SiteSetting.editing_grace_period_max_diff = 1000
+      end
+
       let(:changed_by) { Fabricate(:coding_horror) }
       let!(:result) { subject.revise!(changed_by, raw: "lets update the body. Здравствуйте") }
 
-      it 'returns true' do
+      it 'correctly updates raw' do
         expect(result).to eq(true)
-      end
-
-      it 'updates the body' do
         expect(post.raw).to eq("lets update the body. Здравствуйте")
-      end
-
-      it 'sets the invalidate oneboxes attribute' do
         expect(post.invalidate_oneboxes).to eq(true)
-      end
-
-      it 'increased the versions' do
         expect(post.version).to eq(2)
         expect(post.public_version).to eq(2)
-      end
-
-      it 'has the new revision' do
         expect(post.revisions.size).to eq(1)
-      end
-
-      it "saved the user who made the change in the revisions" do
         expect(post.revisions.first.user_id).to eq(changed_by.id)
-      end
 
-      it "updates the word count" do
+        # updates word count
         expect(post.word_count).to eq(5)
         post.topic.reload
         expect(post.topic.word_count).to eq(5)
       end
 
       context 'second poster posts again quickly' do
-        before do
+
+        it 'is a ninja edit, because the second poster posted again quickly' do
           SiteSetting.editing_grace_period = 1.minute
           subject.revise!(changed_by, { raw: '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
           expect(post.version).to eq(2)
           expect(post.public_version).to eq(2)
           expect(post.revisions.size).to eq(1)

spec/models/post_spec.rb

@@ -674,7 +674,7 @@ describe Post do
       context 'second poster posts again quickly' do
 
         it 'is a ninja edit, because the second poster posted again quickly' do
-          SiteSetting.expects(:editing_grace_period).returns(1.minute.to_i)
+          SiteSetting.editing_grace_period = 1.minute.to_i
           post.revise(changed_by, { raw: 'yet another updated body' }, revised_at: post.updated_at + 10.seconds)
           post.reload
 

spec/models/topic_spec.rb

@@ -734,8 +734,8 @@ describe Topic do
 
       it "doesn't bump the topic on an edit to the last post that doesn't result in a new version" do
         expect {
-          SiteSetting.expects(:editing_grace_period).returns(5.minutes)
+          SiteSetting.editing_grace_period = 5.minutes
-          @last_post.revise(@last_post.user, { raw: 'updated contents' }, revised_at: @last_post.created_at + 10.seconds)
+          @last_post.revise(@last_post.user, { raw: @last_post.raw + "a" }, revised_at: @last_post.created_at + 10.seconds)
           @topic.reload
         }.not_to change(@topic, :bumped_at)
       end