From 303d6ce821fa8f14e7060c9c743bc3b40843bfcf Mon Sep 17 00:00:00 2001 From: r888888888 Date: Thu, 25 Jul 2013 14:12:35 -0700 Subject: [PATCH] fixes #1863 --- app/models/post.rb | 37 +++++++++++++++++++++++++++------- test/unit/post_test.rb | 25 +++++++++++++++++++++++ test/unit/post_version_test.rb | 2 ++ test/unit/tag_alias_test.rb | 12 ++++++++--- 4 files changed, 66 insertions(+), 10 deletions(-) diff --git a/app/models/post.rb b/app/models/post.rb index 86ed69975..031994044 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -915,16 +915,39 @@ class Post < ActiveRecord::Base module VersionMethods def create_version(force = false) if new_record? || rating_changed? || source_changed? || parent_id_changed? || tag_string_changed? || force - CurrentUser.increment!(:post_update_count) - versions.create( - :rating => rating, - :source => source, - :tags => tag_string, - :parent_id => parent_id - ) + if merge_version? + merge_version + else + create_new_version + end end end + def merge_version? + prev = versions.last + prev && prev.updater_id == CurrentUser.user.id && prev.updated_at > 1.hour.ago + end + + def create_new_version + CurrentUser.increment!(:post_update_count) + versions.create( + :rating => rating, + :source => source, + :tags => tag_string, + :parent_id => parent_id + ) + end + + def merge_version + prev = versions.last + prev.update_attributes( + :rating => rating, + :source => source, + :tags => tag_string, + :parent_id => parent_id + ) + end + def revert_to(target) self.tag_string = target.tags self.rating = target.rating diff --git a/test/unit/post_test.rb b/test/unit/post_test.rb index 3935d77f8..3282aa860 100644 --- a/test/unit/post_test.rb +++ b/test/unit/post_test.rb @@ -566,8 +566,32 @@ class PostTest < ActiveSupport::TestCase end context "that has been updated" do + should "create a new version if it's the first version" do + assert_difference("PostVersion.count", 1) do + post = FactoryGirl.create(:post) + end + end + + should "create a new version if it's been over an hour since the last update" do + post = FactoryGirl.create(:post) + Timecop.travel(6.hours.from_now) do + assert_difference("PostVersion.count", 1) do + post.update_attributes(:tag_string => "zzz") + end + end + end + + should "merge with the previous version if the updater is the same user and it's been less than an hour" do + post = FactoryGirl.create(:post) + assert_difference("PostVersion.count", 0) do + post.update_attributes(:tag_string => "zzz") + end + assert_equal("zzz", post.versions.last.tags) + end + should "increment the updater's post_update_count" do post = FactoryGirl.create(:post, :tag_string => "aaa bbb ccc") + post.stubs(:merge_version?).returns(false) assert_difference("CurrentUser.post_update_count", 1) do post.update_attributes(:tag_string => "zzz") @@ -1200,6 +1224,7 @@ class PostTest < ActiveSupport::TestCase context "a post that has been updated" do setup do @post = FactoryGirl.create(:post, :rating => "q", :tag_string => "aaa") + @post.stubs(:merge_version?).returns(false) @post.update_attributes(:tag_string => "aaa bbb ccc ddd") @post.update_attributes(:tag_string => "bbb xxx yyy", :source => "xyz") @post.update_attributes(:tag_string => "bbb mmm yyy", :source => "abc") diff --git a/test/unit/post_version_test.rb b/test/unit/post_version_test.rb index 36efbfc61..79272bff3 100644 --- a/test/unit/post_version_test.rb +++ b/test/unit/post_version_test.rb @@ -17,6 +17,7 @@ class PostVersionTest < ActiveSupport::TestCase context "that has multiple versions: " do setup do @post = FactoryGirl.create(:post, :tag_string => "1") + @post.stubs(:merge_version?).returns(false) @post.update_attributes(:tag_string => "1 2") @post.update_attributes(:tag_string => "2 3") end @@ -59,6 +60,7 @@ class PostVersionTest < ActiveSupport::TestCase setup do @parent = FactoryGirl.create(:post) @post = FactoryGirl.create(:post, :tag_string => "aaa bbb ccc", :rating => "q", :source => "xyz") + @post.stubs(:merge_version?).returns(false) @post.update_attributes(:tag_string => "bbb ccc xxx", :source => "") end diff --git a/test/unit/tag_alias_test.rb b/test/unit/tag_alias_test.rb index c5a702f0f..d32c04185 100644 --- a/test/unit/tag_alias_test.rb +++ b/test/unit/tag_alias_test.rb @@ -62,14 +62,20 @@ class TagAliasTest < ActiveSupport::TestCase end end - should "not push the antecedent's category to the consequent if the antecedent is general" - - should "push the antecedent's category to the consequent" do + should "not push the antecedent's category to the consequent if the antecedent is general" do tag1 = FactoryGirl.create(:tag, :name => "aaa") tag2 = FactoryGirl.create(:tag, :name => "bbb", :category => 1) ta = FactoryGirl.create(:tag_alias, :antecedent_name => "aaa", :consequent_name => "bbb") tag2.reload assert_equal(1, tag2.category) end + + should "push the antecedent's category to the consequent" do + tag1 = FactoryGirl.create(:tag, :name => "aaa", :category => 1) + tag2 = FactoryGirl.create(:tag, :name => "bbb") + ta = FactoryGirl.create(:tag_alias, :antecedent_name => "aaa", :consequent_name => "bbb") + tag2.reload + assert_equal(1, tag2.category) + end end end